You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
[bug] solved_by_pr is null for issues closed NOT_PLANNED, reopened, then solved by a merged PR (closer resolution anchors on the frozen issue.closedAt) #188
issues.solved_by_pr is left NULL for issues that are closed NOT_PLANNED, reopened, then re-closed COMPLETED by a merged same-repo PR. Both resolution paths in selectClosingPr (packages/das/src/webhook/github-fetcher.service.ts) compare against issue.closedAt, but GitHub freezes issue.closedAt at the first time the issue entered the closed state and never advances it on a re-close. So for this lifecycle, closedAt points at the original NOT_PLANNED close (whose ClosedEvent has closer = null), not at the later COMPLETED close performed by the PR.
The effect is deterministic and fires on both the live webhook path (handleIssueClosure → fetchIssueClosingPr) and the backfill path (backfillIssues).
The closer path anchors on exact equality to the frozen closedAt:
// selectClosingPrFromTimeline, line 494if(ev.createdAt!==closedAt)continue;
Walking newest→oldest, the real COMPLETEDClosedEvent (whose createdAt is the re-close time, ≠ frozen closedAt) is skipped; only the original NOT_PLANNED event matches closedAt, and its closer is null → returns null.
The fallback then filters closing-PR references by the same frozen closedAt:
// selectClosingPrFromClosingRefs, lines 567-569// A PR can't have caused a close that predates its merge.if(closedAt==null||!n.mergedAt)returntrue;returnDate.parse(n.mergedAt)<=closedAt;
The real solver PR merged at the re-close time, which is after the frozen closedAt, so mergedAt <= closedAt is false and the PR is filtered out → returns null.
Result: issues.solved_by_pr = NULL for a genuinely solved, COMPLETED issue.
Steps to Reproduce
An issue is closed as NOT_PLANNED at time T1 (e.g. a maintainer triages it shut). GitHub sets issue.closedAt = T1; the ClosedEvent at T1 has closer = null.
The issue is reopened, then a PR with Closes #N merges and re-closes it COMPLETED at T2 > T1. GitHub records a second ClosedEvent at T2 with closer = <the merged PR>, but leaves issue.closedAt = T1.
The issues.closed webhook (or a backfill) runs selectClosingPr. The timeline path skips the T2 event (createdAt = T2 ≠ closedAt = T1); the fallback rejects the PR (mergedAt ≈ T2 > closedAt = T1).
issues.solved_by_pr is stored NULL.
Expected Behavior
issues.solved_by_pr resolves to the PR that performed the current close — the merged same-repo PR recorded on the most-recent ClosedEvent.
Actual Behavior
issues.solved_by_pr = NULL. The issue's solver is dropped.
Proof — real instances in tracked repositories
All currently state = CLOSED, state_reason = COMPLETED, solved by a real merged same-repo PR, and all hit the frozen-closedAt anchor (verified against the live GitHub timeline):
Worked example — entrius/gittensor#1289: timeline is ClosedEvent(T1=05-22T20:38:34Z, NOT_PLANNED, closer=null) then ClosedEvent(T2=05-29T22:22:18Z, COMPLETED, closer=PR#1290), but issue.closedAt is still T1. selectClosingPrFromTimeline skips the T2 PR event (T2 ≠ T1) and returns null; selectClosingPrFromClosingRefs rejects #1290 because mergedAt=05-29T22:22:16Z > closedAt=05-22T20:38:34Z. Stored solved_by_pr = NULL; correct value is 1290 (merged into test).
Impact
solved_by_pr is the authoritative issue-discovery solver signal. With it NULL, the consumer classifies a genuinely solved issue as closed-but-not-solved: the discoverer earns no issue-discovery score for the issue, and the issue instead counts against their issue-credibility ratio (solved / (solved + closed)), which is gated at a hard minimum. So a miner whose issue went through a NOT_PLANNED phase before being solved is both denied the solve and penalized for it. Not a display-only or cosmetic discrepancy — it changes a scoring input, and the triggering lifecycle is live (5 instances across 3 tracked repos in ~3 weeks, 2 of them in entrius/gittensor).
Relationship to prior reports
This is the unhandled case inside the two mechanisms that resolve solved_by_pr today — both assume issue.closedAt reflects the current close, which is not true once an issue has been closed more than once:
fix(mirror): drive solved_by_pr from ClosedEvent.closer #123 (drive solved_by_pr from ClosedEvent.closer) introduced selectClosingPrFromTimeline and added the ev.createdAt === closedAt anchor specifically so that "reopen-then-reclose cycles … attribute [to the current closer], not whichever PR first declared Closes #N." That intent is correct, but because GitHub freezes issue.closedAt at the first close, the anchor matches the first close event — here the NOT_PLANNED one with closer = null — instead of the current COMPLETED close. The anchor therefore produces the opposite of its stated goal whenever the first close was not the solving one.
fix(mirror): link solving PR when issue has no ClosedEvent closer #169 (link solving PR when issue has no ClosedEvent closer) added the fallback's mergedAt <= closedAt gate because "a PR can't [have] cause[d] an earlier close." Also correct in principle, but against the frozen closedAt it rejects the very PR that caused the current close (which merged after the original close).
So this is not a regression of either fix's intended path — a single COMPLETED close still resolves correctly — and not a re-litigation of their design; it's the re-close case both implicitly assumed away. The fix restores exactly the behavior #123 intended (attribute the current closer).
issue.closedAt is unreliable as the "current close" anchor because GitHub freezes it at the first close. Derive the effective close time from the timeline instead — the createdAt of the most-recent ClosedEvent — and use it at both comparison sites:
In selectClosingPr, compute effectiveClosedAt = <createdAt of the last ClosedEvent in timelineItems> (falling back to issue.closedAt only when no ClosedEvent is present).
Pass it to selectClosingPrFromTimeline (use ev.createdAt === effectiveClosedAt at line 494) and to selectClosingPrFromClosingRefs (use Date.parse(n.mergedAt) <= effectiveClosedAt at line 569).
This re-resolves the current close correctly without re-introducing the stale-attribution risk the anchor was added to prevent (a currently NOT_PLANNED issue still resolves to null, since its most-recent ClosedEvent is the NOT_PLANNED one). As a side benefit it also covers the common 1-second closedAt vs ClosedEvent.createdAt skew on ordinary single-closes, which currently relies on the fallback to recover.
Suggested test
Issue with timeline [ClosedEvent(T1, NOT_PLANNED, closer=null), ClosedEvent(T2, COMPLETED, closer=PR#X merged)] and closedAt=T1 → solved_by_pr = X (today: null).
Single COMPLETED close by a merged PR → unchanged (X).
Currently NOT_PLANNED issue with an earlier COMPLETED+PR close (closed→reopened→closed NOT_PLANNED) → null (no false attribution).
COMPLETED close with closedAt and ClosedEvent.createdAt 1 second apart → X via the timeline path.
Verification: live on upstream/test @ 600e222 and main; github-fetcher.service.ts:494 and :567-569 quoted verbatim. All five instances confirmed against the live GitHub timeline (frozen closedAt + merged solver PR). Downstream classification confirmed in the consumer's issue-discovery scorer (not-solved-closed → credibility denominator).
Description
issues.solved_by_pris leftNULLfor issues that are closedNOT_PLANNED, reopened, then re-closedCOMPLETEDby a merged same-repo PR. Both resolution paths inselectClosingPr(packages/das/src/webhook/github-fetcher.service.ts) compare againstissue.closedAt, but GitHub freezesissue.closedAtat the first time the issue entered the closed state and never advances it on a re-close. So for this lifecycle,closedAtpoints at the originalNOT_PLANNEDclose (whoseClosedEventhascloser = null), not at the laterCOMPLETEDclose performed by the PR.The effect is deterministic and fires on both the live webhook path (
handleIssueClosure→fetchIssueClosingPr) and the backfill path (backfillIssues).Affected code
packages/das/src/webhook/github-fetcher.service.tsclosedAt:Walking newest→oldest, the real
COMPLETEDClosedEvent(whosecreatedAtis the re-close time, ≠ frozenclosedAt) is skipped; only the originalNOT_PLANNEDevent matchesclosedAt, and itscloserisnull→ returnsnull.closedAt:The real solver PR merged at the re-close time, which is after the frozen
closedAt, somergedAt <= closedAtis false and the PR is filtered out → returnsnull.Result:
issues.solved_by_pr = NULLfor a genuinely solved,COMPLETEDissue.Steps to Reproduce
NOT_PLANNEDat timeT1(e.g. a maintainer triages it shut). GitHub setsissue.closedAt = T1; theClosedEventatT1hascloser = null.Closes #Nmerges and re-closes itCOMPLETEDatT2 > T1. GitHub records a secondClosedEventatT2withcloser = <the merged PR>, but leavesissue.closedAt = T1.issues.closedwebhook (or a backfill) runsselectClosingPr. The timeline path skips theT2event (createdAt = T2 ≠ closedAt = T1); the fallback rejects the PR (mergedAt ≈ T2 > closedAt = T1).issues.solved_by_pris storedNULL.Expected Behavior
issues.solved_by_prresolves to the PR that performed the current close — the merged same-repo PR recorded on the most-recentClosedEvent.Actual Behavior
issues.solved_by_pr = NULL. The issue's solver is dropped.Proof — real instances in tracked repositories
All currently
state = CLOSED,state_reason = COMPLETED, solved by a real merged same-repo PR, and all hit the frozen-closedAtanchor (verified against the live GitHub timeline):issue.closedAt(frozen, NOT_PLANNED close)COMPLETEDre-closeClosedEvent.createdAttest)Worked example —
entrius/gittensor#1289: timeline isClosedEvent(T1=05-22T20:38:34Z, NOT_PLANNED, closer=null)thenClosedEvent(T2=05-29T22:22:18Z, COMPLETED, closer=PR#1290), butissue.closedAtis stillT1.selectClosingPrFromTimelineskips theT2PR event (T2 ≠ T1) and returnsnull;selectClosingPrFromClosingRefsrejects#1290becausemergedAt=05-29T22:22:16Z > closedAt=05-22T20:38:34Z. Storedsolved_by_pr = NULL; correct value is1290(merged intotest).Impact
solved_by_pris the authoritative issue-discovery solver signal. With itNULL, the consumer classifies a genuinely solved issue as closed-but-not-solved: the discoverer earns no issue-discovery score for the issue, and the issue instead counts against their issue-credibility ratio (solved / (solved + closed)), which is gated at a hard minimum. So a miner whose issue went through aNOT_PLANNEDphase before being solved is both denied the solve and penalized for it. Not a display-only or cosmetic discrepancy — it changes a scoring input, and the triggering lifecycle is live (5 instances across 3 tracked repos in ~3 weeks, 2 of them inentrius/gittensor).Relationship to prior reports
This is the unhandled case inside the two mechanisms that resolve
solved_by_prtoday — both assumeissue.closedAtreflects the current close, which is not true once an issue has been closed more than once:drive solved_by_pr from ClosedEvent.closer) introducedselectClosingPrFromTimelineand added theev.createdAt === closedAtanchor specifically so that "reopen-then-reclose cycles … attribute [to the current closer], not whichever PR first declaredCloses #N." That intent is correct, but because GitHub freezesissue.closedAtat the first close, the anchor matches the first close event — here theNOT_PLANNEDone withcloser = null— instead of the currentCOMPLETEDclose. The anchor therefore produces the opposite of its stated goal whenever the first close was not the solving one.link solving PR when issue has no ClosedEvent closer) added the fallback'smergedAt <= closedAtgate because "a PR can't [have] cause[d] an earlier close." Also correct in principle, but against the frozenclosedAtit rejects the very PR that caused the current close (which merged after the original close).So this is not a regression of either fix's intended path — a single
COMPLETEDclose still resolves correctly — and not a re-litigation of their design; it's the re-close case both implicitly assumed away. The fix restores exactly the behavior #123 intended (attribute the current closer).clear reopened issue solved_by_pr credit) handle the reopen edge by clearing a stale link. This is the symmetric gap on the subsequentCOMPLETEDre-close, where attribution should be re-established but is dropped — the opposite direction, not covered by fix: clear reopened issue solved_by_pr credit #24.closedAtanchor.Suggested fix
issue.closedAtis unreliable as the "current close" anchor because GitHub freezes it at the first close. Derive the effective close time from the timeline instead — thecreatedAtof the most-recentClosedEvent— and use it at both comparison sites:selectClosingPr, computeeffectiveClosedAt = <createdAt of the last ClosedEvent in timelineItems>(falling back toissue.closedAtonly when noClosedEventis present).selectClosingPrFromTimeline(useev.createdAt === effectiveClosedAtat line 494) and toselectClosingPrFromClosingRefs(useDate.parse(n.mergedAt) <= effectiveClosedAtat line 569).This re-resolves the current close correctly without re-introducing the stale-attribution risk the anchor was added to prevent (a currently
NOT_PLANNEDissue still resolves tonull, since its most-recentClosedEventis theNOT_PLANNEDone). As a side benefit it also covers the common 1-secondclosedAtvsClosedEvent.createdAtskew on ordinary single-closes, which currently relies on the fallback to recover.Suggested test
[ClosedEvent(T1, NOT_PLANNED, closer=null), ClosedEvent(T2, COMPLETED, closer=PR#X merged)]andclosedAt=T1→solved_by_pr = X(today:null).COMPLETEDclose by a merged PR → unchanged (X).NOT_PLANNEDissue with an earlierCOMPLETED+PR close (closed→reopened→closed NOT_PLANNED) →null(no false attribution).COMPLETEDclose withclosedAtandClosedEvent.createdAt1 second apart →Xvia the timeline path.Verification: live on
upstream/test@600e222andmain;github-fetcher.service.ts:494and:567-569quoted verbatim. All five instances confirmed against the live GitHub timeline (frozenclosedAt+ merged solver PR). Downstream classification confirmed in the consumer's issue-discovery scorer (not-solved-closed→ credibility denominator).