Nm Sanctum Pr Review

> Night Market Skill — ported from claude-night-market/sanctum. For the full experience with agents, hooks, and commands, install the Claude Code plugin.

Table of Contents

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

When NOT To Use

Scope Classification Framework

Every finding must be classified:

| Category | Definition | Action | |----------|------------|--------| | BLOCKING | Bug, security issue, or regression introduced by this change | Must fix before merge | | IN-SCOPE | Issue directly related to stated requirements | Should address in this PR | | SUGGESTION | Improvement within changed code, not required | Author decides | | BACKLOG | Good idea but outside PR scope | Create GitHub issue | | IGNORE | Nitpick, style preference, or not worth tracking | Skip entirely |

Classification Examples

BLOCKING:

IN-SCOPE:

SUGGESTION:

BACKLOG:

IGNORE:

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
   

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
   

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
   

4. PR/MR description: Author's intent

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

5. Commit messages: Incremental decisions

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

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  --name-only
gh pr diff 
gh pr view  --json additions,deletions,changedFiles,commits
GitLab
glab mr diff 
glab mr view 

Phase 3: Requirements Validation

Before detailed code review, check scope coverage:

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 + 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

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

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
   - 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
   - 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
   - Issue: Empty input not handled per requirement
   - Why: Defensive validation at API boundaries
     prevents cascading failures in downstream logic.
   - Proof: Postel's Law
   - 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--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

Candidates for capture:

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 

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

Quality Gates

A PR should be approved when:

Anti-Patterns to Avoid

Don't: Scope Creep Review

Do: Create backlog issues, keep PR focused.

Don't: Perfect is Enemy of Good

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

Don't: Blocking on Style

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

Don't: Reviewing Unchanged Code

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

Don't: Tests That Prove Old Code Was Bad

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

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

Integration with Other Tools

Slop Detection Integration

Documentation Review

Commit Message Review

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

PR Description Review

Exit Criteria

Supporting Modules

Troubleshooting

Common Issues

Command not found Ensure all dependencies are installed and in PATH

Permission errors Check file permissions and run with appropriate privileges

Unexpected behavior Enable verbose logging with --verbose flag

Common Issues

Command not found Ensure all dependencies are installed and in PATH

Permission errors Check file permissions and run with appropriate privileges

Unexpected behavior Enable verbose logging with --verbose flag