Skip to content

fix: Streaming assistant snapshots rewrite FTS rows on every upsert#827

Closed
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-5730cba8
Closed

fix: Streaming assistant snapshots rewrite FTS rows on every upsert#827
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-5730cba8

Conversation

@sam-saffron-jarvis

Copy link
Copy Markdown
Contributor

What changed

  • Added a streaming-aware message update path in internal/session so interim assistant snapshot updates can skip rewriting messages.text_content until the assistant message is finalized.
  • Narrowed the SQLite FTS update trigger from AFTER UPDATE ON messages to AFTER UPDATE OF text_content ON messages, plus added a schema migration so existing databases pick up the same behavior.
  • Updated the hot streaming persistence call sites in both cmd/serve_runtime.go and internal/tui/chat/streaming.go to:
    • persist ordinary snapshots without touching FTS-backed text,
    • persist final assistant text once on response completion, and
    • avoid a duplicate final FTS rewrite when turn completion follows response completion.
  • Added a regression test proving snapshot updates no longer surface in FTS until finalization, while the final assistant text is still searchable.

Why this is high-value

This path is hit repeatedly during real streaming turns in both the web runtime and TUI. Before this change, every snapshot update rewrote the entire FTS row because UpdateMessage always updated text_content and the messages_au trigger reindexed on every update.

The fix reduces work in the hot path from "reindex the growing assistant reply on every snapshot" to "update non-FTS message fields during snapshots, then index the final text once". That cuts SQLite write amplification, reduces lock contention, and improves responsiveness on long assistant replies and tool-heavy turns without changing final search behavior.

Validation

  • Verified the issue in current code by tracing UpdateMessage updating text_content on every snapshot and the unconditional messages_au FTS trigger.
  • Added TestSQLiteStoreUpdateStreamingMessageDefersFTSRewriteUntilFinalized to cover the exact regression.
  • Ran:
    • gofmt -w cmd/serve_runtime.go internal/session/store.go internal/session/logger.go internal/session/sqlite.go internal/session/update_message_test.go internal/tui/chat/chat.go internal/tui/chat/streaming.go
    • go build ./...
    • go test ./...
    • git diff --check

@SamSaffron

Copy link
Copy Markdown
Owner

Implemented directly in main working tree with the same fixes and regression coverage.

@SamSaffron SamSaffron closed this Jun 14, 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.

2 participants