Fix/backend name as model spec - #57
Merged
Merged
Conversation
…said nothing
Two defects that hid each other. Both found by installing 0.1.2 from PyPI into a
clean venv and driving it against a real `claude` v2.1.220.
**`split_spec` only consulted BACKENDS when the spec contained a slash.** A bare
backend name fell through to "assume the default backend, keep the whole string
as the model", so:
split_spec("claude-cli") -> ("claude-cli", "claude-cli")
split_spec("mock") -> ("claude-cli", "mock")
`--model claude-cli` therefore shelled out to `claude -p --model claude-cli` and
was refused by the CLI on every call (3/3), while `grapharc models --check` went
on reporting the backend `usable` and `grapharc models claude-cli` printed
`model: claude-cli` without complaint — two commands whose job is to say whether
a spec will work, both saying yes about one that never did.
`--model mock` was the worse half: it named the *paid* subscription backend and
spawned the real binary, so the double `models --check` describes as "scripted
test double; never reaches a provider" reached for one. It happened to fail
before billing only because `mock` is not a model name.
This is the same "silently folded into a model name … fails much later with a
confusing error" failure `split_spec` already refuses for a mistyped backend
*with* a slash; it just could not see the case without one.
A bare backend name now resolves to that backend. `claude-cli` takes its own
default model and `mock` takes the scripted double (which ignores the model
segment entirely). `openrouter`, `openai` and `ollama` front catalogues rather
than a model, so a bare name there is refused with a spelling that works rather
than a guess about what to bill someone for. Slash forms and bare *model* names
are untouched.
**The gateway read the wrong stream.** `claude -p` fails with a non-zero exit, an
empty stderr, and its whole explanation in the JSON envelope on stdout:
{"is_error": true, "result": "There's an issue with the selected model
(claude-cli). It may not exist or you may not have access to it."}
`_invoke_cli` reported `proc.stderr`, so the error was `claude -p exited 1: ` —
a sentence that stops at the colon. That is how the bug above presented itself:
as no message at all. The one string that would have diagnosed it in seconds was
captured, held in `proc.stdout`, and discarded. Note the code already parses this
shape a few lines below, but only on the `returncode == 0` path, and the CLI sets
`is_error` *and* exits non-zero, so it took the branch that ignores stdout.
stdout is read first now, falling back to stderr when it is not the documented
envelope. The recovered text also feeds `_cli_failure`, which classifies
transient-vs-deterministic and was previously classifying from `""`.
Tests: every new test was confirmed to fail against the old code and pass against
the new — including the mock one, which asserts no subprocess is *created* rather
than just checking the returned type, since the type was what was wrong and a
refactor could fix the type and still shell out. `BARE_BACKEND_MODEL["claude-cli"]`
is a second copy of the model class's default so the registry need not import a
backend to split a string, and a test pins the two together the way CI already
pins `__version__` to the packaged version.
Verified: `grapharc demo stage1 --model claude-cli` now completes (8 nodes,
target_met, exit 0) where it previously died with an empty error; `--model mock`
resolves to the double with no subprocess; `--model openrouter` exits 2 with an
example. Full suite green on 3.12 and 3.13; ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… in a CHANGELOG
The prose was not the problem — it is dense, specific and honest, and it is
lifted verbatim here. The shape around it was:
**Two sections named some spelling of "quickstart".** `## Quick Start` (a Python
graph) sat at the top and `## Quickstart` (the CLI tour) sat 100 lines below it,
so the page opened on the smaller of two competing answers to the same question,
and each linked to the other. The CLI tour is what actually demonstrates the
project — eight demo stages, the governed planner, the observability commands,
none of which need an API key — so it leads now, and the Python graph follows it
as a single merged `## Quick start`. That also removes a third copy of the same
idea: the CLI section already ended with its own "Building a graph" snippet
nearly identical to the one at the top.
**A bullet list restating a table, and restating itself.** "What makes GraphARC
different" listed six claims already made — better, with mechanisms — by the
"What it adds on top of LangGraph" table further down. Two of its six bullets
("Admission gate" and "Governed topology") were the same claim written twice.
Dropped; the table is the version worth keeping.
**Internals before installation.** The order was Quick Start, Architecture, Core
Components, What it adds, Install, Quickstart — a reader met the module layout
two sections before being told how to install anything. Now: Install, Quick
start, the admission gate, what it adds, architecture, then subsystems.
**A hand-maintained contents list in a different order from the document.**
GitHub renders its own outline from the headings; a second one that had already
drifted (it was missing eleven sections and, until recently, invented two) is a
maintenance burden with no reader benefit. Removed. `Architecture` and `Core
Components` were the diagram and a table of the same system, so they are one
section.
**Seven `*Closed:*` changelog entries filed under "Status and limits".** That
made the list a mix of "still true" and "used to be true", which is the one
distinction a reader of a limits section needs. They move to CHANGELOG.md in
full — they are good engineering history and none of it is lost — and the
section now says only what is still the case.
Also fixed an inaccuracy the restructure surfaced: "the `run` stages use
scripted models" described `grapharc demo`, not `grapharc run`, which is a
different command.
Tests. `test_the_quick_start_block_actually_runs_against_this_tree` asserted
`== ["python"]` on the section's blocks, which encoded the old layout; it now
selects the Python block by language, so the section can lead with bash and the
snippet is still executed and compared against the result printed on the page.
The contents-list link check became `test_every_in_page_link_resolves_to_a_real_heading`
— it no longer depends on a section that no longer exists, and it covers every
in-page link rather than only the ones in that list, which is strictly more than
before.
546 -> 504 README lines with no prose deleted, only moved or de-duplicated.
Full suite green on 3.12; ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.