Skip to content

cli/orchestrator: six error messages say what failed but not what to do next #198

Description

@alphacrack

User story

As a first-time readme2demo operator, I want every error the CLI prints to end with the concrete next step (a flag to change, a command to run, a key to fix), so that I can unblock myself without reading the source.

What

An error-message actionability pass over exactly six user-facing failure sites. Each is a small, self-contained edit: keep the diagnosis, append the remedy.

The repo already has a house style for this — see the two exemplars under "Target quality bar" below. This issue is about bringing six laggards up to that bar, not inventing a new one.

The bounded list

1. src/readme2demo/cli.py:148-152 — the config type-error branch drops the key name.

else:
    console.print(
        f"[red]Invalid configuration in {escape(str(source))}: "
        f"{escape(error['msg'])}.[/]"
    )

location is computed at line 141 but used only in the extra_forbidden branch above it. So a readme2demo.toml containing max_turns = "sixty" prints:

Invalid configuration in readme2demo.toml: Input should be a valid integer, unable to parse string as an integer.

The user is not told which key is wrong. Fix: include location (and ideally the offending value) in this branch too — it is already in scope.

2. src/readme2demo/cli.py:143-147 — unknown config key is a dead end.

console.print(
    f"[red]Unknown config key '{escape(location)}' in "
    f"{escape(str(source))}.[/]"
)

Prints Unknown config key 'max_turn' in readme2demo.toml. and stops. Compare the sibling error in the same file at lines 448-451, which does the right thing:

f"[red]Unknown stage {escape(repr(from_stage))}. "
f"Stages: {', '.join(STAGES)}[/]"

Fix: append a nearest-match suggestion and/or the valid keys, sourced from Config.model_fields (pydantic v2) rather than a hand-maintained list — e.g. Did you mean 'max_turns'?.

3. src/readme2demo/cli.py:615-618 — the common failure path is the one without the resume hint.

except PipelineError as e:
    console.print(f"[red]Pipeline stopped:[/] {escape(str(e))}")
    console.print(escape(summarize(orch.manifest)))
    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))}")
    console.print(escape(summarize(orch.manifest)))
    console.print(
        f"[dim]Fix the cause, then: readme2demo resume {escape(str(orch.run_dir))}[/]"
    )

The generic-Exception branch tells the user how to resume. The PipelineError branch — which handles every deliberate stop (orchestrator.py:101 infeasible, :205 blocked, :207 agent failed, :212 over budget) — does not. Backwards: the expected failures get less help than the unexpected ones. Fix: hoist the resume line so both branches print it.

4. src/readme2demo/cli.py:636-639 — "UNVERIFIED" points at a log, not an action.

console.print(
    f"\n[bold yellow]⚠ Completed UNVERIFIED.[/] "
    f"See {escape(str(orch.run_dir))}/verify.log"
)

Reading verify.log is step one, but the user still has to know that the retry is readme2demo resume <run> --from-stage distill. Fix: add that line.

5. src/readme2demo/orchestrator.py:211-214 — budget stop names no flag.

raise PipelineError(
    f"Agent cost ${cost:.2f} exceeded budget ${self.cfg.budget_usd:.2f}"
)

The knob is --budget-usd (or budget_usd in toml) and the work is not lost — a resume is possible. Neither fact is stated. Contrast the message four lines above it (:207-210), which is already right: "Agent run failed (max turns / timeout). Inspect transcript.ndjson; retry with a higher --max-turns."

6. src/readme2demo/llm.py:315-326 — the three claude -p failures each dead-end.

except subprocess.TimeoutExpired:
    raise LLMError(f"claude -p timed out after {_CLI_TIMEOUT_S}s") from None
if proc.returncode != 0:
    raise LLMError(
        f"claude -p failed ({proc.returncode}): {(proc.stderr or proc.stdout)[:500]}"
    )
...
raise LLMError(f"claude -p returned non-JSON output: {proc.stdout[:300]!r}")
...
raise LLMError(f"claude -p reported an error: {envelope.get('result', '')[:500]}")

All four are diagnosis-only. The overwhelmingly common causes are "not logged in" and "plan usage cap hit", and the remedy is the same for all of them: verify the login with a bare claude -p hello, or switch to --llm-backend api with ANTHROPIC_API_KEY. One shared hint constant appended to these raises is enough.

Target quality bar (already-good messages — do not change these)

src/readme2demo/llm.py:181-184 (check_sdk, the defense from CLAUDE.md failure class 15) — names the package, the extra, and the exact command, with two sibling branches distinguishing absent from broken from too old:

raise LLMError(
    f"{pip_name} is not installed. Install the {spec.title} extra: "
    f"pip install 'readme2demo[{spec.extra}]' (or pip install {pip_name})."
) from e

src/readme2demo/engines/claude_code.py:199-206 (credential hygiene, failure class 12) — diagnosis, likely cause, and a literal three-line recipe:

raise EngineError(
    f"{var} is set but doesn't look like a credential "
    "(contains whitespace/control characters — likely captured "
    "interactive output). Fix:\n"
    f"  unset {var}\n"
    "  claude setup-token   # run it plain, approve in browser\n"
    f"  export {var}=<the sk-ant-... token it prints>"
)

Also worth reading before starting: llm.py:349-354 (_provider_model) and engines/claude_code.py:207-212.

Out of scope (fenced — please do not widen)

Why

Every message on the list sits on a path a new user hits in their first hour: a typo in readme2demo.toml, a first run that stops mid-pipeline, a first run that completes unverified, a first run that overspends the default budget_usd = 5.0, a first run on the claude-cli backend without a live login.

The repo already treats "the error must carry the fix" as a real invariant — CLAUDE.md failure class 15 exists precisely because a hint that was correct got mangled by Rich markup and shipped subtly wrong advice (pip install 'readme2demo' instead of pip install 'readme2demo[openai]'). Item 3 is the sharpest instance: the resume hint was written, and then attached to the wrong except branch, so the failures the pipeline raises on purpose are the ones that don't get it.

Grounding implication: none

No file on this list is in the grounding path. Nothing here can put an unverified command into tutorial.md, step_by_step.md, commands.sh, or demo.tape — these are console strings and PipelineError payloads on the failure path, and item 5's raise happens before any artifact is written. The out-of-scope fence above exists to keep it that way: it excludes exactly the modules whose strings the LLM reads.

Acceptance criteria

  • cli.py config type-error branch names the offending key (location) and the value.
  • cli.py unknown-config-key error suggests a nearest match and/or lists valid keys, derived from Config.model_fields (not a hardcoded list).
  • The readme2demo resume <run-dir> hint prints on the PipelineError branch of _drive, not just the generic-Exception branch.
  • The "Completed UNVERIFIED" tail names a concrete retry command (resume <run> --from-stage distill) alongside the verify.log pointer.
  • The budget-exceeded PipelineError names --budget-usd and states that the run is resumable.
  • The four claude -p failure messages in llm._complete_cli share one appended next-step hint (check the login, or --llm-backend api).
  • Every new interpolated value is wrapped in rich.markup.escape() at the console.print site, and where a value is repr()'d the escape is applied after the repr — see the comment already at cli.py:446-447: "escape AFTER repr — the other order re-breaks the markup (repr doubles escape()'s backslashes, reviving the swallowed tag)."
  • Correspondingly: text raised from llm.py / orchestrator.py stays unescaped at the raise site_preflight (cli.py:605) and _drive (cli.py:616,620) already call escape() when printing, and pre-escaping would double-escape. Only console.print call sites escape.
  • One regression test per changed message, each with a """Regression: ...""" docstring, asserting on the actionable substring (the flag or command name), not on incidental prose.
  • A markup test for at least one new hint containing brackets, following the existing test_unknown_config_key_is_escaped_for_rich_markup pattern (tests/test_cli.py:316-325).
  • No exit code changes; python -m pytest tests/ -q fully green.
  • ruff clean; type hints and docstrings preserved on anything touched.

Notes for contributors

Prerequisites: none beyond Python. Pure Python, no Docker, no API key, no network. Every one of these six sites is reachable in a unit test.

pip install -e ".[dev]"
python -m pytest tests/ -q     # CLAUDE.md states the suite runs in under a second

Where to work:

  • src/readme2demo/cli.py — items 1-4 (_load_config ~line 136, _drive ~line 609)
  • src/readme2demo/orchestrator.py — item 5 (~line 211)
  • src/readme2demo/llm.py — item 6 (_complete_cli, ~lines 315-326)

Where to test:

  • tests/test_cli.py — uses typer's CliRunner; the config-error tests at lines 303-325 are the closest models to copy, including the Rich-markup one.
  • tests/test_orchestrator.py — for item 5.
  • tests/test_llm_backend.py — for item 6; monkeypatch subprocess.run rather than invoking a real claude.

The suite runs without Docker or credentials, so verify locally before opening the PR. Items are independent — a PR doing a strict subset is welcome, just say which items it covers.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:clicli.py, config.py, UXenhancementNew feature or improvementgood first issueSmall, self-contained, newcomer-friendly

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions