ft. ENG-40

[Connorbelez]

No description provided.

[linear]

ENG-40 Implement consumer cursor infrastructure — register, poll, advance

[sourcery-ai]

Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9f19ed88-615d-4ed5-9689-2c879016b9b7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/eng40

📝 Coding Plan

• Generate coding plan for human review comments

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Connorbelez]

• ft. ENG-40 #97 👈 (View in Graphite)
• ENG-27 #90 : 5 other dependent PRs (#91 , #92 , #93 and 2 others)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[Connorbelez]

@copilot please review

[Copilot]

@Connorbelez I've opened a new pull request, #98, to work on those changes. Once the pull request is ready, I'll request review from you.

[greptile-apps]

Greptile Summary

This PR implements ENG-40: the Consumer Cursor Infrastructure for the mortgage ownership ledger. It refactors convex/ledger/cursors.ts from adminQuery/adminMutation to the ledger-domain-standard ledgerQuery/ledgerMutation, adds registerCursor (idempotent, initialises at genesis) and getNewEntries (paginated polling over ledger_journal_entries.by_sequence), tightens advanceCursor to require an existing cursor and validate target sequence numbers, and migrates cursor tests into a dedicated cursors.test.ts file covering the full SPEC §6.7 polling contract.

Key changes:

• registerCursor — idempotent upsert returning the cursor _id, initialised at lastProcessedSequence: 0n
• getNewEntries — queries ledger_journal_entries via by_sequence index, respects batchSize, returns entries, cursorPosition, and hasMore
• advanceCursor — now requires the cursor to exist first (no more upsert) and validates that the target sequence is present in the journal
• resetCursor — now shares the getCursorByConsumerId helper and adds sequence validation for non-zero targets
• Cursor tests moved to convex/ledger/__tests__/cursors.test.ts; the stale T-072 lifecycle test removed from ledger.test.ts

Issues found:

• hasMore false positive (cursors.ts:101): entries.length === batchSize yields true when remaining entries exactly equal batchSize, causing a spurious empty poll round-trip. Fetch batchSize + 1 to detect this reliably.
• advanceCursor allows backward movement (cursors.ts:106–127): No guard prevents lastProcessedSequence from being set to a value less than the current cursor position. In a financial ledger this could cause downstream consumers (accrual engine, dispersal engine) to reprocess already-handled entries.
• Manifest status stale (specs/ENG-40/chunks/manifest.md): Both chunks still show planned while tasks.md records the majority of work as complete.

Confidence Score: 2/5

• Not safe to merge as-is — two logic issues in the core cursor API could cause downstream financial consumers to receive incorrect signals or reprocess ledger entries.
• The hasMore false positive and the missing backward-movement guard in advanceCursor are both correctness issues in a financial ledger context. Reprocessing journal entries due to an accidental cursor rewind could produce duplicate accrual or dispersal calculations. The hasMore issue adds noise to polling contracts that consumers may rely on to determine when to stop. Both issues require changes to cursors.ts before merging.
• convex/ledger/cursors.ts requires fixes at lines 101 (hasMore predicate) and 111–126 (backward-movement guard in advanceCursor).

Important Files Changed

Filename	Overview
convex/ledger/cursors.ts	Core cursor API refactored from adminQuery/adminMutation to ledgerQuery/ledgerMutation; adds registerCursor, getNewEntries, and tightens advanceCursor — but hasMore uses a false-positive predicate and advanceCursor lacks a backward-movement guard, both of which are correctness issues in a financial ledger context.
convex/ledger/tests/cursors.test.ts	New dedicated cursor test file; covers idempotent registration, SPEC §6.7 poll/advance/replay flow, batch limiting, and structured error codes — test suite is solid but the batch-size test avoids the exact-boundary case that would expose the hasMore false positive.
convex/ledger/tests/ledger.test.ts	Removes the now-stale T-072 cursor lifecycle test (which assumed upsert-on-advance behavior); the rest of the ledger test suite is unaffected.
specs/ENG-40/chunks/manifest.md	Chunk manifest added but both entries still show status "planned" while tasks.md marks most work as done; minor doc inconsistency.
specs/ENG-40/tasks.md	Master task list correctly reflects the implementation state; T-006 and T-011 (codegen/test gate) are still open, consistent with the status notes.

Sequence Diagram

sequenceDiagram
    participant C as Consumer (e.g. accrual_engine)
    participant R as registerCursor
    participant G as getNewEntries
    participant A as advanceCursor
    participant DB as ledger_journal_entries

    C->>R: registerCursor(consumerId)
    R-->>C: cursorId (idempotent, lastProcessedSequence=0n)

    loop Poll cycle
        C->>G: getNewEntries(consumerId, batchSize?)
        G->>DB: query by_sequence > lastProcessedSequence, take(batchSize)
        DB-->>G: entries[]
        G-->>C: { entries, cursorPosition, hasMore }

        alt entries.length > 0
            C->>C: process entries
            C->>A: advanceCursor(consumerId, lastProcessedSequence)
            A->>DB: validate sequence exists
            A-->>C: cursor patched
        else no new entries
            C->>C: wait / backoff
        end
    end

Loading

_{Last reviewed commit: 226b476}

[Copilot]

Pull request overview

Adds the ENG-40 “consumer cursor” infrastructure to the ledger domain so downstream consumers can register a cursor, poll new journal entries in sequence order, and advance their cursor position, with dedicated test coverage and accompanying spec/task docs.

Changes:

• Refactors convex/ledger/cursors.ts to use shared helpers, adds registerCursor + getNewEntries, and tightens cursor mutation validation.
• Moves cursor contract tests into a new focused cursors.test.ts and removes the legacy cursor lifecycle assertions from ledger.test.ts.
• Adds ENG-40 planning/status/spec documentation (task lists, chunk contexts/statuses, manifest).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
specs/ENG-40/tasks.md	Master checklist for ENG-40 work across chunks.
specs/ENG-40/chunks/manifest.md	Chunk manifest table and status summary.
specs/ENG-40/chunks/chunk-01-cursor-api/tasks.md	Chunk 01 task breakdown (cursor API refactor).
specs/ENG-40/chunks/chunk-01-cursor-api/status.md	Chunk 01 implementation + quality gate notes.
specs/ENG-40/chunks/chunk-01-cursor-api/context.md	Chunk 01 scope/acceptance criteria/context.
specs/ENG-40/chunks/chunk-02-cursor-tests/tasks.md	Chunk 02 task breakdown (cursor tests).
specs/ENG-40/chunks/chunk-02-cursor-tests/status.md	Chunk 02 implementation + quality gate notes.
specs/ENG-40/chunks/chunk-02-cursor-tests/context.md	Chunk 02 test plan/context.
convex/ledger/cursors.ts	Implements consumer cursor API (register, poll, advance/reset) with validation + structured errors.
convex/ledger/tests/ledger.test.ts	Removes old cursor lifecycle test block (cursor contract moved to dedicated file).
convex/ledger/tests/cursors.test.ts	New dedicated cursor contract tests (SPEC 6.7 flow + edge cases).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.