dev-workflow-forge

{
  "name": "dev-workflow-forge",
  "description": "A generator kit for Claude Code dev tooling. Three skills read your codebase and write a project-specific review skill, a rules set, and a self-learning feedback loop tuned to your stack, instead of handing you tooling written for someone else's project.",
  "version": "1.0.0",
  "author": {
    "name": "Ivan Misic"
  }
}

# dev-workflow-forge

A generator kit, not a fixed toolset. Three skills read your codebase and write project-specific dev tooling tuned to your stack.

Share the generator, not the generated output. A review skill someone else wrote for their PHP MVC app has no idea your project is a Rust CLI. The generators in this bundle read the actual codebase in front of them and produce a review skill, a rules set, and a self-learning loop that fit it, then get out of the way.

## The three generators

| Skill | Produces |
|-|-|
| `a-review-optimizer` | A project-specific `*-review` skill: parallel agents with non-overlapping scopes, a deterministic preflight script, fix-readiness output, `--debt`/`--full` modes where warranted |
| `a-rules-optimizer` | `.claude/rules/*.md` and a `CLAUDE.md` reference table, scoped to your directory layout and cross-checked against the review skill's checks |
| `a-self-learner` | Mines `.claude/reviews/review-issues.jsonl` for recurring findings and proposes rule or preflight updates, always with an approval gate before anything is written |

Each one is surgical: it preserves what already works in an existing skill or rule file and only changes what a concrete gap, false positive, or drift justifies. Run `a-review-optimizer` first on a new project; `a-rules-optimizer` and `a-self-learner` are more useful once a review skill exists to cross-reference.

## Use

```
/a-review-optimizer     # build or improve this project's review skill
/a-rules-optimizer      # audit or create .claude/rules/
/a-self-learner         # propose updates from accumulated review history
```

## The day-to-day loop

The generators produce the tooling; the everyday `fix`, `commit`, and `ship` commands that run on top of it live in the companion **dev-loop** plugin. Install both if you want the full pipeline.

## Install

```
/plugin marketplace add imisic/claude-marketplace
/plugin install dev-workflow-forge@imisic
```

---
name: a-review-optimizer
description: "Analyzes a project's codebase in depth, then surgically improves an existing project-specific review skill (like example-review, site-review, etc.): preserving what already works while filling gaps, fixing false positives, resolving agent scope overlaps, and adding deterministic preflight checks. Use when the user says 'improve my review skill', 'make example-review better', 'optimize this skill', 'this review misses things', 'tighten up my review', 'upgrade review', or passes a review skill SKILL.md and wants it enhanced. Also use when creating a new project-specific review skill from scratch for a codebase."
---

# Review Skill Optimizer

Takes an existing review skill and makes it sharper by deeply analyzing the actual codebase. The approach is surgical: keep everything that works, fix what doesn't, add what's missing. The user built that skill with real experience. Don't throw it away.

**Input:** optional path to a review skill's SKILL.md (e.g., `.claude/skills/example-review/SKILL.md`)

If no path given, auto-detect: scan `.claude/skills/*/SKILL.md` for review-related skills (look for "review" in name, description, or content mentioning agents/preflight/findings). If exactly one found, use it. If multiple found, list them and ask which one. If none found, create a new review skill from scratch.

---

## Core Principle: Preserve First, Improve Surgically

The existing skill reflects real project knowledge: conventions the user discovered, false positives they already solved, agent scopes they tuned through experience. Treat it as the baseline, not a rough draft.

**Default behavior:** Keep every section of the existing skill unless the gap analysis gives a concrete reason to change it. When you do change something, the change must be traceable to a specific gap, overlap, or false positive you found.

**What gets preserved:**
- Agent names and their general scope (unless overlaps are found)
- Known-correct pattern whitelists (these are hard-won project knowledge)
- Output format structure (unless it lacks fix readiness fields)
- Existing preflight checks that still work
- Project-specific context sections and conventions documentation
- Anything the user invested effort into getting right

**What gets changed (with justification):**
- Gaps: missing checks the project needs but the skill doesn't cover
- False positives: patterns the skill flags that are actually correct in this project
- Overlaps: two agents checking the same thing (tighten scopes)
- Stale references: file:line examples pointing to code that no longer exists
- Missing preflight: deterministic checks that should exist but don't
- Missing fix readiness: findings without file:line + current code + proposed fix + why

---

## What This Skill Produces

By the end, you deliver back to the user an improved (or new) review skill package:

1. **Improved SKILL.md**: the existing skill with surgical changes: new checks added, false positive whitelists updated, agent scopes tightened, concrete examples refreshed from current codebase
2. **Preflight script**: new or improved deterministic checks (grep/rg + Python) tailored to patterns actually found in the project
3. **Gap report**: what you found and what you changed, so the user can verify you didn't break what was working
4. **Capture script**: `.claude/scripts/capture-finding.sh` if the skill doesn't already have one, so confirmed findings feed the self-learning loop (see `references/skill-scaffold.md` §6)

A fully-featured generated skill also carries a flag surface, a parallel-dispatch skeleton, a growing "Context Awareness (DO NOT flag)" whitelist, `--debt` scoring, `--full` architecture diagrams, self-learning capture, and rule-candidate surfacing. `references/skill-scaffold.md` documents every one of these sections. When improving, use it as a coverage checklist (missing section → `ADD`); when building from scratch, use it as the blueprint. Emit only the modes the project actually needs.

---

## Phase 1: Ingest the Existing Skill

Read the target SKILL.md completely. Extract, catalog, and **tag each section for preservation**:

- **Agents defined**: names, scopes, what each checks
- **Scope overlaps**: where two agents could flag the same thing
- **Coverage gaps**: categories of issues not assigned to any agent
- **Preflight scripts**: what exists, what checks they run, how results feed into agents
- **Output format**: what findings look like, whether they enforce fix readiness (file:line + current code + proposed fix + why)
- **Known-correct patterns**: whitelists, things agents are told to skip
- **Project-specific rules**: anything hardcoded to this project's conventions

For each section, assign a tag:

| Tag | Meaning | Action |
|-|-|-|
| **KEEP** | Works well, no issues found | Preserve verbatim |
| **UPDATE** | Needs extension (append checks) or correction (stale ref, scope overlap, false positive) | Targeted edit, preserve surrounding context |
| **ADD** | Needed but doesn't exist yet | Add a new section |
| **REMOVE** | Covers a pattern the project no longer uses (rare, prefer UPDATE for most cleanup) | Delete the section |
| **MOVE** | Correct content, wrong agent or scope | Relocate to the right place |

The tag vocabulary is shared with `a-rules-optimizer` so both skills' diff summaries read the same way.

Default tag is **KEEP**. Only change what you have evidence to change.

If no existing review skill was found, skip this phase and proceed to Phase 2 to build one from scratch.

---

## Phase 2: Deep Project Analysis

This is the core of the optimizer. You need to understand the project as well as a senior engineer who's worked on it for 6 months. Don't skim: actually read the code.

### Step 1: Project Profile

The manifest checks below cover Python, JavaScript/TypeScript, PHP, Ruby, Go, Rust, and .NET. For Java add `pom.xml` / `build.gradle`, for Elixir `mix.exs`, for Swift `Package.swift`. **Adapt the stack-specific commands below to whatever language you detect**: the shape is the same (list the manifest, grep dependencies), only the filenames change.

```bash
# Language & framework detection
ls composer.json package.json requirements.txt Pipfile pyproject.toml go.mod Cargo.toml Gemfile *.sln pom.xml build.gradle mix.exs Package.swift 2>/dev/null
cat composer.json 2>/dev/null | grep -A5 '"require"' | head -20
cat package.json 2>/dev/null | grep -A10 '"dependencies"' | head -20
cat requirements.txt pyproject.toml 2>/dev/null | head -30

# Project structure
find . -maxdepth 3 -type d -not -path '*/\.*' -not -path '*/node_modules/*' -not -path '*/vendor/*' -not -path '*/__pycache__/*' -not -path '*/.git/*' | sort

# Entry points & architecture indicators
ls index.php main.py app.py manage.py server.* cmd/ src/ lib/ app/ 2>/dev/null
ls Dockerfile docker-compose.yml Makefile .github/workflows/ 2>/dev/null

# Config patterns
ls .env .env.example config/ *.yaml *.yml 2>/dev/null

# Test structure
find . -path '*/test*' -name '*.py' -o -path '*/test*' -name '*.php' -o -path '*/test*' -name '*.ts' -o -path '*/__tests__/*' -name '*.js' 2>/dev/null | head -20

# Size & complexity overview
find src/ app/ lib/ -name '*.py' -o -name '*.php' -o -name '*.ts' -o -name '*.js' 2>/dev/null | xargs wc -l 2>/dev/null | sort -rn | head -20
```

### Step 2: Convention Discovery

Read the 5-10 largest/most important files and document the conventions the project actually uses. You're looking for:

**Architecture patterns:**
- How is code organized? (MVC, service-repository, layered, modular, flat)
- How do modules communicate? (direct imports, events, message passing, DI container)
- Where does business logic live? (services, models, controllers, standalone functions)

**Error handling patterns:**
- How does the project handle failures? (exceptions, result tuples, error codes, Either types)
- Are there project-specific error classes?
- What's the logging strategy? (logger, print, console, Rich)

**State management:**
- Web: session, state store, context, global
- CLI: config objects, env vars, argument parsing
- Framework-specific: st.session_state (Streamlit), request context (Flask), etc.

**Config access:**
- How are settings loaded? (env, YAML, JSON, PHP config, .ini)
- Is there a centralized config manager or is it scattered?
- How are secrets handled?

**Data access patterns:**
- ORM or raw queries?
- Repository pattern or inline queries?
- How are connections managed?

**Registration patterns:**
- How are routes/views/commands registered?
- Are there maps, decorators, or auto-discovery?
- What needs to be exported/registered when adding new code?

**Return value conventions:**
- Do functions return specific shapes? (e.g., `(bool, str)` tuples, Result objects, response dicts)
- Are there project-specific conventions for success/failure?

### Step 3: Pattern Inventory

Programmatically scan for the patterns that a review skill should know about. Read `references/pattern-detection.md` for the full detection script library. Pick the scans relevant to this project's stack.

The goal: build a concrete list of "things that exist in this codebase" so the improved skill can reference real file:line examples.

```bash
# Example: find all exception handling patterns used in the project
rg -n 'except\s+\w+' --type py src/ | awk -F: '{print $3}' | sort | uniq -c | sort -rn

# Example: find all subprocess usage patterns
rg -n 'subprocess\.' --type py src/

# Example: find all route/view registrations
rg -n 'PAGE_MAP|url_patterns|@app.route|router\.' src/

# Example: find all cache patterns
rg -n 'cache|@st.cache|@lru_cache|Redis' src/
```

Adapt these scans to whatever stack you detected. The output feeds into Phase 3.

### Step 4: Find Real Issues (Sampling)

Actually review 3-5 files from the project yourself, applying the existing skill's checklist. Note:
- Issues the existing skill WOULD catch (working correctly)
- Issues the existing skill WOULD MISS (gaps)
- Things the existing skill WOULD falsely flag (false positives due to project conventions)

This sampling gives you concrete evidence for Phase 3.

---

## Phase 3: Gap Analysis

Compare what the existing skill checks against what the project actually needs. Produce a structured gap report.

> **Output framing.** The reports in 3a-3d are **internal**: Phase 3 is your planning artifact. Do not show these raw blocks to the user. The user-facing deliverable is the Phase 5 **Diff Summary**, which traces each change back to one of these Phase 3 findings.

### 3a: Missing Checks

For each issue category, ask: "Does the existing skill cover this for this specific project?"

Read `references/review-dimensions.md` for the full taxonomy of review dimensions. Cross-reference each dimension against the existing skill's agent prompts.

Output:
```
MISSING CHECKS
==============
[SECURITY] No check for pickle.load(): project uses pickle in src/cache/serializer.py
[ARCHITECTURE] No check for view registration: project uses PAGE_MAP in src/web/views/__init__.py
[QUALITY] No check for Python 3.10+ type modernization: project uses Optional[] in 14 files
[PERFORMANCE] No check for N+1: project uses SQLAlchemy with lazy loading
```

### 3b: False Positive Sources

Identify patterns the existing skill flags (or would flag) that are actually correct in this project.

Output:
```
FALSE POSITIVES TO WHITELIST
============================
[ARCHITECTURE] BackupEngine returns (bool, str) tuples: this is the project convention, not a code smell
[QUALITY] Click command functions appear unused: they're invoked by the Click framework via decorators
[QUALITY] Streamlit render functions appear uncalled: they're dispatched via PAGE_MAP string lookup
[SECURITY] .example config files are committed: this is intentional, real configs are gitignored
```

### 3c: Scope Overlaps

Check every pair of agents for items that both could flag:

Output:
```
SCOPE OVERLAPS
==============
[Security × Architecture] Both check exception handling: Security checks "secrets in error messages", Architecture checks "bare except". These OVERLAP on: except blocks that log sensitive data. Fix: Security owns "what's in the error message", Architecture owns "how the exception is structured"
[Architecture × Quality] Both could flag "missing registration": Architecture checks view registration, Quality checks dead code. Fix: Architecture owns registration, Quality skips functions that appear in PAGE_MAP/route decorators
```

### 3d: Preflight Gaps

What deterministic checks should exist but don't? Compare the project's patterns against `references/pattern-detection.md`:

Output:
```
MISSING PREFLIGHT CHECKS
=========================
[NEED] subprocess calls without timeout: project has 8 subprocess calls, none checked by preflight
[NEED] st.rerun() without preceding invalidate(): project pattern requires cache invalidation before rerun
[NEED] View files not registered in PAGE_MAP: project uses explicit registration
[HAVE] Bare except check: already covered, working
[SKIP] innerHTML check: no HTML rendering in this project
```

---

## Phase 4: Generate Improved Skill

Apply targeted improvements based on the gap analysis. Follow the preservation tags from Phase 1. Default is KEEP: only touch what has a concrete reason to change.

### 4a: Enhance Agent Prompts

For each agent, check its preservation tag. If it's KEEP, don't touch it. If UPDATE, apply these additions/edits while keeping the existing prompt structure intact:

**Add if missing: YOUR SCOPE**: explicit, non-overlapping list. Use the overlap resolution from Phase 3c.

**KNOWN CORRECT PATTERNS (DO NOT FLAG)**: from Phase 3b. Include actual file:line references:
```
These patterns are correct in this project. Do not flag them:
- BackupEngine methods return (bool, str) tuples: see src/core/backup_engine.py:45
- @st.cache_resource used for heavy component init: see src/web/state.py:12
- Storage paths dict uses keys 'local' and 'sync': see src/core/config_manager.py:88
```

**CONCRETE EXAMPLES TO FIND**: from Phase 2 Step 4. Show agents what real issues look like in THIS codebase:
```
Look for patterns like these (found during analysis):
- src/cli.py:622: subprocess.run() without timeout (similar calls may exist elsewhere)
- src/web/views/text_sanitizer.py:45: st.rerun() without invalidate() in preceding lines
```

**PREFLIGHT KNOWN ISSUES**: placeholder for runtime injection from the preflight script:
```
The preflight scan found these issues in your scope. Verify each one:
${PREFLIGHT_SECURITY_FINDINGS}
For each: confirm it's real, add context about why it matters here, and look for similar patterns nearby.
```

**OUTPUT CONSTRAINT**: every finding must include all 4 fields:
```
Every finding MUST include:
1. File:Line: exact location
2. Current code: the line(s) as they exist
3. Proposed fix: what it should look like
4. Why: one sentence, specific to this project (not generic)
Findings missing any field are incomplete. Do not include them.
```

### 4b: Generate Preflight Script

Based on the preflight gaps from Phase 3d, write a bash script (with embedded Python for multi-line detection) that:

1. Runs checks specific to this project's stack and patterns
2. Outputs structured JSON with check IDs, status, file:line locations, and messages
3. Routes results to the correct agent by check ID prefix

Follow the pattern in `references/preflight-template.md` for the script structure.

Every check must:
- Have a unique ID (e.g., SEC-01, SUB-01, WEB-01)
- Map to exactly one agent
- Output file:line locations (not just counts)
- Handle the "not found" case gracefully (don't error on clean code)

For multi-line pattern detection (the #1 source of false negatives in grep-based scripts), use inline Python:
- Read full expressions from `(` to matching `)`
- Scan backward/forward N lines for required companion patterns
- Parse indent-based blocks for Python

### 4c: Add Reconciliation Rules

Add a post-review section that ensures nothing falls through cracks:

```
## Post-Flight Reconciliation

After all agents return:

1. DEDUP: Same file:line from multiple agents → keep the one from the owning agent
2. PREFLIGHT CHECK: Every preflight finding must appear in an agent report as either:
   - Confirmed (with expanded context and fix)
   - Dismissed (with specific reason why it's a false positive)
   If a preflight finding is missing from all reports, it was dropped. Flag it.
3. COMPLETENESS: Reject findings missing file:line, current code, proposed fix, or why.
4. SEVERITY GATE: Apply severity levels based on project-specific impact, not generic rules.
```

### 4d: Apply Changes Using Preservation Tags

Go through the existing skill section by section, applying the tags from Phase 1:

**KEEP sections:** Copy verbatim. Don't rephrase, don't "improve" wording, don't reorganize. If it works, leave it alone. The user will notice if their carefully worded whitelist entry got rephrased into something subtly different.

**UPDATE sections, append mode:** Keep the existing content as-is, then append new items below a clear marker:
```markdown
## Security Agent
[... existing checks preserved exactly ...]

### Added by a-review-optimizer [DATE]
- [NEW] Check for pickle.load(): found in src/cache/serializer.py:22
- [NEW] Check for yaml.load without SafeLoader: found in src/config/loader.py:8
```

**UPDATE sections, correction mode:** Make the minimal targeted edit. Show a before/after in the diff summary so the user can verify:
```
[UPDATE] Security agent scope: removed "exception handling" (was overlapping with Architecture)
  Before: "Check for injection, path traversal, credentials, exception handling"
  After:  "Check for injection, path traversal, credentials"
```

**ADD sections:** Add as new sections at the end of the relevant part of the skill, clearly marked:
```markdown
### [NEW] Preflight Automated Scan
Added by a-review-optimizer: this section did not exist in the original skill.
[... new content ...]
```

**REMOVE / MOVE sections:** Rare for review skills. Only remove content when a whole check targets a pattern the project provably no longer uses (confirmed via grep of current code). Only move when content is correct but clearly belongs under a different agent or phase. In both cases, note the rationale in the Phase 5 diff summary so the user can verify.

**What you must NEVER do:**
- Rewrite agent prompts from scratch when they just need a few additions
- Remove whitelist entries without confirming they're stale (check if the referenced file/pattern still exists)
- Change agent names or restructure the skill's overall flow
- Replace working preflight scripts with new ones (add checks to existing, or create a new script alongside)
- Remove existing check items just because they're not in your gap analysis (they might catch things you haven't seen yet)
- Rephrase project-specific conventions documentation (the user wrote it in their own words for a reason)

### 4e: Wire Modes, Self-Learning, and Rule Candidates

Beyond the agent prompts and preflight, a mature skill carries a set of structural sections that make it as capable as a hand-tuned one. Read `references/skill-scaffold.md` and, for each section, check whether the target skill has it. Missing sections are `ADD`; thin ones are `UPDATE`. Preserve any the user already tuned. This step only ever adds.

- **Flag surface** (§1): `--changed` default, plus `--full` / `--security-only` / `--debt` / `--all` where the project warrants them.
- **Execution / parallel dispatch** (§2): agents launched in one batch, an Owns / Does-NOT-check scope table (this is where Phase 3c overlap resolutions get written down), preflight findings split by check-ID prefix into each agent's `PREFLIGHT KNOWN ISSUES` block.
- **Context Awareness (DO NOT flag)** (§3): a centralized whitelist that GROWS: every preflight finding the review *dismisses* as a false positive gets appended here under a dated `### Added by a-review-optimizer [DATE]` marker with a real file:line referent. This is the same append discipline as 4d; do it for the whitelist, not just the agent prompts.
- **`--debt` mode** (§4): points-per-severity scoring (Critical 8 / High 4 / Medium 2 / Low 1, cap 100), a per-agent breakdown table, tightened thresholds, and an Auto-Fixable safe-vs-needs-confirmation split.
- **`--full` mode** (§5): component + layer Mermaid diagrams (scan the stack's import mechanism, cross-layer edges only) and a weighted Architecture Health Score. Skip for projects with no real layered structure.
- **Self-learning capture loop** (§6): emit `.claude/scripts/capture-finding.sh`, a `Capture Findings for Self-Learning (MANDATORY)` section (one RUN_ID per run, one call per confirmed finding), and a seeded stable category-slug table. This wires the skill into `a-self-learner`.
- **Rule candidates** (§7): surface any pattern hit in 3+ locations as a proposed rule for `.claude/rules/`, asking before adding.

Only emit modes the project needs: don't bolt `--debt` query scoring onto a static site or draw architecture diagrams for a 3-file script. But if the target skill is missing the self-learning loop, the whitelist-growth discipline, or the flag surface, those are real capability gaps: add them.

---

## Phase 5: Deliver

Present to the user:

1. **Gap Report**: summarize what you found (missing checks, false positives, overlaps, preflight gaps). Keep it concise: this is context, not the deliverable.

2. **Improved SKILL.md**: the existing skill with targeted changes applied, ready to drop in. The user should be able to diff this against the original and see exactly what changed.

3. **Preflight Script**: if the project needs one (most do), the generated script.

4. **Diff Summary**: this is the external deliverable (contrast with the internal Phase 3 gap report). What changed, what stayed, and why. Every change must be traceable to a Phase 3 finding:
   ```
   CHANGES FROM ORIGINAL
   =====================
   [KEEP] Severity gating: no issues found, preserved as-is
   [KEEP] Known-correct patterns: all references still valid
   [KEEP] Output format structure: already had fix readiness columns
   [UPDATE] Security agent prompt: appended 2 checks (pickle.load, yaml.load) found in codebase
   [UPDATE] Preflight script: appended 4 new checks (SUB-01, WEB-01, TYPE-01, TYPE-02)
   [UPDATE] Agent scope overlap: Security no longer checks exception handling (Architecture owns it)
   [UPDATE] Stale reference: updated src/web/views/old_view.py:33 → src/web/views/dashboard.py:45
   [ADD] Fix Readiness Requirement: findings now require 4 fields
   [ADD] Post-flight reconciliation: preflight findings verified against agent output
   ```

5. **Verification checklist**: specific things the user should test to confirm the improved skill works:
   ```
   RUN THESE TO VERIFY
   ====================
   1. bash .claude/scripts/preflight-comprehensive.sh | python3 -m json.tool  (valid JSON?)
   2. /example-review --full  (catches known issues X, Y, Z from our analysis?)
   3. Check: zero duplicate findings across agents
   4. Check: every finding has file:line + current code + fix + why
   ```

---

## When No Review Skill Exists

Building from scratch is fundamentally different from improving an existing skill: plan on **2-3× the time** and expect fewer project-specific insights unless Phase 2 is done deeply. Don't rush Phase 2.

The from-scratch workflow:

1. **Phase 1: SKIP**: nothing to ingest, no preservation tags to apply.
2. **Phase 2: FULL**: this is now your primary input. Read the code carefully; conventions you miss here become gaps you won't catch.
3. **Phase 3: SKIP gap-against-existing**: there's no baseline. Instead, treat `references/review-dimensions.md` as the complete checklist and decide which dimensions this project actually needs (a CLI-only Python tool doesn't need a "Web Patterns" agent; a static site doesn't need SSRF checks).
4. **Phase 4: Build from zero, not from a generic template.**
   - Decide how many agents make sense (2-4, based on project complexity, don't over-fragment).
   - Assign scopes based on Phase 2 findings + the dimensions you kept from Phase 3. Scopes must be non-overlapping.
   - Generate agent prompts with **concrete file:line examples from the codebase you actually read**: no generic "look for bare except" lines. Every check needs a real referent.
   - Generate a preflight script using only the checks that detect patterns the project actually uses.
   - Use the output format template from `references/output-template.md`.
   - Build out the structural sections from `references/skill-scaffold.md`: the flag surface, the parallel-dispatch scope table, a starter Context Awareness whitelist (from your Phase 2/3b false positives), the `--debt` / `--full` modes the project warrants, the self-learning capture loop (emit `capture-finding.sh` + the seeded slug table), and rule-candidate surfacing. A skill missing these reads generic. A mature one has all of them.
5. **Phase 5:** deliver as usual; the Diff Summary just reads "built from scratch, no prior version."

The result should be what an experienced engineer would write after working on the project for months, not a generic checklist with the project name swapped in. If the output reads generic, go back to Phase 2.

---

## Capability Checklist

Before delivering (improve or from-scratch), confirm the skill can produce each of these. A gap here is a `MISSING CHECK` for Phase 3; close it (stack-appropriately) unless the project genuinely doesn't need it. Don't remove a capability the user already has.

- [ ] **3+ parallel, non-overlapping agents**: each with an explicit YOUR SCOPE / Owns list and a Does-NOT-check column; overlaps resolved (Phase 3c, `skill-scaffold.md` §2).
- [ ] **Deterministic preflight**: unique check IDs each mapped to one agent, JSON with file:line (not just counts), graceful clean handling, inline-Python for multi-line patterns (`preflight-template.md`).
- [ ] **Fix readiness**: every finding carries file:line + current code + proposed fix + why; incomplete findings rejected (`output-template.md`).
- [ ] **Severity gating**: BLOCK/WARN/INFO tied to project-specific impact (`output-template.md`).
- [ ] **`--debt` mode**: points-per-severity score, per-agent breakdown table, Auto-Fixable safe-vs-needs-confirmation split (`skill-scaffold.md` §4).
- [ ] **`--full` mode**: component + layer Mermaid diagrams and an Architecture Health Score (`skill-scaffold.md` §5).
- [ ] **Flag surface**: `--changed` (default), `--full`, `--security-only`, `--debt`, `--all` (`skill-scaffold.md` §1).
- [ ] **Post-flight reconciliation**: dedup by owning agent, every preflight finding accounted (confirmed/dismissed), completeness reject, severity gate (Phase 4c).
- [ ] **Self-learning loop**: emits `capture-finding.sh`, a mandatory capture section with one RUN_ID per run, and a stable kebab-case slug table feeding `a-self-learner` (`skill-scaffold.md` §6).
- [ ] **Rule candidates**: recurring pattern (3+ locations) → propose a rule for `.claude/rules/`, ask before adding (`skill-scaffold.md` §7).
- [ ] **Context Awareness whitelist that grows**: a centralized DO-NOT-flag section; dismissed false positives appended each run under a dated marker (`skill-scaffold.md` §3).

---

## Reference Files

Read these as needed during analysis:

| File | When to Read | Contains |
|-|-|-|
| `references/review-dimensions.md` | Phase 3a (finding missing checks) | Complete taxonomy of review categories across all stacks |
| `references/pattern-detection.md` | Phase 2 Step 3 and Phase 4b | Library of grep/rg/Python detection scripts per language |
| `references/preflight-template.md` | Phase 4b (generating preflight script) | Bash+Python script template with JSON output |
| `references/output-template.md` | Phase 4 (when building from scratch) | Standard output format with fix readiness columns |
| `references/skill-scaffold.md` | Phase 4d and from-scratch build | Structural sections outside the agents: flags, dispatch skeleton, growing whitelist, `--debt`/`--full` modes, self-learning capture loop, rule candidates |

# Review Output Format Template

Use this when building a new review skill from scratch (Phase 4, no existing skill). The format enforces fix readiness on every finding.

## Report Template

```markdown
# Code Review Report
**Project:** [project name]
**Stack:** [languages + frameworks]
**Target:** [X files: scope description]
**Date:** [timestamp]
**Preflight:** [X checks run, Y findings fed to agents]

## Summary
| Severity | Count |
|-|-|
| Critical | X |
| High | X |
| Medium | X |
| Low | X |

## Critical Issues (Fix Before Merge)
| # | Agent | File:Line | Current Code | Proposed Fix | Why |
|-|-|-|-|-|-|
| 1 | Security | src/auth.py:44 | admin credential hardcoded as a string literal | value read from an environment variable instead | Hardcoded credential accessible to anyone with repo access |

## High Priority
| # | Agent | File:Line | Current Code | Proposed Fix | Why |
|-|-|-|-|-|-|

## Medium Priority
| # | Agent | File:Line | Current Code | Proposed Fix | Why |
|-|-|-|-|-|-|

## Low Priority
[Summary count by category: individual items not listed]
- Type modernization: X items
- Naming: X items
- Style: X items

## Preflight Reconciliation
| Check ID | Status | Agent | Verdict |
|-|-|-|-|
| SEC-01 | fail | Security | Confirmed: see finding #1 |
| SUB-01 | warn | Architecture | Dismissed: timeout not needed (local-only script) |
| WEB-01 | warn | Architecture | Confirmed: see finding #4 |

## Auto-Fixable Issues
**Safe (no logic changes):**
- src/utils.py:1-3: remove unused imports (os, sys, re)
- src/models.py:22: `Optional[str]` → `str | None`
- src/views.py:8: `List[dict]` → `list[dict]`

**Needs confirmation:**
- src/backup.py:140-180: duplicate sync block, extract to helper? (logic might differ subtly)

## Recommended Fix Order
1. [Critical #1] Fix hardcoded credential: security exposure
2. [High #3] Add timeout to subprocess.run: blocks indefinitely on large DBs
3. [Auto-fix batch] Run safe auto-fixes (imports, types)
4. [Medium items] Address in next sprint

## Tech Debt Score (if requested)
**Score: X/100**
| Category | Points |
|-|-|
| Security | X |
| Architecture | X |
| Quality | X |
| Performance | X |

Breakdown: 0 = pristine, 25 = healthy, 50 = needs attention, 75+ = stop and fix
```

## Fix Readiness Rules

Every finding at Critical, High, or Medium severity MUST include all 4 columns:

| Column | Required | What it contains |
|-|-|-|
| File:Line | Always | Exact location, e.g., `src/backup.py:88` |
| Current Code | Always | The actual code as it exists (verbatim, can be truncated with `...`) |
| Proposed Fix | Always | What the code should look like after fixing |
| Why | Always | One sentence, specific to this project, explaining impact |

**Incomplete findings are rejected.** If an agent can't fill all 4 fields, it either needs to investigate more or the finding isn't actionable.

Low-priority findings are summarized by category (no individual table entries) to keep the report scannable.

## Severity Definitions

| Level | Label | Criteria | Action |
|-|-|-|-|
| BLOCK | Critical | Exploitable security vuln, data loss risk, broken core functionality | Fix before merge |
| WARN | High | Missing error handling on critical path, N+1 on hot path, broken contract, auth gap | Fix this sprint |
| WARN | Medium | Dead code, missing types on public API, duplication, modernization with clear benefit | Fix when touching file |
| INFO | Low | Style, naming, minor modernization | Optional cleanup |

## Agent Output Constraint

Include this in every agent prompt to enforce the format:

```
OUTPUT RULES:
- Final response under 3000 characters
- List findings, not your reasoning process
- Every finding: File:Line | Current Code | Proposed Fix | Why
- If you can't fill all 4 fields, investigate more or drop the finding
- Group by severity: Critical first, then High, Medium
- Do not list Low-severity items individually: summarize count by category
- End with: "FINDINGS: X critical, X high, X medium, X low"
```

# Pattern Detection Script Library

Use this during Phase 2 Step 3 (pattern inventory) and Phase 4b (generating preflight scripts). Pick the scripts relevant to the project's stack.

All scripts are designed to be safe (|| true suffix, head limits on output) and produce file:line locations.

## Universal Detection Scripts

### Credentials & Secrets
```bash
# Hardcoded secret assignments (exclude env/config accessors)
rg -n '(password|api_key|secret_key|api_secret|token|private_key)\s*=\s*["\x27][^"\x27]{4,}' \
  --type-add 'code:*.{py,php,js,ts,rb,go,java}' -t code src/ \
  | grep -v 'getenv\|os.environ\|get_setting\|\.get(' || true

# Files that shouldn't be tracked
git ls-files -- '*.env' '.env.*' '*.key' '*.pem' 2>/dev/null \
  | grep -v '.example' | grep -v '.sample' || true
```

### Error Handling
```bash
# Bare except (Python)
rg -n '^\s*except\s*:' --type py src/ || true

# catch(Exception) or catch(\Exception) (PHP)
rg -n 'catch\s*\(\s*\\?Exception' --type php src/ || true

# Generic catch(e) (JS/TS)
rg -n 'catch\s*\(\s*\w+\s*\)\s*\{' --type js --type ts src/ || true

# Silent exception handlers (Python: except block with only pass/continue)
python3 -c "
import re, pathlib
for f in pathlib.Path('src').rglob('*.py'):
    lines = f.read_text().splitlines()
    for i, line in enumerate(lines):
        if re.match(r'\s*except\b', line):
            # Check next non-blank lines in the except block
            body_lines = []
            base_indent = len(line) - len(line.lstrip())
            for j in range(i+1, min(i+5, len(lines))):
                stripped = lines[j].strip()
                indent = len(lines[j]) - len(lines[j].lstrip())
                if indent <= base_indent and stripped: break
                if stripped: body_lines.append(stripped)
            if body_lines and all(b in ('pass', 'continue', '...') for b in body_lines):
                print(f'{f}:{i+1}: silent except: body is only {body_lines[0]}')
" 2>/dev/null || true
```

### Code Metrics
```bash
# Files over 300 lines
find src/ app/ lib/ -name '*.py' -o -name '*.php' -o -name '*.ts' -o -name '*.js' 2>/dev/null \
  | xargs wc -l 2>/dev/null | awk '$1 > 300 && !/total$/' | sort -rn || true

# Deep nesting (5+ levels = 20+ leading spaces)
rg -n '^\s{20,}\S' --type-add 'code:*.{py,php,js,ts}' -t code src/ | head -20 || true

# Long functions (Python: def to next def at same/lower indent, >50 lines)
python3 -c "
import re, pathlib
for f in pathlib.Path('src').rglob('*.py'):
    lines = f.read_text().splitlines()
    func_start = None
    func_name = ''
    func_indent = 0
    for i, line in enumerate(lines):
        m = re.match(r'^(\s*)def\s+(\w+)', line)
        if m:
            if func_start and (i - func_start) > 50:
                print(f'{f}:{func_start+1}: {func_name}() is {i - func_start} lines')
            func_start = i
            func_name = m.group(2)
            func_indent = len(m.group(1))
    if func_start and (len(lines) - func_start) > 50:
        print(f'{f}:{func_start+1}: {func_name}() is {len(lines) - func_start} lines')
" 2>/dev/null || true

# print/var_dump/console.log in production code
rg -n '\bprint\(' --type py src/ --glob '!*test*' --glob '!*__pycache__*' 2>/dev/null | head -20 || true
rg -n '\bvar_dump\(|\bdd\(' --type php src/ 2>/dev/null | head -20 || true
rg -n '\bconsole\.(log|debug)\(' --type js --type ts src/ --glob '!*test*' 2>/dev/null | head -20 || true

# Commented-out code (3+ consecutive comment lines with code patterns)
python3 -c "
import re, pathlib
code_pattern = re.compile(r'#\s*(def |class |import |return |if |for |while |print|self\.|=\s)')
for f in pathlib.Path('src').rglob('*.py'):
    lines = f.read_text().splitlines()
    streak = 0
    streak_start = 0
    for i, line in enumerate(lines):
        if code_pattern.match(line.strip()):
            if streak == 0: streak_start = i
            streak += 1
        else:
            if streak >= 3:
                print(f'{f}:{streak_start+1}: {streak} lines of commented-out code')
            streak = 0
    if streak >= 3:
        print(f'{f}:{streak_start+1}: {streak} lines of commented-out code')
" 2>/dev/null || true
```

## Python-Specific Detection

### Security
```bash
# shell=True
rg -n 'shell\s*=\s*True' --type py src/ || true

# os.system()
rg -n 'os\.system\(' --type py src/ || true

# Unsafe deserialization
rg -n 'pickle\.loads?\(' --type py src/ || true
rg -n 'yaml\.load\(' --type py src/ | grep -v SafeLoader || true
rg -n '\beval\s*\(' --type py src/ | grep -v 'ast.literal_eval' || true
```

### Subprocess (multi-line aware)
```bash
# subprocess without timeout: MUST use Python for multi-line detection
python3 -c "
import re, pathlib
for f in pathlib.Path('src').rglob('*.py'):
    text = f.read_text()
    for m in re.finditer(r'subprocess\.(run|call|check_output|check_call|Popen)\s*\(', text):
        start = m.start()
        depth, pos = 1, m.end()
        while pos < len(text) and depth > 0:
            if text[pos] == '(': depth += 1
            elif text[pos] == ')': depth -= 1
            pos += 1
        call_text = text[m.start():pos]
        if 'timeout' not in call_text:
            line_num = text[:start].count('\n') + 1
            print(f'{f}:{line_num}: subprocess.{m.group(1)}() without timeout=')
" 2>/dev/null || true

# String interpolation in subprocess args
python3 -c "
import re, pathlib
for f in pathlib.Path('src').rglob('*.py'):
    text = f.read_text()
    for m in re.finditer(r'subprocess\.\w+\s*\(', text):
        start = m.start()
        depth, pos = 1, m.end()
        while pos < len(text) and depth > 0:
            if text[pos] == '(': depth += 1
            elif text[pos] == ')': depth -= 1
            pos += 1
        call_text = text[m.start():pos]
        if re.search(r'f[\"\\x27]|\.format\(|%\s', call_text) and 'shell' not in call_text:
            line_num = text[:start].count('\n') + 1
            print(f'{f}:{line_num}: f-string/format in subprocess args')
" 2>/dev/null || true
```

### Type Modernization (Python 3.10+)
```bash
# Old typing imports
rg -n 'from typing import.*(Optional|List|Dict|Tuple|Set|Union)' --type py src/ || true

# Public functions without return type
python3 -c "
import re, pathlib
for f in pathlib.Path('src').rglob('*.py'):
    for i, line in enumerate(f.read_text().splitlines(), 1):
        if re.match(r'^(\s{0,8})def\s+(?!_|test_)\w+\(.*\)\s*:', line) and '->' not in line:
            print(f'{f}:{i}: {line.strip()[:80]}')
" 2>/dev/null | head -30 || true

# Unsafe nested dict access on external data
rg -n '\[.+\]\[.+\]' --type py src/ | grep -v 'test' | head -20 || true
```

### Streamlit-Specific
```bash
# st.rerun() without invalidate() in preceding N lines
python3 -c "
import pathlib
for f in pathlib.Path('src/web').rglob('*.py') if pathlib.Path('src/web').exists() else []:
    lines = f.read_text().splitlines()
    for i, line in enumerate(lines):
        if 'st.rerun()' in line:
            window = lines[max(0,i-10):i]
            if not any('invalidate' in w for w in window):
                print(f'{f}:{i+1}: st.rerun() without invalidate() in preceding 10 lines')
" 2>/dev/null || true

# @st.cache_data without TTL
rg -n '@st\.cache_data' --type py src/ | grep -v 'ttl' || true

# print() in web layer
rg -n '\bprint\(' --type py src/web/ 2>/dev/null | grep -v 'console' || true
```

## PHP-Specific Detection

### Security
```bash
# SQL with string interpolation
rg -n '(query|execute|prepare)\s*\(.*[\$"]' --type php src/ \
  | grep -v 'prepare.*?\?' | grep -v bindParam || true

# Shell execution functions
rg -n '\b(exec|system|shell_exec|passthru|proc_open|popen)\s*\(' --type php src/ || true

# File operations with variables (path traversal risk)
rg -n '(include|require|file_get_contents|fopen|unlink|rmdir)\s*\(\s*\$' --type php src/ || true

# echo/print without escaping
rg -n '(echo|print)\s+\$' --type php src/ | grep -v 'htmlspecialchars\|htmlentities' | head -20 || true

# unserialize on potentially untrusted data
rg -n 'unserialize\s*\(' --type php src/ || true
```

### Architecture
```bash
# env() outside config files (Laravel: returns null when cached)
rg -n '\benv\(' --type php src/ --glob '!config/*' 2>/dev/null | head -20 || true

# Missing $fillable/$guarded on Eloquent models
for f in $(find src/ app/ -name '*.php' 2>/dev/null | xargs grep -l 'extends Model' 2>/dev/null); do
  grep -L 'fillable\|guarded' "$f" && echo "$f: missing \$fillable/\$guarded"
done 2>/dev/null || true
```

### Modernization
```bash
# Legacy array() syntax
rg -n '\barray\s*\(' --type php src/ | head -20 || true

# strpos instead of str_contains (PHP 8.0+)
rg -n 'strpos\s*\(' --type php src/ | head -20 || true
```

## JavaScript/TypeScript-Specific Detection

### Security
```bash
# XSS vectors
rg -n 'innerHTML|outerHTML|document\.write|dangerouslySetInnerHTML|v-html' --type js --type ts src/ || true

# eval / Function constructor
rg -n '\beval\s*\(|new\s+Function\s*\(' --type js --type ts src/ || true

# Hardcoded JWT secrets
rg -n 'jwt\.(sign|verify)\s*\(' --type js --type ts src/ | head -10 || true
```

### TypeScript Quality
```bash
# any type usage
rg -n ':\s*any\b' --type ts src/ | head -20 || true

# Type assertions (hiding real errors)
rg -n '\bas\s+\w' --type ts src/ | head -20 || true

# @ts-ignore without explanation
rg -n '@ts-ignore|@ts-expect-error' --type ts src/ || true

# Non-null assertions
rg -n '\w+!' --type ts src/ | grep -v '!=\|!=' | head -20 || true
```

### React Patterns
```bash
# useEffect without cleanup (missing return in useEffect callback)
# Approximate: agents should verify
rg -n 'useEffect\(' --type ts --type js src/ | head -20 || true

# Missing key prop indicator (map without key)
rg -n '\.map\(' --type ts --type js src/ | head -20 || true

# Large components (>200 lines)
find src/ -name '*.tsx' -o -name '*.jsx' 2>/dev/null | xargs wc -l 2>/dev/null \
  | awk '$1 > 200 && !/total$/' | sort -rn || true
```

### Node.js
```bash
# Sync file operations in non-config code
rg -n 'readFileSync|writeFileSync|existsSync' --type js --type ts src/ \
  | grep -v 'config\|setup\|init' | head -20 || true

# Missing error handling middleware (Express)
rg -n 'app\.(get|post|put|delete|patch)\(' --type js --type ts src/ | head -10 || true
rg -n 'err,\s*req,\s*res,\s*next' --type js --type ts src/ || true
```

## Cross-Reference Detection

These checks compare two sources of truth. They're project-specific by nature: the optimizer should generate them based on what registration patterns the project uses.

### Registration Completeness Template
```bash
# Template: check that all X are registered in Y
# Adapt the patterns to the project's registration mechanism

# Example: Python views registered in PAGE_MAP
python3 -c "
import pathlib, re
view_dir = pathlib.Path('src/web/views')
init_file = view_dir / '__init__.py'
if not init_file.exists(): exit()
init_text = init_file.read_text()
for f in view_dir.glob('*.py'):
    if f.name.startswith('_'): continue
    for m in re.finditer(r'def (render_\w+)', f.read_text()):
        if m.group(1) not in init_text:
            print(f'{f}:{0}: {m.group(1)} not registered in PAGE_MAP')
" 2>/dev/null || true

# Example: PHP routes vs controllers
# Example: React pages vs router config
# Example: CLI commands vs command group registration
```

### Export Completeness Template
```bash
# Template: check that modules export what they define
# Example: Python __init__.py exports
python3 -c "
import pathlib, re
pkg = pathlib.Path('src/web/components')
init = pkg / '__init__.py'
if not init.exists(): exit()
init_text = init.read_text()
for f in pkg.glob('*.py'):
    if f.name.startswith('_'): continue
    for m in re.finditer(r'^def (\w+)|^class (\w+)', f.read_text(), re.MULTILINE):
        name = m.group(1) or m.group(2)
        if name and not name.startswith('_') and name not in init_text:
            print(f'{f}: {name} not exported in __init__.py')
" 2>/dev/null || true
```

# Preflight Script Template

Use this as the skeleton when generating a project-specific preflight script in Phase 4b. Replace the placeholder checks with actual checks derived from the project analysis.

## Script Structure

```bash
#!/usr/bin/env bash
# Preflight review checks for [PROJECT NAME]
# Generated by a-review-optimizer on [DATE]
# Runs deterministic pattern checks, outputs JSON for agent consumption.

set -euo pipefail

SRC_DIR="${1:-src/}"
RESULTS_FILE=$(mktemp)

# Initialize JSON output
echo '{' > "$RESULTS_FILE"
FIRST=true

emit() {
    local id="$1" status="$2" count="$3" message="$4"
    shift 4
    # Remaining args are locations
    local locations="[]"
    if [ $# -gt 0 ]; then
        locations=$(printf '%s\n' "$@" | python3 -c "
import sys, json
print(json.dumps([l.strip() for l in sys.stdin if l.strip()]))")
    fi

    if [ "$FIRST" = true ]; then
        FIRST=false
    else
        echo ',' >> "$RESULTS_FILE"
    fi

    cat >> "$RESULTS_FILE" <<ITEM
  "$id": {
    "status": "$status",
    "count": $count,
    "message": "$message",
    "locations": $locations
  }
ITEM
}

# ============================================================
# GROUP A: Security Checks (route to Security Agent)
# ============================================================

# SEC-01: [description]
HITS=$(rg -n '[PATTERN]' --type [TYPE] "$SRC_DIR" 2>/dev/null || true)
COUNT=$(echo "$HITS" | grep -c . 2>/dev/null || echo 0)
if [ "$COUNT" -gt 0 ]; then
    LOCS=$(echo "$HITS" | awk -F: '{print $1":"$2}')
    emit "SEC-01" "fail" "$COUNT" "$COUNT [description]" $LOCS
else
    emit "SEC-01" "pass" "0" "No [description] found"
fi

# SEC-02: [next check...]
# ... repeat pattern for each check

# ============================================================
# GROUP B: Subprocess/External Call Checks (route to Architecture Agent)
# ============================================================

# SUB-01: Multi-line subprocess check (Python required for balanced-paren scan)
# Example filled in: replace with a pattern that matches THIS project.
# See references/pattern-detection.md for more detection recipes.
HITS=$(python3 -c "
import re, pathlib
for f in pathlib.Path('$SRC_DIR').rglob('*.py'):
    text = f.read_text()
    for m in re.finditer(r'subprocess\.(run|call|check_output|check_call|Popen)\s*\(', text):
        start = m.start()
        depth, pos = 1, m.end()
        while pos < len(text) and depth > 0:
            if text[pos] == '(': depth += 1
            elif text[pos] == ')': depth -= 1
            pos += 1
        call_text = text[m.start():pos]
        if 'timeout' not in call_text:
            line_num = text[:start].count('\n') + 1
            print(f'{f}:{line_num}: subprocess.{m.group(1)}() without timeout=')
" 2>/dev/null || true)
COUNT=$(echo "$HITS" | grep -c . 2>/dev/null || echo 0)
if [ "$COUNT" -gt 0 ]; then
    LOCS=$(echo "$HITS" | awk -F: '{print $1":"$2}')
    emit "SUB-01" "warn" "$COUNT" "$COUNT subprocess calls without timeout" $LOCS
else
    emit "SUB-01" "pass" "0" "All subprocess calls have timeout"
fi

# ============================================================
# GROUP C: Framework-Specific Checks (route varies by check)
# ============================================================

# WEB-01, WEB-02, etc.: only include if project uses a web framework
# CLI-01, CLI-02, etc.: only include if project has CLI commands

# ============================================================
# GROUP D: Type Safety Checks (route to Quality Agent)
# ============================================================

# TYPE-01: [description]
# ...

# ============================================================
# GROUP E: Code Quality Checks (route to Quality Agent)
# ============================================================

# QUAL-01: Files over 300 lines
HITS=$(find "$SRC_DIR" -name '*.py' -o -name '*.php' -o -name '*.ts' -o -name '*.js' 2>/dev/null \
    | xargs wc -l 2>/dev/null | awk '$1 > 300 && !/total$/ {print $2":"$1}' || true)
COUNT=$(echo "$HITS" | grep -c . 2>/dev/null || echo 0)
if [ "$COUNT" -gt 0 ]; then
    LOCS=$(echo "$HITS" | awk -F: '{print $1}')
    emit "QUAL-01" "info" "$COUNT" "$COUNT files over 300 lines" $LOCS
else
    emit "QUAL-01" "pass" "0" "No oversized files"
fi

# ============================================================
# GROUP F: Performance Checks (route to Performance Agent)
# ============================================================

# PERF-01: [description]
# ...

# ============================================================
# Close JSON and output
# ============================================================

echo '' >> "$RESULTS_FILE"
echo '}' >> "$RESULTS_FILE"

cat "$RESULTS_FILE"
rm -f "$RESULTS_FILE"
```

## Check ID Convention

| Prefix | Agent Target | Domain |
|-|-|-|
| SEC-* | Security | Injection, credentials, permissions |
| SUB-* | Architecture | Subprocess safety, external call patterns |
| EXC-* | Architecture | Exception handling patterns |
| WEB-* | Architecture | Web framework patterns (Streamlit, Django, React, etc.) |
| CLI-* | Architecture | CLI framework patterns (Click, argparse, etc.) |
| REG-* | Architecture | Registration completeness (routes, views, exports) |
| TYPE-* | Quality | Type annotations and safety |
| QUAL-* | Quality | Complexity, dead code, formatting |
| DEAD-* | Quality | Dead code detection |
| PERF-* | Performance | Query patterns, caching, I/O |

The check-ID prefix is the routing key: the skill splits the preflight JSON by prefix and injects each group into the owning agent's `PREFLIGHT KNOWN ISSUES` block. For an unambiguous mapping, mature scripts also emit an explicit `"agent"` (and human-readable `"name"`) field per check, e.g.:

```json
"SEC-04": { "name": "POST forms without CSRF", "agent": "security", "status": "fail", "count": 2, "files": ["app/views/x.php:12", "app/views/y.php:40"] }
```

That way the dispatcher can route on the field directly and the JSON is self-documenting. A top-level `"status"` (`pass`/`warn`/`fail`) summarizing the worst check is also useful for a fast gate.

## Rules for Good Preflight Checks

1. **Every check must output file:line locations.** Counts alone aren't useful.
2. **Use Python for multi-line patterns.** grep/rg work for single-line patterns. For anything that spans lines (subprocess calls, except blocks, function signatures with defaults on next line), use inline Python.
3. **Handle "not found" gracefully.** `|| true` on every command. Empty output = pass, not error.
4. **Limit output.** `| head -20` on checks that could produce hundreds of results. The agent will do the full scan.
5. **Include whitelist comments.** If a check has known false positives for this project, document them as comments so the next person maintaining the script understands.
6. **Status meanings:**
   - `pass`: check ran, nothing found
   - `warn`: found something that needs agent review (might be false positive)
   - `fail`: found something almost certainly wrong
   - `info`: metric/count, no action needed (e.g., file size counts)

# Review Dimensions Taxonomy

Use this during Phase 3a to identify what the existing skill is missing. For each dimension, check whether the existing skill covers it AND whether the project actually needs it.

Not every project needs every dimension. A CLI tool doesn't need "WebSocket security." A static site doesn't need "N+1 query detection." Skip what's irrelevant, flag what's missing and needed.

## Security Dimensions

### Input Handling
- SQL injection (parameterized queries, ORM safety)
- Command injection (subprocess, exec, system calls)
- Path traversal (filesystem operations with user input)
- XSS (output encoding, template escaping)
- SSRF (URL construction from user input)
- Deserialization (pickle, unserialize, YAML load, eval)
- ReDoS (catastrophic regex backtracking on user input)
- Header injection (CRLF in user-controlled headers)
- Template injection (SSTI in Jinja2, Twig, EJS)

### Credentials & Secrets
- Hardcoded passwords, API keys, tokens in source
- Secrets in logs, error messages, or stack traces
- Secrets committed to git (even in history)
- Encryption keys with wrong file permissions
- Default/weak passwords in config examples
- Credentials in URLs (basic auth in connection strings)

### Authentication & Authorization
- Missing auth checks on endpoints/routes
- Auth without authz (logged in but not permitted)
- Session fixation (no regeneration after login)
- CSRF protection on state-changing requests
- JWT: hardcoded secret, missing expiry, alg:none
- Cookie flags: httpOnly, secure, sameSite
- Timing attacks on password/token comparison
- Password hashing: bcrypt/argon2 vs MD5/SHA1

### Data Protection
- Sensitive data in localStorage/sessionStorage
- PII in logs or analytics
- Missing encryption at rest for sensitive data
- Broad CORS configuration with credentials
- Missing rate limiting on auth endpoints

## Architecture Dimensions

### Configuration Management
- Hardcoded values that should be configurable
- Config access scattered vs centralized
- Missing defaults for optional config
- Environment-specific logic in business code
- Secrets mixed with non-secret config

### Error Handling Strategy
- Generic catch-all exceptions
- Swallowed exceptions (catch + pass/ignore)
- Missing cleanup on failure (partial state)
- Missing timeouts on external calls
- Error messages that don't help diagnose
- Inconsistent error return shapes
- Missing retry logic with backoff where needed

### State Management
- Framework-appropriate state patterns (session_state, context, store)
- Mutable shared state without synchronization
- State scattered across globals
- Cache invalidation after mutations
- Stale state after redirects/reruns

### Dependency & Coupling
- Circular imports/dependencies
- Layer violations (presentation ↔ data)
- God objects everything depends on
- Components reaching into other components' internals
- Business logic in infrastructure code

### Registration & Wiring
- Routes/views/commands registered correctly
- Middleware/interceptors applied where needed
- Event listeners registered for dispatched events
- Components exported from package indexes
- DI container bindings for all interfaces

### API Design
- Consistent response shapes
- Input validation at boundaries
- Versioning strategy for external APIs
- Batch operations where N individual calls exist
- Pagination on list endpoints

### Consistency
- Same pattern used differently across files
- Naming conventions followed/violated
- Import ordering convention
- File/directory organization convention

## Quality Dimensions

### Type Safety
- Missing type annotations on public interfaces
- Nullable access without null check
- Generic "any"/"mixed" where specific types work
- Type assertions hiding real type errors
- Dict/array access on external data without .get()
- Inconsistent use of type aliases

### Dead Code
- Functions never called from any module
- Unused imports
- Commented-out code blocks (>3 lines)
- Unreachable code after return/throw
- Feature flags always true/false
- Config keys defined but never accessed
- Event handlers never triggered
- Store actions never dispatched

### Complexity
- Files over 300 lines
- Functions over 50 lines
- Nesting over 4 levels
- Cyclomatic complexity over 10
- Parameter count over 5
- Boolean parameters (should be named/enum)

### Duplication
- Same logic in 2+ places
- Similar error handling repeated
- Similar validation in multiple endpoints
- Copy-pasted data transformations
- Similar test setup across test files

### Modernization
- Outdated syntax for the language version
- Old library APIs when newer alternatives exist
- Manual implementations of things the stdlib handles
- Deprecated function/method usage

### Naming
- Misleading names (function does more than name suggests)
- Boolean variables not starting with is/has/can/should
- Unclear abbreviations
- Inconsistent naming convention within a module

### Test Coverage
- Public methods with complex logic but no tests
- Edge cases not covered (empty input, null, overflow)
- Error paths not tested
- Integration points not tested
- Tests that only test the happy path

## Performance Dimensions

### Query Efficiency
- N+1 queries (loop + query pattern)
- SELECT * when specific columns suffice
- Missing LIMIT on unbounded queries
- Filtering/sorting in app instead of DB
- Missing indexes on filtered/joined columns
- Missing connection pooling
- Repeated identical queries per request

### I/O Efficiency
- File/HTTP reads inside loops
- Synchronous I/O in async context
- Loading full files when streaming works
- Not closing handles/connections (resource leaks)
- Missing batching on external API calls

### Caching
- Same expensive computation repeated per request
- Rarely-changing data fetched every time
- Missing cache invalidation after writes
- Cache keys not accounting for all parameters

### Memory
- Full dataset loaded when pagination/chunking works
- String concatenation in loops (vs join/builder)
- Large objects retained when no longer needed
- Missing generators/iterators for large sequences

### Algorithmic
- O(n²) when O(n) or O(n log n) is possible
- Linear search where hash/set lookup works
- Repeated sorts on same data
- Unnecessary serialization/deserialization cycles

### Response/Payload
- API returning more data than consumer needs
- Missing compression on large responses
- Missing pagination
- Full page re-render when partial update suffices

## AI Slop Dimensions (optional, for --slop equivalent)

### Over-Abstraction
- Interface with exactly one implementation
- Factory for something instantiated once
- Strategy with one strategy
- Wrapper that adds no behavior

### Premature Generalization
- Config options no code path exercises
- Parameters always called with same value
- Plugin systems with zero plugins

### Confident-But-Wrong
- Error handling that doesn't actually handle anything
- Retry without idempotency consideration
- Pagination that breaks on last page
- Auth checking login but not permissions

### Copy-Paste Artifacts
- Variable names from a different context
- Comments describing what code used to do
- Imports for unused libraries
- Exception types from wrong framework

# Generated-Skill Scaffold

The parts of a mature review skill that live *outside* the agent prompts and the report format. `output-template.md` covers the report body and severity definitions; this file covers everything else a hand-tuned review skill contains: the flag surface, the parallel-dispatch skeleton, the growing false-positive whitelist, the `--debt` and `--full` modes, the self-learning capture loop, and rule-candidate surfacing.

Use it two ways:
- **Improving an existing skill (Phase 3/4):** treat the section list below as a coverage checklist. For each section, does the target skill have it? Missing ones are `ADD`; thin ones are `UPDATE`. Never rip out a section the user already tuned.
- **From scratch (Phase 4 "When No Review Skill Exists"):** this is the blueprint. Emit the sections the project actually needs (a static site skips `--debt` query scoring; a CLI tool skips web XSS whitelists), each populated with real file:line referents from Phase 2, never generic placeholders.

Everything here is stack-agnostic. The examples span PHP, Python, JS/TS, Go, and Rust on purpose; pick the idiom that matches the project you analyzed.

---

## 1. Flag surface

A mature skill exposes a small, stable set of scope/depth flags. Emit only the ones the project needs, but keep the names identical across projects so muscle memory transfers.

```markdown
## Flags

| Flag | Behavior |
|-|-|
| `--changed` | Default. Review only files changed since HEAD (`git diff --name-only HEAD`) |
| `--full` | Review the whole source tree + architecture diagrams + health score |
| `--security-only` | Run the Security agent alone |
| `--debt` | Add tech-debt scoring, stricter thresholds, and extra checks to every agent |
| `--all` | `--full` + `--debt` combined (most thorough) |

Flags combine where logical (e.g. `--security-only --debt`).
```

Guidance:
- `--changed` is always the default. State the exact git command so the skill is deterministic about scope.
- `--debt` is a *modifier*, not a separate scope: it lowers thresholds (file-length, function-length) and turns on extra checks that are too noisy for every run.
- Only ship `--full`'s diagram half if the project has a real layered architecture to draw. A 3-file script doesn't.

---

## 2. Execution / parallel dispatch

Every mature skill launches its agents **simultaneously** and hands each one only its slice of the preflight output. Teach the generated skill to do the same.

```markdown
## Execution: N Parallel Sub-Agents

Launch all agents in one batch: they have no cross-dependencies.

| Agent | Owns (checks these) | Does NOT check |
|-|-|-|
| Security | injection, authn/authz, secrets, unsafe deserialization, SSRF | error-handling structure, types, dead code |
| Architecture | layering, registration/wiring, error-handling structure, resource cleanup | injection content, type modernization |
| Quality | types, dead code, duplication, complexity, modernization | anything Security or Architecture owns |

If `--security-only` is set, run only the Security agent.
Split preflight findings by check-ID prefix and inject each group into the owning
agent's `PREFLIGHT KNOWN ISSUES` block (SEC-* → Security, ARCH-*/EXC-*/WEB-* →
Architecture, TYPE-*/QUAL-* → Quality). After agents return, run Post-Flight Reconciliation.
```

The **Owns / Does NOT check** two-column form is the anti-overlap device: a finding belongs to exactly one agent, and each agent is told in writing what to leave alone. This is where the Phase 3c overlap resolutions get written down. 3+ agents is the norm; 2 is acceptable for a small single-concern project, more than 4 usually means you fragmented a scope that should be one agent.

---

## 3. Context Awareness (DO NOT flag): and how it grows

The single highest-value section in a mature skill. It records hard-won false positives so the review stops re-flagging correct project conventions. A from-scratch skill starts this section from Phase 2/3b findings; every later optimizer run **appends** to it.

```markdown
## Context Awareness (DO NOT flag)

These patterns are correct in THIS project. Agents must not flag them:
- Base class handles prepared statements via inherited query methods (all Models extend it)
- `<Framework>` command functions look unused: they're invoked by the framework via decorators/registry
- `.example` / `.sample` config files are committed intentionally; real configs are gitignored
- Result-tuple return shape `(ok, msg)` is the project convention, not a smell: see src/core/engine.<ext>:45

### Added by a-review-optimizer [YYYY-MM-DD]
- <newly confirmed false positive>, with a real file:line referent and one-line reason
```

**The growth mechanism (make the optimizer do this every run):**
- When Post-Flight Reconciliation *dismisses* a preflight finding as a false positive, that dismissal is knowledge. Add it here under a dated `### Added by a-review-optimizer [DATE]` marker, with the file:line and the one-line reason it's safe.
- Never delete an existing whitelist entry unless you confirmed (via grep of current code) the referenced pattern is gone. Stale-looking entries still catch things.
- Keep the user's original wording verbatim; only append. A rephrased whitelist entry is a whitelist entry the user can no longer trust.
- Prefer a real referent (`file:line`) over prose: it lets the next run verify the entry is still live.

This section and the self-learning log (§6) are two halves of the same loop: confirmed findings feed the log; dismissed findings feed this whitelist.

---

## 4. `--debt` mode

Turns the review into a scored artifact. Only computed when `--debt` (or `--all`) is set.

```markdown
## Debt Scoring (--debt / --all only)

Assign points per confirmed finding, then sum (cap at 100, lower is better):

| Severity | Points |
|-|-|
| Critical | 8 |
| High | 4 |
| Medium | 2 |
| Low | 1 |

### Score Breakdown
| Agent | Critical | High | Medium | Low | Score |
|-|-|-|-|-|-|
| Security | X | X | X | X | X |
| Architecture | X | X | X | X | X |
| Quality | X | X | X | X | X |
| **Total** | X | X | X | X | **X/100** |

Bands: 0 pristine · 25 healthy · 50 needs attention · 75+ stop and fix.

### Auto-Fixable
**Safe (no logic change):**
- [ ] <mechanical fix, e.g. add missing strict/type declarations in N files>
- [ ] <remove unused imports in N files>
- [ ] <swap debug-print for logger in N locations>

**Needs confirmation (behavior may change):**
- [ ] <narrow a broad catch, verify intended behavior>
- [ ] <remove apparently-dead code, confirm no dynamic dispatch>
- [ ] <add null/None check after lookup, decide the miss behavior>
```

`--debt` also *tightens thresholds*: e.g. file-length flag drops from >500 to >300 lines, function-length from >50 to >30, and checks that are `info` on a normal run escalate to `warn`. Wire those threshold shifts into the agent prompts (a small "`--debt` / `--all` only" subsection under the relevant checks), not just the score.

The safe-vs-needs-confirmation split is load-bearing: it's what lets a downstream `fix` skill batch-apply the safe half without a human in the loop.

---

## 5. `--full` mode: architecture diagrams + health score

Only when `--full` (or `--all`) is set. Skip entirely for projects without a real layered structure.

**Dependency diagram**: scan the import mechanism for the stack (`use` in PHP, `import`/`from` in Python, `import`/`require` in JS/TS, `import` in Go, `use` in Rust) and draw cross-layer edges only (drop same-layer noise):

```markdown
### Component Dependencies
​```mermaid
graph LR
  subgraph Controllers
    BlogController
  end
  subgraph Services
    BlogService
  end
  subgraph Models
    BlogPost
  end
  BlogController --> BlogService
  BlogService --> BlogPost
​```
```

**Layer diagram**: the *allowed* dependency direction. Flag arrows going the wrong way (a violation) with red styling:

```markdown
### Layer Dependencies
​```mermaid
graph TD
  Views --> Controllers
  Controllers --> Services
  Controllers --> Models
  Services --> Models
  Models --> Core
​```
```

The layer names come from Phase 2 (MVC → Views/Controllers/Services/Models; clean/hexagonal → presentation/domain/data; pick what the project actually uses).

**Architecture Health Score**: weighted, project-specific. Weights should sum to 100 and reflect what matters for *this* architecture:

```markdown
### Architecture Health Score: X/100
| Check | Weight | Scoring |
|-|-|-|
| Layer compliance | 30 | -5 per violation |
| Base-class / interface compliance | 20 | -5 per non-compliant unit |
| Boundary pattern (mutations through services/repos) | 20 | -5 per bypass |
| Registration / wiring completeness | 15 | -3 per missing registration |
| File organization / naming | 15 | -3 per misplaced file |
```

---

## 6. Self-learning capture loop (MANDATORY in the generated skill)

A mature review skill doesn't just report: it **records** every confirmed finding to a per-project log so recurring issues can later harden rules and preflight (via `a-self-learner`). Scaffold all three pieces so a from-scratch skill wires the loop, not just the review:

### 6a. Emit the capture script

Write `.claude/scripts/capture-finding.sh` (stack-agnostic, it only appends JSON). Generic version to emit verbatim, adjusting nothing but staying project-neutral:

```bash
#!/usr/bin/env bash
# capture-finding.sh: append one finding to .claude/reviews/review-issues.jsonl
# Call once per confirmed finding. Safe to call from any review skill in any project.
set -euo pipefail

PROJECT=""; SKILL=""; RUN_ID=""; DIMENSION=""; SEVERITY=""
CATEGORY=""; FILE=""; LINE="0"; MESSAGE=""
FIX=""; AGENT=""; CHECK_ID=""; WHITELISTED="false"

while [ $# -gt 0 ]; do
  case "$1" in
    --project) PROJECT="$2"; shift 2 ;;
    --skill) SKILL="$2"; shift 2 ;;
    --run-id) RUN_ID="$2"; shift 2 ;;
    --dimension) DIMENSION="$2"; shift 2 ;;
    --severity) SEVERITY="$2"; shift 2 ;;
    --category) CATEGORY="$2"; shift 2 ;;
    --file) FILE="$2"; shift 2 ;;
    --line) LINE="$2"; shift 2 ;;
    --message) MESSAGE="$2"; shift 2 ;;
    --fix) FIX="$2"; shift 2 ;;
    --agent) AGENT="$2"; shift 2 ;;
    --check-id) CHECK_ID="$2"; shift 2 ;;
    --whitelisted) WHITELISTED="true"; shift ;;
    *) echo "Unknown arg: $1" >&2; exit 2 ;;
  esac
done

for v in PROJECT SKILL RUN_ID DIMENSION SEVERITY CATEGORY FILE MESSAGE; do
  if [ -z "${!v}" ]; then echo "capture-finding.sh: missing required --${v,,}" >&2; exit 2; fi
done

DATE=$(date -u +%Y-%m-%d)
REVIEWS_DIR=".claude/reviews"; LOG="$REVIEWS_DIR/review-issues.jsonl"
mkdir -p "$REVIEWS_DIR"

# category_hash groups findings into pattern classes for a-self-learner.
# The clustering key is the CATEGORY slug alone (lowercased): NOT the free-text
# message and NOT the dimension, either of which fragments one pattern into
# many hashes. Must stay in lockstep with a-self-learner's capture-finding.sh
# and review-log-schema.md "category_hash computation".
HASH=$(printf '%s' "$CATEGORY" | tr '[:upper:]' '[:lower:]' | sha256sum | cut -c1-16)
FINDING_ID=$(head -c 8 /dev/urandom | od -An -tx1 | tr -d ' \n')

python3 - "$DATE" "$PROJECT" "$SKILL" "$RUN_ID" "$FINDING_ID" "$DIMENSION" \
  "$SEVERITY" "$CATEGORY" "$FILE" "$LINE" "$MESSAGE" "$FIX" "$AGENT" \
  "$CHECK_ID" "$HASH" "$WHITELISTED" "$LOG" <<'PY'
import json, sys
(date, project, skill, run_id, finding_id, dimension, severity, category,
 file_, line, message, fix, agent, check_id, category_hash, whitelisted, log) = sys.argv[1:]
rec = {"date": date, "project": project, "skill": skill, "run_id": run_id,
       "finding_id": finding_id, "dimension": dimension, "severity": severity,
       "category": category, "file": file_, "line": int(line or 0),
       "message": message, "category_hash": category_hash,
       "whitelisted": whitelisted == "true"}
if fix: rec["fix_proposed"] = fix
if agent: rec["agent"] = agent
if check_id: rec["check_id"] = check_id
with open(log, "a") as f:
    f.write(json.dumps(rec, separators=(",", ":")) + "\n")
PY
```

### 6b. Emit the capture section in the generated SKILL.md

```markdown
## Capture Findings for Self-Learning (MANDATORY)

After reconciliation, append every CONFIRMED finding (all severities, including INFO)
to `.claude/reviews/review-issues.jsonl` via `.claude/scripts/capture-finding.sh`.
This feeds `/a-self-learner` so recurring issues harden rules/preflight over time.

Generate one `RUN_ID` per review (e.g. `r-YYYYMMDD-HHMM`); reuse it for every finding
in the run. Call once per finding:

​```bash
bash .claude/scripts/capture-finding.sh \
  --project <project> --skill <this-skill> --run-id "$RUN_ID" \
  --dimension <security|architecture|quality|performance> \
  --severity <critical|high|medium|low|info> \
  --category <stable-slug> \
  --file <repo-relative-path> --line <n> \
  --message "<one-line description>" \
  [--fix "<proposed fix>"] [--agent <agent>] [--check-id <preflight id>]
​```

Whitelisted/dismissed findings are NOT captured: only confirmed ones. (Dismissed
findings go to the Context Awareness whitelist instead.)
```

### 6c. Seed the stable category-slug table

The `--category` slug is the clustering key: the **same class of issue must get the same slug every run**, or `a-self-learner` can't see the recurrence. Seed a generic table and let the skill coin project-specific slugs (lowercase kebab-case noun phrases) as new patterns appear:

| Pattern | Slug |
|-|-|
| SQL built by string concatenation/interpolation | `sql-injection-string-concat` |
| Output rendered without escaping (XSS) | `xss-unescaped-output` |
| State-changing endpoint without CSRF check | `missing-csrf-validation` |
| Secret/API key hardcoded in source | `hardcoded-secret` |
| Token/hash compared with `==` not constant-time | `non-constant-time-compare` |
| URL field used without protocol allowlist | `missing-url-protocol-validation` |
| Unsafe deserialization (pickle/unserialize/yaml.load) | `unsafe-deserialization` |
| Subprocess/exec without timeout | `subprocess-without-timeout` |
| Command built with string interpolation | `command-injection-interpolation` |
| Path from input used without containment check | `path-traversal-unchecked` |
| Bare/silent catch that swallows the error | `swallowed-exception` |
| Broad catch with no logging | `broad-catch-no-log` |
| DB/data access outside the data layer | `data-access-outside-layer` |
| Mutation bypassing the service/repository layer | `mutation-bypasses-service` |
| Component/route/view not registered where required | `missing-registration` |
| Check-then-act on the filesystem (TOCTOU) | `toctou-filesystem` |
| Missing null/None check after a lookup | `missing-null-check` |
| Unused import/use statement | `unused-import` |
| Dead private method / unreachable code | `dead-code` |
| Function longer than the project limit | `function-too-long` |
| File longer than the project limit | `file-too-long` |
| Duplicated logic that should be shared | `duplicate-logic` |
| Stringly-typed value where an enum/const exists | `stringly-typed-value` |
| Unbounded query / missing LIMIT | `unbounded-query-no-limit` |
| Query inside a loop (N+1) | `n-plus-one-query` |
| Debug print/log left in source | `debug-statement-left-in` |

---

## 7. Rule-candidate surfacing

When a review keeps finding the same class of issue, the fix is a *rule*, not another finding. Teach the generated skill to surface candidates at the end of every run:

```markdown
## Rule Candidates

A finding is a rule candidate when: the same issue appears in 3+ locations, OR it
reflects an undocumented project convention, OR the fix needs knowledge not obvious
from the code. For each candidate, check `.claude/rules/` (or CLAUDE.md), if it's
not already covered, propose adding it to the right file. Rules are one actionable
statement, no code blocks (use `Reference: path/file.ext` instead).

| Pattern | Occurrences | Proposed Rule | Target File |
|-|-|-|-|
| <recurring pattern> | N | <one-line actionable rule> | <rules file> |

Ask before adding: never write a rule silently.
```

This is the manual, in-the-moment counterpart to `a-self-learner` (which mines the accumulated `review-issues.jsonl` later). Both routes end at `a-rules-optimizer`, which owns the actual rule edit.

---
name: a-rules-optimizer
description: "Audits and optimizes .claude/rules/ files and CLAUDE.md against the actual codebase. Creates missing rules, fixes stale references, removes drift, ensures coverage of review dimensions. Works on any project regardless of stack. Use when the user says 'audit my rules', 'optimize rules', 'fix stale rules', 'create rules', 'sync rules to codebase', 'my rules are out of date', 'rule drift', 'check rules against code', or 'generate rules for this project'. Also use when a .claude/rules/ directory exists but seems out of sync with the code, or when a project has a review skill but no matching rules."
---

# Rules Optimizer

Audit `.claude/rules/` and `CLAUDE.md` against the actual codebase. Creates missing rules, fixes drift, ensures coverage. Works on any project regardless of stack.

**Input:** optional flags
- `--full`: full audit and optimization (default)
- `--audit-only`: report drift without making changes
- `--sync`: just fix stale references and drift, skip coverage analysis

If no `.claude/rules/` directory exists, create it and build rules from scratch. That's the whole point of running this skill.

---

## Core Principles

**Rules are instructions, not code examples.** Each rule is a declarative statement: "do X", "never Y", "Z must always include W". No code blocks longer than a single line. If a pattern needs showing, use `Reference: path/file.py:42`.

**Rules prevent what reviews catch.** Every rule should map to something that would otherwise be flagged during code review. If a rule doesn't prevent a real mistake, it's noise.

**Path scoping saves context.** Rules scoped to `src/web/**/*.py` don't load when editing `src/core/`. Use the narrowest glob that covers the files where the rule matters.

**`@imports` to rule files defeat path-scoping.** If `CLAUDE.md` references a rule file with `@.claude/rules/foo.md`, that file is force-loaded at session start regardless of its `paths:` frontmatter. Always reference rule files by name only ("see `services.md`") so the path scoping the user wrote actually takes effect.

**Rules are pointers, not inventories.** Anything derivable by `ls`, `grep`, or reading one well-known file (base classes, enums, route configs) does not belong in a rule file. Lists of controllers, models, services, helpers, partials, URL routes, directory trees, base-class method signatures, design tokens, all rot the moment the code changes. Replace with one-liners: `Inventory: ls app/src/Models/`, `Methods: read app/src/Core/View.php`, `Routes: app/config/routes.php`. Keep the rule (the "must"/"never"), drop the catalog.

**Preserve the user's voice.** If rules already exist, the user wrote them from experience. Keep their wording, organization, and emphasis. Only change what has a concrete reason (drift, gap, stale reference).

---

## Phase 1: Project Profile

Discover the tech stack, architecture, and patterns. This drives everything else.

### 1a: Stack Detection

Build a project profile. Pick **one** of these two ways:

- **Option 1 (default, preferred):** Dispatch an `Explore` agent with the brief below. Useful when the project is large or you want the analysis to stay out of your own context window.
- **Option 2 (fallback):** If no agent is available, run the commands yourself and summarize the findings.

The brief (paste into the agent's prompt, or walk through it yourself):

```
Analyze the project at [CWD]. Report back concisely:

1. STACK: Language(s), framework(s), runtime version requirements
   - Check: package.json, composer.json, requirements.txt, pyproject.toml, go.mod, Cargo.toml, Gemfile, *.sln
   - Check: Dockerfile, docker-compose.yml if present

2. STRUCTURE: Top-level directory layout. Which directories contain source code vs config vs tests vs assets?
   - Run: find . -maxdepth 3 -type d (excluding .git, node_modules, vendor, __pycache__, venv)

3. ARCHITECTURE PATTERN: How is the code organized?
   - MVC, service-repository, layered, modular, flat, monorepo?
   - Where does business logic live?
   - How do modules communicate?

4. KEY PATTERNS (read the 5 largest source files to discover):
   - Error handling: exceptions, result tuples, error codes?
   - Config access: env vars, YAML, JSON, centralized manager?
   - Data access: ORM, raw queries, repository pattern?
   - State management: framework-specific patterns?
   - Registration: how are routes/views/commands/components wired?
   - Logging: which logger, what patterns?

5. BOUNDARIES: Where does external input enter the system?
   - CLI args, HTTP requests, file uploads, API responses, user config?

6. FILE METRICS:
   - Total source files and lines (by language)
   - Largest files (top 10)
   - Test presence and structure

7. DOMAIN VOCABULARY (drives project-specific rules a generic taxonomy can't):
   - Enums / status fields and the meaning of their values (which value gates a downstream action?)
   - Status machines / lifecycle transitions (draft→published, pending→approved)
   - Distinct subsystems with their own conventions (scraper, consolidator, scorer, feature-flag registry, importer)
   - Cross-entity invariants ("excluded rows never get positive flags", "counts never overwritten on sync")
   - Content-integrity constraints (fields that must not be edited via raw SQL, unique keys, derivation provenance)

Report in structured sections, not prose. Keep under 3000 chars.
```

### 1b: Existing Rules Inventory

If `.claude/rules/` exists, read every rule file. For each one, extract:
- File name and path scope (from frontmatter `paths:` or `alwaysApply:`)
- Section headings (these are the rule categories)
- Individual rules (bullet points under each heading)
- Any file:line references (these need freshness checks)

Build a flat list: `[rule_file, category, rule_text, referenced_paths[]]`

Also read `CLAUDE.md` and extract:
- Sections that describe architecture, patterns, or conventions
- Any references to `.claude/rules/`
- File paths or line numbers mentioned anywhere

### 1c: Review Skill Cross-Reference

If a review skill exists (check `.claude/skills/*/SKILL.md` for review-related skills), read it and extract:
- What checks each agent performs
- What preflight scripts exist and what they scan for
- Known-correct pattern whitelists

This tells you what the review process catches, so rules can prevent those same issues upstream.

If preflight scripts exist (`.claude/scripts/preflight-*.sh`), read them and extract check IDs and what they detect.

---

## Phase 2: Coverage Analysis

Cross-reference what rules exist against what the project needs.

### 2a: Dimension Mapping

Read `references/review-dimensions.md` for the full taxonomy. For each dimension, ask two questions:

1. **Is this dimension relevant to this project?** A CLI tool doesn't need WebSocket rules. A static site doesn't need N+1 query rules. Skip irrelevant dimensions.

2. **Is this dimension covered by existing rules?** Map each relevant dimension to the rule file and bullet point that covers it. Mark uncovered dimensions.

**What "covered" means.** A dimension is covered only when a rule **explicitly forbids or prescribes** the pattern, with a concrete signal (file path, function name, framework primitive), not just a passing mention. "Validate input" alone is not coverage for SQL injection; "All SQL goes through `$pdo->prepare()`; never concatenate `$_POST` into a query string" is. If a rule only names the topic, mark it PARTIAL.

**Derive domain dimensions (do not stop at the fixed taxonomy).** `review-dimensions.md` covers generic engineering concerns (security/architecture/quality/performance). It cannot list the rules that only this codebase needs; those come from the DOMAIN VOCABULARY in the Phase 1 profile. For each item there, add a project-specific dimension row to the matrix and check coverage:

- **Enum/status semantics** → is there a rule stating what each value permits or forbids (which value gates outreach/publishing/deletion), and the `NONE`-vs-`NULL` style distinctions?
- **Status-machine transitions** → is the legal lifecycle written down, and are illegal transitions forbidden?
- **Subsystem invariants** → does each distinct subsystem (scraper, consolidator, importer, feature-flag registry) have a rule file or section capturing its non-obvious contract (SSRF/redirect model, dedup key, sync-safety)?
- **Cross-entity invariants** → is the integrity rule that must hold across tables/modules stated ("excluded rows never get positive flags", "analytics counts never overwritten on import")?
- **Content-integrity constraints** → are the "never edit this field via raw SQL", unique-key, and provenance rules present?

A rule set that covers every generic dimension but none of the domain dimensions is under-covering. These project-specific rules are usually the highest-value ones; they encode knowledge no linter or generic reviewer has. Mark each derived domain dimension Covered / Partial / Gap just like the taxonomy rows.

Output a coverage matrix:

```
COVERAGE MATRIX
===============
Dimension                    | Relevant? | Covered by           | Gap?
SQL injection                | YES       | security.md:Input    | NO
Command injection            | YES       | security.md:Subproc  | NO
Path traversal               | YES       | security.md:File     | NO
XSS                          | YES       | security.md:XSS      | NO
SSRF                         | YES       | security.md:Network  | NO
Cache invalidation           | YES       | web-layer.md:Cache   | NO
N+1 queries                  | NO (no ORM)| -                   | -
Registration/wiring          | YES       | web-layer.md:Reg     | NO
Type safety                  | YES       | code-quality.md:Type | NO
Dead code                    | YES       | code-quality.md:Dead | PARTIAL (no unused import rule)
Atomic file writes           | YES       | (none)               | YES: need file-operations.md
...
```

### 2b: Pattern Scan

Use detection scripts from `references/pattern-detection.md` (pick ones matching the project's stack) to find what actually exists in the codebase. This grounds the analysis in reality rather than theory.

For each pattern found, check: "Is there a rule that would have prevented/guided this?"

### 2c: Drift Detection

**First, check CLAUDE.md for `@imports` that defeat path-scoping.** Run:

```bash
rg -n '^[^`]*@\.claude/rules/' CLAUDE.md 2>/dev/null
```

Any match is a bug: the imported rule file is force-loaded every turn, ignoring its `paths:` frontmatter. Flag each one for removal in Phase 3 (replace the `@path/file.md` reference with a plain `file.md` mention so the path-scoping kicks in).

**Then check inventory bloat.** For each rule file, scan for content that's derivable from the codebase rather than rule content:

- ASCII directory trees (covered by `ls -F`)
- Tables of controllers / models / services / partials / routes (covered by `ls` of the relevant dir)
- Base-class method dumps (covered by reading the base class file)
- Helper function lists (covered by reading `helpers.php` or equivalent)
- URL route enumerations (covered by reading `routes.php` or equivalent)
- Token/color/spacing references (covered by reading the tokens file)

Mark every such block for replacement with a one-line pointer in Phase 3.

**Then existing reference checks.** For every file:line reference in existing rules:
- Does the file still exist?
- Does the referenced line still contain what the rule describes?
- Has the file been moved or renamed?

**Detect probable renames** (file gone but a sibling with a close name exists):

```bash
# For each missing file path referenced by a rule, look for a similar name
# in the same directory. Adjust the basename match to your tolerance.
for ref in $(rg -oN 'Reference:\s*`?([^`\s]+\.(py|php|ts|tsx|js|go|rb|java))' \
                 -r '$1' .claude/rules/ 2>/dev/null | sort -u); do
    if [ ! -f "$ref" ]; then
        dir=$(dirname "$ref")
        base=$(basename "$ref" | sed 's/\.[^.]*$//')
        # Candidates in the same directory whose name shares a token with the old name
        candidates=$(find "$dir" -maxdepth 1 -type f 2>/dev/null \
                     | grep -iE "$(echo "$base" | tr '_-' '|')" || true)
        echo "DRIFT: $ref missing; candidates: ${candidates:-none}"
    fi
done
```

For every pattern described in rules:
- Does the codebase still use this pattern?
- Has the convention changed since the rule was written?

For every path scope in rule frontmatter:
- Do files matching this glob still exist?
- Are there new directories that should be included?

### 2d: Redundancy Check

- Are any rules duplicated across files?
- Do any rules contradict each other?
- Are there rules that duplicate what's already in `CLAUDE.md`?
- Are there rules that duplicate what's in the user's global `~/.claude/CLAUDE.md`?

---

## Phase 3: Generate Changes

Based on the coverage analysis, determine what needs to change. Follow these priorities:

### Priority 1: Fix Drift (accuracy)
- Update stale file:line references
- Remove rules for patterns the project no longer uses
- Update path scopes to match current directory structure

### Priority 2: Restore Path-Scoping (efficiency)
- Replace any `@.claude/rules/foo.md` reference in `CLAUDE.md` with a plain mention (`see services.md`). The `@` prefix forces the file into context every turn and defeats the `paths:` frontmatter the user wrote.
- Verify each rule file actually has `paths:` (or a deliberate `alwaysApply: true`). Add scoping where missing.

### Priority 3: Strip Inventory Bloat (efficiency)
- Replace directory trees with `Layout: ls dir/`.
- Replace controller/model/service/partial/route inventories with `Inventory: ls path/`.
- Replace base-class method dumps with `Read app/path/Base.php for the API`.
- Replace helper/token/route enumerations with a one-line pointer to the source-of-truth file.
- Keep the rule (the "must"/"never"); drop the catalog. Inventories rot the moment files are added or renamed; pointers don't.

### Priority 4: Fill Gaps (completeness)
- Add missing rules for relevant uncovered dimensions
- Prefer adding to existing rule files over creating new ones
- New rules must be written as instructions, not code examples

### Priority 5: Reduce Redundancy (clarity)
- Remove duplicates (keep in the most specific file)
- Collapse near-identical rules into one clear statement
- Remove rules that restate what the framework already enforces
- **Exception: path-visibility duplication is intentional.** Before deleting a "duplicate," check the two files' `paths:` scopes. If the source-of-truth file is scoped to a directory that never loads when editing the file type where the rule is actually violated, the restatement is load-bearing, not noise. (Real case: a "no inline event handlers" rule in `js-standards.md` scoped `public/js/**` never loaded while editing PHP views, where every regression happened; the rule had to be restated in the views-scoped file with a cross-reference.) Keep both; ensure the restatement carries a one-line "source of truth: X" pointer.

### What NOT to Change
- The user's organizational structure (number of files, file names, section headings)
- Working rules that are correctly scoped and accurate
- Project-specific conventions the user documented from experience
- The level of detail the user chose (some projects have dense rules, some have sparse ones)

---

## Phase 4: Apply

### For Existing Rules (`--full` or `--sync`)

Edit rule files using preservation tags:

| Tag | Meaning | Action |
|-|-|-|
| **KEEP** | Accurate, well-scoped, no issues | Don't touch |
| **UPDATE** | Stale reference or path | Minimal targeted edit |
| **ADD** | Missing rule for a real gap | Append to relevant section |
| **REMOVE** | Covers a pattern that no longer exists | Delete the rule |
| **MOVE** | Rule in wrong file or wrong scope | Move to correct file |

For CLAUDE.md:
- Update the rules reference table if rule files were added/removed
- Fix any stale file paths or line numbers
- Don't restructure or rewrite sections that aren't about rules

### For New Rules (no existing `.claude/rules/`)

**First: pick the organization pattern.** Three patterns exist. Default to the first unless you have a specific reason:

| Pattern | When to use | Example files |
|-|-|-|
| **By Domain** (default) | Most projects. Rules grouped around the concern they protect. | `security.md`, `database.md`, `code-quality.md`, `web-layer.md` |
| **By Layer** | Architecture is strictly layered (controllers → services → repositories) and layer-specific rules actually differ. Don't use if "layer" is notional. | `controllers.md`, `services.md`, `repositories.md` |
| **By Platform** | Monorepo with distinct stacks that share a repo (e.g. backend PHP + mobile Dart + web TS). Each stack has its own conventions. | `be-security.md`, `mo-components.md`, `we-architecture.md` + one shared `code-quality.md` |

Pick one and stick to it: don't mix (`security.md` + `controllers.md` + `be-api.md` is noise).

Then determine rule files based on what the project actually needs. Common patterns across stacks:

**Python projects:**
- `security.md`: scoped to `src/**/*.py`
- `error-handling.md`: scoped to `src/**/*.py`
- `code-quality.md`: scoped to `src/**/*.py`
- Framework-specific (e.g., `web-layer.md` for Streamlit/Flask/Django): scoped to web directories
- Domain-specific if the project has distinct subsystems (e.g., `backup-engine.md`, `api.md`)

**PHP projects:**
- `security.md`: scoped to source directories
- `architecture.md`: typically `alwaysApply: true` for service-layer/MVC rules
- `database.md`: scoped to repository/model files
- `php-standards.md`: scoped to all PHP source
- Layer-specific files for each architectural layer (controllers, processors/services, repositories)
- `frontend.md`: scoped to view/template/JS/CSS files

**JavaScript/TypeScript projects:**
- `security.md`: scoped to `src/**/*.{ts,tsx}`
- `architecture.md`: scoped to source directories
- `api-integration.md`: scoped to API/service files
- `components.md`: scoped to component directories
- `state-management.md`: scoped to state/store files

**Multi-platform projects:**
- Prefix with platform: `be-security.md`, `mo-components.md`, `we-architecture.md`
- One cross-platform file for shared rules: `code-quality.md`

Whatever the stack, two things hold for a freshly generated set:

- **Exactly one always-on `security.md`:** either `alwaysApply: true` or scoped to all source. Security review applies to every file; it's the one dimension that shouldn't wait for a path match.
- **Domain rule files derived from the Phase 1 DOMAIN VOCABULARY**, not just the generic stack list above. If the profile surfaced distinct subsystems (scraper, consolidator, scorer, feature-flag registry) or a rich enum/status vocabulary, generate a file (or a dedicated section) per subsystem, scoped to the files that implement it. These are usually the highest-value rules; don't ship a set that's all generic and zero domain.

Write rules as declarative instructions. Each rule is one bullet point. Group related rules under `##` headings. No code blocks longer than a one-liner. Use `Reference: path/file` when showing a pattern would be clearer than describing it. Where a recurring create-flow exists (adding a model/content-type/migration/endpoint), close the relevant file with a numbered "Adding a new X" checklist whose steps point to the sibling rule files; see `references/rule-style-guide.md` "Rule Shapes."

### Review generated rules against anti-patterns

Before declaring Phase 4 done, scan every file you created or touched against the anti-patterns in `references/rule-style-guide.md` (the "Anti-patterns" section near the bottom). Flag and fix any of these:

- **Tutorial file**: prose explaining *what* the framework does instead of *what this project does with it*.
- **Kitchen sink**: one file covers unrelated concerns (security + style + performance mixed together).
- **Aspirational rules**: rules that describe ideal behavior the codebase doesn't actually follow. Either remove or fix the code first.
- **Duplicates**: same rule appears in two files (or restates something already in `CLAUDE.md` or the global `~/.claude/CLAUDE.md`).
- **Stale references**: file paths or line numbers that no longer match the code (drift).
- **Unscoped when it should be scoped**: `alwaysApply: true` on a rule file that only applies to one part of the tree.
- **Inventory dump**: directory trees, lists of controllers/models/services/partials/routes, base-class method signatures, helper indexes, design-token catalogs. Replace with one-line pointers (`Inventory: ls path/`, `Methods: read path/Base.php`).
- **`@import` trap**: `CLAUDE.md` references a rule file with `@.claude/rules/foo.md`. That force-loads it every turn and breaks `paths:` scoping. Replace with a plain mention (`see foo.md`).
- **All-generic, zero-domain.** The set covers security/quality/architecture but encodes none of the project's own enum semantics, status machines, subsystem contracts, or cross-entity invariants (Phase 2a domain dimensions). A rule set a reviewer could have written without reading this codebase is under-covering it.
- **Missing always-on security.** No `security.md`, or it's path-scoped so narrowly it doesn't load on most edits. Security should be the one always-on file.

If you fix any, note them in the Phase 5 report so the user can see why.

After creating rules, add a reference section to CLAUDE.md:

```markdown
## Coding Rules (`.claude/rules/`)

Path-scoped rules auto-loaded when editing matching files.

| Rule file | Scope | Covers |
|-|-|-|
| `security.md` | `src/**/*.py` | Subprocess safety, input validation, secrets, XSS |
| ... | ... | ... |
```

---

## Phase 5: Report

Present the results:

### For `--audit-only`

```
## Rules Audit Report

### Coverage: X/Y relevant dimensions covered (Z%)

### Drift Found
| File | Rule | Issue |
|-|-|-|
| security.md | "ConfigManager handles YAML" | ConfigManager renamed to SettingsManager |
| web-layer.md | path scope `src/web/**` | New directory src/dashboard/ not covered |

### Gaps Found
| Dimension | Severity | Suggested Rule |
|-|-|-|
| Atomic file writes | HIGH | "Critical files must be written atomically via temp + rename" |
| Missing timeouts | MEDIUM | "All subprocess calls must include timeout=" |

### Redundancies
| Rule | Appears in | Keep in |
|-|-|-|
| "Use parameterized queries" | security.md, database.md | database.md |

### Recommendations
- Add file-operations.md with 3 rules for atomic writes, TOCTOU, locking
- Update path scope in web-layer.md to include src/dashboard/
- Remove duplicate SQL injection rule from security.md (covered in database.md)
```

### For `--full` or `--sync` (including new rule creation)

```
## Rules Optimization Report

### Changes Applied

#### Updated Files
| File | Change | Reason |
|-|-|-|
| security.md | Updated 1 stale reference | ConfigManager → SettingsManager at line 15 |
| web-layer.md | Expanded path scope | Added src/dashboard/** to cover new directory |

#### New Files
| File | Scope | Rules added | Covers |
|-|-|-|
| file-operations.md | src/core/**/*.py | 3 rules | Atomic writes, TOCTOU, locking |

#### Removed
| File | Rule | Reason |
|-|-|-|
| security.md | "Use parameterized queries" | Duplicate of database.md rule |

### Coverage After: X/Y relevant dimensions (Z%)

### Review Skill Alignment
- Rules now cover N of M checks from the review skill
- Remaining uncovered checks are runtime-only (can't be prevented by rules)
```

---

## Reference Files

Read as needed during analysis:

| File | When to Read | Contains |
|-|-|-|
| `references/review-dimensions.md` | Phase 2a: dimension mapping | Full taxonomy of review categories |
| `references/pattern-detection.md` | Phase 2b: pattern scanning | Detection scripts per language/stack |
| `references/rule-style-guide.md` | Phase 4: writing new rules | Format, scoping, and style conventions |

# Pattern Detection Script Library

Use this during Phase 2b (Pattern Scan). Pick the scripts relevant to the project's stack.

All scripts are designed to be safe (|| true suffix, head limits on output) and produce file:line locations.

## Universal Detection Scripts

### Credentials & Secrets
```bash
# Hardcoded secret assignments (exclude env/config accessors)
rg -n '(password|api_key|secret_key|api_secret|token|private_key)\s*=\s*["\x27][^"\x27]{4,}' \
  --type-add 'code:*.{py,php,js,ts,rb,go,java}' -t code src/ \
  | grep -v 'getenv\|os.environ\|get_setting\|\.get(' || true

# Files that shouldn't be tracked
git ls-files -- '*.env' '.env.*' '*.key' '*.pem' 2>/dev/null \
  | grep -v '.example' | grep -v '.sample' || true
```

### Error Handling
```bash
# Bare except (Python)
rg -n '^\s*except\s*:' --type py src/ || true

# catch(Exception) or catch(\Exception) (PHP)
rg -n 'catch\s*\(\s*\\?Exception' --type php src/ || true

# Generic catch(e) (JS/TS)
rg -n 'catch\s*\(\s*\w+\s*\)\s*\{' --type js --type ts src/ || true

# Silent exception handlers (Python: except block with only pass/continue)
python3 -c "
import re, pathlib
for f in pathlib.Path('src').rglob('*.py'):
    lines = f.read_text().splitlines()
    for i, line in enumerate(lines):
        if re.match(r'\s*except\b', line):
            # Check next non-blank lines in the except block
            body_lines = []
            base_indent = len(line) - len(line.lstrip())
            for j in range(i+1, min(i+5, len(lines))):
                stripped = lines[j].strip()
                indent = len(lines[j]) - len(lines[j].lstrip())
                if indent <= base_indent and stripped: break
                if stripped: body_lines.append(stripped)
            if body_lines and all(b in ('pass', 'continue', '...') for b in body_lines):
                print(f'{f}:{i+1}: silent except: body is only {body_lines[0]}')
" 2>/dev/null || true
```

### Code Metrics
```bash
# Files over 300 lines
find src/ app/ lib/ -name '*.py' -o -name '*.php' -o -name '*.ts' -o -name '*.js' 2>/dev/null \
  | xargs wc -l 2>/dev/null | awk '$1 > 300 && !/total$/' | sort -rn || true

# Deep nesting (5+ levels = 20+ leading spaces)
rg -n '^\s{20,}\S' --type-add 'code:*.{py,php,js,ts}' -t code src/ | head -20 || true

# Long functions (Python: def to next def at same/lower indent, >50 lines)
python3 -c "
import re, pathlib
for f in pathlib.Path('src').rglob('*.py'):
    lines = f.read_text().splitlines()
    func_start = None
    func_name = ''
    func_indent = 0
    for i, line in enumerate(lines):
        m = re.match(r'^(\s*)def\s+(\w+)', line)
        if m:
            if func_start and (i - func_start) > 50:
                print(f'{f}:{func_start+1}: {func_name}() is {i - func_start} lines')
            func_start = i
            func_name = m.group(2)
            func_indent = len(m.group(1))
    if func_start and (len(lines) - func_start) > 50:
        print(f'{f}:{func_start+1}: {func_name}() is {len(lines) - func_start} lines')
" 2>/dev/null || true

# print/var_dump/console.log in production code
rg -n '\bprint\(' --type py src/ --glob '!*test*' --glob '!*__pycache__*' 2>/dev/null | head -20 || true
rg -n '\bvar_dump\(|\bdd\(' --type php src/ 2>/dev/null | head -20 || true
rg -n '\bconsole\.(log|debug)\(' --type js --type ts src/ --glob '!*test*' 2>/dev/null | head -20 || true

# Commented-out code (3+ consecutive comment lines with code patterns)
python3 -c "
import re, pathlib
code_pattern = re.compile(r'#\s*(def |class |import |return |if |for |while |print|self\.|=\s)')
for f in pathlib.Path('src').rglob('*.py'):
    lines = f.read_text().splitlines()
    streak = 0
    streak_start = 0
    for i, line in enumerate(lines):
        if code_pattern.match(line.strip()):
            if streak == 0: streak_start = i
            streak += 1
        else:
            if streak >= 3:
                print(f'{f}:{streak_start+1}: {streak} lines of commented-out code')
            streak = 0
    if streak >= 3:
        print(f'{f}:{streak_start+1}: {streak} lines of commented-out code')
" 2>/dev/null || true
```

## Python-Specific Detection

### Security
```bash
# shell=True
rg -n 'shell\s*=\s*True' --type py src/ || true

# os.system()
rg -n 'os\.system\(' --type py src/ || true

# Unsafe deserialization
rg -n 'pickle\.loads?\(' --type py src/ || true
rg -n 'yaml\.load\(' --type py src/ | grep -v SafeLoader || true
rg -n '\beval\s*\(' --type py src/ | grep -v 'ast.literal_eval' || true
```

### Subprocess (multi-line aware)
```bash
# subprocess without timeout: MUST use Python for multi-line detection
python3 -c "
import re, pathlib
for f in pathlib.Path('src').rglob('*.py'):
    text = f.read_text()
    for m in re.finditer(r'subprocess\.(run|call|check_output|check_call|Popen)\s*\(', text):
        start = m.start()
        depth, pos = 1, m.end()
        while pos < len(text) and depth > 0:
            if text[pos] == '(': depth += 1
            elif text[pos] == ')': depth -= 1
            pos += 1
        call_text = text[m.start():pos]
        if 'timeout' not in call_text:
            line_num = text[:start].count('\n') + 1
            print(f'{f}:{line_num}: subprocess.{m.group(1)}() without timeout=')
" 2>/dev/null || true

# String interpolation in subprocess args
python3 -c "
import re, pathlib
for f in pathlib.Path('src').rglob('*.py'):
    text = f.read_text()
    for m in re.finditer(r'subprocess\.\w+\s*\(', text):
        start = m.start()
        depth, pos = 1, m.end()
        while pos < len(text) and depth > 0:
            if text[pos] == '(': depth += 1
            elif text[pos] == ')': depth -= 1
            pos += 1
        call_text = text[m.start():pos]
        if re.search(r'f[\"\\x27]|\.format\(|%\s', call_text) and 'shell' not in call_text:
            line_num = text[:start].count('\n') + 1
            print(f'{f}:{line_num}: f-string/format in subprocess args')
" 2>/dev/null || true
```

### Type Modernization (Python 3.10+)
```bash
# Old typing imports
rg -n 'from typing import.*(Optional|List|Dict|Tuple|Set|Union)' --type py src/ || true

# Public functions without return type
python3 -c "
import re, pathlib
for f in pathlib.Path('src').rglob('*.py'):
    for i, line in enumerate(f.read_text().splitlines(), 1):
        if re.match(r'^(\s{0,8})def\s+(?!_|test_)\w+\(.*\)\s*:', line) and '->' not in line:
            print(f'{f}:{i}: {line.strip()[:80]}')
" 2>/dev/null | head -30 || true

# Unsafe nested dict access on external data
rg -n '\[.+\]\[.+\]' --type py src/ | grep -v 'test' | head -20 || true
```

### Streamlit-Specific
```bash
# st.rerun() without invalidate() in preceding N lines
python3 -c "
import pathlib
for f in pathlib.Path('src/web').rglob('*.py') if pathlib.Path('src/web').exists() else []:
    lines = f.read_text().splitlines()
    for i, line in enumerate(lines):
        if 'st.rerun()' in line:
            window = lines[max(0,i-10):i]
            if not any('invalidate' in w for w in window):
                print(f'{f}:{i+1}: st.rerun() without invalidate() in preceding 10 lines')
" 2>/dev/null || true

# @st.cache_data without TTL
rg -n '@st\.cache_data' --type py src/ | grep -v 'ttl' || true

# print() in web layer
rg -n '\bprint\(' --type py src/web/ 2>/dev/null | grep -v 'console' || true
```

## PHP-Specific Detection

### Security
```bash
# SQL with string interpolation
rg -n '(query|execute|prepare)\s*\(.*[\$"]' --type php src/ \
  | grep -v 'prepare.*?\?' | grep -v bindParam || true

# Shell execution functions
rg -n '\b(exec|system|shell_exec|passthru|proc_open|popen)\s*\(' --type php src/ || true

# File operations with variables (path traversal risk)
rg -n '(include|require|file_get_contents|fopen|unlink|rmdir)\s*\(\s*\$' --type php src/ || true

# echo/print without escaping
rg -n '(echo|print)\s+\$' --type php src/ | grep -v 'htmlspecialchars\|htmlentities' | head -20 || true

# unserialize on potentially untrusted data
rg -n 'unserialize\s*\(' --type php src/ || true
```

### Architecture
```bash
# env() outside config files (Laravel: returns null when cached)
rg -n '\benv\(' --type php src/ --glob '!config/*' 2>/dev/null | head -20 || true

# Missing $fillable/$guarded on Eloquent models
for f in $(find src/ app/ -name '*.php' 2>/dev/null | xargs grep -l 'extends Model' 2>/dev/null); do
  grep -L 'fillable\|guarded' "$f" && echo "$f: missing \$fillable/\$guarded"
done 2>/dev/null || true
```

### Modernization
```bash
# Legacy array() syntax
rg -n '\barray\s*\(' --type php src/ | head -20 || true

# strpos instead of str_contains (PHP 8.0+)
rg -n 'strpos\s*\(' --type php src/ | head -20 || true
```

## JavaScript/TypeScript-Specific Detection

### Security
```bash
# XSS vectors
rg -n 'innerHTML|outerHTML|document\.write|dangerouslySetInnerHTML|v-html' --type js --type ts src/ || true

# eval / Function constructor
rg -n '\beval\s*\(|new\s+Function\s*\(' --type js --type ts src/ || true

# Hardcoded JWT secrets
rg -n 'jwt\.(sign|verify)\s*\(' --type js --type ts src/ | head -10 || true
```

### TypeScript Quality
```bash
# any type usage
rg -n ':\s*any\b' --type ts src/ | head -20 || true

# Type assertions (hiding real errors)
rg -n '\bas\s+\w' --type ts src/ | head -20 || true

# @ts-ignore without explanation
rg -n '@ts-ignore|@ts-expect-error' --type ts src/ || true

# Non-null assertions
rg -n '\w+!' --type ts src/ | grep -v '!=\|!=' | head -20 || true
```

### React Patterns
```bash
# useEffect without cleanup (missing return in useEffect callback)
# Approximate: agents should verify
rg -n 'useEffect\(' --type ts --type js src/ | head -20 || true

# Missing key prop indicator (map without key)
rg -n '\.map\(' --type ts --type js src/ | head -20 || true

# Large components (>200 lines)
find src/ -name '*.tsx' -o -name '*.jsx' 2>/dev/null | xargs wc -l 2>/dev/null \
  | awk '$1 > 200 && !/total$/' | sort -rn || true
```

### Node.js
```bash
# Sync file operations in non-config code
rg -n 'readFileSync|writeFileSync|existsSync' --type js --type ts src/ \
  | grep -v 'config\|setup\|init' | head -20 || true

# Missing error handling middleware (Express)
rg -n 'app\.(get|post|put|delete|patch)\(' --type js --type ts src/ | head -10 || true
rg -n 'err,\s*req,\s*res,\s*next' --type js --type ts src/ || true
```

## Cross-Reference Detection

These checks compare two sources of truth. They're project-specific by nature: the optimizer should generate them based on what registration patterns the project uses.

### Registration Completeness Template
```bash
# Template: check that all X are registered in Y
# Adapt the patterns to the project's registration mechanism

# Example: Python views registered in PAGE_MAP
python3 -c "
import pathlib, re
view_dir = pathlib.Path('src/web/views')
init_file = view_dir / '__init__.py'
if not init_file.exists(): exit()
init_text = init_file.read_text()
for f in view_dir.glob('*.py'):
    if f.name.startswith('_'): continue
    for m in re.finditer(r'def (render_\w+)', f.read_text()):
        if m.group(1) not in init_text:
            print(f'{f}:{0}: {m.group(1)} not registered in PAGE_MAP')
" 2>/dev/null || true

# Example: PHP routes vs controllers
# Example: React pages vs router config
# Example: CLI commands vs command group registration
```

### Export Completeness Template
```bash
# Template: check that modules export what they define
# Example: Python __init__.py exports
python3 -c "
import pathlib, re
pkg = pathlib.Path('src/web/components')
init = pkg / '__init__.py'
if not init.exists(): exit()
init_text = init.read_text()
for f in pkg.glob('*.py'):
    if f.name.startswith('_'): continue
    for m in re.finditer(r'^def (\w+)|^class (\w+)', f.read_text(), re.MULTILINE):
        name = m.group(1) or m.group(2)
        if name and not name.startswith('_') and name not in init_text:
            print(f'{f}: {name} not exported in __init__.py')
" 2>/dev/null || true
```

# Review Dimensions Taxonomy

Use this during Phase 2a (Dimension Mapping) to identify what the existing rules are missing. For each dimension, check whether the existing rules cover it AND whether the project actually needs it.

Not every project needs every dimension. A CLI tool doesn't need "WebSocket security." A static site doesn't need "N+1 query detection." Skip what's irrelevant, flag what's missing and needed.

## Security Dimensions

### Input Handling
- SQL injection (parameterized queries, ORM safety)
- Command injection (subprocess, exec, system calls)
- Path traversal (filesystem operations with user input)
- XSS (output encoding, template escaping)
- SSRF (URL construction from user input)
- Deserialization (pickle, unserialize, YAML load, eval)
- ReDoS (catastrophic regex backtracking on user input)
- Header injection (CRLF in user-controlled headers)
- Template injection (SSTI in Jinja2, Twig, EJS)

### Credentials & Secrets
- Hardcoded passwords, API keys, tokens in source
- Secrets in logs, error messages, or stack traces
- Secrets committed to git (even in history)
- Encryption keys with wrong file permissions
- Default/weak passwords in config examples
- Credentials in URLs (basic auth in connection strings)

### Authentication & Authorization
- Missing auth checks on endpoints/routes
- Auth without authz (logged in but not permitted)
- Session fixation (no regeneration after login)
- CSRF protection on state-changing requests
- JWT: hardcoded secret, missing expiry, alg:none
- Cookie flags: httpOnly, secure, sameSite
- Timing attacks on password/token comparison
- Password hashing: bcrypt/argon2 vs MD5/SHA1

### Data Protection
- Sensitive data in localStorage/sessionStorage
- PII in logs or analytics
- Missing encryption at rest for sensitive data
- Broad CORS configuration with credentials
- Missing rate limiting on auth endpoints

## Architecture Dimensions

### Configuration Management
- Hardcoded values that should be configurable
- Config access scattered vs centralized
- Missing defaults for optional config
- Environment-specific logic in business code
- Secrets mixed with non-secret config

### Error Handling Strategy
- Generic catch-all exceptions
- Swallowed exceptions (catch + pass/ignore)
- Missing cleanup on failure (partial state)
- Missing timeouts on external calls
- Error messages that don't help diagnose
- Inconsistent error return shapes
- Missing retry logic with backoff where needed

### State Management
- Framework-appropriate state patterns (session_state, context, store)
- Mutable shared state without synchronization
- State scattered across globals
- Cache invalidation after mutations
- Stale state after redirects/reruns

### Dependency & Coupling
- Circular imports/dependencies
- Layer violations (presentation ↔ data)
- God objects everything depends on
- Components reaching into other components' internals
- Business logic in infrastructure code

### Registration & Wiring
- Routes/views/commands registered correctly
- Middleware/interceptors applied where needed
- Event listeners registered for dispatched events
- Components exported from package indexes
- DI container bindings for all interfaces

### API Design
- Consistent response shapes
- Input validation at boundaries
- Versioning strategy for external APIs
- Batch operations where N individual calls exist
- Pagination on list endpoints

### Consistency
- Same pattern used differently across files
- Naming conventions followed/violated
- Import ordering convention
- File/directory organization convention

## Quality Dimensions

### Type Safety
- Missing type annotations on public interfaces
- Nullable access without null check
- Generic "any"/"mixed" where specific types work
- Type assertions hiding real type errors
- Dict/array access on external data without .get()
- Inconsistent use of type aliases

### Dead Code
- Functions never called from any module
- Unused imports
- Commented-out code blocks (>3 lines)
- Unreachable code after return/throw
- Feature flags always true/false
- Config keys defined but never accessed
- Event handlers never triggered
- Store actions never dispatched

### Complexity
- Files over 300 lines
- Functions over 50 lines
- Nesting over 4 levels
- Cyclomatic complexity over 10
- Parameter count over 5
- Boolean parameters (should be named/enum)

### Duplication
- Same logic in 2+ places
- Similar error handling repeated
- Similar validation in multiple endpoints
- Copy-pasted data transformations
- Similar test setup across test files

### Modernization
- Outdated syntax for the language version
- Old library APIs when newer alternatives exist
- Manual implementations of things the stdlib handles
- Deprecated function/method usage

### Naming
- Misleading names (function does more than name suggests)
- Boolean variables not starting with is/has/can/should
- Unclear abbreviations
- Inconsistent naming convention within a module

### Test Coverage
- Public methods with complex logic but no tests
- Edge cases not covered (empty input, null, overflow)
- Error paths not tested
- Integration points not tested
- Tests that only test the happy path

## Performance Dimensions

### Query Efficiency
- N+1 queries (loop + query pattern)
- SELECT * when specific columns suffice
- Missing LIMIT on unbounded queries
- Filtering/sorting in app instead of DB
- Missing indexes on filtered/joined columns
- Missing connection pooling
- Repeated identical queries per request

### I/O Efficiency
- File/HTTP reads inside loops
- Synchronous I/O in async context
- Loading full files when streaming works
- Not closing handles/connections (resource leaks)
- Missing batching on external API calls

### Caching
- Same expensive computation repeated per request
- Rarely-changing data fetched every time
- Missing cache invalidation after writes
- Cache keys not accounting for all parameters

### Memory
- Full dataset loaded when pagination/chunking works
- String concatenation in loops (vs join/builder)
- Large objects retained when no longer needed
- Missing generators/iterators for large sequences

### Algorithmic
- O(n²) when O(n) or O(n log n) is possible
- Linear search where hash/set lookup works
- Repeated sorts on same data
- Unnecessary serialization/deserialization cycles

### Response/Payload
- API returning more data than consumer needs
- Missing compression on large responses
- Missing pagination
- Full page re-render when partial update suffices

## AI Slop Dimensions (optional, for --slop equivalent)

### Over-Abstraction
- Interface with exactly one implementation
- Factory for something instantiated once
- Strategy with one strategy
- Wrapper that adds no behavior

### Premature Generalization
- Config options no code path exercises
- Parameters always called with same value
- Plugin systems with zero plugins

### Confident-But-Wrong
- Error handling that doesn't actually handle anything
- Retry without idempotency consideration
- Pagination that breaks on last page
- Auth checking login but not permissions

### Copy-Paste Artifacts
- Variable names from a different context
- Comments describing what code used to do
- Imports for unused libraries
- Exception types from wrong framework

# Rule Style Guide

How to write `.claude/rules/` files that are effective, concise, and correctly scoped.

## File Structure

Every rule file starts with YAML frontmatter for scoping, then markdown content.

### Path-Scoped (most files)
```yaml
---
paths:
  - "src/**/*.py"
  - "lib/**/*.py"
---
```

### Always-Applied (rare, for project-wide architecture rules)
```yaml
---
alwaysApply: true
---
```

Use `alwaysApply` only for rules that genuinely matter regardless of which file is being edited (e.g., architecture layer rules, fundamental conventions). Default to `paths:` scoping.

### Additional frontmatter notes

- **Two `paths:` forms are equally valid:** the block-list form above and an inline comma-separated form (`paths: app/**/*.php, public/index.php, composer.json`). They behave identically. Match whichever the existing rule set already uses; don't rewrite one into the other during an audit.
- **`paths:` entries can be exact files, not just globs.** A subsystem rule scopes cleanly to the handful of files that implement it: `paths: src/consolidator.py`, or `paths: app/src/Core/FeatureFlags.php, app/src/Services/OAuthService.php, .env`. The rule loads exactly when those files are edited and stays out of context otherwise.
- **`security.md` is the canonical always-on file.** Security review applies to every source file, so mark it `alwaysApply: true` or scope it to all source (`src/**/*.py`, `app/**/*.php`). Every mature rule set has exactly one always-on security file.

## Scoping Guidelines

| Pattern | When to use |
|-|-|
| `src/**/*.py` | Language-wide rules (security, error handling, quality) |
| `src/web/**/*.py` | Framework-specific rules (Streamlit, Flask, Django) |
| `src/core/**/*.py` | Domain-specific rules (backup engine, data processing) |
| `backend/**/*.php` | Platform rules in a monorepo |
| `tests/**/*.py` | Test-specific conventions |

**Too broad:** `**/*.py` loads on test files, scripts, config generators. Scope to where source code actually lives.

**Too narrow:** `src/core/backup/engine.py` only loads for one file. Broaden to `src/core/**/*.py` unless the rule truly applies to only that file.

**When single-file scope IS right:** the "unless it truly applies to only that file" case is common, since a rule governing one subsystem/module (a scraper, a consolidator, a dashboard, a feature-flag registry) should scope to the single file (or small set) that implements it. Only broaden a single-file scope when the rule is really a language-wide concern mis-filed under one file: a generic "catch specific exceptions" rule pinned to `engine.py` belongs in the language-wide error-handling file instead.

**Multi-pattern:** Use when rules apply to related but separate directories:
```yaml
paths:
  - "src/core/**/*.py"
  - "src/utils/**/*.py"
```

## Writing Rules

### Format

```markdown
## Section Heading

- Rule statement as a declarative instruction
- Another rule, direct and specific
- Rule with rationale: "Never do X because Y" (only when the reason isn't obvious)
```

### Style

**DO:**
- Write imperative statements: "Always set timeout= on subprocess calls"
- Be specific: "Use `html.escape()` before embedding in `unsafe_allow_html=True`"
- Group related rules under `##` headings
- One concern per bullet point
- Reference files when a pattern is project-specific: `Reference: src/core/config_manager.py`

**DON'T:**
- Write code blocks longer than one line (this is a rule file, not a tutorial)
- Explain how the language works ("In Python, exceptions are...")
- Include examples of correct code (describe the rule, don't demonstrate it)
- Duplicate rules across files (put each rule in the most specific file)
- Restate what the language/framework already enforces
- Use passive voice ("Exceptions should be caught" → "Catch specific exception types")

### Granularity

A good rule is:
- Actionable in the moment of writing code
- Specific enough that you can tell if code violates it
- General enough to apply to more than one line of code

**Too vague:** "Write secure code"
**Too specific:** "Line 42 of engine.py must use parameterized queries"
**Right level:** "All YAML operations go through ConfigManager, never raw yaml.load()"

### When to Include Rationale

Most rules don't need a "because" clause. Include one only when:
- The rule is counterintuitive ("Allow direct Repository access from Controllers for reads, because Processor overhead isn't justified for simple GETs")
- The rule has project-specific history ("Never use offset-based pagination in the validator, because validated rows drop out of the result set")
- Violating the rule causes a non-obvious failure ("Write metadata JSON atomically via temp+rename, because a crash mid-write corrupts the file")

## Rule Shapes Beyond the One-Line "Do X / Never Y"

Most rules are single declarative bullets. Mature rule sets also use two other shapes; recognize and preserve them:

### Procedural create-flow checklist

A numbered "Adding a new X" list that enumerates the must-not-skip steps for a recurring create-flow (a new model, content type, migration, feature, endpoint). This IS a rule (it prevents the "forgot step 4" class of mistake), and it belongs at the bottom of the relevant rule file. Each step points to the sibling rule/file that governs it. Keep the steps; don't inline the details of each (those live in the sibling file).

Example shape (not literal content): `Adding a new content type: 1) add route + feature flag, 2) create model (see models.md), 3) create category table, 4) add pivot table, 5) add Tag methods, 6) update this doc.` Derive the actual steps from how the codebase's existing entities were wired.

### Domain / business-invariant rule

Rules that encode facts a generic engineering taxonomy can't know: which enum value blocks a downstream action, what a status transition means, the integrity constraint that must hold across entities ("excluded records never receive positive flags"), the recommended count bound ("3-7 tags per item"), the SSRF/redirect model a fetcher must follow. These come from the project's enums, status machines, subsystem boundaries, and cross-entity invariants, not from the review-dimensions taxonomy. Every mature project has a handful of these; a rule set without any is almost certainly under-covering the domain. See SKILL.md Phase 2a "Derive domain dimensions."

## Organization Patterns

### By Domain (recommended for most projects)
```
rules/
├── security.md          # Input validation, secrets, injection prevention
├── error-handling.md    # Exception patterns, logging, failure modes
├── code-quality.md      # Size limits, types, dead code, performance
├── database.md          # Query safety, access patterns, schema conventions
├── web-layer.md         # Framework-specific UI patterns
└── architecture.md      # Layer rules, module boundaries, conventions
```

### By Architectural Layer (for strict layered architectures)
```
rules/
├── controllers.md       # HTTP layer rules
├── processors.md        # Business logic layer rules
├── repositories.md      # Data access layer rules
├── security.md          # Cross-cutting security rules
└── php-standards.md     # Language conventions
```

### By Platform (for monorepos)
```
rules/
├── be-security.md       # Backend security
├── be-database.md       # Backend data access
├── mo-components.md     # Mobile component rules
├── mo-state.md          # Mobile state management
├── we-architecture.md   # Web frontend architecture
└── code-quality.md      # Cross-platform quality rules
```

## CLAUDE.md Integration

When rules exist, CLAUDE.md should reference them with a summary table:

```markdown
## Coding Rules (`.claude/rules/`)

Path-scoped rules auto-loaded when editing matching files.

| Rule file | Scope | Covers |
|-|-|-|
| `security.md` | `src/**/*.py` | Subprocess safety, input validation, secrets |
| `web-layer.md` | `src/web/**/*.py` | Cache invalidation, rerun discipline |
```

Keep it to a one-line description per file. The rules themselves have the detail.

**Critical: never use `@` import syntax for rule files in CLAUDE.md.** Writing `@.claude/rules/security.md` force-loads that file at session start, every turn, regardless of its `paths:` frontmatter. The whole point of `paths:` scoping is on-demand loading; `@imports` defeat it. Reference rule files by name only (`see security.md`, or in a table as above).

## Anti-Patterns

**The tutorial file:** A rule file that explains how to use the framework, with code examples and documentation. Rules files are instructions, not onboarding docs.

**The kitchen sink:** One file with 100+ rules covering everything. Split by domain and scope properly.

**The aspirational rule:** Rules for patterns the project doesn't use yet. Only write rules for things that exist or are about to be built.

**The duplicate rule:** Same rule in security.md and database.md. Keep each rule in exactly one file. *One deliberate exception:* when a rule's source-of-truth file is path-scoped to a directory the rule *also* needs to govern from a different file type, the scoping hides it. Real case: a "no inline event handlers" rule lived in `js-standards.md` (scoped `public/js/**`), but the regressions all happened in PHP view files, where that file never loads. The fix is to *restate* the rule in the in-scope file (`views.md`, scoped to the views dir) with a one-line cross-reference to the source of truth: deliberate duplication for path-visibility. Only do this when path-scoping genuinely hides the rule from where violations occur; don't use it as license to copy rules around freely.

**The stale reference:** `Reference: src/old_module.py:42` pointing to a file that was renamed 3 months ago. References need freshness checks.

**The inventory dump:** A rule file padded with content that's derivable from the codebase: directory trees, lists of controllers/models/services/partials, URL route enumerations, base-class method signatures, helper function indexes, design-token catalogs. These rot the moment a file is added or renamed and contribute zero rule signal. Replace with one-line pointers: `Inventory: ls app/src/Models/`, `Methods: read app/src/Core/View.php`, `Routes: app/config/routes.php`, `Tokens: public/css/tokens.css`. Keep the rule (the "must"/"never"); drop the catalog.

*Carve-out: enum/status semantics are not inventory.* The *list* of enum values is derivable (`SHOW COLUMNS`, read the enum class, use a pointer). The *meaning attached to each value* is not: "which `email_tier` values are outreach-eligible," "`NONE` means checked-and-empty vs `NULL` means not-yet-evaluated," "archived tools stay publicly linkable but drafts don't." That semantic layer is load-bearing domain knowledge no schema query returns; keep it. Drop only the mechanically-derivable column list; preserve the invariants and eligibility rules bolted to the values.

**The `@import` trap:** Referencing a rule file from `CLAUDE.md` with `@.claude/rules/foo.md` force-loads it every turn and overrides whatever `paths:` scoping the file declared. Always reference rule files by name only.

---
name: a-self-learner
description: "Turns past review findings into preventive rule/skill updates. Ingests .claude/reviews/review-issues.jsonl (per project) plus project memory feedback files, groups recurring issues, and delegates proposed updates to a-rules-optimizer or a-review-optimizer. Use when the user says 'self-learn', 'analyze review history', 'recurring issues', 'propose rule updates from reviews', 'learn from past reviews', 'close the feedback loop', or when they want rules/skills to harden based on what past reviews keep catching. The skill proposes: the user must approve each change before any file is written or committed (per global CLAUDE.md per-commit-ask rule)."
---

# Self-Learner: Review → Rule/Skill Feedback Loop

Closes the loop between review skills and rule/skill files. When a project review catches the same class of issue repeatedly, this skill proposes a preventive update (a new rule, a new preflight check, or a whitelist entry) and hands it off to `a-rules-optimizer` or `a-review-optimizer` for application.

**Input:** optional flags
- `--dry-run` (default): analyze and propose, never write
- `--apply`: after each proposal, ask user, apply on approval
- `--since YYYY-MM-DD`: only consider findings from this date forward
- `--threshold N`: override recurrence threshold (default 3 for recurring, 5 for chronic)

If the project has no `.claude/reviews/` directory yet, the skill reports "no history to learn from" and offers to scaffold the convention (see `references/review-log-schema.md`).

---

## Core Principles

**Propose, never silently write.** Every rule/skill change must be shown to the user as a diff with rationale, then applied only after explicit approval. The global `~/.claude/CLAUDE.md` rule ("never commit without explicit per-commit ask") applies here with full force: a proposal workflow is not commit authorization.

**Delegate writes to the other optimizers.** This skill never edits rule files or review SKILL.md directly. It generates a structured proposal and invokes `a-rules-optimizer` (for rule writes) or `a-review-optimizer` (for preflight/agent writes). Keeps each skill's scope tight.

**Evidence > opinion.** A recurrence claim needs ≥ N findings of the same category across distinct dates. "Feels recurring" is not good enough; the cluster must survive the grouping algorithm in `references/recurrence-detection.md`.

**Record rejections.** Proposals the user declines get logged to `rejected-proposals.md` with the reason. Next run shouldn't re-raise the same rejection: only surface if new evidence appears (e.g., the issue recurred another 5 times since rejection).

**Per-project scope.** Each project owns its own `.claude/reviews/` directory. The skill does not cross-pollinate learnings between projects (that's a future extension; for now, deliberate isolation keeps project conventions separate).

---

## Phase 1: Ingest

Read the signals. The skill needs data before it can cluster anything.

### 1a: Review log

```bash
test -f .claude/reviews/review-issues.jsonl && wc -l .claude/reviews/review-issues.jsonl
test -f .claude/reviews/review-issues-archive.jsonl && wc -l .claude/reviews/review-issues-archive.jsonl
```

If neither exists, this project hasn't been capturing findings yet. Stop here and report:

> No review history found at `.claude/reviews/review-issues.jsonl`. To enable self-learning, add capture calls to this project's `*-review` skill (see `references/review-log-schema.md` for the format and `references/capture-finding.sh` for the helper). Until then, there's nothing for this skill to learn from.

**Optionally, offer to scaffold the convention** so the user doesn't have to wire it by hand. If they accept, this is the one thing `a-self-learner` writes directly (inert plumbing, not a rule/skill change; still show what lands and get an explicit yes first):

1. **Drop the helper.** Copy `references/capture-finding.sh` to `.claude/scripts/capture-finding.sh` and `chmod +x` it. It creates `.claude/reviews/` on first call, computes `category_hash`, and appends one JSON line per finding.
2. **Seed the ledgers.** Create `.claude/reviews/` with empty `applied-learnings.md` and `rejected-proposals.md` (just their `#` header lines from `references/review-log-schema.md`). The `review-issues.jsonl` and `review-issues-archive.jsonl` files appear on the first capture and first archive respectively; don't pre-create them.
3. **Git posture.** These files are meant to be tracked per-project (they document how defenses evolved). Do NOT gitignore them.
4. **Wire the capture calls (delegate, don't hand-edit).** The review skill's SKILL.md must call `capture-finding.sh` once per confirmed finding. Editing that review skill is `a-review-optimizer`'s job, not this skill's. Emit a proposal for `a-review-optimizer`: "add a capture-finding.sh call per confirmed finding, passing `--project/--skill/--run-id/--dimension/--severity/--category/--file/--line/--message`; slug the `--category` as one-pattern-one-slug (see `references/review-log-schema.md` slug hygiene)." Route it through the normal Phase 4 approval gate. If no `*-review` skill exists yet, tell the user to create one (via `a-review-optimizer`) first, since capture has nothing to hook into otherwise.

Either way, stop for this run: even scaffolded, the log is empty until the next review populates it. Re-run after findings accumulate.

If the file exists, read every line as JSON. Drop malformed lines with a warning (don't silently discard, print them so the user can fix the capture).

### 1b: Project memory

Also ingest:

- Project `MEMORY.md` "Review History" section, if present (existing convention in some projects, surface debt scores and round summaries).
- `.claude/projects/<project-id>/memory/feedback_fp_*.md`: documented false positives. These become the KEEP-THIS-WHITELISTED signal for Phase 3.
- `.claude/projects/<project-id>/memory/feedback_review_*.md`: severity / priority feedback (e.g. "deprioritize X for this threat model").

### 1c: Existing review & rules context

- Read the project's `*-review` SKILL.md (whichever skill writes to the log). Extract: agent names, current check IDs, known-correct pattern whitelists.
- Read `.claude/scripts/preflight*.sh` if present. Extract check IDs.
- Read `.claude/rules/` index.

Knowing what's already checked prevents proposing duplicates.

### 1d: Previously processed

Read `.claude/reviews/applied-learnings.md` and `rejected-proposals.md` if present. Cluster IDs already applied or rejected don't need re-proposing unless new evidence arrived.

---

## Phase 2: Cluster

Group findings into patterns. See `references/recurrence-detection.md` for the full algorithm. Summary:

1. **Group** by `category_hash`: precomputed by the capture helper as `sha256(category)[:16]`, a pure function of the stable `category` slug. One cluster per category.
2. The hash deliberately excludes the free-text `message` (method names / paths vary per finding) and the `dimension` (the same pattern gets tagged differently by different agents): including either one fragmented a single pattern into many hashes. See `references/review-log-schema.md`.
3. **Count** occurrences per hash across all ingested findings.
4. **Classify**:
   - `Recurring`: ≥ 3 occurrences across ≥ 2 distinct dates
   - `Chronic`: ≥ 5 occurrences OR spans > 30 days OR in ≥ 3 distinct files
   - `False Positive`: user dismissed ≥ 2 times (from feedback_fp_* or explicit rejection)
   - `One-off`: everything else; ignore
5. **Attribute** each cluster to a review dimension (security / architecture / quality / performance). For a cluster whose findings carry more than one dimension (~4% of categories drift this way), pick the dominant one (most frequent, ties broken by highest severity). Determines which optimizer gets the proposal.

Between grouping (2) and counting (3), run the **near-duplicate slug gate** (`recurrence-detection.md` Step 2b): the hash is the slug, so slug drift (`inline-event-handler` vs `inline-event-handler-in-view`) splits one pattern into several sub-threshold clusters. Surface candidate merge-families for human confirmation before classifying: don't auto-merge.

Emit `recurring-patterns.md` as the Phase 2 artifact: one section per cluster with the data from `references/recurrence-detection.md` output format.

---

## Phase 3: Propose

For each cluster, decide the target and draft a proposal. See `references/proposal-template.md` for the exact format.

### 3a: Recurring / Chronic → preventive action

Pick one of these target actions based on the cluster's dimension and what's already in place:

| Cluster dimension | No existing check | Check exists but misses | Pattern-level (architectural) |
|-|-|-|-|
| Security | Propose preflight check via `a-review-optimizer` | Update check pattern via `a-review-optimizer` | Propose rule file via `a-rules-optimizer` |
| Architecture | Propose preflight check | Update check pattern | Propose rule file |
| Quality | Propose preflight check | Update check pattern | Propose rule, possibly scoped narrowly |
| Performance | Propose preflight check | Update check pattern | Propose rule file |

**When in doubt, lean toward `a-rules-optimizer`**: rules prevent issues before code is written; preflight checks catch them after. Prevention is cheaper.

**Before proposing a NEW rule, check whether the rule already exists but isn't loading.** A cluster can keep recurring while the governing rule already exists, because it lives in a file whose `paths:` scope never matches the file type where violations happen (e.g. a JS rule scoped `public/js/**` that never loads while editing PHP views, where every regression sits). Signal: the cluster's `file` values cluster in one file type, and grep finds the rule already stated in a differently-scoped rule file. When you see this, the proposal is not "add a rule"; it's "restate the existing rule in a file scoped to where the violations occur, with a one-line cross-reference to the source of truth," routed to `a-rules-optimizer`. This path-visibility fix is cheaper and truer than inventing a duplicate rule. Name the root cause explicitly in the proposal's `rationale`.

### 3b: False Positive clusters → whitelist action

Target: the review skill's "KNOWN CORRECT PATTERNS (DO NOT FLAG)" section (see `a-review-optimizer`'s Phase 4a output). Propose adding:

```
- <normalized pattern>: <project-specific reason>, see <file:line of representative occurrence>
```

Use the existing `feedback_fp_*.md` text verbatim where possible: the user wrote it in their own words for a reason.

### 3c: Proposal structure

Every proposal must include (see `references/proposal-template.md`):

- `cluster_id`: stable identifier (hash prefix)
- `pattern`: one-line human description
- `evidence`: list of findings supporting the cluster (file:line + date + severity)
- `rationale`: why a preventive change is warranted (count, span, severity distribution)
- `target_skill`: `a-rules-optimizer` or `a-review-optimizer`
- `target_file`: which file the other skill should modify
- `proposed_diff`: concrete change, not a description of one
- `expected_effect`: what future reviews should differ about after this lands

Missing fields → proposal is incomplete, don't show it to the user.

---

## Phase 4: User approval gate (hard stop)

**This phase must pause for user input.** Present proposals one at a time, batched by priority (Chronic first, then Recurring, then False Positive whitelists). For each:

1. Show the proposal in human-readable form (pattern, evidence count, rationale, the diff).
2. Ask: "Apply this? [y/n/skip/details]"
3. On `y` → delegate to the target skill with the proposal as input. That skill writes the file. Do not write anything directly from `a-self-learner`.
4. On `n` → append to `rejected-proposals.md` with reason (ask the user for the reason if `n` alone; "no reason given" is acceptable).
5. On `skip` → leave for next run, neither apply nor reject.
6. On `details` → show evidence in full (all finding lines, not just count), then re-prompt.

**Commit policy.** Even after approval, this skill does not commit. The target skill writes the file; the user decides when to stage and commit per the global per-commit-ask rule. Do not invoke `git commit` from this workflow.

---

## Phase 5: Record

After the approval loop finishes:

1. **`applied-learnings.md`**: append one entry per applied proposal: date, cluster_id, target, summary, file(s) modified. This becomes the project's "how our defenses hardened" changelog.
2. **`rejected-proposals.md`**: already appended to in Phase 4 for each rejection. Also record the evidence count at rejection time so re-proposal threshold can be computed ("this was rejected when count was 4; only re-raise when count exceeds 8").
3. **Archive processed findings**: move all findings that participated in applied or rejected clusters from `review-issues.jsonl` to `review-issues-archive.jsonl`, stamped with a processed-date. Next run starts from a smaller log and doesn't re-cluster old data.
4. **Report summary**: count of proposals applied / rejected / skipped, list of files modified, suggestion for when to re-run (typically: "after the next 5 reviews, or in ~4 weeks").

---

## What this skill does NOT do

- **Does not write rule files or review SKILLs directly.** Always delegates.
- **Does not auto-commit.** User owns commits per CLAUDE.md.
- **Does not cross-pollinate between projects.** Each project's feedback loop is isolated.
- **Does not re-review the codebase.** It only operates on the review log and feedback memory. If the log is empty, the skill has nothing to do.
- **Does not replace `a-rules-optimizer` or `a-review-optimizer`.** Those remain the authoritative ways to audit against the codebase. This skill adds a *historical-evidence* input that those skills can consume.

---

## Reference files

| File | When to read | Contains |
|-|-|-|
| `references/review-log-schema.md` | Phase 1a (understanding the log format) and any capture-side integration | JSONL schema, field definitions, `.claude/reviews/` directory convention |
| `references/recurrence-detection.md` | Phase 2 (clustering) | Grouping, hashing, thresholds, cluster classification |
| `references/proposal-template.md` | Phase 3 (drafting proposals) | Required fields, format, target-skill routing rules |
| `references/capture-finding.sh` | Setup: helper that existing review skills call to append findings | Small bash script (~30 lines), safe to source and call |

#!/usr/bin/env bash
# capture-finding.sh: append one finding to .claude/reviews/review-issues.jsonl
#
# Helper for project *-review skills. Call once per finding. Safe to call from
# any review skill across any project.
#
# Usage:
#   bash capture-finding.sh \
#       --project example-api \
#       --skill example-review \
#       --run-id "$RUN_ID" \
#       --dimension security \
#       --severity high \
#       --category subprocess-without-timeout \
#       --file src/core/backup_engine.py \
#       --line 142 \
#       --message "subprocess.run() without timeout=" \
#       [--fix "add timeout=30"] \
#       [--agent security] \
#       [--check-id SUB-01]
#
# The helper creates .claude/reviews/ if missing, computes category_hash,
# assigns a finding_id, and appends one JSON line. No other side effects.

set -euo pipefail

# Defaults
PROJECT=""; SKILL=""; RUN_ID=""; DIMENSION=""; SEVERITY=""
CATEGORY=""; FILE=""; LINE="0"; MESSAGE=""
FIX=""; AGENT=""; CHECK_ID=""; WHITELISTED="false"

while [ $# -gt 0 ]; do
    case "$1" in
        --project)     PROJECT="$2"; shift 2 ;;
        --skill)       SKILL="$2"; shift 2 ;;
        --run-id)      RUN_ID="$2"; shift 2 ;;
        --dimension)   DIMENSION="$2"; shift 2 ;;
        --severity)    SEVERITY="$2"; shift 2 ;;
        --category)    CATEGORY="$2"; shift 2 ;;
        --file)        FILE="$2"; shift 2 ;;
        --line)        LINE="$2"; shift 2 ;;
        --message)     MESSAGE="$2"; shift 2 ;;
        --fix)         FIX="$2"; shift 2 ;;
        --agent)       AGENT="$2"; shift 2 ;;
        --check-id)    CHECK_ID="$2"; shift 2 ;;
        --whitelisted) WHITELISTED="true"; shift ;;
        *)             echo "Unknown arg: $1" >&2; exit 2 ;;
    esac
done

# Required-field check
for v in PROJECT SKILL RUN_ID DIMENSION SEVERITY CATEGORY FILE MESSAGE; do
    if [ -z "${!v}" ]; then
        echo "capture-finding.sh: missing required --${v,,}" >&2
        exit 2
    fi
done

DATE=$(date -u +%Y-%m-%d)
REVIEWS_DIR=".claude/reviews"
LOG="$REVIEWS_DIR/review-issues.jsonl"
mkdir -p "$REVIEWS_DIR"

# category_hash groups findings into pattern classes for a-self-learner.
# The clustering key is the CATEGORY slug alone (lowercased): NOT the free-text
# message (which carries method names / paths / identifiers that fragment one
# pattern into dozens of hashes), and NOT the dimension (the same pattern is
# tagged 'architecture' by one agent and 'quality' by another, which would
# re-split it). Slug hygiene is load-bearing: keep slugs specific enough that one
# slug == one pattern, and consistent enough that one pattern == one slug (no
# 'inline-event-handler' vs 'inline-event-handler-in-view' drift).
# See review-log-schema.md "category_hash computation".
HASH=$(printf '%s' "$CATEGORY" | tr '[:upper:]' '[:lower:]' | sha256sum | cut -c1-16)

# Simple UUID-ish id: 16 hex chars from urandom
FINDING_ID=$(head -c 8 /dev/urandom | od -An -tx1 | tr -d ' \n')

# Build JSON object. python3 handles escaping cleanly.
python3 - "$DATE" "$PROJECT" "$SKILL" "$RUN_ID" "$FINDING_ID" "$DIMENSION" \
               "$SEVERITY" "$CATEGORY" "$FILE" "$LINE" "$MESSAGE" \
               "$FIX" "$AGENT" "$CHECK_ID" "$HASH" "$WHITELISTED" "$LOG" <<'PY'
import json, sys
date, project, skill, run_id, finding_id, dimension, severity, category, \
    file_, line, message, fix, agent, check_id, category_hash, whitelisted, log = sys.argv[1:]

rec = {
    "date": date, "project": project, "skill": skill, "run_id": run_id,
    "finding_id": finding_id, "dimension": dimension, "severity": severity,
    "category": category, "file": file_, "line": int(line or 0),
    "message": message, "category_hash": category_hash,
    "whitelisted": whitelisted == "true",
}
if fix: rec["fix_proposed"] = fix
if agent: rec["agent"] = agent
if check_id: rec["check_id"] = check_id

with open(log, "a") as f:
    f.write(json.dumps(rec, separators=(",", ":")) + "\n")
PY

# Proposal Template

The shape of the proposal that `a-self-learner` Phase 3 generates and Phase 4 presents. Every proposal must be complete: missing fields mean the proposal isn't ready to show the user.

## Required fields

```yaml
cluster_id:        <hash prefix, e.g. "a3f1c2b4">
pattern:           <one-line human description>
dimension:         <security | architecture | quality | performance>
classification:    <Recurring | Chronic | RE-EMERGENT | False Positive>

evidence:
  count:           <integer>
  span_days:       <integer>
  distinct_files:  <integer>
  first_seen:      <YYYY-MM-DD>
  last_seen:       <YYYY-MM-DD>
  examples:        <list of 3-5 findings: file:line, severity, date>

rationale:         <why this warrants a preventive change, 1-3 sentences tied to the evidence>

target_skill:      <a-rules-optimizer | a-review-optimizer>
target_file:       <specific file the target skill should modify>
proposed_change:   <concrete diff or rule text, NOT a description>

expected_effect:   <what should differ about future reviews after this lands>

re_propose_if:     <condition that would justify re-raising if rejected>
```

## Routing: which skill writes what

| Target file | Target skill | Use when |
|-|-|-|
| `.claude/rules/<file>.md` (new or existing) | `a-rules-optimizer` | Pattern-level issue that a rule could prevent upstream (e.g. "all SQL uses prepared statements"). Preferred when the pattern represents a project convention. |
| `.claude/rules/<in-scope file>.md` (restate existing rule) | `a-rules-optimizer` | The governing rule already exists but its `paths:` scope never loads on the file type where violations occur. Propose restating it in a file scoped to those files, with a cross-reference to the source of truth, not a brand-new rule. |
| `.claude/scripts/preflight*.sh` | `a-review-optimizer` | Issue is best caught by a deterministic grep/python check. Preferred when the pattern has a mechanical signature. |
| Review skill's capture calls | `a-review-optimizer` | Scaffolding a project's capture mechanism: the review SKILL.md must call `capture-finding.sh` per confirmed finding. `a-self-learner` drops the helper script, but wiring calls into the review skill is `a-review-optimizer`'s edit. |
| Review skill's `KNOWN CORRECT PATTERNS` section | `a-review-optimizer` | False-positive whitelist additions. |
| Review skill's agent prompt | `a-review-optimizer` | New checks that require LLM judgment (not mechanical). |

**When ambiguous, prefer `a-rules-optimizer` over `a-review-optimizer`.** Rules prevent at write-time; preflight catches at review-time. Prevention is cheaper.

## Writing `proposed_change`

The proposal must be **executable** by the target skill, not descriptive. Two examples:

### Good: rule file proposal (target: `a-rules-optimizer`)

```markdown
target_file: .claude/rules/security.md

proposed_change: |
  Append to `security.md` under a new section `## Subprocess Safety`:

    - Every `subprocess.*` call must include `timeout=`: no exceptions for "quick" commands.
    - `shell=True` is forbidden outside `scripts/`: argv form only.
    - Reference: src/core/backup_engine.py:142 for the canonical pattern.
```

### Bad: too vague to act on

```markdown
target_file: .claude/rules/security.md

proposed_change: "Add a rule about subprocess safety."
```

The second form forces the target skill to reinvent the proposal. Don't do that.

### Good: preflight check proposal (target: `a-review-optimizer`)

```markdown
target_file: .claude/scripts/preflight.sh

proposed_change: |
  Replace the current SUB-01 Python block with:

    ```python
    import re, pathlib
    for f in pathlib.Path('src').rglob('*.py'):
        text = f.read_text()
        for m in re.finditer(r'subprocess\.(run|call|check_output|check_call|Popen)\s*\(', text):
            ...  # balanced-paren scan checking 'timeout' not in call_text
    ```

  Reason: current regex misses `.check_output` and `.Popen` forms: evidence at src/cli.py:622, scripts/ingest.py:394.
```

## Presentation to the user

When Phase 4 shows a proposal, display it as:

```
────────────────────────────────────────────────
Proposal a3f1c2b4: subprocess-without-timeout
Classification: Chronic (10 findings, 2026-01-15 → 2026-04-19, 4 files)

Rationale:
  Current preflight catches .run() but misses .check_output() and .Popen().
  Pattern has recurred for 3 months across 4 files.

Target: a-review-optimizer → .claude/scripts/preflight.sh
Change: update SUB-01 regex to cover all subprocess variants (see diff above)

Expected effect:
  Next review should catch subprocess timeout issues in cli.py:622 and
  scripts/ingest.py:394 (both currently missed).

Apply this? [y/n/skip/details]
────────────────────────────────────────────────
```

Keep the prompt terse. The user reads the rationale, looks at the diff, decides. Don't pad with hedging or alternatives: the skill already picked one option; the user either agrees or doesn't.

## After approval

Invoke the target skill with the proposal as structured input. Example (illustrative, actual invocation shape depends on how the skill wrapper is called):

```
Delegate to a-review-optimizer with:
  action: update-preflight-check
  check_id: SUB-01
  file: .claude/scripts/preflight.sh
  proposed_change: <the diff text from the proposal>
  source_cluster: a3f1c2b4
```

The target skill applies the change and reports back with the modified file paths. `a-self-learner` then appends to `applied-learnings.md` with the cluster_id, target, and files modified.

## After rejection

Append to `rejected-proposals.md`:

- Cluster ID, pattern, date of rejection
- Evidence count at rejection time
- Reason (quote the user's reply or note "no reason given")
- Re-proposal threshold (typically: count must exceed rejected_count × 2 or a new distinct file must appear)

Re-proposal thresholds prevent the skill from pestering the user about rejected ideas.

# Recurrence Detection Algorithm

How `a-self-learner` Phase 2 turns a pile of findings into clustered patterns ready for Phase 3 proposals.

## Inputs

- `review-issues.jsonl` (current window)
- `review-issues-archive.jsonl` (optional, only read if `--deep` flag set or if window is very small)
- `feedback_fp_*.md` files from project memory: for marking clusters as False Positive

## Step 1: Parse and filter

```python
import json
from pathlib import Path
from datetime import datetime

findings = []
for line in Path(".claude/reviews/review-issues.jsonl").read_text().splitlines():
    line = line.strip()
    if not line:
        continue
    try:
        f = json.loads(line)
    except json.JSONDecodeError as e:
        print(f"MALFORMED: {line[:80]}... ({e})")
        continue
    findings.append(f)
```

Drop findings where:
- `whitelisted = true` (already dismissed in-run; keep the dismissal data for FP analysis, but don't cluster into recurring)
- `severity = info` (not actionable by itself)
- `dimension = other` and no explicit category

## Step 2: Group by `category_hash`

```python
from collections import defaultdict

clusters = defaultdict(list)
for f in findings:
    clusters[f["category_hash"]].append(f)
```

`category_hash` is `sha256(category)[:16]` (see `review-log-schema.md`), so this is identical to grouping by `category`: one cluster per pattern class. Two different categories sharing a hash would be a sha256 collision (effectively impossible). If you ever see one category split across multiple hashes, the log predates a hash-definition change and needs a backfill: don't treat the fragments as distinct patterns.

## Step 2b: Surface near-duplicate slug families (human merge gate)

Because `category_hash` is the slug, clustering is only as good as slug hygiene, and review skills drift: the same pattern arrives as `inline-event-handler`, `inline-event-handler-in-view`, `inline-event-handler-js`. Each becomes its own cluster, and the split can push a genuinely-recurring pattern below the Step 3 thresholds (one Chronic cluster of 7 becomes one Recurring of 3 plus three ignored One-offs).

Before classifying, detect candidate merge-families and **ask the human**: never auto-merge (some near-twins are intentionally distinct, e.g. `method-over-30-lines` vs `method-over-50-lines`).

```python
import difflib
from itertools import combinations

def _toks(slug): return set(slug.split("-"))

def near_dup(a, b):
    ta, tb = _toks(a), _toks(b)
    jaccard = len(ta & tb) / len(ta | tb)
    ratio = difflib.SequenceMatcher(None, a, b).ratio()
    return (a in b or b in a) or jaccard >= 0.5 or ratio >= 0.72

# union-find over near-dup edges → connected components of slugs
slugs = sorted(clusters)                       # one slug per category_hash
parent = {s: s for s in slugs}
def find(x):
    while parent[x] != x:
        parent[x] = parent[parent[x]]; x = parent[x]
    return x
for a, b in combinations(slugs, 2):
    if near_dup(a, b):
        parent[find(a)] = find(b)

families = {}
for s in slugs:
    families.setdefault(find(s), []).append(s)
families = {root: m for root, m in families.items() if len(m) > 1}
```

For each family with >1 member, present it sorted by finding count and ask:

```
Possible slug drift: one pattern under several slugs?
  inline-event-handler-in-view   (11 findings, 2 dates)
  inline-event-handler           (5,  1 date)
  inline-event-handler-js        (1)
  inline-event-handlers          (1)
  inline-event-handlers-admin    (1)
Merge for this run? [canonical slug / n = keep separate]
```

On confirm, treat the family as **one cluster for this run's Step 3** (merge in-memory under the chosen canonical slug; sum the counts/dates/files). Don't rewrite `category` on historical findings unless you deliberately want to normalize the log: that mutates the authoritative clustering key. To make a merge *durable*, do one of: record the canonical mapping in a `slug-aliases.md` note read at the top of Step 2b, or fix the slug at source so the review skill emits the canonical form going forward. Record confirmed keep-separate decisions too, so the gate stops re-asking about intentional twins.

Thresholds (Jaccard ≥ 0.5, ratio ≥ 0.72, or substring) are tuned to catch real families without flooding: loosen to surface more, tighten if it over-suggests. Union-find chains transitively (A~B~C even when A and C aren't alike), so a family may contain a stray: in practice the gate groups `method-over-30-lines` + `method-over-50-lines` correctly but also drags in `file-over-500-lines` via the shared `over…lines` tokens. Split strays when confirming. This is a suggestion engine, not an authority: the human decides every merge.

## Step 3: Classify each cluster

For each cluster, compute:

- `count` = number of findings in the cluster
- `distinct_dates` = number of distinct `date` values
- `distinct_files` = number of distinct `file` values
- `first_seen`, `last_seen` = min / max of `date`
- `span_days` = (last_seen - first_seen).days
- `severity_mix` = count by severity
- `is_fp_feedback_present` = True if any `feedback_fp_*.md` file mentions this `category`

Classification:

```
if is_fp_feedback_present:
    cls = "False Positive"
elif count >= 5 or span_days > 30 or distinct_files >= 3:
    cls = "Chronic"
elif count >= 3 and distinct_dates >= 2:
    cls = "Recurring"
else:
    cls = "One-off"
```

Thresholds are adjustable via the skill's `--threshold N` flag (overrides the `3` for Recurring; `Chronic` uses `threshold + 2`).

## Step 4: Skip clusters already resolved

For each cluster, check `applied-learnings.md` and `rejected-proposals.md`:

- If cluster_id already in `applied-learnings.md` AND no new findings since the applied date → skip (already addressed, and hasn't re-emerged).
- If cluster_id already in `applied-learnings.md` AND new findings since → **re-surface with a "RE-EMERGENT" flag**. The prior fix didn't hold. Investigate.
- If cluster_id in `rejected-proposals.md` AND current count ≤ re-proposal threshold stored there → skip.
- If cluster_id in `rejected-proposals.md` AND current count exceeds the threshold → resurface for a new decision.

## Step 5: Rank for presentation

Order clusters for Phase 3 presentation:

1. **RE-EMERGENT** clusters first: a prior fix failed; this is highest value to investigate.
2. **Chronic** clusters (broad, long-standing patterns) tend to translate into rule files.
3. **Recurring** clusters (narrower, newer) tend to translate into preflight checks.
4. **False Positive** clusters: whitelist proposals.

Within each tier, sort by `count` descending. Present top-N per tier (default N = 5; user can override).

## Step 6: Emit `recurring-patterns.md`

See `review-log-schema.md` for the output format. Include in each section:

- cluster_id, pattern name, dimension
- classification and counts
- first/last seen dates
- distinct files affected
- representative findings (3-5 example lines with file:line and severity)
- recommended action (preflight vs rule vs whitelist)
- target skill (`a-review-optimizer` vs `a-rules-optimizer`)

## Edge cases

- **Cluster with one dominant file.** If 10 findings are all in `src/legacy/*.py`, the fix might be "deprecate that module" rather than "add a rule." Flag these as `LOCALIZED`: they may deserve a different proposal (code fix vs rule) or a path-scoped rule.
- **Cluster that spans multiple skills.** If `skill` varies within a cluster (e.g. both `example-review` and an older `example-review-legacy` captured it), note it: the fix might need to apply to both review skills.
- **Runaway cluster from a single bad run.** If all findings share the same `run_id` and the cluster has no findings from other runs, treat as One-off regardless of count. A single bad PR scan that flagged 30 things isn't a recurring pattern.
- **Empty project history.** Zero findings → skill reports "no history, cannot learn" and exits cleanly. Not an error.

## Calibration

These thresholds (3 for Recurring, 5 for Chronic, 30 days for span, 3 files) are defaults. For a new capture convention, findings accumulate slowly: thresholds may need lowering in the first few months. Users can pass `--threshold 2` to see near-patterns early.

Thresholds should NOT be tuned per-project in the skill itself. Pass them as arguments. Keeps behavior predictable.

# Review Log Schema

The `.claude/reviews/` directory holds per-project review history that `a-self-learner` consumes. Each project owns its own directory: no cross-project sharing.

## Directory layout

```
.claude/reviews/
├── review-issues.jsonl           Append-only log of findings (current window).
├── review-issues-archive.jsonl   Findings already processed by a-self-learner.
├── recurring-patterns.md         Generated by a-self-learner Phase 2.
├── applied-learnings.md          Changelog of accepted proposals.
└── rejected-proposals.md         Proposals the user declined, with reasons.
```

All files are meant to be git-tracked (per-project). They document how the project's defenses evolved. Size budget: `review-issues.jsonl` rolls to archive after each `a-self-learner` run; archive can grow indefinitely but is rarely read.

## review-issues.jsonl: one JSON per line

Every line is a complete JSON object. No arrays, no pretty-printing across lines. This makes `wc -l`, `grep`, `head`, and `jq -c` work naturally.

### Required fields

| Field | Type | Meaning |
|-|-|-|
| `date` | ISO 8601 date (YYYY-MM-DD) | When the finding was produced |
| `project` | string | Project name (e.g. `example-api`, `example-web`) |
| `skill` | string | Which review skill produced it (e.g. `example-review`, `web-review`) |
| `run_id` | UUID or timestamp | Groups findings from the same review run |
| `finding_id` | UUID | Unique per finding |
| `dimension` | enum | `security` / `architecture` / `quality` / `performance` / `other` |
| `severity` | enum | `critical` / `high` / `medium` / `low` / `info` |
| `category` | string | Short slug: the pattern class (e.g. `subprocess-without-timeout`, `st-rerun-without-invalidate`, `raw-sql-with-interpolation`). **Stable across runs**, this is the clustering key. |
| `file` | string | Repo-relative path |
| `line` | integer | Line number (1-based); use 0 if file-level without a line |
| `message` | string | Human-readable description of the issue |
| `category_hash` | string (sha256 hex, 16 chars) | Pre-computed `sha256(category)[:16]`: a pure function of the `category` slug, for fast grouping. The capture helper fills this. |

### Optional fields

| Field | Type | Meaning |
|-|-|-|
| `fix_proposed` | string | The proposed fix shown to the user |
| `agent` | string | Which review agent flagged it (e.g. `security`, `architecture`) |
| `check_id` | string | Preflight check that surfaced it (e.g. `SEC-14`, `SUB-01`) |
| `whitelisted` | boolean | True if the review ultimately dismissed via a known-correct whitelist |
| `confirmed_by_user` | boolean | True if the user explicitly acknowledged the finding during review |

### Example lines

```json
{"date":"2026-04-19","project":"example-api","skill":"example-review","run_id":"r-20260419-1","finding_id":"f-9a2c","dimension":"security","severity":"high","category":"subprocess-without-timeout","file":"src/core/backup_engine.py","line":142,"message":"subprocess.run() without timeout=","fix_proposed":"add timeout=30","category_hash":"a3f1c2b4d5e6f789","agent":"security","check_id":"SUB-01"}
{"date":"2026-04-19","project":"example-api","skill":"example-review","run_id":"r-20260419-1","finding_id":"f-9a2d","dimension":"architecture","severity":"medium","category":"st-rerun-without-invalidate","file":"src/web/views/dashboard.py","line":88,"message":"st.rerun() without preceding invalidate()","fix_proposed":"invalidate() then st.rerun()","category_hash":"b7e2f3a4c5d6e890","agent":"architecture","check_id":"WEB-03"}
```

## `category_hash` computation

The capture helper computes `category_hash = sha256(category.lower())[:16]`: a pure function of the lowercased `category` slug, nothing else:

```bash
HASH=$(printf '%s' "$CATEGORY" | tr '[:upper:]' '[:lower:]' | sha256sum | cut -c1-16)
```

Lowercasing is so `Dead-Code` and `dead-code` co-cluster (case is not semantic in a slug). `category` is the **load-bearing clustering key**, so the hash deliberately excludes:

- **The free-text `message`**: it carries method names, file paths, and identifiers that vary per finding. Hashing it fragmented a single pattern class into dozens of hashes (e.g. `method-over-30-lines` → 71 hashes), defeating grouping. This was the pre-2026-06-14 bug.
- **The `dimension`**: the same pattern is tagged `architecture` by one agent and `quality` by another (~4% of categories drift this way), which would re-split the cluster. Dimension is decided per-cluster at Phase 2 instead.

Consequence: grouping by `category_hash` is identical to grouping by `category`. Two different categories cannot collide (barring a sha256 collision); one category cannot fragment. When this definition changes, recompute (backfill) the hash for historical findings so old and new captures still group together.

**Cross-implementation invariant.** Any code that recomputes the hash (the python backfill, an alternate capture path) MUST produce byte-identical output: `hashlib.sha256(category.lower().encode("utf-8")).hexdigest()[:16]`. The shell `printf '%s'` emits no trailing newline, so the hashed bytes are exactly the lowercased category string, no separator, no newline.

**Slug hygiene is load-bearing.** Because the hash is the slug, clustering is only as good as the slugs review skills emit. Near-duplicate slugs for one pattern (`inline-event-handler` vs `inline-event-handler-in-view`, `file-put-contents-no-lock` vs `file-put-contents-without-lock`) form separate clusters and can drop a genuinely-recurring pattern below the recurrence thresholds. Phase 2 (`recurrence-detection.md`) should surface near-duplicate slug families for human merge before classifying.

## recurring-patterns.md: generated output

Generated by `a-self-learner` Phase 2. One section per cluster:

```markdown
## [cluster a3f1c2b4] subprocess-without-timeout

**Dimension:** security
**Severity:** high (8 findings), medium (2 findings)
**Classification:** Chronic (10 occurrences, spans 2026-01-15 → 2026-04-19, 4 distinct files)
**First seen:** 2026-01-15
**Last seen:** 2026-04-19
**Files affected:**
- src/core/backup_engine.py:142
- src/cli.py:622
- src/sync/worker.py:88
- scripts/ingest.py:394

**Recommended action:** Update preflight check SUB-01 pattern: it's catching some but missing others (lines 622 and 394 don't fit the current regex). Target skill: `a-review-optimizer`.
```

## applied-learnings.md: human-readable changelog

Append-only. One entry per applied proposal:

```markdown
### 2026-04-19: [cluster a3f1c2b4] Subprocess timeout enforcement

**Source:** Recurring pattern: 10 findings across 2026-01-15..2026-04-19.
**Applied via:** a-review-optimizer
**Files modified:** `.claude/scripts/preflight.sh` (updated SUB-01 regex to catch `subprocess.run`, `.call`, `.check_output`, and `.Popen` forms).
**Expected effect:** Next review should catch all 4 previously-missed files on the first run.
```

## rejected-proposals.md: declined decisions

Append-only. One entry per rejection:

```markdown
### 2026-04-19: [cluster b2d4e6f8] Ban print() in src/

**Evidence at rejection:** 4 findings (`src/cli.py`, `src/debug_helpers.py`).
**Reason:** User maintains that print() in `src/cli.py` is the intentional CLI output channel and in `debug_helpers.py` is intentional debug output. Not an issue.
**Re-proposal threshold:** Do not re-raise unless count exceeds 12 AND appears outside `src/cli.py` / `debug_helpers.py`.
```

## Size / retention policy

- `review-issues.jsonl`: active window; should rarely exceed a few thousand lines. `a-self-learner` archives processed findings on each run.
- `review-issues-archive.jsonl`: grows indefinitely but is cold storage. Never read during normal clustering; only referenced when investigating a specific historical finding.
- `recurring-patterns.md`: regenerated each run; treat as derived, not authoritative.
- `applied-learnings.md` / `rejected-proposals.md`: authoritative, append-only, never rewrite.