review-feedback-audit skill: codify the post-deploy quality and feedback audit - #349
Conversation
…fy the post-deploy quality and feedback audit
🦋 Changeset detectedLatest commit: 7b217fc The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Review Guidancegithub-actions (1 file)
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
re-review's collection fixes - Scope the read-only claim to Steps 1-4; Step 5 is a separate write phase needing more than a GET-only broker. - Detect the Search API's silent 1,000-result ceiling when seeding prs.txt, with a Known-constraints entry on splitting the window. - Collect guidance-comment reactions (issue-comment endpoint) so the reaction tally covers more than inline findings. - Known-constraints entry naming the defining file for each sentinel string the audit greps for, so a zero count prompts re-derivation.
…lies to bot-opened threads; state pending features as checks Re-review on #349: replies.tsv kept every non-bot reply repo-wide, so replies in human-opened threads polluted the outcome metrics; a final filter now produces bot_replies.tsv from the bot parent-id set, and later steps read that. The nudge-seeding and visible-footer states are now written as checks against the audited checkout instead of assumed feature timelines.
| or a new-PR / design-note candidate: | ||
|
|
||
| ```sh | ||
| gh pr list --repo Khan/actions --state open --limit 50 \ |
There was a problem hiding this comment.
nitpick (non-blocking): Step 5's gh pr list --limit 50 silently truncates when Khan/actions has more than 50 open PRs, so the mapping step can misreport a finding as a new-PR candidate when an existing open PR already covers it. Introduced by this change (new file). The whole point of Step 5 is to map every report finding to an existing open PR or flag it as new work, but --limit 50 caps the candidate list with no truncation signal — findings whose covering PR falls past the cap get mislabeled as new-PR candidates. The skill is otherwise careful about exactly this failure mode: the Known constraints section documents that search/issues silently caps at 1,000 and the Step 1 collection block explicitly compares total_count against the cap and warns. Applying the same discipline here — raise the limit generously and/or check whether the returned count hit the limit — keeps the mapping step's exhaustiveness claim honest. No investigation call was needed; the cap and the contrast with the skill's own search-cap guard are evident from the diff itself.
| gh pr list --repo Khan/actions --state open --limit 50 \ | |
| gh pr list --repo Khan/actions --state open --limit 200 \ | |
| --json number,title,isDraft > "$WORK/open_prs.json" | |
| [ "$(jq length "$WORK/open_prs.json")" -lt 200 ] || | |
| echo "WARNING: open-PR list hit the --limit cap; raise it" >&2 |
Lower-confidence observations (1)
.claude/skills/review-feedback-audit/SKILL.md:44thought (non-blocking): Step 1 is fully deterministic; the PR's stated goal of comparable audits argues for a committed collection script the skill invokes, with prose reserved for the judgment steps.
| compares `total_count` against the cap; when it warns, split the window | ||
| into `updated:A..B` sub-ranges or report every reviews-derived count as | ||
| a lower bound. | ||
| - **Sentinel strings live in lib code.** The markers this audit greps for |
There was a problem hiding this comment.
suggestion (non-blocking): The sentinel-drift risk this constraint documents could be a CI sync test instead of an operator discipline, matching the repo's existing pattern. The repo already solves this exact doc-vs-code drift class mechanically: workflows/review/version-sync.test.ts fails any PR where review.md's literals diverge from the package version (verified by reading that test). The skill instead asks the auditor to re-derive review-thumbs-followup, the reason vocabulary, and the sketch-block phrase from the checkout by hand — the strings are real (confirmed review-thumbs-followup in lib/thumbs-sweep.ts:156), but nothing keeps the SKILL.md copies true.
A sketch, not a committable replacement:
Add a small sync test asserting the sentinel strings quoted in SKILL.md match the lib constants (export them or grep the source files), so a rename PR fails CI rather than relying on the 'Before trusting any zero measurement, re-derive' instruction.
2 participants
What
A new skill,
.claude/skills/review-feedback-audit/SKILL.md: a runbook for auditing the reviewer's posted output and human feedback in a consumer repo over a bounded window, typically everything since a reviewer deploy. It encodes the procedure just executed by hand against Khan/webapp for the 2026-08-11/12 window: GET-only collection withbash/jq/gh(inline comments, guidance comments by the engine-appendedgh-aw-agentic-workflowmarker, reviews via PR search plus per-PR listing, reaction detail, human replies including parents older than the window), classification (Conventional-Comment labels, thumbs-sweep follow-ups by theirreview-thumbs-followupmarker, the closed reason vocabulary), the metric set (verdict mix, label mix, verbosity with sketch-block share, duplication at three grains, suppression notes parsed from review bodies, feedback-loop latencies, reply outcomes), a report template, and a final step mapping every finding to an open Khan/actions PR or a new-PR candidate.Why
The audit produced the measured inputs behind the current feedback-effort PRs (#332's cross-file family data, #333's end-to-end downvote-to-
duplicateloop timing), but the procedure lived in one session's history. Codifying it makes the next post-deploy audit repeatable and comparable: same collection filters, same grains of duplication, same caveats.Semantics
created_at/submitted_atbecause everysinceparameter filters onupdated_at; an old comment edited inside the window would otherwise pollute the sample.pr-reviewer:marker absence is expected until a visible footer ships; and a GET-only gh broker blocks GraphQL, in which case thread-resolution counts are reported as unavailable rather than approximated from reply activity.Tests
Doc-only change (skill markdown plus changeset); no lib code touched. Full
workflows/reviewsuite green (1326 tests);pnpm lintandpnpm typecheckclean.