Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions assistant/src/api/events/assistant-text-delta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,19 @@
* message; the matching `message_complete` event marks the turn done.
*
* `messageId` is the database row id of the assistant message this
* delta belongs to — stamped from the pre-allocated turn anchor (see
* `reserveMessage` / `AssistantTurnStartEvent`). Absent on streams
* produced by older daemons that pre-date the anchor protocol, or on
* synthetic deltas (canned greetings, slash-command echoes, live-voice
* transcript injections) that don't bind to a row.
* delta belongs to. The main agent loop (post PR 1 of the
* streaming-message-architecture plan) always allocates and emits a
* `messageId` for every delta it produces, via `ensureMessageOpen` in
* `conversation-agent-loop-handlers.ts`. The field stays optional in
* this schema because synthetic emitters that don't bind to a persisted
* row (canned greetings, slash-command echoes, live-voice transcript
* injections, wake-target replays, recording handler echoes) still emit
* deltas without one; those streams are consumed by channel adapters,
* not by the `MessageStreamReducer` path.
*
* `blockIndex` and `seq` are populated whenever `messageId` is, so a
* client receiving any of the three is guaranteed to receive all three
* (idempotent reducer keying invariant — see `MessageStreamReducer`).
*
* Canonical wire-contract source. Daemon code imports the type directly
* from this file; external consumers import via `@vellumai/assistant-api`.
Expand Down
11 changes: 11 additions & 0 deletions assistant/src/api/events/message-complete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@
* side effects on `source !== "aux"`. Absent is treated as `"main"`
* for backwards compatibility.
*
* Streaming-architecture status (post-PR 6): the new addressable-event
* protocol uses `message_open` / `message_close` as the canonical
* lifecycle pair for the `MessageStreamReducer` path. `message_complete`
* remains the canonical end-of-turn signal for the wider event-consumer
* fleet — CLI, voice session bridge, channel retry sweep, background
* dispatch — which still gate on it for side effects (task-complete
* sound, attachment delivery, channel idle bookkeeping). It is the
* post-persistence event with the authoritative DB row id, attachments,
* and `source` discriminator; `message_close` is the pre-persistence
* streaming signal and does not carry those fields.
*
* Canonical wire-contract source. Daemon code imports the type directly
* from this file; external consumers import via `@vellumai/assistant-api`.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,14 @@ enum TranscriptProjector {
highlightedMessageId: UUID?,
autoRoutedProfileLabel: String? = nil
) -> TranscriptRenderModel {
// Deduplicate visible messages (streaming can produce duplicate IDs).
let visibleMessages: [ChatMessage] = {
var seen = Set<UUID>()
return paginatedVisibleMessages.filter { seen.insert($0.id).inserted }
}()
// Per streaming-message-architecture PR 6: the MessageStreamReducer is
// the sole writer for assistant content and applies events idempotently
// by `(messageId, blockIndex, seq)`. `renderedMessages` and
// `renderedPaginatedVisibleMessages` on `ChatViewModel` already merge
// legacy rows with `MessageStore` snapshots using stable, deterministic
// ids, so the projector's input is already unique by id and the
// previous defensive dedupe pass is no longer needed.
let visibleMessages = paginatedVisibleMessages

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve row de-dupe before rendering

When duplicate ChatMessage.ids reach the list, this pass-through lets both rows into state.rows; MessageListContentView then builds Dictionary(uniqueKeysWithValues:) from those IDs, which traps on duplicates. Duplicate IDs are still possible outside the reducer path—for example, pagination/history merging prepends chatMessages + self.messages without ID de-duping, so a retried/late older-page response can leave the same history row in the transcript twice. The previous filter made that case harmless; without an equivalent guard upstream, scrolling/loading history can crash the chat view.

Useful? React with 👍 / 👎.


// --- Structural metadata ---

Expand Down
25 changes: 16 additions & 9 deletions clients/macos/vellum-assistantTests/TranscriptProjectorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -446,16 +446,23 @@ final class TranscriptProjectorTests: XCTestCase {
XCTAssertFalse(model.canInlineProcessing)
}

// MARK: - Duplicate Message Deduplication
// MARK: - Message Identity Trust
//
// Per streaming-message-architecture PR 6, the projector no longer
// dedupes by id — the `MessageStreamReducer` is the sole writer for
// assistant content and its idempotent apply means upstream input is
// already unique by id. The renderer trusts that contract.

func testProjectorPreservesInputOrderAndIdentity() {
let msg1 = makeMessage(role: .assistant, text: "First")
let msg2 = makeMessage(role: .user, text: "Second")
let msg3 = makeMessage(role: .assistant, text: "Third")

func testDuplicateMessageIdsAreDeduped() {
let sharedId = UUID()
let msg1 = makeMessage(id: sharedId, role: .assistant, text: "First")
let msg2 = makeMessage(id: sharedId, role: .assistant, text: "Duplicate")

let model = project(messages: [msg1, msg2])
XCTAssertEqual(model.rows.count, 1, "Duplicate IDs should be deduped")
XCTAssertEqual(model.rows[0].message.text, "First", "First occurrence wins")
let model = project(messages: [msg1, msg2, msg3])
XCTAssertEqual(model.rows.count, 3, "All input rows pass through 1:1")
XCTAssertEqual(model.rows[0].message.text, "First")
XCTAssertEqual(model.rows[1].message.text, "Second")
XCTAssertEqual(model.rows[2].message.text, "Third")
}

// MARK: - Active Pending Request ID Pass-Through
Expand Down
34 changes: 19 additions & 15 deletions clients/shared/Features/Chat/MessageStore+ChatMessageBridge.swift
Original file line number Diff line number Diff line change
@@ -1,24 +1,28 @@
import Foundation

/// Bridge from the new `MessageStore` (the streaming-message-architecture
/// reducer's source of truth) to the legacy `ChatMessage` shape that the
/// existing chat renderer consumes.
/// reducer's source of truth) to the `ChatMessage` shape that the existing
/// chat renderer consumes.
///
/// As of PR 4 of the streaming-message-architecture plan, the chat view list
/// renders from `ChatViewModel.renderedMessages`, which merges the legacy
/// `messages` array (user bubbles, history, confirmations, system messages)
/// with assistant content materialized from `MessageStore` snapshots through
/// this bridge.
/// The chat view list renders from `ChatViewModel.renderedMessages`, which
/// merges the legacy `messages` array (user bubbles, history rows, system
/// messages, confirmations) with assistant content materialized from
/// `MessageStore` snapshots through this bridge. For any assistant message
/// the daemon emits over the new addressable-event protocol, the
/// `MessageStore` snapshot wins over the legacy row — the legacy bubble
/// created by `appendTextToCurrentMessage` is silently dropped from the
/// merged transcript.
///
/// The legacy streaming helpers on `ChatViewModel` still mutate the legacy
/// `messages` array as a side effect, but those mutations no longer drive
/// rendering for messages that have a corresponding `MessageStore` snapshot.
/// This is what makes the streaming-then-reload duplication symptom
/// structurally impossible: the renderer reads a single source of truth keyed
/// by the daemon's stable `messageId` regardless of how many times the legacy
/// path lazy-creates bubbles.
/// `MessageStreamReducer` is the sole writer for `MessageStore`, and its
/// `apply()` is idempotent per `(messageId, blockIndex, seq)`. This is what
/// makes the streaming-then-reload duplication symptom structurally
/// impossible: re-applying the same event (e.g. on `Last-Event-Id` reconnect
/// replay from PR 2) is a no-op, and the renderer always reads a single
/// source of truth keyed by the daemon's stable `messageId`.
///
/// PR 6 of the plan will remove the legacy `messages` array entirely.
/// Non-assistant rows and history rows that pre-date the new event protocol
/// still flow through the legacy `messages` array, which is why the merge
/// in `ChatViewModel.renderedMessages` is still needed.
@MainActor
extension MessageStore {

Expand Down
Loading