From 249ab89cebae026e1553016b42b8f59b9537eee2 Mon Sep 17 00:00:00 2001 From: Boden Fuller Date: Tue, 12 May 2026 22:01:57 -0400 Subject: [PATCH] refactor(cli): delete dead LoadCycleHistory + sync cobra-cmd test list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle 156, phase-2 of the DDD/hex rescope. First phase-2 migration attempt (soc-8fjo: metrics_health.go → LoopReaderPort) surfaced a bigger finding than the rescope predicted: the function we planned to migrate has zero production callers. quality.LoadCycleHistory in cli/internal/quality/metrics_health.go and its one-line shim cli/cmd/ao/metrics_health.go::loadCycleHistory are defined, tested directly, and called by nothing else. Two tests in cli/cmd/ao/metrics_health_test.go probe the dead path. The right phase-2 move was delete, not migrate. Removed: - cli/internal/quality/metrics_health.go: LoadCycleHistory (23 lines) + now-unused bufio/encoding-json imports - cli/cmd/ao/metrics_health.go: loadCycleHistory shim (1 line) - cli/cmd/ao/metrics_health_test.go: TestMetricsHealth_LoadCycleHistory + TestMetricsHealth_LoadCycleHistoryMissingFile (~22 lines) Net: ~46 lines of dead code removed. Full test suite green. Drive-by fix while running the regression bundle: cobra_commands_test TestCobraExpectedCmdsMatchRegistration was failing because cycles 144-154 added 6 new top-level commands (ci, citation, claim, harness, loop, operator) without updating the expectedCmds list. That drift was pre-existing, surfaced by running the full bundle. Added the 6 to both expectedCmds lists in the test. Also surfaced (deferred, not done): cli/internal/lifecycle/defrag.go ::SweepOscillatingGoals keys on a 'Target' field that ports.CycleEntry does NOT project. Worse, zero entries in cycle-history.jsonl have a 'target' field — the production sweep is a permanent no-op. Migration to LoopReaderPort requires a schema decision (extend CycleEntry vs re-target defrag at a different artifact). Filed as deferred under soc-1q1x. Closing soc-8fjo (no longer applicable: target deleted). Updated docs/rescope/2026-05-13-ddd-hex-architecture-rescope.md with an addendum capturing both findings. --- cli/cmd/ao/cobra_commands_test.go | 16 ++++++------ cli/cmd/ao/metrics_health.go | 2 -- cli/cmd/ao/metrics_health_test.go | 24 ----------------- cli/internal/quality/metrics_health.go | 26 ------------------- ...2026-05-13-ddd-hex-architecture-rescope.md | 18 +++++++++++++ 5 files changed, 26 insertions(+), 60 deletions(-) diff --git a/cli/cmd/ao/cobra_commands_test.go b/cli/cmd/ao/cobra_commands_test.go index e2ae4fba3..71a67d086 100644 --- a/cli/cmd/ao/cobra_commands_test.go +++ b/cli/cmd/ao/cobra_commands_test.go @@ -435,12 +435,12 @@ func TestCobraCommandTreeRegistration(t *testing.T) { // Verify all top-level commands are registered (flat namespace) expectedCmds := []string{ - "agents", "anti-patterns", "autodev", "badge", "batch-feedback", "beads", "completion", "config", + "agents", "anti-patterns", "autodev", "badge", "batch-feedback", "beads", "ci", "citation", "claim", "completion", "config", "constraint", "context", "codex", "compile", "contradict", "corpus", "curate", "dedup", "daemon", "defrag", "demo", "doctor", "eval", "evolve", "extract", "factory", "feedback", "feedback-loop", - "findings", "flywheel", "forge", "gate", "goals", "handoff", "harvest", "hooks", - "index", "init", "inject", "knowledge", "lookup", "maturity", - "memory", "metrics", "migrate", "mind", "mine", "notebook", "overnight", "patterns", "plans", + "findings", "flywheel", "forge", "gate", "goals", "handoff", "harness", "harvest", "hooks", + "index", "init", "inject", "knowledge", "lookup", "loop", "maturity", + "memory", "metrics", "migrate", "mind", "mine", "notebook", "operator", "overnight", "patterns", "plans", "pool", "quick-start", "ratchet", "retrieval-bench", "rpi", "registry", "scenario", "schedule", "scope", "search", "seed", "session", "session-outcome", "sessions", "skills", "status", "store", "task-feedback", "task-status", "task-sync", "temper", @@ -496,12 +496,12 @@ func TestCobraExpectedCmdsMatchRegistration(t *testing.T) { // Same list as TestCobraCommandTreeRegistration expectedCmds := []string{ - "agents", "anti-patterns", "autodev", "badge", "batch-feedback", "beads", "completion", "config", + "agents", "anti-patterns", "autodev", "badge", "batch-feedback", "beads", "ci", "citation", "claim", "completion", "config", "constraint", "context", "codex", "compile", "contradict", "corpus", "curate", "dedup", "daemon", "defrag", "demo", "doctor", "eval", "evolve", "extract", "factory", "feedback", "feedback-loop", - "findings", "flywheel", "forge", "gate", "goals", "handoff", "harvest", "hooks", - "index", "init", "inject", "knowledge", "lookup", "maturity", - "memory", "metrics", "migrate", "mind", "mine", "notebook", "overnight", "patterns", "plans", + "findings", "flywheel", "forge", "gate", "goals", "handoff", "harness", "harvest", "hooks", + "index", "init", "inject", "knowledge", "lookup", "loop", "maturity", + "memory", "metrics", "migrate", "mind", "mine", "notebook", "operator", "overnight", "patterns", "plans", "pool", "quick-start", "ratchet", "retrieval-bench", "rpi", "registry", "scenario", "schedule", "scope", "search", "seed", "session", "session-outcome", "sessions", "skills", "status", "store", "task-feedback", "task-status", "task-sync", "temper", diff --git a/cli/cmd/ao/metrics_health.go b/cli/cmd/ao/metrics_health.go index c088f13e2..c900ea9a2 100644 --- a/cli/cmd/ao/metrics_health.go +++ b/cli/cmd/ao/metrics_health.go @@ -372,8 +372,6 @@ func countUniqueSessions(citations []types.CitationEvent) int { return quality.CountUniqueSessions(citations) } -func loadCycleHistory(baseDir string) []map[string]any { return quality.LoadCycleHistory(baseDir) } - // printHealthTable prints a formatted health metrics table. func printHealthTable(w io.Writer, hm *healthMetrics) { fmt.Fprintln(w) diff --git a/cli/cmd/ao/metrics_health_test.go b/cli/cmd/ao/metrics_health_test.go index d1093cd72..18f3b1f4a 100644 --- a/cli/cmd/ao/metrics_health_test.go +++ b/cli/cmd/ao/metrics_health_test.go @@ -670,30 +670,6 @@ func TestMetricsHealth_TablelessRun(t *testing.T) { } } -func TestMetricsHealth_LoadCycleHistory(t *testing.T) { - dir := t.TempDir() - evolveDir := filepath.Join(dir, ".agents", "evolve") - if err := os.MkdirAll(evolveDir, 0o755); err != nil { - t.Fatal(err) - } - historyPath := filepath.Join(evolveDir, "cycle-history.jsonl") - data := `{"cycle":1,"status":"pass"}` + "\n" + "invalid-json" + "\n" + `{"cycle":2,"status":"fail"}` + "\n" - if err := os.WriteFile(historyPath, []byte(data), 0o644); err != nil { - t.Fatal(err) - } - - entries := loadCycleHistory(dir) - if len(entries) != 2 { - t.Fatalf("expected 2 parseable entries, got %d", len(entries)) - } -} - -func TestMetricsHealth_LoadCycleHistoryMissingFile(t *testing.T) { - if got := loadCycleHistory(t.TempDir()); got != nil { - t.Fatalf("expected nil history for missing file, got %v", got) - } -} - func TestComputeLoopDominance_StaleUsesNinetyDayWindow(t *testing.T) { dir := setupHealthTestDir(t) learningsDir := filepath.Join(dir, ".agents", "learnings") diff --git a/cli/internal/quality/metrics_health.go b/cli/internal/quality/metrics_health.go index de7257f35..ebb762eee 100644 --- a/cli/internal/quality/metrics_health.go +++ b/cli/internal/quality/metrics_health.go @@ -1,8 +1,6 @@ package quality import ( - "bufio" - "encoding/json" "os" "path/filepath" "strings" @@ -125,27 +123,3 @@ func CountUniqueSessions(citations []types.CitationEvent) int { } return len(seen) } - -// LoadCycleHistory loads cycle-history.jsonl entries. Returns nil on missing file. -func LoadCycleHistory(baseDir string) []map[string]any { - path := filepath.Join(baseDir, ".agents", "evolve", "cycle-history.jsonl") - f, err := os.Open(path) - if err != nil { - return nil - } - defer func() { - _ = f.Close() - }() - - var entries []map[string]any - scanner := bufio.NewScanner(f) - buf := make([]byte, 0, 64*1024) - scanner.Buffer(buf, 1024*1024) - for scanner.Scan() { - var entry map[string]any - if err := json.Unmarshal(scanner.Bytes(), &entry); err == nil { - entries = append(entries, entry) - } - } - return entries -} diff --git a/docs/rescope/2026-05-13-ddd-hex-architecture-rescope.md b/docs/rescope/2026-05-13-ddd-hex-architecture-rescope.md index 8a1a13d75..98834feed 100644 --- a/docs/rescope/2026-05-13-ddd-hex-architecture-rescope.md +++ b/docs/rescope/2026-05-13-ddd-hex-architecture-rescope.md @@ -101,3 +101,21 @@ This document. Plus the three concrete phase-2 picks above, ready to be filed as --- **Bottom line:** stop adding adapters and CLI commands. Start rewiring real consumers (defrag, metrics_health, context_assemble) to use the ports we already built. Three migrations close the BC3 loop and prove the hex architecture is load-bearing rather than decorative. + +## Addendum (cycle 156) — first migration target was dead code + +**Update from the first phase-2 attempt.** When opening `cli/internal/quality/metrics_health.go::LoadCycleHistory` to migrate it to `LoopReaderPort`, two findings surfaced: + +1. **Schema mismatch on defrag.go.** `defrag.go::SweepOscillatingGoals` reads a `Target` field that `CycleEntry` does NOT project. Worse, **zero entries in the actual `cycle-history.jsonl` have a `target` field** — the production sweep is a permanent no-op against a per-goal schema that never materialized. Migrating defrag.go to LoopReaderPort would not be a clean swap; it'd require either (a) adding `Target` to `CycleEntry` and the ledger writers, or (b) routing defrag at a different artifact (e.g. goals.jsonl). + +2. **`metrics_health.LoadCycleHistory` is dead code.** `loadCycleHistory` is defined in `cli/cmd/ao/metrics_health.go:375` as a one-line shim around `quality.LoadCycleHistory`, but the shim has **zero production callers**. The only references are: the shim's own definition, the underlying `quality` function's definition, and two tests that probe the dead path directly. The cycle 144 `loadCycleHistory` collision was actually a collision against dead code — the cleanup was already overdue. + +**Action taken in cycle 156:** deleted `quality.LoadCycleHistory` (~23 lines), the shim in `cmd/ao/metrics_health.go` (1 line), and the two dead tests in `cmd/ao/metrics_health_test.go` (~22 lines). Net: ~46 lines of dead code removed, full test suite still green. + +**Rescope thesis re-validated:** the architecture exists, but the production code that *should* consume it never did. We kept building infrastructure for consumers that turned out to be either schema-mismatched (defrag) or dead code (metrics_health). The cleaner phase-2 path is **delete dead code AND migrate `context_assemble.go`**, not "migrate all three" as originally listed. Closing `soc-8fjo` (metrics_health migration) — no longer applicable, the target is gone. + +Updated phase-2 picks (cycle 156 forward): +- ~~soc-8fjo~~ — RESOLVED via deletion in cycle 156. +- `soc-0pku` — context_assemble.go → LoopReaderPort + CorpusReaderPort. Still the right migration. +- `soc-1q1x` — defrag.go → LoopReaderPort. **Deferred**, pending Target-field schema decision on `CycleEntry`. Filed as a discussion-then-migration follow-up, not an immediate cycle pick. +