← back to Cli Printing Press
feat(cli): add printing-press polish --remove-dead-code (#105)
2c9d57d7d28358ba25163bf250d803048ef8f806 · 2026-04-01 17:03:53 -0700 · Trevin Chow
Deterministic dead-code removal using Go AST parsing. Runs dogfood to
identify dead functions, parses each file's AST, removes the function
declarations, writes back with go/printer, then verifies go build passes.
If build fails after removal (false positive from dogfood), restores the
original files and reports the error. Supports --dry-run for preview.
This replaces unreliable LLM instructions ("don't write dead code") with
a deterministic binary pass that catches dead code every time.
Includes Steam Run 5 retro and auto-dead-code-removal plan.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Files touched
A docs/plans/2026-04-01-004-feat-auto-dead-code-removal-plan.mdA docs/retros/2026-04-01-steam-run5-retro.mdA internal/cli/polish.goM internal/cli/root.goA internal/pipeline/polish.goA internal/pipeline/polish_test.go
Diff
commit 2c9d57d7d28358ba25163bf250d803048ef8f806
Author: Trevin Chow <trevin@trevinchow.com>
Date: Wed Apr 1 17:03:53 2026 -0700
feat(cli): add printing-press polish --remove-dead-code (#105)
Deterministic dead-code removal using Go AST parsing. Runs dogfood to
identify dead functions, parses each file's AST, removes the function
declarations, writes back with go/printer, then verifies go build passes.
If build fails after removal (false positive from dogfood), restores the
original files and reports the error. Supports --dry-run for preview.
This replaces unreliable LLM instructions ("don't write dead code") with
a deterministic binary pass that catches dead code every time.
Includes Steam Run 5 retro and auto-dead-code-removal plan.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
...6-04-01-004-feat-auto-dead-code-removal-plan.md | 128 +++++++++
docs/retros/2026-04-01-steam-run5-retro.md | 88 +++++++
internal/cli/polish.go | 115 ++++++++
internal/cli/root.go | 1 +
internal/pipeline/polish.go | 290 +++++++++++++++++++++
internal/pipeline/polish_test.go | 231 ++++++++++++++++
6 files changed, 853 insertions(+)
diff --git a/docs/plans/2026-04-01-004-feat-auto-dead-code-removal-plan.md b/docs/plans/2026-04-01-004-feat-auto-dead-code-removal-plan.md
new file mode 100644
index 00000000..50cbf845
--- /dev/null
+++ b/docs/plans/2026-04-01-004-feat-auto-dead-code-removal-plan.md
@@ -0,0 +1,128 @@
+---
+title: "feat: Deterministic dead-code removal via printing-press polish"
+type: feat
+status: active
+date: 2026-04-01
+origin: docs/retros/2026-04-01-steam-run5-retro.md
+---
+
+# Deterministic Dead-Code Removal
+
+## Overview
+
+Add a `printing-press polish --remove-dead-code` command that runs dogfood to identify dead functions and flags, then uses Go's AST parser to surgically remove them from the generated CLI. This replaces unreliable LLM instructions ("don't write dead code") with a deterministic binary pass that catches dead code every time — regardless of what the agent did during the build phase.
+
+## Problem Frame
+
+Across 5 Steam runs, dead code appeared in every single generated CLI. The agent that builds wrapper commands sometimes defines helper functions that end up unused. Dogfood catches them, but removal requires either manual intervention or a polish pass by Claude — which is unreliable. The retro concluded: "move quality enforcement from LLM instructions (unreliable) to deterministic binary passes (reliable)."
+
+(see origin: docs/retros/2026-04-01-steam-run5-retro.md — Key Insight section)
+
+## Requirements Trace
+
+- R1. `printing-press polish --remove-dead-code --dir <cli-dir>` removes all dead functions identified by dogfood
+- R2. Removal uses Go AST parsing — not regex or string manipulation — to delete function nodes cleanly
+- R3. After removal, `go build ./...` must pass (safety net: if removing a function breaks the build, it wasn't actually dead — dogfood had a false positive)
+- R4. The command reports what was removed and verifies the build
+
+## Scope Boundaries
+
+- This plan covers dead FUNCTIONS only (from `helpers.go` and other CLI files). Dead FLAGS are a separate concern with different removal mechanics (editing cobra registration in `root.go`).
+- Not changing the dogfood detection logic — it already works. This plan adds removal.
+- Not adding this as an automatic post-generation step yet. It's a manual command first. Automation can come later after validation.
+- Not removing `usageErr` from the template (that's a separate template fix). This command handles whatever dead code exists in a generated CLI, regardless of source.
+
+## Context & Research
+
+### Relevant Code and Patterns
+
+- `internal/pipeline/dogfood.go:420` — `checkDeadFunctions()` identifies dead functions by name. Returns `DeadCodeResult{Items: []string{"funcName1", "funcName2"}}`.
+- `internal/cli/dogfood.go` — existing CLI command pattern. `polish` would follow the same structure.
+- Go standard library: `go/parser`, `go/ast`, `go/printer`, `go/token` — all needed for AST-based removal.
+- Generated CLIs have dead functions in `internal/cli/helpers.go` (from templates) and in command files (from Claude's build phase).
+
+### Key Constraint
+
+Dogfood currently only scans `helpers.go` for dead functions. Claude-introduced dead code (like `formatCompact`) may be in other files under `internal/cli/`. The dead-function scanner may need to be extended to scan ALL `.go` files in `internal/cli/`, not just `helpers.go`. **Defer this decision to implementation** — check whether `checkDeadFunctions` already scans all files or just `helpers.go`.
+
+## Key Technical Decisions
+
+- **Go AST over regex:** Regex can find function definitions but can't cleanly remove them (comments above the function, blank lines after, associated doc comments). Go's AST parser handles all of this correctly — `ast.File.Decls` contains each function as a discrete node that can be removed.
+
+- **Safety net via `go build`:** After removing functions, run `go build ./...` on the CLI directory. If the build fails, a "dead" function was actually called through a path dogfood missed (false positive). Restore the file and report the error. This makes the command safe to run even if dogfood has bugs.
+
+- **Manual command first, not automatic:** The `/printing-press` skill can call this command after the build phase, but it shouldn't be hardwired into the generator or dogfood. The user or skill decides when to run it. This prevents the tool from deleting code the user intended to keep.
+
+## Open Questions
+
+### Resolved During Planning
+
+- **Should it modify files in place or write to a temp dir?** In place — the CLI directory is a working copy, not a precious artifact. The `go build` safety net catches mistakes.
+
+### Deferred to Implementation
+
+- **Does `checkDeadFunctions` scan all CLI files or just helpers.go?** Read the function. If just helpers.go, the removal command should also only target helpers.go initially. Extending to all files is a follow-up.
+- **Should dead flags also be removed?** Flags require editing `root.go`'s `PersistentFlags()` registration, which is more complex. Defer to a follow-up.
+
+## Implementation Units
+
+- [ ] **Unit 1: Add `printing-press polish` subcommand with `--remove-dead-code` flag**
+
+**Goal:** A new CLI command that runs dogfood dead-function detection, then AST-removes the identified functions
+
+**Requirements:** R1, R2, R3, R4
+
+**Dependencies:** None
+
+**Files:**
+- Create: `internal/cli/polish.go`
+- Create: `internal/pipeline/polish.go`
+- Test: `internal/pipeline/polish_test.go`
+- Modify: `internal/cli/root.go` (register the command)
+
+**Approach:**
+- The CLI command (`internal/cli/polish.go`) handles flags (`--dir`, `--remove-dead-code`, `--json`, `--dry-run`) and delegates to `pipeline.RunPolish()`.
+- The pipeline function (`internal/pipeline/polish.go`) orchestrates: (1) run `checkDeadFunctions` to get the dead list, (2) for each dead function, parse the file's AST, find the function declaration, remove it from `ast.File.Decls`, (3) write the file back with `go/printer`, (4) run `go build ./...` as safety net, (5) if build fails, restore the original file.
+- `--dry-run` reports what would be removed without modifying files.
+- Output: list of removed functions, build verification result.
+
+**Patterns to follow:**
+- `internal/cli/dogfood.go` for CLI command structure
+- `internal/pipeline/dogfood.go` for pipeline function structure
+- Go standard library `go/parser` + `go/ast` + `go/printer` for AST manipulation
+
+**Test scenarios:**
+- Happy path: helpers.go with 2 dead functions (defined but never called in any other file) → both removed, file rewritten, `go build` passes
+- Happy path: helpers.go with 0 dead functions → no changes, clean output
+- Edge case: removing a function that has a doc comment above it → comment also removed (AST handles this)
+- Error path: dogfood falsely identifies a function as dead (it's actually called through an interface or reflection) → `go build` fails after removal → original file restored, error reported
+- Edge case: `--dry-run` → reports what would be removed, no files modified
+- Integration: run on the Steam CLI after a fresh generation + build → dead functions removed, scorecard dead_code improves
+
+**Verification:**
+- `printing-press polish --remove-dead-code --dir <steam-cli>` removes `usageErr` and `formatCompact`
+- `go build ./...` passes after removal
+- `printing-press dogfood` reports 0 dead functions after polish
+
+## System-Wide Impact
+
+- **New command surface:** Adds `printing-press polish` as a new subcommand. No changes to existing commands.
+- **Dogfood reuse:** Uses `checkDeadFunctions` from the dogfood package — no duplication.
+- **Generated CLI files modified:** The command writes to generated CLI files. This is intentional — it's a post-generation polish step.
+- **Unchanged:** Generator templates, scorecard, verify, dogfood detection logic.
+
+## Risks & Dependencies
+
+| Risk | Mitigation |
+|------|------------|
+| AST removal leaves broken formatting | `go/printer` with `format.Source` produces correctly formatted output |
+| Dogfood false positive → build breaks after removal | Safety net: run `go build`, restore on failure |
+| Function removal breaks an import (removed function was the only user of a package) | `goimports` or `go build` will flag unused imports; the safety net catches this |
+
+## Sources & References
+
+- **Origin:** [docs/retros/2026-04-01-steam-run5-retro.md](docs/retros/2026-04-01-steam-run5-retro.md) — Key Insight section
+- Dogfood dead-function detection: `internal/pipeline/dogfood.go:420`
+- CLI command pattern: `internal/cli/dogfood.go`
+- Go AST docs: `go/ast`, `go/parser`, `go/printer`, `go/token` (standard library)
+- PRs #100-104 (mvanhorn/cli-printing-press)
diff --git a/docs/retros/2026-04-01-steam-run5-retro.md b/docs/retros/2026-04-01-steam-run5-retro.md
new file mode 100644
index 00000000..59948810
--- /dev/null
+++ b/docs/retros/2026-04-01-steam-run5-retro.md
@@ -0,0 +1,88 @@
+# Printing Press Retro: Steam Web API (Run 5)
+
+## Session Stats
+- API: Steam Web API
+- Spec source: Zuplo/Steam-OpenAPI (158 operations, OpenAPI 3.0)
+- Scorecard: **87/100 Grade A** (up from 84 in Run 4)
+- Verify: 75% (61/81, 0 critical)
+- Machine PRs: #100, #101, #102, #103, #104
+- Journey: 68 → 84 → 85 → 84 → 87
+
+## What PR #104 Changed
+- **Sync Correctness: 7 → 10** (+3). The rescale for APIs without parameterized list endpoints works correctly. This is the single biggest score improvement from one change across all 5 runs.
+- **usageErr gate: did NOT work.** `HasMultiPositional` is true for the Steam spec (some endpoints technically have 2+ path params in the Zuplo spec), so `usageErr` is still emitted. But no generated command calls it because the help-guard pattern replaced all usageErr calls. The gating logic checks the spec, but the real signal is whether generated commands actually call the function.
+- **Profiler searchable fields from GET params: uncertain.** Need to check if this actually produced more FTS5 tables. Data Pipeline is still 7/10.
+
+## Findings
+
+### 1. usageErr gate: wrong signal (Generator bug — scorer correct)
+
+- **Scorer correct?** Yes. `usageErr` is defined but never called. Dead code.
+- **What went wrong:** PR #104 gates `usageErr` behind `HasMultiPositional` (true when any endpoint in the spec has 2+ positional params). The Zuplo Steam spec has some endpoints with 2+ path params, so the flag is true. But the help-guard pattern (PR #100) replaced all `usageErr` calls with `cmd.Help()` in the endpoint template. So `HasMultiPositional` is true (spec has the params) but no generated command emits a call to `usageErr` (template uses help-guard instead).
+- **Root cause:** The flag checks what the spec HAS. The real signal is what the template EMITS. Since the help-guard catches `len(args) == 0` and the 2nd-arg check at `{{if gt $i 0}}` still calls `usageErr`, the function should only be emitted when there are actually commands with 2+ positional params that use the `{{if gt $i 0}}` path. But the endpoint template's help-guard catches ALL zero-arg cases — and for Steam, no command has 2 positional params, so the `gt $i 0` path never fires.
+- **Fix:** The simplest fix: remove `usageErr` from the template entirely. Replace `usageErr(fmt.Errorf(...))` at line 56 of `command_endpoint.go.tmpl` with `fmt.Errorf(...)` wrapped in a non-zero exit. Or: keep `usageErr` but gate it on whether ANY generated command file actually contains the string `usageErr(` — a post-generation dead-code sweep.
+- **Verdict: Generator template fix needed. The conditional flag approach doesn't work because it checks the spec, not the generated output.**
+
+### 2. formatCompact: Claude-introduced dead code (Agent compliance issue)
+
+- **Scorer correct?** Yes. `formatCompact` is defined but never called.
+- **What happened:** The agent that builds wrapper/transcendence commands defined `formatCompact` in one of the command files but no command actually calls it. This is a recurring pattern — Claude writes helper functions during the build phase that end up unused.
+- **Root cause:** The agent builds commands in bulk and sometimes adds helpers "just in case" that turn out unused. The dogfood catches them, but the agent should have verified each function is called before leaving it in.
+- **Fix:** Two paths: (1) Add a post-build step to the skill instruction: "After building all commands, run dogfood and remove any dead functions before proceeding to shipcheck." (2) Make the agent prompt explicitly say "do not define helper functions unless they are called by at least one command."
+- **Verdict: Skill instruction improvement. The agent prompt should say "verify every helper function is called."**
+
+### 3. README 7/10 despite having all sections (Scorer investigation needed)
+
+- **Scorer correct?** Need to investigate. The README has all 5 required sections + 3 extras. If the scorer is giving 7/10 with all sections present, the deduction is for content quality within those sections, not missing sections.
+- **What to check:** Run the scorecard with verbose README output to see which quality checks failed. The scorer may check: (a) Quick Start has a real command example (not just "see Install"), (b) Cookbook has 3+ code blocks, (c) Agent Usage has --json/--select examples.
+- **Verdict: Need to trace the scorer's README logic to understand the 3-point deduction.**
+
+### 4. Data Pipeline 7/10 — profiler search improvement uncertain
+
+- **What happened:** PR #104 added GET param analysis to `collectStringFields`, but Data Pipeline is still 7/10. This suggests either: (a) the profiler change didn't produce more FTS5 tables for Steam, or (b) the scorer checks for something else that's still missing.
+- **Verdict: Need to check if the profiler produces more SearchableFields for the Steam spec with the GET param addition.**
+
+## Key Insight: Don't Rely on Instructions for Quality Enforcement
+
+An earlier draft of this retro said "agent compliance is the new bottleneck" and proposed fixing it with better skill instructions. That framing is wrong.
+
+**Skill instructions are suggestions to an LLM, not deterministic code.** Evidence from this session:
+
+| Instruction | Did the agent follow it? | Result |
+|-------------|--------------------------|--------|
+| "Preserve all 5 README sections" | Yes — all 5 sections present | But content quality still scored 7/10. Structure followed, quality didn't. |
+| "Verify every helper function is called" | No — `formatCompact` defined and never called | Agent built in bulk, didn't verify its own output |
+| "Wire auth from research when spec detection fails" | Unknown — didn't check | May or may not have fired this run |
+
+Instructions work for **structural rules** ("always include these sections") ~80% of the time. They are unreliable for **quality judgment** ("verify every function is called", "enrich terse descriptions"). The agent will sometimes follow them. Not always.
+
+**What actually works reliably:**
+
+| Approach | Reliability | Example |
+|----------|-------------|---------|
+| **Template gates** | 100% deterministic | `{{if .HasMultiPositional}}` always fires correctly |
+| **Binary post-processing** | 100% deterministic | `printing-press dogfood` catches dead code every time |
+| **Scorer checks** | 100% deterministic | Scorecard detects missing README sections mechanically |
+| **Skill instructions** | ~70-90% | Agent sometimes follows, sometimes doesn't |
+
+**The pattern: move quality enforcement from LLM instructions (unreliable) to deterministic binary passes (reliable).**
+
+Concrete fixes that follow this pattern:
+- **Dead code:** Don't tell the agent "don't write dead code." Instead, add `printing-press polish --remove-dead-code` as a binary pass that dogfood identifies dead functions and auto-deletes them. Run it after the build phase, before scoring.
+- **README quality:** Don't hope the agent writes good cookbook examples. Have the scorer validate: cookbook section has 3+ parseable code blocks, Quick Start has a real command, Agent Usage shows --json. Have `printing-press polish` auto-fix thin sections by regenerating them from the CLI's `--help` output.
+- **Auth compensation:** Don't rely on the agent reading the research brief and wiring auth. Have the generator detect the pattern (PR #103's auth inference) and the verify tool report when auth is needed but not configured. Make it a build failure, not a suggestion.
+- **usageErr:** Don't gate it on a flag that checks the spec. Remove it from the template entirely and let `cmd.Help()` handle all zero-arg cases. If a future template reintroduces `usageErr` calls, the build fails — which is the right signal.
+
+**The remaining 13 points should be pursued through binary passes and template fixes, not through more skill instructions.** Instructions are cheap to add and feel productive, but they don't compound reliably. Binary passes compound because they're deterministic.
+
+## Score Trajectory
+
+| Run | Score | Key change |
+|-----|-------|------------|
+| 1 | 68 | Baseline |
+| 2 | 84 | Scorer fixes (verify naming, dogfood) |
+| 3 | 85 | Behavioral detection |
+| 4 | 84 | Auth inference, sync paths |
+| **5** | **87** | **Sync rescale (+3), improved profiler** |
+
+**19-point improvement over 5 runs.** The improvement came from deterministic machine changes (template gates, scorer fixes, binary post-processing), not from skill instructions. The next step is: (1) add `printing-press polish --remove-dead-code` as a binary pass, (2) remove `usageErr` from the template, (3) test on a different API to validate generalization.
diff --git a/internal/cli/polish.go b/internal/cli/polish.go
new file mode 100644
index 00000000..2da9df2c
--- /dev/null
+++ b/internal/cli/polish.go
@@ -0,0 +1,115 @@
+package cli
+
+import (
+ "encoding/json"
+ "fmt"
+ "os"
+ "path/filepath"
+
+ "github.com/mvanhorn/cli-printing-press/internal/pipeline"
+ "github.com/spf13/cobra"
+)
+
+func newPolishCmd() *cobra.Command {
+ var dir string
+ var removeDeadCode bool
+ var asJSON bool
+ var dryRun bool
+
+ cmd := &cobra.Command{
+ Use: "polish",
+ Short: "Deterministic post-generation quality fixes",
+ Long: "Run deterministic quality fixes on a generated CLI. Unlike LLM instructions, these are mechanical transformations that produce the same result every time.",
+ Example: ` # Preview dead code that would be removed
+ printing-press polish --remove-dead-code --dir ./steam-web-pp-cli --dry-run
+
+ # Remove dead functions and verify build
+ printing-press polish --remove-dead-code --dir ./steam-web-pp-cli
+
+ # JSON output for programmatic use
+ printing-press polish --remove-dead-code --dir ./steam-web-pp-cli --json`,
+ RunE: func(cmd *cobra.Command, args []string) error {
+ if !removeDeadCode {
+ return fmt.Errorf("specify at least one polish action (e.g., --remove-dead-code)")
+ }
+
+ if removeDeadCode {
+ result, err := pipeline.RemoveDeadCode(dir, dryRun)
+ if err != nil {
+ return &ExitError{Code: ExitGenerationError, Err: fmt.Errorf("polish: %w", err)}
+ }
+
+ if asJSON {
+ enc := json.NewEncoder(os.Stdout)
+ enc.SetIndent("", " ")
+ return enc.Encode(result)
+ }
+
+ printPolishResult(result)
+ }
+
+ return nil
+ },
+ }
+
+ cmd.Flags().StringVar(&dir, "dir", "", "Path to the generated CLI directory (required)")
+ cmd.Flags().BoolVar(&removeDeadCode, "remove-dead-code", false, "Remove dead functions identified by dogfood")
+ cmd.Flags().BoolVar(&asJSON, "json", false, "Output as JSON")
+ cmd.Flags().BoolVar(&dryRun, "dry-run", false, "Report what would be removed without modifying files")
+ _ = cmd.MarkFlagRequired("dir")
+ return cmd
+}
+
+func printPolishResult(r *pipeline.PolishResult) {
+ name := filepath.Base(r.Dir)
+
+ if r.DryRun {
+ fmt.Printf("Polish (dry run): %s\n", name)
+ } else {
+ fmt.Printf("Polish: %s\n", name)
+ }
+ fmt.Println("================================")
+
+ if len(r.DeadFunctions) == 0 {
+ fmt.Println("\nNo dead functions found. Nothing to remove.")
+ return
+ }
+
+ fmt.Printf("\nDead functions found: %d\n", len(r.DeadFunctions))
+ for _, fn := range r.DeadFunctions {
+ fmt.Printf(" - %s\n", fn)
+ }
+
+ if r.DryRun {
+ fmt.Println("\nDry run — no files modified.")
+ return
+ }
+
+ if len(r.Removed) > 0 {
+ fmt.Printf("\nRemoved: %d functions\n", len(r.Removed))
+ for _, fn := range r.Removed {
+ fmt.Printf(" - %s\n", fn)
+ }
+ }
+
+ if len(r.Restored) > 0 {
+ fmt.Printf("\nRestored (build failed after removal): %d functions\n", len(r.Restored))
+ for _, fn := range r.Restored {
+ fmt.Printf(" - %s\n", fn)
+ }
+ fmt.Printf("\nBuild error:\n%s\n", r.BuildError)
+ }
+
+ if r.BuildVerified {
+ fmt.Println("\nBuild verified: PASS")
+ }
+
+ fmt.Printf("\nVerdict: ")
+ if len(r.Removed) > 0 && r.BuildVerified {
+ fmt.Printf("CLEANED (%d dead functions removed)\n", len(r.Removed))
+ } else if len(r.Restored) > 0 {
+ fmt.Println("REVERTED (build failed, all changes restored)")
+ } else {
+ fmt.Println("CLEAN (no dead code)")
+ }
+}
diff --git a/internal/cli/root.go b/internal/cli/root.go
index 12821515..116b52e3 100644
--- a/internal/cli/root.go
+++ b/internal/cli/root.go
@@ -50,6 +50,7 @@ func Execute() error {
rootCmd.AddCommand(newCatalogCmd())
rootCmd.AddCommand(newLibraryCmd())
rootCmd.AddCommand(newPublishCmd())
+ rootCmd.AddCommand(newPolishCmd())
return rootCmd.Execute()
}
diff --git a/internal/pipeline/polish.go b/internal/pipeline/polish.go
new file mode 100644
index 00000000..efe0f7c0
--- /dev/null
+++ b/internal/pipeline/polish.go
@@ -0,0 +1,290 @@
+package pipeline
+
+import (
+ "fmt"
+ "go/ast"
+ "go/parser"
+ "go/printer"
+ "go/token"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "regexp"
+ "sort"
+ "strings"
+)
+
+// PolishResult describes what the polish pass did.
+type PolishResult struct {
+ Dir string `json:"dir"`
+ DeadFunctions []string `json:"dead_functions_found"`
+ Removed []string `json:"removed"`
+ BuildVerified bool `json:"build_verified"`
+ BuildError string `json:"build_error,omitempty"`
+ Restored []string `json:"restored,omitempty"`
+ DryRun bool `json:"dry_run"`
+}
+
+// RemoveDeadCode finds dead functions across ALL CLI files, then AST-removes them.
+// Loops until no dead functions remain (max 3 passes to catch chained dead code).
+// If dryRun is true, reports what would be removed without modifying files (1 pass only).
+func RemoveDeadCode(dir string, dryRun bool) (*PolishResult, error) {
+ result := &PolishResult{
+ Dir: dir,
+ DryRun: dryRun,
+ }
+
+ cliDir := filepath.Join(dir, "internal", "cli")
+ cmdDir := filepath.Join(dir, "cmd")
+
+ // Validate the directory is a CLI project
+ if _, err := os.Stat(cliDir); os.IsNotExist(err) {
+ return nil, fmt.Errorf("%s is not a valid CLI directory (missing internal/cli/)", dir)
+ }
+
+ const maxPasses = 3
+ for pass := 0; pass < maxPasses; pass++ {
+ deadFuncs := findAllDeadFunctions(cliDir, cmdDir)
+ if len(deadFuncs) == 0 {
+ break
+ }
+
+ // First pass populates DeadFunctions for reporting
+ if pass == 0 {
+ result.DeadFunctions = deadFuncs
+ } else {
+ result.DeadFunctions = append(result.DeadFunctions, deadFuncs...)
+ }
+
+ if dryRun {
+ // Dry-run only reports the first pass — can't predict cascading removals
+ return result, nil
+ }
+
+ // For each dead function, find its file and remove it via AST
+ removedByFile := map[string][]string{}
+ for _, funcName := range deadFuncs {
+ file, found := findFunctionFile(cliDir, funcName)
+ if !found {
+ continue
+ }
+ removedByFile[file] = append(removedByFile[file], funcName)
+ }
+
+ backups := map[string][]byte{}
+ for file, funcs := range removedByFile {
+ original, err := os.ReadFile(file)
+ if err != nil {
+ continue
+ }
+ backups[file] = original
+
+ if err := removeFunctionsFromFile(file, funcs); err != nil {
+ _ = os.WriteFile(file, original, 0o644)
+ return nil, fmt.Errorf("removing functions from %s: %w", filepath.Base(file), err)
+ }
+
+ result.Removed = append(result.Removed, funcs...)
+ }
+
+ // Clean up unused imports left by removal (e.g., if the dead function
+ // was the only caller of a package). Run goimports before go build
+ // so stale imports don't cause a false build failure.
+ for file := range removedByFile {
+ importsCmd := exec.Command("goimports", "-w", file)
+ if importsErr := importsCmd.Run(); importsErr != nil {
+ // goimports not installed — fall back to gofmt (won't fix imports
+ // but at least formats). The go build safety net will catch
+ // remaining import issues and restore.
+ fmtCmd := exec.Command("gofmt", "-w", file)
+ _ = fmtCmd.Run()
+ }
+ }
+
+ // Verify build after each pass
+ buildCmd := exec.Command("go", "build", "./...")
+ buildCmd.Dir = dir
+ buildOutput, buildErr := buildCmd.CombinedOutput()
+
+ if buildErr != nil {
+ result.BuildVerified = false
+ result.BuildError = strings.TrimSpace(string(buildOutput))
+ result.Restored = append(result.Restored, deadFuncs...)
+ // Remove the funcs we just tried from the removed list
+ result.Removed = result.Removed[:len(result.Removed)-len(deadFuncs)]
+
+ for file, original := range backups {
+ _ = os.WriteFile(file, original, 0o644)
+ }
+ return result, nil
+ }
+
+ // goimports already ran above — no need for a separate gofmt pass
+ }
+
+ result.BuildVerified = true
+ return result, nil
+}
+
+// findAllDeadFunctions scans ALL .go files in a CLI's internal/cli/ directory
+// for top-level function definitions, then checks if each is called anywhere
+// in cliDir and any additional search directories (e.g., cmd/).
+// Returns the names of functions that are defined but never called.
+func findAllDeadFunctions(cliDir string, extraSearchDirs ...string) []string {
+ files := listGoFiles(cliDir)
+ if len(files) == 0 {
+ return nil
+ }
+
+ funcRe := regexp.MustCompile(`(?m)^func\s+([A-Za-z_]\w*)\s*\(`)
+
+ // Collect function definitions from cliDir only
+ type funcDef struct {
+ name string
+ file string
+ }
+ var allDefs []funcDef
+ var allContent []string
+
+ for _, file := range files {
+ data, err := os.ReadFile(file)
+ if err != nil {
+ continue
+ }
+ content := string(data)
+ allContent = append(allContent, content)
+
+ matches := funcRe.FindAllStringSubmatch(content, -1)
+ for _, match := range matches {
+ allDefs = append(allDefs, funcDef{name: match[1], file: file})
+ }
+ }
+
+ // Also read files from extra search dirs for usage detection
+ // (e.g., cmd/ where main.go calls exported functions like ExitCode)
+ for _, searchDir := range extraSearchDirs {
+ _ = filepath.WalkDir(searchDir, func(path string, d os.DirEntry, err error) error {
+ if err != nil || d.IsDir() || !strings.HasSuffix(d.Name(), ".go") {
+ return nil
+ }
+ data, err := os.ReadFile(path)
+ if err != nil {
+ return nil
+ }
+ allContent = append(allContent, string(data))
+ return nil
+ })
+ }
+
+ combined := strings.Join(allContent, "\n")
+
+ // Check each function. Skip Go entry points (main, init) and test
+ // helpers (TestXxx) which are called by the runtime, not by other code.
+ skipNames := map[string]bool{"main": true, "init": true}
+ seen := map[string]bool{}
+ var dead []string
+ for _, def := range allDefs {
+ if seen[def.name] || skipNames[def.name] {
+ continue
+ }
+ if strings.HasPrefix(def.name, "Test") || strings.HasPrefix(def.name, "Benchmark") {
+ continue
+ }
+ seen[def.name] = true
+
+ // Match calls but not definitions. A call looks like `name(` without
+ // `func ` or `func (receiver) ` preceding it. We count occurrences:
+ // if the name appears with `(` more times than it has `func name(`
+ // definitions, it's called somewhere.
+ allRe := regexp.MustCompile(`\b` + regexp.QuoteMeta(def.name) + `\s*\(`)
+ defRe := regexp.MustCompile(`func\s+` + regexp.QuoteMeta(def.name) + `\s*\(`)
+ totalMatches := len(allRe.FindAllString(combined, -1))
+ defMatches := len(defRe.FindAllString(combined, -1))
+ if totalMatches <= defMatches {
+ dead = append(dead, def.name)
+ }
+ }
+
+ sort.Strings(dead)
+ return dead
+}
+
+// findFunctionFile searches all .go files in dir for a top-level function definition.
+func findFunctionFile(dir, funcName string) (string, bool) {
+ entries, err := os.ReadDir(dir)
+ if err != nil {
+ return "", false
+ }
+
+ needle := "func " + funcName + "("
+ for _, entry := range entries {
+ if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".go") {
+ continue
+ }
+ path := filepath.Join(dir, entry.Name())
+ data, err := os.ReadFile(path)
+ if err != nil {
+ continue
+ }
+ if strings.Contains(string(data), needle) {
+ return path, true
+ }
+ }
+ return "", false
+}
+
+// removeFunctionsFromFile parses a Go file and removes the named functions,
+// including their associated doc comments.
+func removeFunctionsFromFile(path string, funcNames []string) error {
+ fset := token.NewFileSet()
+ node, err := parser.ParseFile(fset, path, nil, parser.ParseComments)
+ if err != nil {
+ return fmt.Errorf("parsing %s: %w", path, err)
+ }
+
+ nameSet := map[string]bool{}
+ for _, name := range funcNames {
+ nameSet[name] = true
+ }
+
+ // Collect doc comment positions for dead functions so we can prune them
+ deadDocComments := map[*ast.CommentGroup]bool{}
+ var filtered []ast.Decl
+ for _, decl := range node.Decls {
+ fn, ok := decl.(*ast.FuncDecl)
+ if ok && fn.Recv == nil && nameSet[fn.Name.Name] {
+ // Mark the doc comment for removal
+ if fn.Doc != nil {
+ deadDocComments[fn.Doc] = true
+ }
+ continue
+ }
+ filtered = append(filtered, decl)
+ }
+ node.Decls = filtered
+
+ // Prune orphaned doc comments from the comment map
+ if len(deadDocComments) > 0 {
+ var kept []*ast.CommentGroup
+ for _, cg := range node.Comments {
+ if !deadDocComments[cg] {
+ kept = append(kept, cg)
+ }
+ }
+ node.Comments = kept
+ }
+
+ // Write back
+ f, err := os.Create(path)
+ if err != nil {
+ return fmt.Errorf("creating %s: %w", path, err)
+ }
+ defer func() { _ = f.Close() }()
+
+ cfg := &printer.Config{Mode: printer.UseSpaces | printer.TabIndent, Tabwidth: 8}
+ if err := cfg.Fprint(f, fset, node); err != nil {
+ return fmt.Errorf("writing %s: %w", path, err)
+ }
+
+ return nil
+}
diff --git a/internal/pipeline/polish_test.go b/internal/pipeline/polish_test.go
new file mode 100644
index 00000000..7eada32f
--- /dev/null
+++ b/internal/pipeline/polish_test.go
@@ -0,0 +1,231 @@
+package pipeline
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+func TestRemoveFunctionsFromFile(t *testing.T) {
+ t.Run("removes dead function and preserves live ones", func(t *testing.T) {
+ dir := t.TempDir()
+ path := filepath.Join(dir, "helpers.go")
+
+ content := `package cli
+
+import "fmt"
+
+func liveFunc() string {
+ return "I am used"
+}
+
+func deadFunc() string {
+ return "nobody calls me"
+}
+
+func anotherLive() {
+ fmt.Println(liveFunc())
+}
+`
+ require.NoError(t, os.WriteFile(path, []byte(content), 0o644))
+
+ err := removeFunctionsFromFile(path, []string{"deadFunc"})
+ require.NoError(t, err)
+
+ result, err := os.ReadFile(path)
+ require.NoError(t, err)
+
+ assert.Contains(t, string(result), "func liveFunc()")
+ assert.Contains(t, string(result), "func anotherLive()")
+ assert.NotContains(t, string(result), "func deadFunc()")
+ assert.NotContains(t, string(result), "nobody calls me")
+ })
+
+ t.Run("removes multiple dead functions", func(t *testing.T) {
+ dir := t.TempDir()
+ path := filepath.Join(dir, "helpers.go")
+
+ content := `package cli
+
+func keepMe() string { return "keep" }
+
+func dead1() string { return "dead" }
+
+func dead2() int { return 0 }
+
+func alsoKeep() string { return keepMe() }
+`
+ require.NoError(t, os.WriteFile(path, []byte(content), 0o644))
+
+ err := removeFunctionsFromFile(path, []string{"dead1", "dead2"})
+ require.NoError(t, err)
+
+ result, err := os.ReadFile(path)
+ require.NoError(t, err)
+
+ assert.Contains(t, string(result), "func keepMe()")
+ assert.Contains(t, string(result), "func alsoKeep()")
+ assert.NotContains(t, string(result), "func dead1()")
+ assert.NotContains(t, string(result), "func dead2()")
+ })
+
+ t.Run("no-op when no dead functions", func(t *testing.T) {
+ dir := t.TempDir()
+ path := filepath.Join(dir, "helpers.go")
+
+ content := `package cli
+
+func liveFunc() string { return "alive" }
+`
+ require.NoError(t, os.WriteFile(path, []byte(content), 0o644))
+
+ err := removeFunctionsFromFile(path, []string{})
+ require.NoError(t, err)
+
+ result, err := os.ReadFile(path)
+ require.NoError(t, err)
+ assert.Contains(t, string(result), "func liveFunc()")
+ })
+
+ t.Run("skips method receivers", func(t *testing.T) {
+ dir := t.TempDir()
+ path := filepath.Join(dir, "helpers.go")
+
+ content := `package cli
+
+type Foo struct{}
+
+func (f *Foo) Method() string { return "method" }
+
+func deadTopLevel() string { return "dead" }
+`
+ require.NoError(t, os.WriteFile(path, []byte(content), 0o644))
+
+ err := removeFunctionsFromFile(path, []string{"deadTopLevel", "Method"})
+ require.NoError(t, err)
+
+ result, err := os.ReadFile(path)
+ require.NoError(t, err)
+
+ assert.NotContains(t, string(result), "func deadTopLevel()")
+ assert.Contains(t, string(result), "func (f *Foo) Method()")
+ })
+
+ t.Run("removes doc comments with dead function", func(t *testing.T) {
+ dir := t.TempDir()
+ path := filepath.Join(dir, "helpers.go")
+
+ content := `package cli
+
+// liveFunc does something useful.
+func liveFunc() string { return "alive" }
+
+// deadFunc is no longer needed.
+// It used to do something important.
+func deadFunc() string { return "dead" }
+`
+ require.NoError(t, os.WriteFile(path, []byte(content), 0o644))
+
+ err := removeFunctionsFromFile(path, []string{"deadFunc"})
+ require.NoError(t, err)
+
+ result, err := os.ReadFile(path)
+ require.NoError(t, err)
+
+ assert.Contains(t, string(result), "// liveFunc does something useful.")
+ assert.NotContains(t, string(result), "// deadFunc is no longer needed.")
+ assert.NotContains(t, string(result), "It used to do something important.")
+ assert.NotContains(t, string(result), "func deadFunc()")
+ })
+}
+
+func TestFindFunctionFile(t *testing.T) {
+ dir := t.TempDir()
+
+ helpers := filepath.Join(dir, "helpers.go")
+ _ = os.WriteFile(helpers, []byte("package cli\n\nfunc helperFunc() {}\n"), 0o644)
+
+ commands := filepath.Join(dir, "commands.go")
+ _ = os.WriteFile(commands, []byte("package cli\n\nfunc commandFunc() {}\n"), 0o644)
+
+ t.Run("finds function in helpers.go", func(t *testing.T) {
+ path, found := findFunctionFile(dir, "helperFunc")
+ assert.True(t, found)
+ assert.True(t, strings.HasSuffix(path, "helpers.go"))
+ })
+
+ t.Run("finds function in other file", func(t *testing.T) {
+ path, found := findFunctionFile(dir, "commandFunc")
+ assert.True(t, found)
+ assert.True(t, strings.HasSuffix(path, "commands.go"))
+ })
+
+ t.Run("returns false for missing function", func(t *testing.T) {
+ _, found := findFunctionFile(dir, "nonexistent")
+ assert.False(t, found)
+ })
+}
+
+func TestFindAllDeadFunctions(t *testing.T) {
+ t.Run("finds dead functions across multiple files", func(t *testing.T) {
+ dir := t.TempDir()
+
+ // helpers.go: deadHelper is defined but never called anywhere
+ _ = os.WriteFile(filepath.Join(dir, "helpers.go"), []byte(`package cli
+
+func liveHelper() string { return "used" }
+
+func deadHelper() string { return "unused" }
+`), 0o644)
+
+ // command.go: calls liveHelper (making it live), defines deadCommand (never called)
+ _ = os.WriteFile(filepath.Join(dir, "command.go"), []byte(`package cli
+
+func init() {
+ runCommand()
+}
+
+func runCommand() {
+ liveHelper()
+}
+
+func deadCommand() string { return "also unused" }
+`), 0o644)
+
+ dead := findAllDeadFunctions(dir)
+
+ assert.Contains(t, dead, "deadHelper")
+ assert.Contains(t, dead, "deadCommand")
+ assert.NotContains(t, dead, "liveHelper")
+ assert.NotContains(t, dead, "runCommand")
+ })
+
+ t.Run("no dead functions when all are called", func(t *testing.T) {
+ dir := t.TempDir()
+
+ _ = os.WriteFile(filepath.Join(dir, "a.go"), []byte(`package cli
+
+func main() { funcA() }
+
+func funcA() { funcB() }
+`), 0o644)
+
+ _ = os.WriteFile(filepath.Join(dir, "b.go"), []byte(`package cli
+
+func funcB() { funcA() }
+`), 0o644)
+
+ dead := findAllDeadFunctions(dir)
+ assert.Empty(t, dead)
+ })
+
+ t.Run("empty directory", func(t *testing.T) {
+ dir := t.TempDir()
+ dead := findAllDeadFunctions(dir)
+ assert.Empty(t, dead)
+ })
+}
← 55c5525b fix(cli): machine context compensation — scorer, generator,
·
back to Cli Printing Press
·
fix(skills): add secret leak prevention rules (#107) fc94557c →