Repository navigation
feat(skills): add pr-review skill + /v:pr-review command - #1
Merged
Merged
Conversation
Adds a two-axis (Standards ⊥ Spec), stack-agnostic deep code-review skill for a PR/MR or a local diff, plus a thin /v:pr-review command in the /v:* family that runs it. Review-only — never edits, commits, or merges code. Auto-detects the host: GitHub (gh), GitLab (glab), or a hostless local branch. - skills/pr-review/ — SKILL.md + 4 references (exploration checklist, review domains, findings format, comment-posting) - commands/v-pr-review.md — namespaced entry point in the /v:* family - plugin.json + marketplace.json bumped 2.5.5 -> 2.6.0 (version lockstep) - CHANGELOG entry for 2.6.0 Passes the plugin's local gates: frontmatter linter, JSON version lockstep, and intra-plugin .md link resolution.
…ode, drop argument-hint - posting-comments.md: replace the PCRE lazy quantifier (+?) in the GitLab project-path sed — POSIX ERE has no equivalent and BSD/macOS `sed -E` rejects it — with a portable 3-step extraction (BSD/macOS + GNU sed; handles https/ssh/scp-style URLs, ports, and nested group/subgroup paths) - SKILL.md: define "Auto Mode" at first use (Phase 5) - SKILL.md: drop the inert `argument-hint` frontmatter field (slash-command only; no-op on a SKILL.md) - findings-format.md: note the launch-repo-root ./reviews/ location is a deliberate, target-repo-scoped choice (not a doc-placement violation) Docs/skill content only; no version bump.
copeus
added a commit
that referenced
this pull request
Jul 12, 2026
…s (Codex) capped listing, later-token timeout (rc 124), and oversized (>MAX_FILE_READ_BYTES) file each now set confidence=ambiguous (upstream Layer-A override #1 -> FULL_PIPELINE) instead of mislabeling exact; zero-match under incomplete evidence -> ambiguous not failed. 41 selftests green under LANG=C. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
copeus
added a commit
that referenced
this pull request
Jul 13, 2026
…; usage-object conformance; advisor_calls derived Codex findings #1(BLOCKER),#2,#3,#4: empty/string/negative usage no longer becomes measured:true 0/0; aggregate emits null (not real 0) when nothing measured; advisor_calls aggregates independent of token-measured; collect-results deep-validates the usage object + derives advisor_calls from the per-job advisor log (<run-dir>/logs/<job-id>.advisor.jsonl). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
copeus
added a commit
that referenced
this pull request
Sep 1, 2026
Codex round 2: every CRITICAL from round 1 is CLOSED (subagent hold, fail-open-as-mechanism, is_terminal misuse, the detached completion protocol). Four HIGHs remained -- and every one of them said the same thing: the feature needs a STATE-TRANSITION TABLE the spec does not have. For the hook that is two paragraphs, and they are written: - The counter needed a TRANSACTION, not an atomic write. Atomic replacement makes the write atomic, not read-check-increment-write: two writers can both read n, both emit a block, and persist only n+1; a concurrent epic mutation can overwrite the increment (losing the bound) or be overwritten by it (losing epic progress). Now taken under the flock sidecar claim_resume already uses, released BEFORE emitting JSON, bounded wait, fail open on contention. - Enforcement was HOMELESS. It must work when no goal is armed, but armed-record discovery deliberately fails open on zero matches, and .claude/ is not gitignored here so a file there would be committed by accident and trip the scope gate. Its once-per-session marker now lives in the OS temp dir keyed by session_id -- session-scoped by nature, never committed, never reaching the gate. - timeout_sec got its missing numeric contract: integer seconds, 60 to 21600, default 900 UNCHANGED so every existing manifest behaves identically, and the launcher's outer timeout must exceed the job's or the field is decorative. C and D are DEFERRED, with their design problem named rather than quietly dropped. Both need transition tables, and both land in the SAME resume/reconcile machinery (72 call sites in a 5,500-line file pinned by three golden key-set selftests) -- specifying them apart is strictly worse than specifying them together. v2.19 = "the epic state machine learns time and waiting". This is the third cut, so it is checked against salami-slicing in the spec itself: each was driven by a finding, not by workload. E's PREMISE was false; detachment is a completion protocol, not a flag; C and D need transition tables. Against the diagnosis's own ranking v2.18 still ships cause #1 (no blocking primitive) and cause #5 (the codex cap). Acceptance criteria re-scoped to A/B/F/G/H, with an explicit note that criteria for deferred work would be exactly the body/AC contradiction caught three times in this spec already. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
copeus
added a commit
that referenced
this pull request
Sep 1, 2026
A live `/v:triage` run on a real small change this job creates the need for — a TROUBLESHOOTING.md Symptom entry for a stale landing lock. Committed rather than left on disk because an uncommitted record is invisible to the Stop-hook triage gate, which reads committed files back with jq, and because `git worktree remove` deletes an uncommitted run artifact the moment a branch is merged or discarded — the v2.6.4 data-loss shape. The record is schema-valid, its `digest` verifies WITH the Feature-C binding included, and `docs/superpowers/memory/triage-outcomes.jsonl` receives its first data point ever. Verdict low/high => FULL: `content:config_literal` raised the impact band, so predicate 1 correctly fails while 2 through 6 all pass. TWO THINGS THIS RUN EXPOSED, recorded rather than smoothed over. 1. In THIS repository the DIRECT tier is close to unreachable through the real localizer. `compound-v-localize.py` unions a grep over every extracted token, and this repo is documentation-dense, so ordinary requests resolve to hundreds of files at `ambiguous` confidence — which override #1 sends to FULL. Only a request built from a single token unique to one file localizes narrowly. The sizing engine is not wrong; the proportionate tiers simply will not fire here often, and that is worth knowing before anyone measures precision on it. 2. `docs/superpowers/memory/triage-outcomes.jsonl` is in NO job's `write_allowed` in this run's manifest, yet AC-07 requires it to receive `predicted` events and `/v:triage` is their only producer. The event is kept, because suppressing it would ship Feature A5 inert — the no-caller defect this release exists to kill — and the lane gap is reported instead: task-4's `write_allowed` needs that one path added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
copeus
added a commit
that referenced
this pull request
Sep 1, 2026
…T was unreachable Found by dogfooding, not by a test. A request consisting of nothing but an existing filename — `TROUBLESHOOTING.md` — resolved to 301 paths at confidence `ambiguous`, fired Layer-A override #1, and fail-closed to FULL_PIPELINE. Token extraction split it into `.md` and `TROUBLESHOOTING`, treated `.md` as a CSS-ish selector, and grepped it across every markdown file in the repo. The consequence was not "triage is imprecise". It was that the DIRECT tier was **unreachable through /v:triage for any request whatsoever** — and with it the entire nine-predicate auto-route class, its CAS landing gate and its circuit breaker, all of which 3.0 and 3.0.2 built and tested. Predicates 2 through 6 all failed on a request naming one file; after the fix all five pass and README, loops.md and the metaphor doc reach FASTPATH_ELIGIBLE. The fast path is deliberately narrow: only whitespace-separated words that are ALREADY a regular file in the repo. No globbing, no basename search, no fuzzy matching — a guess here would hand a cheap tier to the wrong file. Second defect, same dogfood: re-running triage on the same request died with a raw FileExistsError. `run_preeval` deliberately REDISCOVERS an existing id rather than minting a second one, so a re-run is a supported operation — and the write half then refused it. Byte-identical content is now an idempotent no-op and the PREDICTED append is skipped with it, because `write_record` is precisely what stopped the flow before `append_predicted`: returning "fine" without that signal would put a second predicted event on the stream the breaker reads. Content that DIFFERS is still refused, now naming the fields that moved instead of a stack trace. I got that one wrong first: I made the write a no-op and the selftest caught it, because the assertion it broke was load-bearing rather than stale. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
copeus
added a commit
that referenced
this pull request
Sep 1, 2026
…o-outcome system Ran the shipped localizer and scorer against a real single-file change, no fixtures. Four findings, all reproducible: - A plain-sentence request resolves to 296 paths at confidence ambiguous, fires hard override #1 and fail-closes to FULL with unknown bands. Request-level triage is inert here until localization narrows. - scripts/** and hooks/** are banded high/high, so no executable file in this repo can reach a cheap tier. Deliberate hardening from the build; also means the ladder has no cheap rung for code here. - SCOPED appears unreachable from path rules: the taxonomy declares only low/low and high/high, and the matrix needs a medium band. Content rules do carry medium, and one was probed and did not fire -- recorded as observed, not as proven impossible. - tests/** and examples/** are unbanded, so they cost a model call to classify a class that is obviously plumbing. Nothing widened. Band widths are a safety decision and this document exists to put evidence in front of it rather than make it quietly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
copeus
added a commit
that referenced
this pull request
Sep 3, 2026
…record ever bound to a real run [2026-09-02T170442Z-v3-4-native-first-replace-the-epic-goal-feature-a-of-hooks-e-5664] Phase T of /v:triage, run as documented: localization ambiguous (nine resolved paths across hooks/, scripts/ and commands/), override #1, FULL_PIPELINE. Bound to this session and to base aeae409. The design was approved by the maintainer through four structured questions; the spec records them verbatim. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
2 participants
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.
What
Adds a
pr-reviewskill — a two-axis (Standards ⊥ Spec), stack-agnostic deep code-review skill for a pull/merge request or a local diff — plus a thin/v:pr-reviewcommand in the/v:*family that runs it.Review-only — it never edits, commits, pushes, or merges code.
Why
Compound V orchestrates writing changes (partition → dispatch → 3-pass review gate). This adds a focused, standalone PR/diff review capability contributors can invoke directly (
/v:pr-review <PR>, or empty = current branch vs. its base). It complements/v:review-plan(which reviews a plan before dispatch) with a review of the resulting diff.How it works
gh), GitLab (glab), or a hostless local branch.Contents
skills/pr-review/SKILL.md+ 4references/(exploration checklist, review domains, findings format, comment-posting)commands/v-pr-review.md— namespaced/v:*entry point.claude-plugin/plugin.json+.claude-plugin/marketplace.jsonbumped2.5.5 → 2.6.0(version lockstep)CHANGELOG.mdentry for 2.6.0Conventions / gates (verified locally)
scripts/lint-frontmatter.pyclean (description length, no-Haiku, valid YAML)plugin.json/marketplace.jsonvalid JSON + versions in lockstep (2.6.0).mdcross-refs resolve (dead-link gate)🤖 Generated with Claude Code