fix(db): add id tiebreak to product-usage rollup and review-suppression queries - #4560
Conversation
…on queries (#4501) Two queries in repositories.ts ordered by a timestamp column alone with no secondary tiebreak, the same bug class already fixed in listPullRequestFiles (#4481): same-millisecond ties under concurrent writes make row order (and therefore which rows survive a cap) query-plan-dependent instead of deterministic. Adds an id tiebreak to the product-usage daily rollup's main and retention scans, and to review-suppression's eviction and read queries, matching the established desc(id) convention elsewhere in this file.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4560 +/- ##
=======================================
Coverage 94.08% 94.08%
=======================================
Files 427 427
Lines 37961 37961
Branches 13864 13864
=======================================
Hits 35715 35715
Misses 1586 1586
Partials 660 660
🚀 New features to boost your workflow:
|
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Gittensory review result - approve/merge recommendedReview updated: 2026-07-10 02:38:15 UTC
✅ Suggested Action - Approve/Merge
Review summary Nits — 5 non-blocking
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Summary
src/db/repositories.tsordered by a timestamp column alone with no secondary tiebreak — the same bug class already fixed inlistPullRequestFiles(fix(review): make listPullRequestFiles order deterministic to stabilize content fingerprints #4481): same-millisecond ties under concurrent writes make row order (and therefore which rows survive a cap) query-plan-dependent instead of deterministic.upsertProductUsageDailyRollup's main scan and retention scan now order by(occurredAt, id)/(occurredAt desc, id desc)respectively.pruneReviewSuppressionsOverCap's eviction query andlistReviewSuppressions's read query now order by(createdAt desc, id desc), matching the established tiebreak convention already used atrepositories.ts:3046/5315/5372(auditEvents,gateOutcomes,agentPendingActions).enforces the per-repo bound...) with fake timers: under fast in-memory D1 execution, its 501 sequential inserts could tie oncreatedAt, making its "hash-0 is oldest" assumption depend on the (now-added) id tiebreak rather than insertion order — confirmed empirically by reproducing the flake against the fix and verifying 8/8 clean runs afterward.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlint(vianpm run test:ci)npm run typechecknpm run test:coveragelocally (unsharded) — new invariant/regression tests confirmed to FAIL without the fix and pass with it, verified by temporarily reverting the fix and re-runningnpm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateSafety
UI Evidence— N/A, no visible/UI change.