test(retention): cover pruneExpiredRecords's two defensive ?? 0 arms

[kai392]

Fixes #8370

Root cause

pruneExpiredRecords in src/db/retention.ts has two defensive ?? 0 guards against a D1 driver returning an unexpected shape:

• the dry-run count path — deleted: Number(row?.n ?? 0)
• the delete-loop path — const changes = Number(result.meta?.changes ?? 0)

Neither had direct coverage. The identical pattern on this file's sibling dedupeSignalSnapshots is already tested, so this was a gap in one of two twins rather than an untested idea.

Fix approach

Test-only — pruneExpiredRecords's logic is untouched, per the issue's explicit scope.

Four tests mirroring dedupeSignalSnapshots's existing approach: a hand-built env.DB whose prepare().bind() returns the degenerate shape, and a single-rule policy so exactly one table is walked. Each arm gets both of its failure shapes, because they reach the fallback differently:

Arm	Absent shape	Present-but-null shape
dry-run row?.n ?? 0	first() → undefined (no row)	first() → { n: null }
delete meta?.changes ?? 0	run() → {} (no meta)	run() → { meta: { changes: null } }

The split matters: the optional-chain and the ?? are separate branches, and only the absent shape exercises the optional-chain arm. The null-valued cases also assert Number.isNaN(...) === false, which is the guard's actual purpose — Number(null) is 0 but Number(undefined) is NaN, so an unguarded read would put NaN into the returned deleted count.

Impact / risks

• No production code changed, so no runtime risk.
• No src/** lines in the diff, so this carries no Codecov patch percentage; the coverage it adds to the two existing branches is the whole deliverable.

Validation

• Both target branches confirmed covered via lcov: line 78 (row?.n ?? 0) and line 88 (meta?.changes ?? 0) each report both branch paths taken. Measured with the other 18 tests in the file skipped, so the coverage comes from these four tests directly rather than incidentally from the D1-backed tests.
• npm run typecheck — clean; oxlint — clean.
• Branch is current main (9b9924b3).

One note for the reviewer: 16 pre-existing tests in this file fail on my local win32 checkout with ERR_SQLITE_ERROR: column index out of range from the test/helpers/d1.ts shim. That count is identical on unmodified main — I verified by stashing this change — and my four tests use plain mocks with no D1 involvement, so they pass regardless. Flagging it only so the local-vs-CI difference isn't mistaken for something this PR introduced.

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[loopover-orb]

Caution

🛑 LoopOver review result - fixes required

_{Review updated: 2026-07-24 15:39:37 UTC}

1 file · 1 AI reviewer · no blockers · CI failing · blocked

🛑 Suggested Action - Fix Blockers

Review summary
This is a test-only PR adding four unit tests to cover the two defensive `?? 0` fallback arms in `pruneExpiredRecords` (dry-run row count and delete-loop meta.changes), mirroring the existing pattern already tested for the sibling `dedupeSignalSnapshots`. The tests correctly hand-build a minimal `env.DB` mock returning the degenerate shapes and assert both `deleted === 0` and `Number.isNaN(deleted) === false`, which validates the actual reason for the guard (undefined → NaN vs null → 0). The PR is properly scoped: no production code changed, explicitly tied to issue #8370, and the diff matches its stated intent exactly.

Nits — 3 non-blocking

• The two `validate` and `validate-tests-merge` CI failures are undetailed, and this branch is 8 commits behind the default branch, so rebasing first would help confirm whether the failures are pre-existing/upstream rather than caused by this diff.
• The inline comments explaining `Number(null)` vs `Number(undefined)` behavior are a nice touch but slightly verbose for a one-line guard; could be trimmed if the maintainer prefers terser test comments.
• Once rebased past the 8-commit gap, re-run CI to confirm `validate` and `validate-tests-merge` pass cleanly on this PR's own changes.

CI checks failing

• validate
• validate-tests-merge

Decision drivers

• ✅ Code review — No blockers (1 reviewer)
• ✅ Gate result — Passing (No configured blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#8370
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 135 registered-repo PR(s), 68 merged, 10 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	kai392; Gittensor profile; 135 PR(s), 10 issue(s).
Improvement	ℹ️ Insufficient signal	risk: clean · value: insufficient-signal · LLM: minor

Linked issue satisfaction

Addressed
The PR adds four tests directly targeting pruneExpiredRecords's dry-run row?.n ?? 0 and delete-loop meta?.changes ?? 0 fallbacks, mirroring the sibling function's existing test approach and asserting fallback to 0 without NaN.

Review context

• Author: kai392
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: Python, JavaScript, Kotlin, Perl, TypeScript, Vue
• Official Gittensor activity: 135 PR(s), 10 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: Triage stale or unlinked PRs.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask 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.

• @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
• A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

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.

• Re-run LoopOver review

[loopover-orb]

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.