feat(runner): a planned node answers with the model the operator chose - #248
Open
jitokim wants to merge 10 commits into
Open
feat(runner): a planned node answers with the model the operator chose#248jitokim wants to merge 10 commits into
jitokim wants to merge 10 commits into
Conversation
…operator chose `--setting-sources ""` withholds the whole of ~/.claude/settings.json from a planned node, because withholding the document is the only lever the CLI offers. The operator's model preference happens to live in that document, so a planned node answers with whatever the CLI defaults to — measured across this repository's corpus, 181 of 187 planned nodes ran a model nobody selected (docs/measurements/0034-planned-node-model.go). internal/usermodel reads exactly one key, `model`, out of $CLAUDE_CONFIG_DIR/settings.json (else ~/.claude/settings.json), into a struct with exactly one field, and returns it verbatim. No allowlist: an allowlist goes stale with the CLI's release cadence and would then substitute a default for a name the operator really chose, which is the defect under repair. Nothing about the ceiling moves. It bounds CAPABILITY — which tools a node holds, which files load into it, whose hooks run — and a model name grants none of those; it reaches argv, not a prompt, and cannot come from planner output (there is no `model` key in the graph schema). The package doc carries that argument, and the terms on which we parse a file another product owns: a renamed key or a changed type degrades to no flag, never to a guess. The five cases are the tests: key present (verbatim, "[1m]" suffix intact), key absent, blank value (absent — the CLI rejects an empty --model), file absent (not an error), malformed and unreadable (one error naming the path, never the contents — that file also holds credentials). Temp dirs throughout, never the real home. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com>
…an agent
NodeInvocation gains `Model string`, and claudeProtocol.buildArgs emits
`--model <value>` immediately after --permission-mode and before
--max-budget-usd — neither of those is a ceiling flag, so both sit ahead of the
ones that are. Emitted verbatim: the vocabulary belongs to the CLI, an unknown
name is a node failure carrying the CLI's own "Unknown model" stderr, and there
is no allowlist to go stale and substitute a default for a name the operator
really chose.
Two suppressions, both deliberate:
- empty Model emits nothing, so a node that expressed no choice keeps a
byte-identical argv and the CLI's own default. That is every hand-written
node and every machine with no settings key.
- an agent-mapped node gets no --model at all. `--agent` supplies a model
from the definition's frontmatter — 6 of the 187 planned nodes measured for
this change take that route — and the CLI tracks `modelCli` and
`agentSelectedByCli` as distinct sources with no documented winner. The
more specific choice wins by the flag being withheld, rather than by us
finding out which wins in production.
codexProtocol.buildArgs ignores Model, and now says why in place: Codex carries
the identical defect (--ignore-user-config withholds ~/.codex/config.toml,
where the operator's model lives) and the mechanism to fix it exists, but no
codex node's model is observable in this repository's corpus, so there is no
measured population to fix it for. That silence is now legible instead of
accidental, and pinned by a test.
DESIGN.md's argv block and claude_test.go's element-by-element `want` move
together, as that block requires.
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
… does not move
The coordinator reads the operator's model choice once per plan
(WithUserSettingsPath -> usermodel.Read), carries it on Plan.Model, and the CLI
hands it to schedule.Options.Model, which the scheduler forwards to every
NodeInvocation. A resumed planned leg re-reads the same file, because without
it the second leg reproduces the defect one leg late.
Byte-for-byte unchanged: SettingSources "", --allowedTools, --tools, the deny
list, --strict-mcp-config, enterprise policy, internal/childenv.Scrub, and the
count of exec seams. This is argv. A model name grants no tool, loads no file
and runs no hook — the assertion is a test, not a claim: the ceiling comparison
in TestPlan_ModelLeavesTheCeilingUntouched diffs every layer of a plan that read
a model against one that did not.
The three open questions, answered as ADR 0034 decides, each with its silence
made legible at the call site:
- planner and assessor: no --model. The planner already loads the operator's
settings (it sets no SettingSources), so there is nothing to fix; both are
left alone because THE ENGINE PARSES THESE REPLIES, and changing the model
behind a parser is a compatibility change wearing a preference's clothes.
coordinatorInvocation and assessorInvocation now say so.
- codex: no code, documented asymmetry. The defect is identical and the
mechanism is known, but no codex node's model is observable in this
repository's corpus, so it would be a fix for an unmeasured population.
docs/LIMITATIONS.md states it where the user meets it; #245 carries the
research.
- per-run flag: none. One surface, the settings file — a flag would need
persisting and re-applying on resume, or a resumed leg silently runs a
different model than its first, which is the class of bug being fixed.
Disclosure amended where it was made: toolPolicyFor's comment and the README
both promised a planned node loses that whole file, and one key of it now
arrives. The node's capability ceiling is unchanged; one preference crosses it,
by name, and only that one.
Measurement committed with the decision: 181/6 planned, 851/267 hand-written,
the contaminated directory census printed beside the state.json join so the
difference is visible, and the limit stated out loud — message.model is a
FAMILY census, so the [1m] half of the defect is neither confirmed nor refuted
by it. Re-running the program reproduced every node figure; recomputing the
aggregates off the raw rows a second way agrees.
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
Signed-off-by: jitokim <pigberger70@gmail.com>
…fter The Unreleased entry already said the key is read and passed as `--model`; it did not say, in those terms, what the node answered with BEFORE. "A model nobody selected" is the finding, not the mechanism — the mechanism is that the CLI falls back to its own default when no settings source and no flag names one. Both halves are now stated as a pair, and the AFTER half says verbatim out loud: no normalisation, no case-folding, and no stripping of a variant suffix like [1m], because that suffix is exactly the value an author of a well-meaning allowlist would drop. No release, no tag, no version bump; no released heading touched. Signed-off-by: jitokim <pigberger70@gmail.com> Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
…t one
SECURITY.md is the document the README sends a reader to for "the layer-by-layer
stance", and it was the file ADR 0034's own sweep missed: three passages here
enumerate what a planned node loses, and after the change one key of that file
does reach it. A sweep that stops at the README leaves the authoritative
statement contradicting the shipped argv.
Three sites, each stating the same distinction rather than repeating a feature
note:
- after the layer table, where the reader learns what the five layers ARE:
every row bounds CAPABILITY — grants, tools, settings, hooks, CLAUDE.md —
and `--model` grants none of them. Not one row moves. The value reaches
argv and not a prompt, and the graph schema has no `model` key, so no
planner output can select it. One key; a second needs its own ADR, because
the same file's permissions block holds the standing grants layer 1 exists
to withhold.
- "Planned nodes are more isolated and less capable": "less capable" is now
said to be about tools, files and hooks and never about which model does
the thinking — and the codex asymmetry is stated where that claim is made,
not left to LIMITATIONS.md alone.
- the mapped-node cost paragraph: an agent-mapped node is the one planned
node that gets NO --model, since its definition declares one. That is where
a reader asks, having just been told what mapping costs.
Documentation only; no code, no ceiling, no argv.
Signed-off-by: jitokim <pigberger70@gmail.com>
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
…a decision The argv block already carries `[ --model <value> ]` (3d5671a). The two places a reader goes to ask what that flag MEANS did not: - "The tool ceiling — one layered policy" is the enumeration of what bounds a planned node, and it now says what --model is not: not a row of the table, not a sixth layer, not even a ToolPolicy field — it rides on NodeInvocation beside the prompt. Every layer bounds capability; a model name binds no grant, adds no tool and loads no file, so ONE key crosses layer 1 without moving a row of it. Cited to the test that proves it rather than asserted: TestPlan_ModelLeavesTheCeilingUntouched diffs every layer of a plan that read a model against one that did not. - the Codex protocol block, whose argv listing has no --model beside a Claude block that now does. Left implied, that reads as an oversight. It is not: the defect is identical (--ignore-user-config withholds $CODEX_HOME/config.toml) and the mechanism exists (`-c model="…"`, the same override this protocol already uses for approval_policy) — what is missing is a measured population, because a codex thread writes no ~/.claude/projects transcript and the corpus therefore holds no codex node's model at all. Shipping it would be a fix for nobody we have counted. Documentation only. Code and DESIGN.md move together, as this file requires. Signed-off-by: jitokim <pigberger70@gmail.com> Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
README.md gained the paragraph in fd82b7a; README.ko.md, which mirrors the same "one boundary to read before you trust it" section for a Korean reader, did not. A Korean reader was left with the pre-change claim that a planned node keeps nothing of their configuration, which the shipped argv contradicts. Same three statements as the English paragraph, plus the codex half stated in place rather than by link alone: the model key is read on its own and passed as `--model`; the capability ceiling does not move, because a model name grants no tool, loads no file and runs no hook; nothing else in the file is read, and a `--runtime codex` run reads nothing at all, so its planned nodes answer with codex's own default. Documentation only. Signed-off-by: jitokim <pigberger70@gmail.com> Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
…oses
docs/EXAMPLES.md is the document internal/docsclaims exists because of — the
last sweep to condition an absolute conditioned it in DESIGN, SECURITY, both
READMEs and LIMITATIONS, and left this file stating it flat (see that package's
doc.go). Three passages here enumerate the loss, so three are amended:
- the "two things that come with it" bullet: your model choice is the one
thing a planned node keeps, so "less capable" is about tools, files and
hooks and never about which model answers. Claude-only, said there.
- the agent-mapping cost paragraph: a mapped node takes its model from the
agent file, and is the one planned node that gets no --model at all.
- the skill-activation "the tool ceiling does not move" claim: "load none of
your settings" now names the one key read back out of that file by name.
Each says the same distinction the ceiling rests on — capability versus
preference — rather than repeating the feature note.
Documentation only.
Signed-off-by: jitokim <pigberger70@gmail.com>
Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev>
…e-run moves The 0034 writeup already carried the question, the method, the two buckets and the [1m] limit. Two things it did not carry, both about the edges of the claim rather than the claim: WHAT IS NOT MEASURED, as a list rather than as prose scattered through the argument. Six lines, each marked, each with the address of the number on it: the [1m] context-window variant; a codex node's model (12 records join to no claude transcript, because a codex node writes none); the 77 records with no session_id and the 3 with no assistant record; whether the operator's settings held one model value across the corpus window; the hand-written bucket split by agent, which was never run; and whether ADR 0034's --model repairs any of it, which no run in this corpus is late enough to show. WHAT A RE-RUN NOW REPORTS. Re-running the program today does NOT reproduce the figures, and saying "re-running reproduced them" without a date would have gone stale silently. The runs directory grew: 45 -> 46 planned runs, 181 -> 186 planned claude-opus-5, 1397 -> 1402 raw rows. The hand-written bucket does not move by a single record (851/267, 1118 of 1202), and the whole delta is planned nodes this branch's own work created after the raw file was written. The document therefore states which column it commits to — the committed raw JSON, the one with a durable address — and gives the other as a dated observation. The re-run command in the file runs from a scratch directory on purpose: the program writes its raw output to a path relative to the working directory, so reproducing it inside the repo overwrites the very file the figures cite. Also stated plainly, because the writeup implied it without answering it: the hand-written bucket is NOT contaminated, by construction — it is reached only through state.json node records. The 1156 transcripts claimed by no graph node are the contamination a directory census would have added, measured instead of asserted. No number in this commit was re-derived without its command beside it, and no figure already committed was changed. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com>
…umber The Codex model-asymmetry follow-up was filed in the public tracker and then cited from six places. Our own follow-ups belong in the operator's private backlog (oh-my-graph-hq notes/open.md), so the citations now name that address instead of an issue number. Where a citation carried the research as well as the follow-up, the research now points at the place it actually lives — ADR 0034 section 2.6, or the comment above codexProtocol.buildArgs. No claim, wording or other address changed: the asymmetry, its one reason (no codex node's model is observable in this corpus) and the mechanism stay exactly as written. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
The maintainer's decision: inherit the user's setting. A planned node was answering with whatever the CLI defaults to, because
--setting-sources ""drops the file the operator'smodellives in — and oh-my-graph never passed--modelat all.The argument this rests on
The auto ceiling bounds capability: which tools a node holds, which files load, whose hooks run. Model choice grants none of those. Inheriting it does not weaken the ceiling; it separates two things that were only ever coupled by the bluntness of
--setting-sources "".The tension it had to resolve
Reading
~/.claude/settings.jsonmeans parsing a file another product owns — the same shape ADR 0009 refused for the CLI's prose reset time, and the #218 measurement refused for transcripts. The brief required an answer for all four failure cases (key absent, file absent, malformed, unknown value) with one rule fixed in advance:Status
10 commits,
make localgreen. Goal not met — read the run's own remaining note before merging; it is on the branch and this PR is not claiming it is finished work.🤖 Generated with Claude Code