test(infra): cover canonical archive snapshot residuals - #3882
Conversation
Capture path-independent raw referents, durable user state, web constructs, excision tombstones, and index revision heads in the canonical comparator. Derive default FTS probes from real block text and add red mutations for each newly covered semantic surface.
📝 WalkthroughWalkthroughCanonical archive snapshots now cover user state, web constructs, excision content, raw revision heads, and normalized raw identities. Default search probes support public projection capture. Tests cover relocation equivalence and mutations across these relations. ChangesCanonical snapshot comparison
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/infra/archive_canonical_snapshot.py`:
- Around line 411-420: Update _default_search_queries to derive candidates using
the archive’s tokenizer-compatible rules, including short and
punctuation-separated searchable terms, and retain only stable candidates that
match the public-search FTS relation. In
tests/unit/infra/test_archive_canonical_snapshot.py lines 155-174, add a
regression fixture containing only those term shapes and verify that deleting an
FTS posting is reported as red.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cf649f57-fafa-4ebf-973a-24b84b8ef2ad
📒 Files selected for processing (2)
tests/infra/archive_canonical_snapshot.pytests/unit/infra/test_archive_canonical_snapshot.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27533e4a78
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for row in connection.execute("SELECT raw_id, origin, source_index, blob_hash, native_id FROM raw_sessions"): | ||
| raw_id, origin, source_index, blob_hash, native_id = row | ||
| blob_hash_hex = blob_hash.hex() if isinstance(blob_hash, bytes) else str(blob_hash) | ||
| stable_identity = f"raw[{origin}|{source_index}|{blob_hash_hex}|{native_id!r}]" |
There was a problem hiding this comment.
Preserve source paths in normalized raw identities
When two acquisitions have the same origin, source index, bytes, and native ID but come from different source paths, this expression maps their distinct raw IDs to the same comparator identity, even though deterministic_raw_session_id() deliberately hashes source_path. A route can then associate a session, membership, or revision head with the wrong acquisition and still compare equal because every raw-reference column collapses to the same value; include an archive-relative normalized source path so this mapping remains one-to-one.
Useful? React with 👍 / 👎.
| normalized = " ".join(str(search_text).split()) | ||
| tokens.update(token for token in normalized.split() if len(token) >= 4 and token.isalnum()) |
There was a problem hiding this comment.
Derive FTS probes with the configured tokenizer
For archives whose searchable words are attached to punctuation, such as hello. or alpha,beta, whitespace splitting leaves tokens that fail isalnum(), even though the configured unicode61 tokenizer indexes those words. If all block text has that shape, this returns no default queries and deleting or corrupting every FTS posting remains invisible to the canonical comparison; extract tokens with semantics compatible with unicode61 instead.
AGENTS.md reference: AGENTS.md:L76-L78
Useful? React with 👍 / 👎.
| if isinstance(value, str) and _is_raw_id_column(column): | ||
| value = raw_identity_map.get(value, value) |
There was a problem hiding this comment.
Normalize raw identities embedded in authority plans
When compared archives contain raw-authority plans, raw IDs are embedded in fields such as raw_authority_plans.input_raw_ids_json and also determine input_digest, plan_id, and the foreign keys referencing that plan, but this code only rewrites a value when the entire scalar column is named like a raw ID. Since production raw IDs change with the source root, semantically identical authority plans created under two roots retain different JSON, digests, and plan keys and therefore compare unequal; normalize the structured plan identity graph as well as scalar raw-reference columns.
Useful? React with 👍 / 👎.
| } | ||
| ids = tuple(session_ids) if session_ids is not None else _session_ids(connections["index"]) | ||
| public = _capture_public_projections(root, ids, tuple(search_queries)) | ||
| effective_search_queries = tuple(search_queries) or _default_search_queries(connections["index"]) |
There was a problem hiding this comment.
Canonicalize tied FTS search results
When two blocks match a selected probe with equal BM25 rank, search_archive_blocks() orders only by rank, so SQLite returns tied rows in rowid/insertion order. The order-invariance properties compare archives ingested in different orders; enabling default probes here now places those unstable lists in every snapshot and makes semantically identical archives compare unequal whenever a selected token has tied matches. Sort the returned block IDs for the snapshot or add a stable block-ID tie-breaker to the search.
Useful? React with 👍 / 👎.
Problem: default canonical snapshot probes were derived by splitting block text and excluding short terms, so punctuation-separated tokenizer terms could be absent from the public search projection. What changed: derive bounded probes from the SQLite FTS5 vocabulary, validate each candidate against the public FTS relation, and add a short punctuation-term regression fixture. Compatibility/migration: test infrastructure only; archive schemas and production behavior are unchanged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b132c7d3a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ("source", "raw_hook_events"), | ||
| ), | ||
| "authority": ( | ||
| ("index", "raw_revision_heads"), |
There was a problem hiding this comment.
Exclude the revision-head wall-clock timestamp
When two equivalent routes materialize the same accepted revision head at different times, capturing this entire relation also compares decided_at_ms, which the production writer sets from datetime.now(UTC). Both tests/infra/reindex_differential.py and tests/unit/storage/test_incremental_rebuild_equivalence.py already classify that column as volatile, so canonical route comparisons involving real revision heads will report false differences; add index.raw_revision_heads.decided_at_ms to the run-local normalization allowlist.
Useful? React with 👍 / 👎.
Summary
Expand the canonical archive snapshot comparator and its mutation suite to cover the six residual proof surfaces identified for Ref polylogue-canonical-snapshot, including tokenizer-compatible default FTS probes.
Problem
The comparator omitted path-derived raw identities, default FTS postings, durable user state, web content constructs, excision tombstones, and the index-owned raw revision heads. The initial default probe derivation also missed short, punctuation-separated, and reserved operator terms accepted by the archive tokenizer.
Solution
The comparator now opens user.db, derives stable raw identities from source evidence, selects bounded probes from the SQLite FTS5 vocabulary and validates them against the public FTS relation, excludes reserved operator words, handles an absent public FTS relation, includes the missing relations, and reports the new user_state section in snapshot diffs. The focused tests add relocation equivalence, semantic red mutations, and tokenizer regressions. No production code, schema, or live archive state changed.
Verification
direnv exec . devtools test tests/unit/infra/test_archive_canonical_snapshot.py: 17 passed.direnv exec . devtools verify --quick: all 24 steps passed at 4f72518; the final test-only commit passed its focused suite.Bead disposition matrix
polylogue-canonical-snapshotcommit:27533e4a7,commit:2b132c7d3,commit:4f72518c0,commit:9108e75cb,test:17 canonical snapshot tests passed,command:devtools verify --quick: all 24 steps passed