Name the model in use in the warping row (APP-5532) - #15323
Name the model in use in the warping row (APP-5532)#15323warp-agent-staging[bot] wants to merge 7 commits into
Conversation
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Wilson Factory.
|
|
This PR was generated with Warp. Comment |
The server already reports the model a response is running on mid-stream, as a ModelUsed message on the first LLM attempt, but the client only ever showed that name when the model was a fallback. Generalize the fallback message so the warping row names whatever model the exchange reported, behind the new WarpingModelName flag, and keep the fallback path's lookback and explanation line behind the flag they already had. APP-5532
61a4a4d to
ef4a225
Compare
Xavientois
left a comment
There was a problem hiding this comment.
Code looks good. Waiting for screenshots
There was a problem hiding this comment.
Overview
The warping row now names the model the current exchange reported via ModelUsed, keeping today's generic text until one arrives, behind the new dogfood-gated WarpingModelName. No correctness, security, or standards defect was found in the change; the two items below need a human decision rather than a code fix.
Concerns
- Per-request flicker, and the shimmer restart that comes with it. Because nothing is borrowed from a previous exchange, the row now goes
Warping...->Warping with X.on every request, and each text change restarts the shimmer from zero (crates/warpui_core/src/elements/gui/shimmering_text.rs:193-204); across a tool-call cycle it readsWarping with X.->Executing command...->Warping...->Warping with X.. Never showing a stale name is the right side of that trade, and the copy and gating are one site, so this is cheap to adjust. It is still a user-visible copy change on the live warping row thatspecs/CODE-1828/TECH.mdsection 4 deliberately kept clear, so it wants a decision on DES-819 and a design reviewer on this PR. - Untouched code: the
ModelUsedingest targets the last exchange, not the message'stask_id.app/src/ai/agent/conversation.rs:3029-3034resolves the target asadded_exchanges_by_response[stream].last(), so once a stream has lazily appended a subagent exchange, a later primary-agentModelUsedlands on the subagent's exchange and that block's row would name the wrong model. Pre-existing and out of scope here, but this change raises its visible cost from the fallback path alone to any conversation; the fix is to match theAddedExchangewhosetask_idequals the message's, and QUALITY-395 is the existing home for it.
Verdict
Checks: build pass (CI formatting + clippy green on Linux, macOS, Windows and wasm; release-flag compilation green), tests pass locally 11/11 with a red-green demonstration but CI test jobs still pending, CI green for the jobs that have run, visual proof missing - captures of the running UI are in progress and are required before merge on a user-facing change.
Found: 0 critical, 0 important, 0 suggestions, 0 nits, 2 questions for human decision. The review's remaining findings (1 important, 2 suggestions, 2 nits) are being addressed by the author and are not listed here.
A fallback attempt is still the model in use, so naming it no longer depends on FallbackModelLoadOutputMessaging surviving; that flag now only controls the explanation line and the previous-exchange lookback. Renames the message's is_fallback to show_fallback_explanation, which is what it actually drives. Also covers the OutputModelInfo -> ModelInUse conversion, including the empty display name that master rendered as "Warping with .", drops the routing sequence test whose cases its siblings already reach, and trims the unused Clone/Eq derives.
describes_an_unnamed_fallback_model_generically now builds its input through
ModelInUse::from instead of by hand, so the empty-name normalization is covered
by what it renders ("Warping with ." without it) rather than only by the
conversion's output.
…ellipsis The row's named copy now ends in the same trailing ellipsis as its generic copy, and the name reaches the other messages that are a model working: Generating plan, Updating plan, Generating fix, Creating diff, Preparing question, Adjusting tasks. One helper owns that copy, so the generic slot and the rest read the same way. Phases that are not a model working stay unnamed - executing or monitoring a command, searching the codebase, grep, file glob, MCP calls, web search, waiting for input. Summarization is excluded too: it is an LLM call, but the server resolves it to a separately chosen model and never reports that model to the client, so naming the exchange's model there would attribute the work to a model that is not doing it.
The fallback copy ships today and the requester wants it left alone, so the two rules now sit side by side at the one site that owns the copy, with a comment saying why, and a test asserting both so neither drifts into the other. An unnamed model with no fallback message to fall back on keeps the generic copy rather than borrowing the fallback's wording.
The lookback test borrows the previous exchange's fallback model, so it is the fallback message and ends in a period. Its two siblings were changed back and this one was not, which left the branch head red.
There was a problem hiding this comment.
Overview
Second pass over the copy change and the extension of model naming to the other status messages. One defect went back to the author; the item below is a product decision rather than a code fix, and it is the last thing outstanding from review.
Concerns
- The named messages are accurate about whose output it is, not about what is happening this instant. Four of the six named messages key off properties of accumulated output rather than stream liveness —
contains_create_document_action/contains_update_document_actionmatch any message in the output, so once a plan action appears the message latches for the rest of the exchange, andis_last_message_requesting_file_editsstays true while the client turns the action into a diff. SoGenerating plan with gpt-5.5 (high)...can remain on screen while that model is idle and the client is applying the plan. Under the reading the copy makes — that model produced this content — all six are correct, and this over-persistence is pre-existing onmasterfor these messages and for the generic row, so the change does not introduce it. - Tightening it is available but not free.
props.model.status(app).is_streaming()is already used a few lines below in the same function and would confine every name to a live stream. It would also strip the name from the shipped fallback message during post-stream windows wheremastershows it today, which is a change to existing behaviour rather than only to the new naming. Worth an explicit decision either way; the current behaviour is defensible and shipping it as-is is a valid answer.
Verdict
Checks: build pass, tests pass 32/32 with format, both clippy lines and the feature check green on a6c3663, CI green, visual proof present — 293s of named row across three long turns, including 28.25s of Generating plan with gpt-5.5 (high).... Note that two of the six naming sites have visual evidence and four have neither test nor capture; the approved named/unnamed list rests on review, and pinning it means extracting the message-selection chain from the render path, which belongs in its own PR.
Found: 0 critical, 0 important, 0 suggestions, 0 nits, 1 question for decision. The remaining findings from this pass (1 important, 1 suggestion, 3 nits) went to the author and are not listed here.
…hange Review caught a leak: the name handed to the other status messages was lifted from the message struct with no flag check, so on the shipped configuration - naming off, fallback messaging in default - a fallback exchange would have rendered "Generating plan with Claude Haiku 4.5..." where master renders "Generating plan...". The lookback's borrowed name reached them the same way. The name now travels only when WarpingModelName is on and the model came from the exchange being rendered. Two tests pin it, one per path, and the shipped configuration test no longer encodes the leak it was meant to catch. Also derives the fallback copy's stem from LOAD_OUTPUT_MESSAGE so a change to the row's verb cannot leave it behind, renames the local to is_fallback_message now that it decides three things, and says what to do with the fallback branch when the legacy flag is cleaned up.


Description
The server already reports which model a response is running on while that response is still streaming:
LLMAgent.sendModelUsedsends aMessage.ModelUsedon the first LLM attempt of every primary-agent turn, and again on each retry or fallback. The client stored it inoutput.model_infobut only ever displayed the name when the model was a fallback, so in the ordinary case the row shimmeredWarping...for the whole response even though the model was known.The agent status row now names whatever model the current exchange reported —
Warping with Claude Sonnet 4.5...— and keeps the generic copy until a model is reported, which is exactly the window whereautoand custom model routers have not resolved one yet.This generalizes the existing fallback message instead of adding a second mechanism beside it.
resolve_fallback_warping_messagebecomesresolve_warping_model_messageover a smallModelInUse/WarpingModelMessagepair, and the fallback path keeps both of its extras behind the flag it already had,FallbackModelLoadOutputMessaging: the one-exchange lookback that avoids a flicker on agent-initiated follow-ups, and theThe primary model (X) failed…explanation line. Nothing outside that path borrows another exchange's model, so an exchange that has not reported yet shows the generic copy rather than a name that may not be the one running.Which messages name the model
The name is not limited to the generic slot. Every message in the row that represents a model producing output carries it, via one helper,
status_message_naming_model, which inserts the name ahead of the trailing ellipsis:Warping with {model}...Generating plan with {model}...,Updating plan with {model}...Generating fix with {model}...,Creating diff with {model}...Preparing question with {model}...,Adjusting tasks with {model}...Messages for phases where no model is working stay unnamed, because naming one there would be a claim we cannot support:
Executing command...,Writing command input...,Waiting for command to exit...,Agent waiting for instructions...,Searching codebase...,Grepping...,Finding files...,Reading files...,Calling "X" MCP tool...,Reading "X" MCP resource...,Searching the web..., and the cloudSetting up environment.Summarizing conversation...andSummarizing command output...are excluded deliberately, even though they are LLM calls.LLMAgent.SummarizeConversationnever callssendModelUsed, and that work bills to separate categories (UsageCategoryCompaction,UsageCategoryToolSummarization); the model doing it is resolved server-side and never reported to the client, so the only name available is the primary agent's, which is not the model summarizing. Naming summarization would need a server change to report its model first.Copy: two rules, on purpose
Named messages end in the row's trailing ellipsis. The pre-existing fallback message keeps its full stop —
Warping with Claude Haiku 4.5.— because it ships today and the requester asked for it to be left alone. Both rules sit next to each other inwarping_model_messagewith a comment saying why, and a test asserts them side by side so neither drifts into the other.Gating
New
FeatureFlag::WarpingModelName(cargo featurewarping_model_name), enabled inDOGFOOD_FLAGSand not indefault. Rolling it out to everyone is addingwarping_model_nametodefaultinapp/Cargo.toml.The split between the two flags follows "do we know the model":
WarpingModelNamedecides whether the row names a model at all, andFallbackModelLoadOutputMessagingkeeps only the two things specific to a fallback attempt — the explanation line and the previous-exchange lookback. So a fallback model is named whenever naming is on, rather than losing its name the day the older flag is cleaned up, and the shipped configuration (naming off, fallback messaging on) behaves exactly as it does today, down to the full stop.Scope
Naming the model for non-primary agents is out of scope: the server still restricts
ModelUsedtoAgentId_PRIMARY(shouldSendModelUsedNotification, TODO(QUALITY-395)), so CLI / full-terminal-use, computer use, advice and conversation-search exchanges report nothing and correctly fall back to the generic copy. Per-turn usage and cost stay out of scope; that is DES-819's.This is the GUI desktop app only. The TUI has its own indicator at
crates/warp_tui/src/warping_indicator.rs, which shares none of these constants, so the CLI's row keeps readingWarping...unnamed. That is a decision for this PR, not an oversight; extending it there is separate work.Linked Issue
APP-5532 — tracked in Linear, not as a GitHub issue, so the label checkbox below does not apply.
ready-to-specorready-to-implement.Testing
15 unit tests in
app/src/ai/blocklist/block/status_bar_tests.rsand 2 inapp/src/ai/blocklist/block/view_impl/common_tests.rs, covering: reading a reported model off the exchange output; an empty display name treated as no name (whichmasterrendered as the literalWarping with .); nothing reported yet, i.e. theauto/ custom-router pre-resolution window; a reported model; both feature flags in each combination, including today's shipped configuration; the fallback lookback and its suppression after a new user query; an ordinary model never borrowed from the previous exchange; the two copy rules side by side; and the naming helper's ellipsis handling.All green on
490354c:cargo nextest run --no-fail-fast -p warp -E 'test(/^ai::blocklist::block::status_bar::tests::/) or test(/^ai::blocklist::block::view_impl::common::tests::/)'→32 tests run: 32 passed, 6446 skipped../script/format— no diff.cargo clippy -p warp -p warp_features --all-targets -- -D warnings— zero warnings.cargo clippy --workspace --exclude warp_completer --all-targets --tests -- -D warnings— zero warnings. Worth running because this adds aFeatureFlagvariant, which a two-package clippy would not catch against an exhaustive match elsewhere.cargo check -p warp --features warping_model_name— clean, so the cargo-feature wiring compiles.Red-green on the copy rule: making
status_message_naming_modelreturnformat!("{message} with {model_display_name}")— i.e. dropping the ellipsis handling — fails four tests, including the one that pins the two rules together, and leaves all three fallback-copy tests green, which is what proves the fallback path really does bypass the helper:Red-green on the per-message naming: nothing failed, and that is worth stating rather than glossing. Reverting all six
naming_model(...)call sites inrender_warping_indicatorto the plain constants leaves the suite at 32/32.render_warping_indicatortakes an&AppContextand returns aBox<dyn Element>, so reaching it means building a GUI element tree — integration-test territory, not unit-test territory. A revert that also removed the now-unused prop would be caught by nothing at all; a partial revert is caught only by clippy's dead-code lint on the orphaned field, which is not behavioral coverage. If that is not good enough, the fix is extracting the message-selection chain into a pure function a test can drive, which this PR does not do.Be precise about what the captures do and do not show, too. A three-turn take can only exercise the phases those turns hit:
Warping with {model}...andGenerating plan with {model}...are on screen and measured.Updating plan...,Generating fix...,Creating diff...andPreparing question...appear in neither a test nor a capture — two of the six naming sites have visual evidence and four rest on the reviewed list. The gate that decides whether a name is available at all is unit-tested; the per-message wiring is what is uncovered.The flag gate on the propagated name is pinned per guard, not just in aggregate. Reverting
model_display_nameto the ungated value fails exactly two tests — the shipped-configuration one and the lookback one — withtextandshow_fallback_explanationidentical on both sides, so the assertions fail purely on the leaked name. Dropping each half of the conjunction on its own fails exactly one test each: removing the naming-flag guard fails only the shipped-configuration test, removing the provenance guard fails only the lookback test. Neither guard is riding on the other../script/runUnticked because it was not local: the app was built from this branch and driven through real agent-mode responses on a Linux cloud runner, since
./script/runthere needs the privatewarp-channel-configgenerator and silently downgrades towarp-osswithout it. Everything under Screenshots / Videos is from that running build.Screenshots / Videos
All of these are the running desktop app on a Linux runner (Xvfb, lavapipe software rendering). AFTER is this branch; BEFORE is its parent commit
27f8ee6c, i.e.master, which needed its own build because there is no runtime off-switch for the flag.Videos
5e9ee02— the current one to watch. Three long turns in one conversation, a different model picked on camera for each, the third submitted through/plan. Named row totals 293 seconds across the take, including 28.25s ofGenerating plan with gpt-5.5 (high)...: https://oz.staging.warp.dev/artifacts/01a01cb9-42f3-7571-bd8c-ff8e1be2badc5e9ee02— the same take captured at 1080p/4fps, which is the file the timings below were measured from: https://oz.staging.warp.dev/artifacts/01a01cbf-8c4b-70ce-b2ca-764ee7db04de67a2d77— 4:37, three long-reasoning turns from before the per-message naming landed: 39.6s and 32.6s of named row: https://oz.staging.warp.dev/artifacts/01a01c84-8b98-75fb-906a-73efe0656ae067a2d77— 61s with short turns, where the row's brief generic stretches are visible: https://oz.staging.warp.dev/artifacts/01a01bb6-2752-7171-aa7e-3713999716a9master: https://oz.staging.warp.dev/artifacts/01a01b89-f998-7462-a512-38e87323c4b3Screenshots
/planprompt, plan document streaming into the side panel, selector readinggpt-5.5 (high), row readingGenerating plan with gpt-5.5 (high)...: https://oz.staging.warp.dev/artifacts/01a01cb8-959d-7569-8bc6-c4cd35e1be6eclaude sonnet 4.5selected, mid-response: the row readsWarping..., and did so for the whole response on every prompt: https://oz.staging.warp.dev/artifacts/01a01b8a-6e14-73ae-a447-1ee465bc0d49auto (genius)resolving to a different model — the row follows what the server reported, not what the selector says: https://oz.staging.warp.dev/artifacts/01a01b78-d19d-70f5-8109-5a122fa5855bNot exercised visually: a custom model router (this staging account has none configured) and a genuine fallback/retry, which cannot be forced from the client. Those two paths rest on the unit tests.
Measured behavior
From the 1080p capture, read frame by frame (2430 frames decoded, status-row band cropped and thresholded, one frame transcribed per run; boundaries exact to the 0.25s frame interval):
claude sonnet 4.5 (thinking): 5.75sWarping..., then 41.00s ofWarping with claude sonnet 4.5 (thinking)...auto (genius), resolved toclaude opus 5 (max): 3.75s generic, then 186.00s continuous naming the resolved modelgpt-5.5 (high)via/plan: 2.25s generic → 20.50s named → 2.75sSearching the web..., correctly unnamed → 12.25s named → 28.25s ofGenerating plan with gpt-5.5 (high)...→ 2.50s generic → 5.25s namedEvery named string ended in the ellipsis; no frame showed the fallback's single period, which is the other copy rule staying where it belongs.
How long the name is on screen tracks how long the call takes. On these long turns it is continuously readable. On very short turns it is a flicker: in the 61s clip, across six LLM calls the row named a model in two of them for about half a second each. That is the intended consequence of naming only what the current exchange reported — an agent-initiated follow-up has not reported yet, and the row will not borrow the previous exchange's model.
mastershows generic copy for those same stretches, so nothing regressed.Decided: always accurate. The requester chose that with the measurements in hand — "let's opt for always accurate because the end-user's use-case is only an issue for long-running turns". Holding the last known name would have been wrong in a case these captures actually contain: one turn where the row named
claude haiku 4.5for the call that composed a command andclaude opus 5 (high)for the call that composed the answer.One sub-3-second transient, not chased: between the plan phase ending and the wrap-up generation the row dropped to
Warping...for 2.50s before naming the model again, which is the next attempt starting before itsModelUsedarrives — the documented unnamed case. No phase was ever mis-named.Decided: ship this timing as-is. Four of the six named messages —
Updating plan,Generating plan,Creating diff,Preparing question— are chosen from what the output already contains rather than from an in-flight call, soGenerating plan with {model}...can stay on screen after that model has finished its part. The requester was asked whether the naming should be gated on stream liveness and chose not to: "I think shipping as-is is fine". Recording it so a later reader sees a decision rather than an oversight.Agent Mode