Skip to content

fix(conversations): compare-and-swap the denormalized message cache - #877

Open
leseb wants to merge 1 commit into
praxis-proxy:mainfrom
leseb:leseb/validate-issue-662
Open

fix(conversations): compare-and-swap the denormalized message cache#877
leseb wants to merge 1 commit into
praxis-proxy:mainfrom
leseb:leseb/validate-issue-662

Conversation

@leseb

@leseb leseb commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Concurrent conversation item appends each rebuilt the denormalized messages cache from the authoritative item rows and wrote it back unconditionally, so a slower writer could overwrite a newer writer's cache with a stale snapshot and drop items from the history that rehydration consumes. This replaces the blind write with a compare-and-swap that retries a lost race with a freshly read snapshot, and reuses the create/delete caller's existing pre-mutation snapshot to skip a redundant cache reload; because the written value is always a fresh rebuild from item rows, a stale snapshot only ever loses the swap and never persists stale data. Exhausting the bounded retries logs and returns Ok since the row mutation is already durably committed and any later write rebuilds the cache, while genuine store errors and a disappeared conversation still propagate.

Related issue

Closes #662

Validation

  • Unit tests — cargo test -p praxis-ai-apis (2592 passed, 0 failed; six Concurrent item appends can overwrite the denormalized conversation history #662 cache-sync tests covering stale-cache repair, concurrent-clobber, snapshot fast-path with no reload, stale-snapshot fresh-read fallback, missing-conversation error, and retry exhaustion)
  • Integration or functional tests — n/a; the concurrency is exercised deterministically via a fault-injecting store wrapper over the real SQLite store, and there is no new config surface to add a functional example test for
  • make lint and make build

Checklist

  • I reviewed every changed line and can explain the change.
  • New capabilities include an example config and functional example test. — n/a; bug fix, no new capability or config surface
  • User-facing behavior and generated documentation are updated. — n/a; internal cache-consistency behavior only
  • Performance-sensitive changes include appropriate benchmark or load-test evidence. — n/a; correctness fix (bounded CAS retries only on contention)
  • Commits are signed and include a Signed-off-by trailer.

Breaking changes

None. sync_conversation_messages is a crate-internal (pub(super)) helper; the signature change is not part of any public API.

Concurrent item appends each rebuilt the denormalized conversation `messages` cache from the authoritative item rows and wrote it back unconditionally, so a slower writer could clobber a newer writer's cache with a stale snapshot, dropping items from the history that rehydration consumes (praxis-proxy#662).

Rebuild the cache and write it with a compare-and-swap, retrying a lost race with a freshly read snapshot. Create/delete callers reuse the snapshot they already read before their row mutation; the append-back path passes none. The written value is always a fresh rebuild, so a stale snapshot only loses the swap and never writes stale data.

Exhausting the bounded retries logs a warning and returns Ok: the item-row mutation is already durably committed and any later write rebuilds the cache, so failing here would report a committed create/delete as an error. Genuine store errors and a disappeared conversation still propagate.

Signed-off-by: Sébastien Han <seb@redhat.com>
@leseb
leseb requested review from a team and aslakknutsen September 2, 2026 10:51

@franciscojavierarceo franciscojavierarceo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reviewed the current head, all available comments and review threads, the architectural paths, and the completed checks. i found no actionable issues. approving this commit.

@leseb
leseb added this pull request to the merge queue Sep 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Sep 2, 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.

Concurrent item appends can overwrite the denormalized conversation history

2 participants