diff --git a/src/readme2demo/cli.py b/src/readme2demo/cli.py index 4800c46..bb92f2c 100644 --- a/src/readme2demo/cli.py +++ b/src/readme2demo/cli.py @@ -17,6 +17,7 @@ import sys from pathlib import Path import json +from difflib import get_close_matches from typing import Any, Optional import typer @@ -141,14 +142,31 @@ def _load_config(config_file: Optional[Path], **overrides: Any) -> Config: location = ".".join(str(part) for part in error["loc"]) source = config_file or Path("readme2demo.toml") if error["type"] == "extra_forbidden": + # Exclude deprecated no-op shims (e.g. vhs_image) from suggestions. + valid_keys = sorted( + k for k, f in Config.model_fields.items() if not f.exclude + ) + suggestion = get_close_matches(location, valid_keys, n=1, cutoff=0.6) + hint = ( + f" Did you mean '{escape(suggestion[0])}'?" + if suggestion + else f" Valid keys: {escape(', '.join(valid_keys))}." + ) console.print( f"[red]Unknown config key '{escape(location)}' in " - f"{escape(str(source))}.[/]" + f"{escape(str(source))}.{hint}[/]" ) else: + bad_input = error.get("input", None) + value_bit = ( + f" (got {escape(repr(bad_input)[:120])})" + if bad_input is not None + else "" + ) + key_bit = f" for '{escape(location)}'" if location else "" console.print( - f"[red]Invalid configuration in {escape(str(source))}: " - f"{escape(error['msg'])}.[/]" + f"[red]Invalid configuration in {escape(str(source))}{key_bit}: " + f"{escape(error['msg'])}{value_bit}.[/]" ) raise typer.Exit(2) from None @@ -615,6 +633,9 @@ def _drive(orch: Orchestrator) -> None: except PipelineError as e: console.print(f"[red]Pipeline stopped:[/] {escape(str(e))}") console.print(escape(summarize(orch.manifest))) + console.print( + f"[dim]Fix the cause, then: readme2demo resume {escape(str(orch.run_dir))}[/]" + ) raise typer.Exit(1) except Exception as e: # noqa: BLE001 — stage errors are already in the manifest console.print(f"[red]{type(e).__name__}:[/] {escape(str(e))}") @@ -637,6 +658,10 @@ def _drive(orch: Orchestrator) -> None: f"\n[bold yellow]⚠ Completed UNVERIFIED.[/] " f"See {escape(str(orch.run_dir))}/verify.log" ) + console.print( + f"[dim]Retry after fixes: readme2demo resume " + f"{escape(str(orch.run_dir))} --from-stage distill[/]" + ) if __name__ == "__main__": diff --git a/src/readme2demo/llm.py b/src/readme2demo/llm.py index a75c916..2e1e84e 100644 --- a/src/readme2demo/llm.py +++ b/src/readme2demo/llm.py @@ -88,6 +88,10 @@ class LLMError(RuntimeError): _backend: Backend = "auto" _CLI_TIMEOUT_S = 600 +_CLI_NEXT_STEP_HINT = ( + " Check login with `claude -p hello`, or switch to " + "`--llm-backend api` with ANTHROPIC_API_KEY set." +) @dataclass(frozen=True) @@ -313,17 +317,26 @@ def _complete_cli(system: str, user: str, model: str) -> LLMResponse: timeout=_CLI_TIMEOUT_S, errors="replace", env=_sanitized_env(), ) except subprocess.TimeoutExpired: - raise LLMError(f"claude -p timed out after {_CLI_TIMEOUT_S}s") from None + raise LLMError( + f"claude -p timed out after {_CLI_TIMEOUT_S}s." + _CLI_NEXT_STEP_HINT + ) from None if proc.returncode != 0: + detail = (proc.stderr or proc.stdout or "")[:500].strip() raise LLMError( - f"claude -p failed ({proc.returncode}): {(proc.stderr or proc.stdout)[:500]}" + f"claude -p failed ({proc.returncode}): {detail}." + _CLI_NEXT_STEP_HINT ) try: envelope = json.loads(proc.stdout) except json.JSONDecodeError as e: - raise LLMError(f"claude -p returned non-JSON output: {proc.stdout[:300]!r}") from e + raise LLMError( + f"claude -p returned non-JSON output: {proc.stdout[:300]!r}." + + _CLI_NEXT_STEP_HINT + ) from e if envelope.get("is_error"): - raise LLMError(f"claude -p reported an error: {envelope.get('result', '')[:500]}") + detail = str(envelope.get("result", "") or "")[:500] + raise LLMError( + f"claude -p reported an error: {detail}." + _CLI_NEXT_STEP_HINT + ) return LLMResponse( text=envelope.get("result", ""), cost_usd=float(envelope.get("total_cost_usd") or 0.0), diff --git a/src/readme2demo/orchestrator.py b/src/readme2demo/orchestrator.py index 10068e5..ede2a61 100644 --- a/src/readme2demo/orchestrator.py +++ b/src/readme2demo/orchestrator.py @@ -210,7 +210,9 @@ def _stage_normalize(self) -> None: ) if self.cfg.budget_usd and cost > self.cfg.budget_usd: raise PipelineError( - f"Agent cost ${cost:.2f} exceeded budget ${self.cfg.budget_usd:.2f}" + f"Agent cost ${cost:.2f} exceeded budget ${self.cfg.budget_usd:.2f}. " + "Raise budget_usd in readme2demo.toml (or --budget-usd on a fresh run), " + "then resume this run — the agent work is already saved." ) def _stage_distill(self, feedback: str = "") -> None: diff --git a/tests/test_cli.py b/tests/test_cli.py index 27e7a62..731ab4d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -299,7 +299,9 @@ def test_run_rejects_missing_config_file_before_preflight(tmp_path): result = runner.invoke(app, ["run", _URL, "--config", str(missing)]) assert result.exit_code != 0 - assert "does not exist" in result.output + # Rich may soft-wrap long absolute paths; compare normalized output. + normalized = result.output.replace("│", " ") + assert "does not exist" in " ".join(normalized.split()) def test_run_reports_unknown_config_key_without_traceback(tmp_path): @@ -335,7 +337,9 @@ def test_resume_rejects_missing_run_dir(tmp_path): missing = tmp_path / "missing-run" result = runner.invoke(app, ["resume", str(missing)]) assert result.exit_code != 0 - assert "does not exist" in result.output + # Rich may soft-wrap long absolute paths; compare normalized output. + normalized = result.output.replace("│", " ") + assert "does not exist" in " ".join(normalized.split()) def test_resume_rejects_file_run_dir(tmp_path): @@ -652,3 +656,92 @@ def test_report_json_and_markdown_are_mutually_exclusive(tmp_path): # A usage error, not silent precedence: neither format was emitted. assert "Verified" not in result.output assert '"stages"' not in result.output + + +def test_invalid_config_type_names_key_and_value(tmp_path): + """Regression: type errors must name the offending key and value.""" + config_file = tmp_path / "readme2demo.toml" + config_file.write_text('max_turns = "sixty"\n', encoding="utf-8") + + result = runner.invoke(app, ["run", _URL, "--config", str(config_file)]) + + assert result.exit_code == 2 + assert "max_turns" in result.output + assert "sixty" in result.output + + +def test_unknown_config_key_suggests_nearest_match(tmp_path): + """Regression: unknown keys should suggest a nearest valid key name.""" + config_file = tmp_path / "readme2demo.toml" + config_file.write_text("max_turn = 99\n", encoding="utf-8") + + result = runner.invoke(app, ["run", _URL, "--config", str(config_file)]) + + assert result.exit_code == 2 + assert "Unknown config key 'max_turn'" in result.output + assert "max_turns" in result.output # nearest match / valid key + + +def test_config_type_error_value_with_brackets_is_escaped(tmp_path): + """Regression: bracketed config *values* must survive escape(repr(...)). + + Failure class 15: escape(repr(x)) is the safe order. This pins the new + value_bit path, not the unknown-key path already covered above. + """ + config_file = tmp_path / "readme2demo.toml" + # max_turns expects int; a bracketed string hits the type-error branch. + config_file.write_text('max_turns = "[bold]sixty[/bold]"\n', encoding="utf-8") + + result = runner.invoke(app, ["run", _URL, "--config", str(config_file)]) + + assert result.exit_code == 2 + assert "max_turns" in result.output + # Literal brackets from the value must appear (not be swallowed by Rich). + assert "[bold]sixty[/bold]" in result.output + assert "Traceback" not in result.output + + +def test_pipeline_error_prints_resume_hint(tmp_path, monkeypatch): + """Regression: PipelineError path must print an explicit resume command.""" + from readme2demo.orchestrator import Orchestrator, PipelineError + from readme2demo.manifest import Manifest + + run_dir = tmp_path / "r1" + Manifest.create(run_dir, _URL, "claude-code", "img").save() + + def boom(self): + raise PipelineError("synthetic stop for resume-hint test") + + monkeypatch.setattr(Orchestrator, "run", boom) + monkeypatch.setattr("readme2demo.cli._preflight", lambda cfg: None) + + result = runner.invoke(app, ["resume", str(run_dir)]) + assert result.exit_code == 1 + assert "Pipeline stopped" in result.output + assert "readme2demo resume" in result.output + # Rich may soft-wrap long absolute paths; compare collapsed whitespace. + collapsed = " ".join(result.output.split()) + assert str(run_dir) in collapsed or run_dir.name in result.output + + +def test_unverified_completion_prints_from_stage_distill(tmp_path, monkeypatch): + """Regression: UNVERIFIED completion must suggest resume --from-stage distill.""" + from readme2demo.orchestrator import Orchestrator + from readme2demo.manifest import Manifest + + run_dir = tmp_path / "r2" + m = Manifest.create(run_dir, _URL, "claude-code", "img") + m.verified = False + m.save() + + def fake_run(self): + return self.manifest + + monkeypatch.setattr(Orchestrator, "run", fake_run) + monkeypatch.setattr("readme2demo.cli._preflight", lambda cfg: None) + + result = runner.invoke(app, ["resume", str(run_dir)]) + # Rich may soft-wrap long absolute paths; compare collapsed whitespace. + collapsed = " ".join(result.output.split()) + assert "--from-stage distill" in collapsed + assert "UNVERIFIED" in collapsed or "unverified" in collapsed.lower() diff --git a/tests/test_llm_backend.py b/tests/test_llm_backend.py index 3c1f2c0..3b15159 100644 --- a/tests/test_llm_backend.py +++ b/tests/test_llm_backend.py @@ -551,7 +551,7 @@ def test_cli_backend_error_envelope(monkeypatch): cmd, 0, stdout=_fake_cli_envelope("rate limited", is_error=True), stderr="" ), ) - with pytest.raises(LLMError, match="reported an error"): + with pytest.raises(LLMError, match=r"reported an error.*--llm-backend api"): llm._complete_cli("s", "u", "m") @@ -561,7 +561,7 @@ def test_cli_backend_nonzero_exit(monkeypatch): "readme2demo.llm.subprocess.run", lambda cmd, **kw: subprocess.CompletedProcess(cmd, 1, stdout="", stderr="boom"), ) - with pytest.raises(LLMError, match="failed"): + with pytest.raises(LLMError, match=r"failed.*--llm-backend api"): llm._complete_cli("s", "u", "m") @@ -975,3 +975,40 @@ def fake_run(cmd, **kw): render_mod.run_render(tmp_path, Config(allow_docker_socket=True)) assert "--group-add" in captured["cmd"] assert "999" in captured["cmd"] + + +def test_cli_timeout_error_includes_next_step_hint(monkeypatch): + """Regression: claude -p failures should hint login AND --llm-backend api.""" + import subprocess + from readme2demo import llm as llm_mod + + def boom(*a, **k): + raise subprocess.TimeoutExpired(cmd=["claude", "-p"], timeout=1) + + monkeypatch.setattr(llm_mod.shutil, "which", lambda _: "/usr/bin/claude") + monkeypatch.setattr(llm_mod.subprocess, "run", boom) + with pytest.raises( + llm_mod.LLMError, + match=r"timed out.*claude -p hello.*--llm-backend api", + ): + llm_mod._complete_cli("sys", "user", "") + + +def test_cli_nonjson_error_includes_next_step_hint(monkeypatch): + """Regression: non-JSON claude -p output carries the same next-step hint.""" + import subprocess + from readme2demo import llm as llm_mod + + monkeypatch.setattr(llm_mod.shutil, "which", lambda _: "/usr/bin/claude") + monkeypatch.setattr( + llm_mod.subprocess, + "run", + lambda *a, **k: subprocess.CompletedProcess( + ["claude"], 0, stdout="not-json{", stderr="" + ), + ) + with pytest.raises( + llm_mod.LLMError, + match=r"non-JSON.*--llm-backend api", + ): + llm_mod._complete_cli("sys", "user", "") diff --git a/tests/test_orchestrator.py b/tests/test_orchestrator.py index 72ba618..df5bb20 100644 --- a/tests/test_orchestrator.py +++ b/tests/test_orchestrator.py @@ -390,3 +390,41 @@ def test_summarize_markdown_escapes_table_breaking_error_text(): def test_summarize_markdown_no_artifacts_omits_section(): md = summarize_markdown(make_report_manifest(verified=False, stages={}), []) assert "**Artifacts**" not in md + + +def test_budget_exceeded_message_mentions_flag(tmp_path: Path, monkeypatch): + """Regression: budget stop names config remedy that works with resume.""" + from readme2demo import normalize as normalize_mod + from readme2demo.types import AgentResult, CommandLog + + cfg = Config(runs_dir=tmp_path, budget_usd=5.0) + orch = Orchestrator.new_run("https://github.com/x/y", cfg) + + def fake_ingest(repo_url, run_dir, model, **kwargs): + plan = make_plan() + (run_dir / "plan.json").write_text(plan.model_dump_json()) + return plan, "abc1234", 0.005 + + def fake_run_agent(run_dir, plan, engine, c): + (run_dir / "transcript.ndjson").write_text("") + return run_dir / "transcript.ndjson" + + def fake_normalize(path, engine, run_dir): + log = CommandLog( + engine="claude-code", + result=AgentResult(outcome="success", cost_usd=6.0), + ) + (run_dir / "command_log.json").write_text(log.model_dump_json()) + return log + + monkeypatch.setattr(ingest_mod, "ingest", fake_ingest) + monkeypatch.setattr("readme2demo.orchestrator.run_agent", fake_run_agent) + monkeypatch.setattr(normalize_mod, "normalize", fake_normalize) + + with pytest.raises(PipelineError, match="budget_usd") as excinfo: + orch.run() + msg = str(excinfo.value) + assert "resume" in msg.lower() + assert "readme2demo.toml" in msg + # --budget-usd is only for a *fresh* run, not resume + assert "fresh run" in msg