docs: the example config names the model the project actually runs - #198
Conversation
`config.example.yaml` is what an operator copies to stand Iris up, and it
pinned Opus 4.7 on both providers. The reference deployment serves every
capability with Sonnet 4.6 and has for as long as there is a record, so
the file handed to a new operator configured a model this project does
not run and has never measured — at 1.67× the per-page cost of the
figures it publishes, for someone who changed nothing.
Both `default_model`s and both `per_capability` blocks now name Sonnet
4.6, with one note saying why in the one place it belongs: it is the
only model any published number was measured on, and `max_tokens` is our
setting rather than the model's — the overrun rate, the correction shrink
floor and the section headroom were all sized against how this model
behaves at that ceiling, so an operator on a different one is past the
measurements without being told. It also aligns what a copy of this file
publishes as an upload limit with what the deployment accepts, since the
long edge follows the model generation.
The commented `per_agent` examples stay, because they teach the
mechanism rather than a model — but the `page:` one named Sonnet as the
"cheaper/faster" override, which is a no-op now that Sonnet is the
default. It illustrates the stronger direction instead, and `table:`
takes over showing the provider+model form.
prd.md §10.3's block is illustration — it pins `openai/gpt-5` for one
capability and names agents that are not dispatched — so it says so, and
points at config.example.yaml as the file to copy. Its ids move to
Sonnet too, and its Bedrock one loses the bare `anthropic.` prefix that
the example config's own comment says is rejected for on-demand use.
`providers.default` stays `openrouter` against the deployment's
`bedrock`, deliberately: it decides which credential a new operator has
to obtain, and one API key with no cloud account is the shorter path to
a first run. The file now says that is the reason.
Verified: `loadConfig("config.example.yaml")` accepts the file and
resolves all eight ids. tsc clean, 765 tests pass, e2e ALL ENDPOINTS
PASSED.
Closes #176
Co-authored-by: bbertucc <46652+bbertucc@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Docs and example-config only — no src/, agents/ or .github/ changes — and all six checks pass. The model swap is correct and internally consistent; two notes below, neither blocking.
Verified rather than assumed:
- All eight new ids parse via
modelGeneration()to{major:4,minor:6}: at or aboveCACHING_FROM_GENERATION {3,7}so prompt caching stays on (src/providers/promptCache.ts:45,182), and belowHIGH_RES_FROM_GENERATION {4,7}solongEdgeFor()returns 1568 (src/providers/imageLimits.ts:55-57). No other generation-gated behaviour flips for an operator who copies the file. - No stale
opusreference left inprd.md,README.md,docs/oragents/.docs/API.md:365,386already publish 1568 px / Sonnet 4.6, so the example now agrees with the published docs instead of contradicting them — the old file madeGET /v1/limitspublish 2576 px for a copy of it. - Bedrock ids keep the
us.prefix the file's own comment (config.example.yaml:272-277) says is required for on-demand use, andprd.md:1108drops the bareanthropic.form. test/image-limits.test.ts:136("the config example recommends running verification on a stronger model than extraction") is now true rather than a no-op — the commentedfeedback:override was Opus 4.7 against an Opus 4.7 default before.
Non-blocking notes
1. config.example.yaml:183-185 overstates the change on the accept side.
```
tuned for. It also moves the upload limits this deployment publishes, since the
long edge follows the model generation (see image_limits below), so a copy of
this file now accepts exactly what the reference deployment accepts.
```
The long edge is advice, not a gate — line 219 of this same file says `max_long_edge_px: 1568 # published as advice; nothing is rejected for it`, and `src/providers/imageLimits.ts:47-52` says "Not an error, and deliberately not enforced". The two numbers an upload is actually rejected by are `max_image_bytes` (3,932,160, derived from the 5 MB base64 cap both providers in `BASE64_CAP_BY_PROVIDER` share) and `max_dimension_px` (8000, model-independent), and both are byte-for-byte identical before and after this diff. What moved is what `GET /v1/limits` and the upload hint publish: 2576 px → 1568 px. "publishes" is exact where "accepts" claims something the code does not do. Latent by construction — it is a comment, and the direction of the real change (advice down to the honest number) is the right one.
2. Nothing in the suite parses config.example.yaml.
Grep finds only README.md:71 and CONTRIBUTING.md:47 (cp config.example.yaml config.yaml) — which is why this PR's verification had to be a throwaway probe, as the description says. loadConfig already takes a path (src/config.ts:608), so a test that calls loadConfig(\"config.example.yaml\") and asserts resolveAgentModel for the dispatched agents would make that probe permanent; without it, an example config that is schema-invalid or has an unexpandable ${...} ships silently, and the operator hits it on their first run. A new test file also has to be registered in the test script in package.json or it never runs.
Accessibility impact: none — no code, agent prompt or output path changes; the only user-visible effect on a fresh copy of the example is that GET /v1/limits and the upload hint now publish the honest 1568 px long edge instead of 2576 px, which is advice about downscaling and rejects nothing.
…ng edge does
Two notes from the review of this branch.
The comment claimed a copy of this file "now accepts exactly what the
reference deployment accepts". It does not: the long edge is advice about
downscaling and rejects nothing, and the two numbers an upload is
actually refused by — the byte cap both providers share, and the
model-independent pixel ceiling — are identical before and after this
diff. What moved is what `GET /v1/limits` and the upload hint PUBLISH,
which is what the comment says now.
And nothing in the suite parsed `config.example.yaml`, which is why this
branch had to verify it with a throwaway probe. It is the file README.md
and CONTRIBUTING.md tell an operator to copy, so a renamed key, an
invalid block, or a `${VAR}` that no longer expands would ship green and
be found by the operator on their first run — the one run they cannot
debug. `test/config-example.test.ts` makes the probe permanent: it loads,
it needs exactly the one credential its `default` names, every dispatched
agent resolves to a model for every capability, and both blocks name one
model in the two id spellings the providers use.
That last assertion is on internal agreement rather than on a model name,
because the name changes and the agreement is the property #176 was
about. It also pins the two ways a wrong id is silent — an id
`modelGeneration` cannot read falls back to the oldest model's published
long edge, and a model outside the cache-eligible generations turns every
cached prefix into a full-price one, and only the bill says so. Checked
by mutation: pointing one of the eight ids at Opus 4.7 fails the run.
(No package.json change needed — the `test` script is a glob over
test/*.test.ts, so the file is picked up by being in the directory.)
Co-authored-by: bbertucc <46652+bbertucc@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both notes taken. 1. "accepts" was wrong — thanks. The long edge is advice and rejects nothing; the two numbers an upload is actually refused by (the shared byte cap and the model-independent pixel ceiling) are identical before and after this diff. The comment now says the long edge is what 2. The example config is now parsed by the suite —
Mutation-checked rather than assumed — pointing one of the eight ids at Opus 4.7 fails the run: One correction on the note: no
|
There was a problem hiding this comment.
All six checks pass. The real delta against main is config.example.yaml, prd.md §10.3, and test/config-example.test.ts — the src/pipeline/lint.ts / review.ts / test/lint-nodes.test.ts hunks in my context are #197, already merged as 9c499f0 (the diff base 5c0ef27 predates it), so I have not re-reviewed them here.
Both notes from my review of 9651f24 are addressed:
config.example.yaml:183-187now says what the change does: "moves the long edge ... PUBLISH" plus "Only what is published: the long edge is advice about downscaling and rejects nothing, and the two limits an upload is actually refused by do not follow the model at all." That is exact.- Nothing parsed the example;
test/config-example.test.tsnow does.
Withdrawing one half of that second note: I wrote "A new test file also has to be registered in the test script in package.json or it never runs." That is wrong for this repo — package.json:14 is node --test "test/*.test.ts", a glob. There was nothing to register.
Verified rather than assumed: the example carries 9 ${...} placeholders and only OPENROUTER_API_KEY is fatal when unset, because validateConfig checks only reachable providers (src/config.ts:349-351) and an unset variable expands to "" — so test 1's expected throw and test 2's successful load with one variable are both right, and the bedrock block is correctly not credential-checked. No stale opus id left in prd.md, README.md or docs/.
Non-blocking notes
1. The new test asserts internal agreement, not agreement with the numbers the docs publish.
test/config-example.test.ts checks one generation, one family, both id spellings, and cache eligibility across all eight ids. What it does not check is the one number a model swap moves outward. docs/API.md:365 ("1568 px on Sonnet 4.6") and docs/API.md:386 ("max_long_edge_px": 1568) are hardcoded, so a future bump of the example to a generation at or above HIGH_RES_FROM_GENERATION {4,7} makes GET /v1/limits publish 2576 for a fresh copy of the example while those two lines still say 1568 — the same doc-versus-deployment drift #176 was about, and all three new tests stay green through it (same generation, same family, cacheable, both prefix forms valid). One line — resolveImageLimits(loadConfig(EXAMPLE)).max_long_edge_px asserted against what docs/API.md prints — closes it. Latent: today the example and the docs agree.
2. test/config-example.test.ts:43-53 is vacuous while per_agent: {}.
const { provider, model } = resolveAgentModel(providers, agent, capability); assert.equal(provider, "openrouter", `${agent}/${capability} left the default provider`);
resolveAgentModel (src/providers/index.ts:35-41) returns cfg.default plus per_capability[capability] ?? default_model for any agent string when there is no override, so the AGENTS list — including whether copy_editor and builder are spelled the way the dispatcher spells them — is not what this exercises; a typo or a renamed agent passes identically. What actually makes it green is test 3's "both blocks fully populated". Worth keeping as a guard for the day per_agent is non-empty, but the comment above it ("so a resolution hole cannot hide behind an agent that happens not to ask for vision") claims more than the assertions can see.
3. config.example.yaml:164 and :170 now recommend against each other.
# page: { model: anthropic/claude-opus-4.7 } # stronger model, same provider ... # Run the Feedback Agent (VERIFY/CLASSIFY/TRAIN) on a DIFFERENT or stronger # model than the page agent ... (recommended): # feedback: { model: anthropic/claude-opus-4.7 }
Both examples are the same id, so an operator who follows the page: line and the recommended feedback: line lands with verification on the same model as extraction — the thing that prose says not to do, and the arrangement test/image-limits.test.ts:136 leans on ("the config example recommends running verification on a stronger model than extraction"). Before this PR the pair read Sonnet/Opus and was consistent. Latent: both lines are comments and neither is uncommented by default; naming a third model on one of them, or dropping anthropic/claude-opus-4.7 from the page: line back to a same-generation alternative, restores the pairing.
Accessibility impact: none — no code, agent prompt or output path changes; the only user-visible effect on a fresh copy of the example is that GET /v1/limits and the upload hint publish the honest 1568 px long edge instead of 2576 px, which is advice about downscaling and rejects nothing.
|
One piece of outside evidence for the claim this PR turns on, and one measurement that corroborates the reviewer's note 1. Not a defect report; docs and an example config, and I would take it as it stands. The deployment agrees, and says so without being asked
That also puts a floor under the reviewer's suggested one-liner. Asserting Note 1, measuredThe reviewer's point that the three new tests are insensitive to which model the example names holds concretely, not just prospectively. I ran this PR's Uniformly wrong is internally agreed, so the property holds. Worth saying because the comment at Measured with |
…r_agent examples undoing their own advice Three non-blocking notes from the second review on #198. The example config's model choice publishes exactly one number — the long edge `GET /v1/limits` derives from the model generation — and docs/API.md §3.1 prints a sample response with that number spelled out. Nothing connected the two, so changing the model in the example would leave the documented sample quietly wrong for whichever client hardcoded it from the docs. The new test asserts the resolved value against the number parsed out of docs/API.md rather than against 1568, so it is a drift check and not a third copy of the figure, and it fails with the name of the file left to update. `page: { model: anthropic/claude-opus-4.7 }` and the recommended `feedback: { model: anthropic/claude-opus-4.7 }` had come to name the same model, which is the one thing the recommendation right below them says not to do: verification on the generator's own model shares the generator's blind spots, and test/image-limits.test.ts leans on that pairing being what the example advises. The `page:` line now illustrates the provider-routing shorthand instead — the one of the three forms no example showed — so it keeps the default model and the pairing survives. And the per_agent resolution loop's comment claimed more than the loop does: with `per_agent: {}` every pair walks the same fallback chain, so it cannot catch a hole hiding behind an agent that does not ask for vision. It still catches a provider the file does not define and an agent the comment lists that resolution has no route for, which is what the comment now says. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: bbertucc <46652+bbertucc@users.noreply.github.com>
|
All three notes addressed in 3rd commit. 1. The long edge, tied to the docs. Done as one assertion, though not hardcoded — const printed = docs.match(/"max_long_edge_px":\s*(\d+)/g) ?? [];
assert.equal(printed.length, 1, `docs/API.md prints max_long_edge_px ${printed.length} times`);
assert.equal(Number(printed[0]!.match(/(\d+)/)![1]), resolved);The count assertion is there because a second sample block added later would otherwise drift unwatched. The prose at 2. The vacuous loop. The comment overclaimed and now says what the loop does: with 3. The collapsed pairing. You were right that this undid the advice sitting three lines under it. Fixed by changing the # page: bedrock # one agent on the other provider, at ITS model
# table: { model: anthropic/claude-opus-4.7 } # stronger model, same provider
# reader: { provider: bedrock, model: us.anthropic.claude-haiku-4-5-20251001-v1:0 }So Gates: |
There was a problem hiding this comment.
All six checks pass. The delta since 72e209d is only config.example.yaml:164-172 and the fourth test in test/config-example.test.ts — the src/pipeline/lint.ts / review.ts / test/lint-nodes.test.ts material in my context is still #197 (merged as 9c499f0; diff base 5c0ef27 predates it), so it is not re-reviewed here.
Both notes from my 72e209d review are addressed:
- Note 1 (the test asserted internal agreement, not the number the docs publish) —
test/config-example.test.ts:110-127now derivesresolveImageLimits(loadConfig(EXAMPLE)).max_long_edge_pxand asserts it against the figuredocs/API.mdprints. Verified this is a live check and not a tautology: the example'simage_limitsblock is commented out (config.example.yaml:220-223), sonormalizeOverride(override?.max_long_edge_px, longEdgeFor(model))(src/providers/imageLimits.ts:273) falls through to the model-derived value, and a bump to a generation at or aboveHIGH_RES_FROM_GENERATIONwould make it 2576 against the docs' 1568 and fail. - Note 3 (
page:andfeedback:recommended the same model) — thepage:example is nowpage: bedrock, and the bare-string form is real, not aspirational:agentOverride(src/providers/index.ts:14-18) reads a string entry as{ provider }, andvalidateConfig(src/config.ts:388-405) adds it toreferencedand requires onlydefault_modelfor bedrock, which the file sets. So an operator who used that line gets the routing it advertises, and thefeedback:Opus recommendation no longer reads against thepage:line.
Non-blocking notes
1. Uncommenting any of the per_agent examples is a YAML parse error. config.example.yaml:163-172:
per_agent: {}
# page: bedrock # one agent on the other provider, at ITS model
The examples are indented as children of a key whose value is already {}, so removing the # gives yaml (the parser loadConfig uses) All mapping items must start at the same column — the operator has to also delete the {}, which nothing says. Measured against yaml@^2.6.0 from package.json:25. Pre-existing shape rather than something this PR introduced, and this push is the one that makes the page: line worth copying — the new prose at :169-171 ("the page: line above routes rather than upgrades") points a reader straight at it. It fails loudly at startup with the line number, so nobody ships on it; it is a first-run papercut on the file whose whole job is the first run.
2. The drift test covers the JSON sample, not the two prose copies beside it. test/config-example.test.ts:123-126 asserts on docs/API.md:386 (\"max_long_edge_px\": 1568), and its printed.length === 1 guard is what keeps that anchor honest. docs/API.md:365 ("1568 px on Sonnet 4.6, 2576 px …") and :367 ("Re-saving a 12-megapixel scan at 1568 px") hardcode the same number and a model name, and neither is checked — line 365 additionally names the model, which is the fact #176 was about. In practice the failure points at API.md, so whoever bumps the model is already in the right file; the test comment ("The prose above the sample quotes the same figure beside the model's name") acknowledges this rather than asserting it. Latent today: all three agree.
Accessibility impact: none — no code, agent prompt or output path changes; the only user-visible effect on a fresh copy of the example is that GET /v1/limits and the upload hint publish the honest 1568 px long edge, which is advice about downscaling and rejects nothing.
|
Closing my own note from #198 (comment), which said the suite as it then stood could not catch the failure mode: the 3rd commit's assertion can. Ran the four tests at So the drift is caught in the shape it actually occurred in — a model in the example config that no longer matches the number the docs sample prints — and it is caught without a second copy of |
… prose long edge too Both non-blocking notes from the third review on #198. The examples were indented under `per_agent: {}`, so uncommenting any one of them is a YAML parse error — `All mapping items must start at the same column`, measured against the parser loadConfig uses — and nothing said the `{}` had to go too. The block is now commented out key and all, with the active `per_agent: {}` and a line saying to delete it. The new test does what that comment tells the operator to do: it uncomments the block, loads the result, and asserts all three override forms resolve — the bare `<agent>: <provider>` string, a model pinned on the default provider, and both at once — plus the property the block recommends, that verification does not land on the model extraction used. The drift test covered the JSON sample and not the two prose copies beside it, one of which states the figure next to the model's name and is the copy a reader is likelier to act on. It now asserts that number as well. Only the number: the model name is deliberately unpinned in this file, since asserting agreement rather than a name is the whole approach #176 asked for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: bbertucc <46652+bbertucc@users.noreply.github.com>
|
Both notes fixed in the 4th commit. The first was a real defect and I had made it worse, so thank you for measuring it rather than eyeballing the indentation. 1. The uncommentable examples. Confirmed against The block is now commented out key and all, with the active
That last row is the guard against note 3 from your previous review coming back: whichever models the example names, verification cannot land on the family extraction uses without failing the suite. 2. The prose copies. Added, for the figure only: const sentence = docs.match(/long-edge limit \((\d+) px on /);
assert.equal(Number(sentence[1]), resolved, "the prose long edge and the configured model disagree");The model name at Gates: Merging once the review lands on this commit. |
Closes #176.
config.example.yamlis the file an operator copies to stand Iris up, and it pinned Opus 4.7 on both providers. The reference deployment serves every capability with Sonnet 4.6, and per #176's diagnostics sweep has done so in every benchmark round on record. So the example configured a model this project does not run and has never measured — at 1.67× the per-page cost of the figures it publishes, for an operator who changed nothing.What changed
config.example.yamlopenrouterdefault_model+per_capability×3anthropic/claude-opus-4.7anthropic/claude-sonnet-4.6config.example.yamlbedrockdefault_model+per_capability×3us.anthropic.claude-opus-4-7us.anthropic.claude-sonnet-4-6prd.md§10.3 example blockanthropic/claude-opus-4.7,anthropic.claude-opus-4-7-v1us.-prefixedThe note, and where it lives
One note, above the first provider block, with the bedrock block pointing at it — the file's existing idiom (
See the note on openrouter.max_tokens above) and the rule that a model fact belongs in exactly one place, since the model gets swapped.What it says, beyond "this is what we run":
max_tokensis our setting and not the model's, and the overrun rate, the correction shrink floor and the section headroom were all sized against how this model behaves at that ceiling. An operator on Opus 4.7 is outside those measurements and nothing in the file said so. It also notes that the choice moves the upload limits the deployment publishes, since the long edge follows the model generation — so a copy of this file now accepts exactly what the reference deployment accepts.Judgment calls
The commented
per_agentexamples stay — they teach the mechanism, not a model. Butpage: { model: anthropic/claude-sonnet-4.6 } # cheaper/faster extractionbecomes a no-op the moment Sonnet is the default, and a commented example that overrides nothing teaches nothing. It now illustrates the stronger direction, andtable:takes over demonstrating the{ provider:, model: }form so all three mechanisms still have a line.providers.defaultstaysopenrouteragainst the deployment'sbedrock, which #176 left to the maintainer. It decides which credential a new operator must obtain, and one API key with no cloud account is the shorter path to a first working run. The drift is real, so the file now states the reason rather than leaving the reader to find the mismatch.PRD §10.3 is illustration and now says so. It pins
openai/gpt-5for one capability and names agents (image_analysis) that are not dispatched, so treating it as a recommendation was never right; it points atconfig.example.yamlas the file to copy and the one place that recommends. Its Bedrock id also loses the bareanthropic.prefix thatconfig.example.yaml's own comment says is rejected for on-demand use — a copied example that 400s on the first call is worse than one naming the wrong model.Verification
loadConfig("config.example.yaml")accepts the file and resolves all eight ids (run against a throwaway probe, since nothing in the suite loads this file):npx tsc --noEmitclean ·npm test765 pass / 0 fail ·bash test/e2e.shALL ENDPOINTS PASSED ✅Not changed:
docs/API.mdalready named Sonnet 4.6 in both places, andtest/image-limits.test.ts's comment about the example recommending a stronger model for verification stays true — thefeedback:override is untouched.🤖 Generated with Claude Code