Hotfix/fe core realtime chat pipeline#1491
Closed
pavlo-flamingo wants to merge 8 commits into
Closed
Conversation
…econnect gaps, multi-connection NATS Realtime chunk pipeline fixes across the NATS chat stack: useNatsChatAdapter: - honor SegmentsUpdateMetadata.append: post-MESSAGE_END continuation fragments append into the trailing bubble instead of replacing it - wire onToolExecuted (cross-message merge + append fallback), onApprovalResolved (no more empty-emit bubble wipe), onUserMessage / onDirectMessage / onSystemMessage (multi-party messages rendered) - reconnect back-fill: buffer live chunks BEFORE the history refetch, then resetAndCatchUp; adopt-once flag cleared in finally so a stale flag can't let the next turn overwrite a completed bubble - pass displayApprovalTypes / approvalStatuses through to the processor useRealtimeChunkProcessor: - reset() clears isInStream/hasEverStreamed (no cross-dialog leak) - escalated approval_result post-END emits the resolved card as an append delta instead of a cumulative replace that wiped the bubble useChunkCatchup: - live path now CHECKS dedup keys (fetch/live overlap rendered twice) - id-less chunks: keep in boundary filters, sort as newest, no dedup collapse of identical streaming deltas - per-messageType boundaries (legacy Redis transport has independent seq counters per chatType; one stream's END no longer truncates the other's tool chunks) - catch-up requested mid-fetch is queued and re-run, not dropped - resetAndCatchUp keeps a caller-armed buffer Subscriptions: - topics identity removed from effect deps (teardown/resub churn lost messages on plain NATS) - JetStream: highestStreamSeq reset moved BEFORE the subscribe effect (dialog switch no longer resumes from the previous dialog's offset) shared-connection: Map keyed by URL instead of a single slot — two surfaces on different endpoints (/ws/nats + /ws/nats-api) no longer force-close each other's socket. process-historical-messages: forward batchApprovalsEnabled into processMessageData so history matches realtime rendering. Adds unit tests for the adapter's message updaters.
- catchup: scope queued catch-ups to their originating dialog (a stale fetch's rerun can no longer apply dialog A's offset to dialog B) and guard the finally block so a stale-dialog fetch never touches the new dialog's cycle state; per-messageType resume checkpoints (legacy Redis streams number each chatType independently — one global offset skipped the slower stream's newer chunks); startInitialBuffering is idempotent (a second reconnect no longer erases the buffer collected during the first back-fill's history await) - adapter + accumulator + consumer note: never downgrade a 'done' batch execution slot back to 'executing' on a redelivered EXECUTING chunk - appendToTrailingAssistant: approval deltas upsert by request id (batch) / (requestId, command) (legacy) — replayed escalated-result emits no longer duplicate the card - participant dedup reworked to layered identity: streamSeq seen-set first, one-shot optimistic-echo consumption second, and a short same-author content window only for seq-less transports — repeated messages stay visible and an admin's text can no longer suppress a user's identical message (and vice versa) - process-historical-messages: displayApprovalTypes defaults to ['CLIENT'], matching the realtime processor (non-client approvals no longer appear as actionable cards only after reload) +3 unit tests (batch no-downgrade, approval-delta upserts).
…eline' into hotfix/fe-core-realtime-chat-pipeline
Follow-up review (event-sequence + parallel-path angles) surfaced 9 confirmed defects beyond the first CodeRabbit pass; fix the load-bearing ones in the lib: - process-historical-messages: revert the displayApprovalTypes default back to 'omitted = show every type'. Defaulting it to ['CLIENT'] made consumers that pass a wider list to realtime but omit it on history (tickets/mingo admin) silently drop pending ADMIN approval cards on every reload/reconnect refetch. Parity is now opt-in via an explicit list. Add a regression test. - use-chunk-catchup: the stale-dialog finally now only discards a queued catch-up that belongs to the stale dialog (was unconditionally nulling it, wiping a catch-up queued for the CURRENT dialog -> permanent transcript hole). The queued re-run is now awaited, not fire-and-forget, so it stays inside the caller's onAgentBusy suppression window (a detached re-run replayed a dead-tail EXECUTING chunk with suppression already lowered, locking the composer forever). - use-nats-chat-adapter: suppressAgentBusyRef is now a COUNTER, so an overlapping initial-catchup and reconnect back-fill no longer drop each other's suppression mid-replay. Reconnect back-fill and initial catchup now scope their flag clears (adoptTrailingAssistant, resetAndCatchUp) to the still-active dialog via currentDialogIdRef, so a stale continuation after a dialog switch can't clobber the new dialog's state. adoptTrailingAssistant is armed only when the catchup buffer is still active (empty replay past the 10-min retention no longer leaves the flag set for the next genuine turn to overwrite the partial bubble). onUserMessage records streamSeq into the seen-set BEFORE the optimistic-echo early return, closing a duplicate-on-redelivery hole. Add .claude/skills/realtime-chat-review — the two extra review angles (event-sequence adversary, parallel-path consistency) this pass used. Lib: type-check clean, 270 tests (+3).
Client→viewer duplicate: a CLIENT user message sent from the Tauri client renders TWICE in the tickets viewer (assistant reply once). Root cause is a missing backend signal, not a double publish: the AI path persists the user MESSAGE_REQUEST row with lastChunkStreamSeq=null (DIRECT/SYSTEM rows persist the chunk seq). So the persisted user row contributes 0 to maxPersistedStreamSeq → optStartSeq never covers the user chunk → JetStream replays it → a fresh user- synthetic is minted that neither seq-coverage (history max never reaches its streamSeq) nor wall-clock (replay has a fresh timestamp) can drop, and the merge's content-fallback was assistant-only. The assistant replay collapses via that fallback; the user one survives — 'user twice, Fae once'. mergeHistoryWithRealtime now also collapses a user- synthetic against an identical-text persisted user twin that is NOT the trailing history row — a user row followed by another message is a completed turn, so the synthetic is provably its replay. A trailing (or absent) twin is left alone so a just-sent message's live echo survives. +3 tests. This is a targeted workaround; the durable fix is the backend stamping lastChunkStreamSeq on user rows (see PR discussion) — that also stops the replay at the source.
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR updates realtime chat delivery across NATS connections, JetStream catch-up, dialog subscriptions, message aggregation, approval handling, history merging, and reconnect flows. It also adds regression tests and a review skill covering event ordering, deduplication, stale async state, and parallel processing paths. ChangesRealtime chat pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant NATS
participant Catchup
participant History
participant Adapter
participant Processor
NATS->>Catchup: deliver live chunks
Catchup->>History: request per-type history
History-->>Catchup: return historical chunks
Catchup->>Adapter: flush ordered deduplicated chunks
Adapter->>Processor: process segments and metadata
Processor->>Adapter: append continuations or resolved approvals
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openframe-frontend-core/.claude/skills/realtime-chat-review/SKILL.md`:
- Around line 30-31: Update the SYSTEM / DIRECT_MESSAGE guidance in the
realtime-chat review skill to state that these messages are excluded from the
local chunk store but remain recoverable through persisted history and JetStream
replay on reconnect, removing the inaccurate “only from Mongo history” wording.
- Around line 20-22: Update the CHAT_CHUNKS retention guidance in the realtime
chat review skill to require history refetch only when a reconnect gap is older
than 10 minutes or falls outside the retained JetStream range; state that gaps
within retention can be recovered through catch-up.
In `@openframe-frontend-core/src/components/chat/hooks/use-chunk-catchup.ts`:
- Around line 348-356: In the fetch completion flow, validate dialog ownership
immediately after Promise.all resolves and return before processing any results
when dialogId no longer matches dialogIdRef.current. Ensure stale fetches cannot
drain the new dialog’s buffer, invoke its callback, or mark catch-up complete;
retain the existing finally guard for cleanup.
In `@openframe-frontend-core/src/components/chat/hooks/use-nats-chat-adapter.ts`:
- Around line 219-227: Normalize displayApprovalTypes in the adapter to a stable
['CLIENT'] default when omitted, then reuse that normalized value for both
realtime and history processing paths. Update the relevant adapter options and
processor invocations so neither path receives undefined or independently
applies different defaults.
- Around line 765-768: Update the optimistic echo tracking around
pendingEchoTextsRef and publishUserMessage so each pending send uses a unique
token rather than only its text. On publish rejection, remove that exact token
before rethrowing, and preserve the one-entry consumption behavior when the
MESSAGE_REQUEST echo arrives across the related send and participant-message
handling paths.
- Around line 445-453: Update the approval_batch branch in the merge logic to
merge the incoming resolution delta with the existing segment instead of
replacing it. Preserve hydrated data.executions and other existing batch fields
when the delta omits them, while applying the delta’s updated fields and
retaining the existing request-id deduplication behavior.
In `@openframe-frontend-core/src/components/chat/utils/history-merge.ts`:
- Around line 257-260: Update the user-message filtering logic around
processedToUse and twinIdx so historical text equality alone cannot deduplicate
synthetic user messages. Require stronger replay identity, such as a matching
synthetic assistant replay or stream sequence, before returning false; otherwise
preserve the new message, and add coverage for repeated content during
persistence lag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f5a078dd-4ad4-421e-922f-103d953bd512
📒 Files selected for processing (13)
openframe-frontend-core/.claude/skills/realtime-chat-review/SKILL.mdopenframe-frontend-core/src/components/chat/hooks/__tests__/nats-adapter-message-updaters.test.tsopenframe-frontend-core/src/components/chat/hooks/use-chunk-catchup.tsopenframe-frontend-core/src/components/chat/hooks/use-jetstream-dialog-subscription.tsopenframe-frontend-core/src/components/chat/hooks/use-nats-chat-adapter.tsopenframe-frontend-core/src/components/chat/hooks/use-nats-dialog-subscription.tsopenframe-frontend-core/src/components/chat/hooks/use-realtime-chunk-processor.tsopenframe-frontend-core/src/components/chat/utils/__tests__/history-merge.test.tsopenframe-frontend-core/src/components/chat/utils/__tests__/process-historical-messages-approvals.test.tsopenframe-frontend-core/src/components/chat/utils/history-merge.tsopenframe-frontend-core/src/components/chat/utils/message-segment-accumulator.tsopenframe-frontend-core/src/components/chat/utils/process-historical-messages.tsopenframe-frontend-core/src/nats/shared-connection.ts
Comment on lines
+20
to
+22
| - **CHAT_CHUNKS retention is 10 minutes.** A reconnect after a gap cannot be | ||
| healed from the stream alone — history refetch is mandatory; missing it is | ||
| a bug, not an edge case. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Limit the history-refetch requirement to gaps outside retention.
As written, this says every reconnect gap requires history, but gaps within the 10-minute JetStream retention window are recoverable through catch-up. Clarify this as “a gap older than 10 minutes or outside the retained range requires history refetch” to avoid false review findings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openframe-frontend-core/.claude/skills/realtime-chat-review/SKILL.md` around
lines 20 - 22, Update the CHAT_CHUNKS retention guidance in the realtime chat
review skill to require history refetch only when a reconnect gap is older than
10 minutes or falls outside the retained JetStream range; state that gaps within
retention can be recovered through catch-up.
Comment on lines
+30
to
+31
| - **SYSTEM / DIRECT_MESSAGE are instant types** — never stored in the chunk | ||
| store, recoverable only from Mongo history. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Clarify SYSTEM/DIRECT_MESSAGE persistence versus local chunk storage.
The supplied history-merge.ts contract states that these messages are persisted and replayed by JetStream on reconnect, so “recoverable only from Mongo history” is inaccurate or ambiguous. Specify that they are excluded from the local chunk store while remaining recoverable through persisted history and replay.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openframe-frontend-core/.claude/skills/realtime-chat-review/SKILL.md` around
lines 30 - 31, Update the SYSTEM / DIRECT_MESSAGE guidance in the realtime-chat
review skill to state that these messages are excluded from the local chunk
store but remain recoverable through persisted history and JetStream replay on
reconnect, removing the inaccurate “only from Mongo history” wording.
Comment on lines
+348
to
+356
| // A fetch whose dialog is no longer active must not touch the NEW | ||
| // dialog's cycle state — `resetChunkTracking` already re-armed the | ||
| // flags for it, and clearing `fetchingInProgress` / finalizing the | ||
| // buffer here would corrupt the new dialog's in-flight catch-up. | ||
| // That includes the pending queue: only discard an entry that belongs | ||
| // to THIS stale dialog — a reconnect during the new dialog's initial | ||
| // fetch queues an entry for the new dialog, and wiping it here would | ||
| // silently drop that gap's re-fetch (permanent transcript hole). | ||
| if (dialogId !== dialogIdRef.current) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject stale fetch results before processing them.
This finally guard runs too late: an old dialog's resolved fetch can already drain the new dialog's buffer, invoke its current callback, and mark its catch-up complete. Check dialog ownership immediately after Promise.all.
Proposed fix
const allChunkResults = await Promise.all(chunkPromises)
+ if (dialogId !== dialogIdRef.current) {
+ return
+ }
const allCatchupChunks: BufferedChunk[] = allChunkResults.flat()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // A fetch whose dialog is no longer active must not touch the NEW | |
| // dialog's cycle state — `resetChunkTracking` already re-armed the | |
| // flags for it, and clearing `fetchingInProgress` / finalizing the | |
| // buffer here would corrupt the new dialog's in-flight catch-up. | |
| // That includes the pending queue: only discard an entry that belongs | |
| // to THIS stale dialog — a reconnect during the new dialog's initial | |
| // fetch queues an entry for the new dialog, and wiping it here would | |
| // silently drop that gap's re-fetch (permanent transcript hole). | |
| if (dialogId !== dialogIdRef.current) { | |
| const allChunkResults = await Promise.all(chunkPromises) | |
| if (dialogId !== dialogIdRef.current) { | |
| return | |
| } | |
| const allCatchupChunks: BufferedChunk[] = allChunkResults.flat() | |
| // A fetch whose dialog is no longer active must not touch the NEW | |
| // dialog's cycle state — `resetChunkTracking` already re-armed the | |
| // flags for it, and clearing `fetchingInProgress` / finalizing the | |
| // buffer here would corrupt the new dialog's in-flight catch-up. | |
| // That includes the pending queue: only discard an entry that belongs | |
| // to THIS stale dialog — a reconnect during the new dialog's initial | |
| // fetch queues an entry for the new dialog, and wiping it here would | |
| // silently drop that gap's re-fetch (permanent transcript hole). | |
| if (dialogId !== dialogIdRef.current) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openframe-frontend-core/src/components/chat/hooks/use-chunk-catchup.ts`
around lines 348 - 356, In the fetch completion flow, validate dialog ownership
immediately after Promise.all resolves and return before processing any results
when dialogId no longer matches dialogIdRef.current. Ensure stale fetches cannot
drain the new dialog’s buffer, invoke its callback, or mark catch-up complete;
retain the existing finally guard for cleanup.
Comment on lines
+219
to
+227
| /** | ||
| * Approval types rendered as actionable cards inline. Mirrors | ||
| * `UseRealtimeChunkProcessorOptions.displayApprovalTypes` (default | ||
| * `['CLIENT']`) and is forwarded to the history processor so both paths | ||
| * agree. Hosts whose backend emits other types (e.g. `USER`) MUST set this | ||
| * — otherwise those approvals are escalated to a callback this adapter | ||
| * doesn't surface and the card never renders. | ||
| */ | ||
| displayApprovalTypes?: string[] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Normalize displayApprovalTypes once for both processing paths.
When omitted, realtime defaults to ['CLIENT'], while history receives undefined and displays every type. Non-CLIENT approvals can therefore disappear live and reappear after history hydration. Use a stable ['CLIENT'] default in the adapter and always forward it to both processors.
Also applies to: 684-684, 1163-1165, 1217-1220
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openframe-frontend-core/src/components/chat/hooks/use-nats-chat-adapter.ts`
around lines 219 - 227, Normalize displayApprovalTypes in the adapter to a
stable ['CLIENT'] default when omitted, then reuse that normalized value for
both realtime and history processing paths. Update the relevant adapter options
and processor invocations so neither path receives undefined or independently
applies different defaults.
Comment on lines
+445
to
+453
| } else if (seg.type === 'approval_batch') { | ||
| // Approval deltas must be IDEMPOTENT: the escalated-result emit can be | ||
| // seen twice (live + catch-up replay over hydrated history), so upsert | ||
| // by request id instead of raw-appending a duplicate card. | ||
| const idx = merged.findIndex( | ||
| (m) => m.type === 'approval_batch' && m.data.approvalRequestId === seg.data.approvalRequestId, | ||
| ) | ||
| if (idx !== -1) merged[idx] = seg | ||
| else merged.push(seg) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Merge approval deltas instead of replacing the batch.
A hydrated batch may already contain completed data.executions. Replacing it with a resolution delta—which supplies no executions—erases that state during replay.
Proposed fix
- if (idx !== -1) merged[idx] = seg
- else merged.push(seg)
+ if (idx !== -1) {
+ const existing = merged[idx] as Extract<
+ MessageSegment,
+ { type: 'approval_batch' }
+ >
+ merged[idx] = {
+ ...existing,
+ ...seg,
+ data: {
+ ...existing.data,
+ ...seg.data,
+ executions: seg.data.executions ?? existing.data.executions,
+ },
+ resolvedByName: seg.resolvedByName ?? existing.resolvedByName,
+ }
+ } else {
+ merged.push(seg)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } else if (seg.type === 'approval_batch') { | |
| // Approval deltas must be IDEMPOTENT: the escalated-result emit can be | |
| // seen twice (live + catch-up replay over hydrated history), so upsert | |
| // by request id instead of raw-appending a duplicate card. | |
| const idx = merged.findIndex( | |
| (m) => m.type === 'approval_batch' && m.data.approvalRequestId === seg.data.approvalRequestId, | |
| ) | |
| if (idx !== -1) merged[idx] = seg | |
| else merged.push(seg) | |
| } else if (seg.type === 'approval_batch') { | |
| // Approval deltas must be IDEMPOTENT: the escalated-result emit can be | |
| // seen twice (live + catch-up replay over hydrated history), so upsert | |
| // by request id instead of raw-appending a duplicate card. | |
| const idx = merged.findIndex( | |
| (m) => m.type === 'approval_batch' && m.data.approvalRequestId === seg.data.approvalRequestId, | |
| ) | |
| if (idx !== -1) { | |
| const existing = merged[idx] as Extract< | |
| MessageSegment, | |
| { type: 'approval_batch' } | |
| > | |
| merged[idx] = { | |
| ...existing, | |
| ...seg, | |
| data: { | |
| ...existing.data, | |
| ...seg.data, | |
| executions: seg.data.executions ?? existing.data.executions, | |
| }, | |
| resolvedByName: seg.resolvedByName ?? existing.resolvedByName, | |
| } | |
| } else { | |
| merged.push(seg) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openframe-frontend-core/src/components/chat/hooks/use-nats-chat-adapter.ts`
around lines 445 - 453, Update the approval_batch branch in the merge logic to
merge the incoming resolution delta with the existing segment instead of
replacing it. Preserve hydrated data.executions and other existing batch fields
when the delta omits them, while applying the delta’s updated fields and
retaining the existing request-id deduplication behavior.
Comment on lines
+765
to
+768
| // Texts `sendMessage` rendered optimistically and whose MESSAGE_REQUEST | ||
| // echo hasn't come back yet — the echo consumes exactly ONE entry, so a | ||
| // genuinely repeated send ("yes", "yes") still renders twice. | ||
| const pendingEchoTextsRef = useRef<string[]>([]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear optimistic-echo tracking when publishing fails.
A rejected publishUserMessage leaves its text pending, so a later legitimate same-text participant message can be swallowed. Track pending sends with unique tokens and remove the exact token on rejection before rethrowing.
Also applies to: 1036-1041, 1476-1502
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openframe-frontend-core/src/components/chat/hooks/use-nats-chat-adapter.ts`
around lines 765 - 768, Update the optimistic echo tracking around
pendingEchoTextsRef and publishUserMessage so each pending send uses a unique
token rather than only its text. On publish rejection, remove that exact token
before rethrowing, and preserve the one-entry consumption behavior when the
MESSAGE_REQUEST echo arrives across the related send and participant-message
handling paths.
…reload/stop) Messages the user already saw vanished from the thread on ANY merge recompute after a page reload — triggered by a Stop from another client, a new incoming message, or a reconnect. Affected all three surfaces (openframe-chat, tickets, mingo). Root cause: mergeHistoryWithRealtime dropped a live synthetic via the per-message seq-coverage test 'historyMaxStreamSeq >= m.streamSeq', using a SINGLE global max over all persisted rows as a proxy for 'this synthetic's turn is in history'. That proxy is false when persistence is async/out-of-order or the turn was interrupted: a later or OTHER-ROLE row (e.g. a user MESSAGE_REQUEST) pushes the global max past a synthetic whose own turn isn't in the snapshot, so it's dropped with no twin — and JetStream won't replay it (seq already consumed). The synthetic was only protected while it held the single streamingMessageId exemption, which any recompute strips (stop flips streaming off; a new turn steals the pointer; earlier post-reload synthetics were never exempt at all). Fix: decide coverage against a PER-ROLE max computed from history rows that carry their own streamSeq — a synthetic is covered only when a persisted row of ITS OWN role actually reached its seq. A role with no seq-stamped persisted row (e.g. user rows the backend leaves unstamped) yields 0, so seq-coverage doesn't apply and the synthetic falls through to the content / wall-clock rules rather than being dropped by an unrelated row. Fully backward-compatible: when NO history row is seq-stamped (legacy hosts) the previous global behaviour is unchanged. - history-merge.ts: per-role coverage max + gate; doc updates. +4 tests. - process-historical-messages.ts: carry lastChunkStreamSeq → the processed message's streamSeq (assistant turns take the MAX across the grouped rows; user/system/direct via their row). Both processors. - message.types.ts: HistoricalMessage.lastChunkStreamSeq + ProcessedMessage.streamSeq. Hosts activate it by feeding lastChunkStreamSeq into HistoricalMessage and mapping the processed streamSeq onto the rendered message (done in openframe-chat automatically via node spread; tickets/mingo via explicit map — those consumer edits ride separate PRs). Lib: type-check clean, 277 tests (+4).
This was referenced Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Improvements
Summary by CodeRabbit
Bug Fixes
New Features