mirror of
https://github.com/boshu2/agentops.git
synced 2026-09-14 15:08:13 +08:00
refactor(cli): delete dead LoadCycleHistory + sync cobra-cmd test list
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.
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user