feat(sdk): change feed carries every kind and verbatim columns - #260
Conversation
The change feed reports all twelve evidence tables the export journal captures -- adding history, presence, commit_link, trajectory, source_observation and observation_evidence -- and every upsert carries the row as stored (StoredRow: every column but revision, in table order, raw SQLite values, read from the live table) beside the typed row. Every change carries the journal's record key, tombstones included, and a row from an unknown source is carried with source None and source_name set instead of failing the drain. A database the six-kind feed reached stamps the new tables once on open, above its head. The session re-stamp triggers move to their own names and skip a presence's own stamp; the history FTS update trigger fires only on the columns it indexes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 47 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change feed now covers 12 evidence kinds. Changes include identity keys and stored-row columns on upserts, preserve unknown source names, and support migration of existing databases. The history FTS update trigger now watches only indexed columns. ChangesExpanded change feed
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Consumer
participant ChangeFeed
participant SQLite
Consumer->>ChangeFeed: Request changes_since cursor
ChangeFeed->>SQLite: Read revisions and tombstones
SQLite-->>ChangeFeed: Return stored rows and identities
ChangeFeed-->>Consumer: Return typed or untyped changes
Suggested reviewers: Merge Risk: 🔵 Low · up to The change feed now covers twelve evidence kinds and carries stored-row payloads and identity keys. Named consumers that explicitly listed the original six kinds will get an error on their first resume after upgrading. The fix is a new consumer name or a resync from the start. The PR can merge with low risk once the changelog explains this upgrade effect. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.10% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 59 functions across 6 files. (4 skipped: 4 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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. A rabbit checks the feed at dawn Comment |
| changed.push(format!("OLD.{0} IS NOT NEW.{0}", self.session)); | ||
| changed.push(format!( | ||
| "{} IS NOT {}", | ||
| self.record_key_sql("OLD"), | ||
| self.record_key_sql("NEW") | ||
| )); |
There was a problem hiding this comment.
🔴 Existing stores retain incorrect history deletes
On an existing store, init_schema retains the old history triggers because their names already exist. Changing a prompt's session then emits a delete for its unchanged key, followed by an upsert.
Learn more
An existing store has history stamping triggers installed under fixed names. On open, init_schema builds new SQL for these names but uses CREATE TRIGGER IF NOT EXISTS at trigger creation. SQLite retains the old trigger bodies, which treat every session change as an identity change. The feed now expects prompt identity to exclude the session, so a session update produces a false delete and the stored tombstone session differs from what the new code documents.
Example: A store initialized before this change contains a prompt with key ["history","claude",42,"hello"] and no session. Updating its session to s1 generates a delete for that key followed by an upsert, although the prompt still exists under the same key.
Recommended fix: Recreate the history insert, update, and delete triggers in a dedicated migration for stores that already have change_feed_v2. Reconcile any existing history tombstones keyed with nonempty sessions if the new tombstone scheme must clear them on reinsertion; test an upgraded store as well as a fresh one.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Valid, fixed in b6e6a75. A history row's key is (source, timestamp_ms, prompt), so identity_changed_sql now compares the session only when it is part of the key (FedTable::session_keyed). History tombstones store '' as their session, so a tombstone names exactly the record's key and a later insert of that key clears it whatever session it carries. The parity test now asserts that no History delete appears after UPDATE history SET session_id = 'c1'. With the old behavior restored (tombstone keeps the row's session, identity compares it), that assertion fails; with the fix it passes.
There was a problem hiding this comment.
Checked, and no store can be in that state. The history stamping triggers and the change_feed_v2 marker exist only on this PR's branch. On origin/main (9ce674b3, released as 0.27.0) the feed marker is change_feed_v1 and history has no revision column and no change_feed_history_* triggers. No tag contains the branch's first commit (git tag --contains b5ef512f is empty). So every store that reaches change_feed_v2 has its history triggers created by the code at b6e6a75, which compares the session only for kinds that key on it. An upgraded store is covered by a_store_fed_before_every_kind_gains_the_rest_above_its_head, which rewinds a store to the released six-kind schema and migrates it. A trigger-rebuild migration would only serve databases built from an unmerged intermediate commit, so I haven't added one.
A history row is unique on source, timestamp and prompt, so a prompt that gains or changes its session is an upsert of the same record. Its tombstones store an empty session and the identity check skips the session column, so no transient delete sits between the two writes. Document that a cursor stored as `*` now spans all twelve kinds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What the feed provides
SessionStore::changes_sincegives an embedder everything it needs to rebuild each evidence row exactly as stored, on the default public API. An upload daemon can build its records from the feed alone, which lets the Agent Relay desktop probe stop using theexportcapture journal.ChangeKindcoverssession,session_event,tool_call,file_edit,session_marker,relationship,history,presence,commit_link,trajectory,source_observationandobservation_evidence. The wire names are the journal's kinds. Each table gets the same insert/update/delete stamping triggers and tombstones, so every write site is covered.Change::columns: Option<StoredRow>is every column exceptrevision, in table order, with raw SQLite values: JSON text stays text, integers stay integers, NULL staysnull. The columns are read from the live table, so a column a migration adds shows up without a code change. The journal's capture payload is built from the same column list (change_feed::stored_columns), so the two can't drift. The typed rows are unchanged;EvidenceRow::History(HistoryEntry)is new, andEvidenceRow::Untypedmarks the kinds that have no typed row.Change::key: Vec<Value>is the journal'sTable::key,[kind, ...key columns], for example["history", source, timestamp_ms, prompt]or["trajectory", id]. Upserts and tombstones carry the same key. A tombstone rebuilds it fromrecord_key, which for a multi-column identity is a JSON array of the stored values, so a history delete keepstimestamp_msas an integer.Change::sourceisOption<Source>andChange::source_nameholds the stored name. A row written by a newer release under a source this build doesn't know appears withsource: Noneinstead of failing the drain.No consent or exclusion rule is applied here; the daemon does its own selection.
Migration
The feed marker moves to
change_feed_v2. On a database the six-kind feed already reached, opening it writable stamps the new tables' existing rows once, in rowid order, above the current head. A cursor bound to every kind therefore resumes into all of those rows and none it has already seen. The old session re-stamp triggers held the names the presence kind now uses, so they are dropped and recreated aschange_feed_session_locations_*. Their update trigger now skips a presence's own stamp, so a presence write stamps its session exactly once.history_aunow fires only onUPDATE OF id, prompt, project(the same fixsession_events_augot earlier), so the feed's stamp doesn't re-index the history FTS table twice per insert.Proof
change_feed::tests::every_kind_matches_the_export_journal_for_the_same_writes(exportfeature) writes to all 12 tables with a subscription active: inserts, value updates, one identity change (tool_use_id), a history row gaining a session, direct deletes and a cascading session delete. After each step it checks that the state the feed replays to (key →StoredRow, column order included) equals the state the journal replays to. It also checks that each upsert's key equals the key rebuilt from its stored row. Removing one column from the feed's view makes it fail.tests/change_feed.rsexercises the public API on default features:history.jsonlprompt reaches the feed, keyed and with its stored row;tool_callsafter the feed was built is carried verbatim, in table order;timestamp_ms;source: None;a_store_fed_before_every_kind_gains_the_rest_above_its_headrewinds a store to the six-kind schema, triggers and all. After migration, an all-kinds cursor resumes into exactly the new kinds' rows, the retired trigger names stamp presences, and a presence write costs two revisions (presence plus session).Benchmark
node scripts/benchmark-sync.mjs --gateon the same machine (Apple M2 Max, debug profile):All phases are within about 2–6% of main and the gate passes.
Compatibility
Change::sourcechanges fromSourcetoOption<Source>. It's the one change that breaks source, and it's what lets an unknown source be carried.examples/rust-consumerbuilds and runs unchanged. A consumer that drains every kind now also receives the six new kinds.🤖 Generated with Claude Code
Note
Medium Risk
Public API and migration changes affect incremental consumers and cursors; behavior is heavily tested against the export journal but wrong consumer resume handling could miss or duplicate data.
Overview
Expands
SessionStore::changes_sinceso embedders can rebuild the ledger from the feed alone—covering all twelve evidence tables (history, presences, commit links, trajectories, connector observations, etc.), not just the original six session/evidence kinds.Each
Changenow carries the row as SQLite stores it viaStoredRow(columns), plus a journal-alignedkey(kind + uniqueness columns) on upserts and deletes. Typed upserts addEvidenceRow::History; kinds without a typed struct useEvidenceRow::Untyped.Change::sourcebecomesOption<Source>withsource_name, so unknown future sources still drain instead of erroring.Opening a writable store runs
change_feed_v2: new tables get revision stamps/triggers/tombstones, existing rows on newly fed tables are backfilled above the current head, and presence/session re-stamp triggers are rebuilt (includinghistory_auonly re-indexing FTS on indexed columns). Export capture payloads now use the samestored_columnslist as the feed so shapes cannot drift.Compatibility: a named consumer that previously committed with an explicit six-kind filter may hit
ConsumerKindsMismatchagainst today'sChangeKind::ALL—use a new consumer name or replay fromWatermark::START.Change::sourcegoing optional is the other source-breaking API tweak.Reviewed by Cursor Bugbot for commit b6e6a75. Bugbot is set up for automated code reviews on this repo. Configure here.