Skip to content

Page the admin inbox without skipping messages that share a millisecond (#138) - #116

Merged
theobong merged 1 commit into
mainfrom
fix/inbox-cursor-precision
Sep 24, 2026
Merged

theobong merged 1 commit into
mainfrom
fix/inbox-cursor-precision

Conversation

@theobong

Copy link
Copy Markdown
Member

Part of civfix/issue-tracker#138

What changed

Paging through the admin inbox with "Load more" no longer skips or repeats a message that arrived in the same millisecond as the last one on the page. Admin.

Before you start

  • Where: staging (admin.civfix.dev) after the main push
  • Sign in as: operator
  • Data: more than 25 messages under "Inbox" → "All" to reach a second page; with 25 or fewer, only step 1 applies

Verify

[Admin]

  1. Open "Mail", click "Inbox", then "All" — Expect: the number beside the list heading is 25 or less.
  2. Note the sender, subject and time of the last message in the list.
  3. Click "Load more" — Expect: "Loading…", then up to 25 more messages below; the first new one is older than the one you noted and is not a copy of it.
  4. Click "Load more" until it is gone — Expect: no message appears twice, and times only get older down the list.
  5. Repeat steps 2–4 on "Replies" and "Needs review" — Expect: the same.

Regression

Dashboard mail preview — [Admin]

  1. Open the Dashboard — Expect: the "Mail" tile shows its preview messages as before.

Paging other admin lists — [Admin]

  1. Open "Mail", stay on "Outreach" and click "Load more" — Expect: more threads load, none repeated.
  2. Open "Reports" from the Dashboard and click "Load more" — Expect: more reports load, none repeated.
  3. Open "Users" from the Dashboard and click "Load more" — Expect: more users load, none repeated.
  4. Open "Moderation" from the Dashboard and click "Load more" under "Queue" — Expect: more items load, none repeated.

Not covered

  • Two messages that arrive within the same millisecond right at a page boundary: a tester cannot time that. Checked by the automated suites, which page through rows a few microseconds apart, and exact ties, on both inbox lists.
  • A page position saved before this change still loading the next page: the admin keeps its place only in memory, so a reload starts over. Checked by the cursor tests.

@theobong
theobong merged commit f428b39 into main Sep 24, 2026
3 checks passed
@theobong
theobong deleted the fix/inbox-cursor-precision branch September 24, 2026 01:22
@greptile-apps

greptile-apps Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

Not safe to merge until existing millisecond-only inbox cursors continue without omitting same-millisecond messages.

Fix All in Claude CodeFindings

  1. P1 Preserve legacy cursor pages ▶
Fix with agent prompt
### Issue 1
services/api/src/db/cursor-helpers.ts:35-36
A composite cursor issued before this change contains only milliseconds. This code pads a legacy value such as `.123Z` to `.123000Z`, and the new descending keyset queries then use that reconstructed value as an exact microsecond anchor. Continuing from a cursor for a row at `.123900Z` omits an older row at `.123100Z`, so operators continuing an existing inbox page after deployment can permanently miss messages. Keep millisecond-compatible continuation behavior for legacy composite cursors while using precise instants for newly emitted cursors.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1) · Last reviewed commit: "Page the admin inbox by the row's micros..."

Comment on lines +35 to +36
const micros = (CURSOR_FRACTION_RE.exec(iso)?.[1] ?? "").padEnd(6, "0").slice(3)
return { at, instant: at.toISOString().replace(/Z$/, `${micros}Z`) }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Preserve legacy cursor pages

A composite cursor issued before this change contains only milliseconds. This code pads a legacy value such as .123Z to .123000Z, and the new descending keyset queries then use that reconstructed value as an exact microsecond anchor. Continuing from a cursor for a row at .123900Z omits an older row at .123100Z, so operators continuing an existing inbox page after deployment can permanently miss messages. Keep millisecond-compatible continuation behavior for legacy composite cursors while using precise instants for newly emitted cursors.

Artifacts

Legacy cursor integration test

  • The exact focused test source seeds microsecond-separated rows and compares exact versus legacy cursor continuation; it exercises both affected repositories.

Exact-microsecond cursor control

  • Executed PostgreSQL integration test output for cursor anchors that preserve `.123900Z`; both inbound and inbox-feed continuations return all expected rows.

Legacy millisecond cursor failure

  • Executed PostgreSQL integration test output for legacy `.123Z|UUID` cursors; both repository paths omit the row at `.123100Z`, confirming the defect.

Combined cursor integration run

  • Executed all four focused database cases together; exact-microsecond controls pass and legacy-millisecond cases fail, confirming the incompatible behavior.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: services/api/src/db/cursor-helpers.ts
Line: 35-36

Comment:
**Preserve legacy cursor pages**

A composite cursor issued before this change contains only milliseconds. This code pads a legacy value such as `.123Z` to `.123000Z`, and the new descending keyset queries then use that reconstructed value as an exact microsecond anchor. Continuing from a cursor for a row at `.123900Z` omits an older row at `.123100Z`, so operators continuing an existing inbox page after deployment can permanently miss messages. Keep millisecond-compatible continuation behavior for legacy composite cursors while using precise instants for newly emitted cursors.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

@greptile-apps

greptile-apps Bot commented Sep 24, 2026

Copy link
Copy Markdown

Comments Outside Diff

These findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.

  • P1 Preserve legacy millisecond cursor semantics when applying the microsecond keyset anchor ▶

    • Bug
      • A pre-deployment composite cursor such as 2025-01-01T00:00:00.123Z|<id of row at .123900Z> is parsed as exactly .123000Z. The new strict descending predicate therefore excludes another row at .123100Z, even though that row appeared before the cursor anchor in the old millisecond-based ordering. Real PostgreSQL reproduction fails for both InboundRepository.list and InboxFeedRepository.list: expected [.123100Z, .122900Z], received only [.122900Z].
    • Cause
      • parseCursorInstant pads absent sub-millisecond digits with zero (.123Z becomes .123000Z) at services/api/src/db/cursor-helpers.ts:35-36; the repositories then use that reconstructed microsecond timestamp in their (timestamp, id) < (anchor, id) predicates at inbound-repository.drizzle.ts:146 and inbox-feed-repository.drizzle.ts:114. Legacy cursors do not carry enough information to represent the original microsecond anchor.
    • Fix
      • Distinguish cursors that explicitly contain 4–6 fractional digits from legacy millisecond-only cursors. For the latter, retain the prior millisecond-compatible continuation behavior (for example, compare against the JS Date/millisecond anchor rather than the synthesized .000000 instant), while continuing to use exact microseconds for newly emitted cursors. Add the authored regression cases to the permanent integration suite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant