Skip to content

fix: stop mis-filing reviewed PRs as co-authored - #153

Merged
adiati98 merged 1 commit into
mainfrom
fix/grouping
Jul 17, 2026
Merged

fix: stop mis-filing reviewed PRs as co-authored#153
adiati98 merged 1 commit into
mainfrom
fix/grouping

Conversation

@adiati98

@adiati98 adiati98 commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Problem

An external PR (docs repo, PR #838) appeared under co-authored PRs despite never being co-authored — it was reviewed (9 reviews, ending in an approval).

Two independent causes:

1. Co-author detection was too loose. A commit counted as yours if the author's email or name merely contained your username, or if a message had a Co-authored-by: trailer and mentioned your username anywhere. A bot commit saying Address @adiati98 review alongside an unrelated trailer was enough to qualify.

2. A wrong verdict could never self-correct. Co-authored is the only category whose evidence — a commit in the PR branch — can be erased afterwards by a force-push, squash, or rebase (that PR was force-pushed). Because the merge step only ever preserved existing entries, and coAuthoredPrs outranks collaborations, a bad entry was stuck permanently.

Changes

  • New scripts/utils/commit-helpers.js — one strict isCommitByUser, shared by the historical crawler and the Active Workbench, which previously kept two diverging inline copies. It counts only positively-established authorship: a GitHub-resolved account, an unambiguous noreply email, or a real Co-authored-by: trailer line. Merge commits never count — merging a target branch into a contributor's branch isn't authoring.
  • Web-flow policy stays per-caller via excludeWebFlow. History still credits genuine web-UI edits, including a review suggestion the PR author committed (GitHub records those with a real trailer). The Workbench keeps treating applying a suggestion as reviewing, not authoring.
  • Self-healing merge in main.js — the Workbench already re-reads commits for every open commented-on PR each run, so it now also reports which it examined and rejected under the historical rule; the merge drops those and lets the PR re-file into its correct bucket the same run. Only open PRs are touched (merged history is frozen), and a failed fetch counts as unknown, never a rejection.

The last point is deliberate and load-bearing: the Workbench's rule is stricter than history's, so pruning history with the Workbench's verdict deletes real co-authored work (it removed an entry whose co-authorship rests on an accepted review suggestion). The two verdicts are computed separately for that reason.

Verification

Re-audited all 275 stored co-authored PRs against live commit data:

  • 273 re-confirmed genuine.
  • Only two fall out: the reviewed-not-authored PR above, and one other entry whose sole user commit is a branch merge.
  • All 7 remaining open co-authored PRs correctly retained under the new rule — including the ones whose credit comes from accepted review suggestions.
  • No category-hierarchy violations; grouper runs clean.

Data and generated report files are intentionally excluded from this PR — the pipeline regenerates them.

🤖 Generated with Claude Code

Co-authored detection counted a commit whenever the author's email or name
merely *contained* the tracked username, or whenever a message had a
`Co-authored-by:` trailer *and* mentioned the username anywhere. Those loose
rules let PRs the user only reviewed or commented on be filed as authored
work.

Co-authored is also the only category whose evidence -- a commit in the PR
branch -- can be erased later by a force-push, squash, or rebase. Combined
with a merge step that only ever preserved existing entries, a wrong verdict
could never self-correct.

- Add scripts/utils/commit-helpers.js: one strict isCommitByUser shared by the
  historical crawler and the Active Workbench, which until now kept two
  diverging inline copies. It counts only positively-established authorship: a
  GitHub-resolved account, an unambiguous noreply e-mail, or a real
  Co-authored-by trailer line. Merge commits never count -- merging a target
  branch into a contributor's branch is not authoring.
- Keep the web-flow policy per caller via `excludeWebFlow`. History still
  credits genuine web-UI edits, including a review suggestion the PR author
  committed (GitHub records those with a real Co-authored-by trailer), while
  the Workbench keeps treating applying a suggestion as reviewing.
- Self-heal stale co-authored entries in main.js. The Workbench already
  re-reads commits for every open commented-on PR each run, so it now also
  reports which it examined and rejected *under the historical rule*, and the
  merge drops those. Reusing the Workbench's stricter verdict here would
  delete real co-authored work, so the two are deliberately kept distinct.
  Only open PRs are touched; merged history is frozen. A failed fetch is
  treated as unknown, never as a rejection.

Verified against all 275 stored co-authored PRs using live commit data: 273
re-confirmed genuine; two fall out (one reviewed rather than authored, one
whose sole user commit is a branch merge).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@adiati98
adiati98 merged commit 64d2475 into main Jul 17, 2026
@adiati98
adiati98 deleted the fix/grouping branch July 17, 2026 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant