feat(sdk): session-filtered change feed and a paged read of every session identity - #261
Conversation
…sion identity ChangeQuery::session(source, session_id) restricts a drain to one session: the same Change values the unfiltered drain reports for it, tombstones included, read through each table's (source, session_id) index. It is a one-shot backfill read, so naming a consumer with it is InvalidArgument. SessionStore::session_identities(IdentityQuery) pages every (source_name, session_id) the store holds evidence under, catalogued or not, as a merge of covering index seeks across every table that stores a session. storage::session_identities_after is the same read. sessions gains a (source, session_id) index for it. 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 31 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 (7)
📝 WalkthroughWalkthroughThe Rust API adds paged enumeration of session identities across evidence tables and adds session filters to change-feed queries. The implementation adds session-indexed reads, pagination, validation, public types, and tests for identity results and filtered changes. ChangesSession-scoped reads
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Client
participant ChangeQuery
participant Changes
participant SQLite
Client->>ChangeQuery: Set source and session filter
Client->>Changes: Start one-shot drain
Changes->>SQLite: Select filtered page keys
Changes->>SQLite: Read matching upserts and tombstones
SQLite-->>Changes: Return matching changes
Changes-->>Client: Return page
Suggested reviewers: Merge Risk: 🟡 Moderate · up to Identity enumeration can miss a concurrently added session, and paging on existing databases may become slow. Fix the snapshot and upgrade-index paths before merging. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new reads are limited to the configured store, and the session-filtered feed prevents a partial drain from advancing a shared consumer cursor. No introduced security failure was established. Callers must still control who can access a store-wide identity list. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 69.39% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 49 functions across 7 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 session rows, Comment |
…ge is one snapshot idx_sessions_identity is a required index, so a writable open of an existing store creates it; a read-only store without it answers session_identities with StaleSchema and keeps every other read. Every identity arm skips an empty session id as well as NULL, so every identity listed is one ChangeQuery::session accepts. identities_after reads a page on the caller's transaction, or its own deferred one on an autocommit connection, so an identity that moves between tables mid-page is still listed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
What this provides
Two reads for an embedder that follows sessions selectively, on the default public API. The desktop probe needs both to move off the
exportjournal andunstable-internal.ChangeQuery::session(source, session_id)Restricts a
changes_sincedrain to one session. It reports exactly the changes whosesource_nameandsession_idmatch, with the samecolumns,key,revisionand tombstones the unfiltered drain reports for that session, bounded to the head at open.trajectorysource.sourceis the stored name, so a source this build doesn't know works.Error::InvalidArgument.Every page read seeks the session's own index; the range is written
+revisionso the revision index can't be chosen, and only that session's rows are sorted. Default features:change_feed::tests::a_session_page_reads_the_session_indexasserts this for every kind and for tombstones: aSEARCHthat binds source and session, no table scan, and no revision index.SessionStore::session_identities(IdentityQuery { after, limit })Pages every
(source_name, session_id)the store holds evidence under, catalogued or not. That's the union ofsessions,history,session_events,tool_calls,file_edits,session_markers,session_relationships(by parent),session_presences,session_commit_links,session_observations,observation_evidenceandtrajectories, returned distinct and in byte order.SessionIdentity { source_name, session_id }hassource()for the parsedSource. It's also the typeChangeQuery::sessionholds.storage::session_identities_after, which the probe uses on its snapshot connection, is now this same read. It covers all those tables instead of three, so a sidechain's tool calls or a connector's observations can't escape a consent baseline.The read is a merge: each table offers its next identity after the cursor through a covering index that leads with
(source, session). That's one seek per identity per table that holds it, and no payload is read.sessionsgainsidx_sessions_identity (source, session_id)for this, because its primary key leads withsession_id.session_identities::tests::every_seek_is_an_index_search_on_the_sessionasserts a covering index and no sort for every table.Timing
Release build on an M2 Max. The store has 2,000 sessions × 52 rows plus one session of 50,000 events (154,001 rows), measured by
a_session_drain_reads_one_session_of_many(ignored; run it with--ignored --nocapture).A session drain re-seeks its session on each page, so a long session is fastest with a large
batch; the docs say so.The throughput gate passes with origin/main on the same machine: cold sync 1,449 vs 1,350 records/s, incremental 226 vs 213 ms, unchanged 48 vs 45 ms, hydrate cold 916 vs 948 records/s, hydrate unchanged 27.0 vs 26.2 ms. The machine was under load (calibration 1.26x on main, 1.34x on the branch), and the new index only affects
sessionswrites.Tests (outside the crate, default features)
a_session_drain_is_the_feed_restricted_to_that_sessionsyncs Claude and Codex fixtures, writes an update, deletes a row and cascade-deletes a whole session. For every session in the feed, from START and from a watermark, the session drain equals the unfiltered drain restricted to that session. The deleted session's own tombstones reach its drain, and a prompt with no session reaches none.a_session_drain_takes_any_stored_sourcecovers an unknown source and a trajectory.a_session_drain_refuses_a_consumercovers the misuse errors andcommit.tests/session_identities.rschecks that an events-only, history-only, tool-call-only, edit-only, marker-only, parent-only, presence-only, commit-link-only, observation-only (unknown source) and trajectory session each appear exactly once and in order. A prompt with no session and a child named only by an edge don't appear. Paging at 1, 2, 3, 7, 64 and the default size returns the same complete list with no repeats. The identities equal the sessions the feed names, and each identity's session drain is non-empty.plugins/relayhistory/rust), which usesession_identities_after, pass.🤖 Generated with Claude Code
Note
Medium Risk
Touches change-feed SQL paths and adds a migration index on
sessionswrites; behavior is heavily tested but incorrect session filtering or identity merge logic could miss or duplicate data for consumers.Overview
Adds two default-feature
ai_histAPIs for embedders that follow sessions selectively (e.g. moving off export/unstable-internal).ChangeQuery::session(source, session_id)narrowschanges_sinceto one session: sameChangerows and tombstones as the global drain, bounded to the head at open. Pagination uses each table’s(source, session_id)indexes (with+revisionordering) instead of walking the full revision range. Session drains are one-shot backfills—pairing with a consumer orcommitisInvalidArgument, as are empty source or session ids. Prompts with no session (and their deletes) stay on the unfiltered feed only.SessionStore::session_identities(IdentityQuery)returns keyset-paged distinctSessionIdentitypairs across catalog and evidence tables (plus trajectories undertrajectory), via a merge of covering index seeks with no payload reads. Each page uses one snapshot (including on autocommit).storage::session_identities_afternow shares that implementation and covers all those tables, not just catalog/prompts/events.Schema:
sessionsgainsidx_sessions_identityon first writable open; read-only stores without it getStaleSchemaforsession_identities. Docs andpublic-api.txtare updated; integration tests assert parity between session drains and filtered full feeds and complete identity paging.Reviewed by Cursor Bugbot for commit 529b558. Bugbot is set up for automated code reviews on this repo. Configure here.