[NopeCharon]

Bottom line

On a Bedrock route, the Claude Code harness now writes the resolved inference-profile id (for example eu.anthropic.claude-sonnet-5) to model_used, per iteration and on the task. Before this, it wrote the SDK's friendly alias (claude-sonnet-5). The raw alias stays on each messages[].model.

Fixes #99.

What was wrong

_ClaudeTurnState.on_assistant_message copies AssistantMessage.model into sdk_model_used, and finalize put that value on AgentEndEvent.model_used. On Bedrock, the SDK reports the alias, not the route model. So:

• runs on different Bedrock routes (eu. / us. / global.) grouped under one key;
• a campaign that pins a qualified id could not prove the pin from model_used alone, and had to cross-check environment_info.bedrock_model.

The agent already knows the route model: _resolve_effective_model resolves it (config model wins, bare aliases are region-qualified), writes it to ANTHROPIC_MODEL, and stores it on state.effective_model for cost backfill.

The fix

• New _ClaudeTurnState.model_used(): on a BedrockRoute with a resolved model, it returns effective_model; otherwise it returns the SDK-reported model.
• finalize uses it for AgentEndEvent.model_used.
• Direct and LiteLLM routes do not change. On the direct API the SDK value is the precise dated id, so it stays.
• Per-message AssistantMessage.model and TurnStartEvent.model keep the raw SDK value.
• docs/agents/CLAUDE_CODE.md § AWS Bedrock states the behaviour.

Pricing does not change: pricing._normalize_model and the evalboard's normalizeModel both strip the region and anthropic. prefixes before the rate lookup, and the run-header model tally groups on that same normalized key.

Tests

New parametrized TestClaudeTurnState::test_model_used_on_agent_end in tests/test_agent.py. It drives the turn state with an AssistantMessage whose model is the alias claude-sonnet-5:

• Bedrock with a resolved eu.anthropic.claude-sonnet-5 → model_used is the qualified id (fails without the fix);
• Bedrock with no resolved model → falls back to the SDK value;
• direct route with config sonnet → keeps the SDK value.

Each case also checks that messages[0].model keeps the raw alias.

Local checks: ruff format --check and ruff check are clean, the custom lint suite (tests/test_custom_lint.py, 741 tests) and tests.lint.prose_budget pass, and pyright reports no new errors (the remaining ones are imports of optional extras not installed locally). pytest -m "not live and not lint" tests/: 6413 passed. The 12 failures are local-environment only (the litellm extra is not installed, and a node_modules directory sits above the checkout) and happen without this change too.

[uipreliga]

Review: fix(claude-code): record the Bedrock route model as model_used

PR #249 by @NopeCharon · fix/claude-bedrock-model-used → main · OPEN · reviewed against 6e9db40 · 2026-11-05T10:26Z · workflow variant

Change class: complex — changes the value semantics of the persisted model_used run-record field on Bedrock routes (a cross-repo contract), not just a mechanical edit

PR 249 is a correct, narrowly scoped fix: on a BedrockRoute only, model_used now records the resolved inference-profile id, and direct and LiteLLM routes keep the SDK-reported model. No finding can change a task's score or final_status for identical agent output, and there are no critical, high or medium findings. The remaining risks are all low: the persisted model_used changes meaning by route but the schema SSOTs do not say so, and the new test does not cover the persisted TurnRecord, the crash path, the LiteLLM side of the guard, or real alias qualification. Bottom line: safe to merge after the small doc and test fixes below.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	10 / 10	0	0	0	0	—
2. Type Safety	9.9 / 10	0	0	0	1	New parametrized test leaves its parameters unannotated, unlike neighboring parametrized tests
3. Test Health	9.7 / 10	0	0	0	3	New model_used test asserts only the AgentEndEvent, not the persisted TurnRecord or the crashed=True partial-turn path
4. Security	10 / 10	0	0	0	0	—
5. Architecture & Design	10 / 10	0	0	0	0	—
6. Error Handling & Resilience	10 / 10	0	0	0	0	—
7. API Surface & Maintainability	9.8 / 10	0	0	0	2	Per-route meaning of persisted model_used changed, but only the agent page documents it. The schema SSOTs and the downstream-impact note were not updated.
8. Evaluation Harness Quality	10 / 10	0	0	0	0	—

Overall Score: 9.9 / 10 · Weakest Axis: Test Health at 9.7 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 0 · 🔵 6 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

None.

Nits

1. [Axis 2] New parametrized test leaves its parameters unannotated, unlike neighboring parametrized tests (tests/test_agent.py:1965) — def test_model_used_on_agent_end(self, route, effective_model, expected): has no annotations on its parametrized arguments. Nearby parametrized tests in the same file do annotate them (e.g. line 438 async def test_blank_system_prompt_is_treated_as_unset(blank: str):, line 532 ...(blank: str):). Annotate them as route: ApiRoute, effective_model: str | None, expected: str so the test's input shape is self-documenting, the None case is explicit, and the test matches file convention. This is a cosmetic, cold-path issue: pyright excludes tests, and production typing is correct. The new _ClaudeTurnState.model_used(self) -> str | None (claude_code_agent.py:703) matches AgentEndEvent.model_used: str | None (streaming/events.py:167) and both effective_model/sdk_model_used fields, which are typed str | None.
2. [Axis 3] New model_used test asserts only the AgentEndEvent, not the persisted TurnRecord or the crashed=True partial-turn path (tests/test_agent.py:1976-1977) — The test checks (end,) = [e for e in events if isinstance(e, AgentEndEvent)] / assert end.model_used == expected, and drives only state.finalize(AgentEndStatus.COMPLETED). The docs promise more than that: docs/agents/CLAUDE_CODE.md:67 says "model_used (per iteration and on the task) records that resolved inference-profile id". The per-iteration value is the TurnRecord that EventCollector.build_turn_record rebuilds as model_used=end.model_used or self._model (streaming/collector.py:221), and that record is what goes into task.json. A real EventCollector is already wired into _state() (line 1890), so the fix is cheap. Add assert state.collector.build_turn_record().model_used == expected. Also add one crashed=True case, state.finalize(AgentEndStatus.TIMEOUT, crashed=True, crash_reason="x") followed by assert agent.pending_turn.model_used == expected, so the timeout and crash rows that get persisted are covered too. Severity is Low because the collector only passes the value through and every finalize path calls the same self.model_used(). Still, as written, a collector change that preferred self._model (the SDK alias from TurnStartEvent) would pass this test while silently putting the alias back into task.json.
3. [Axis 3] No LiteLLMRoute case for the reverse side of the isinstance(BedrockRoute) guard (tests/test_agent.py:1962) — The PR contract says direct and LiteLLM routes keep the SDK-reported model. Only the direct route is tested: pytest.param(DirectRoute(), "sonnet", "claude-sonnet-5", id="direct-records-sdk-model"). LiteLLM is the sibling route most likely to be broken by an over-broad future change. _resolve_effective_model resolves a non-None effective_model for LiteLLM and syncs ANTHROPIC_MODEL the same way it does for Bedrock. Separately, test_litellm_route.py:642 depends on the SDK-reported converse/... model_used for pricing. If someone widened isinstance(self._agent.route, BedrockRoute) to any route with a resolved model, no test would fail. Add pytest.param(LiteLLMRoute(...), "converse/zai.glm-5", "claude-sonnet-5", id="litellm-records-sdk-model"), using the constructor already used in tests/test_litellm_route.py, so the guard's reverse side is pinned for both non-Bedrock routes.
4. [Axis 3] Bedrock model_used test sets effective_model directly, so the BedrockRoute(model=...) argument is inert and route.model and alias qualification are not tested (tests/test_agent.py:1951-1954) — The case builds BedrockRoute(region="eu-north-1", model="eu.anthropic.claude-sonnet-5") but then writes state.effective_model = effective_model itself (line 1971). The route's model is never read, so the id bedrock-records-route-model claims more than the test checks. The usual user path is a bare agent.model: claude-sonnet-5 that _resolve_effective_model qualifies to eu.anthropic.claude-sonnet-5. That path is not tested together with the new model_used(). Fix: either drop model= from the route and rename the case (e.g. bedrock-records-effective-model), or get the value from the real resolver with state.effective_model = agent._resolve_effective_model("claude-sonnet-5", {}, None) and assert eu.anthropic.claude-sonnet-5. The second option tests the documented 'alias auto-qualified, then recorded' contract directly.
5. [Axis 7] Per-route meaning of persisted model_used changed, but only the agent page documents it. The schema SSOTs and the downstream-impact note were not updated. (docs/agents/CLAUDE_CODE.md:67) — The PR adds this text at docs/agents/CLAUDE_CODE.md:67-69: "On Bedrock, model_used (per iteration and on the task) records that resolved inference-profile id, e.g. eu.anthropic.claude-sonnet-5." From this change on, the value depends on the route. On a direct route it is the id the SDK reports. On a Bedrock route it is the resolved route id (claude_code_agent.py:710-712: if isinstance(self._agent.route, BedrockRoute) and self.effective_model: return self.effective_model). The Pydantic field descriptions are the documented SSOT, and they still describe one meaning. TurnRecord.model_used in models/results.py:345-346 reads "Model identifier used for this turn (e.g., 'claude-sonnet-4-5-20250514')", and EvaluationResult.model_used at models/results.py:538-539 is similar. docs/REPORT_SCHEMA.md:82/144/232 lists the field without the per-route meaning. A consumer that reads task.json/run.json, such as the coder-eval-uipath nightly, will see Bedrock rows change from claude-sonnet-5 to eu.anthropic.claude-sonnet-5. If it groups on the raw value, the history splits into two series. In this repo the evalboard and pricing normalize the id, so they are not affected. The PR body does not say what happens to the downstream nightly. Recommended fix: put the per-route meaning into the model_used Field descriptions (TurnRecord and EvaluationResult) and add one line to REPORT_SCHEMA.md. Then the CLAUDE_CODE.md paragraph can link to them instead of being the only place that states the contract. Also state in the PR whether the downstream grouping keys on the raw model_used. The agent page is the only file in scope, so the finding is anchored there.
6. [Axis 7] _ClaudeTurnState.model_used() is a single-call-site method that does not match the parallel turn-state attribute API (src/coder_eval/agents/claude_code_agent.py:703) — The PR adds def model_used(self) -> str | None: (line 703) next to the self.sdk_model_used attribute (line 339). It is called only once, as model_used=self.model_used(), at line 771. The parallel turn state in delegate_agent.py (class _TurnState, line 418) exposes the same concept as a plain attribute: self.model_used: str | None = model (line 446). So state.model_used is a value in one agent and a bound method in the other, and passing it without the call parentheses would not cause an immediate error. Make it a @property (or inline it at line 771) so that every agent's turn state has the same shape. Also change the docstring phrase "comparison-facing model" to a plain statement of the contract, for example "the model recorded on AgentEndEvent.model_used".

What's Missing

Parallel paths:

• 🟡 The task-level fallback in orchestrator.py:1040-1041 (self.result.model_used = self.task.agent.model) was not updated. It still records the unresolved configured model. A Bedrock row that errors before any turn finalizes therefore records the bare alias (claude-sonnet-5), or None when only BEDROCK_MODEL supplied the model. Rows that complete now record eu.anthropic.claude-sonnet-5. Before this PR both kinds of row agreed on the bare alias, so the PR creates a split inside one run. The evalboard's tallyModels comment (runs.ts:970-975) already describes this exact split. Either resolve the fallback through the same route-aware logic, or document the difference. (trigger: src/coder_eval/agents/claude_code_agent.py)
• 🔵 Only AgentEndEvent.model_used changed. TurnStartEvent.model (claude_code_agent.py:420) still carries sdk_model_used, and so does the mid-stream snapshot path in EventCollector.build_turn_record (collector.py:187, model_used=self._model when no AgentEndEvent has arrived yet). An in-flight or stop_early snapshot TurnRecord on Bedrock therefore reports the SDK alias, and the finalized record reports the profile id. State that the snapshot path is intentionally the raw alias, or route it through the same model_used() logic. (trigger: src/coder_eval/agents/claude_code_agent.py)

Tests:

• 🔵 No test runs the docs claim "model_used ... on the task" from end to end. The new test stops at AgentEndEvent. It does not check that the orchestrator's last-turn-wins resolution (orchestrator.py:1035-1039) gives EvaluationResult.model_used == eu.anthropic.claude-sonnet-5 in task.json/run.json. It also does not cover a Bedrock row with no iterations. No Bedrock-route golden stream exists under tests/_fixtures/golden_streams/expected/ (claude_a..claude_i are all direct-route). Add one orchestrator-level or golden case. (trigger: tests/test_agent.py) (restates: Axis 3: New model_used test asserts only the AgentEndEvent, not the persisted TurnRecord or the crashed=True partial-turn path)

Downstream consumers:

• 🔵 The Python report layer uses the raw model_used string without normalization. markdown.py:432 builds the header Model(s) line from sorted({t["model_used"] ...}), html.py:345 and junit.py:286 print it directly, and Harbor ATIF export copies it into model_name (atif_emit.py:216/329). The evalboard groups through normalizeModel() and pricing goes through _normalize_model(), so neither has this problem. The Python reports do not. A Bedrock run that mixes completed rows (profile id) and pre-turn errored rows (bare alias, see the parallel-path item) now lists two models in the markdown header. Cross-run comparisons that key on the raw value also split. Check these consumers or normalize at the header. (trigger: src/coder_eval/agents/claude_code_agent.py)

Nightly pipeline:

• 🔵 The PR changes the value that the cross-repo contract (task.json / run.json model_used) carries for every Bedrock row, from claude-sonnet-5 to eu.anthropic.claude-sonnet-5. The PR body does not say whether the coder-eval-uipath nightly or eval-runner groups, filters or dashboards on the raw model_used. If it does, the history splits into two series at this merge. State the nightly impact. Also update the schema SSOTs (TurnRecord/EvaluationResult Field descriptions and REPORT_SCHEMA.md), which still describe one meaning for the field. (trigger: docs/agents/CLAUDE_CODE.md) (restates: Axis 7: Per-route meaning of persisted model_used changed, but only the agent page documents it. The schema SSOTs and the downstream-impact note were not updated.)

Harness & Lint Improvements

Static checks (lint / type):

• [ce-lint] CE069 route-matrix-completeness: in tests/, a @pytest.mark.parametrize decorator whose argvalues construct two or more distinct members of ApiRoute (DirectRoute(...), BedrockRoute(...), LiteLLMRoute(...)) must construct all of them. The rule takes the member set from typing.get_args(coder_eval.models.routing.ApiRoute) and does not hardcode it, so a fourth route automatically widens every route-matrix test. Add it as a @pytest.mark.lint class TestCE069RouteMatrixComplete in tests/test_custom_lint.py, because it is a whole-tree test-file rule and not a src AST rule. CE067 was retired (see .claude/harness-candidates.md:950) and CE068 is taken, so CE069 is the next free number. An AST scan of the PR-249 tree finds exactly one violation: tests/test_agent.py:1965 test_model_used_on_agent_end (only Bedrock and Direct). The baseline is therefore zero after the fix, and the rule needs no exemption list. Prevents: Axis 3 'No LiteLLMRoute case for the reverse side of the isinstance(BedrockRoute) guard' (tests/test_agent.py:1962). The rule would have failed make verify on the PR's two-of-three route parametrization. In future it also catches a widened isinstance(self._agent.route, BedrockRoute) guard that leaks the resolved model onto LiteLLM, because the LiteLLM case is then required to exist.
• [pyright] No new rule. Record a deliberate decision about the 'method vs attribute' hazard. src/ is already covered: AgentEndEvent / TurnRecord are Pydantic v2 models (dataclass_transform), so in standard mode pyright rejects model_used=self.model_used without the parentheses. A bound method () -> str | None is not assignable to str | None. The remaining work for the finding is only consistency (@property or inline at claude_code_agent.py:771), and a reviewer settles that. A rule that forces every _*TurnState to share one attribute shape would be machinery for a single call site. Prevents: Axis 7 '_ClaudeTurnState.model_used() is a single-call-site method that does not match the parallel turn-state attribute API' (claude_code_agent.py:703). The silent-misuse failure mode in that finding is already a pyright error. The shape inconsistency itself is out of reach of static checks by design.
• [ruff] Do not add a rule. Record the decision: the PR-249 tree has 154 parametrized test functions with unannotated parameters. Pyright excludes tests/. Enabling ruff ANN001 for tests/ (or a CE cap rule) would create a large backlog for a cosmetic issue on a cold path. A cap rule would have to start below 154 and ratchet down, which costs more than the finding is worth. Fix the one new test by hand. Prevents: Axis 2 'New parametrized test leaves its parameters unannotated' (tests/test_agent.py:1965). This is not worth a gate. The entry is here so that the boundary is recorded as a decision and not missed by accident.

Harness improvements (not statically reachable):

• Add a Bedrock-route scenario to the Claude golden-master suite (tests/test_agent_golden_master.py + tests/_fixtures/golden_streams/claude_fixtures.py). Today run_claude_scenario builds the agent with no route argument, so every committed snapshot is the direct route. Let a scenario pass route=BedrockRoute(region=...) and a bare agent.model: claude-sonnet-5, and snapshot the full scrubbed TurnRecord / pending_turn. Include one timeout/crash scenario so that the crashed=True partial turn is snapshotted too. With this, a per-route change to a persisted field shows up as a reviewed JSON snapshot diff (GOLDEN_REGEN=1). It is not hidden behind a hand-picked AgentEndEvent assertion. Why not static: The value depends on how a live event stream is replayed through communicate(), _resolve_effective_model's alias qualification and EventCollector.build_turn_record. Only executing the turn loop can observe the persisted TurnRecord. Prevents: Axis 3 'asserts only the AgentEndEvent, not the persisted TurnRecord or the crashed=True partial-turn path' (tests/test_agent.py:1976-1977). Axis 1/3 'BedrockRoute(model=...) argument is inert; alias qualification untested' (tests/test_agent.py:1951-1954): the golden scenario goes through the real resolver and does not set state.effective_model directly.
• Add a route-dependent-semantics checklist line to the PR template and to the Axis 8 reviewer prompt: 'If a persisted run-record field (TurnRecord / EvaluationResult / run.json) changes meaning per route or per backend, update its Pydantic Field(description=...) (the SSOT), the matching row in docs/REPORT_SCHEMA.md, and state the effect on the downstream coder-eval-uipath nightly grouping keys.' With the golden-master Bedrock scenario above, the snapshot diff makes such a change visible. The checklist makes sure that someone writes down what the change means. Why not static: The value's type (str | None) and field name do not change. Only the meaning changes, and no AST or doc-parity rule can detect that a description is now incomplete. CE030 checks that a field is present, not that its semantics are current. Prevents: Axis 7 'Per-route meaning of persisted model_used changed, but only the agent page documents it' (docs/agents/CLAUDE_CODE.md:67): models/results.py:345-346 and :538-539 and REPORT_SCHEMA.md:82/144/232 still describe one meaning, and the PR did not say what happens to the nightly.
• Add a shared ALL_ROUTES test fixture in tests/_fixtures, with one representative instance per typing.get_args(ApiRoute) member and LiteLLM built the way tests/test_litellm_route.py builds it. Route-conditional tests parametrize over this fixture and do not hand-build route lists. This is the low-friction way to satisfy the proposed CE069. Most of the work is putting a valid LiteLLMRoute(...) constructor in one place, which is the step authors skip. Why not static: This is a test-authoring convenience, not a detectable defect. CE069 enforces completeness, and the fixture makes compliance cheap. Prevents: Axis 3 'No LiteLLMRoute case for the reverse side of the isinstance(BedrockRoute) guard' (tests/test_agent.py:1962).

Top 5 Priority Actions

1. Document the per-route meaning of model_used (resolved Bedrock profile id, e.g. eu.anthropic.claude-sonnet-5, vs. the SDK-reported id) in the TurnRecord.model_used and EvaluationResult.model_used Field descriptions (models/results.py:345, :538) and in docs/REPORT_SCHEMA.md, link to them from docs/agents/CLAUDE_CODE.md:67, and state in the PR body whether the downstream nightly groups on the raw value, because Bedrock rows in task.json/run.json change from claude-sonnet-5 to eu.anthropic.claude-sonnet-5.
2. Extend the new test at tests/test_agent.py:1976-1977 so that it also asserts state.collector.build_turn_record().model_used == expected and adds a crashed=True case with finalize(AgentEndStatus.TIMEOUT, crashed=True, crash_reason=...), which checks agent.pending_turn.model_used, so that the persisted value is tested and not only the AgentEndEvent.
3. Add a LiteLLMRoute case next to the direct-route case at tests/test_agent.py:1962 that expects the SDK-reported model, so that a later widening of the isinstance(self._agent.route, BedrockRoute) guard (src/coder_eval/agents/claude_code_agent.py:710) fails a test and does not silently change LiteLLM pricing input.
4. Make the Bedrock case at tests/test_agent.py:1951-1954 test real behavior: set state.effective_model from agent._resolve_effective_model("claude-sonnet-5", {}, None) and assert eu.anthropic.claude-sonnet-5, or remove the unused model= argument from the route and rename the case, because the route model is not used in the test now.
5. Change _ClaudeTurnState.model_used() (src/coder_eval/agents/claude_code_agent.py:703) to a @property, or inline it at its single call site (line 771), so that it has the same shape as _TurnState.model_used in delegate_agent.py:446, and annotate the parametrized test arguments at tests/test_agent.py:1965 (route: ApiRoute, effective_model: str | None, expected: str) to follow the file convention.

---

Change class: complex — changes the value semantics of the persisted model_used run-record field on Bedrock routes (a cross-repo contract), not just a mechanical edit
Stats: 0 🔴 · 0 🟠 · 0 🟡 · 6 🔵 across 8 axes reviewed.

[NopeCharon]

@uipreliga addressed the six nits in 5b2b2c7. model_used is now a property. The test resolves the Bedrock alias through _resolve_effective_model, adds a LiteLLM case, checks the collector's TurnRecord and covers a crashed TIMEOUT turn, and its parameters are annotated. The TurnRecord/EvaluationResult field descriptions and REPORT_SCHEMA.md now state the per-route meaning, including that the agent-config fallback is not route-resolved. I left the orchestrator fallback and the mid-stream snapshot code as they are, and I can't speak to how the downstream nightly groups model_used.