Skip to content

fix(miner): countConsecutiveDisengagements miscounts the streak when a PR outcome is corrected after later PRs resolve #7222

Description

@JSONbored

Context

packages/loopover-miner/lib/loop-reentry.js:34-42's countConsecutiveDisengagements is documented as walking backward from the most recently recorded PR for a repo until a merged outcome breaks the streak:

export function countConsecutiveDisengagements(eventLedger, repoFullName) {
  const outcomes = [...readPrOutcomes(eventLedger, { repoFullName }).values()];
  let count = 0;
  for (let i = outcomes.length - 1; i >= 0; i -= 1) {
    if (!isDisengagedOutcome(outcomes[i])) break;
    count += 1;
  }
  return count;
}

It treats readPrOutcomes(...).values() array order as chronological recency. But pr-outcome.js:79-90's readPrOutcomes builds a Map keyed by repoFullName:prNumber; per JS Map semantics, Map.set() on an already-present key updates its value but never moves its iteration position — the key stays where it was first inserted. So once the same PR number gets a second, later outcome event (a real scenario: a PR closed-without-merge, then reopened and merged — pr-disposition-poller.js re-polls and recordPrOutcomeSnapshot re-fires for the same PR number with a new decision, and nothing dedupes this), that PR's entry stays frozen at its original (older) position in iteration order even though its value now reflects the newest event.

This function feeds directly into attemptLoopReentry's (loop-reentry.js:78) safety circuit-breaker (maxConsecutiveDisengagements) that pauses a repo from further autonomous re-entry — a miscount here is safety-relevant, not cosmetic.

Reproduced: three PRs closed in sequence, then PR #1 (the earliest-inserted) reopened and merged (the chronologically latest event) → countConsecutiveDisengagements returns 2 instead of the correct 0.

Requirements

  • countConsecutiveDisengagements (or its data source) must order outcomes by true event recency, not Map insertion-of-first-occurrence order, before doing the backward walk.
  • Suggested minimal fix: in pr-outcome.js's readPrOutcomes (lines 79-90), call latest.delete(key) immediately before latest.set(key, value) on every iteration — this makes Map iteration order always reflect "most-recently-updated last," matching the function's own doc claim ("a later event for the same repo/PR supersedes an earlier one") and fixing the downstream consumer without changing readPrOutcomes's public shape (still a Map).
  • buildAnonymizedOrbBatch (orb-export.js:74), the only other consumer of readPrOutcomes, already sorts its own output by prHash independently of input order — confirm this change doesn't alter its behavior via its existing test suite.
  • Must not change countConsecutiveDisengagements's behavior for the common case (each PR resolved exactly once) — only the reopen/re-decide case.

Deliverables

  • Fix in packages/loopover-miner/lib/pr-outcome.js's readPrOutcomes (or equivalently in loop-reentry.js) so iteration/consumption order reflects true latest-event recency per (repo, PR) key.
  • Regression test in test/unit/miner-loop-reentry.test.ts reproducing the reopen-then-merge scenario: several PRs closed, then an earlier-numbered PR reopened and merged after them, asserting countConsecutiveDisengagements returns 0 (not the pre-fix miscount).
  • Confirm test/unit/miner-pr-outcome.test.ts (if present) and orb-export.js's tests still pass unchanged.

Test Coverage Requirements

99%+ Codecov patch coverage (branch-counted) on every changed line in pr-outcome.js and/or loop-reentry.jspackages/loopover-miner/lib/** is a covered path under this repo's Codecov config. Both the reopen-correction branch and the ordinary single-resolution branch must be exercised.

Expected Outcome

countConsecutiveDisengagements — and by extension attemptLoopReentry's circuit-breaker — reflects the actual most recent PR-outcome history for a repo, so a genuine recent merge correctly resets the disengagement streak, and symmetrically a genuine recent disengagement is not masked by an older merge sitting later in Map insertion order.

Links & Resources

  • packages/loopover-miner/lib/loop-reentry.js:30-42 (countConsecutiveDisengagements)
  • packages/loopover-miner/lib/pr-outcome.js:73-90 (readPrOutcomes)
  • packages/loopover-miner/lib/pr-disposition-poller.js:145-173 (the real poller that can legitimately re-observe and re-record a reopened PR's new terminal state)
  • packages/loopover-miner/lib/orb-export.js:59-76 (buildAnonymizedOrbBatch, the other readPrOutcomes consumer, order-independent — confirms the fix is safe to make in the shared function)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions