Pr Review

Table of Contents

• Core Principle
• When to Use
• Scope Classification Framework
• Classification Examples
• Workflow
• Phase 1: Establish Scope Baseline
• Phase 2: Gather Changes
• Phase 3: Requirements Validation
• Phase 1.5: Version Validation (MANDATORY)
• Phase 4: Code Review with Scope Context
• Phase 4.5: Additive Bias Audit
• Phase 5: Backlog Triage
• Phase 6: Generate Report
• Phase 7: Knowledge Capture
• Quality Gates
• Anti-Patterns to Avoid
• Don't: Scope Creep Review
• Don't: Perfect is Enemy of Good
• Don't: Blocking on Style
• Don't: Reviewing Unchanged Code
• Integration with Other Tools
• Exit Criteria

Scope-Focused PR Review

Review pull/merge requests with discipline: validate against original requirements, prevent scope creep, and route out-of-scope findings to issues on the detected platform.

Platform detection is automatic via leyline:git-platform. Use gh for GitHub, glab for GitLab. Check session context for git_platform:.

Core Principle

A PR review validates scope compliance, not code perfection.

The goal is to validate the implementation meets its stated requirements without introducing regressions. Improvements beyond the scope belong in future PRs.

When To Use

• Before merging any feature branch
• When reviewing PRs from teammates
• To validate your own work before requesting review
• To generate a backlog of improvements discovered during review

When NOT To Use

• Preparing PRs - use pr-prep instead
• Deep code

review - use pensive:unified-review

• Preparing PRs - use pr-prep instead
• Deep code

review - use pensive:unified-review

Scope Classification Framework

Every finding must be classified:

Classification Examples

BLOCKING:

• Null pointer exception in new code path
• SQL injection in new endpoint
• Breaking change to public API without migration
• Test that was passing now fails

IN-SCOPE:

• Missing error handling specified in requirements
• Feature doesn't match spec behavior
• Incomplete implementation of planned functionality

SUGGESTION:

• Better variable name in changed function
• Slightly more efficient algorithm
• Additional edge case test

BACKLOG:

• Refactoring opportunity in adjacent code
• "While we're here" improvements
• Technical debt in files touched but not changed
• Features sparked by seeing the code

IGNORE:

• Personal style preferences
• Theoretical improvements with no practical impact
• Premature optimization suggestions

Workflow

Phase 1: Establish Scope Baseline

Before looking at ANY code, understand what this PR is supposed to accomplish.

Note: Version validation (Phase 1.5) runs AFTER scope establishment but BEFORE code review. See modules/version-validation.md for details.

Search for scope artifacts in order:

1. Plan file: Most authoritative (check spec-kit locations first, then root)

   # Spec-kit feature plans (preferred - structured implementation blueprints)
   find specs -name "plan.md" -type f 2>/dev/null | head -1 | xargs cat 2>/dev/null | head -100
   # Legacy/alternative locations
   ls docs/plans/ 2>/dev/null
   # Root plan.md (may be Claude Plan Mode artifact from v2.0.51+)
   cat plan.md 2>/dev/null | head -100

Verification: Run the command with --help flag to verify availability.

2. Spec file: Requirements definition (check spec-kit locations first)

   find specs -name "spec.md" -type f 2>/dev/null | head -1 | xargs cat 2>/dev/null | head -100
   cat spec.md 2>/dev/null | head -100

Verification: Run the command with --help flag to verify availability.

3. Tasks file: Implementation checklist (check spec-kit locations first)

   find specs -name "tasks.md" -type f 2>/dev/null | head -1 | xargs cat 2>/dev/null
   cat tasks.md 2>/dev/null

Verification: Run the command with --help flag to verify availability.

4. PR/MR description: Author's intent

   # GitHub
   gh pr view <number> --json body --jq '.body'
   # GitLab
   glab mr view <number> --json description --jq '.description'

Verification: Run the command with --help flag to verify availability.

5. Commit messages: Incremental decisions

   # GitHub
   gh pr view <number> --json commits --jq '.commits[].messageHeadline'
   # GitLab
   glab mr view <number> --json commits

Verification: Run the command with --help flag to verify availability.

Output: A clear statement of scope:

"This PR implements [feature X] as specified in plan.md. The requirements are:

1. [requirement]

2. [requirement]

3. [requirement]"

If no scope artifacts exist, flag this as a process issue but continue with PR description as the baseline.

Phase 2: Gather Changes

# GitHub
gh pr diff <number> --name-only
gh pr diff <number>
gh pr view <number> --json additions,deletions,changedFiles,commits

# GitLab
glab mr diff <number>
glab mr view <number>

Verification: Run the command with --help flag to verify availability.

Phase 3: Requirements Validation

Before detailed code review, check scope coverage:

• [ ] Each requirement has corresponding implementation
• [ ] No requirements are missing
• [ ] Implementation doesn't exceed requirements (overengineering signal)

Phase 1.5: Version Validation (MANDATORY)

Run version validation checks BEFORE code review.

See modules/version-validation.md for detailed validation procedures.

Quick reference: 1. Check if bypass requested (--skip-version-check, label, or PR marker) 2. Detect if version files changed in PR diff 3. If changed, run project-specific validations:

• Claude marketplace: Check marketplace.json vs plugin.json versions
• Python: Check pyproject.toml vs __version__
• Node: Check package.json vs package-lock.json
• Rust: Check Cargo.toml vs Cargo.lock

4. Validate CHANGELOG has entry for new version 5. Check README/docs for version references 6. Classify findings as BLOCKING (or WAIVED if bypassed)

All version mismatches are BLOCKING unless explicitly waived by maintainer.

Phase 3.5: PR Hygiene Checks

Before diving into code, run the PR hygiene checks from modules/pr-hygiene.md:

1. Atomicity check: Does this PR contain one logical change? Flag mixed commit types (feat, refactor, and fix), formatting commits bundled with logic, or changes spanning unrelated subsystems. Large PRs get 30% defect detection vs 75% for focused ones.

2. Agent curation check: Does the code show signs of iterative AI generation without a cleanup pass? Look for redundant implementations, premature abstractions, incomplete refactors, and scope drift.

3. Self-review signals: Are there unsquashed fixup commits, debug statements, or commented-out code that suggest the author did not read their own diff before sending?

Classify findings per modules/pr-hygiene.md severity tables.

Phase 4: Code Review with Scope Context

Use pensive:unified-review on the changed files. For comment quality assessment, see modules/comment-guidelines.md.

Critical: Evaluate each finding against the scope baseline:

**Verification:** Run the command with `--help` flag to verify availability.
Finding: "Function X lacks input validation"
Scope check: Is input validation mentioned in requirements?
  - YES → IN-SCOPE
  - NO, but it's a security issue → BLOCKING
  - NO, and it's a nice-to-have → BACKLOG

Verification: Run the command with --help flag to verify availability.

Phase 4.5: Additive Bias Audit

Run Skill(imbue:justify) on the PR changes to detect AI additive bias, test-logic tampering, and unnecessary complexity.

Key checks:

1. Additive bias score: flag changes with high add/delete ratio (>5:1) that lack justification 2. Iron Law compliance: verify test assertions were not weakened to match broken implementations 3. Minimal intervention: confirm each changed file was necessary and the change was the smallest fix

Classify justify findings using the scope framework:

Include the additive bias score and Iron Law status in the Phase 6 report.

Phase 4.6: Invariant Conflict Detection

Check whether the PR touches existing design invariants. This is a judgment problem that models get wrong far too often: surface conflicts for human review rather than silently accepting or rejecting them.

Quick detection heuristic:

1. Do changed files cross module boundaries that previously didn't interact? 2. Do changes introduce a new pattern alongside an existing one (two ways to do the same thing)? 3. Do interface/type/schema files change shape? 4. Do data flow directions change? 5. Are ADR-documented decisions being contradicted?

# Check for structural pattern changes
git diff --name-only HEAD...origin/master 2>/dev/null \
  | rg "(interface|types|schema|model|base|core|contract)" \
  || git diff --name-only HEAD...origin/master 2>/dev/null \
  | grep -E "(interface|types|schema|model|base|core|contract)"

When a conflict is detected:

Do NOT resolve it. Add to the report as a special category:

For each invariant conflict, present:

1. The invariant: Name the design decision and why it was made (reference ADRs if available) 2. The conflict: What this PR does that clashes 3. Option A, Preserve: Don't merge this change; the invariant pays dividends elsewhere 4. Option B, Layer: Merge as-is, accepting inelegance; not every feature must be elegant 5. Option C, Revise: The invariant is wrong; here's what a redesign would look like

Classification: INVARIANT findings are always BLOCKING, not because the code is wrong, but because the judgment call requires human input. Only the human reviewer can decide which of the three options is right.

Why this matters: Bad invariant decisions compound. A few wrong calls and the codebase becomes unsalvageable. This is not a context problem solvable with better documentation: it is a judgment problem that requires human wisdom.

Phase 5: Backlog Triage

For each BACKLOG item, create an issue on the detected platform:

# GitHub
gh issue create \
  --title "[Tech Debt] Brief description" \
  --body "## Context
Identified during PR #<number> review.
..." \
  --label "tech-debt"

# GitLab
glab issue create \
  --title "[Tech Debt] Brief description" \
  --description "## Context
Identified during MR !<number> review.
..." \
  --label "tech-debt"

Verification: Run the command with --help flag to verify availability.

Ask user before creating: "I found N backlog items. Create issues? [y/n/select]"

Phase 6: Generate Report

Structure the report by classification. Every BLOCKING and IN-SCOPE finding MUST include educational insights per modules/educational-insights.md: Why (the principle), Proof (link to best practice), and a Teachable Moment (generalized lesson). SUGGESTION findings include Why and optionally Proof. BACKLOG items need only a brief rationale.

## PR #X: Title

### Scope Compliance
**Requirements:** (from plan/spec)
1. [x] Requirement A - Implemented
2. [x] Requirement B - Implemented
3. [ ] Requirement C - **Missing**

### Blocking (1)
1. [B1] SQL injection via string concatenation
   - **Location**: `db/queries.py:89`
   - **Anchor**: `cursor.execute("SELECT * FROM t WHERE id = " + uid)`
   - **Issue**: User input interpolated directly into SQL
   - **Why**: String-interpolated SQL allows attackers to
     execute arbitrary queries (CWE-89). This is the #1
     web application vulnerability per OWASP Top 10.
   - **Proof**: [OWASP SQL Injection](https://owasp.org/www-community/attacks/SQL_Injection)
   - **Teachable Moment**: Always use parameterized queries
     or an ORM. This applies everywhere user input reaches
     a database, cache, or search engine query.
   - **Fix**: Use parameterized query:
     `cursor.execute("SELECT * FROM t WHERE id = ?", (uid,))`

### In-Scope (1)
1. [S1] Missing validation for edge case
   - **Location**: `api.py:45`
   - **Anchor**: `def handle_request(payload):`
   - **Issue**: Empty input not handled per requirement
   - **Why**: Defensive validation at API boundaries
     prevents cascading failures in downstream logic.
   - **Proof**: [Postel's Law](https://en.wikipedia.org/wiki/Robustness_principle)
   - **Teachable Moment**: Validate inputs at system
     boundaries (API handlers, CLI args, file parsers)
     but trust internal function contracts.

### Suggestions (1)
1. [G1] Consider extracting helper function
   - **Why**: The repeated pattern on lines 30-35 and
     72-77 violates DRY. Extracting it reduces future
     bug surface.
   - Author's discretion

### Backlog → GitHub Issues (3)
1. #142 - Refactor authentication module
2. #143 - Add caching layer
3. #144 - Update deprecated dependency

### Recommendation
**APPROVE WITH CHANGES**
Address B1 and S1 before merge.

Local Output (--local)

When --local [path] is passed, write the Phase 6 report to a local .md file instead of posting via API. Default path: .pr-review/pr-<number>-review.md. The file includes the review summary, test plan, and backlog items in a single document. Issue creation and PR description updates are skipped. Knowledge capture (Phase 7) still runs.

Phase 7: Knowledge Capture

After generating the report, evaluate findings for knowledge capture into the project's review chamber.

Trigger: Automatically for findings scoring ≥60 on evaluation criteria.

# Capture significant findings to review-chamber
# Uses memory-palace:review-chamber evaluation framework

Verification: Run the command with --help flag to verify availability.

Candidates for capture:

• BLOCKING findings with architectural context → decisions/
• Recurring patterns seen in multiple PRs → patterns/
• Quality standards and conventions → standards/
• Post-mortem insights and learnings → lessons/

Output: Add to report:

### Knowledge Captured 📚

| Entry ID | Title | Room |
|----------|-------|------|
| abc123 | JWT over sessions | decisions/ |
| def456 | Token refresh pattern | patterns/ |

View: `/review-room list --palace <project>`

Verification: Run the command with --help flag to verify availability.

See modules/knowledge-capture.md for full workflow.

Quality Gates

A PR should be approved when:

• [ ] All stated requirements are implemented
• [ ] No BLOCKING issues remain
• [ ] IN-SCOPE issues are resolved or acknowledged
• [ ] BACKLOG items are tracked as GitHub issues
• [ ] Tests cover new code paths
• [ ] Tests would fail if the fix were reverted (the revert test)
• [ ] No obvious agent-generated code left uncurated
• [ ] Author can explain how each changed section works and how

it could fail (understanding check, not just "tests pass")

Anti-Patterns to Avoid

Don't: Scope Creep Review

"While you're here, you should also refactor X, add feature Y, and fix Z in adjacent files."

Do: Create backlog issues, keep PR focused.

Don't: Perfect is Enemy of Good

"This works but could be 5% more efficient with different approach."

Do: If it meets requirements and has no bugs, it's ready.

Don't: Blocking on Style

"I prefer tabs over spaces."

Do: Use linters for style, reserve review for logic.

Don't: Reviewing Unchanged Code

"The file you imported from has some issues..."

Do: That's a separate PR. Create an issue if important.

Don't: Tests That Prove Old Code Was Bad

"Here's a test showing the old behavior was wrong."

Do: Write tests that break if your fix is reverted. Tests should protect against regressions in your code, not document why the change was needed. See modules/pr-hygiene.md Principle 4.

Don't: Bundling Unrelated Changes

"I also reformatted the file and fixed a typo in another module."

Do: One PR = one logical change. Formatting, refactors, and unrelated fixes belong in separate PRs. See modules/pr-hygiene.md Principle 2.

Don't: Merge Code You Cannot Explain

"It works and the tests pass."

A PR where the author cannot explain how each changed section works and how it might fail is not ready to merge. This is especially true for AI-assisted code: generation speed creates the illusion of understanding.

Do: Before marking a PR ready, ask the reviewing agent to question you about the changed code: how each part works, what assumptions it makes, and what inputs would break it. Continue until you can answer without hesitation. Only merge code you own front-to-back.

This applies to self-reviews: run the same probe before requesting external review. Do not submit a PR for review that you yourself do not fully understand.

Verify Findings Are Grounded (pr-review:findings-verified)

Every finding must cite a real location and a verbatim anchor. Write findings to .review/findings.json and confirm each citation resolves:

python plugins/imbue/scripts/citation_verifier.py \
  --findings .review/findings.json --repo-root .

Drop or label UNVERIFIED any finding the verifier fails (exit 1); only verified findings enter the report. See Skill(imbue:review-core) Step 5 and Skill(imbue:structured-output) for the schema.

Integration with Other Tools

• `/fix-pr`: After review identifies issues, use this to address them
• `/pr`: To prepare a PR before review
• `pensive:unified-review`: For the actual code analysis
• `pensive:bug-review`: For deeper bug hunting if needed
• `scribe:slop-detector`: For documentation AND commit message quality analysis
• `scribe:doc-generator`: For PR description writing guidelines (slop-free)

Slop Detection Integration

Documentation Review

For all changed .md files, invoke Skill(scribe:slop-detector):

• Score ≥ 3.0: Flag as IN-SCOPE (should remediate)
• Score ≥ 5.0: Flag as BLOCKING if --strict mode

Commit Message Review

Scan all PR commit messages for slop markers:

gh pr view <number> --json commits --jq '.commits[].messageBody' | \
  grep -iE 'leverage|seamless|comprehensive|delve|robust|utilize|facilitate'

If slop found in commits: Add to SUGGESTION category with remediation guidance.

PR Description Review

Apply scribe:slop-detector to PR body:

• Tier 1 words in description → SUGGESTION to rephrase
• Marketing phrases ("unlock potential") → Flag for removal

Exit Criteria

• Scope baseline established
• All changes reviewed against scope
• Findings classified correctly
• Backlog items tracked as issues
• Clear recommendation provided
• Every BLOCKING and IN-SCOPE finding carries a Location (file:line) and

verbatim Anchor; citation_verifier.py confirmed all citations (exit 0) or unverified findings are dropped/labeled UNVERIFIED

Supporting Modules

• GitHub PR comment patterns - gh api patterns for inline and summary PR comments

Code Comment Quality Guidelines

Guidance on when and how to write effective code comments that add value without bloat.

Core Philosophy: Why, Not What

Good comments explain why code exists, not what it does. The code already shows what it does.

When Comments Are Warranted

Require Comments For

Don't Require Comments For

Examples

Good Comments (Explain Why)

def retry_with_backoff(max_attempts=3):
    """Retry with exponential backoff for transient failures.

    AWS Lambda has a 15-minute timeout, so max_attempts=3 prevents
    exceeding this limit with our 1s/2s/4s backoff strategy.
    """
    ...

# Use set for O(1) membership testing instead of list O(n)
# Critical for processing 100k+ items in batch jobs
seen_ids = set()

Bad Comments (Explain What - Avoid These)

# Bad: Restates code
i = 0  # Set i to 0

# Bad: Obvious from code
if user_input == "":  # Check if user_input is empty string
    return DEFAULT_VALUE

# Bad: Redundant docstring
def add(a, b):
    """Add two numbers and return the result."""
    return a + b

Anti-Patterns

Over-Commenting (Bloat)

Problem: Too many comments obscure code and become maintenance burden.

Symptoms:

• Comment-to-code ratio > 1:3
• Comments on every line
• Comments restating variable names

Solution: Improve code clarity instead. Better names, smaller functions.

Stale Comments (Out of Sync)

Problem: Comments that don't match current code behavior are worse than no comments.

Symptoms:

• Function behavior changed, comment didn't
• TODO comments for completed work
• References to deleted code

Solution: Update comments with code changes. Delete outdated TODOs.

Commented-Out Code

Problem: Dead code clutters codebase and confuses readers.

Solution: Delete it. Git preserves history.

Docstring Conventions

When Required

• Public functions/methods
• Classes with non-obvious purpose
• Modules with significant complexity

Format (Python)

def process_order(order: Order, apply_discount: bool = False) -> Receipt:
    """Process an order and generate a receipt.

    Validates inventory, applies pricing rules, and records transaction.

    Args:
        order: The order to process.
        apply_discount: Whether to apply member discount (default: False).

    Returns:
        Receipt with itemized charges and total.

    Raises:
        InsufficientStockError: If any item is out of stock.
        PaymentFailedError: If payment processing fails.
    """

Format (TypeScript)

/**
 * Process an order and generate a receipt.
 *
 * Validates inventory, applies pricing rules, and records transaction.
 *
 * @param order - The order to process
 * @param applyDiscount - Whether to apply member discount
 * @returns Receipt with itemized charges and total
 * @throws InsufficientStockError if any item is out of stock
 */
function processOrder(order: Order, applyDiscount = false): Receipt {

Review Checklist

When reviewing code comments:

• [ ] Comments explain WHY, not WHAT
• [ ] No redundant comments restating code
• [ ] Complex logic is documented
• [ ] Business rules have context
• [ ] No stale/outdated comments
• [ ] No commented-out code
• [ ] Public APIs have docstrings
• [ ] TODOs have issue references

Integration with KISS/YAGNI

From conserve:code-quality-principles:

• KISS: If code needs extensive comments to understand, simplify the code
• YAGNI: Don't comment for hypothetical future readers - comment for current needs

Summary

Educational Insights for PR Review Findings

Every finding in a PR review is a learning opportunity. Each reported issue, suggestion, or error MUST include educational context so the review improves both the code and the person who wrote it.

The Three Pillars

Each finding includes three educational elements:

Enriched Finding Format

Every finding entry (BLOCKING, IN-SCOPE, SUGGESTION) MUST use this extended format:

1. [S1] Missing input validation on user-supplied path
   - **Location**: `api/handlers.py:45`
   - **Issue**: Path traversal possible via `../` in filename
   - **Why**: Unsanitized file paths allow directory traversal
     attacks (CWE-22). An attacker can read or overwrite
     files outside the intended directory.
   - **Proof**: [OWASP Path Traversal](https://owasp.org/www-community/attacks/Path_Traversal)
   - **Teachable Moment**: Always normalize paths with
     `os.path.realpath()` and verify they stay within the
     expected root. This applies to any function accepting
     file paths from external input.
   - **Fix**:

real = os.path.realpath(user_path) if not real.startswith(allowed_root): raise ValueError("Path outside allowed directory")

How to Source Proof Links

Use authoritative references in this priority order:

1. Language/framework docs (python.org, docs.rs, developer.mozilla.org) 2. Security standards (OWASP, CWE, NIST) 3. Style guides (PEP 8, Google Style Guide, Effective Go) 4. Well-known articles (Martin Fowler, Dan Abramov, Kent Beck) 5. RFCs and specifications (IETF RFCs, W3C specs)

When no authoritative URL exists, cite the principle by name (e.g., "Liskov Substitution Principle") and briefly explain it inline.

Insight Depth by Classification

BLOCKING and IN-SCOPE findings always include proof links. SUGGESTION findings include them when a well-known source exists. BACKLOG items need only a brief rationale since they become separate issues with their own context.

Anti-Patterns

Don't: Lecture Without Context

"You should use pathlib instead of os.path."

Do: Explain why:

"pathlib provides object-oriented path handling that

prevents string concatenation bugs (PEP 428). It also

makes path operations cross-platform by default."

Don't: Link Without Explaining

"See https://owasp.org/..."

Do: Summarize what the link teaches:

"OWASP classifies this as CWE-22 (Path Traversal).

The linked guide shows three defense layers:

canonicalization, allowlisting, and sandboxing."

Don't: Over-Teach on Trivial Findings

[Three paragraphs explaining why a typo matters]

Do: Match depth to severity. A typo fix needs one line, not a lecture.

Integration with Phase 6 Report

The Phase 6 report template already groups findings by classification. Educational insights are embedded inline within each finding, not in a separate section. This keeps the insight next to the code it explains, making the review scannable and the lessons immediately visible.

Exit Criteria

• [ ] Every BLOCKING finding has Why + Proof + Teachable Moment
• [ ] Every IN-SCOPE finding has Why + Proof
• [ ] SUGGESTION findings have Why (Proof if available)
• [ ] Proof links are to authoritative, stable URLs
• [ ] Insights explain the principle, not just the symptom

GitHub PR Comment Patterns

Reusable patterns for posting comments to GitHub PRs via the gh CLI.

Key API Differences

Common Mistakes

Wrong: Individual Comments Endpoint with line parameter

# This will FAIL with HTTP 422
gh api repos/{owner}/{repo}/pulls/{pr}/comments \
  -X POST \
  -f path='file.rs' \
  -f line=63 \  # ERROR: "line" is not a permitted key
  -f body='Comment'

Right: Reviews Endpoint with Comments Array

# This works correctly
gh api repos/{owner}/{repo}/pulls/{pr}/reviews \
  --method POST \
  -f event="COMMENT" \
  -f body="Review summary" \
  -f 'comments[][path]=file.rs' \
  -F 'comments[][line]=63' \  # Use -F for integers!
  -f 'comments[][body]=Inline comment text'

Pattern: Single Inline Comment

gh api repos/{owner}/{repo}/pulls/{pr_number}/reviews \
  --method POST \
  -f event="COMMENT" \
  -f body="See inline comment." \
  -f 'comments[][path]=src/auth/jwt.rs' \
  -F 'comments[][line]=63' \
  -f 'comments[][side]=RIGHT' \
  -f 'comments[][body]=**[IN-SCOPE]** JWT secondary secret

This fallback secret poses a security risk.

**Recommendation:** Fail-fast on missing JWT_SECRET.'

Pattern: Multiple Inline Comments

For multiple comments, use JSON input via --input -:

gh api repos/{owner}/{repo}/pulls/{pr_number}/reviews \
  --method POST \
  --input - <<'EOF'
{
  "event": "COMMENT",
  "body": "Review with inline comments",
  "comments": [
    {
      "path": "src/auth.rs",
      "line": 26,
      "side": "RIGHT",
      "body": "**[IN-SCOPE]** Basic email validation"
    },
    {
      "path": "src/routes.rs",
      "line": 45,
      "side": "RIGHT",
      "body": "**[SUGGESTION]** Consider rate limiting"
    }
  ]
}
EOF

Note: The indexed array syntax (comments[0][path]) does NOT work with gh api - it creates an object instead of an array. Always use JSON input for multiple comments.

Pattern: General PR Comment (Not Inline)

For findings not on diff lines or when inline fails:

gh pr comment $PR_NUMBER --body '## Detailed Findings

### IN-SCOPE (Should fix before merge)

#### 1. JWT Fallback Secret (`src/auth/jwt.rs:62-63`)
**Risk**: If deployed without `JWT_SECRET`, tokens use known secret.
**Fix**: Fail-fast on missing secret.

#### 2. Basic Email Validation (`src/routes/auth.rs:26`)
**Risk**: Accepts invalid emails like `@@` or `test@`.
**Fix**: Use proper email validation.'

Pattern: Submit Review with Summary

# Determine event based on findings
EVENT="COMMENT"  # or "REQUEST_CHANGES" or "APPROVE"

gh pr review $PR_NUMBER \
  --event $EVENT \
  --body "$(cat <<'EOF'
## PR Review Summary

### Blocking Issues (2)
- [B1] Missing token validation (auth.py:45)
- [B2] SQL injection risk (models.py:123)

### Suggestions (3)
- See inline comments for details

**Action Required:** Address blocking issues before merge.
EOF
)"

Secondary Strategy

When inline comments fail (line not in diff, API issues):

1. Try inline first via reviews API 2. On failure, fall back to PR comment with file:line reference in body 3. Always post a summary comment with all findings aggregated

# Secondary: Post as regular comment with location reference
gh pr comment $PR_NUMBER --body "**[B1] Issue at src/auth.rs:45**

This line was not in the PR diff, but the issue was identified during review.

Issue: Missing validation
Severity: BLOCKING
Fix: Add input sanitization"

Extracting Owner/Repo

# From remote URL
REMOTE_URL=$(git remote get-url origin)
OWNER_REPO=$(echo "$REMOTE_URL" | sed -E 's/.*github\.com[:/]([^/]+\/[^/.]+).*/\1/')
OWNER=$(echo "$OWNER_REPO" | cut -d'/' -f1)
REPO=$(echo "$OWNER_REPO" | cut -d'/' -f2)

# Or from gh CLI
gh repo view --json owner,name --jq '"\(.owner.login)/\(.name)"'

Getting Commit SHA for Comments

# Get HEAD commit of the PR
COMMIT_SHA=$(gh pr view $PR_NUMBER --json headRefOid --jq '.headRefOid')

# Or get the latest commit
COMMIT_SHA=$(gh pr view $PR_NUMBER --json commits --jq '.commits[-1].oid')

PR Insight Generation

After completing the PR review analysis, generate insights from the review findings and post them to Discussions.

When to Run

Run this module AFTER the main review is complete and findings have been documented. Only generate insights for findings with severity "high" or "medium".

Process

1. Collect review findings from the current PR analysis 2. For each high/medium finding, create a Finding object:

cd /home/alext/claude-night-market
python3 -c "
import sys, json
sys.path.insert(0, 'plugins/abstract/scripts')
from insight_types import Finding
from post_insights_to_discussions import post_findings

findings = [
    Finding(
        type='PR Finding',
        severity='$SEVERITY',
        skill='',
        summary='PR #$PR_NUMBER: $FINDING_SUMMARY',
        evidence='$EVIDENCE',
        recommendation='$RECOMMENDATION',
        source='pr-review',
        related_files=$CHANGED_FILES,
    )
]
urls = post_findings(findings)
for url in urls:
    print(f'Posted: {url}')
"

3. The posting script handles all dedup automatically 4. Report posted URLs in the review summary

Finding Types

Map review categories to insight types:

Knowledge Capture Module

Capture significant PR review findings into the project's review chamber.

Integration Point

This module executes after Phase 6 (Generate Report) and before posting to GitHub.

Phase 6: Generate Report
    ↓
[KNOWLEDGE CAPTURE MODULE]
    ↓
Phase 7: Post to GitHub

Trigger Conditions

Evaluate knowledge capture when:

1. Review contains BLOCKING findings with architectural context 2. Review contains recurring patterns (seen in 2+ PRs) 3. Review establishes new conventions or standards 4. Review documents a significant decision with rationale

Capture Workflow

Step 1: Extract Capture Candidates

From the review findings, identify candidates for knowledge capture:

def extract_candidates(findings, pr_info):
    """Identify findings worth capturing."""
    candidates = []

    for finding in findings:
        # Score using evaluation criteria
        score = evaluate_finding(finding, pr_info)

        if score >= 60:
            candidates.append({
                "finding": finding,
                "score": score,
                "room_type": classify_room_type(finding),
            })

    return candidates

Step 2: Classify Room Type

Route each candidate to the appropriate review-chamber room:

Step 3: Create Review Entries

For each approved candidate:

---
source_pr: "#42 - Add authentication"
date: 2025-01-15
participants: [author, reviewers...]
palace_location: review-chamber/decisions
tags: [authentication, jwt, security]
---

## Decision Title

### Decision
[What was decided]

### Context
[Discussion that led to decision]

### Captured Knowledge
- Pattern: [reusable pattern]
- Tradeoff: [key tradeoffs]
- Application: [where to apply]

### Connected
- [[related-room]] - connection type

Step 4: User Confirmation

Before capturing, present candidates to user:

## 📚 Knowledge Capture

Found **3** findings worth capturing to review-chamber:

| # | Title | Score | Room | Action |
|---|-------|-------|------|--------|
| 1 | JWT over sessions | 95 | decisions | Capture |
| 2 | API error format | 77 | standards | Capture |
| 3 | Missing null check | 25 | - | Skip |

**Options:**
- [Y] Capture all (2 findings)
- [S] Select which to capture
- [N] Skip knowledge capture
- [E] Edit before capture

Step 5: Store in Project Palace

from memory_palace.project_palace import (
    ProjectPalaceManager,
    ReviewEntry,
    capture_pr_review_knowledge,
)

def store_findings(findings, pr_info):
    """Store findings in project palace."""

    # Get or create project palace
    manager = ProjectPalaceManager()
    palace = manager.get_or_create_project_palace(
        repo_name=pr_info.repo,
        repo_url=pr_info.repo_url,
    )

    # Create entries for each finding
    created = []
    for finding in findings:
        entry = ReviewEntry(
            source_pr=f"#{pr_info.number} - {pr_info.title}",
            title=finding.title,
            room_type=finding.room_type,
            content={
                "decision": finding.description,
                "context": finding.context,
                "captured_knowledge": {
                    "severity": finding.severity,
                    "category": finding.category,
                    "file": finding.file,
                    "line": finding.line,
                },
                "connected_concepts": finding.related,
            },
            participants=pr_info.participants,
            tags=finding.tags,
        )

        if manager.add_review_entry(palace["id"], entry):
            created.append(entry.id)

    return created

Record the Tradeoff (decision journal)

When this step settles a decision with real alternatives, record it to docs/tradeoffs.md while the reasoning is live (draft and confirm):

• If leyline is installed, invoke Skill(leyline:decision-journal) and append

a tradeoff entry (the decision, the options weighed, and what was sacrificed; set phase to review). Show the draft; append on confirmation.

• Fallback (leyline absent): append to docs/tradeoffs.md using the in-file

ENTRY TEMPLATE; assign the next TR-NNN id.

Output Integration

Add knowledge capture summary to the review report:

## PR #42: Add Authentication

### Scope Compliance
...

### Blocking Issues
...

### In-Scope Issues
...

### Knowledge Captured 📚

The following findings were stored in the project's review chamber:

| Entry ID | Title | Room |
|----------|-------|------|
| abc123 | JWT over sessions | decisions/ |
| def456 | Token refresh pattern | patterns/ |

View in palace: `python scripts/palace_manager.py list-reviews --palace project-id`

CLI Integration

Automatic (Default)

When running /pr-review, knowledge capture triggers automatically for high-scoring findings:

/pr-review 42
# ... review output ...
# → Knowledge Capture: Captured 2 findings to review-chamber

Manual Override

# Skip knowledge capture
/pr-review 42 --no-capture

# Force capture all findings
/pr-review 42 --capture-all

# Review and select interactively
/pr-review 42 --capture-interactive

Retroactive Capture

Capture knowledge from a past review:

/review-room capture 42
# Fetches PR #42 review comments and extracts knowledge

Configuration

In memory-palace/config/settings.json:

{
  "review_chamber": {
    "auto_capture": true,
    "capture_threshold": 60,
    "require_confirmation": true,
    "default_rooms": ["decisions", "patterns", "standards", "lessons"],
    "excluded_categories": ["typo", "formatting", "style"]
  }
}

Evaluation Criteria

Uses memory-palace:review-chamber evaluation framework:

Threshold: Score ≥ 60 triggers capture consideration.

Dependencies

• memory-palace:project-palace - Project palace management
• memory-palace:review-chamber - Room structure and evaluation
• memory-palace:knowledge-intake - Evaluation framework

PR Hygiene: Four Principles

Research-backed practices for PR quality that apply to every review. These checks run during scope establishment (Phase 1) and code quality analysis (Phase 2.5).

See Also: Main Skill |

Review Framework

Principle 1: Self-Review Before Sending

Open your own PR in the diff view and read it as if you are a reviewer seeing it for the first time. This catches scope creep, formatting commits, and unclear changes before anyone else spends time on them.

Detection (during /pr-review)

When reviewing a PR, check for signs the author skipped self-review:

# Check for formatting-only commits mixed with feature work
gh pr view $PR_NUMBER --json commits \
  --jq '.commits[].messageHeadline' | \
  grep -iE '(fmt|format|lint|style|whitespace|cleanup)' \
  && echo "WARNING: Formatting commits mixed with feature work"

# Check for fixup/amend commits that should have been squashed
gh pr view $PR_NUMBER --json commits \
  --jq '.commits[].messageHeadline' | \
  grep -iE '(fixup|fix typo|oops|wip|forgot|actually)' \
  && echo "WARNING: Unsquashed fixup commits suggest no self-review"

Classification

Guidance for /pr-prep (Self-Review Checklist)

Before sending the PR, the author should verify:

• [ ] Read the diff as a reviewer would
• [ ] No debug statements left in
• [ ] No commented-out code
• [ ] No formatting-only commits mixed with logic changes
• [ ] No fixup commits that should be squashed
• [ ] Changes are limited to what the PR description promises

Principle 2: One PR = One Logical Change

The Single Responsibility Principle for PRs. Small, focused PRs get reviewed faster, get reviewed more thoroughly (75%+ defect detection vs 30% for large PRs), and are easier to revert if something goes wrong.

Detection (during Phase 1: Scope Establishment)

Analyze commit messages and changed files for mixed concerns:

# Count distinct conventional commit types
COMMIT_TYPES=$(gh pr view $PR_NUMBER --json commits \
  --jq '.commits[].messageHeadline' | \
  grep -oE '^(feat|fix|refactor|docs|test|chore|style|perf)' | \
  sort -u | wc -l)

if [[ "$COMMIT_TYPES" -gt 2 ]]; then
  echo "WARNING: $COMMIT_TYPES distinct commit types - possible mixed concerns"
fi

# Check for unrelated directory changes
CHANGED_DIRS=$(gh pr diff $PR_NUMBER --name-only | \
  awk -F/ '{print $1"/"$2}' | sort -u | wc -l)

# Large PRs with many unrelated directories
CHANGED_FILES=$(gh pr view $PR_NUMBER --json changedFiles \
  --jq '.changedFiles')

if [[ "$CHANGED_FILES" -gt 30 ]]; then
  echo "WARNING: $CHANGED_FILES files changed - consider splitting"
fi

Atomicity Signals

Classification

• BLOCKING: Never. Splitting a PR is the author's

judgment call, not a gate.

• IN-SCOPE: When mixed concerns are obvious and the

split would be straightforward (e.g., a cargo fmt commit bundled with a feature).

• SUGGESTION: When the PR is large but logically

coherent, note the size for awareness.

Recommendation Template

When atomicity concerns are found:

**[G-ATOMICITY] Consider splitting this PR**

This PR contains N distinct concerns:
1. [concern A] (files: ...)
2. [concern B] (files: ...)

Smaller PRs get reviewed more thoroughly (75%+ defect
detection rate vs 30% for large PRs) and are easier to
revert. Consider splitting into:
- PR 1: [concern A]
- PR 2: [concern B]

Author's discretion - this is a suggestion, not a blocker.

Principle 3: Agent-Generated Code Needs Human Curation

AI coding tools produce code quickly, but the output needs careful review for: redundant code, unnecessary complexity, incomplete refactors, and scope drift. Formatting commits and mixed-concern refactors are telltale signs of iterative AI generation without a final cleanup pass.

Detection (during Phase 2.5: Code Quality)

Look for patterns characteristic of AI-generated code that was not curated by a human before submission.

Tier 1: Structural checks (always run)

# 1. Wrapper functions (function body is a single call)
# Agent pattern: create_user() just calls _do_create_user()
gh pr diff $PR_NUMBER | \
  awk '/^\+.*def |^\+.*fn |^\+.*function /{name=$0; getline; \
  if(/^\+\s*(return |self\.)/ && !/^\+\s*$/) print name " -> WRAPPER?"}' \
  2>/dev/null || true

# 2. Redundant implementations (same logic, different names)
gh pr diff $PR_NUMBER | \
  grep -E '^\+.*(def |fn |function |func )' | \
  awk '{print $NF}' | sort | uniq -d

# 3. Over-abstraction signals
# New interfaces/traits/protocols with single implementations
NEW_ABSTRACTIONS=$(gh pr diff $PR_NUMBER | \
  grep -cE '^\+.*(trait |interface |protocol |abstract class )' || true)
if [[ "$NEW_ABSTRACTIONS" -gt 0 ]]; then
  echo "CHECK: $NEW_ABSTRACTIONS new abstractions - verify each has 2+ implementations"
fi

# 4. Incomplete refactors
# Old function still called after new replacement added
NEW_FUNCS=$(gh pr diff $PR_NUMBER | \
  grep -E '^\+.*(def |fn |function )' | \
  sed 's/.*\(def\|fn\|function\) \+\([a-zA-Z_]*\).*/\2/' | head -10)
for func in $NEW_FUNCS; do
  OLD_VARIANT=$(echo "$func" | sed 's/new_//;s/_v2$//;s/_updated$//')
  if [[ "$OLD_VARIANT" != "$func" ]]; then
    STILL_CALLED=$(gh pr diff $PR_NUMBER | grep -c "$OLD_VARIANT" || true)
    if [[ "$STILL_CALLED" -gt 0 ]]; then
      echo "INCOMPLETE REFACTOR? $func replaces $OLD_VARIANT but old version still referenced"
    fi
  fi
done

Tier 2: Diff-ratio checks (run for PRs > 10 files)

# 5. Addition-heavy ratio (agents add more than they remove)
STATS=$(gh pr view $PR_NUMBER --json additions,deletions \
  --jq '"\(.additions) \(.deletions)"')
ADDITIONS=$(echo $STATS | cut -d' ' -f1)
DELETIONS=$(echo $STATS | cut -d' ' -f2)

if [[ "$DELETIONS" -gt 0 ]]; then
  RATIO=$((ADDITIONS / DELETIONS))
  if [[ "$RATIO" -gt 5 ]]; then
    echo "CHECK: Add/delete ratio $RATIO:1 - agents tend to add without removing"
  fi
fi

# 6. Import bloat (new imports that may be unused)
ADDED_IMPORTS=$(gh pr diff $PR_NUMBER | \
  grep -cE '^\+.*(^import |^from .* import |^use |require\()' || true)
if [[ "$ADDED_IMPORTS" -gt 10 ]]; then
  echo "CHECK: $ADDED_IMPORTS new imports - verify none are unused"
fi

# 7. Scope drift via directory spread
CHANGED_DIRS=$(gh pr diff $PR_NUMBER --name-only | \
  awk -F/ 'NF>1{print $1"/"$2}' | sort -u)
DIR_COUNT=$(echo "$CHANGED_DIRS" | wc -l)
if [[ "$DIR_COUNT" -gt 5 ]]; then
  echo "CHECK: Changes span $DIR_COUNT directories:"
  echo "$CHANGED_DIRS"
fi

Tier 3: Content-level checks (reviewer judgment)

These cannot be fully automated. The reviewer should manually check for:

• Boilerplate inflation: Does the PR add config

files, CI changes, or documentation that is not required by the stated goal?

• Defensive over-engineering: Are there error

handlers for conditions that cannot happen? Try/catch blocks wrapping infallible operations?

• Naming inconsistency: Do new functions follow

existing naming conventions, or do they introduce a different style (camelCase vs snake_case, different prefix patterns)?

• Comment density spike: Agent-generated code

often has more comments per line than human code. If the PR's comment density is notably higher than the surrounding code, flag it.

Agent Code Curation Signals

Recommendation Template

When agent curation issues are found:

**[G-CURATION] Agent-generated code needs cleanup pass**

This PR shows signs of iterative AI generation without
a final curation pass:

- [specific finding 1]
- [specific finding 2]

Recommendation: Review the PR with fresh eyes, asking
"does every change here serve the stated goal?" Remove
redundant code, collapse unnecessary abstractions, and
split unrelated changes into separate PRs.

Integration with Anti-Slop (Phase 1.7)

Agent curation overlaps with slop detection but targets structural issues rather than prose issues:

• Slop detection (Phase 1.7): AI markers in prose,

documentation, and commit messages

• Agent curation (Phase 2.5): AI patterns in code

structure, architecture, and implementation choices

Both should run. Slop detection catches the writing; agent curation catches the engineering.

Principle 4: Tests Should Test Your Code

Tests should break if someone reverts your fix, not demonstrate why the fix was needed. Assertion blocks showing unrelated functionality are documentation, not regression protection.

Detection (during Phase 2.5 and Test Plan)

Analyze test files changed in the PR:

# Get test files in the PR
TEST_FILES=$(gh pr diff $PR_NUMBER --name-only | \
  grep -E '(test_|_test\.|\.test\.|\.spec\.)')

# Check for tests that only assert existing behavior
# without connecting to the changed code
for file in $TEST_FILES; do
  # Look for assertions about code NOT changed in this PR
  gh pr diff $PR_NUMBER -- "$file" | \
    grep -E '^\+.*assert' | head -10
done

Test Quality Signals

The Revert Test

The gold standard for test quality: if someone reverts the fix, at least one test should fail. If no test fails on revert, the tests are documentation, not protection.

# Mental model for each test:
# 1. Does this test touch code changed in this PR?
# 2. Would reverting the PR changes cause this test to fail?
# 3. If not, what regression does this test actually prevent?

Classification

• BLOCKING: Tests that pass even when the fix is

reverted (they test nothing about the new code)

• IN-SCOPE: Tests that only assert old behavior

without covering the new code path

• SUGGESTION: Tests that work but could be more

targeted or better named

Recommendation Template

When test quality issues are found:

**[S-TESTS] Tests should protect against regressions**

The following tests don't break if someone reverts this
PR's changes:

- `test_existing_behavior` in `test_module.py`
  Asserts pre-existing behavior unrelated to the fix.

Write tests that:
1. Would FAIL if the fix is reverted
2. Cover the specific code path changed in this PR
3. Protect against the exact regression being fixed

Tests should answer: "what breaks if someone undoes my
change?" not "what was wrong before my change?"

Integration Checklist

When this module is loaded, the following checks are added to the review workflow:

Phase 1 (Scope Establishment)

• [ ] Check PR atomicity (Principle 2)
• [ ] Flag mixed commit types
• [ ] Note PR size for awareness

Phase 2.5 (Code Quality)

• [ ] Scan for agent curation signals (Principle 3)
• [ ] Check for redundant implementations
• [ ] Flag premature abstractions
• [ ] Identify incomplete refactors

Phase 2.5 (Test Quality)

• [ ] Apply the revert test mentally (Principle 4)
• [ ] Verify tests touch changed code
• [ ] Flag demonstration-only assertions

Report Generation (Phase 6)

• [ ] Include self-review checklist if signals found (Principle 1)
• [ ] Include atomicity recommendation if warranted (Principle 2)
• [ ] Include curation findings (Principle 3)
• [ ] Include test quality findings (Principle 4)

Version Validation Module

Purpose: Enforce version consistency checks in PR reviews to catch version mismatches before merge.

When to Run

MANDATORY for every PR review UNLESS:

• Maintainer explicitly passes --skip-version-check flag
• PR is labeled with skip-version-check in GitHub
• PR description contains [skip-version-check] marker

Validation Checklist

1. Detect Project Type & Version Files

# Determine project structure
PROJECT_TYPE=""
VERSION_FILES=()

if [[ -f "Cargo.toml" ]]; then
  PROJECT_TYPE="rust"
  VERSION_FILES+=("Cargo.toml")
elif [[ -f "package.json" ]]; then
  PROJECT_TYPE="node"
  VERSION_FILES+=("package.json")
elif [[ -f "pyproject.toml" ]]; then
  PROJECT_TYPE="python"
  VERSION_FILES+=("pyproject.toml")
elif [[ -f ".claude-plugin/marketplace.json" ]]; then
  PROJECT_TYPE="claude-marketplace"
  VERSION_FILES+=(".claude-plugin/marketplace.json")
fi

# Always check CHANGELOG if it exists
[[ -f "CHANGELOG.md" ]] && VERSION_FILES+=("CHANGELOG.md")
[[ -f "CHANGELOG" ]] && VERSION_FILES+=("CHANGELOG")

2. Check Branch Name for Version Indicator

# Extract version from branch name if present
BRANCH_NAME=$(gh pr view $PR_NUMBER --json headRefName -q .headRefName)
BRANCH_VERSION=""

# Match patterns: release/1.2.3, version-1.2.3, feature-name-1.2.3, v1.2.3-branch
if echo "$BRANCH_NAME" | grep -qE '[0-9]+\.[0-9]+\.[0-9]+'; then
  BRANCH_VERSION=$(echo "$BRANCH_NAME" | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | head -1)
  echo "Branch name indicates version: $BRANCH_VERSION"
fi

3. Check if Version Changed in PR

# Get PR diff for version files
VERSION_CHANGED=false
for file in "${VERSION_FILES[@]}"; do
  if gh pr diff $PR_NUMBER --name-only | grep -qF "$file"; then
    if gh pr diff $PR_NUMBER -- "$file" | grep -qE '^\+.*version|^\+.*## \['; then
      VERSION_CHANGED=true
      break
    fi
  fi
done

4. If Version Changed, Run Full Validation

If VERSION_CHANGED=true, perform detailed checks:

A. Extract Version from Each Source

# Example for claude-marketplace
MARKETPLACE_VERSION=$(jq -r '.metadata.version' .claude-plugin/marketplace.json)

# For each plugin in marketplace
jq -r '.plugins[] | "\(.name):\(.version)"' .claude-plugin/marketplace.json > /tmp/marketplace_versions.txt

# For each actual plugin
for plugin_dir in plugins/*/; do
  PLUGIN_NAME=$(basename "$plugin_dir")
  ACTUAL_VERSION=$(jq -r '.version' "$plugin_dir/.claude-plugin/plugin.json" 2>/dev/null || echo "MISSING")
  echo "$PLUGIN_NAME:$ACTUAL_VERSION" >> /tmp/actual_versions.txt
done

B. Compare Marketplace vs Actual

# Cross-reference versions
while IFS=: read -r name marketplace_version; do
  actual_version=$(grep "^$name:" /tmp/actual_versions.txt | cut -d: -f2)

  if [[ "$marketplace_version" != "$actual_version" ]]; then
    # BLOCKING ISSUE FOUND
    echo "[B-VERSION] Version mismatch for $name: marketplace=$marketplace_version, actual=$actual_version"
  fi
done < /tmp/marketplace_versions.txt

C. Verify CHANGELOG Updated

# Check if CHANGELOG has entry for new version
if [[ -f "CHANGELOG.md" ]]; then
  NEW_VERSION=$(jq -r '.metadata.version' .claude-plugin/marketplace.json)

  if ! grep -q "\[$NEW_VERSION\]" CHANGELOG.md; then
    # BLOCKING ISSUE FOUND
    echo "[B-VERSION] CHANGELOG.md missing entry for version $NEW_VERSION"
  fi

  # Check for release date
  if grep -q "\[$NEW_VERSION\] - Unreleased" CHANGELOG.md; then
    # SUGGESTION
    echo "[G-VERSION] CHANGELOG shows version $NEW_VERSION as Unreleased - update date before merge"
  fi
fi

D. Validate Branch Name Version Matches Marketplace Version

# If branch name contains a version, it MUST match the marketplace/project version
if [[ -n "$BRANCH_VERSION" ]]; then
  # Get current project version based on project type
  CURRENT_VERSION=""

  if [[ "$PROJECT_TYPE" == "claude-marketplace" ]]; then
    CURRENT_VERSION=$(jq -r '.metadata.version' .claude-plugin/marketplace.json)
  elif [[ "$PROJECT_TYPE" == "python" ]]; then
    CURRENT_VERSION=$(grep "^version" pyproject.toml | grep -oE '[0-9]+\.[0-9]+\.[0-9]+')
  elif [[ "$PROJECT_TYPE" == "node" ]]; then
    CURRENT_VERSION=$(jq -r '.version' package.json)
  elif [[ "$PROJECT_TYPE" == "rust" ]]; then
    CURRENT_VERSION=$(grep "^version" Cargo.toml | head -1 | grep -oE '[0-9]+\.[0-9]+\.[0-9]+')
  fi

  if [[ -n "$CURRENT_VERSION" ]] && [[ "$BRANCH_VERSION" != "$CURRENT_VERSION" ]]; then
    # BLOCKING ISSUE FOUND
    echo "[B-VERSION] Branch name suggests version $BRANCH_VERSION, but marketplace/project version is $CURRENT_VERSION"
    echo "  Branch: $BRANCH_NAME"
    echo "  Expected: Version files should match branch name version"
    echo "  Fix: Update version files to $BRANCH_VERSION OR rename branch to match $CURRENT_VERSION"
  fi
fi

E. Check README Version References

# Check if README mentions version
if [[ -f "README.md" ]]; then
  if grep -q "version" README.md; then
    # Extract version mentions
    grep -i "version" README.md | while read -r line; do
      # Check if it references old version
      if echo "$line" | grep -qE "[0-9]+\.[0-9]+\.[0-9]+"; then
        echo "[INFO] README mentions version - verify accuracy"
      fi
    done
  fi
fi

5. Project-Specific Validations

Claude Plugin Marketplace

# Additional checks for claude-night-market structure
if [[ "$PROJECT_TYPE" == "claude-marketplace" ]]; then

  # Check metadata.version matches all plugin versions (unless independent cycle)
  ECOSYSTEM_VERSION=$(jq -r '.metadata.version' .claude-plugin/marketplace.json)

  # Get independent release plugins from CHANGELOG or docs
  INDEPENDENT_PLUGINS=()
  if grep -q "independent release cycle" CHANGELOG.md; then
    # Extract plugin names marked as independent
    INDEPENDENT_PLUGINS+=($(grep -A2 "independent release cycle" CHANGELOG.md | grep -oE '[a-z-]+' | head -5))
  fi

  # Verify non-independent plugins match ecosystem version
  jq -r '.plugins[] | "\(.name):\(.version)"' .claude-plugin/marketplace.json | while IFS=: read -r name version; do
    # Skip if independent
    if [[ " ${INDEPENDENT_PLUGINS[@]} " =~ " ${name} " ]]; then
      continue
    fi

    if [[ "$version" != "$ECOSYSTEM_VERSION" ]]; then
      echo "[B-VERSION] Plugin $name should be $ECOSYSTEM_VERSION (ecosystem version), but is $version"
    fi
  done
fi

Python Projects

if [[ "$PROJECT_TYPE" == "python" ]]; then
  # Check __version__ in source
  if [[ -d "src" ]]; then
    VERSION_PY=$(find src -name "__init__.py" -exec grep -l "__version__" {} \; | head -1)
    if [[ -n "$VERSION_PY" ]]; then
      CODE_VERSION=$(grep "__version__" "$VERSION_PY" | grep -oE '[0-9]+\.[0-9]+\.[0-9]+')
      TOML_VERSION=$(grep "^version" pyproject.toml | grep -oE '[0-9]+\.[0-9]+\.[0-9]+')

      if [[ "$CODE_VERSION" != "$TOML_VERSION" ]]; then
        echo "[B-VERSION] __version__ ($CODE_VERSION) doesn't match pyproject.toml ($TOML_VERSION)"
      fi
    fi
  fi
fi

Classification of Version Issues

All version mismatches are BLOCKING unless explicitly waived:

Bypass Mechanism

Maintainer can bypass with one of:

1. CLI flag: /pr-review <pr> --skip-version-check 2. GitHub label: Add skip-version-check label to PR 3. PR description marker: Include [skip-version-check] in PR body

When bypassed:

• Still run validation and report findings
• Mark as [WAIVED] instead of [BLOCKING]
• Add note: "Version validation bypassed by maintainer"

Output Format

### Version Validation

**Status:** ✅ PASSED | ⚠️ WAIVED | ❌ FAILED

**Version Detected:** 1.2.3 → 1.2.4

**Files Checked:**
- [x] .claude-plugin/marketplace.json: 1.2.4 ✓
- [x] plugins/*/plugin.json: 1.2.4 ✓ (11 plugins)
- [x] plugins/memory-palace/plugin.json: 1.3.0 ✓ (independent cycle)
- [x] CHANGELOG.md: Entry for 1.2.4 ✓
- [x] README.md: References updated ✓

**Blocking Issues (0):**
None - all version files consistent.

---

OR with issues:

### Version Validation

**Status:** ❌ FAILED

**Branch Name:** skills-improvements-1.2.2
**Version Detected:** 1.1.0 → 1.2.1

**Files Checked:**
- [ ] Branch name version: 1.2.2 ≠ Marketplace version: 1.2.1 ❌
- [x] .claude-plugin/marketplace.json: 1.2.1 ✓
- [x] plugins/abstract/plugin.json: 1.2.1 ✓
- [ ] plugins/memory-palace/plugin.json: Marketplace lists 1.2.1 but actual is 1.2.0 ❌
- [x] CHANGELOG.md: Entry for 1.2.1 ✓
- [ ] README.md: Still references 1.1.0 ⚠️

**Blocking Issues (2):**
- [B-VERSION-1] Branch name suggests version 1.2.2, but marketplace/project version is 1.2.1
  - Branch: skills-improvements-1.2.2
  - Expected: Version files should match branch name version
  - Fix: Update version files to 1.2.2 OR rename branch to match 1.2.1
- [B-VERSION-2] Version mismatch: memory-palace
  - Marketplace: 1.2.1
  - Actual: 1.2.0
  - Fix: Update marketplace.json line 52 to "1.2.0"

**In-Scope Issues (1):**
- [S-VERSION-1] README references old version 1.1.0
  - Fix: Update README.md to reference 1.2.1

Integration with PR Review Workflow

This module runs in Phase 1.5 (after scope establishment, before code analysis):

Phase 1: Scope Establishment
Phase 1.5: Version Validation ← NEW
Phase 2: Code Analysis
Phase 3: Synthesis & Validation
Phase 4: GitHub Review Submission
Phase 5: Test Plan Generation

Why Phase 1.5?

• Version issues are blocking and should be caught early
• Prevents wasting time on detailed code review if basic version hygiene fails
• Provides fast feedback to PR author

Error Handling

Missing Version Files

⚠️ Version validation skipped: No version files detected in repository.
Consider adding CHANGELOG.md for release tracking.

Multiple Version Schemes

ℹ️ Detected multiple version schemes:
- Python package: 2.1.0 (pyproject.toml)
- Frontend: 1.5.0 (package.json)

Validated each independently.

Parse Failures

if ! jq -e '.metadata.version' .claude-plugin/marketplace.json >/dev/null 2>&1; then
  echo "[B-VERSION] Failed to parse version from marketplace.json - invalid JSON?"
fi

Testing the Module

# Test version validation manually
Skill(sanctum:pr-review)

# With bypass
/pr-review 42 --skip-version-check

# Dry run to see what would be checked
/pr-review 42 --dry-run

Maintenance Notes

• Update detection patterns when new project types are added
• Keep version file list synchronized with sanctum:version-updates skill
• Consider adding support for monorepo version strategies
• May need adjustment for projects using date-based or git-hash versions