[BUG] `superpowers:requesting-code-review` (and its dispatched `code-reviewer` subagent)

Preflight Checklist

• [x] I have searched existing issues and this hasn't been reported yet
• [x] This is a single bug report (please file separate reports for different bugs)
• [x] I am using the latest version of Claude Code

What's Wrong?

Enhancement Proposal: Context-Aware, Chained Code Review

Target skill: superpowers:requesting-code-review (and its dispatched code-reviewer subagent) Status: Draft / RFC Authored against: superpowers 5.0.7 Motivation: Real bug from production use — see "Empirical Motivation" below

TL;DR

The current code-reviewer template is a point-in-time diff audit. It has no awareness of:

1. Project intent — conventions declared in CLAUDE.md / AGENTS.md, ongoing migrations, deprecation timelines, feature-flag gating
2. Deletion asymmetry — the checklist treats removals the same as additions, despite deletions having cascade risk that additions do not
3. Review history — prior reviewers' decisions and intervening changes are invisible to a new reviewer, so decisions made in March can be acted on in April without delta verification

This proposal adds a three-part enhancement that makes the reviewer context-aware (reads a project-declared manifest), deletion-cautious (explicit protocol for removal diffs), and chained (each review links to its predecessor and computes deltas against intervening commits). All changes are opt-in with graceful degradation — projects that don't adopt the conventions get the current behavior unchanged.

Empirical Motivation

This proposal is grounded in a real bug that made it through a clean code review and crashed a user-facing chat endpoint in production:

A migration from hardcoded tier config (TIER_BY_ID in-memory map) to DB-driven config (AiTierConfig table) was partially completed. The data structure was emptied:

/** @deprecated DEAD CODE — DB-driven via AiTierConfig. Remove after 2026-04-05 */
export const TIER_BY_ID: Record<string, TierDef> = {};

export function getTier(id: string): TierDef {
  return TIER_BY_ID[id] ?? (TIER_BY_ID["standard"] as TierDef);
}

The accessor getTier() was left in place, with a cast-to-TierDef that lied to the type checker. Four production callers remained (routeTier() in the AI router and three /api/generate/* routes). Every call to getTier() now returned undefined, and the first property access on the return value crashed:

const tier = getTier(tierId);
return { creditCost: tier.creditCost, ... };  // TypeError at runtime

A pre-merge code review was performed. It approved the change. The review:

• Read the diff and confirmed the deprecation comment was valid
• Did not grep the repository for getTier( callers
• Did not read CLAUDE.md (which lists the cleanup conventions and migration state)
• Did not consult docs/pending-work.md (which tracked the migration as "Tier 2 research — partial, callers not yet moved")
• Did not note that four active callers would break when the data structure returned undefined

The bug was caught by a user reporting "500 on chat send" after deploy. Root cause investigation took ~20 minutes; the fix was four getTier → getTierAsync edits.

The reviewer was not lazy. The reviewer followed the existing template exactly. The template does not ask for any of the checks that would have caught this.

Root Cause Analysis

Three structural gaps in the current code-reviewer.md template:

Gap 1: No context load step

The template accepts {WHAT_WAS_IMPLEMENTED} and {PLAN_OR_REQUIREMENTS} as the only project context. The reviewer subagent runs in a fresh context (by design — to isolate it from the caller's conversation history) but has no path to the project's declarative truth. It cannot know:

• That a specific symbol is mid-migration
• That a specific directory has conventions documented in CLAUDE.md
• That a scheduled deprecation has not yet had all callers migrated
• That a previous review flagged related issues

Gap 2: Deletions treated symmetrically with additions

The current checklist ("Code Quality", "Architecture", "Testing", "Requirements", "Production Readiness") is additive-focused. There is no section for removals. In practice, removals carry asymmetric risk:

• Adding bad code is usually safe to revert in isolation
• Deleting needed code cascades — every caller downstream is now broken, and the causal chain is harder to trace because the deleted symbol no longer exists to grep for

The current template has no "before approving a removal, grep repo-wide for the symbol" step. No "check SQL seeds, CI configs, test mocks, dynamic string lookups" guidance. No "half-cleanup detector" that flags when a data structure is emptied but its accessor functions remain.

Gap 3: Reviews are ephemeral and isolated

Each review runs fresh with no memory of prior reviews. There is no artifact produced that a future reviewer can reference. If reviewer N's verdict from three weeks ago is cited as justification for a cleanup today, reviewer N+1 has no mechanism to:

• Find reviewer N's output
• Verify that reviewer N's assumptions still hold
• Compute the git delta between reviewer N's base SHA and HEAD
• Surface new callers, tests, or usages added in the intervening commits

Reviews are, in effect, fire-and-forget. They cannot build on each other.

Proposal

Three components, designed as one coherent system but adoptable piece by piece:

Component A: Project Context Manifest

Projects opt in by adding a canonical section to their CLAUDE.md (or AGENTS.md / GEMINI.md):

## Project Context Manifest

<!-- Agents: read entries whose `when` matches your current task.
     Schema: path | when (tag list) | contains (short description) -->

- path: docs/pending-work.md
  when: cleanup, deletion, deprecation, migration-review
  contains: in-progress migrations, deprecation schedules, known half-states

- path: docs/conventions/imports.md
  when: any-edit
  contains: canonical import patterns

- path: docs/conventions/models.md
  when: schema, database, prisma
  contains: data model quick-reference

- path: docs/superpowers/reviews/INDEX.md
  when: any-review, cleanup, before-merge
  contains: review history, prior verdicts, delta baselines

- path: ~/.claude/projects/<project>/memory/MEMORY.md
  when: any-task
  contains: persistent observations about user, project, feedback

Schema:

• path — file location, relative or absolute
• when — list of tags; skill matches against its current task type
• contains — short description for the reviewer to decide relevance

when: vocabulary (starter set, extensible):

• any-edit — always load
• any-review — any code review
• cleanup / deletion / deprecation — removal-oriented reviews
• schema / database — data model changes
• feature-flag / access-control — security and gating
• before-merge — pre-merge gates
• migration-review — in-progress migration checks

Why a canonical markdown section instead of YAML frontmatter or a separate file?

• Markdown is human-editable and already conventional in CLAUDE.md
• Grep-discoverable (rg '## Project Context Manifest')
• Colocated with the conventions it indexes — one place to maintain
• Degrades gracefully: if the section is missing, skills fall back to reading CLAUDE.md whole

Component B: Code Reviewer Enhancements

Three additions to code-reviewer.md:

1. Step 0 — Context Load (mandatory, non-skippable):

## Step 0: Context Load (run BEFORE examining the diff)

1. Read the project's conventions file:
   - Try `CLAUDE.md`, then `AGENTS.md`, then `GEMINI.md` in the repo root
   - Locate the `## Project Context Manifest` section
   - If found: classify this review's concerns (see "Concern Tags" below)
     and read every manifest entry whose `when` includes a matching tag
   - If not found: read the conventions file whole and note that the
     project would benefit from adopting a manifest

2. Summarize the 3–5 conventions most relevant to the touched files.
   If any diff hunk would violate a convention, flag it as a Critical issue.

3. Check git history on each touched file:
   `git log --oneline -20 -- <file>`
   Look for recent deprecation notes, migration commits, or reversions.

4. Grep touched files for:
   @deprecated | TODO | FIXME | DEAD CODE | feature flag names

2. Deletion Protocol — triggered when the diff removes exported symbols, empties data structures, or deletes files:

## Deletion Protocol

If this review includes any of the following, run the checks below BEFORE
issuing a verdict. Deletions have asymmetric cascade risk vs additions.

**Triggers:**
- Removed export statement
- Data structure emptied (array → [], map → {}, etc.)
- Deleted file
- Removed function body while retaining signature
- Removed case from switch / branch from if-else

**Required checks:**

1. **Repo-wide usage grep.** For each removed symbol:
   `rg "<symbol>" --type-add 'all:*' -tall`
   Search ALL files, not just `src/` — include:
   - Test files and mocks
   - SQL seeds and migrations
   - CI configs (.github/, .gitlab-ci.yml)
   - Config files (*.json, *.toml, *.yaml)
   - Dynamic string lookups (e.g. `map[key]` where key could be a string literal)
   - Documentation examples

2. **Half-cleanup detector.** If a data structure has been emptied:
   - Identify all accessor functions that read the structure
   - Verify accessors are ALSO removed or migrated to an alternative source
   - Flag any accessor left in place that will now return undefined/empty
   - Check for TypeScript casts (`as T`) that may hide the undefined from
     the type checker

3. **Deprecation-date validity.** If a deprecation comment cites a date:
   - Is the date in the past?
   - Have all callers been migrated?
   - Run `git log --since="<comment_date>" -S "<symbol>"` to find any
     new usages added AFTER the comment was written

4. **Intent preservation default.** Code with a deprecation comment,
   feature flag reference, migration bridge, or CLAUDE.md citation is
   PRESUMED INTENTIONAL. Do not recommend removal unless you can prove
   the intent no longer holds.

3. Prior Review Delta Step — chains reviews together:

## Prior Review Delta (if reviews directory is declared in manifest)

1. Scan `docs/superpowers/reviews/` (or the path declared in the manifest)
   for review files whose `scope` overlaps with files in this diff.

2. Load the most recent matching review. From its frontmatter, extract
   `head_sha` (the commit reviewed last time).

3. Compute the intervening changes:
   `git log <prior.head_sha>..HEAD -- <touched files>`

4. Answer these questions explicitly in your output:
   - Were the prior review's "Important" or "Critical" issues fixed in
     the intervening commits?
   - Did the intervening commits introduce new issues in the same files?
   - Does any prior "approved with notes" assumption still hold given
     the new state?

5. Reference the prior review by filename in your output so the chain
   is traceable.

Component C: Review Artifact Registry

Reviews become durable, chained artifacts. This is what makes Component B Step 3 possible.

Directory: docs/superpowers/reviews/ (alongside plans/ and specs/)

Filename convention: YYYY-MM-DD-<short-sha>-<scope-slug>.md

Example: 2026-04-05-8807b5b-tier-cleanup-fix.md

• Sortable chronologically
• Tied to the exact commit reviewed (via short SHA)
• Human-readable scope hint
• No numbering system, no ID allocator, no registry service

File format:

---
base_sha: 02348a2
head_sha: 8807b5b
reviewer: code-reviewer@superpowers-5.0.7
scope:
  - src/lib/aiRouter.ts
  - src/app/api/generate/text/route.ts
  - src/app/api/generate/text/stream/route.ts
  - src/app/api/generate/refine/route.ts
concerns: [cleanup, deletion-safety, migration-half-state]
verdict: approved-with-notes
prior_review: 2026-04-03-a2b3c4d-tier-migration.md
---

# Code Review: Tier Cleanup Fix

## Context Loaded
- CLAUDE.md sections: AI Model Routing, Quick Reference / Common Bugs
- docs/pending-work.md: "Tier migration — callers still reference static map"
- Prior review 2026-04-03-a2b3c4d: approved removal of TIER_BY_ID data
  with note "callers to be migrated in follow-up"

## Delta Since Prior Review
- 3 commits between prior head 02348a2 and this review's head 8807b5b
- Prior review noted: "caller migration pending" — this diff completes it
- No new callers of getTier() added in the interval

## Strengths
- All four call sites migrated from sync getTier() to await getTierAsync()
- No new type cast shortcuts introduced
- TypeScript compiles clean

## Issues
None blocking. One minor observation: getTier() still exists as a dead
function — safe to delete in a follow-up PR since zero callers remain.

## Verdict
Ready to merge.

INDEX.md auto-regeneration: Last step of the reviewer skill scans
reviews/*.md, parses frontmatter, writes a sorted table. Humans never
edit INDEX.md directly.

Tiered persistence — not every review writes a file:

| Review type | Persistence |
|---|---|
| Per-task review in subagent-driven-development (<3 files, no deletions) | Ephemeral (inline only) |
| Review involving deletions or cleanups | Persistent |
| Review touching schema / migrations / feature flags | Persistent |
| Review touching >3 files | Persistent |
| Pre-merge gate review | Persistent |
| Final review at end of plan execution | Persistent |

The reviewer skill auto-classifies. Keeps signal high and avoids drowning
in trivial review files.

Hard rule — reviews do not review reviews. The reviewer skill MUST
exclude docs/superpowers/reviews/** from its diff scope. Otherwise the
commit that writes review A becomes a change that review B would audit,
creating an infinite regress.

Graceful Degradation

Every piece of this proposal is opt-in:

| Feature | Adoption required | Fallback behavior |
|---|---|---|
| Context Manifest | Add a section to CLAUDE.md | Reviewer reads CLAUDE.md whole; emits a one-line suggestion to adopt the manifest |
| Deletion Protocol | None — automatic | Always runs when diff contains deletions |
| Prior Review Delta | Adopt review artifact directory | Skipped silently if no reviews directory exists |
| Review Artifacts | Adopt review artifact directory | Reviews stay ephemeral (current behavior) |

Projects that adopt none of it get exactly the current behavior plus one new feature (the Deletion Protocol, which runs automatically and has no dependencies). Projects that adopt all of it get the full chain-aware, context-aware experience.

Walk-through: How This Catches the Motivating Bug

Replaying the TIER_BY_ID case with the enhanced reviewer:

Step 0 — Context Load:

• Reads CLAUDE.md, finds manifest, sees docs/pending-work.md tagged when: cleanup, deprecation
• Classifies the review as cleanup (removing dead code) and migration-review (partial AiTierConfig migration)
• Loads pending-work.md, finds the note "Tier migration — callers still reference static map"
• Summary: reviewer now knows the migration is in progress and has a specific flag about caller state

Deletion Protocol — triggered because TIER_BY_ID = []:

• Runs rg "getTier\(" --type-add 'all:*' -tall
• Finds 4 callers in aiRouter.ts, text/route.ts, text/stream/route.ts, refine/route.ts
• Half-cleanup detector fires: the data structure was emptied but getTier() accessor remains, with a cast to TierDef that hides the undefined return
• Intent preservation: the @deprecated DEAD CODE comment says "remove after 2026-04-05" — but the callers are not yet migrated, so the intent (migration complete) has not been achieved

Prior Review Delta:

• Finds 2026-04-03-a2b3c4d-tier-migration.md (the prior review that approved the partial cleanup)
• Loads its frontmatter, sees verdict: approved-with-notes, notes: "callers to be migrated in follow-up"
• Reviewer now knows the prior review explicitly deferred caller migration — and the current diff does not do that migration
• Flags as Critical: "Prior review deferred caller migration; current diff finalizes data removal without completing the deferred work"

Verdict: Critical issues found, cannot approve. Recommends either (a) completing the caller migration in this diff, or (b) deferring the data removal until callers are migrated.

The bug never ships.

Open Questions

1. Manifest format churn. The when: vocabulary will grow organically. Should it be defined in a separate file (.claude/tags.md) or evolved per-project with a starter set in the skill?

1. Scope of "any-review" fallback. Should reviews that don't match any specific tag always load all any-review entries, or should the reviewer explicitly opt in?

1. Cross-repo reviews. For projects that span multiple repos (monorepo tooling, git submodules), how should the manifest handle paths that cross repo boundaries?

1. Review artifact cleanup. Reviews accumulate over time. Is there a retention policy? (Suggestion: archive reviews older than 1 year to docs/superpowers/reviews/archive/YYYY/, keep them grep-able but out of the main index.)

1. Tiered persistence thresholds. The "> 3 files" threshold is arbitrary. Should it be configurable in the manifest? E.g.:

```

• config: review-persistence

threshold_files: 3
threshold_deletions: 1
```

Summary of Changes

New files to create:

• skills/project-context-manifest/SKILL.md — defines the manifest convention and when: vocabulary
• skills/reviews-as-artifacts/SKILL.md — defines directory layout, filename convention, frontmatter schema, auto-regeneration, tiered persistence, and the no-meta-review rule

Existing files to modify:

• skills/requesting-code-review/code-reviewer.md — add Step 0 (Context Load), Deletion Protocol, Prior Review Delta sections; add template placeholders {PROJECT_CONVENTIONS_FILE} and {PRIOR_REVIEW_REF}
• skills/requesting-code-review/SKILL.md — document the new template placeholders and the tiered persistence behavior

No breaking changes. All enhancements are additive. Projects that don't adopt the manifest get the current behavior plus the automatic Deletion Protocol (which is the highest-leverage piece on its own).

Appendix: Relationship to Other Superpowers Skills

This proposal is designed to interoperate cleanly with existing skills:

• subagent-driven-development — Per-task reviews remain ephemeral (tiered persistence). The final review at the end of a plan execution becomes persistent.
• writing-plans — Plans should cite their base review SHA in frontmatter, so the plan's execution can be chained back to the review that validated the design.
• executing-plans — Checkpoint reviews between batches can use the Prior Review Delta to measure progress against the initial plan review.
• finishing-a-development-branch — The final pre-merge review writes a persistent artifact that serves as the branch's audit record.
• systematic-debugging — Debugging sessions that discover a bug missed by review can reference the review file that missed it, creating a feedback signal for improving future reviews.

Rationale: Why This Is Worth the Added Weight

The honest trade-off: these changes add cognitive weight to the review process. Step 0 is not free. Deletion Protocol grep passes take real seconds. Review artifacts accumulate in the repo and must be regenerated in an index.

The cost is justified because:

1. Review misses are expensive. The motivating bug took 20 minutes to diagnose and a deploy cycle to fix. A few seconds of context loading per review would have prevented it.

1. Codebases grow; context fades. Solo-dev projects can get away with implicit context ("I know what my code does"). Teams and long-lived projects cannot. The Context Manifest makes project intent machine-readable so agents can participate in review at team scale.

1. Reviews want to be a running conversation. The current ephemeral model wastes the reviewer's work — every run starts from scratch. Chaining reviews via the artifact registry turns the review history into an asset that compounds over time.

1. Graceful degradation protects small projects. The opt-in design means hobby projects and experiments pay zero cost. The weight is borne only by projects that actively want the features.

---

Checklist for Implementation (for a maintainer picking this up)

• [ ] Draft skills/project-context-manifest/SKILL.md with schema and when: starter vocabulary
• [ ] Draft skills/reviews-as-artifacts/SKILL.md with directory convention, frontmatter schema, index regeneration, no-meta-review rule
• [ ] Patch skills/requesting-code-review/code-reviewer.md with Step 0, Deletion Protocol, Prior Review Delta sections
• [ ] Patch skills/requesting-code-review/SKILL.md to document new template placeholders
• [ ] Write a migration note for existing superpowers users: "Adopting the manifest is optional; the Deletion Protocol runs automatically."
• [ ] Add an example CLAUDE.md with a populated manifest block to the superpowers documentation
• [ ] Bootstrap one example review artifact in the docs, using a real historical bug as the walkthrough
• [ ] Test the enhanced reviewer against 2–3 real diffs (one addition-heavy, one deletion-heavy, one mixed) before merging

---

Prepared 2026-04-05. Grounded in a production bug encountered during active development with superpowers 5.0.7.

What Should Happen?

The code-reviewer skill should load project context before examining a diff,
treat deletions with asymmetric caution, and produce durable review artifacts
that future reviews can reference.

Specifically:

1. Before reviewing, the reviewer should read the project's conventions file

(CLAUDE.md / AGENTS.md), locate a Project Context Manifest section, and
load supplemental files whose declared tags match the current review type
(e.g. pending-work.md for cleanup reviews, models.md for schema reviews).

1. When a diff removes exported symbols or empties data structures, the

reviewer should run a repo-wide grep for the removed symbol, check for
half-cleanups where data is gone but accessor functions remain, and verify
any cited deprecation dates by searching for new callers added since the
deprecation comment was written.

1. The reviewer should produce a durable artifact (a markdown file with

frontmatter recording base_sha, head_sha, scope, and a pointer to the
prior review on the same scope) that subsequent reviews can chain to and
compute deltas against.

All three enhancements should be opt-in with graceful degradation — projects
that don't adopt the manifest or the reviews directory get current behavior
unchanged, plus the automatic Deletion Protocol.

Error Messages/Logs

Steps to Reproduce

Minimal reproduction of the class of bug this proposal addresses:

1. In a project using superpowers 5.0.7, create a dead-code cleanup PR that

empties a data structure but leaves its accessor function in place, e.g.:

/** @deprecated DEAD CODE — remove after <date> */
export const LOOKUP: Record<string, T> = {};

export function getItem(id: string): T {
return LOOKUP[id] ?? (LOOKUP["default"] as T); // cast hides undefined
}

1. Ensure active callers of getItem() exist elsewhere in the codebase

(API routes, services, etc.).

1. Also have a CLAUDE.md with conventions and a docs/pending-work.md noting

that the migration is incomplete ("callers not yet moved to the new API").

1. Dispatch the superpowers:code-reviewer subagent against the cleanup

commit using the current code-reviewer.md template.

1. The review returns an "approved" verdict without:

• Reading CLAUDE.md or pending-work.md (the template does not ask for it)
• Grepping the repository for callers of getItem()
• Noting that the accessor returns undefined when the data is empty
• Noting that the type cast hides the undefined from the type checker

1. Merge and deploy. Runtime crashes on the first call site that hits

getItem() — every call now returns undefined and the property access
throws TypeError.

The review was not wrong by its own rules; it followed the template exactly.
The template has no step that would have caught this.

---
Actual behavior:
The current code-reviewer.md template is a point-in-time diff audit. It has
no awareness of project intent (CLAUDE.md, ongoing migrations, deprecation
timelines), treats deletions symmetrically with additions (no repo-wide
callers grep, no half-cleanup detector), and has no link to prior reviews
(each run starts from scratch, cannot compute delta against intervening
commits since a previous verdict).

In the reproduction above, the reviewer correctly read the diff, saw the
deprecation comment, confirmed the type checker was happy, and approved.
Four production API routes broke at runtime.

---
Additional context:
Full proposal with root cause analysis, proposed architecture (Project Context
Manifest + Code Reviewer Enhancements + Review Artifact Registry), graceful
degradation matrix, walk-through of how the proposal catches the motivating
bug, and implementation checklist:

[link to / attach the proposal markdown file]

Target skill: superpowers:requesting-code-review
Authored against: superpowers 5.0.7
Grounded in: real production bug encountered 2026-04-05

Claude Model

Opus

Is this a regression?

No, this never worked

Last Working Version

_No response_

Claude Code Version

v2.1.90

Platform

Anthropic API

Operating System

Windows

Terminal/Shell

VS Code integrated terminal

Additional Information

We keep having code remove for working services and api on code reviews so We ran a superpowers on the code review using the bug and the removed code from our repo and Opus 4.6 can up with this solution. code review is working better now but maybe you can take this and make code review even better