fix: resolve real max_tokens for Bedrock cross-region Anthropic model IDs

[imranq2]

Summary

• ChatAnthropicBedrock (langchain-aws) inherits ChatAnthropic.set_default_max_tokens, which resolves an unset max_tokens from langchain_anthropic's own model-profile registry — keyed by bare Anthropic API names (claude-sonnet-4-5-20250929), not Bedrock cross-region inference profile IDs (us.anthropic.claude-sonnet-4-5-20250929-v1:0). The lookup misses and silently falls back to 4096 tokens even though the model supports 64000.
• ModelFactory._create_anthropic_bedrock_model now explicitly resolves max_tokens from langchain_aws's own Bedrock-aware profile registry (which does have entries keyed by the full Bedrock ID) before constructing ChatAnthropicBedrock, so the incorrect fallback never triggers.

Impact

Reproduced via baileyai-skills-service's AI skill-authoring chat: asking Bailey to draft a comprehensive skill caused generation to hit the (wrongly low) 4096-token ceiling mid-tool-call, leaving invalid/truncated JSON for a large tool-call argument. Downstream code coerced that to {}, propose_skill failed validation, and the agent looped retrying the same failing call indefinitely (observed for 5+ minutes without resolving). Verified this fix resolves it end-to-end against a live Bedrock-backed baileyai instance.

Fixes BAI-343.

Test plan

• New unit tests (tests/models/test_model_factory_bedrock_max_tokens.py) covering the profile lookup and its wiring into _create_anthropic_bedrock_model, including the "don't override an explicit max_tokens" and "unknown model falls back gracefully" cases
• Full suite: docker compose run --rm dev pytest tests — 724 passed
• ruff check, ruff format --check, mypy --strict clean on changed files
• Manually verified against a live baileyai-skills-service + baileyai (Bedrock) stack: before the fix, asking Bailey to author a skill looped forever on empty propose_skill arguments; after mounting this fix, the same request completed successfully with SKILL.md + a resource file proposed and saved

🤖 Generated with Claude Code

[gecko-security]

Gecko PR Review

No vulnerabilities found, LGTM.

Summary

This PR introduces MCP tool progress forwarding and synthetic heartbeat functionality to prevent SSE stream idle timeouts during long-running tool calls. Changes span three files: a detailed implementation plan document, updates to model_factory.py for Bedrock max-tokens handling, and new test coverage for that factory logic. The core risk involves ensuring the heartbeat mechanism doesn't introduce race conditions or excessive event spam, and that environment variable defaults preserve backward compatibility.

Important Files Changed

3 important files

File Path	Why it matters
languagemodelcommon/models/model_factory.py	Likely implements or modifies Bedrock model token limit logic that interacts with MCP tool calls; changes here affect how max_tokens constraints are enforced during streaming.
tests/models/test_model_factory_bedrock_max_tokens.py	New test file that must verify Bedrock max-tokens boundary conditions to catch token calculation bugs before they cause model API failures or truncation.
docs/superpowers/plans/2026-07-22-mcp-tool-progress-heartbeat.md	Comprehensive implementation plan specifying environment variable defaults, custom-event routing, and backward-compatibility constraints that all code changes must follow.

Sequence Diagram

sequenceDiagram
    participant Tool as MCP Tool
    participant Session as session.call_tool()
    participant Heartbeat as Heartbeat Watchdog
    participant Dispatcher as adispatch_custom_event
    participant SSE as SSE Stream
    Tool->>Session: progress_callback fires
    Session->>Dispatcher: mcp_task_progress event
    Dispatcher->>SSE: task.progress
    par Heartbeat Check
        Heartbeat->>Session: timeout check
        Heartbeat->>Dispatcher: mcp_tool_heartbeat event (synthetic)
        Dispatcher->>SSE: task.progress (synthetic)
    end

Loading

_{Edit Gecko PR Settings | Reviewed commit: 8845022}

[claude]

LGTM — approving with the two nits noted inline (unlogged ImportError fallback, keyword-only convention on the new helper), neither of which affects correctness today.

Extended reasoning...

Overview

The functional change is confined to languagemodelcommon/models/model_factory.py: a new _resolve_anthropic_bedrock_max_tokens static helper looks up the real max_output_tokens for a Bedrock-prefixed Anthropic model ID via langchain_aws's internal profile registry, and _create_anthropic_bedrock_model now calls it to set max_tokens explicitly before constructing ChatAnthropicBedrock, only when the caller hasn't already supplied one. The accompanying tests/models/test_model_factory_bedrock_max_tokens.py covers the resolver directly (known ID, known ID across region prefixes, unknown ID) and the wiring into _create_anthropic_bedrock_model (sets when unset, doesn't override an explicit value, leaves unset for unknown models). The included docs file under docs/superpowers/plans/ is a planning document for unrelated future work and carries no runtime risk.

Security risks

None identified. No auth, tenant-isolation, PHI/PII, or FHIR data-modeling code paths are touched. The change only affects how a numeric generation-limit parameter is resolved before constructing a third-party chat model client.

Level of scrutiny

This is a targeted, well-tested bug fix to a production code path (Bedrock Anthropic model construction), not a config tweak, but it is narrowly scoped, has clear before/after behavior (matches the documented BAI-343 production incident and was verified end-to-end against a live stack per the PR description), and ships with unit tests for the new logic including edge cases (explicit override, unknown model fallback). The reliance on a private/underscore-prefixed langchain_aws symbol is a legitimate fragility point, but it fails closed (falls back to prior behavior via ImportError) rather than raising, so the blast radius of a future breakage is a silent regression, not a crash — the bug hunting system's inline nit about adding a log line there appropriately captures this without blocking the fix.

Other factors

I checked the mutation-in-place concern raised by the Aikido bot comment: model_parameters_dict is a fresh dict constructed locally inside get_model on every call before being passed down, so the in-place model_parameters_dict["max_tokens"] = ... in _create_anthropic_bedrock_model has no caller-visible side effect in this codepath — not a real issue. The two items from this run's bug hunt are both explicitly tagged as nits (observability gap on a private-API fallback, and a keyword-argument convention miss on one new helper), neither changes behavior, and the PR's own test suite plus a live end-to-end verification give good confidence in correctness.

[imranq2]

Addressed the 3 review nits in 1278a30:

• _resolve_anthropic_bedrock_max_tokens is now keyword-only (*, model_name) and its call site updated, per CLAUDE.md convention.
• The except ImportError fallback now logs a logger.warning for observability symmetry with the success path.
• model_parameters_dict is copied before mutation in _create_anthropic_bedrock_model to avoid caller-visible side effects.

Full test suite (748 tests) + ruff + mypy pass locally.

[kenanspruill]

LGTM. See Claude suggestions