Skip to content

Tell the user when a picked model is not the one answering - #722

Merged
realtonyyoung merged 3 commits into
mainfrom
claude-tyoung/ai-2385-model-fallback-note
Aug 31, 2026
Merged

realtonyyoung merged 3 commits into
mainfrom
claude-tyoung/ai-2385-model-fallback-note

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

Closes #716 — AI-2385

What & why

A launch carrying a model the vendor does not publish runs the vendor's default instead. That is correct — the model list only arrives with session/new, so no upstream gate can refuse the launch over it — but the only trace was a daemon-log warning, and the launcher went on showing the model the user picked while a different one answered. The runtime now puts one system_note on the transcript naming the dropped model and the vendor whose default is running.

Both launcher surfaces already clear the model when the vendor changes, so this closes the half of the issue that was still open. The Launching agent log line now also carries the vendor, so a future requested-vs-running mismatch is attributable from one line rather than by correlating with a later runtime line — which PTY vendors never emit.

Where to look

The note is gated on the launch having picked a model, not merely on one reaching the selector: ResolveRequestedModel substitutes the daemon-wide default, and a default the vendor omits would otherwise produce this note on every launch. That gate and the substitution share one predicate so they cannot disagree.

Verification

Three new pins in AcpModelFallbackNoticeTests, driving the real factory against FakeAcpAgent: the mismatch note, silence when the model is applied, and silence for a daemon default the vendor does not publish (asserting ResolvedModel is null, so the drop really happens). Daemon unit suite 2869 total, 0 failed. AOT publish of the daemon clean of IL3050/IL2026.

🤖 Generated with Claude Code

The vendor's list only arrives with session/new, so no upstream gate can
refuse the launch over an unavailable model. The note is gated on the launch
having picked one: a daemon-wide default the vendor omits would nag every time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Aug 31, 2026

Copy link
Copy Markdown

AI-2385

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Surface ACP model fallback in the transcript

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Add a transcript system note when an explicitly selected ACP model is unavailable.
• Suppress fallback notices for applied models and unavailable daemon defaults.
• Include vendor context in launch logs and cover fallback behavior with factory-level tests.
Diagram

sequenceDiagram
    actor User
    participant Orchestrator
    participant Factory
    participant Runtime
    participant Vendor
    participant Transcript
    User->>Orchestrator: Launch with model
    Orchestrator->>Factory: Start with vendor
    Factory->>Factory: Detect explicit pick
    Factory->>Runtime: Start requested model
    Runtime->>Vendor: Create session
    Vendor-->>Runtime: Available models
    Runtime->>Runtime: Resolve model
    alt Explicit model unavailable
        Runtime->>Transcript: Emit fallback note
    else Model applied or default
        Runtime->>Runtime: Continue silently
    end
Loading
High-Level Assessment

The chosen approach is appropriate because model availability is only known after the vendor returns session metadata, making earlier launch rejection impossible. Carrying an explicit-pick flag from the same predicate used for default substitution prevents false notices, while a transcript system note is more actionable than relying on daemon logs alone.

Files changed (4) +177 / -14

Bug fix (3) +39 / -14
AcpHostedAgentRuntime.csEmit a transcript note for unresolved explicit model picks +17/-1

Emit a transcript note for unresolved explicit model picks

• Extends runtime startup with an explicit-model-pick flag. After model resolution, it emits a vendor-specific system note before the initial turn when the chosen model is unavailable and the vendor default will answer instead.

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs

AcpHostedAgentRuntimeFactory.csDistinguish launch model picks from daemon defaults +16/-10

Distinguish launch model picks from daemon defaults

• Adds a shared predicate for detecting explicit per-launch model choices, reuses it during requested-model resolution, and passes the result into the runtime. This keeps fallback notification gating consistent with default-model substitution.

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntimeFactory.cs

AgentOrchestrator.csAdd vendor context to agent launch logs +6/-3

Add vendor context to agent launch logs

• Includes the vendor in the existing launch log entry so requested model and vendor mismatches can be diagnosed from one line, including for PTY vendors without later runtime logs.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

Tests (1) +138 / -0
AcpModelFallbackNoticeTests.csCover ACP model fallback notification branches +138/-0

Cover ACP model fallback notification branches

• Adds a factory-level fake ACP harness and tests that verify one fallback note for an unavailable explicit model, no note for an applied model, and no note when an unavailable daemon default is dropped.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AcpModelFallbackNoticeTests.cs

@qodo-code-review

qodo-code-review Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Resolver docs narrate implementation ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The expanded ResolveRequestedModel documentation restates its conditional precedence and narrates
internal predicate and downstream-resolution relationships across many lines. It is not sparse and
duplicates behavior already evident from the method body.
Code

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntimeFactory.cs[R704-707]

+    /// <paramref name="ctx"/>'s own <c>Model</c> takes precedence when the launch picked one, else
+    /// <paramref name="descriptor"/>'s <c>ResolveDefaultModel</c>. Shares
+    /// <see cref="LaunchPickedTheModel"/> with the fallback notice, so which value won here and
+    /// whether the user is told it was dropped cannot disagree. The merged value is still a bare
Evidence
Rule 24 requires comments to be sparse and to explain non-obvious constraints rather than duplicate
implementation. The added XML documentation spells out the conditional precedence and the exact
helper relationship immediately implemented by the one-line method.

CLAUDE.md: Comments Must Be Sparse, Current, and Explain Live Non-Obvious Constraints
src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntimeFactory.cs[704-713]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Replace the extended `ResolveRequestedModel` implementation narrative with a brief comment only if a non-obvious live constraint still needs explanation.

## Issue Context
Rule 24 requires comments to be sparse and useful, and prohibits comments that merely restate code. The method body already expresses model-pick precedence through `LaunchPickedTheModel`.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntimeFactory.cs[704-710]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Test docs narrate history ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The new class documentation says the fallback was visible “until now” only in logs and describes the
former launcher behavior. This is prohibited change-history narration rather than a concise
statement of what the tests pin.
Code

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AcpModelFallbackNoticeTests.cs[R11-14]

+/// A requested model the vendor does not publish is dropped for the vendor's default — correct, and
+/// until now visible only in the daemon log, so the launcher kept showing the model the user picked
+/// while another one answered. These pin the note that tells them, and pin that a model which WAS
+/// applied stays silent (a note on every launch would train the user to ignore it).
Evidence
Rule 24 bans development-history narration in comments, and Rule 25 allows historical references
only for live action, measurement, or compatibility. The added phrases until now and `kept
showing` describe obsolete behavior rather than the current test contract.

CLAUDE.md: Comments Must Be Sparse, Current, and Explain Live Non-Obvious Constraints
CLAUDE.md: Historical References in Comments Are Allowed Only for Live Action, Measurement, or Compatibility
test/Capacitor.Cli.Daemon.Tests.Unit/Services/AcpModelFallbackNoticeTests.cs[11-14]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Remove the historical narrative from the test class documentation and state only the live behaviors pinned by the tests.

## Issue Context
Rules 24 and 25 prohibit comments that narrate prior behavior or how the code changed. Test documentation may concisely state the behavior under test and a necessary precondition.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/Services/AcpModelFallbackNoticeTests.cs[11-14]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Fallback reason is misreported ✓ Resolved 🐞 Bug ≡ Correctness
Description
EmitModelFallbackNote says the vendor does not offer the requested model whenever _resolvedModel
is null, but null also results when an offered model's selection RPC fails or when the vendor has no
selection hook. Users are therefore given a false availability diagnosis even though the actual
problem may be applying the model.
Code

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[2410]

+            Text: $"{_vendor} does not offer the model '{requestedModel}'; this session is running {_vendor}'s default model instead.",
Evidence
The runtime reduces every selector failure to _resolvedModel == null and emits the same “does not
offer” text. The selectors explicitly return null after an RPC failure for a model that already
matched the published list, and the no-op selector returns null without inspecting that list at all.

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1023-1038]
src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[2403-2411]
src/Capacitor.Cli.Daemon/Acp/IAcpModelSelector.cs[110-129]
src/Capacitor.Cli.Daemon/Acp/IAcpModelSelector.cs[188-201]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The fallback note claims the vendor does not offer the selected model for every null selection result, although null also represents model-application RPC failures and vendors without a selection hook.

## Issue Context
Either preserve a typed selection outcome/reason from the selector through the runtime, or use accurate neutral wording such as “could not apply” when the reason is unavailable.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1034-1038]
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[2403-2411]
- src/Capacitor.Cli.Daemon/Acp/IAcpModelSelector.cs[110-129]
- src/Capacitor.Cli.Daemon/Acp/IAcpModelSelector.cs[188-201]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Fallback note can disappear ✗ Dismissed 🐞 Bug ☼ Reliability
Description
The note is emitted before the initial turn and becomes the oldest item in a 2,000-entry
DropOldest transcript channel, while forwarding starts only after the server bind completes. If
that bind stalls and the initial turn produces enough envelopes, the note is evicted, so the
promised fallback disclosure never reaches the transcript after reconnection.
Code

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[R1037-1038]

+        if (modelWasPickedForThisLaunch && !string.IsNullOrWhiteSpace(requestedModel) && _resolvedModel is null)
+            EmitModelFallbackNote(requestedModel);
Evidence
The new note is synchronously enqueued before the initial turn, the transcript buffer explicitly
evicts its oldest envelope at capacity, and the sole forwarder does not begin draining until
AcpSessionStartedAsync returns. A delayed bind therefore leaves initial-turn output free to evict
this first envelope.

src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[197-228]
src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1034-1047]
src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1760-1793]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[4001-4015]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The startup fallback note can be evicted as the oldest buffered envelope before transcript forwarding starts.

## Issue Context
Preserve the disclosure independently until the forwarder is bound, or otherwise make this startup system note non-evictable while retaining bounded buffering for ordinary runtime envelopes.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[197-228]
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1034-1047]
- src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs[1760-1793]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[4001-4015]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread test/Capacitor.Cli.Daemon.Tests.Unit/Services/AcpModelFallbackNoticeTests.cs Outdated
Comment thread src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntimeFactory.cs Outdated
Comment thread src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs Outdated
Comment thread src/Capacitor.Cli.Daemon/Services/AcpHostedAgentRuntime.cs Outdated
realtonyyoung and others added 2 commits August 31, 2026 15:30
Which of the three the selector hit does not reach the runtime, so the note
cannot name one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…elector

A vendor whose selector cannot apply a model is the case most in need of the
note, and the clear that keeps the dashboard honest is what hid it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@realtonyyoung
realtonyyoung merged commit 2b60373 into main Aug 31, 2026
6 checks passed
@realtonyyoung
realtonyyoung deleted the claude-tyoung/ai-2385-model-fallback-note branch August 31, 2026 20:33
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.

Launcher lets a remembered model ride a different vendor's launch

1 participant