pr-self-review: auto-ack + source-issue exception (closes #62)#78
Merged
SnowboardTechie merged 6 commits intoMay 3, 2026
Merged
Conversation
…loses #62) - Phase 2.3 accept semantics extended: a triaged accept that produces no worktree diff by end of pass is auto-classified as an acknowledgment — key enters suppression_set, finding lands under a new ## Acknowledged section in summary.md, and observational findings stop re-surfacing across passes. No fifth verb; the four-action vocab is unchanged. - Phase 2.3 related_issue pre-skip rule gains a source-issue exception: findings tagged related_issue: #N where #N is matched via a declarative close tag (Closes/Fixes/Resolves) in the PR body do NOT pre-select skip — those findings are about the PR's own implementation, not about overlap with separately-tracked work. Refs/Related and timeline cross-references keep the pre-skip default. - Phase 1.1 plumbing: parse list adds Resolves; match_reason enum splits the previous "linked" value into "closes" (declarative) and "refs" (informational + cross-references) so the exception above is mechanical.
- Cross-file edit detection (correctness+security Major): track files touched per finding at triage time via accepts_per_pass[].files_touched instead of inferring from `git diff --name-only HEAD` against the cited file. A fix that lands in a helper file no longer misclassifies as ack. - Defer accept-bullet to Phase 2.4 (simplicity Major): the `accept` semantic now states the contract; reconciliation mechanics live in 2.4. - Outstanding-section coverage (correctness Major): Critical/Major template prose lists "acknowledged" alongside pushed-back/skipped/filed. - Batch mode source-issue exception (correctness+security Minor): the pre-skip rule and exception are restated under batch mode with a default annotation pattern. - Phrasing tightenings: "pre-select accept instead" mirrors skip wording; source-issue exception drops "intentionally narrow" preamble; match_reason prose note drops the rationale clauses (the source-issue exception block carries them); Phase 2.4 drops the "Concretely:" preamble and the regression-protection parenthetical (covered by the edge-case row); ## Acknowledged section description tightened.
- Revert pass-1 mistake: outstanding-section descriptions added "acknowledged" to the unresolved axis, but acks are suppressed and have their own ## Acknowledged section. Including them outstanding would double-list and inflate ship-readiness counts. Restore the original outstanding list and add a forward-pointer to ## Acknowledged from the description. - Description frontmatter mentions auto-ack so readers see the model upfront. - State Root session-state enumeration includes acks. - match_reason note clarifies timeline events always classify as refs. - Batch-mode source-issue annotation includes the issue number for sanity check. - ## Acknowledged section drops the over-hyphenation. - CHANGELOG entry for the source-issue exception drops implementation detail (enum split, parse-list addition) in favor of observable behavior.
…mbiguous
- Pre-pr mode synthesizes a source-issue closes-entry. issue-work now
passes source_issue ({owner}/{repo}#N); pr-self-review fetches the
issue and seeds related-issues.json with match_reason: "closes" so
the source-issue exception fires in the issue-work flow (previously
a no-op in pre-pr because dimension A was wholly skipped). Without
source_issue, dimension A still degrades cleanly to zero entries.
- Batch-mode omission rule is unambiguous: omitted findings are skipped
even when their annotation says "default: accept" (annotations are
recommendations to engage, not behavioral defaults). Closes the
silent-suppression path where omitting a source-issue finding would
auto-accept → auto-ack → suppress without user engagement.
- ## Acknowledged description: trigger is mechanical (empty
files_touched after any accept), not semantic (reviewer prose).
- match_reason note: parenthetical aside on timeline classification
becomes a direct sentence; same content, less dense.
…uched contract
- Batch-mode annotation no longer says "default: accept" (which lied —
omission still routes to skip). New form: "↳ source issue: #N —
type {num} accept to triage" puts the actual mechanic in the hint
the user reads. The same pattern applies to related_issue / related_note
hints. The omission rule prose drops the now-redundant "even if
annotation suggests accept" caveat.
- Pre-pr source-issue synthesis now mandates owner/repo/N validation
against the same metacharacter pattern dimension B uses. source_issue
crosses the trust boundary into gh; without the guard a malformed
value would interpolate into the gh invocation.
- accept semantics: files_touched contract is now explicit on the
no-edit-attempted path. If Claude agrees with the reviewer that no
fix is required, populate files_touched as an empty set explicitly
rather than leaving it undefined. Closes a contract gap that would
break Phase 2.4 reconciliation if an executor inferred "no edit
needed" without writing files_touched.
…cate body at ingest
- Source-issue exception protection extended to batch-mode case. Phase 2.3
now separates source-issue findings (related_issue matches cached
match_reason: closes) before mode dispatch; they always go through
per-finding mode regardless of unsuppressed count, then non-source-issue
findings dispatch normally per the >5 threshold. Closes the gap where
batch-mode omission-equals-skip silently dropped a source-issue finding
whose annotation said "type {num} accept to triage" — annotation alone
was advisory, not protective.
- Pre-pr synthesis truncates issue body to 400 chars at ingest, mirroring
the body_excerpt schema's write-time bound. Full body never enters LLM
context regardless of consumer-side handling.
- Two-axis-shape narrative names the section "## Accepted this session"
matching the actual template header.
SnowboardTechie
deleted the
issue-62-observational-ack-source-issue-exception
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two coordinated spec edits to
pr-self-reviewplus a small contract change toissue-work:Auto-ack on accept-without-diff. An
accepttriage that produces no worktree edit (typical for observational findings the reviewer prose-flagged as "no fix required") is auto-classified as an acknowledgment: the suppression key enterssuppression_setand the finding lands under a new## Acknowledgedsection insummary.md. Observational findings stop re-surfacing across passes. No fifth verb — the four-action vocab is unchanged. Mechanism: eachacceptrecords itsfiles_touchedset at triage time; Phase 2.4 reconciliation classifies entries with empty sets as acks before deciding whether to commit.Source-issue exception to
related_issuepre-skip. Findings tagged with the issue this PR commits to closing (parsed fromCloses #N/Fixes #N/Resolves #NPR-body tags) no longer pre-selectskip. In per-finding mode they pre-selectaccept; in the >5 batch case they're separated from the batch and triaged individually. A real defect about the PR's own intent now surfaces for explicit triage instead of being silently dropped. Phase 1.1'smatch_reasonenum splits the previouslinkedvalue intocloses(declarative tags) andrefs(informational tags + timeline cross-references) so the exception is mechanical.Pre-pr coverage.
issue-workPhase 4 now passessource_issue: {owner}/{repo}#{N}topr-self-review. In pre-pr mode (no PR body yet),pr-self-reviewfetches the source issue viagh issue viewand seeds a syntheticmatch_reason: closescache entry, so the exception fires in the dominant code path. Thesource_issuearg is validated against^[A-Za-z0-9_.-]+$/^[0-9]+$before anyghinterpolation, mirroring Phase 1.1 dimension B's metacharacter rejection rule.Closes #62.
Test plan
Skill bodies are LLM instructions; no executable tests. Verified via:
/pr-self-reviewself-review loop — 5 passes (hit cap), 0 Critical / 0 Major outstanding. Full audit trail at~/.claude/issue-work/SnowboardTechie-athena-notes-62/summary.md.python3 scripts/lint-frontmatter.py— clean (14 agents, 18 skills).docs-lint(relative trailing-slash links) — clean./pr-self-reviewrun: confirm a pass-Naccepton a "no fix required" finding doesn't re-surface in pass N+1, and a finding taggedrelated_issue: #Nwhere #N is the source issue is not pre-skipped.Changelog
## [Unreleased]inCHANGELOG.md.