Skip to content

fix(chat): disclose auto in the per-turn model footer - #4658

Open
dwu96 wants to merge 1 commit into
mainfrom
fix/footer-auto-model
Open

fix(chat): disclose auto in the per-turn model footer#4658
dwu96 wants to merge 1 commit into
mainfrom
fix/footer-auto-model

Conversation

@dwu96

@dwu96 dwu96 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Problem

The per-turn footer's model chip disappeared on every Auto turn — the line read 0.56 credits · 19s with no model, while a pinned turn in the same session read claude-opus-5 · 0.64 credits · 6.1s. A blank there is indistinguishable from a turn that reported no measurement at all, so it reads as a broken footer rather than as "the backend chose".

Mechanically: read_effective_model skips the auto sentinel by design, and picking Auto writes that sentinel into both _model and _resolved_model_id (session_handle.py:1102/1122, client.py:2360/2361), clobbering the concrete currentModelId that session/new reported. Both candidates were the sentinel, so the reader returned "" and the footer omitted the whole segment.

This also contradicted _attach_turn_stats's own docstring, which claimed the auto path was "the disclosure of what auto resolved to" — the one path where it disclosed nothing.

Verified against live data before changing anything: five session files written minutes apart on the current build carried turn_stats with elapsed_ms + credits and no model.

Change

read_turn_model splits the single blank into three distinct answers:

State Value When
Resolved e.g. global.anthropic.claude-opus-4-8[1m] A concrete id reached the provider chain
Auto auto The session is on Auto and the backend disclosed no id
Unattributable "" (key omitted) No model information at all

Auto's per-turn choice is not on the ACP wire — the _kiro.dev/metadata frame carries contextUsagePercentage and meteringUsage only (acp/_dispatch.py:148), and currentModelId is session-scoped (session/new / session/load). So auto is the whole of what can be said truthfully. It is never presented as a model id, and read_effective_model remains the reader for pricing and context-window lookups where the sentinel is not a usable key.

The telemetry path (_resolve_model) already had an equivalent Auto fallback, so the usage row store knew a turn was Auto while the footer did not. Both now go through one reader; the pre-existing TestModelSourceFallback::test_auto_source_fills_empty_model covers that the refactor is behaviour-preserving.

No frontend production change. The existing chip renders the sentinel verbatim, so auto displays as-is and the tooltip becomes … · model: auto through the existing turn_model key — zero new i18n keys.

Attribution semantics (unchanged by this PR, worth stating)

The value is read at that turn's EVENT_COMPLETE and frozen into the message's meta.turn_stats; the frontend renders the persisted meta and never recomputes. So switching the model later does not relabel an older message, which is correct — that turn really was served by what it says.

Switching mid-turn is not a quiet relabel either: the live-switch path refuses when has_active_turn() and falls back to a session reset, which ends the turn — and a turn that never reaches EVENT_COMPLETE keeps _turn_model = "" (chat_runner.py:4990), so nothing is attached. Background ops (titles, suggestions) are separate ACP sessions with their own handle, and _wrapper_chain visits _runtime last so process-level --model cannot outrank session state.

The honest limit: the value is derived from mutable session state at turn end, not recorded when the prompt was sent, so this rests on the no-mid-turn-switch invariant rather than on the data model.

Not in scope

Disclosing which model Auto picked requires the backend to report it per turn. _dispatch.py's _log_unrecognized_metadata_fields already logs novel metadata fields once each, so it will surface if kiro-cli starts sending one.

Tests

  • test/test_usage.py::TestReadTurnModel — the three attribution states, sentinel matched case/space-insensitively, found deeper in the wrapper chain, never raises on a hostile source.
  • test/test_turn_stats.py — the sentinel is carried like any other value; plus a binding test that the footer uses read_turn_model, since both readers exist and differ only on the Auto path, so binding the wrong one is a silent regression that every pinned-model assertion still passes.
  • website/src/test/AssistantMessage.test.tsx — an auto turn renders the chip and the full line; fmtTurnModel('auto') passes through unchanged.

Mutation-verified both layers. Neutering _source_requests_auto in the helper failed 4 tests (3 new + the pre-existing telemetry one). Adding && turnStats.model !== 'auto' to the renderer failed the new frontend test. Both reverted.

Renamed test_model_omitted_when_unresolvedtest_model_omitted_when_unattributable: "unresolved auto" is no longer that case.

Docs

docs/system-specs/features/turn-stats-footer.md never documented the model field at all. Added the meta-contract row, the three-state table, and why Auto's per-turn choice is unobtainable.

Unrelated hunk, and why it is required

chat_runner.py is not in .github/black-baseline.txt but was not black-clean on origin/main either (verified against pristine main: one hunk, a manual wrap black would join at :6703). The formatting gate is diff-scoped and requires any non-baselined file in scope to be clean, so the first PR to touch this file pays that one line. Applied by hand exactly as black asked rather than letting black rewrite the file.

Verification

Green locally: pytest targeted (1475 across every module importing the touched code), isort, flake8, mypy (999 files), the black gate at real diff scope, docs-lint, tsc -b, eslint, jscpd, i18n:check with I18N_BASE_REF, and the full frontend suite — 22208 passed, 0 failed.

Full backend suite: 58188 passed, 43 failed, none of them this change. Not one of the 12 failing files references handlers.usage, chat_runner, turn_stats, or either reader. 7 are self-inflicted by the dev host: its glibc is too old for Pillow 12's manylinux wheels, so Pillow had to be built from source with -C jpeg=disable (no libjpeg headers) and PIL.features.check('jpg') is False, which is exactly what test_acp_prompt_blocks's jpeg/webp mime tests assert on. The other 36 are host/tooling (playwright node bootstrap, ssh multiplexing, ps parsing, macOS release, GitHub workflow triage, xdist host-budget at load average 27). CI runs on a clean image and is the check that matters for these.

Follow-up noticed, not fixed here

chat_handlers.py:3312 asserts "The UI disables the model button while a turn runs", but no run-state disabled guard is apparent on the model button in ChatPage.tsx. It does not change behaviour (the backend has_active_turn() guard plus the reset path covers it), but the comment may be stale and the front line of defence is the backend, not the UI.

The footer's model chip vanished on every Auto turn. `read_effective_model`
skips the `auto` sentinel by design, and picking Auto writes that sentinel into
both `_model` and `_resolved_model_id`, so both candidates were the sentinel and
the reader returned "" — which the footer renders as nothing, indistinguishable
from a turn that reported no measurement at all.

Add `read_turn_model`, which splits that single blank into two answers: a
concrete id when one is available, the bare `auto` when the turn was handed to
Auto and the backend disclosed no id, and "" only when nothing is known. Auto's
per-turn choice is not on the ACP wire (the `_kiro.dev/metadata` frame carries
context and metering only, and `currentModelId` is session-scoped), so `auto` is
the whole of what can be said truthfully; it is never presented as a model id,
and pricing/window lookups keep using `read_effective_model`.

Point the telemetry path at the same helper. Its `_resolve_model` already had an
equivalent Auto fallback, so the row store knew a turn was Auto while the footer
did not — one reader now serves both.

The frontend needs no change: the existing chip renders the sentinel verbatim.
Tests pin that, since a filter added there would silently restore the blank.

`chat_runner.py` is not in the black baseline but was not black-clean on main,
so the diff-scoped formatting gate requires the one unrelated hunk at :6703.
@dwu96
dwu96 requested a review from a team as a code owner August 20, 2026 08:14
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

UX-level review of de853c8ea6e561306565541d1e7a9e5df81b9a63 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

auto in the model slot truthfully confirms the user's own picker choice, replacing a blank that read as a broken footer — strictly clearer than before.

[UX-REVIEWED] de853c8

@github-actions

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of de853c8ea6e561306565541d1e7a9e5df81b9a63 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] de853c8

False positive or not applicable? A repository writer can comment:
/ai-review override gpt de853c8ea6e561306565541d1e7a9e5df81b9a63: <one-sentence reason>

@github-actions

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed de853c8ea6e561306565541d1e7a9e5df81b9a63 — this comment is updated in place on each push.

Review details

The discovery pass produced no candidates, and my own re-derivation of the _resolve_model refactor confirms behavioral equivalence across all input combinations (concrete id, auto-with-no-resolved-id, blank, and model_source is None). read_turn_model composes read_effective_model + _source_requests_auto correctly, is exception-guarded, and never presents "auto" as a model id. Nothing grounds a new finding at the 80+ bar.

No findings.

[OPUS-REVIEWED] de853c8

Verdict parsed from the review's SHA-scoped output markers for commit de853c8ea6e561306565541d1e7a9e5df81b9a63.

False positive or not applicable? A repository writer can comment:
/ai-review override fable de853c8ea6e561306565541d1e7a9e5df81b9a63: <one-sentence reason>

@github-actions

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Design-level review of de853c8ea6e561306565541d1e7a9e5df81b9a63 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design review complete. The PR splits "no model information" from "the user handed the turn to Auto" via a new read_turn_model reader, unifies the telemetry path onto it, and documents the contract in the same commit. I checked the claims against the code: the two-reader split is deliberate and correctly bounded (read_effective_model stays the pricing/window-lookup reader where auto is not a usable key), the sentinel-in-model shape matches the pre-existing usage row store convention, the footgun of two near-identical readers is pinned by a binding test, and the "unrelated" black hunk at chat_runner.py:6703 is real and exists in the commit as described. Persisting "auto" into meta.turn_stats is backward- and forward-compatible: old frontends render it verbatim through the existing chip, and no consumer uses turn_stats.model as a lookup key.

Design-Verdict: PASS

Honest three-state attribution at the only layer that can know it, with the reader split correctly guarded against future misuse.

[DESIGN-REVIEWED] de853c8

@github-actions

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of de853c8ea6e561306565541d1e7a9e5df81b9a63 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All counts are run; the picture is complete. Composing the final review.

First-Principles-Verdict: PASS

A reported blank-footer defect, fixed by deleting a second spelling: both readers now share one Auto-aware helper, and the wire limit is named, not guessed.

What this change ships

Intent: make an Auto turn's footer say "the backend chose" instead of rendering nothing — a FIX.

  1. Auto turns now show auto in the footer model chip instead of blank — justified (reported defect, live-data verified)
  2. Footer tooltip on Auto turns now reads model: auto — rides along via the existing turn_model key; zero new surface
  3. New backend reader read_turn_model — justified; 2 counted consumers (chat_runner.py:6632, _resolve_model at usage.py:1034)
  4. Telemetry row store and footer collapse onto one reader — declared, subtractive (deletes _resolve_model's inline duplicate of the same fallback)
  5. Spec gains the model meta row and three-state table — mandated (AGENTS.md same-commit spec rule)

The depth question is answered by a protocol limit, not preference: Auto's per-turn pick is absent from the _kiro.dev/metadata frame, so auto is the maximum truthful disclosure and the display fix is cause-level within reach. The one prior sibling (telemetry's own Auto fallback) is unified by this change, not left behind.

Subtractions

  • Drop the spec sentence "read_effective_model remains the reader for pricing and context-window lookups" (turn-stats-footer.md:60, echoed in the read_turn_model docstring). Grepped read_effective_model across src/ outside handlers/usage.py: 0 production callers — pricing/window lookups use _resolved_model_id or _model inline (client.py:5684, session_handle.py:2407). Its sole consumer is read_turn_model; the sentence names a consumer relationship that does not exist.

[FIRST-PRINCIPLES-REVIEWED] de853c8

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

readiness: passed Eligible automated validation passed for the current revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant