Repository navigation
fix(core): migrate the v1 thinking block-binding opt-out - #52327
Conversation
V1 opts a model out of Anthropic thinking block binding with `blockBinding: false` in its `thinking` (or Bedrock `reasoningConfig`) options. The V2 migration copied that into model settings, which nothing reads, so the opt-out stopped working after upgrading. Move it to `compatibility.supportsThinkingBlockBinding`, which the Anthropic protocol already honors, and declare that field on `Model.Compatibility` so config decoding keeps it. Assisted-by: LLM
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
When both legacy option objects opt out, migration still leaves one blockBinding flag in settings.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
This PR restores the V1 thinking block-binding opt-out when legacy model configs are migrated to V2.
Changes:
- Moves
blockBinding: falseinto model compatibility and removes the migrated setting. - Exposes the compatibility flag in the schema and generated client type, with a normalization test.
| File | Description |
|---|---|
packages/schema/src/model.ts |
Declares the compatibility flag. |
packages/core/test/config/normalization.test.ts |
Tests migration of each legacy option location. |
packages/core/src/v1/config/migrate.ts |
Migrates the opt-out into compatibility. |
packages/client/src/promise/generated/types.ts |
Exposes the flag to client types. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A model can set `blockBinding: false` under both `thinking` and `reasoningConfig`. Remove it from each instead of only the first match. Assisted-by: LLM
|
Can confirm this fixes the issue for me. Using Sonnet 5.5 / Opus 5.5 via a Bedrock-backed Anthropic-compatible gateway work again with my v1 config (blockBinding: false). This is the main thing blocking me on v2, so I'd love to see it merged. Anything I can do to help? |
|
@rekram1-node @fwang |
|
@adamdotdevin bump |
|
yes will merge a fix need to think if i still like the name |
|
cheers! |
Issue for this PR
Closes #52325
Type of change
What does this PR do?
V1 users opt out of thinking block binding with
blockBinding: falsein the model'sthinking(or BedrockreasoningConfig) options. The V2 migration forwarded that into model settings, which nothing reads, so the opt-out stopped working after upgrading.migrateModelnow turns it intocompatibility.supportsThinkingBlockBinding: falseand drops the key from settings. The Anthropic protocol already honors that override. Configs without the opt-out migrate exactly as before.It also declares
supportsThinkingBlockBindingonModel.Compatibility(plus the regenerated client type), because config decoding otherwise strips it. #51864 and #52172 add the same field, so that line goes away when either lands. Draft until then.Per-variant opt-outs aren't migrated, since compatibility is per model.
How did you verify your code works?
migrate.tschange, passes with it.bun test test/config/ test/models.test.ts test/model-resolver.test.tsinpackages/core: 270 pass. Pre-push lint and typecheck pass.block_binding(Claude Opus 5.5):v2before this change)Extra inputs are not permitted, so the default is unchangedScreenshots / recordings
N/A, not a UI change.
Checklist
Assisted-by: opus 5.5