feat(data): add contributor reconciliation reports - #123
Conversation
Compare official contributor totals with cached GitHub context and keep maintainer-lane history separated in private outcome data.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e380346b7b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cd041b312
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c204f2a4b7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
JSONbored
left a comment
There was a problem hiding this comment.
@oktofeesh1 this is merge-ready from my pass.
- Deterministic: check rollup is green and merge-tree against current
origin/mainis clean. - Maintainer judgment: official-vs-cached contributor reconciliation is wired into outcome history, decision-pack paths, MCP/API schema, and maintainer-lane separation.
- Validation: tests cover official-authoritative data, GitHub-only context, cached-stat separation, and maintainer association from issues.
extractLinkedIssues populates the linkedIssues the MCP sends for eligibility, score preview, and gate prediction. Its regex matched the closing keywords without a word boundary, so a keyword embedded in a longer word captured the trailing number: `hotfix 5` -> [5], `prefixes 12` -> [12], `unclosed 9` -> [9]. Anchor the keyword alternatives with `\b`, matching the two canonical server-side extractors (src/db/repositories.ts and src/signals/engine.ts, which both use `\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)`). The bare `#` branch stays boundary-free so `#123` still matches anywhere. Adds a regression test for the embedded-keyword non-matches and the standalone-keyword matches. No issue because issue creation is restricted on this repo; this is a small, self-evident correctness fix in a pure helper with no schema or API change.
The linked-issue detector that populates the stored pr.linkedIssues field (what the actual gate-close disposition reads) only matched GitHub's bare `Closes #123` closing-keyword form, not the equally valid, fully-qualified `Closes owner/repo#123` form. A PR whose only closing reference used the qualified form was scored "missing linked issue" and closed under the linked-issue-required policy, even though it correctly referenced a real, open issue. A separate, already-correct implementation of the same qualified-form matching existed in signals/engine.ts (added for #1988), but was only used for pre-open preflight planning, not the post-open gate-evaluation path that actually decides to close a PR -- the two implementations had drifted apart. Consolidated to one: db/repositories.ts now owns the canonical extractLinkedIssueNumbers/extractLinkedIssueNumbersWithOverflow, extended to accept a repoFullName and match owner/repo#N only when owner/repo case-insensitively equals the PR's own repo (a reference to a different repo closes an issue there, not here). engine.ts's local duplicate is removed in favor of importing the canonical version.
…ax (#3880) * fix(signals): recognize the qualified owner/repo#N closing-issue syntax The linked-issue detector that populates the stored pr.linkedIssues field (what the actual gate-close disposition reads) only matched GitHub's bare `Closes #123` closing-keyword form, not the equally valid, fully-qualified `Closes owner/repo#123` form. A PR whose only closing reference used the qualified form was scored "missing linked issue" and closed under the linked-issue-required policy, even though it correctly referenced a real, open issue. A separate, already-correct implementation of the same qualified-form matching existed in signals/engine.ts (added for #1988), but was only used for pre-open preflight planning, not the post-open gate-evaluation path that actually decides to close a PR -- the two implementations had drifted apart. Consolidated to one: db/repositories.ts now owns the canonical extractLinkedIssueNumbers/extractLinkedIssueNumbersWithOverflow, extended to accept a repoFullName and match owner/repo#N only when owner/repo case-insensitively equals the PR's own repo (a reference to a different repo closes an issue there, not here). engine.ts's local duplicate is removed in favor of importing the canonical version. * test(github): cover the nullish body fallback in unlinkedPullRequests counting
extractLinkedIssueNumbersWithOverflow scanned raw PR body text for "Closes #N"-style keywords with no regard for markdown code spans. This repo's own PR template checklist contains the literal example text "(e.g. `Closes #123`)", so every PR that keeps the unmodified checklist item spuriously linked to issue #123 -- exactly the kind of linked-issue misdetection this review stack needs to get right. GitHub's own native closing-keyword linker does not treat backtick-wrapped text as a real directive either, so stripping inline code spans before scanning also brings this closer to GitHub's actual behavior.
Port inline code-span stripping and qualified owner/repo#N scoping from repositories.ts (JSONbored#4039/JSONbored#3862) into the MCP local-branch mirror and the predicted-gate engine so template checklist text does not spuriously link issue JSONbored#123 and branch-analysis gate predictions match server preflight. Co-authored-by: Cursor <cursoragent@cursor.com>
Port inline code-span stripping and qualified owner/repo#N scoping from repositories.ts (JSONbored#4039/JSONbored#3862) into the MCP local-branch mirror and the predicted-gate engine so template checklist text does not spuriously link issue JSONbored#123 and branch-analysis gate predictions match server preflight. Co-authored-by: Cursor <cursoragent@cursor.com>
Port inline code-span stripping and qualified owner/repo#N scoping from repositories.ts (JSONbored#4039/JSONbored#3862) into the MCP local-branch mirror and the predicted-gate engine so template checklist text does not spuriously link issue JSONbored#123 and branch-analysis gate predictions match server preflight. Co-authored-by: Cursor <cursoragent@cursor.com>
…dIssueNumbers (#6630) `predicted-gate-engine.ts`'s local `extractLinkedIssueNumbers` (used by `buildPreflightResult` for the miner's local predicted-gate preview) had no inline-code-span exclusion, unlike its canonical twin `extractLinkedIssueNumbersWithOverflow` in `src/db/repositories.ts`. Since this repo's own PR template contains the literal `` `Closes #123` `` example, a miner previewing an unfilled/lightly-edited template body would misread that backtick-wrapped boilerplate as a real linked-issue reference — silently suppressing a `missing_linked_issue` warning the live gate would still raise, and breaking predicted/live gate parity. Port the canonical exclusion: collect inline code-span ranges via `text.matchAll(/`[^`\n]*`/g)` and skip any closing-keyword match whose range overlaps a span, across all three match forms (bare `#N`, qualified `owner/repo#N`, full URL). The span text itself is not stripped — per the canonical comment, blanking spans would let text on either side combine into a fake closing reference. Scoped only to this file; the canonical extractor and `engine.ts` (which imports the canonical one) are untouched. Extends test/unit/predicted-gate-engine-coverage.test.ts: a backtick-wrapped `` `Closes #123` `` (and qualified/URL variants) resolves to no linked issues, while the same unwrapped `Closes #123` still returns [123]. Closes #6630
…mns; handle transfers; wake on legacy CI events (#9116) * fix(webhook): stop a stale payload reverting close-critical columns; handle transfers; wake on legacy CI events #9057 -- the out-of-order guard protected only state/headSha/mergedAt/ githubUpdatedAt/labels. Every OTHER column was written unconditionally from a possibly-stale payload, including the ones the linked-issue gate reads -- a hard blocker and a close reason. Real sequence: a contributor pushes (`synchronize`, T1), then edits the body to add "Fixes #123" (`edited`, T2). GitHub does not guarantee delivery order, and once heads diverge the two jobs have different coalesce keys, so `synchronize` can be dequeued LAST. The guard correctly preserved state/headSha/labels -- then reverted body and linkedIssuesJson to the pre-edit snapshot, and since `synchronize` is itself in PR_PUBLIC_SURFACE_ACTIONS the same pass re-ran the gate against the reverted body: a wrong autonomous close for "No linked issue detected" on a PR that has one. That close reason appears in the live ledger. body/linkedIssuesJson/linkedIssueClaimedAt now reuse the existing sparse-body preservation path (the incoming value is not current truth in either case), and title/baseRef/headRef/authorAssociation are protected on the same condition. Applied to the onConflictDoUpdate SET clause as well as the INSERT values -- the UPDATE is the path that actually runs for an existing row -- and to the returned record, which callers act on in-process. #9056 -- `repository.transferred` was never handled despite already being in WEBHOOK_METRIC_ACTIONS, so the case was anticipated and never implemented. Since `repositories` is keyed by full_name with no github_id, a transfer just INSERTed a fresh row and orphaned every table renameRepositoryIdentity migrates -- including repository_settings, so autonomy and gate config silently reverted to defaults while the repo kept operating, and staged approvals in agent_pending_actions were lost. Now derives the old name from changes.owner.from.{organization,user}.login plus the current repo name and runs the same identity migration as a rename. #9059(b) -- `status`/`workflow_run` invalidated the CI cache but never woke a re-review, which only moved the stall: the PR then waited on the ~2-minute sweep, which REST-budget backpressure can skip. That matters specifically because `codecov/patch` arrives as a commit STATUS and is the gate's hardest required check, so a repo whose last green signal is a status got no webhook wake at all and the gate looked hung. Now re-reviews behind the same ciReReviewCoalesced guard the check_run/check_suite path uses, so a status storm cannot amplify. Closes #9056 Closes #9057 Tests: 2 stale-payload regressions (a stale delivery cannot revert body, linked issues, title, baseRef or headRef; a newer one still updates all of them), a status-wakes-re-review test, and the two existing invalidation tests updated to pre-claim the coalesce window so the invalidation stays observable alongside the new wake. Full suite green: 22,175 passing. * fix(webhook): address review — protect authorAssociation on the returned record, and close the coverage gaps Review on #9116 was correct on both counts. 1. The returned record overrode title/headSha/headRef/baseRef/mergedAt/labels but omitted authorAssociation, so an in-process caller could still read the stale value even though the DB row was correctly protected -- contradicting the very invariant the previous commit stated. Added, and asserted on the return value (not just the stored row) in the regression test. 2. codecov/patch was low because the transferred-event derivation and the authorAssociation stale-path had no tests. Both covered now. The transfer-predecessor derivation is extracted into an exported pure helper, resolveRepositoryIdentityPredecessor, because several of its shapes are unreachable through the live pipeline -- the repository upsert running alongside this handler requires a full_name and fails first -- so they could not be tested end to end. That also let the dead `?? payload.repository.owner/name` fallbacks go: full_name is authoritative and already required. Full suite green: 22,182 passing. 100% line+branch coverage on all added lines.
Summary
What changed
Why
Validation
Closes #108