Skip to content

Latest commit

 

History

History
297 lines (256 loc) · 16.2 KB

File metadata and controls

297 lines (256 loc) · 16.2 KB

Build plan

Status key: 🔲 not started · 🚧 in progress · ✅ done

Phase 0 — grounding (done)

  • ✅ Confirmed fullsend-ai/agents already runs a per-PR agent-vs-human delta analysis (skills/autonomy-readiness) as part of the retro agent.
  • ✅ Quantified the historical gap catalog from agent/review-labeled issues and their "Evidence for #N" trails (see README baseline findings).
  • ✅ Mapped the exact comment markers, schemas, and label conventions needed to parse review/retro activity out of a PR's comment stream (see README "Data sources & parsing rules").

Phase 1 — gather-review-observations skill (done)

✅ Built skills/gather-review-observations/ and smoke-tested against 2 weeks of fullsend-ai/fullsend (178 PRs). Found and fixed two real bugs along the way: an ARG_MAX overflow on PRs with long review histories, and a race where the async retro agent's summary comment could land after a PR was already indexed as "fetched," permanently hiding it from a naive head-SHA cache. See the repo's auto-memory for details if revisiting this.

Goal: turn "PRs from the last N period" into structured, reusable JSON.

Arguments: $ARGUMENTS = time period (e.g. "2 weeks", default "1 week"), optional repo filter (default: both fullsend-ai/fullsend and fullsend-ai/agents).

Scripts (pattern after ~/.claude/skills/weekly-summary/scripts/ for pagination handling):

  • fetch-prs-in-period.sh REPO SINCE UNTILgh pr list --state all --search "closed:${SINCE}..${UNTIL}" (handles both merged and closed-without-merge, since #1980-style rejected bot PRs matter too), returns {number, title, url, state, mergedAt, isDraft, headRefName, author}[].
  • fetch-pr-activity.sh REPO NUMBER — fetches pulls/{n}/reviews and issues/{n}/comments, then classifies each item:
    • review_agent_finding — bot identity match + <!-- fullsend:review-agent -->
    • retro_summary — author is fullsend-ai-retro[bot], excluding <!-- fullsend:agent-status:... -->-only status pings
    • human_review / human_commentuser.type == "User"
    • bot_noise — everything else (Qodo, Codecov, github-actions, dependabot) kept but tagged, not analyzed
  • fetch-pr-activity.sh should also record the PR's outcome label (ready-for-merge / requires-manual-review / rejected / none — meaning the last verdict was request-changes, per the label caveat in the README) so downstream skills don't need to re-derive it.

Output: one JSON file per PR under data/<repo>/<number>.json in this repo (gitignored — this is observational data, not something to commit wholesale, though small illustrative samples may be worth keeping for regression-testing the parsing logic). A top-level data/<repo>/index.json lists what's been gathered for a given period, so re-runs can skip PRs already fetched at the same head SHA.

Smoke test: run against the last 2 weeks of fullsend-ai/fullsend before moving on. Spot-check 5 PRs by hand against the actual GitHub UI to confirm classification is correct, especially the retro-comment detection (no marker, so false negatives are the main risk).

Phase 2 — audit-pr-review-gap skill (done)

✅ Built skills/audit-pr-review-gap/ and validated against 8 PRs that already had a retro comment (fullsend-ai/fullsend #4075, #2706, #5407, #5856, #5564, #5589, #5736, #5807). Building it surfaced a Phase 1 gap first: fetch-pr-activity.sh never fetched pulls/{n}/comments (inline review comments), so most substantive human findings were invisible — fixed by adding that endpoint + a pr_author field before trusting Phase 2's input.

The mechanical verdict-mismatch check (skills/audit-pr-review-gap/scripts/check-verdict-mismatch.py) needed two rounds of fixing before it was trustworthy: (1) it initially scanned a sticky comment's entire body, misreading a stale finding from a collapsed <details> history block as live; (2) nearest-by-time pairing between a verdict and its findings comment only ever matched the first run on any PR, since the sticky comment's created_at never changes on edit — fixed by pairing on commit_id against each history block's own head-SHA marker instead. Both bugs were caught before Phase 3 could inherit them.

Validation result — take this to Phase 3 as a top-line finding, not a footnote: of the 8 PRs audited, 3 (37.5%) were retro_missed_gap — the retro agent's own self-assessment didn't mention a gap this independent audit found — and 0 were retro_overstated_gap. The retro agent's self-grading is directionally biased toward under-reporting, not just noisy. This confirms the "Retro self-grading validity" risk in Open questions below; Phase 3 should surface retro_agreement distribution as its own top-line metric, not bury it inside per-category counts.

Validation also surfaced a second finding-set pattern worth handling explicitly, alongside the existing silent-approval rule: on 2 of 8 PRs (#5736, #5807), all "human" review/comment signal traced back to either the PR's own author or someone purely relaying resolution status ("Fixed"/"Acknowledged") in reply to the review agent's own findings — not an independent reviewer. Codified as the "author-as-reviewer rule" in audit-pr-review-gap/SKILL.md.

Goal: an independent, sub-agent-launchable second opinion on a single PR's review-agent-vs-human delta — not just re-reporting what the retro agent already said.

Input: one PR's gathered JSON from Phase 1, or a bare PR URL (in which case it calls Phase 1's fetch scripts itself).

Procedure:

  1. Build the two finding sets and classify (matched / gap / novel) exactly as fullsend-ai/agents skills/autonomy-readiness/SKILL.md does — reuse that classification logic verbatim so output is comparable to the platform's own self-assessment, not a competing taxonomy. Same silent- approval-is-inconclusive rule.
  2. If a retro comment exists for this PR, treat it as a hypothesis to check, not a ground truth to repeat: independently re-derive the delta from the raw review/human comments first, then compare against what the retro said. Record agreement as one of agrees / retro_missed_gap / retro_overstated_gap / no_retro.
  3. For each gap, note which diagnostic category it falls under (missing context / missing test / missing CI gate / missing skill guidance / missing docs — same five categories autonomy-readiness uses), so Phase 3 can aggregate by root cause, not just by symptom.
  4. Specifically check the #2940 pattern on every PR: did the agent's own summary use blocking language ("must be fixed", "should be addressed before merge") while the verdict was COMMENT/APPROVE? That mismatch is cheap to detect mechanically and is the single highest-value signal this skill can produce given the baseline findings.

Output: {pr, gaps: [...], novel_findings: [...], retro_agreement, verdict_language_mismatch: bool, notes} per PR.

Validation before trusting this skill's output: run it against 8-10 PRs that already have a retro comment and manually compare the two assessments. If independent and retro assessments disagree often, that's itself a finding worth writing up before proceeding to Phase 3.

Phase 3 — mine-review-gap-patterns skill (done)

✅ Built skills/mine-review-gap-patterns/ (collect-audits.py + taxonomy.md + SKILL.md) and ran it for real against the 8 audits from Phase 2's validation batch, producing reports/2026-08-12-phase2-validation-batch.md.

Building it surfaced two real bugs/gaps in earlier phases, both fixed before trusting Phase 3's input (same pattern as Phase 2 finding gaps in Phase 1):

  1. audit-pr-review-gap's output had no structured way to distinguish "no gap because coverage was genuinely complete" (autonomy-increasing signal) from "no gap because there was no independent human signal to check against" (inconclusive) — both showed up as gaps: [] with the distinction buried in free-form notes. Added inconclusive (bool) and inconclusive_reason (enum) to the schema, backfilled onto all 8 existing audits by re-reading each one's own notes text (not re-running the sub-agents — the classifications were already stated, just not structured).
  2. diagnostic_category had spelling drift across the 8 audits (missing-test-coverage/missing-test, insufficient-repo-documentation/insufficient-docs for the same two categories) that would have silently split counts. Pinned exact literal strings in audit-pr-review-gap/SKILL.md, normalized the 8 existing files, and added a defensive alias map in collect-audits.py for older data.

First real report's top-line findings (n=8, a validation batch, not a representative period sample — see the report's own Scope caveat):

  • Retro self-grading: 3/8 (37.5%) retro_missed_gap, 0/8 retro_overstated_gap — confirms Phase 2's under-reporting-bias finding with structured counts, not just narrative.
  • 0 of 5 conclusive PRs (0%) showed full independent coverage with no gap — every conclusive PR had at least one real human-caught gap.
  • 0 genuine verdict-language mismatches (fullsend#2940 pattern) in 8 PRs — the one mechanical hit was confirmed to be a check-verdict-mismatch.py pairing artifact (sticky-comment created_at never updates on edit), not a real case. That script needs a head-SHA-based pairing fix before its any_mismatch output can be trusted at scale — same class of bug audit-pr-review-gap/SKILL.md step 2 already had to work around for findings-parsing.
  • Two topic clusters found live recurrence of known, currently open issues via gh search: agents#542 (docs/plans/ awareness, from #5564) and fullsend#1653 (mergeability-status-ignored, from #5736, 2 instances). One had only a partial-scope match (fullsend#5332, dispatch-infra-specific, vs. #5856's mint-entrypoint case — same failure class, different subsystem). Two topic patterns found no existing tracking issue at all as of 2026-08-12: breaking-change-marker-enforcement (closest precedent: one-off fullsend#2654) and review-agent-fails-to-redispatch-after-late-changes (genuinely novel, single-PR evidence so far).

See skills/mine-review-gap-patterns/taxonomy.md for the full pinned category list with search evidence, and the report itself for the complete breakdown.

Known limitation carried forward, not fixed in Phase 3: gathered PR data (data/<repo>/<number>.json) doesn't store closedAt, so there's no persisted way to filter audits by time period yet — Phase 3 currently operates on "whatever's in audits/" rather than a true period slice. Only 8 of the 178 already-gathered fullsend-ai/fullsend PRs have a corresponding audit. Don't build period-filtering machinery until it's actually needed (i.e. until audit-pr-review-gap has been run at enough scale that period scoping matters) — premature per this repo's own avoid-overengineering convention.

Goal: turn N single-PR audits into a periodic report.

Input: the set of audit-pr-review-gap outputs for PRs gathered in a period.

Procedure:

  1. Cluster gap descriptions into categories. Don't re-cluster from scratch every run — pin the taxonomy after the first real pass (start from the known agent/review catalog's top categories) so trend lines are comparable period over period, and only add a new category when a gap genuinely doesn't fit an existing one.
  2. For each cluster, search agent/review-labeled issues (gh api search/issues?q=...+repo:fullsend-ai/fullsend+label:agent/review) to link it to a known parent issue, or flag it as a novel pattern with no existing tracking issue.
  3. Report frequency this period vs. the historical baseline (is a known gap recurring at the same rate, growing, or shrinking after a fix landed?).
  4. Surface verdict_language_mismatch counts specifically, given it's the cheapest high-confidence signal from Phase 2.

Output: a markdown report (period, top gap categories ranked by count, known-vs-novel split, trend vs. baseline, and any autonomy-increasing signal — PRs where the independent audit found full coverage with no gap).

Phase 4 — generate-review-gap-report skill (done)

✅ Built skills/generate-review-gap-report/ as an on-demand orchestrator, not the monitor-style cadence originally sketched below — after discussion, the chosen shape is "run the full gather → audit → mine pipeline for a period whenever invoked," not a background cron job. Takes period + repo filter (same args as Phase 1) plus an optional sample percentage (default 100%, i.e. audit every PR in the window) to bound the cost of Phase 2's one-sub-agent-per-PR audit step on large windows.

New script scripts/select-audit-sample.sh re-lists a period's PRs and returns a uniform-random sample of ceil(N × PCT / 100) PR numbers — sampling only bounds Step 4 (audits), not Step 2 (gathering, which stays cheap/scripted and always covers the full window so data/ isn't artificially incomplete). Verified the ceil math and no-duplicate selection at PCT ∈ {1, 10, 25, 50, 100} against a real 44-PR window before wiring it in.

Validated end-to-end against a real (not hand-picked) sample: 1 PR from fullsend-ai/agents (100%) + 6 of 27 PRs from a fullsend-ai/fullsend window (20%) — see reports/2026-08-12-phase4-validation-run.md. This is the first genuinely period-sampled report (Phase 2/3's batch was hand-picked for retro-comment availability). Also explicitly verified the "reuse existing audit, don't re-launch" logic by resampling the same window at 100% and confirming all 7 already-audited PRs (including one from before this run, #5736) correctly showed as reusable rather than being queued for a new sub-agent launch.

Building this surfaced two real findings, folded into skills/mine-review-gap-patterns/taxonomy.md:

  1. A new coverage-category taxonomy entry, external-contributor-dispatch-gap — a period-sampled run pulled in a PR from an external contributor where the agent was correctly never dispatched (permission gate), which meant zero agent-vs-human signal despite a real, substantial independent human review. This is a large, already-known cluster (fullsend#2552, #4374, #2967, and #5817 — a meta-issue to consolidate 22+ overlapping dispatch-skip issues) that the hand-picked Phase 2/3 batch never surfaced, because it was selected for PRs that already had review-agent activity to compare. Random period sampling finds gap shapes that batch selection bias systematically excludes.
  2. A third occurrence of cross-entrypoint-deployment-validation-gap (fullsend#6037), closer to the taxonomy entry's original per-org/ per-repo framing than the second occurrence was, and notable because the underlying anti-pattern had already been fixed once elsewhere (fullsend#5854/#5855, closed) — the agent missed a reintroduction of an already-fixed design flaw in new code, which a diff-only review without closed-issue-history context couldn't be expected to catch.

Known limitation carried forward: inconclusive_reason: "no-review-activity" (from audit-pr-review-gap) currently conflates "neither side reviewed" with "the agent was gated off but a human did review" — two different evidentiary situations. Not fixed this phase (single occurrence so far); would need a new audit-pr-review-gap schema value (e.g. agent-not-dispatched) if this recurs at volume.

Open questions / risks

  • Retro self-grading validity: Phase 2 step 2 is the direct mitigation, but if independent and retro assessments disagree frequently, that finding should go into the Phase 3 report as its own top-line item, not get buried.
  • GitHub API volume: fetching full comment/review history for every PR across both repos over a multi-week period is a lot of API calls; Phase 1 scripts need pagination and should support incremental gathering (skip PRs already fetched at the same head SHA) from the start rather than bolting it on later.
  • Clustering drift: an LLM re-clustering gap descriptions from scratch each run will produce different category boundaries each time, making trend lines meaningless. Pinning the taxonomy (Phase 3, step 1) is a hard requirement, not a nice-to-have.