code-coherence-review

Purpose

A correct function that violates a naming rule ships with a warning. A rule-compliant function with broken logic cannot ship. This skill checks what matters most: does the code make sense, survive real-world input, and respect the project's boundaries? Run it first — nothing else matters until the code is coherent.

Procedure

1. Initialize the progress file. Create .memory/reviews/review-coherence-<timestamp>.md:

   # Coherence Review Progress
   
   ## Status
   - Last updated: <timestamp>
   - Overall: In Progress
   
   ## Phases
   - [ ] 1. Logic coherence
   - [ ] 2. Dead code, obsolete artifacts, and comments
   - [ ] 3. Correctness
   - [ ] 4. Structural coherence
   - [ ] 5. Duplication detection
   
   ## Findings
   

2. Logic coherence. Read the work as a narrative. Trace the flow from entry point to exit. Check:

   • Does the algorithm solve what the task brief says it should?
   • Are there circular logic paths or infinite loops?
   • Do the data structures fit the problem, or is the code fighting its own model?
   • For plans: check for ambiguity (instructions that can be read two ways), logical gaps (steps that assume preconditions without establishing them), redundancy (duplicate steps), and contradictions.
   • For code and other non-plan artifacts: review against the task brief's acceptance criteria. Stress-test for blindspots, ambiguity, and false assumptions.

   Write any findings to the progress file under ## Findings with the heading ### Logic coherence. Mark phase 1 as [x].

3. Dead code, obsolete artifacts, and comments. Scan every changed file for code that is no longer useful or reachable, and for comments that did not earn their place. For each file, check:

   • Unused functions and methods — functions or methods defined but never called anywhere in the codebase. Run rg -n 'func <name>' or rg -n 'def <name>' to find definitions, then verify with rg '<name>\(' that they are actually invoked elsewhere. If a function has zero callers, it is dead.
   • Unused variables and constants — variables or constants assigned but never read. Trace each assignment to its usage sites. Variables declared and initialized but never referenced in any subsequent statement are dead.
   • Unused imports — import statements for modules, packages, or symbols that are never referenced in the file. Compare every import against actual usage in the file body.
   • Unreachable code — code after return, break, continue, raise, exit, or panic statements within the same block. Code in conditional branches that can never be true (e.g., if false, if 1 == 0, or branches contradicted by earlier guards).
   • Commented-out code — commented code of any length. Version control holds old code, not inline comments; no justification preserves a commented-out block. Flag for removal.
   • Explanatory comments — comments that restate, narrate, or explain what code does. The fix is a rename, an extraction, or a type — not the comment. Do not flag a comment that explains why under an external constraint no code can express (see the Comments section in rules/code/general.md), and do not flag a comment a domain rule or tool contract mandates (shell headers, suppression justifications) — those are structure, not confession.
   • Deprecated or superseded logic — code paths replaced by newer implementations but not removed. Check for conditional branches that always take one path because a feature flag is permanent, or old implementations kept "just in case" with no callers.