Harness Codebase Cleanup

# Harness Codebase Cleanup

> Orchestrate dead code removal and architecture violation fixes with a shared convergence loop. Catches cross-concern cascades that individual skills miss.

## When to Use

- After a major refactoring or feature removal when both dead code and architecture violations are likely
- As a periodic comprehensive codebase hygiene task
- When `cleanup-dead-code` or `enforce-architecture` individually are not catching cascading issues
- When you want hotspot-aware safety classification
- NOT for quick single-concern checks -- use `cleanup-dead-code` or `enforce-architecture` directly
- NOT when tests are failing -- fix tests first
- NOT during active feature development

## Flags

| Flag                  | Effect                                                            |
| --------------------- | ----------------------------------------------------------------- |
| `--fix`               | Enable convergence-based auto-fix (default: detect + report only) |
| `--dead-code-only`    | Skip architecture checks                                          |
| `--architecture-only` | Skip dead code checks                                             |
| `--dry-run`           | Show what would be fixed without applying                         |
| `--ci`                | Non-interactive: apply safe fixes only, report everything else    |

## Process

### Phase 1: CONTEXT -- Build Hotspot Map

1. **Run hotspot detection** via git log analysis:
   ```bash
   git log --format=format: --name-only --since="6 months ago" | sort | uniq -c | sort -rn | head -50
   ```
2. **Build churn map.** Parse output into a `file -> commit count` mapping.
3. **Compute top 10% threshold.** Sort all files by commit count. The file at the 90th percentile defines the threshold. Files above this threshold are "high churn."
4. **Store as HotspotContext** for use in Phase 3 (CLASSIFY).

Run `detect_stale_constraints` to find architectural constraints referencing removed or restructured code. Run `detect_anomalies` to identify structural irregularities that should be addressed during cleanup.

### Phase 2: DETECT -- Run Both Concerns in Parallel

1. **Dead code detection** (skip if `--architecture-only`):
   - Run `harness cleanup --type dead-code --json`
   - Captures: dead files, dead exports, unused imports, dead internals, commented-out code blocks, orphaned dependencies

2. **Architecture detection** (skip if `--dead-code-only`):
   - Run `harness check-deps --json`
   - Captures: layer violations, forbidden imports, circular dependencies, import ordering issues

3. **Merge findings.** Convert all raw findings into `CleanupFinding` objects using `classifyFinding()`. This normalizes both concerns into a shared schema.

### Phase 3: CLASSIFY -- Safety Classification and Dedup

1. **Apply safety classification.** Each `CleanupFinding` already has a safety level from `classifyFinding()`. Review the classification rules:

   **Dead code safety:**

   | Finding                   | Safety        | Condition                                   |
   | ------------------------- | ------------- | ------------------------------------------- |
   | Dead files                | Safe          | Not entry point, no side effects            |
   | Unused imports            | Safe          | Zero references                             |
   | Dead exports (non-public) | Safe          | Zero importers, not in package entry point  |
   | Dead exports (public API) | Unsafe        | In package entry point or published package |
   | Commented-out code        | Safe          | Always (code is in git history)             |
   | Orphaned npm deps         | Probably safe | Needs install + test verification           |
   | Dead internals            | Unsafe        | Cannot reliably determine all callers       |

   **Architecture safety:**

   | Violation                           | Safety        | Condition                  |
   | ----------------------------------- | ------------- | -------------------------- |
   | Import ordering                     | Safe          | Mechanical reorder         |
   | Forbidden import (with alternative) | Probably safe | 1:1 replacement configured |
   | Forbidden import (no alternative)   | Unsafe        | Requires restructuring     |
   | Design token (unambiguous)          | Probably safe | Single token match         |
   | Design token (ambiguous)            | Unsafe        | Multiple candidates        |
   | Upward dependency                   | Unsafe        | Always                     |
   | Skip-layer dependency               | Unsafe        | Always                     |
   | Circular dependency                 | Unsafe        | Always                     |

2. **Apply hotspot downgrade.** For each finding, check if the file is in the top 10% by churn (from Phase 1 HotspotContext). If so, downgrade `safe` to `probably-safe`. Do not downgrade `unsafe` findings.

3. **Cross-concern dedup.** Call `deduplicateFindings()` to merge overlapping findings:
   - A dead import from a forbidden layer = one finding (dead-code concern, noting architecture overlap)
   - A dead file that has architecture violations = one finding (dead-code, noting violations resolved by deletion)

### Phase 4: FIX -- Convergence Loop

**Only runs when `--fix` flag is set.** Without `--fix`, skip to Phase 5 (REPORT).

```
findings = classified findings from Phase 3
previousCount = findings.length
iteration = 0

while iteration < 5:
  iteration++

  # Batch 1: Apply safe fixes silently
  safeFixes = findings.filter(f => f.safety === 'safe')
  apply(safeFixes)

  # Batch 2: Present probably-safe fixes
  if --ci mode:
    skip probably-safe fixes (report only)
  else:
    probablySafeFixes = findings.filter(f => f.safety === 'probably-safe')
    presentAsDiffs(probablySafeFixes)
    apply(approved fixes)

  # Verify: lint + typecheck + test
  verifyResult = run("pnpm lint && pnpm tsc --noEmit && pnpm test")

  if verifyResult.failed:
    revertBatch()
    reclassify failed fixes as unsafe
    continue

  # Re-detect both concerns
  newFindings = runDetection()  # Phase 2 again
  newFindings = classify(newFindings)  # Phase 3 again

  if newFindings.length >= previousCount:
    break  # No progress, stop

  previousCount = newFindings.length
  findings = newFindings
```

**Verification gate:** Every fix batch must pass lint + typecheck + test. If verification fails:

1. Revert the entire batch (use git: `git checkout -- .`)
2. Reclassify all findings in the batch as `unsafe`
3. Continue the loop with remaining findings

**Cross-concern cascade examples:**

- Dead import from forbidden layer: removing the dead import also resolves the architecture violation. Single fix, both resolved.
- Architecture fix creates dead code: replacing a forbidden import makes the old module's export dead. Next detect cycle catches it.
- Dead file resolves multiple violations: deleting a dead file that imports from wrong layers resolves those violations too.

### Phase 5: REPORT -- Actionable Output

Generate a structured report with two sections:

**1. Fixes Applied:**
For each fix that was applied:

- File and line
- What was fixed (finding type and description)
- What action was taken (delete, replace, reorder)
- Verification status (pass/fail)

**2. Remaining Findings (requires human action):**
For each unsafe finding that was not auto-fixed:

- **What is wrong:** The finding type, file, line, and description
- **Why it cannot be auto-fixed:** The safety reason and classification logic
- **Suggested approach:** Concrete next steps for manual resolution

Example report output:

```
=== HARNESS CODEBASE CLEANUP REPORT ===

Fixes applied: 12
  - 5 unused imports removed (safe)
  - 3 dead exports de-exported (safe)
  - 2 commented-out code blocks deleted (safe)
  - 1 forbidden import replaced (probably-safe, approved)
  - 1 orphaned dependency removed (probably-safe, approved)

Convergence: 3 iterations, 12 → 8 → 3 → 3 (stopped)

Remaining findings: 3 (require human action)

  1. UNSAFE: Circular dependency
     File: src/services/order-service.ts <-> src/services/inventory-service.ts
     Why: Circular dependencies require structural refactoring
     Suggested: Extract shared logic into src/services/stock-calculator.ts

  2. UNSAFE: Dead internal function
     File: src/utils/legacy.ts:45 — processLegacyFormat()
     Why: Cannot reliably determine all callers (possible dynamic usage)
     Suggested: Search for string references, check config files, then delete if confirmed unused

  3. UNSAFE: Public API dead export
     File: packages/core/src/index.ts — legacyHelper
     Why: Export is in package entry point; external consumers may depend on it
     Suggested: Deprecate with @deprecated JSDoc tag, remove in next major version
```

## Rationalizations to Reject

These are common rationalizations that sound reasonable but lead to incorrect results. When you catch yourself thinking any of these, stop and follow the documented process instead.

| Rationalization                                                                                               | Why It Is Wrong                                                                                                                                  |
| ------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------ |
| "This dead export is in a high-churn file but the removal is clearly safe"                                    | High-churn files have more hidden consumers. Safe findings in the top 10% by churn are downgraded to probably-safe, requiring explicit approval. |
| "The convergence loop is not reducing findings quickly enough, so I will apply unsafe fixes to make progress" | Unsafe findings are never auto-fixed, regardless of convergence pressure. Each requires human judgment.                                          |
| "The verification gate failed on a probably-safe fix, but I am confident the fix is correct"                  | When verification fails after a fix batch, the entire batch must be reverted and all findings reclassified as unsafe.                            |
| "I will skip the hotspot context phase since it adds time and the churn data is just supplementary"           | The hotspot map drives safety classification accuracy. Without it, safe fixes in high-churn areas are not downgraded.                            |

## Examples

### Example: Post-Refactoring Cleanup

After removing the `legacy-auth` module:

1. **Phase 1 (CONTEXT):** Hotspot analysis shows `src/services/auth.ts` has 42 commits (top 5%).
2. **Phase 2 (DETECT):** Dead code detects 3 dead exports in `src/utils/token.ts` (were only used by legacy-auth). Architecture detects 1 forbidden import in `src/services/session.ts` (still importing from removed module's location).
3. **Phase 3 (CLASSIFY):** Dead exports classified as `safe` but downgraded to `probably-safe` because `token.ts` is in a high-churn file. Forbidden import classified as `unsafe` (no alternative configured).
4. **Phase 4 (FIX):** First iteration removes 3 dead exports (approved as probably-safe). Re-detect finds `token.ts` now has zero exports and becomes a dead file. Second iteration deletes the dead file. Convergence stops -- the forbidden import requires manual restructuring.
5. **Phase 5 (REPORT):** 4 fixes applied (3 dead exports + 1 dead file), 1 remaining finding (forbidden import requiring restructuring).

## Harness Integration

- **`harness cleanup --type dead-code --json`** -- Dead code detection input
- **`harness check-deps --json`** -- Architecture violation detection input
- **`git log` analysis** -- Hotspot context for safety classification (inline command, no skill invocation needed)
- **`harness validate`** -- Final validation after all fixes
- **`harness check-deps`** -- Final architecture check after all fixes

## Success Criteria

- All safe fixes are applied without test failures
- Probably-safe fixes are presented as diffs for approval (or skipped in CI mode)
- Unsafe findings are never auto-fixed
- Convergence loop catches cross-concern cascades
- Report includes actionable guidance for every remaining finding
- `harness validate` passes after cleanup

## Escalation

- **When convergence loop does not converge after 5 iterations:** The codebase has deeply tangled issues. Stop and report all remaining findings. Consider breaking the cleanup into focused sessions.
- **When a safe fix causes test failures:** The classification was wrong. Revert, reclassify as unsafe, and investigate the hidden dependency. Document the false positive for future improvement.
- **When the hotspot detector is unavailable:** Skip the hotspot downgrade. All safety classifications use their base level without churn context.
- **When dead code and architecture fixes conflict:** The convergence loop handles this naturally. If removing dead code creates an architecture issue (rare), the next detection cycle catches it.