Skip to content

feat(ai): lower reasoning into bedrock requests#342

Merged
zmaril merged 2 commits into
mainfrom
feat/bedrock-reasoning-lowering
Jul 21, 2026
Merged

feat(ai): lower reasoning into bedrock requests#342
zmaril merged 2 commits into
mainfrom
feat/bedrock-reasoning-lowering

Conversation

@zmaril

@zmaril zmaril commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Follow-up to #309. PR D of the per-driver reasoning-lowering series: wires the bedrock dialect so SimpleStreamOptions.reasoning / thinking_budgets reach the ConverseStream request, using mistral_backend.rs / anthropic_backend.rs (#309) as the override template.

Before / After

Before: BedrockBackend inherited the default Provider::stream_simple, which extracts .base and drops reasoning. A caller asking for a thinking level got a request with no thinking config — the ported BedrockOptions.reasoning / thinking_budgets fields were orphaned.

After: BedrockBackend overrides stream_simple, mapping the seam options into BedrockOptions and routing through a new driver::stream_simple that lowers reasoning into the request exactly as pi does.

How

New driver::stream_simple faithfully ports pi's bedrock-converse-stream.ts streamSimple (:392-438), which is model-family-aware:

  • No reasoning (:398): the backend short-circuits to the raw stream, so the request is byte-identical to the pre-seam path (no thinking config added).
  • Claude + adaptive (:403-408, gated on supportsAdaptiveThinking): pass reasoning + thinkingBudgets straight through; the driver's build_additional_model_request_fields emits thinking.type = "adaptive" + output_config.effort.
  • Claude, non-adaptive (:413-430): fit thinking inside the output cap via the shared adjustMaxTokensForThinking (simple-options.ts:50), re-clamp to the context window with clampMaxTokensToContext (simple-options.ts:15), and override the clamped level's budget with min(adjusted.thinkingBudget, max(0, maxTokens - 1024)) (:428).
  • Non-Claude (:433-437): reasoning passes through but the driver ignores it (build_additional_model_request_fields returns None for non-Claude), so no thinking config reaches the request — matching pi.

base.maxTokens in every branch is pi's buildBaseOptions output, i.e. clampMaxTokensToContext(model, context, options.maxTokens ?? model.maxTokens) (simple-options.ts:29).

Helpers

  • Promoted clamp_reasoning (xhigh/max -> high) from private to pub(crate) in api/anthropic/simple_options.rs so Bedrock can override the clamped level's budget with the same collapse pi applies (:428). Behavior-preserving for Anthropic — it still calls the helper unchanged from adjust_max_tokens_for_thinking.
  • Reused the existing pub adjust_max_tokens_for_thinking (api/anthropic/simple_options.rs).
  • Ported a BedrockModel-typed clamp_max_tokens_to_context in api/bedrock.rs (the Anthropic one is typed to Model<AnthropicMessagesCompat>); it reuses the shared context-token estimator pi imports from utils/estimate.ts. Added a context_window field to the lean BedrockModel slice to feed it.
  • Claude / adaptive detection (is_anthropic_claude_model, supports_adaptive_thinking) were already ported for the driver's build_additional_model_request_fields; promoted from private to pub(crate) so the sibling stream_simple can reuse them (no re-port).

Tests (param-exact; mirror #309 + pi bedrock-thinking-payload.test.ts)

  • Adaptive Claude (anthropic.claude-opus-4-8) + high -> thinking.type = "adaptive" + output_config.effort = "high".
  • Non-adaptive Claude (claude-3-5-sonnet) + medium, caller cap 2000, model cap 32000 -> inferenceConfig.maxTokens = 10192, thinking.budget_tokens = 8192.
  • Non-adaptive Claude + custom medium budget 20000, model cap 16000 -> budget override bites: maxTokens = 16000, budget_tokens = 14976 (= min(14976, 16000-1024)).
  • Non-Claude (meta.llama3) + reasoning -> no additionalModelRequestFields (driver ignores).
  • No reasoning -> byte-identical to the raw stream path.

Verification

The mapping was verified directly against pi source (vendor/pi/packages/ai/src/api/bedrock-converse-stream.ts:392-438 and simple-options.ts:15,29,47-77) since the providers co-sign session is dormant. Each branch and helper is cited against those .ts lines in the code comments.

CI

cargo build, cargo test -p pidgin-ai (794 lib + 9 integration passing, incl. 5 new bedrock reasoning tests), cargo clippy --all-targets -- -D warnings, and cargo fmt --check all clean. No touched file crosses the 1500-line straitjacket file-size limit; existing duplication markers preserved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GkuR3Xbx7yeqvEv6MPbFg7


Generated by Claude Code

claude added 2 commits July 21, 2026 17:12
Override `stream_simple` on the Bedrock backend so the unified
`reasoning`/`thinking_budgets` controls reach the ConverseStream request,
faithfully porting pi's `bedrock-converse-stream.ts` `streamSimple`
(:392-438). The lowering is model-family-aware:

- No reasoning: the backend short-circuits to the raw `stream`, so the
  request stays byte-identical to the pre-seam path (no thinking config).
- Claude + adaptive (supportsAdaptiveThinking): pass reasoning + budgets
  through; the driver emits `thinking.type = "adaptive"` + `output_config`.
- Claude, non-adaptive: fit thinking inside the output cap via the shared
  `adjustMaxTokensForThinking`, re-clamp to the context window, and override
  the clamped level's budget with `min(adjusted, max(0, maxTokens - 1024))`.
- Non-Claude: reasoning passes through but the driver ignores it
  (`build_additional_model_request_fields` returns None), matching pi.

Promote anthropic's private `clamp_reasoning` to `pub(crate)`
(behavior-preserving for anthropic) and reuse the existing
`adjust_max_tokens_for_thinking`; port a `BedrockModel`-typed
`clamp_max_tokens_to_context` (add `context_window` to the model slice)
since the anthropic one is typed to `Model<AnthropicMessagesCompat>`.

Tests mirror #309 + pi's `bedrock-thinking-payload.test.ts`: adaptive
Claude passthrough, non-adaptive Claude maxTokens/budget adjustment with
exact numbers, non-Claude passthrough (no thinking config), and the
no-reasoning byte-identical guard. Follow-up to #309.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GkuR3Xbx7yeqvEv6MPbFg7
straitjacket's [duplication] rule flagged the new BedrockModel-typed
`clamp_max_tokens_to_context` as a 9-line/58-token clone of the Anthropic
one. pi has a single generic `clampMaxTokensToContext`; the two Rust copies
existed only because the model types differ. Extract the shared arithmetic
into a window-keyed `clamp_max_tokens_to_context_window(context_window, ...)`
core in anthropic/simple_options.rs; both dialects' typed wrappers now
delegate to it. Genuine de-duplication (one implementation, as in pi), not a
suppression. Behavior-preserving; all tests, clippy, and fmt stay green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GkuR3Xbx7yeqvEv6MPbFg7
@github-actions

Copy link
Copy Markdown

Conformance smoke: agent + ai + tui

  • pi_sha: 3da591ab74ab9ab407e72ed882600b2c851fae21
  • manifest_native_modules: 52

Headline is rust-backed: passing cases in files whose module-under-test is a native (Rust addon) module. Raw all-pass is shown secondary — it is inflated by unflipped TypeScript that passes without touching any Rust.

Package rust-backed Native modules raw pass (secondary) Failing Skipped
agent 73/180 (40.6%) 7 180/180 0 0
ai 79/1296 (6.1%) 9 553/1296 3 740
tui 375/678 (55.3%) 14 678/678 0 0
smoke total 527/2154 (24.5%) 30 1411/2154 3 740

agent: 73/180 (40.6%) · ai: 79/1296 (6.1%) · tui: 375/678 (55.3%)

rust-backed = passing / total tests run, per the manifest's per-native-row tests lists. The Native modules column counts modules served by the Rust addon. This is the agent+ai+tui smoke subset; the full baseline lives in committed conformance.json.

Attribution

A pi test file is rust-backed iff the module it primarily exercises — its module-under-test — is status=native in conformance/manifest.json. A package's rust-backed count is the passing cases in its rust-backed files. Transitive or infrastructure use does not count (e.g. ~30 ai files construct the native faux provider but test other things — only faux-provider.test.ts, whose subject is faux, counts). A file that substantially tests both a native and an original module is excluded rather than counted, so the number under-reports rather than over-claims.

Per-file decisions (from each native manifest row's tests list):

Native module Test file Decision
ai/api/anthropic-messages.ts test/anthropic-sse-parsing.test.ts counted
ai/providers/faux.ts test/faux-provider.test.ts counted
coding-agent/utils/ansi.ts test/ansi-utils.test.ts counted
coding-agent/utils/changelog.ts test/changelog.test.ts counted
coding-agent/utils/git.ts test/git-ssh-url.test.ts counted
coding-agent/core/tools/path-utils.ts test/path-utils.test.ts counted
tui/keys.ts test/keys.test.ts counted
tui/utils.ts test/truncate-to-width.test.ts, test/regression-regional-indicator-width.test.ts counted
coding-agent/utils/mime.ts test/image-process.test.ts mixed — excluded (2 of 3 cases test processImage/original)
coding-agent/utils/version-check.ts test/version-check.test.ts mixed — excluded (fetch cases mock the original; not separable per-file)
coding-agent/core/export-html/ansi-to-html.ts test/export-html-whitespace.test.ts mixed — excluded (asset-grep + tool-renderer/original dominate)
coding-agent/core/tools/truncate.ts no dedicated test (exercised via tools.test.ts, subject is the read/edit factory/original)
coding-agent/core/tools/edit-diff.ts no dedicated test (edit-tool tests' subject is the edit tool/original)

CLI conformance (black-box, against the pidgin binary)

  • CLI conformance: 15/15 pass against target/release/pidgin (pass delta ±0, fail delta ±0).
File Passing Failing Skipped
packages/coding-agent/test/session-file-invalid.test.ts 1 0 0
packages/coding-agent/test/session-id-readonly.test.ts 7 0 0
packages/coding-agent/test/startup-session-name.test.ts 2 0 0
packages/coding-agent/test/stdout-cleanliness.test.ts 5 0 0

The four repointed coding-agent CLI test files spawn the compiled pidgin binary via $PIDGIN_BIN instead of pi's own cli.ts. This is a separate signal from the module smoke table and is never folded into the per-package Native count.

@zmaril
zmaril marked this pull request as ready for review July 21, 2026 18:01
@zmaril
zmaril merged commit 4c3d1f9 into main Jul 21, 2026
14 checks passed
@zmaril
zmaril deleted the feat/bedrock-reasoning-lowering branch July 21, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants