From e148590b11b169228c102a865c80774f3ec9d169 Mon Sep 17 00:00:00 2001 From: imkp1 Date: Wed, 29 Jul 2026 01:52:07 +0530 Subject: [PATCH 1/2] fix(repo-finder): grade de facto maintainers whose association under-reports author_association reports public org membership only, so a private member or a lead never added to the org comments as CONTRIBUTOR and their confirmation is discarded. Add a third set, strictly narrower than the 3c union: authors of >=5 of the last 20 merged PRs, bots excluded. Grade on association union de facto. $mca and maintainer_comment_assoc stay association-only, and the union stays ungraded, so the distinction PR #25 drew is unchanged. Threshold measured across 18 repos, not picked: with bots removed the per-author distribution has a clean gap between 3 and 5 of 20. All six non-bot authors at >=5 are insiders whose association under-reports. A cutoff of 3 admits 23 authors and re-creates the commit-rank error #25 fixed. --defacto is optional; an absent list restores the previous behaviour byte for byte. Reuses the merged-PR list from 3a, so no extra API calls. --- agents/repo-finder.md | 25 ++++++++++++++++++++++-- scripts/orchestrator/triage_filter.sh | 26 ++++++++++++++++++++++--- tests/scripts/test_triage_filter.sh | 28 ++++++++++++++++++++++++--- 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/agents/repo-finder.md b/agents/repo-finder.md index abde717..8ae2975 100644 --- a/agents/repo-finder.md +++ b/agents/repo-finder.md @@ -601,6 +601,27 @@ fi ```bash +# De facto maintainers, for --defacto. Association reports PUBLIC org membership +# only, so a private member or a lead never added to the org comments as +# CONTRIBUTOR and their confirmation is discarded. These are the people who +# demonstrably lead the repo: >=5 of the last 20 merged PRs, bots excluded. +# +# 5, not 3: measured across 18 Go and AI repos, the non-bot distribution has a +# clean gap there. 6 authors sit at 5-15 of 20 and every one is an insider whose +# association under-reports; below the gap, 13 authors sit at exactly 3 and are +# ordinary contributors. A cutoff of 3 admits 23 authors and re-creates the +# commit-rank error PR #25 fixed. +# +# $BOT_NAMES is a first pass only. It misses handles like `...-cherrypick-robot`, +# where `bot` is not a whole word. That is safe: the filter's own bot regex is a +# superset (`bot$` catches it) and runs before grading, so a bot that survives +# here is dropped there and can never be graded. +# +# Reuses $MERGED_PRS from 3a. No extra API calls. +DEFACTO="$SCRATCH/defacto.txt" +jq -r '[.[].author.login] | group_by(.) | map(select(length >= 5)) | .[][0]' \ + "$MERGED_PRS" | sort -u | grep -ivE "$BOT_NAMES" > "$DEFACTO" || : > "$DEFACTO" + # Pure function of $ISSUES and $MAINTAINERS — no API calls. Issues failing the # age or triage-signal gate produce no output; every later rejection carries a # reason. See scripts/orchestrator/triage_filter.sh. @@ -610,7 +631,7 @@ fi # leaves a TRUNCATED file that reads as a clean, shorter candidate set — the exact # failure this file exists to prevent. Abort instead of trusting it. if ! "${CLAUDE_PLUGIN_ROOT}/scripts/orchestrator/triage_filter.sh" \ - --issues "$ISSUES" --maintainers "$MAINTAINERS" > "$SCRATCH/stage_a_all.jsonl"; then + --issues "$ISSUES" --maintainers "$MAINTAINERS" --defacto "$DEFACTO" > "$SCRATCH/stage_a_all.jsonl"; then echo "FATAL: triage_filter.sh failed — Stage A output is unreliable, aborting." >&2 exit 10 fi @@ -751,7 +772,7 @@ visibly drop one. Apply the prose here, then say in `notes` that it did. - **Is a bug** (not a feature request): +3 - **Has reproduction steps**: +2 - **Labeled good-first-issue or help-wanted**: +2 -- **Maintainer signal**, graded from `maintainer_signal`: `invites_pr` **+5** · `confirms` **+3** · `neutral` **+1** · `none` **0**. Association is the prerequisite, never the grade — a union-only commenter earns nothing here. +- **Maintainer signal**, graded from `maintainer_signal`: `invites_pr` **+5** · `confirms` **+3** · `neutral` **+1** · `none` **0**. Association is the prerequisite, never the grade. A union-only commenter earns nothing here; a de facto maintainer (>=5 of the last 20 merged PRs, bots excluded) is graded, because association reports public org membership only and under-reports exactly those leads. - **Maintainer engaged within the last 28 days**: +2 - **Issue age**, graded: 2–30d **+1** · 30–90d **0** · 90–365d **−1** · >365d **−2**. Scored, never hard-skipped — old is riskier, not worthless. - **Scope is small** (likely < 100 lines, single-file fix): +2 diff --git a/scripts/orchestrator/triage_filter.sh b/scripts/orchestrator/triage_filter.sh index cb5fc14..3f26eb9 100755 --- a/scripts/orchestrator/triage_filter.sh +++ b/scripts/orchestrator/triage_filter.sh @@ -25,11 +25,12 @@ set -euo pipefail die() { echo "FATAL: $*" >&2; exit 10; } -ISSUES="" MAINTAINERS="" NOW="" +ISSUES="" MAINTAINERS="" DEFACTO="" NOW="" while [ $# -gt 0 ]; do case "$1" in --issues) [ $# -ge 2 ] || die "--issues needs a value"; ISSUES="$2"; shift 2 ;; --maintainers) [ $# -ge 2 ] || die "--maintainers needs a value"; MAINTAINERS="$2"; shift 2 ;; + --defacto) [ $# -ge 2 ] || die "--defacto needs a value"; DEFACTO="$2"; shift 2 ;; --now) [ $# -ge 2 ] || die "--now needs a value"; NOW="$2"; shift 2 ;; *) die "unknown argument: $1" ;; esac @@ -39,6 +40,10 @@ done [ -n "$MAINTAINERS" ] || die "--maintainers is required" [ -f "$ISSUES" ] || die "issues file not found: $ISSUES" [ -f "$MAINTAINERS" ] || die "maintainers file not found: $MAINTAINERS" +# Optional: callers predating the de facto tier pass no list, and an absent list +# reads as empty, which restores the association-only behaviour exactly. +[ -n "$DEFACTO" ] && { [ -f "$DEFACTO" ] || die "defacto file not found: $DEFACTO"; } +[ -n "$DEFACTO" ] || DEFACTO=/dev/null command -v jq >/dev/null 2>&1 || die "jq is required" # Injectable clock so the 24h rule is testable against a fixed fixture. @@ -47,8 +52,11 @@ case "$NOW" in (*[!0-9]*) die "--now must be a unix timestamp";; esac # Regexes use `.` where an apostrophe belongs (don.t, I.ll): this jq program is a # single-quoted shell string and cannot contain one. -jq -c --rawfile m "$MAINTAINERS" --argjson now "$NOW" ' +jq -c --rawfile m "$MAINTAINERS" --rawfile d "$DEFACTO" --argjson now "$NOW" ' ($m | rtrimstr("\n") | split("\n") | map(select(. != ""))) as $maint + # De facto maintainers: a strict subset of $maint, admitted to grading because + # they demonstrably lead the repo. See the caller for how the set is built. + | ($d | rtrimstr("\n") | split("\n") | map(select(. != ""))) as $defacto # Bots are not maintainers. Superset of the 3c bot regex: a repo whose CI bot # files, labels and comments its own issues yields a perfect triage signal on @@ -136,13 +144,25 @@ jq -c --rawfile m "$MAINTAINERS" --argjson now "$NOW" ' | ($human | map(select( .authorAssociation | IN("OWNER","MEMBER","COLLABORATOR") ))) as $mca + + # Grading set, deliberately distinct from both sets above. Association is the + # primary key, but it reports *public* org membership only, so a private member + # or a lead never added to the org arrives as CONTRIBUTOR and their confirmation + # is discarded. $defacto restores exactly those. It is not the $maint union: + # commit rank is not authority, and grading off the union would score any + # prolific committer as a maintainer, which is the distinction PR #25 drew. + # $mca stays association-only because maintainer_comment_assoc reports it. + | ($human | map(select( + (.authorAssociation | IN("OWNER","MEMBER","COLLABORATOR")) + or (.author.login as $a | $defacto | index($a)) + ))) as $mgrade | ($i.labels | map(.name | ascii_downcase)) as $L | ($mc | map(.body // "") | join("\n") | ascii_downcase) as $mbody # Grade only what is left after stripping links and bare @mentions: a comment # that is only a pointer somewhere else states no position on this issue, and no # sentiment tier can read one. - | ($mca | map((.body // "") + | ($mgrade | map((.body // "") | ascii_downcase | gsub("https?://\\S+"; " ") | gsub("@[a-z0-9_-]+"; " ") diff --git a/tests/scripts/test_triage_filter.sh b/tests/scripts/test_triage_filter.sh index c51901b..9c4115b 100755 --- a/tests/scripts/test_triage_filter.sh +++ b/tests/scripts/test_triage_filter.sh @@ -16,7 +16,12 @@ FRESH="2026-07-11T18:00:00Z" # 6 hours old — must be dropped RECENT="2026-07-10T00:00:00Z" # 2 days old — a claim this fresh is still live STALE="2026-05-20T00:00:00Z" # 53 days old — a claim this old is abandoned -printf 'unionlead\n' > "$tmpdir/maintainers.txt" +printf 'unionlead\ndefactolead\n' > "$tmpdir/maintainers.txt" + +# De facto maintainers: authored a large share of the recent merged PRs, but +# GitHub reports them as CONTRIBUTOR (private org membership, or a lead who was +# never added to the org). A strict subset of the 3c union. +printf 'defactolead\n' > "$tmpdir/defacto.txt" cat > "$tmpdir/issues.json" < "$tmpdir/issues.json" < "$tmpdir/issues.json" < "$OUT" +"$FILTER" --issues "$tmpdir/issues.json" --maintainers "$tmpdir/maintainers.txt" \ + --defacto "$tmpdir/defacto.txt" --now "$NOW" > "$OUT" verdict() { jq -r --argjson n "$1" 'select(.number == $n) | .verdict' "$OUT"; } reason() { jq -r --argjson n "$1" 'select(.number == $n) | .reason' "$OUT"; } @@ -356,9 +367,20 @@ want 916 KEEP "stale claim is abandoned, not competing" want 917 KEEP "bare link is not a defect signal" [ "$(field 917 maintainer_signal)" = "none" ] || { echo "FAIL #917 signal: got '$(field 917 maintainer_signal)'"; exit 1; } -# A union-only lead still earns no signal, whatever they wrote. +# A union-only lead still earns no signal, whatever they wrote. Commit rank is +# not authority: the 3c union is deliberately wide so the fortress check errs +# toward skipping, and reading endorsement off it would grade any prolific +# committer as a maintainer. [ "$(field 904 maintainer_signal)" = "none" ] || { echo "FAIL #904 signal: got '$(field 904 maintainer_signal)'"; exit 1; } +# A de facto lead is graded. They are in the union too, so the association-only +# rule above would silently discard the strongest signal the repo has, which is +# how a repo led by a long-term contributor outside the org scores `none` on an +# issue its lead confirmed. +want 927 KEEP "de facto lead confirmed" +[ "$(field 927 maintainer_signal)" = "confirms" ] || { echo "FAIL #927 signal: got '$(field 927 maintainer_signal)'"; exit 1; } +[ "$(field 927 maintainer_comment_assoc)" = "false" ] || { echo "FAIL #927 assoc must stay false"; exit 1; } + # The strongest signal is also the shortest one anyone writes. A prose floor # applied before the match would grade this as no signal at all. want 920 KEEP "terse invite" From e31ca64f5749dc95300495eef3828e3f408d953e Mon Sep 17 00:00:00 2001 From: imkp1 Date: Wed, 29 Jul 2026 02:06:02 +0530 Subject: [PATCH 2/2] fix(repo-finder): admit de facto leads to $mc, fail loud when deriving them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects in the de facto tier as first written. $defacto was documented as a strict subset of $maint and only unioned into the grading set. It is not a subset by construction: $maint ranks contributors by all-time commits, so a lead who joined recently clears 5 of the last 20 merged PRs and still sits outside the top 25, and $maint's association legs cannot catch them either — under-reported association is why they are in the de facto list at all. Such a lead was graded but absent from $mc, which drives the triage gate, maintainer_commented and last_maintainer_comment. The issue then failed the gate and never reached the ranker, which is the silent discard this tier exists to fix; when a good-first-issue label rescued it, the row read maintainer_commented: false beside maintainer_signal: confirms. Union $defacto into $mc instead of assuming containment. The derivation masked its own failures. grep exits 1 on no match, and no match is the ordinary case here, so `|| : > "$DEFACTO"` could not tell an empty and correct result from a jq that died. Both filters now run inside jq and a real failure is fatal. It also counted ghost authors: a deleted account serializes as author: null, and five of them grouped into a literal `null` maintainer. Guard with `// empty`, as 3c already does. An unset $BOT_NAMES would make test("") match every login and empty the set in silence; assert it instead. Rubric prose said association is the prerequisite, which the de facto tier contradicts. Gradeable is association ∪ de facto; the 3c union stays ungraded. Tests: fixture #928 is a de facto lead outside the union, labelled `bug` only so no label can rescue it at the gate — it fails without the $mc change. A legacy run without --defacto asserts #927 falls back to `none`, #928 fails the gate, and every other verdict is byte-identical. 61/64 scripts pass; test_audit_impact, test_parse_workflows and test_smoke_gate fail identically on main here for missing yq and GNU timeout. Co-Authored-By: Claude Opus 5 --- agents/repo-finder.md | 27 +++++++++++++-- scripts/orchestrator/triage_filter.sh | 17 ++++++++-- tests/scripts/test_triage_filter.sh | 49 +++++++++++++++++++++++++-- 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/agents/repo-finder.md b/agents/repo-finder.md index 8ae2975..cc2f9fe 100644 --- a/agents/repo-finder.md +++ b/agents/repo-finder.md @@ -617,10 +617,31 @@ fi # superset (`bot$` catches it) and runs before grading, so a bot that survives # here is dropped there and can never be graded. # +# Both filters run inside jq. `grep -ivE "$BOT_NAMES"` exits 1 when it matches +# nothing, and matching nothing is the ordinary case here — most repos have no +# bot above the threshold — so the pipeline could not tell an empty-but-correct +# result from a jq that died, and `|| :` turned both into "no de facto +# maintainers". Empty is a valid answer; a failure to compute one is not, and +# only jq's own exit code separates them. +# +# `// empty` drops ghost authors: a deleted account serializes as `author: null`, +# which `.author.login` yields as null and five of which would group into a +# literal `null` maintainer. Same guard as 3c, same reason. +# +# An unset $BOT_NAMES would make `test("")` match every login and empty the set +# in silence — the failure mode this tier exists to remove. Assert it instead. +# # Reuses $MERGED_PRS from 3a. No extra API calls. +: "${BOT_NAMES:?BOT_NAMES is unset — define it (3d) before deriving de facto maintainers}" DEFACTO="$SCRATCH/defacto.txt" -jq -r '[.[].author.login] | group_by(.) | map(select(length >= 5)) | .[][0]' \ - "$MERGED_PRS" | sort -u | grep -ivE "$BOT_NAMES" > "$DEFACTO" || : > "$DEFACTO" +if ! jq -r --arg bots "$BOT_NAMES" ' + [ .[].author.login // empty ] + | group_by(.) | map(select(length >= 5) | .[0]) + | .[] | select(test($bots; "i") | not) + ' "$MERGED_PRS" > "$DEFACTO"; then + echo "FATAL: could not derive de facto maintainers from $MERGED_PRS" >&2 + exit 10 +fi # Pure function of $ISSUES and $MAINTAINERS — no API calls. Issues failing the # age or triage-signal gate produce no output; every later rejection carries a @@ -772,7 +793,7 @@ visibly drop one. Apply the prose here, then say in `notes` that it did. - **Is a bug** (not a feature request): +3 - **Has reproduction steps**: +2 - **Labeled good-first-issue or help-wanted**: +2 -- **Maintainer signal**, graded from `maintainer_signal`: `invites_pr` **+5** · `confirms` **+3** · `neutral` **+1** · `none` **0**. Association is the prerequisite, never the grade. A union-only commenter earns nothing here; a de facto maintainer (>=5 of the last 20 merged PRs, bots excluded) is graded, because association reports public org membership only and under-reports exactly those leads. +- **Maintainer signal**, graded from `maintainer_signal`: `invites_pr` **+5** · `confirms` **+3** · `neutral` **+1** · `none` **0**. Gradeable means association **or** de facto membership (>=5 of the last 20 merged PRs, bots excluded) — never the grade itself, which the comment text sets. The de facto tier exists because association reports public org membership only and under-reports exactly those leads. The 3c union stays ungraded: a union-only commenter earns nothing here, because commit rank is not authority. - **Maintainer engaged within the last 28 days**: +2 - **Issue age**, graded: 2–30d **+1** · 30–90d **0** · 90–365d **−1** · >365d **−2**. Scored, never hard-skipped — old is riskier, not worthless. - **Scope is small** (likely < 100 lines, single-file fix): +2 diff --git a/scripts/orchestrator/triage_filter.sh b/scripts/orchestrator/triage_filter.sh index 3f26eb9..70deb37 100755 --- a/scripts/orchestrator/triage_filter.sh +++ b/scripts/orchestrator/triage_filter.sh @@ -54,8 +54,14 @@ case "$NOW" in (*[!0-9]*) die "--now must be a unix timestamp";; esac # single-quoted shell string and cannot contain one. jq -c --rawfile m "$MAINTAINERS" --rawfile d "$DEFACTO" --argjson now "$NOW" ' ($m | rtrimstr("\n") | split("\n") | map(select(. != ""))) as $maint - # De facto maintainers: a strict subset of $maint, admitted to grading because - # they demonstrably lead the repo. See the caller for how the set is built. + # De facto maintainers: narrower than $maint by intent, admitted to grading + # because they demonstrably lead the repo. See the caller for how it is built. + # Narrower by intent is not narrower by construction: $maint ranks contributors + # by ALL-TIME commits, so a lead who joined recently can clear 5 of the last 20 + # merged PRs and still sit outside the top 25 — and the association legs of + # $maint cannot catch them either, since under-reported association is the whole + # reason they are here. So $defacto is unioned into $mc below rather than + # assumed to be inside it. | ($d | rtrimstr("\n") | split("\n") | map(select(. != ""))) as $defacto # Bots are not maintainers. Superset of the 3c bot regex: a repo whose CI bot @@ -137,9 +143,16 @@ jq -c --rawfile m "$MAINTAINERS" --rawfile d "$DEFACTO" --argjson now "$NOW" ' # `// ""` guards ghost authors: a deleted account serializes as `author: null`, # and `null | test(...)` throws, aborting the whole batch. | ($i.comments | map(select((.author.login // "") | test($bots) | not))) as $human + # $defacto belongs here, not only in $mgrade. This set drives the triage gate, + # `maintainer_commented` and `last_maintainer_comment`. Grading a de facto lead + # who is outside $maint while leaving them out of $mc drops the issue at the + # gate unless a label rescues it — the exact silent discard this tier exists to + # fix — and any issue that a label does rescue emits the contradiction + # `maintainer_commented: false` beside `maintainer_signal: confirms`. | ($human | map(select( (.authorAssociation | IN("OWNER","MEMBER","COLLABORATOR")) or (.author.login as $a | $maint | index($a)) + or (.author.login as $a | $defacto | index($a)) ))) as $mc | ($human | map(select( .authorAssociation | IN("OWNER","MEMBER","COLLABORATOR") diff --git a/tests/scripts/test_triage_filter.sh b/tests/scripts/test_triage_filter.sh index 9c4115b..2c70eb0 100755 --- a/tests/scripts/test_triage_filter.sh +++ b/tests/scripts/test_triage_filter.sh @@ -20,8 +20,14 @@ printf 'unionlead\ndefactolead\n' > "$tmpdir/maintainers.txt" # De facto maintainers: authored a large share of the recent merged PRs, but # GitHub reports them as CONTRIBUTOR (private org membership, or a lead who was -# never added to the org). A strict subset of the 3c union. -printf 'defactolead\n' > "$tmpdir/defacto.txt" +# never added to the org). +# +# Two of them, because containment in the 3c union is an intent and not a +# guarantee: `defactolead` is in the union, `outsidelead` is not. The union ranks +# contributors by ALL-TIME commits, so a lead who joined recently clears 5 of the +# last 20 merged PRs while sitting outside the top 25, and association cannot +# catch them either — under-reported association is why they are in this file. +printf 'defactolead\noutsidelead\n' > "$tmpdir/defacto.txt" cat > "$tmpdir/issues.json" < "$tmpdir/issues.json" < "$OUT_LEGACY" +legacy() { jq -r --argjson n "$1" "select(.number == \$n) | .$2" "$OUT_LEGACY"; } + +# 927's author is in the union, so the issue still clears the gate — but with no +# list to lift them past association, the signal they earned above is gone. +[ "$(legacy 927 maintainer_signal)" = "none" ] || { echo "FAIL legacy #927 signal: got '$(legacy 927 maintainer_signal)'"; exit 1; } +# 928's author is in neither set without the list, so the issue is not a +# candidate at all. This is the pre-fix behaviour, reproduced exactly. +[ -z "$(legacy 928 verdict)" ] || { echo "FAIL legacy #928: must fail the gate, got '$(legacy 928 verdict)'"; exit 1; } +# Every issue that does not turn on the de facto tier must be byte-identical. +diff <(grep -Ev '"number":(927|928)[,}]' "$OUT") \ + <(grep -Ev '"number":(927|928)[,}]' "$OUT_LEGACY") \ + || { echo "FAIL: --defacto changed a verdict it must not touch"; exit 1; } + echo "OK test_triage_filter.sh"