From 1cf2e3dd7bee50e63202114317073ff54d5b62f9 Mon Sep 17 00:00:00 2001 From: opal Date: Fri, 22 May 2026 16:31:04 -0500 Subject: [PATCH] feat(cli): expose result_id + saved_path in --save --output-format json (sy-659) Closes #471 Panel runs with --save and --output-format json now include a stable machine-readable handle on stdout: { "result_id": "result-...", "saved_path": "/.../.synthpanel/results/result-....json", ... } Agents driving follow-up commands (report/inspect/analyze, MCP tools) can read the handle directly from the JSON payload instead of scraping the "Result saved: " line from stderr. Human-oriented stderr output is unchanged. The persistence call moved ahead of emit() in both the main panel-run path and the ensemble (--models a,b) path so the handle can land in the JSON payload before it ships. Added a public get_result_path() helper in synth_panel.mcp.data so call sites derive the path through the canonical layout function instead of duplicating it. Tests: 3 new in TestPanelRunSave covering --save+json (single model and ensemble) and the no-save baseline. All 175 CLI tests pass. --- README.md | 19 ++++ src/synth_panel/cli/commands.py | 86 ++++++++++-------- src/synth_panel/cli/parser.py | 5 +- src/synth_panel/mcp/data.py | 11 +++ tests/test_cli.py | 154 ++++++++++++++++++++++++++++++++ 5 files changed, 237 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 346e849b..71a395a9 100644 --- a/README.md +++ b/README.md @@ -566,6 +566,25 @@ synthpanel report path/to/result.json synthpanel report -o report.md ``` +Agents driving follow-up commands programmatically should combine +`--save` with `--output-format json` — the stdout JSON payload then +includes a `result_id` and `saved_path` for the persisted file: + +```bash +synthpanel --output-format json panel run \ + --personas examples/personas.yaml \ + --instrument examples/survey.yaml \ + --save \ + | jq '{result_id, saved_path}' +# { +# "result_id": "result-20260522-211748-a1b2c3", +# "saved_path": "/Users/you/.synthpanel/results/result-20260522-211748-a1b2c3.json" +# } +``` + +The same handle is still printed as `Result saved: ` on +stderr, so interactive callers see it without parsing JSON. + Every rendered report opens with a mandatory synthetic-panel banner and closes with a matching footer so the output can't be mistaken for real-user research: diff --git a/src/synth_panel/cli/commands.py b/src/synth_panel/cli/commands.py index 267226c0..422867e0 100644 --- a/src/synth_panel/cli/commands.py +++ b/src/synth_panel/cli/commands.py @@ -1538,7 +1538,6 @@ def system_prompt_fn(persona: dict) -> str: template_vars=template_vars or None, panelist_per_model=ens_per_model_meta, ) - emit(fmt, message="Ensemble complete", extra=output) # hq-0pnq: --save was a silent no-op in the ensemble path because # this branch returned before the persistence block at the end of @@ -1546,8 +1545,11 @@ def system_prompt_fn(persona: dict) -> str: # standard saved-result shape so consumers (`synthpanel inspect`, # `synthpanel analyze`) see ensemble runs the same way they see # single-model runs. + # sy-659: persist before emit so JSON output can include + # ``result_id`` + ``saved_path`` without forcing agents to scrape + # stderr. if getattr(args, "save", False): - from synth_panel.mcp.data import save_panel_result + from synth_panel.mcp.data import get_result_path, save_panel_result inst_name: str | None = None inst_arg = getattr(args, "instrument", None) @@ -1560,7 +1562,7 @@ def system_prompt_fn(persona: dict) -> str: for pr in mr.panelist_results: ensemble_results.append(_fmt_panelist(pr, mr.model)) - result_id = save_panel_result( + ens_result_id = save_panel_result( results=ensemble_results, model=ensemble_models[0], total_usage=ens_result.total_usage.to_dict(), @@ -1570,8 +1572,11 @@ def system_prompt_fn(persona: dict) -> str: instrument_name=inst_name, models=list(ensemble_models), ) - print(f"Result saved: {result_id}", file=sys.stderr) + output["result_id"] = ens_result_id + output["saved_path"] = str(get_result_path(ens_result_id)) + print(f"Result saved: {ens_result_id}", file=sys.stderr) + emit(fmt, message="Ensemble complete", extra=output) return 0 # ── sp-inline-calibration (sp-a6jc): --calibrate-against validation ── @@ -2363,6 +2368,41 @@ def _on_complete(pr: PanelistResult) -> None: convergence_report["human_baseline_error"] = convergence_baseline_error convergence_tracker.close() + # ── sy-659: persist before output emission ─────────────────────── + # save runs ahead of the text/json split so machine-readable output + # can include ``result_id`` + ``saved_path``. Stderr still gets the + # human-readable "Result saved" line so interactive callers see it. + saved_result_id: str | None = None + saved_result_path: str | None = None + if getattr(args, "save", False): + from synth_panel.mcp.data import get_result_path, save_panel_result + + inst_name = None + inst_arg = getattr(args, "instrument", None) + if inst_arg: + inst_path = Path(inst_arg) + inst_name = inst_path.stem if inst_path.exists() else inst_arg + + all_models: list[str] | None = None + if persona_models: + all_models = sorted(set(persona_models.values())) + elif model_spec: + all_models = [m for m, _w in model_spec] + + saved_result_id = save_panel_result( + results=results, + model=model, + total_usage=total_usage.to_dict(), + total_cost=total_cost_est.format_usd(), + persona_count=len(personas), + question_count=len(questions), + instrument_name=inst_name, + models=all_models, + synthesis=synthesis_dict, + ) + saved_result_path = str(get_result_path(saved_result_id)) + print(f"Result saved: {saved_result_id}", file=sys.stderr) + if fmt is OutputFormat.TEXT: if banner: print(banner, file=sys.stderr) @@ -2666,6 +2706,11 @@ def _cli_panelist_formatter(pr: PanelistResult, panel_model: str) -> dict[str, A } if run_invalid or strict_violation: extra["message"] = banner.replace("\n", " ").strip() if banner else "PANEL RUN INVALID" + # sy-659: surface saved result handle so agents can drive follow-up + # commands (report/inspect/analyze) without scraping stderr. + if saved_result_id is not None: + extra["result_id"] = saved_result_id + extra["saved_path"] = saved_result_path emit(fmt, message=extra.get("message", "Panel complete"), extra=extra) # ── sp-ezz: opt-in SynthBench submission ───────────────────────── @@ -2714,39 +2759,6 @@ def _cli_panelist_formatter(pr: PanelistResult, panel_model: str) -> dict[str, A file=sys.stderr, ) - # ── sp-7vp: auto-save results with --save ───────────────────────── - if getattr(args, "save", False): - from synth_panel.mcp.data import save_panel_result - - # Determine instrument name (pack name or filename stem) - inst_name = None - inst_arg = getattr(args, "instrument", None) - if inst_arg: - inst_path = Path(inst_arg) - if inst_path.exists(): - inst_name = inst_path.stem - else: - inst_name = inst_arg # pack name - - all_models: list[str] | None = None - if persona_models: - all_models = sorted(set(persona_models.values())) - elif model_spec: - all_models = [m for m, _w in model_spec] - - result_id = save_panel_result( - results=results, - model=model, - total_usage=total_usage.to_dict(), - total_cost=total_cost_est.format_usd(), - persona_count=len(personas), - question_count=len(questions), - instrument_name=inst_name, - models=all_models, - synthesis=synthesis_dict, - ) - print(f"Result saved: {result_id}", file=sys.stderr) - # sp-2hg: exit non-zero when the run is invalid so automation (CI, # refinery, wrapper scripts) can detect silent-failure scenarios. if strict_violation: diff --git a/src/synth_panel/cli/parser.py b/src/synth_panel/cli/parser.py index 48e4a0a5..bd591239 100644 --- a/src/synth_panel/cli/parser.py +++ b/src/synth_panel/cli/parser.py @@ -388,7 +388,10 @@ def build_parser() -> argparse.ArgumentParser: help=( "Save panel results to ~/.synthpanel/results/ with a generated " "result ID. The result ID is printed to stderr on completion and " - "can be passed to 'synthpanel analyze '." + "can be passed to 'synthpanel analyze '. With " + "--output-format json, the stdout payload also includes " + "'result_id' and 'saved_path' so agents can drive follow-up " + "commands without scraping stderr." ), ) panel_run_parser.add_argument( diff --git a/src/synth_panel/mcp/data.py b/src/synth_panel/mcp/data.py index c1fa99cf..1770e78b 100644 --- a/src/synth_panel/mcp/data.py +++ b/src/synth_panel/mcp/data.py @@ -75,6 +75,17 @@ def _results_dir() -> Path: return d +def get_result_path(result_id: str) -> Path: + """Return the absolute filesystem path where ``result_id`` is persisted. + + The path is the same one :func:`save_panel_result` writes to and + :func:`get_panel_result` reads from. Exposed so machine-readable CLI + output (``--output-format json --save``) can emit ``saved_path`` + without scraping stderr or duplicating the layout logic. + """ + return _results_dir() / f"{result_id}.json" + + # --------------------------------------------------------------------------- # Bundled packs (shipped with the package) # --------------------------------------------------------------------------- diff --git a/tests/test_cli.py b/tests/test_cli.py index ad95e073..35b57f8c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -4,6 +4,7 @@ import io import json +from pathlib import Path from unittest.mock import MagicMock, patch import pytest @@ -1555,6 +1556,159 @@ def _fake_run_parallel(**kwargs): models_seen = {r.get("model") for r in data["results"]} assert models_seen == {"haiku", "sonnet"} + # ── sy-659: --save + --output-format json surfaces a saved handle ── + + @patch("synth_panel.cli.commands.synthesize_panel") + @patch("synth_panel.orchestrator.AgentRuntime") + @patch("synth_panel.cli.commands.LLMClient") + def test_save_json_output_includes_result_id_and_saved_path( + self, mock_client_cls, mock_runtime_cls, mock_synth, capsys, tmp_path, monkeypatch + ): + """sy-659: ``--save --output-format json`` puts the saved handle on stdout. + + Agents driving follow-up commands (report/inspect/analyze) must be able + to read ``result_id`` + ``saved_path`` from the machine-readable stdout + payload without scraping the stderr "Result saved: ..." line. + """ + mock_runtime = MagicMock() + mock_runtime.run_turn.return_value = _mock_turn_summary("answer") + mock_runtime_cls.return_value = mock_runtime + mock_synth.return_value = _mock_synthesis_result() + + monkeypatch.setenv("SYNTH_PANEL_DATA_DIR", str(tmp_path / "data")) + + personas_file = tmp_path / "personas.yaml" + personas_file.write_text("personas:\n - name: Alice\n age: 30\n") + survey_file = tmp_path / "survey.yaml" + survey_file.write_text("instrument:\n questions:\n - text: What do you think?\n") + + code = main( + [ + "--output-format", + "json", + "panel", + "run", + "--personas", + str(personas_file), + "--instrument", + str(survey_file), + "--save", + ] + ) + assert code == 0 + captured = capsys.readouterr() + + # stdout MUST be a single JSON object carrying the handle. + data = json.loads(captured.out) + assert "result_id" in data, f"result_id missing from JSON output: {data!r}" + assert "saved_path" in data, f"saved_path missing from JSON output: {data!r}" + assert data["result_id"].startswith("result-") + # The path keeps pointing at the file the loader will open. + result_file = Path(data["saved_path"]) + assert result_file.exists(), f"saved_path {result_file} does not exist" + assert result_file == tmp_path / "data" / "results" / f"{data['result_id']}.json" + + # And stderr still carries the human-readable line so interactive + # shells aren't suddenly silent. + assert f"Result saved: {data['result_id']}" in captured.err + + @patch("synth_panel.cli.commands.synthesize_panel") + @patch("synth_panel.orchestrator.AgentRuntime") + @patch("synth_panel.cli.commands.LLMClient") + def test_json_output_without_save_omits_result_id_and_saved_path( + self, mock_client_cls, mock_runtime_cls, mock_synth, capsys, tmp_path, monkeypatch + ): + """sy-659: ``--output-format json`` (no --save) must not invent a handle. + + Skipping ``--save`` means nothing was persisted; the JSON output must + not advertise a handle that points at a file the agent can't load. + """ + mock_runtime = MagicMock() + mock_runtime.run_turn.return_value = _mock_turn_summary("answer") + mock_runtime_cls.return_value = mock_runtime + mock_synth.return_value = _mock_synthesis_result() + + monkeypatch.setenv("SYNTH_PANEL_DATA_DIR", str(tmp_path / "data")) + + personas_file = tmp_path / "personas.yaml" + personas_file.write_text("personas:\n - name: Alice\n age: 30\n") + survey_file = tmp_path / "survey.yaml" + survey_file.write_text("instrument:\n questions:\n - text: What do you think?\n") + + code = main( + [ + "--output-format", + "json", + "panel", + "run", + "--personas", + str(personas_file), + "--instrument", + str(survey_file), + ] + ) + assert code == 0 + data = json.loads(capsys.readouterr().out) + assert "result_id" not in data + assert "saved_path" not in data + + @patch("synth_panel.ensemble.run_panel_parallel") + @patch("synth_panel.cli.commands.LLMClient") + def test_save_json_output_in_ensemble_mode_includes_handle( + self, mock_client_cls, mock_rpp, capsys, tmp_path, monkeypatch + ): + """sy-659: ensemble path also surfaces ``result_id`` + ``saved_path`` in JSON.""" + from synth_panel.cost import TokenUsage as CostTokenUsage + from synth_panel.orchestrator import PanelistResult + + def _fake_run_parallel(**kwargs): + model = kwargs["model"] + personas = kwargs["personas"] + results = [ + PanelistResult( + persona_name=p["name"], + responses=[{"question": "Q1", "response": f"answer from {model}", "error": False}], + usage=CostTokenUsage(input_tokens=100, output_tokens=50), + model=model, + ) + for p in personas + ] + sessions = {p["name"]: MagicMock() for p in personas} + return results, MagicMock(), sessions + + mock_rpp.side_effect = _fake_run_parallel + + monkeypatch.setenv("SYNTH_PANEL_DATA_DIR", str(tmp_path / "data")) + + personas_file = tmp_path / "personas.yaml" + personas_file.write_text("personas:\n - name: Alice\n age: 30\n - name: Bob\n age: 25\n") + survey_file = tmp_path / "survey.yaml" + survey_file.write_text("instrument:\n questions:\n - text: What do you think?\n") + + code = main( + [ + "--output-format", + "json", + "panel", + "run", + "--personas", + str(personas_file), + "--instrument", + str(survey_file), + "--models", + "haiku,sonnet", + "--save", + ] + ) + assert code == 0 + data = json.loads(capsys.readouterr().out) + assert "result_id" in data + assert "saved_path" in data + assert data["result_id"].startswith("result-") + result_file = Path(data["saved_path"]) + assert result_file.exists() + assert result_file == tmp_path / "data" / "results" / f"{data['result_id']}.json" + class TestPanelRunBlendEnsemble: """hq-hjq8: ``--blend`` + ensemble-shape ``--models`` must produce