Skip to content

Swallow omp's trailing post-end assistant snapshot so a reply renders once (fixes #19) - #20

Closed
Errnolink wants to merge 1 commit into
MYSTRAVIL:mainfrom
Errnolink:fix/trailing-snapshot-duplicate
Closed

Errnolink wants to merge 1 commit into
MYSTRAVIL:mainfrom
Errnolink:fix/trailing-snapshot-duplicate

Conversation

@Errnolink

Copy link
Copy Markdown
Contributor

Fixes #19.

What happens

Wire capture from a phone-connected session (one plain turn, no steering):

{"t":"msg","phase":"end","msgId":"assistant-1790488577355","text":"Standing by for your next task"}
{"t":"msg","phase":"start","msgId":"assistant-1790488577355-2","text":"Standing by for your next task"}

omp fires one more message_update carrying the final full snapshot after the assistant message_end. FeedMsgIds is doing its documented job — after end() closed occurrence 1, id() for the same role+timestamp allocates occurrence 2 — so the bridge emits a start for a brand-new row with identical text and the phone renders the reply twice. Reproduced on every text reply in 3/3 fresh sessions.

Fix

The bridge records each ended assistant row (timestamp → {msgId, text}). A post-end update whose text is identical to the ended frame is omp's trailing snapshot repeating itself — swallow it. Text equality is the discriminator available here (the bridge sees no message_start): a genuine second reply that merely shares the millisecond starts from different text and falls through to feedIds.id() as before, keeping its own row — exactly the case the existing same-millisecond test pins.

Testing

  • New regression test feeding the observed wire sequence (delta → end → trailing identical update → next turn) and asserting no second row opens: packages/bridge/test/session-switch.test.ts.
  • bun test packages/bridge: 95 pass, 0 fail. tsc --noEmit clean.
  • Running in production on our fork since yesterday; fresh phone sessions render one row per reply (verified in-browser against a live session).

Caveat

If omp ever emits a post-end update with different text, this opens occurrence 2 (pre-fix behavior) rather than amending the ended row. That signature has not been observed — every captured trailing snapshot matched the ended text — and the alternative (unconditional in-place update) breaks the same-millisecond second-reply semantics the test suite defines.

MYSTRAVIL added a commit that referenced this pull request Sep 27, 2026
…); ask options declare object items for strict providers

Reworked from #20 and #21 by @Errnolink.

Co-authored-by: Errnolink <Errnolink@users.noreply.github.com>
@MYSTRAVIL

Copy link
Copy Markdown
Owner

Thanks, your diagnosis was right: the extra row was a regression from the #8 keying. Landed reworked in 769babb (you're credited as co-author):

  • The check lives in FeedMsgIds (@omp-remote/protocol) as repeatsEnded(role, ts, text), so the Collab translator gets it too. It uses the same keyer, and Collab is the mode our own host runs.
  • It keeps only the last ended message per role rather than a per-timestamp map that grew for the whole session. The trailing snapshot only ever follows the reply that just ended.
  • Your rule is unchanged: same timestamp and same text is dropped, and a new reply in that millisecond still gets its own row.
  • Your regression test is kept, and there is a matching Collab test.

@MYSTRAVIL MYSTRAVIL closed this Sep 27, 2026
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.

Assistant replies render twice: omp's trailing message_update after message_end opens a second FeedMsgIds occurrence

2 participants