A bare backend name was read as a model name, and the CLI's failures … - #56
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>
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
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.
…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
claudev2.1.220.split_speconly 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:--model claude-clitherefore shelled out toclaude -p --model claude-cliand was refused by the CLI on every call (3/3), whilegrapharc models --checkwent on reporting the backendusableandgrapharc models claude-cliprintedmodel: claude-cliwithout complaint — two commands whose job is to say whether a spec will work, both saying yes about one that never did.--model mockwas the worse half: it named the paid subscription backend and spawned the real binary, so the doublemodels --checkdescribes as "scripted test double; never reaches a provider" reached for one. It happened to fail before billing only becausemockis not a model name.This is the same "silently folded into a model name … fails much later with a confusing error" failure
split_specalready 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-clitakes its own default model andmocktakes the scripted double (which ignores the model segment entirely).openrouter,openaiandollamafront 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 -pfails with a non-zero exit, an empty stderr, and its whole explanation in the JSON envelope on stdout:_invoke_clireportedproc.stderr, so the error wasclaude -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 inproc.stdout, and discarded. Note the code already parses this shape a few lines below, but only on thereturncode == 0path, and the CLI setsis_errorand 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-clinow completes (8 nodes, target_met, exit 0) where it previously died with an empty error;--model mockresolves to the double with no subprocess;--model openrouterexits 2 with an example. Full suite green on 3.12 and 3.13; ruff clean.