yoloyolo/.planning/milestones/v1.2-phases/08-test-and-constant-cleanup/08-RESEARCH.md

Phase 8: Test and Constant Cleanup - Research

Researched: 2026-03-27 Domain: Go test cleanup, dead code removal, test assertion generalization Confidence: HIGH

<user_constraints>

User Constraints (from CONTEXT.md)

Locked Decisions

• D-01: Delete NumLayers and GainPerLayer constants entirely from synth/config.go. They are dead code — NewBank already computes gainPerLayer dynamically as 1.0 / float64(len(cfgs)) (bank.go:21). No external callers reference either constant outside the test file.
• D-02: Replace the hardcoded [60, 1100] bounds in TestFrequenciesInRange with dynamic validation — derive the valid range from the ClassFreqConfigs data itself (e.g., check that all frequencies are positive and below Nyquist) rather than hardcoding a new magic number that would need manual updating when Phase 9/10 add classes above 1100 Hz.
• D-03: Rename TestNumLayersMatchesAllClasses to TestClassFreqConfigsMatchAllClasses (or similar) to reflect the actual invariant being tested after NumLayers removal.

Claude's Discretion

• Whether to use a generous static upper bound vs a computed Nyquist-based bound for D-02 — either approach satisfies the constraint
• Whether WhisperFloor or other constants in config.go need any adjustment (they don't reference NumLayers, so likely no)
• Whether TestClassFreqConfigsComplete (line 53) should be consolidated with the renamed test since both verify the same invariant

Deferred Ideas (OUT OF SCOPE)

• Adding protocols (IMAP, POP3, SNMP, FTP, etc.) — this is Phase 10 scope; Phase 8 only removes blockers </user_constraints>

<phase_requirements>

Phase Requirements

ID	Description	Research Support
CLEAN-01	Remove stale NumLayers constant and hardcoded frequency range test assertions that would block new class additions	D-01 removes the constants; D-02 replaces hardcoded bounds with future-proof validation; D-03 renames stale test function
</phase_requirements>

---

Summary

Phase 8 is a pure cleanup phase with two narrowly scoped targets: (1) two dead exported constants in synth/config.go and (2) three test functions in synth/config_test.go that need renaming or rewiring. No new features, no new packages, no new dependencies.

NumLayers = 14 and GainPerLayer = 1.0 / float64(NumLayers) in synth/config.go are provably dead code. synth/bank.go:NewBank computes gainPerLayer dynamically at line 21 as 1.0 / float64(len(cfgs)). Neither constant is referenced anywhere in the production code path — only in the test file's function name TestNumLayersMatchesAllClasses (which itself does not use either constant in its body). Deleting both constants removes a misleading signal and eliminates the risk of future callers accidentally hardcoding the stale count 14.

TestFrequenciesInRange asserts cfg.BaseHz < 60 || cfg.BaseHz > 1100. Phase 9 will redistribute frequencies and Phase 10 will add classes whose frequencies will exceed 1100 Hz. The test will produce false failures the moment any ClassFreqConfigs entry above 1100 Hz is added. Replacing the hardcoded upper bound with a Nyquist-based check (or a generous static bound like 8000 Hz) makes the test structurally future-proof without encoding new domain knowledge in this phase.

Primary recommendation: Three surgical edits to two files — delete 2 lines in config.go, update 1 test function body + rename 2 test functions in config_test.go. Total change surface is under 15 lines.

---

Standard Stack

No new dependencies. This phase touches only existing Go source files.

Core

Library	Version	Purpose	Why Standard
testing	stdlib	Test assertions	Already used throughout the codebase

Installation: None required — no new packages.

---

Architecture Patterns

Files Modified (exhaustive list)

synth/
├── config.go       # Delete NumLayers and GainPerLayer constants (lines 9-10)
└── config_test.go  # Update TestFrequenciesInRange body; rename two test functions

No other files are modified. The CONTEXT.md explicitly states: "Only synth/config.go and synth/config_test.go are modified — no downstream package changes expected."

Pattern 1: Dead Constant Removal

What: Delete lines 9-10 from synth/config.go.

Current state:

// synth/config.go lines 5-12
const (
    SampleRate       = 44100
    WindowMs         = 500
    SamplesPerWindow = SampleRate * WindowMs / 1000   // 22050
    NumLayers        = 14
    GainPerLayer     = 1.0 / float64(NumLayers)       // D-10: ~0.0714
    WhisperFloor     = 0.03
)

After deletion:

const (
    SampleRate       = 44100
    WindowMs         = 500
    SamplesPerWindow = SampleRate * WindowMs / 1000   // 22050
    WhisperFloor     = 0.03
)

Verification: grep -r "NumLayers\|GainPerLayer" . must return zero hits in *.go files after deletion. The only non-test reference is in .planning/research/ARCHITECTURE.md (planning docs — not compiled).

Pattern 2: Nyquist-Based Frequency Range Validation

What: Replace the hardcoded [60, 1100] upper bound in TestFrequenciesInRange.

Recommended approach (Nyquist-based): Phase 9 will add classes up to ~4000 Hz. Nyquist at 44100 Hz sample rate is 22050 Hz. A Nyquist check is mathematically correct and never needs updating regardless of how many new classes are added:

func TestFrequenciesInRange(t *testing.T) {
    const nyquist = float64(synth.SampleRate) / 2.0  // 22050 Hz
    for class, cfg := range synth.ClassFreqConfigs {
        if cfg.BaseHz <= 0 {
            t.Errorf("class %q BaseHz=%.1f must be positive", class, cfg.BaseHz)
        }
        if cfg.BaseHz >= nyquist {
            t.Errorf("class %q BaseHz=%.1f exceeds Nyquist (%.1f Hz)", class, cfg.BaseHz, nyquist)
        }
    }
}

Alternative approach (generous static bound): A bound of [20, 8000] also satisfies D-02's constraint since all planned Phase 9/10 frequencies are under 4000 Hz. However the Nyquist approach is self-documenting — it explains why there's an upper bound rather than encoding an arbitrary number. Either is acceptable per Claude's Discretion.

Key invariant preserved: Both approaches ensure adding a new class at any humanly-audible frequency (20 Hz – 20 kHz, well within Nyquist) will NOT require editing this test.

Pattern 3: Test Function Rename

What: Rename TestNumLayersMatchesAllClasses at line 61. The test body already tests the correct invariant (len(synth.ClassFreqConfigs) == len(classify.AllClasses())); only the name is stale.

Current:

func TestNumLayersMatchesAllClasses(t *testing.T) {
    if len(synth.ClassFreqConfigs) != len(classify.AllClasses()) {
        t.Errorf("ClassFreqConfigs has %d entries but AllClasses() has %d entries",
            len(synth.ClassFreqConfigs), len(classify.AllClasses()))
    }
}

After rename:

func TestClassFreqConfigsMatchAllClasses(t *testing.T) {
    if len(synth.ClassFreqConfigs) != len(classify.AllClasses()) {
        t.Errorf("ClassFreqConfigs has %d entries but AllClasses() has %d entries",
            len(synth.ClassFreqConfigs), len(classify.AllClasses()))
    }
}

Pattern 4: Consolidation Decision (Claude's Discretion)

TestAllClassesHaveConfig (lines 10-16) and TestClassFreqConfigsComplete (lines 53-59) test the same invariant: every class in AllClasses() has an entry in ClassFreqConfigs. They are exact duplicates in semantics (different error messages but identical logic). The renamed TestClassFreqConfigsMatchAllClasses (formerly TestNumLayersMatchesAllClasses) tests the converse: lengths match.

Recommendation: Remove TestClassFreqConfigsComplete (lines 53-59) as a duplicate of TestAllClassesHaveConfig. This leaves three non-overlapping coverage tests:

• TestAllClassesHaveConfig — every AllClasses() member has a map entry
• TestClassFreqConfigsMatchAllClasses — count parity (catches extra entries not in AllClasses)
• TestFrequenciesInRange — all BaseHz values are positive and below Nyquist

Alternatively, leave both functions if deduplication is not worth the discussion. Both pass and both protect the invariant. This is truly Claude's discretion.

Anti-Patterns to Avoid

• Updating NumLayers instead of deleting it: The decision (D-01) is deletion, not update. An updated constant would still be a maintenance burden.
• Replacing [60, 1100] with [60, 4000]: A new hardcoded number has the same fragility as the old one — it becomes stale when the frequency spectrum changes again in a future milestone.
• Touching bank.go: The dynamic gainPerLayer computation in bank.go is already correct. No changes needed.
• Touching classify/types.go: AllClasses() is not modified in this phase.
• Touching WhisperFloor: It does not reference NumLayers or GainPerLayer; leave it unchanged.

---

Don't Hand-Roll

Not applicable. This phase contains no algorithmic code — it is deletion and test rewriting.

---

Common Pitfalls

Pitfall 1: Leaving the GainPerLayer comment reference orphaned

What goes wrong: GainPerLayer at config.go line 10 has a comment // D-10: ~0.0714. After deletion, the decision reference D-10 disappears from the source. This is fine — D-10 is still documented in the planning research files. But if the comment is moved to bank.go line 21 (where the dynamic computation lives), it improves traceability without leaving an orphan.

How to avoid: Either delete both lines cleanly with no compensation, or add // D-10: gain is 1/N computed dynamically to bank.go:21. Both are acceptable.

Warning signs: Go compiler catches unused constants — if NumLayers or GainPerLayer are deleted and the code still compiles, they were indeed dead.

Pitfall 2: Using synth.SampleRate in the test without verifying the export

What goes wrong: SampleRate is an exported constant in synth/config.go. The test file is in package synth_test (external test package), so it accesses synth.SampleRate. Verify SampleRate is exported (capital S) before referencing it from the test.

How to avoid: Already confirmed — SampleRate = 44100 is exported at config.go line 6. No issue.

Warning signs: Compiler error synth.sampleRate undefined would indicate a lowercase constant.

Pitfall 3: Test duplication confusion

What goes wrong: TestAllClassesHaveConfig and TestClassFreqConfigsComplete look different but test the same invariant. During code review or future debugging, someone might wonder why there are two tests for the same thing.

How to avoid: If consolidating (removing TestClassFreqConfigsComplete), add a comment to TestAllClassesHaveConfig noting it replaced the duplicate. If not consolidating, no action needed.

---

Code Examples

Resulting synth/config.go constant block

// Source: synth/config.go — after Phase 8 cleanup
const (
    SampleRate       = 44100
    WindowMs         = 500
    SamplesPerWindow = SampleRate * WindowMs / 1000 // 22050
    WhisperFloor     = 0.03                         // D-08/D-09: 3% of max amplitude
)

Resulting TestFrequenciesInRange (Nyquist approach)

// Source: synth/config_test.go — after Phase 8 cleanup
func TestFrequenciesInRange(t *testing.T) {
    const nyquist = float64(synth.SampleRate) / 2.0
    for class, cfg := range synth.ClassFreqConfigs {
        if cfg.BaseHz <= 0 {
            t.Errorf("class %q BaseHz=%.1f must be positive", class, cfg.BaseHz)
        }
        if cfg.BaseHz >= nyquist {
            t.Errorf("class %q BaseHz=%.1f exceeds Nyquist (%.1f Hz)", class, cfg.BaseHz, nyquist)
        }
    }
}

Resulting TestClassFreqConfigsMatchAllClasses

// Source: synth/config_test.go — after rename from TestNumLayersMatchesAllClasses
func TestClassFreqConfigsMatchAllClasses(t *testing.T) {
    if len(synth.ClassFreqConfigs) != len(classify.AllClasses()) {
        t.Errorf("ClassFreqConfigs has %d entries but AllClasses() has %d entries",
            len(synth.ClassFreqConfigs), len(classify.AllClasses()))
    }
}

---

State of the Art

Old Approach	Current Approach	When Changed	Impact
NumLayers = 14 static constant	Dynamic 1.0 / float64(len(cfgs)) in NewBank	Phase 6/7 (v1.1)	Static constant is now dead code; remove it
TestFrequenciesInRange checks [60, 1100]	Nyquist-based check (this phase)	Phase 8 (v1.2)	Test survives any future frequency allocation

---

Validation Architecture

Test Framework

Property	Value
Framework	testing stdlib, Go 1.24
Config file	none (standard go test)
Quick run command	go test ./synth/...
Full suite command	go test ./...

Phase Requirements → Test Map

Req ID	Behavior	Test Type	Automated Command	File Exists?
CLEAN-01 (constant removal)	NumLayers and GainPerLayer are not exported from synth package	unit — compile check	go build ./synth/...	✅ (config.go exists; delete lines)
CLEAN-01 (no broken references)	Full test suite passes after deletion	integration	go test ./...	✅
CLEAN-01 (range test future-proof)	TestFrequenciesInRange passes with any BaseHz in (0, Nyquist) range	unit	go test ./synth/... -run TestFrequenciesInRange	✅ (config_test.go exists; update body)
CLEAN-01 (test rename)	TestClassFreqConfigsMatchAllClasses exists and passes	unit	go test ./synth/... -run TestClassFreqConfigsMatchAllClasses	✅ (rename existing function)

Sampling Rate

• Per task commit: go test ./synth/...
• Per wave merge: go test ./...
• Phase gate: go test ./... green before /gsd:verify-work

Wave 0 Gaps

None — existing test infrastructure covers all phase requirements. No new test files, fixtures, or framework setup needed.

---

Environment Availability

Step 2.6: SKIPPED (no external dependencies — pure Go source edits, no new tools or services required).

Current test suite state confirmed: go test ./... passes on all 7 packages.

---

Open Questions

1. Consolidate TestAllClassesHaveConfig and TestClassFreqConfigsComplete?

   • What we know: They test the same invariant; both currently pass; no correctness issue either way
   • What's unclear: Whether the planner wants one clean authoritative test or is fine leaving both
   • Recommendation: Remove TestClassFreqConfigsComplete (lines 53-59) as it duplicates TestAllClassesHaveConfig. The named TestAllClassesHaveConfig is more expressive. If this causes any concern, leave both — both are correct.

2. Add D-10 comment to bank.go after deleting GainPerLayer?

   • What we know: GainPerLayer carries // D-10: ~0.0714; bank.go line 21 is where the actual computation lives
   • What's unclear: Whether the project wants decision-reference comments preserved at the implementation site
   • Recommendation: Add // D-10: gainPerLayer = 1/N so all N layers at full amplitude sum to 1.0 to bank.go line 21. Low-cost, improves traceability.

---

Sources

Primary (HIGH confidence)

• synth/config.go — Direct inspection: NumLayers = 14, GainPerLayer = 1.0 / float64(NumLayers) at lines 9-10; SampleRate = 44100 at line 6
• synth/bank.go — Direct inspection: gainPerLayer: 1.0 / float64(len(cfgs)) at line 21 — confirms constants are dead code
• synth/config_test.go — Direct inspection: TestFrequenciesInRange body at lines 18-25; TestNumLayersMatchesAllClasses at lines 61-66; TestClassFreqConfigsComplete at lines 53-59
• classify/types.go — Direct inspection: AllClasses() returns 14 entries; SampleRate = 44100 used for Nyquist calculation
• .planning/phases/08-test-and-constant-cleanup/08-CONTEXT.md — Locked decisions D-01, D-02, D-03
• .planning/research/PITFALLS.md — Pitfall C3 (NumLayers stale constant) and C4 (TestFrequenciesInRange hardcoding)
• .planning/research/ARCHITECTURE.md lines 414-422 — NumLayers/GainPerLayer dead code analysis

Secondary (MEDIUM confidence)

• go test ./... output — All 7 packages pass; current baseline confirmed

---

Metadata

Confidence breakdown:

• Standard stack: HIGH — no new dependencies; pure stdlib
• Architecture: HIGH — all target lines verified by direct file inspection
• Pitfalls: HIGH — sourced from project research files and direct code inspection

Research date: 2026-03-27 Valid until: Until Phase 9 begins (frequency redistribution) — this research is tied to current synth/config.go line numbers which Phase 9 will change