Repository navigation
fix(native-chat): match glue against each pending send's own boundary - #14927
brennanb2025 wants to merge 1 commit into
Conversation
Desktop glue matching filtered candidate transcript rows against the OLDEST still-open send's boundary and then matched the ENTIRE open queue against them. A prompt queued after a row landed could be consumed by that row and pruned, so a real queued prompt disappeared with no bubble and no delivery. Carry every send's own boundary into the match: `gluedCandidateRows` tags each candidate row with the pending indices it landed after, and the matcher ends the glue run at the first send the row predates (adjacency is what makes it glue, so an ineligible entry is a stop, not a skip). Sends with no recorded message boundary fall back to their send time, as the exact path already does. Splits the slash-command marker cache out to native-chat-command-marker.ts to keep native-chat-pending.ts under its line budget; no behavior change there. Fixes STA-4477 Refs #14663, #14262
📝 WalkthroughWalkthroughNative chat command-marker handling moved into a dedicated module with scoped caching, Merge Risk: 🔵 Low · up to The change fixes pending-message matching, but it also adds a command marker that is hard-coded in English, so users in other locales may see untranslated text. The PR is otherwise mergeable with explicit owner awareness or a follow-up localization fix. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a59f0d02-280e-402d-b922-d2ee58b73d9e
📒 Files selected for processing (6)
src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsxsrc/renderer/src/components/native-chat/NativeChatView.tsxsrc/renderer/src/components/native-chat/native-chat-command-marker.tssrc/renderer/src/components/native-chat/native-chat-pending-occurrence.tssrc/renderer/src/components/native-chat/native-chat-pending.test.tssrc/renderer/src/components/native-chat/native-chat-pending.ts
Included review availability: Your plan includes up to 10 reviews per rolling hour; 6 remain after this review.
| return markers.map((marker) => ({ | ||
| id: `command:${marker.id}`, | ||
| role: 'system' as const, | ||
| blocks: [{ type: 'text' as const, text: `Ran ${marker.command}` }], | ||
| timestamp: marker.sentAt, | ||
| source: 'scrape' as const | ||
| })) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the command marker text.
Ran ${marker.command} is new user-visible text. Register and use a renderer locale key for the template instead of rendering English directly.
Based on learnings: renderer locale keys use the first 10 SHA-1 characters of <filePath>:<text>.
Source: Learnings
|
Superseded by #14935, which covers the same ticket (STA-4477, per-send boundary in glue matching) from a branch cut fresh off current main and carrying the verification this one lacks: a surgical mutation with both call sites independently guarded, and the lint gate confirmed live via an injected violation rather than inferred from a silent clean run. Same change, verified. Closing this to keep one review surface per ticket. |
Fixes STA-4477. Follow-up to #14663 — not a revert of it (reverting would re-open #14262).
The bug
Desktop native chat renders an optimistic "queued" bubble per composer send and prunes it once the real transcript row lands. When a lost Enter glues two rapid sends into ONE transcript row, neither exact key matches, so #14663 added a glue matcher.
That matcher filtered candidate rows against the oldest still-open send's boundary (
gluedCandidateMessagesreadsopen[0]only) and then matched the entire open queue against them. A prompt queued after a row landed could therefore be consumed by that row and pruned — the bubble vanishes and the prompt is treated as delivered when it is still in flight.Minimal repro (this is a test in the PR, and it was reproduced independently by a separate
codexsession given only the two files):p2's exact path correctly keeps it (messagesAfterPendingBoundary(messages, p2)is empty). The glue path overrides that.The fix
Every send carries its own boundary into the match.
gluedCandidateRowstags each candidate row with the set of pending indices whose boundary it landed after, using the existingmessagesAfterPendingBoundaryper entry.selectPendingIndicesRepresentedByUserRowsends the glue run at the first send the row predates. Glue is an adjacency claim, so an ineligible entry is a stop, not a skip — skipping it would let a row glue across a gap.advancedNativeChatUserTexts/matchingNativeChatUserTextsbecame…UserRowsso rows keep their id and eligibility can be computed per row.The mobile matcher already enforced this per-send (
matchGluedRunchecks each segment's own tail); desktop was the outlier.Tests (RED → GREEN)
Both new tests were run against
origin/mainfirst and fail there:With the fix:
73 files / 768 tests passedacrosssrc/renderer/src/components/native-chat/.typecheck:webandoxlintclean.Note on the file split
native-chat-pending.tswas at its 300-line budget, so the slash-command marker cache (a separate concern that happened to live there) moved tonative-chat-command-marker.ts. No behavior change; nomax-linesdisable or baseline bump.🤖 Generated with Claude Code