test(retention): cover pruneExpiredRecords's two defensive ?? 0 fallback arms#8504
Conversation
…ack arms pruneExpiredRecords has two defensive `?? 0` arms -- the dry-run count path (`Number(row?.n ?? 0)`) and the delete-loop path (`Number(result.meta?.changes ?? 0)`) -- with no direct test coverage, unlike the identical pattern on its sibling dedupeSignalSnapshots (which has dedicated tests). Add two tests mirroring that sibling's approach: mock env.DB so the count query returns no row / the delete run() result lacks meta, and assert the function falls back to 0 rather than producing NaN. No source change -- coverage-only for existing, correct code.
|
🚨 Contributor flagged. Click here for more info: Superagent Dashboard |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-24 16:48:02 UTC
Review summary Nits — 3 non-blocking
CI checks failing
Flagged checks (non-blocking)
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-tests-merge)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
What & why
Closes #8370.
pruneExpiredRecords(src/db/retention.ts) has two defensive?? 0fallback arms guarding against D1 driver anomalies — the dry-run count path (Number(row?.n ?? 0), line 75) and the delete-loop path (Number(result.meta?.changes ?? 0), line 85) — but neither had direct test coverage. The identical pattern on this file's siblingdedupeSignalSnapshotsdoes have dedicated tests, and the repo's Codecov patch gate counts branches, so these two untested nullish-coalescing arms are exactly the gap the gate is meant to catch.What's added (test-only — no source change)
Two tests in
test/unit/retention.test.ts, mirroringdedupeSignalSnapshots's existing equivalents:env.DBso the count query'sfirst()returnsundefined→ assertrow?.n ?? 0falls back todeleted: 0(notNaN).run()to return an object with nometa→ assertresult.meta?.changes ?? 0falls back to0(which is< batchSize, so the loop terminates immediately withdeleted: 0).Per the issue,
pruneExpiredRecords's actual logic is untouched — this is coverage for existing, already-correct defensive code.Validation
test/unit/retention.test.ts: 22 tests pass (20 existing + 2 new); typecheck clean;git diff --checkclean.?? 0to a non-zero fallback makes the corresponding new test fail, proving the tests genuinely exercise the fallback arm.main, mergeable-clean.