test(sources): prove excluded cursor revival - #3852
Conversation
Problem: parser-fingerprint revival needed an executable proof of indexed, unchanged-excluded, and typed-terminal outcomes without confusing fixture coverage for a live census. What changed: add a production-route candidate harness with a self-hashed receipt contract, red-mutation cursor assertions, and catch-up round-robin anti-starvation coverage. Compatibility/migration: no runtime behavior changes; the terminal candidate records its readiness-frontier injection as an explicit residual. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: the parent cursor implementation was closed without an immutable effect receipt, leaving the live-proof successor unbound. What changed: record the self-hashed candidate receipt, register its fixture and receipt in the incident coverage ledger, and bind the parent red mutation to the new fixture. Compatibility/migration: the receipt states that the historical live census was not run and names polylogue-excluded-cursor-live-proof as the residual successor. Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 25 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds a production live-proof harness for excluded cursors. It records revival, retry, terminal, indexing, fairness, and receipt evidence. Tests cover receipt validation and catch-up ordering. Incident coverage references the new proof artifacts. ChangesExcluded cursor proof
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ProofRunner
participant LiveWatcher
participant CursorStore
participant SQLite
participant Receipt
ProofRunner->>CursorStore: Seed excluded cursor state
ProofRunner->>LiveWatcher: Run parser-fingerprint cases
LiveWatcher->>CursorStore: Evaluate cursor work gate
LiveWatcher->>SQLite: Ingest candidates and record attempts
ProofRunner->>SQLite: Collect ingestion and indexing evidence
ProofRunner->>Receipt: Write and verify proof receipt
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f19ccf934
ℹ️ 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".
| "cases": cases, | ||
| "fairness": { | ||
| "planner": "_interleave_by_source", | ||
| "property": "one candidate from each present source family reaches the first round", |
There was a problem hiding this comment.
When browser-capture and another source are both present, this receipt records a false property: _interleave_by_source prepends the entire browser-capture bucket before constructing the round-robin iterators (polylogue/sources/live/watcher.py:1610-1611), so a large capture spool prevents every other family from reaching the first round. The added test covers only Codex and Hermes and therefore cannot detect this intentional exception; narrow the receipt claim to non-browser-capture families or encode and test the priority exception before treating it as proof.
AGENTS.md reference: AGENTS.md:L477-L479
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the pushed proof commits. The receipt now states browser-capture priority explicitly, and the fairness property is limited to non-browser-capture families. Focused coverage also asserts browser-capture intra-bucket path order. Final head: 987b800.
| patch( | ||
| "polylogue.sources.live.batch._jsonl_provider_and_session_artifact", | ||
| lambda _path, _fallback: (Provider.CODEX, True), | ||
| ) |
There was a problem hiding this comment.
For the truncated Codex input constructed here, the unpatched production detector sees only the valid session_meta line; is_supported_session_stream requires a message (polylogue/sources/parsers/codex.py:2328-2337), so _jsonl_provider_and_session_artifact returns parse_as_session=False and live ingest re-excludes the path before recording terminal raw evidence. This lambda forces admission and manufactures the terminal_corrupt_input outcome that the receipt labels as a production-route proof; use an input the real detector admits, or disclose and retain this admission gap as residual work instead.
AGENTS.md reference: AGENTS.md:L477-L479
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the pushed proof commits. The truncated terminal fixture now reaches detector admission without the lambda override. The remaining terminal frontier limitation is recorded as an explicit receipt residual because this candidate has no accepted byte head. Final receipt: 69e75004783fc3f0af38b85ef01136ac8ece609db850b730773b42b56589e350.
Problem: the cursor proof forced terminal admission for a truncated stream and stated a universal fairness property despite browser-capture priority.\n\nWhat changed: exercise detector admission with a valid Codex prefix plus terminal corruption, remove the detector override, and cover the browser-priority exception while narrowing the receipt property to non-browser families.\n\nCompatibility/migration: test and evidence-only change.\n\nRef #3852
Problem: the committed cursor receipt described the superseded detector override and universal fairness claim.\n\nWhat changed: record the detector-admitted valid-prefix terminal fixture and the browser-capture priority exception with a new immutable receipt hash.\n\nCompatibility/migration: evidence-only change.\n\nRef #3852
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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/excluded_cursor_live_proof.py`:
- Around line 27-29: Add a module-level NEW_PARSER_FINGERPRINT constant beside
OLD_PARSER_FINGERPRINT with value "live-batched-v2", then replace the repeated
"live-batched-v2" literals at all four call sites with NEW_PARSER_FINGERPRINT.
- Around line 63-70: Update _attempts_for_path to explicitly close each sqlite3
connection after use, including the queries at the referenced locations.
Preserve the existing transaction context behavior while wrapping or otherwise
guaranteeing connection cleanup, following the established CursorStore._connect
pattern.
- Around line 155-158: Update _attempts_for_path to order attempts by
started_at_ms descending with raw_id descending as the deterministic secondary
key. Preserve the existing ordering direction and ensure proof_attempts[0]
consistently selects the newest proof attempt when timestamps are identical.
- Around line 294-307: Update the outcome expressions in the outcomes map to
validate indexed["attempt"] and typed_terminal["terminal_evidence"] are present
before subscripting them. Place each presence check first in its corresponding
and chain so missing data evaluates to False and reaches the existing
AssertionError with the outcomes map instead of raising TypeError.
In `@tests/unit/sources/test_excluded_cursor_live_proof.py`:
- Around line 126-127: Update test_committed_candidate_receipt_is_self_hashed to
derive the committed receipt path from __file__, resolving through the
repository root (confirm the appropriate parents[3] depth or reuse an existing
repository-root fixture), instead of relying on the process working directory.
- Around line 118-123: Rename
test_receipt_round_trip_preserves_machine_readable_fields to reflect that it
verifies rejection of a receipt with a mismatched hash, keeping its existing
verify_receipt assertion unchanged.
- Around line 90-104: Isolate the second CursorStore instance by constructing it
under a separate temporary directory, ensuring both its _db_path and
_ops_db_path differ from the first store while preserving the existing cursor
setup and assertions.
In `@tests/unit/sources/test_live_watcher_catchup_order.py`:
- Around line 53-54: Replace the vacuous second assertion in the
_interleave_by_source test with a direct assertion of the deterministic
first-two source order: codex followed by hermes. Preserve the existing
candidate set assertion and remove the retry.jsonl path-name condition.
- Around line 61-67: Extend the assertions in the test around
live_watcher._interleave_by_source to verify the first two ordered candidates
have paths capture-a.json followed by capture-b.json. Keep the reversed input
order so the test explicitly covers the promised per-bucket path sorting, while
retaining the existing source-name assertion.
🪄 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: 8c6c93a9-64cb-4715-9d8c-79fb9d2ae75b
📒 Files selected for processing (5)
docs/evidence/polylogue-excluded-cursor-live-proof-2026-08-06.jsondocs/plans/reindex-incident-coverage.jsontests/infra/excluded_cursor_live_proof.pytests/unit/sources/test_excluded_cursor_live_proof.pytests/unit/sources/test_live_watcher_catchup_order.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e26e748e1
ℹ️ 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".
| cursor=cursor, | ||
| parser_fingerprint="live-batched-v2", | ||
| ) | ||
| baseline = asyncio.run(processor.ingest_files([path])) |
There was a problem hiding this comment.
The indexed case fully ingests this exact file here before _seed_excluded runs, so _indexed_counts already returns one session before the fingerprint-triggered reattempt. The later outcome and anti_vacuity.indexed_session_count therefore remain true even if the revived ingest only records a successful content-hash no-op or stops indexing altogether; seed readiness with unrelated material or assert the source has zero indexed sessions before the reattempt and gains one afterward.
AGENTS.md reference: AGENTS.md:L477-L482
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the pushed proof commits. The indexed case seeds only byte-proven source evidence plus a matching revision head, asserts zero indexed sessions before automatic catch-up, and asserts one after revival. Final receipt: 69e75004783fc3f0af38b85ef01136ac8ece609db850b730773b42b56589e350.
| else nullcontext() | ||
| ) | ||
| with frontier_patch: | ||
| metrics = asyncio.run(watcher._ingest_files([path])) if ingest and needs_work else None |
There was a problem hiding this comment.
When an unchanged excluded file is meant to revive after a daemon restart or periodic sweep, the production path is _scan_catch_up_candidates → _plan_catch_up → the chunk/coordinator loop, but this harness manually joins _needs_work directly to _ingest_files. It therefore still reports a successful production-route reattempt if candidate discovery omits excluded files, planning drops them, or the real authority/coordinator ordering prevents ingestion; the separate fairness test only invokes _interleave_by_source. Drive the case through _catch_up or _catch_up_candidates before recording automatic reattempt evidence.
AGENTS.md reference: AGENTS.md:L480-L482
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the pushed proof commits. The proof now invokes LiveWatcher._catch_up, which drives candidate scanning, planning, and coordinated chunk ingestion. The receipt records that route explicitly.
Problem: the indexed proof pre-seeded an indexed session and the direct ingest path bypassed the watcher catch-up planner. After switching to the real route, the fixture lacked a comparable cursor/head authority row.\n\nWhat changed: seed only byte-proven source evidence and a matching revision head, derive its session identity and content hash from the parser, then assert zero indexed sessions before automatic catch-up and one after revival. The harness now records the full catch-up route, uses the parser fingerprint constant, closes attempt reads, and tightens ordering and null assertions.\n\nCompatibility/migration: test harness and proof fixtures only; production code is unchanged.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
Apply the repository formatter's required wrapping to the cursor proof predicate.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
Problem: the committed candidate receipt described the pre-repair indexed fixture and direct-ingest path.\n\nWhat changed: record the automatic catch-up route, the zero-to-one indexed transition, and the byte-proven source/head authority setup from the repaired proof harness.\n\nCompatibility/migration: this is an immutable candidate-fixture receipt; live census remains explicitly not run.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
Problem: mypy inferred the persisted integer excluded flag as a non-boolean union in the proof summary.\n\nWhat changed: coerce the cursor flag explicitly before recording the fingerprint transition.\n\nCompatibility/migration: proof harness only; production code is unchanged.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/infra/excluded_cursor_live_proof.py (1)
366-370: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRequire parse-error evidence for the terminal outcome.
The
typed_terminaloutcome accepts the artifact and support-status labels without requiringparse_error_present. Addand terminal_evidence["parse_error_present"] is Trueso the proof verifies terminal corruption, not only its classification labels.🤖 Prompt for 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. In `@tests/infra/excluded_cursor_live_proof.py` around lines 366 - 370, Require parse-error evidence in the "typed_terminal" assertion by adding a check that terminal_evidence["parse_error_present"] is True alongside the existing artifact_kind and support_status checks. Preserve the current retry-state validations.
🤖 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/excluded_cursor_live_proof.py`:
- Around line 96-115: Update the database setup around the sqlite3 connection
used by the raw_revision_heads INSERT to explicitly close conn in a finally
block. Preserve the existing commit behavior while ensuring the connection is
closed on both successful execution and exceptions; replace the current with
sqlite3.connect context usage as needed.
---
Duplicate comments:
In `@tests/infra/excluded_cursor_live_proof.py`:
- Around line 366-370: Require parse-error evidence in the "typed_terminal"
assertion by adding a check that terminal_evidence["parse_error_present"] is
True alongside the existing artifact_kind and support_status checks. Preserve
the current retry-state validations.
🪄 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: 59c46f62-596f-49dd-a906-474cbcae0f7a
📒 Files selected for processing (4)
docs/evidence/polylogue-excluded-cursor-live-proof-2026-08-06.jsontests/infra/excluded_cursor_live_proof.pytests/unit/sources/test_excluded_cursor_live_proof.pytests/unit/sources/test_live_watcher_catchup_order.py
Problem The excluded-cursor proof left its index connection for garbage collection after seeding the byte head. What changed Close the fixture connection in a finally block while preserving the explicit commit. Verification direnv exec . devtools test tests/unit/sources/test_excluded_cursor_live_proof.py tests/unit/sources/test_live_watcher_catchup_order.py Co-Authored-By: Codex <noreply@openai.com>
Problem\nThe typed-terminal proof accepted terminal classification labels without proving that parsing actually failed.\n\nWhat changed\nRequire parse_error_present in the terminal outcome while preserving the existing retry-state assertions.\n\nVerification\ndirenv exec . devtools test tests/unit/sources/test_excluded_cursor_live_proof.py tests/unit/sources/test_live_watcher_catchup_order.py\n\nRef polylogue-excluded-cursor-live-proof\n\nCo-Authored-By: Codex <noreply@openai.com>
Problem\nThe proof's terminal parse-error requirement had no direct receipt assertion, so a malformed terminal fixture could regress without the unit test noticing.\n\nWhat changed\nAssert the typed-terminal receipt records parse_error_present as true.\n\nVerification\ndirenv exec . devtools test tests/unit/sources/test_excluded_cursor_live_proof.py tests/unit/sources/test_live_watcher_catchup_order.py\n\nRef polylogue-excluded-cursor-live-proof\n\nCo-Authored-By: Codex <noreply@openai.com>
1 participant
Summary
Add an excluded-cursor proof for parser-fingerprint revival, detector-admitted terminal corruption, and source-aware catch-up fairness. The proof emits an immutable candidate-fixture receipt and records that the historical live census remains open.
Problem
The indexed proof had been vacuous because it pre-ingested the same file, and the first real catch-up attempt was refused because the fixture had no comparable cursor/head authority. The terminal proof previously forced detector admission for a truncated stream, and its fairness wording ignored the production scheduler’s intentional browser-capture priority.
Solution
Exercise the production cursor reattempt and classification seams with fingerprint changes, unchanged exclusions, and a valid Codex session prefix followed by a malformed terminal record. The indexed fixture seeds byte-proven source evidence plus a matching revision head without an index session, then asserts zero indexed sessions before the automatic
_catch_uproute and one after revival. Remove the detector override, add browser-capture priority coverage, and qualify the receipt’s round-robin property to non-browser-capture families. The readiness-gate exception remains explicitly recorded only for the typed-terminal candidate because it has no accepted byte head.Verification
direnv exec . devtools test tests/unit/sources/test_excluded_cursor_live_proof.py tests/unit/sources/test_live_watcher_catchup_order.pypassed: 14 tests in focused run20260806T113721Z-focused-test-343435-4ce30777.direnv exec . devtools verify --quickpassed: 24 steps, exit 0 in run20260806T110142Z-quick-4172753-a33c0e75. Receipt self-hash verification passed with SHA69e75004783fc3f0af38b85ef01136ac8ece609db850b730773b42b56589e350. The terminal outcome also requiresparse_error_present: true. Logical commits:e2c40029c,af3b2a690,92af9b809,987b80084,76c62b3ea,1363639120b79c7b5edaed484318120c8bbad870,ae10af89d10215935c1fd84ed0780e8d74861b4e.Follow-ups
The historical live cursor census was not run. This PR therefore does not claim that the live excluded population has been reattempted or drained. Retain the live operation receipt under
polylogue-live-operation-receiptsbefore treating the acceptance bead as complete.Ref polylogue-excluded-cursor-live-proof