Fix #1641: default the claude consult lane to the 1M context window - #1657
Fix #1641: default the claude consult lane to the 1M context window#1657mohidmakhdoomi wants to merge 9 commits into
Conversation
`consult -m claude` failed with "Prompt is too long" on any large review — 5 attempts across 2 independent callers on PR #1640 produced no review file, leaving `porch next 1481` permanently blocked. Root cause: the lane shipped the bare model id `claude-opus-5`. The Agent SDK's bundled Claude Code runtime budgets context by model id — its window function returns 1,000,000 only when the id carries a `[1m]` suffix, and 200,000 otherwise. The claude lane runs agentically (`allowedTools: Read/Glob/Grep`, `maxTurns: 200`), so an impl or integration review of a 41-file PR reads its way past 200K and the API rejects the request. The two saved failing sessions show 29 and 51 tool calls with last-successful inputs of 174,521 and 175,930 tokens before the error — accumulated context, not an oversized opening prompt. `MODEL_ID_RE` rejected `[` and `]`, so neither `consult.models.claude` nor `--model-id claude-opus-5[1m]` could work around it. That is why the failure was unrecoverable rather than merely inconvenient. Changes: - `DEFAULT_CLAUDE_MODEL` is now `claude-opus-5[1m]`. - `MODEL_ID_RE` accepts one optional, case-insensitive `[1m]` suffix, with the 1-200 character bound now applied to the id as a whole via a lookahead. - Validator message updated to describe the suffix. - Mirrored the consult docs in `codev/` and `codev-skeleton/`. Explicitly configured model ids still pass through unchanged, and `--model-id` still takes precedence over config. The SDK strips the suffix before the API request, so the provider still sees `claude-opus-5`. Regression coverage fails without the production fix (13 failures on baseline with only the test files applied) and passes with it: the shipped default, the SDK boundary, configured ids including `[1m]`, CLI override resolution, the 200-character total-length boundary, and malformed suffixes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Architect integration review — preliminary (medium risk)I read the full diff and verified that all eight seeded source/test/doc files match the handoff manifest. The change is appropriately scoped: select Please correct the PR body and builder narrative, keeping the eight seeded files unchanged:
These are factual corrections to the review record, not a request to expand the fix. This comment is not human PR-gate or merge approval. |
Architect integration review — Claude findings adjudicatedRisk: medium (10 files, shared model-ID validation; no protocol/state-machine implementation changes). The required independent Claude integration review completed in 254.0s with COMMENT, confirming the fix, validation boundaries, backwards compatibility, and contained scope. I checked its findings against the actual SDK rather than accepting them uncritically. Disposition
The earlier narrative corrections are appropriate. Do not amend or force-push old commits just to revise their historical wording; the corrected PR body and committed narrative are the durable correction. Keep all eight seeded files unchanged. No implementation blocker remains from architect integration review. Final builder Claude CMAP and the narrative commit are still pending. This is a technical review disposition, not human PR-gate or merge approval. |
Three factual corrections from architect review, applied to the PR body and the builder thread log. No production, test, or shipped-doc change: all eight seeded files are unchanged and still hash-match the handoff manifest. 1. Over-broad framing of the reported failures. The original narrative implied all five attempts on PR #1640 were context failures and generalized to "any large review". Four were "Prompt is too long"; the third builder-side attempt hit a separate subscription usage limit. Scoped to the observed failures. 2. Over-broad runtime claim. The original narrative said the SDK returns 1M "only when the id carries a [1m] suffix, and 200K otherwise". The window function has four branches before the 200K fallback — a DISABLE_COMPACT / CLAUDE_CODE_MAX_CONTEXT_TOKENS env override, the [1m] suffix, a beta-header branch, and a sonnet-4-6-gated branch — so that is not universal. Restated as the verified pair: bare claude-opus-5 budgets 200,000 and claude-opus-5[1m] budgets 1,000,000, under SDK 0.2.105 and 0.2.141. 3. Over-claimed mechanism. The original narrative said the model "reads its way past 200K and the API rejects the request". The saved sessions establish substantial prior tool activity and accumulated-context exhaustion surfacing as "Prompt is too long" — not a specific client-vs-server rejection point, and not that all 41 changed files were read in full. Also corrected: the gemini lane did not review PR #1640. Its artifact is a 358-byte non-blocking skip notice ("agy exited with code 1"). The original narrative repeated "gemini wrote its file" from the issue text without opening the file. Recorded alongside these: the architect's adjudication of the independent Claude integration review, and the explicit opt-out now stated in the PR body — consult.models.claude: "claude-opus-5" retains the previous selection and its 200K budget on the inspected runtimes. Historical commit b73c5d2 is deliberately left as written; per architect direction the corrected PR body and this commit are the durable correction, rather than an amend and force-push that would invalidate green CI and completed reviews. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…me description All three CMAP lanes returned APPROVE at HIGH confidence with no blocking issues. Corrects one further narrative inaccuracy of my own, surfaced by the Claude review. I had written that the context-budget mechanism was "verified against the bundled runtime in SDK 0.2.105 and 0.2.141". That is wrong for 0.2.141: it has no bundled runtime. 0.2.105 ships the runtime as cli.js; 0.2.141 is a thin client carrying no [1m] logic of its own, delegating to a runtime-resolved native binary from @anthropic-ai/claude-agent-sdk-<platform>. I had taken that half from the handoff investigation instead of checking it. Verified directly: the native binary carries the identical gate rG(H) = /\[1m\]/i.test(H), plus context-1m-2025-08-07 (4 occurrences) and CLAUDE_CODE_DISABLE_1M_CONTEXT (2 occurrences). The claim was true; the description of how it was true was not. PR body corrected. Also re-verified the suffix strip on both runtimes, closing the one item the Claude reviewer could not confirm: UT(q) in 0.2.105 and KL(H) in the 0.2.141 native binary, both replace(/\[(1|2)m\]/gi, ""), applied at request construction. The reviewer stopped at X5 normalization, which re-attaches the marker for concrete ids. Non-blocking items referred to the architect rather than actioned, because they would require editing seeded files: the CLAUDE_CODE_DISABLE_1M_CONTEXT doc nuance and a premium-pricing line in shipped consult.md. The pricing note is recorded in the PR body instead. All eight seeded files remain unchanged and hash-match the handoff manifest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Final CMAP disposition — no implementation blockersI read the final builder Claude review: APPROVE / HIGH, joining Gemini and Codex (APPROVE / HIGH). All eight seeded files still hash-match. No further source, test, or shipped-doc changes are requested. A few advisory assertions in that review are incorrect; please correct only the PR/thread record rather than importing those assertions into the shipped docs:
Keep this correction concise, commit the narrative once, and retain the already corrected PR summary rather than replacing it with the reviewer's generated summary (which repeats the original overclaims). These factual dispositions do not warrant another model review of the unchanged fix. CI was all green on |
…ckaging wording
Narrative-only. No source, test, or shipped-doc change; all eight seeded files
remain unchanged and hash-match the handoff manifest.
Three advisory assertions from the final Claude review do not survive checking,
and one piece of my own wording was misleading. Verified each against the source
rather than accepting the disposition:
1. Disable-context: WITHDRAWN. The review claimed a concrete id keeps [1m] on the
wire under CLAUDE_CODE_DISABLE_1M_CONTEXT=1. UT(q) = q.replace(/\[(1|2)m\]/gi,
"") consults neither ZG() nor the env var, and its call site is an unguarded
beta.messages.create({...P, model: UT(P.model)}). The strip is unconditional.
The shipped doc line needs no fix.
2. Pricing: WITHDRAWN. The PR body had imported "premium >200K input pricing"
from the review. Anthropic documents standard model pricing for the 1M window.
Replaced with the accurate note: longer reviews can consume more total tokens,
and the maxBudgetUsd: 25 per-consultation cap is unchanged.
3. consult stats split: WITHDRAWN. MetricsDB.summary() groups by model (the lane)
at metrics.ts:371, not model_id. Recorded concrete ids change as intended, but
lane totals do not split.
4. Runtime packaging: CORRECTED. My phrasing implied an arbitrary, unversioned
system CLI. 0.2.141 pins every @anthropic-ai/claude-agent-sdk-<platform>
optional dependency to exactly 0.2.141 and SDK resolution locates that
package's binary. The 0.2.105-JS / 0.2.141-native layout distinction is real;
both are SDK-distributed and version-locked.
This supersedes the two items previously referred to the architect for a possible
seeded-doc edit; both rested on false premises, so no such edit is warranted.
The corrected PR summary is retained rather than the reviewer's generated
PR_SUMMARY, which repeats the original overclaims.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Final CI — frozen head
|
| Check | Result | Duration |
|---|---|---|
| Artifact-Canvas Browser Tests | pass | 48s |
| CLI Integration Tests | pass | 1m7s |
| CLI Tests (macos-latest) | pass | 58s |
| CLI Tests (ubuntu-latest) | pass | 1m5s |
| Package Install Verification | pass | 2m24s |
| Tower Integration Tests | pass | 1m45s |
| Unit Tests | pass | 2m45s |
Review status
CMAP complete — all three lanes APPROVE / HIGH: gemini (49.1s), codex (148.0s),
claude (750.4s), no blocking issues from any lane. Architect integration review returned
COMMENT with no implementation blocker, and three of the final review's advisory
assertions were withdrawn after verification against the source (disable-context,
premium pricing, consult stats split).
State
- All 8 seeded files unchanged — 8/8 hash-match the handoff manifest.
- The three commits after the fix are narrative-only; no source, test, or shipped-doc
change sinceb73c5d21d. - Regression evidence unchanged: 13 tests fail with the test files applied alone against
unfixed production code, 358/358 pass with the fix. Full suite 5754 passed / 48 skipped
/ 0 failed.
Head is frozen for human review. No gate approval or merge has been performed — awaiting
an explicit human decision.
Final architect integration review — ready for human decisionReviewed head 23de988. Medium risk: 10 files, +331/-25 including the protocol record and narrative; the functional patch remains the original eight files (+68/-25), all SHA-256-identical to the handoff.
Gemini, Codex, and Claude final CMAP verdicts are APPROVE / HIGH. The additional required Claude integration review returned COMMENT; its findings and the CMAP advisories were independently adjudicated in the preceding comments. Factual corrections are recorded, with no remaining implementation blocker. Technical recommendation: merge after explicit human approval. This comment does not approve the human Porch gate. Builder retains ownership of gate execution, merge, and protocol verification; architect will close the issue after merge. |
|
The human approved the submitted fix in the architect conversation. Porch recorded that local PR-gate decision in 1fe1037. I independently verified its delta from the reviewed head is only approval bookkeeping in We are contributors, not upstream maintainers. Our local sign-off is not GitHub maintainer approval. This PR remains open awaiting an eligible maintainer/reviewer approval and an authorized upstream merge. No self-review or branch-protection bypass is authorized. The branch is frozen and the builder worktree is preserved. No extra narrative commit is needed; this comment records the disposition. Protocol completion, issue closure, and cleanup wait for a real upstream merge. |
Fixes #1641
The bug
consult -m claudefailed repeatedly on PR #1640 (41 files, +4877/−110). Five attemptsacross two independent callers — one architect-side (
--type integration), fourbuilder-side (
--protocol pir --type impl) — produced no claude review file, leavingporch next 1481blocked onRun remaining consultations (claude).Four of those five ended in
Prompt is too long; the third builder-side attempt hit aseparate subscription usage limit and is not evidence of this bug. Of the other lanes run
against the same target, codex completed (239.4s,
REQUEST_CHANGES); gemini didnot review — its artifact is a non-blocking skip notice (
Gemini lane skipped — agy exited with code 1), so it is not evidence that the target was reviewable either.Root cause
The claude lane shipped the bare model id
claude-opus-5.The Agent SDK's bundled Claude Code runtime derives its context budget from the model id.
Its window function has several branches — an env override, the
[1m]suffix, abeta-header branch, and a
sonnet-4-6-gated branch — before falling through to a 200,000default, so this is not a universal "1M with a suffix, 200K otherwise" rule. The scoped
fact that matters here, verified on both runtimes this repo runs against — SDK 0.2.105
(the lockfile version, which bundles the runtime as
cli.js) and 0.2.141 (the globallyinstalled build, whose JS package is a thin client over a separately packaged native
runtime). Both layouts are SDK-distributed and version-locked: 0.2.141 pins every
@anthropic-ai/claude-agent-sdk-<platform>optional dependency to exactly0.2.141, andSDK resolution locates that package's binary — it is not an arbitrary system CLI.
claude-opus-5claude-opus-5[1m]runClaudeConsultationruns the lane agentically —allowedTools: ['Read','Glob','Grep'],maxTurns: 200— so a review of a PR this size accumulates review context until the 200Kbudget is exhausted, which surfaces as
Prompt is too long. The two saved failing sessionsshow substantial prior tool activity before the error: 29 and 51 tool calls, with
last-successful inputs of 174,521 and 175,930 tokens. That establishes accumulated-context
exhaustion; it does not establish where exactly the request was refused, nor that every
changed file was read in full. It is not an oversized opening prompt — the opening prompt
for issue 1481 inlines a 2.5K spec and a 34K plan, roughly 10K tokens.
The second half of the bug is why it was unrecoverable:
MODEL_ID_RErejected[and], so neitherconsult.models.claudenor--model-id claude-opus-5[1m]could workaround it.
The fix
DEFAULT_CLAUDE_MODEL→claude-opus-5[1m].MODEL_ID_REaccepts one optional, case-insensitive[1m]suffix. The 1–200 characterbound now applies to the id as a whole, via a lookahead, so the suffix counts toward
the limit rather than escaping it.
codev/andcodev-skeleton/.Explicitly configured model ids still pass through unchanged and
--model-idstill takesprecedence over config, so anyone pinning a bare id keeps today's behavior.
Opt-out. Setting
consult.models.claude: "claude-opus-5"retains the previousselection and its 200,000-token budget on the inspected runtimes.
--model-id claude-opus-5does the same for a single invocation, andCLAUDE_CODE_DISABLE_1M_CONTEXT=1disables 1M context at the runtime level. There is noautomatic fallback and no change to entitlement policy: extended context depends on
model and account availability, which the shipped docs link to.
On cost: Anthropic documents standard model pricing for the 1M window, so this is not a
move into a premium tier. Longer reviews can consume more total tokens; the existing
maxBudgetUsd: 25per-consultation cap is unchanged.The suffix does not reach the provider. Internal normalization (
X5) re-attaches themarker for concrete ids, so it is visible right up to the request — but the strip happens at
the API boundary itself, on both runtimes:
cli.js):UT(q) = q.replace(/\[(1|2)m\]/gi, ""), applied atbeta.messages.create({...P, model: UT(P.model)}), atcountTokens, and on the bedrock path.KL(H) = H.replace(/\[(1|2)m\]/gi, ""), applied asmodel: KL(A.model).The context budget and the
context-1m-2025-08-07beta header are separate effects of thesuffix; the native binary carries that header (4 occurrences) and
CLAUDE_CODE_DISABLE_1M_CONTEXTalongside the samerG(H) = /\[1m\]/i.test(H)gate.8 files, +68/−25 (excluding the builder thread log).
Regression evidence
The tests were applied alone first, against the unfixed production code:
Failures on baseline were exactly the expected ones —
expected 'claude-opus-5' to be 'claude-opus-5[1m]'andInvalid model id "claude-opus-5[1m]"thrown fromvalidateModelId.Coverage: the shipped default, the SDK boundary (what actually reaches
options.model),configured ids including
[1m],--model-idoverride resolution, the 200-charactertotal-length boundary (196+
[1m]accepted, 197+[1m]rejected), and malformed suffixes(
[1m]alone,opus[1m]extra,opus[1m][1m],opus[2m],opus[).Verification
pnpm build— passed.@cluesmith/codevsuite — 5754 passed, 48 skipped, 0 failed (288 files).git diff --checkclean; the twoconsult.mdcopies verified byte-identical.validateModelIdis the only production consumer ofMODEL_ID_RE.[1m]default installed, the original PR afx send: add --interrupt-after <seconds> (bounded patience, then one forced delivery) #1640 review completed in 356.7s across 41 tool calls
at 188,387 input tokens with no context error, and a separate bounded test processed
465,431 input tokens in a single request.
🤖 Generated with Claude Code