fix(mirror): resolve solved_by_pr from the effective (most-recent) close#189
Open
JSONbored wants to merge 1 commit into
Open
fix(mirror): resolve solved_by_pr from the effective (most-recent) close#189JSONbored wants to merge 1 commit into
JSONbored wants to merge 1 commit into
Conversation
GitHub freezes issue.closedAt at the first close, so both selectClosingPr paths mis-resolve a NOT_PLANNED -> reopen -> COMPLETED-by-PR issue: the timeline anchor matches the first (NOT_PLANNED, closer=null) event and the fallback rejects the solver PR (merged after the frozen closedAt). Derive the effective close time from the most-recent CLOSED_EVENT and use it at both sites. solved_by_pr now resolves to the real solver; a currently-NOT_PLANNED issue still resolves to null.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
issues.solved_by_pris resolved from the issue's effective (most-recent) close instead of the frozenissue.closedAt.NOT_PLANNED → reopen → COMPLETED-by-PRcase, where both theClosedEvent.closerpath and the closing-references fallback currently returnnullfor a genuinely solved issue.effectiveClosedAthelper derives the current close time from the latestCLOSED_EVENT; bothselectClosingPrFromTimelineandselectClosingPrFromClosingRefsuse it. No schema change, no new query fields (both paths already fetch the close timeline).Root cause
GitHub freezes
issue.closedAtat the first time the issue entered the closed state and does not advance it on a re-close. Both resolution sites ingithub-fetcher.service.tscompare againstissue.closedAt:selectClosingPrFromTimelinerequiresev.createdAt === closedAt, so it matches the originalNOT_PLANNEDClosedEvent(whosecloserisnull) and skips the laterCOMPLETEDevent whose closer is the merged PR →null.selectClosingPrFromClosingRefsfiltersmergedAt <= closedAt, so the solver PR (merged at the re-close time, after the frozenclosedAt) is rejected →null.This is the unhandled re-close case inside the anchors added by #123 (the
closedAtanchor, added precisely to attribute reopen-reclose cycles to the current closer) and #169 (themergedAt <= closedAtgate). Both assumeclosedAtreflects the current close; the fix restores that assumption by deriving the effective close from the timeline.Proof — real instances in tracked repositories
All currently
CLOSED/COMPLETED, solved by a merged same-repo PR, currently stored withsolved_by_pr = NULL:issue.closedAt(frozen, NOT_PLANNED)COMPLETEDre-closetest)With the fix,
entrius/gittensor#1289resolves to1290(wasnull).The fix preserves every other case
COMPLETEDclose by a merged PR → unchanged (the latestCLOSED_EVENTis that close).NOT_PLANNEDissue with an earlierCOMPLETED+PR close (closed → reopened → closed NOT_PLANNED) → stillnull: the latestCLOSED_EVENTis theNOT_PLANNEDone, so no false attribution.closedAtvsClosedEvent.createdAtskew on single closes, which previously resolved only via the fallback.Related Issues
Fixes #188
Type of Change
Testing
npm run buildinpackages/daspassesnpm run lintpassesnpm run format:checkpassesgit diff --checkcleanselectClosingPragainst the live timelines of all five instances above: returns the solver PR where it previously returnednull, and returnsnullfor the currently-NOT_PLANNEDre-close case.Checklist
fetchIssueClosingPrdocstring now reflects the effective-close anchor)