feat(claude): add Claude Fable 5.1 model (cherry-pick upstream #9078) - #103
Conversation
There was a problem hiding this comment.
Do not approve. Behavior looks correct and the fork URL change is well-structured, but this misses an obvious code-judo move and grows an already-tangled version-policy ladder.
Adding Claude Fable 5.1 currently means cloning a catalog entry, a min-version constant, a supports* helper, a formatter, a filter branch, an xhigh allowlist entry, and a sixth nested-ternary rung. Those are the same fact restated seven times. A newest-first version-gate table deletes the helpers, the formatters, the slug if-chain, and the ternary. The next model is then a row, not another copy of this PR.
The 1k-line rule does not strictly apply: ClaudeProvider.ts was already 1021 lines and is now 1072. This PR did not cross the threshold. The table would still shrink the file instead of adding another ~50 lines of clones.
The ModelManifest URL retarget (fence, yaml intent, watch, guard test) is the right permanent fork divergence. Leave that. The unfenced catalog/manifest slug adds are the right last-resort shape if upstream is expected to supersede them. That fork hygiene does not excuse growing the version-policy spaghetti in the same change.
Sent by Cursor Automation: Thermo nuke 4.6
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
NoahHendrickson
left a comment
There was a problem hiding this comment.
Review
The server-side change is correct and I found no runtime bug in it. The problem is the premise: upstream already shipped this. pingdotgg/t3code merged c17d02cff feat(claude): add Claude Fable 5.1 model (#9078) at 19:20Z; this PR opened at 19:26Z. Upstream's ClaudeProvider.ts hunk is byte-identical to yours (same 2.1.257 gate, same option surface, same ternary chain, same xhigh allowlist). Upstream's main manifest already serves ["claude-fable-5-1", "claude-opus-5", "claude-sonnet-5"]. So the stated reason for the URL repoint ("upstream's currentModels list has never heard of it") was already false when the PR was opened.
I simulated the next daily sync with git merge-tree <PR head> upstream/main. Result:
ClaudeProvider.ts,ClaudeAdapter.test.ts: merge cleanly to a single copy (identical hunks). Good.model-manifest.jsonandModelManifest.test.ts: conflict. Upstream's commit also demotesclaude-fable-5to legacy; this PR keeps it current.ProviderRegistry.test.ts: auto-merges but ends up with two copies of both new tests, because you inserted them before "includes Claude Fable 5" and upstream inserted them after it.- Upstream also added
fable/fable-5.1/claude-fable-5.1aliases inpackages/contracts/src/model.ts; this PR has none.
Recommendation
Close this PR (or strip it to nothing) and take Fable 5.1 from the next claude/sync-* PR. That gets the catalog entry, the Fable 5 demotion, and the contracts aliases with zero fork divergence. If you want it on custom before the sync lands, cherry-pick c17d02cff so the later sync is a no-op.
The model-manifest-from-fork repoint should be dropped along with its guard and customizations.yaml entry. It now protects no real case (upstream has added every Claude model same-day: Opus 5, Fable 5, Fable 5.1), and it costs a permanent Tier 4 fence plus manifest freshness that is only as good as the last human-merged sync PR.
If there is a deliberate fork policy that claude-fable-5 should stay current after upstream demotes it, that is a separate customization that needs its own customizations.yaml entry and guard, not a side effect of this PR. Inline comments below cover what to fix if you keep any part of this.
Verification
- Fetched upstream
mainand both live raw.githubusercontent.com manifests. git merge-tree --write-tree 025e526e3 upstream/mainand inspected the resultingProviderRegistry.test.ts,model-manifest.json,ModelManifest.test.ts.- Read
.fork/AGENTS.mdfence/watch rules and the JSON-file precedents incustomizations.yaml.
Reviewed by Claude Fable 5.1 in Claude Code.
025e526 to
b182a32
Compare


Problem
Claude Code 2.1.257 shipped Claude Fable 5.1 (
claude-fable-5-1), and the Claude provider's model catalog is hardcoded, so it needs a code change to appear.Fix
Upstream landed pingdotgg#9078 (
c17d02cff) right as the first version of this PR opened. This PR is now a clean cherry-pick of that commit ontocustom— byte-identical to upstream, so the next daily sync merges without conflict. It brings:claude-fable-5-1catalog entry (effort incl. ultracode, 200k/1M context window defaulting to 1M), gated on Claude Code >= 2.1.257.fable/fable-5.1/claude-fable-5.1slug aliases in contracts, and the manifest update that marks Fable 5.1 current (and, per upstream's policy, demotes Fable 5 to legacy).The earlier revision of this PR hand-rolled the same catalog change plus a fork-specific manifest-URL retarget; review feedback correctly flagged that upstream had already shipped, so the hand-rolled version and the now-unnecessary fork customization were dropped wholesale in favor of the cherry-pick.
Verification
vp test runonModelManifest.test.ts,ProviderRegistry.test.ts,ClaudeAdapter.test.ts(131 passed) andpackages/sharedmodel.test.ts(7 passed).Babysat by Claude Fable 5 in Claude Code.
🤖 Generated with Claude Code