docs(sessions): what has to be true for records to be a conversation store - #5497
docs(sessions): what has to be true for records to be a conversation store#5497mmabrouk wants to merge 1 commit into
Conversation
…store Records the outcome of designing a Redis buffer in front of the session-record read, and why that buffer is not the fix it was meant to be. The review found the design's central guarantee false, with four counterexamples, and found three ways the pipeline loses records permanently today (issue #5496). It also surfaces one product decision that cannot be made in code: a turn that may never fail and a history that cannot prove it is complete are contradictory requirements once the client stops sending the conversation. No implementation. The document states the order the work has to happen in and what a later buffer must correct. Claude-Session: https://claude.ai/code/session_01KM69J7uHafgciiN5zfG7qR
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
the
size:L
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
1 participant
Why this exists
#5486 makes the session-record log load-bearing for correctness: the runner rebuilds the conversation from it, so a missing record becomes an agent that silently forgets and answers anyway.
The plan was a Redis buffer in front of the record read, to fix the staleness measured during QA of #5486. This document is the design for that buffer, and the outcome of reviewing it: the buffer is not the fix it was meant to be, and the pipeline underneath it loses records permanently today.
No code. This is a decision document.
What it says
The measured bug is real but the cause was mis-stated. The lag is not the worker's 5000ms blocking read. A blocked
XREADGROUPwakes when an entry arrives; the delay is the deliberate 250ms accumulation window (consumer.py:114), which matches the ~213ms measured.Three ways records are lost permanently today, filed separately as #5496:
{"ok": true}even when the durable publish failed, so the runner never retries.Records have no stable identity. Only tool-family records carry an id; ordinary messages get a random one minted in the worker's mapping. So a redelivery inserts a second row for the same message, and nothing outside Postgres can deduplicate against the eventual row.
The buffer's guarantee was false, with four counterexamples in the document. The sharpest: EE retention deletes the oldest rows, so a query legitimately returns a conversation whose beginning is gone, and the runner treats any non-empty result as complete.
The decision this forces
Two requirements now contradict each other, and the choice is a product call:
Once the client stops keeping the conversation, both cannot hold. The document lays out the two options with costs and recommends reporting completeness on the query and failing the turn when history is provably incomplete, because a visible failure is recoverable and a confident wrong answer is not.
What happens next
Nothing gets built on top of this store until it stops losing records. The document states the order: fix the loss paths, give records a stable identity, make the completeness decision, and only then build a buffer, with corrected claims and a list of what to change from the first attempt.
Reviewed at high reasoning effort by an independent model before publication; its findings are folded in rather than appended.