fix(miner): order pr-outcome map by event recency so disengagement streaks are correct (#7222)#7239
Conversation
…reaks are correct (JSONbored#7222)
|
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 #7239 +/- ##
=======================================
Coverage 93.81% 93.81%
=======================================
Files 704 704
Lines 69483 69485 +2
Branches 18912 18912
=======================================
+ Hits 65187 65189 +2
Misses 3302 3302
Partials 994 994
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-19 10:44:26 UTC
Review summary Nits — 2 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.
|
Summary
loop-reentry.js'scountConsecutiveDisengagementswalks backward from the end ofreadPrOutcomes(...).values(), treating array order as event recency. Butpr-outcome.js'sreadPrOutcomesbuilds aMapkeyed byrepoFullName:prNumber, and a bareMap.set()on analready-present key updates its value while leaving the key frozen at its first-seen iteration
position. So once the same PR gets a second, later outcome — a real case: a PR closed-without-merge,
then reopened and merged, which
pr-disposition-poller.jsre-polls and re-records — that entry stayedat its old slot even though its value was now the newest. The backward walk then mis-counted a
resolved streak (e.g. reported 1 consecutive disengagement when the most recent event was actually a
merge, which should reset the streak to 0), wrongly pausing re-entry for the repo.
This applies the issue's suggested minimal fix:
readPrOutcomesnowdeletes the key immediatelybefore
setting it, so Map iteration order always reflects most-recently-updated last — matchingthe reducer's own documented "a later event supersedes an earlier one" contract — without changing its
public
Mapshape. The only other consumer,orb-export.js'sbuildAnonymizedOrbBatch, sorts its ownoutput by
prHashindependently of input order, so it is unaffected (confirmed by its existing suite).Closes #7222
Scope
site//CNAME/lovable; no new dependency.Closes #7222).Validation
git diff --checknpm run typecheck(root — clean)npm run test:coverageon the changed module — 100% of the changed lines covered. New regression inminer-loop-reentry.test.ts: PR 1 closed, PR 2 closed, then PR 1 reopened+merged (a later event for thesame PR number) now yields a streak of 0; it returned 1 before the fix.
miner-pr-outcomeandminer-orb-exportsuites pass unchanged (the other consumer is order-independent).npm run build:miner(node --checkpasses).If any required check was skipped, explain why:
packages/loopover-miner/lib/**(one module + one test); CI runs the full suite.Safety
readPrOutcomes's shape is unchanged.