fix(metrics): bucket closed PRs by close time, not their last-updated time

[shin-core]

What & why

buildPublicQualityTrend buckets each PR's terminal outcome into a weekly bucket (src/services/public-quality-metrics.ts). The merged arm keyed on the true terminal timestamp (mergedAt), but the closed arm used updatedAt — which GitHub bumps on every later comment, label change, or edit. So a PR closed in week 1 that receives a comment in week 8 was counted in week 8's outcomesClosed, deflating week 1's mergeRatioPct toward 100% and inflating week 8's toward 0%. This trend is exposed publicly (opt-in publicQualityMetrics).

The in-repo sibling already does this correctly: src/services/review-recap.ts's closedAtMs is documented as "the best-available terminal timestamp for a closed-unmerged PR: closedAt when GitHub's payload carried one, else updatedAt". The public-metrics version also carried a dead ?? pr.createdAt third fallback — updatedAt is always written on the row, and createdAt was never a close time.

The fix

• Export closedAtMs from review-recap.ts (behaviour and doc comment unchanged), so there is one definition of "when did this PR close", and import it into public-quality-metrics.ts.
• The closed arm becomes finiteOrNull(closedAtMs(pr)) — a tiny local adapter that maps closedAtMs' NaN-means-skip contract onto the null-means-skip shape parseStamp already uses at the call site, so a PR whose closedAt and updatedAt are both unusable is skipped exactly as before.
• The dead ?? pr.createdAt fallback is removed. closedAtMs already carries the updatedAt fallback for rows written before closed_at was persisted, so no PR that used to count via updatedAt stops counting.
• No change to terminalOutcome, the gate-outcome bucketing (outcome.blockedAt ?? outcome.updatedAt, a different record type), MIN_GATE_TREND_SAMPLE, or mergeRatioPct. No new column or migration.

grep confirms pr.updatedAt ?? pr.createdAt no longer appears in public-quality-metrics.ts.

Tests (test/unit/public-quality-metrics.test.ts)

New #9700 suite:

• A PR with closedAt in an early week and updatedAt bumped into a later week is counted in the close week, not the update week (named regression; fails on current main).
• A closed PR with closedAt: null and a valid updatedAt is still counted, in updatedAt's week (fallback arm).
• A closed PR with both closedAt and updatedAt missing/unparseable is skipped and appears in no bucket (fails on current main, which fell back to createdAt).
• Merged-PR bucketing unchanged — asserted with a merged PR whose closedAt/updatedAt point at other weeks.

The existing "skips trend rows with unparseable timestamps" case, which relied on the removed createdAt fallback, is updated to assert the created-only PR is now skipped.

Validation

• npm run typecheck green; both suites green.
• Diff branch coverage on both changed src files is 100% (every added line and branch); their whole-file branch coverage (98.1% / 100%) is well above the gate floor.
• git diff --check clean; no route/schema/wrangler/migration change, nothing to regenerate.
• Diff is three files: the two services + the unit test.

Closes #9700

[superagent-security]

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

[loopover-orb]

Warning

⏸️ LoopOver review result - manual review recommended

_{Review updated: 2026-07-29 08:10:36 UTC}

3 files · 1 AI reviewer · no blockers · CI green · unstable

⏸️ Suggested Action - Manual Review

Review summary
This PR fixes a real bucketing bug: closed PRs were keyed on updatedAt (which drifts with later comments/labels/edits) instead of their actual close time, skewing the public merge-ratio trend. The fix reuses the already-correct closedAtMs from review-recap.ts via export, adds a small finiteOrNull adapter to preserve the skip-on-unparseable behavior, and removes a dead createdAt fallback that could never fire since updatedAt is always populated. Test coverage is thorough, covering the early-week bucketing, the updatedAt fallback, the both-missing skip case, and confirming merged-PR bucketing is untouched.

Nits — 4 non-blocking

• src/services/public-quality-metrics.ts:98-100 — 'finiteOrNull' is a fairly generic name for a function whose real job is adapting closedAtMs' NaN-skip contract; the doc comment covers this but a name like 'closedAtMsOrNull' would self-document.
• The 'orb(public-stats): bucket closed PRs by closedAt, not by last-edit time #9700' issue reference embedded in code comments (public-quality-metrics.ts:97,159; test file) is unusual for inline comments — consider keeping issue refs in commit/PR metadata rather than baked into source, though this is a style preference and matches an existing pattern in the codebase (review-recap.ts's own doc comments).
• Consider a brief note in the PR description on why gate-outcome bucketing (outcome.blockedAt ?? outcome.updatedAt, public-quality-metrics.ts:135) doesn't need the same closedAtMs-style fix — it's a different record type but a reader may wonder about the asymmetry.
• The exported closedAtMs in review-recap.ts:66 now has two call sites; worth double-checking no other module duplicates similar closed-time logic that should also be consolidated.

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	#9700
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	❌ 5/25	Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload	✅ 10/10	Author activity: 50 registered-repo PR(s), 35 merged, 0 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	shin-core; Gittensor profile; 50 PR(s), 0 issue(s).
Improvement	✅ Minor	risk: clean · value: minor

Linked issue satisfaction

Addressed
The diff exports closedAtMs from review-recap.ts unchanged, imports it into public-quality-metrics.ts, replaces the closed-arm expression with finiteOrNull(closedAtMs(pr)) removing the dead createdAt fallback, and adds named regression tests covering the closedAt-present, updatedAt-fallback, both-unusable-skip, and unchanged-merged-bucketing cases exactly as required.

Review context

• Author: shin-core
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is registered but has no active allocation in the current snapshot.
• Public profile languages: not available
• Official Gittensor activity: 50 PR(s), 0 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: Await review-lane availability.
• Then work through the remaining 1 step in the Signals table above.

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.

Decision record

• action: hold · clause: success
• config: 4af0cc197041af741fce63e3dcde573050a950ad5c8ef3d2f458405c4bc646c1 · pack: oss-anti-slop · ci: passed
• record: a2961c34ce4969988332439ad6be61e53cf9ef4b00a64eec7c75433227b9d457 (schema v5, head 0b92660)

_{🟩 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

[superagent-security]

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

[loopover-orb]

Held for manual review: the gate and required CI are green, but GitHub reports this pull request's mergeable state as unstable because a non-required check or status is not passing, so LoopOver will not auto-merge. A maintainer can resolve the failing check or review and merge manually. This is an automated maintenance action.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.64%. Comparing base (81cecee) to head (0b92660).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9774      +/-   ##
==========================================
+ Coverage   76.58%   76.64%   +0.05%     
==========================================
  Files         282      284       +2     
  Lines       59464    59610     +146     
  Branches     6555     6597      +42     
==========================================
+ Hits        45543    45689     +146     
  Misses      13639    13639              
  Partials      282      282              

Flag	Coverage Δ
backend	100.00% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/services/public-quality-metrics.ts	100.00% <100.00%> (ø)
src/services/review-recap.ts	100.00% <ø> (ø)