Show one live question as a row in the chat - #936
nortonandreev wants to merge 2 commits into
Conversation
PR Summary by QodoRender deduplicated live questions in the chat transcript
AI Description
Diagram
High-Level Assessment
Files changed (12)
|
Code Review by Qodo
1. Repeated prompts hide live requests
|
| if (local.ServerRequestId is null) { | ||
| local.ServerRequestId = twin.RequestId; | ||
| _cache.Refresh(local); |
There was a problem hiding this comment.
2. Old prompts return after reconnecting 🐞 Bug ≡ Correctness
ShadowTwin writes a heuristic server identifier into the local request, but the later exact daemon mapping overwrites it without releasing the previously shadowed server item. If the text join initially selects the wrong same-prompt request, that request remains hidden and is restored as pending when the local subscription reconnects.
Agent Prompt
## Issue description
A prompt-derived server mapping can be superseded by the daemon's exact mapping while its old server request remains indefinitely in the shadow dictionary.
## Fix Focus Areas
- src/Capacitor.App/Services/PermissionService.cs[230-248]
- src/Capacitor.App/Services/PermissionService.cs[269-276]
- src/Capacitor.App/Services/PermissionService.cs[285-295]
## Recommended Fix
Track whether a local mapping was inferred from prompt text. Before replacing an inferred identifier with a different daemon-provided identifier, remove the old twin from `_shadowed` and restore it to the cache when it is still pending and not tombstoned, then shadow only the confirmed identifier.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Same case as the prompt-join thread: overwriting ServerRequestId with a different id only happens if the text join already picked the wrong twin. One local card plus one server elicitation is a no-op overwrite; a leftover shadow does not arise on reconnect.
| public async Task A_question_header_paints_like_the_tool_kind_chip() { | ||
| await RunOnUiAsync(async () => { | ||
| var host = new Host(); | ||
| await host.LoadAsync(Tmp.CreateFile("ask.jsonl", [ |
There was a problem hiding this comment.
4. Failed smoke checks leave test windows open 🐞 Bug ☼ Reliability
The newly added smoke tests create a Host and invoke CloseAsync only after all assertions succeed, rather than in a finally block. If a wait or assertion throws, the Avalonia window plus chat and terminal subscriptions survive into subsequent tests in the process-global UI session.
Agent Prompt
Issue description
The new headless UI tests close their hosts only on the success path. A failed assertion before that call leaves the window and view-model subscriptions alive, contaminating the shared Avalonia test session.
Fix Focus Areas
- test/Capacitor.App.Tests.Unit/ChatTabViewSmokeTests.cs[986-1089]
Recommended Fix
Wrap each newly added `Host` lifecycle in `try`/`finally` and call `await host.CloseAsync()` in the `finally` block. Apply this to every new test that creates a host, so cleanup occurs after failures as well as successful assertions.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Same CloseAsync-after-assert pattern as the rest of ChatTabViewSmokeTests. These tests already share the Avalonia session lock; wrapping only the new ones would not change the suite contract.
| foreach (var twin in _cache.Items.Where(i => i.Lane == PermissionLane.Server && local.SameQuestionAs(i)).ToList()) | ||
| ShadowTwin(local, twin); |
There was a problem hiding this comment.
1. Repeated prompts hide live requests 🔗 Cross-repo conflict ≡ Correctness
SameQuestionAs, ShadowMatchingServerQuestions, and LocalQuestionClaimant use same-session first-prompt text as request identity, allowing one local card to remove multiple server elicitations while ShadowTwin retains only the first ServerRequestId. When distinct concurrent requests share that prompt, settling the visible card concludes only its recorded ID and leaves the other request pending but inaccessible in _shadowed until the local lane disconnects.
Agent Prompt
## Issue description
Prompt equality is not a unique cross-repository identity. Multiple kcap-server interactions in one session may have identical prompts and distinct request IDs, but the current reconciliation can shadow all of them behind one local card while retaining and concluding only one ID.
## Fix Focus Areas
- src/Capacitor.App/Services/PermissionService.cs[262-283]
- src/Capacitor.App/Services/PendingPermissionRequest.cs[103-122]
## Recommended Fix
Make heuristic prompt-based reconciliation strictly one-to-one. Only consider unmatched local and server questions, never prompt-match a local card that already has a different `ServerRequestId`, and shadow only when exactly one unmatched local candidate and one unmatched server candidate share the fingerprint; stop after assigning one server request to one local request. Preserve exact identifier-based matching for confirmed daemon mappings, leave ambiguous same-prompt requests visible until authoritative request-ID correlation is available, and add tests for repeated concurrent prompts arriving in both orders.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Prompt-text join is the correlation we have until the daemon writes ServerRequestId. Hosted Claude has one live AskUserQuestion; a Bash permission in the same session does not match. Two distinct concurrent questions with the same first-prompt text is not a path here, and requiring a unique unmatched pair would leave a duplicate hub replay as a second card.
51a94a4 to
52d2d24
Compare
|
On the informational finding: PendingCardItem belongs with the other public chat row types in ChatItems.cs (UserTurnItem, ToolGroupItem, and the rest). Splitting it out would not match how this file is owned. |
What & why
Hosted Claude can show the same AskUserQuestion twice — a local radio card and a server prompt-only card — in a docked pane off the transcript.
This shadows the server twin as soon as a local card exists, joining on session plus overlapping prompt text rather than waiting for ServerRequestId, and renders that one card as a virtualized chat row with the composer pinned.
Composer and Other… use the kit field chrome; the question header matches the tool-kind label; a card that follows its tool group drops the paragraph gap so the two read as one block.
Where to look
SameQuestionAsin PermissionService: a Bash permission and an elicitation in the same session must stay two cards.Follow-tail in ChatTabView: a TextBox in the list is editing, so a click on Other… must not ScrollToEnd and recycle the row.
Verification
dotnet run --project test/Capacitor.App.Tests.Unit/Capacitor.App.Tests.Unit.csproj— 1935 succeeded, 0 failed.Visuals
Before
After