Skip to content

pr-management-stats: require full engagement schema + add reference impl#257

Merged
potiuk merged 4 commits into
apache:mainfrom
potiuk:pr-management-stats-reference-impl-and-fixes
May 24, 2026
Merged

pr-management-stats: require full engagement schema + add reference impl#257
potiuk merged 4 commits into
apache:mainfrom
potiuk:pr-management-stats-reference-impl-and-fixes

Conversation

@potiuk

@potiuk potiuk commented May 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Three correctness fixes + a canonical reference implementation for pr-management-stats, bundled with a CI fix + GitHub-Actions version bumps that should have come in via dependabot but couldn't.

What was broken

While running /pr-management-stats on <upstream> after a 277-mutation triage session, the dashboard reported:

  • 225 untriaged non-draft PRs (real: ~2)
  • 53 over 4 weeks (real: 0)
  • Health: 🔥 Action needed (real: ⚠️ Needs attention)

Root cause: fetch.md instructed agents NOT to fetch reviewThreads (and by omission, latestReviews + timelineItems). The is_engaged predicate in classify.md counts ALL of those as maintainer engagement. The mismatch meant a maintainer who only left a line-level review comment looked like "no engagement" → PR classified as untriaged → 10× over-count.

Additionally, the agent was caught skipping the line-chart panels, CODEOWNERS panel, and triager-activity panel — the SKILL.md golden rules didn't explicitly forbid partial rendering.

What this PR changes

  1. fetch.md — corrects the OPEN_PRS_QUERY template to include latestReviews, reviewThreads, and timelineItems (with LABELED_EVENT/READY_FOR_REVIEW_EVENT/CONVERT_TO_DRAFT_EVENT). Bumps comments(last:N) from 10 → 25 so the QC-marker scan finds the marker on chatty PRs. Rewrites the "Why no..." section to explain WHICH fields are required vs. genuinely not needed. Drops default batch size from 50 → 30 to absorb the complexity increase.

  2. SKILL.md — new Golden rule 8 ("render ALL sections, never silently skip") + Golden rule 9 (FULL engagement schema is required, not optional).

  3. tools/pr-management-stats/reference.py — canonical reference implementation of the fetch + classify contract.

  4. tools/pr-management-stats/README.md — describes the anti-skip contract and cross-references the existing skill docs.

  5. .github/dependabot.yml — fix invalid cooldown schema. The github-actions and pre-commit ecosystem blocks carried semver-{major,minor,patch}-days keys that those ecosystems don't accept. Dependabot rejected both blocks outright since adoption on 2026-04-29 (zero PRs ever for either ecosystem in 4 weeks; only the uv blocks ran). Strip the unsupported keys, keep default-days: 7.

  6. Workflow pinned-action bumps — all past the 7-day cooldown, applied manually since dependabot couldn't propose them:

    • actions/cache v4.2.2 → v5.0.5 (Node-24 runner; GitHub-hosted satisfies)
    • github/codeql-action v4.35.2 → v4.35.5 (v4.36.0 still in cooldown)
    • zizmorcore/zizmor-action v0.5.2 → v0.5.6
    • astral-sh/setup-uv v7.3.0 → v8.1.0

    ASF allowlist: setup-uv@08807647 and zizmor-action@5f14fd08 are on approved_patterns.yml; actions/* and github/* are exempt via TRUSTED_OWNERS in apache/infrastructure-actions/allowlist-check/check_asf_allowlist.py.

Verified

Re-running the dashboard after the fixes on the same <upstream> queue:

  • Untriaged: 225 → 24 (just adding is_engaged predicate fix) → 2 (with full engagement schema)
  • Untriaged 4w+: 53 → 1 → 0
  • Health: 🔥 → ⚠️

Both remaining "untriaged" PRs are correctly identified — one was opened today (~5 min before the snapshot), one was a documentation PR that the maintainer's triage genuinely missed.

Test plan

  • Reference script runs without errors against <upstream> (530 open PRs)
  • Output JSON sidecar has expected counts
  • Skill markdown lints (no broken links to new paths)
  • pytest (skill-validator) passes (was failing on hardcoded apache/airflow references; now using <upstream> placeholder per skill convention)
  • prek (pre-commit / typos / placeholder checks) passes locally
  • All workflows pinned with SHAs and version comments after bumps
  • Reviewer: re-read SKILL.md and fetch.md changes — does anything need to flow into aggregate.md or render.md?
  • Reviewer: confirm scope-mix (stats fix + CI bump) is acceptable, or ask for it to be split
Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

potiuk added 4 commits May 24, 2026 22:22
@potiuk
pr-management-stats: require full engagement schema + add reference impl
3bd8967 
Fixes three correctness issues in pr-management-stats that cause severe
under-counting of engagement and over-counting of untriaged PRs:

1. `fetch.md` previously said "no statusCheckRollup / mergeable /
   reviewThreads" — claiming none were needed for stats. This is wrong
   for `reviewThreads`, `latestReviews`, and `timelineItems`: the
   `is_engaged` predicate in classify.md explicitly counts maintainer
   line-level review comments, submitted reviews, and label/draft
   timeline events as engagement. Omitting those fields means a
   maintainer who only left a line-level review comment is treated as
   "no engagement" and the PR is misclassified as untriaged. On a
   ~530-PR queue this inflates the untriaged count ~10× (observed:
   225 → 24, then 24 → 2 with full schema + reviewThreads added).

2. `SKILL.md` had no explicit no-skip rule for the 11 dashboard panels.
   Agents under context pressure were observed simplifying away the
   line charts, CODEOWNERS table, and triager-activity table. New
   Golden rule 8 requires all sections to render; missing-data
   stubs are allowed but silent omission is not.

3. New Golden rule 9 documents the FULL engagement schema explicitly
   so agents don't trim the query "to save complexity points".

Also adds:
- `tools/pr-management-stats/reference.py` — canonical reference
  implementation of the fetch + classify contract. Encodes the full
  engagement schema and serves as the single source of truth agents
  can read from.
- `tools/pr-management-stats/README.md` — describes how the agent
  invokes the reference + the anti-skip contract.
- Updated GraphQL template in fetch.md to include the engagement
  fields, with batch size dropped from 50 to 30 to absorb the
  ~11-point complexity increase per page.
@potiuk
chore(ci): fix dependabot cooldown schema and bump pinned actions
d293dc0 
The github-actions and pre-commit ecosystem blocks in
.github/dependabot.yml carried `semver-{major,minor,patch}-days`
cooldown keys, which those ecosystems do not accept. Dependabot
rejected both blocks outright with:

    The property '#/updates/0/cooldown/semver-major-days' is not
    supported for the package ecosystem 'github-actions'.
    The property '#/updates/1/cooldown/semver-major-days' is not
    supported for the package ecosystem 'pre-commit'.
    ...

which is why neither ecosystem produced a single PR in the four
weeks since adoption on 2026-04-29 (the uv blocks were unaffected
and ran normally — see apache#130, apache#233). Strip the unsupported keys and
keep `default-days: 7` for the 7-day settle window.

Apply the bumps that would have landed already had dependabot been
running, all past the 7-day cooldown:

  actions/cache                v4.2.2  -> v5.0.5
  github/codeql-action         v4.35.2 -> v4.35.5
  zizmorcore/zizmor-action     v0.5.2  -> v0.5.6
  astral-sh/setup-uv           v7.3.0  -> v8.1.0

actions/cache@v5 needs runner >= 2.327.1 (Node 24), which the
GitHub-hosted runners we target already satisfy. setup-uv@v8 is a
major bump; CI on this commit is the smoke test.

ASF allowlist: setup-uv@08807647 and zizmor-action@5f14fd08 are
already on approved_patterns.yml. actions/cache and
github/codeql-action are exempt — `actions` and `github` are in
TRUSTED_OWNERS in apache/infrastructure-actions/allowlist-check/
check_asf_allowlist.py.

Generated-by: Claude Code (Opus 4.7)
@potiuk
fix(pr-management-stats): use placeholders in fetch.md + README example
fe31693 
`check-placeholders` pre-commit hook (and skill-validator pytest)
rejected hardcoded `apache/airflow` references in:

  .claude/skills/pr-management-stats/fetch.md:414
  tools/pr-management-stats/README.md:39

Replace with `<upstream>` (the skill's existing placeholder for the
project repo, used on fetch.md lines 171, 178, 185, 193, 203, 223,
238, 348). Also swap `potiuk` for `<maintainer-handle>` in the
README invocation so the example matches the placeholder convention
end-to-end. doctoc TOC added by pre-commit on first README edit.

Generated-by: Claude Code (Opus 4.7)
@potiuk
fix(pr-management-stats): typos in reference.py flagged by typos hook
49e6255 
prek's `typos` hook caught:
- `invokable` -> `invocable` (docstring, line 23)
- `thr` -> `thread` (loop variable in the reviewThreads walk,
  lines 257-258)

Pure rename + spelling fix; no behaviour change.

Generated-by: Claude Code (Opus 4.7)
@potiuk potiuk merged commit b56f74d into apache:main May 24, 2026
15 checks passed
This was referenced May 25, 2026
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@potiuk