From 467d415e94613cd4e05f9e6cef303eac1414589b Mon Sep 17 00:00:00 2001 From: Markus Graf Date: Fri, 27 Mar 2026 08:43:01 +0100 Subject: [PATCH] docs(08): capture phase context --- .../08-CONTEXT.md | 88 +++++++++++++++++++ .../08-DISCUSSION-LOG.md | 60 +++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 .planning/phases/08-test-and-constant-cleanup/08-CONTEXT.md create mode 100644 .planning/phases/08-test-and-constant-cleanup/08-DISCUSSION-LOG.md diff --git a/.planning/phases/08-test-and-constant-cleanup/08-CONTEXT.md b/.planning/phases/08-test-and-constant-cleanup/08-CONTEXT.md new file mode 100644 index 0000000..53fcd1c --- /dev/null +++ b/.planning/phases/08-test-and-constant-cleanup/08-CONTEXT.md @@ -0,0 +1,88 @@ +# Phase 8: Test and Constant Cleanup - Context + +**Gathered:** 2026-03-27 +**Status:** Ready for planning + + +## Phase Boundary + +Remove stale exported constants (`NumLayers`, `GainPerLayer`) from the synth package and update hardcoded test assertions (`TestFrequenciesInRange`) so that subsequent v1.2 phases can add new traffic classes and frequencies without triggering false CI failures. This is pure cleanup — no new features, no new protocols. + + + + +## Implementation Decisions + +### Constant Removal Strategy +- **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. + +### Frequency Range Test Bounds +- **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. The specific approach (positive+Nyquist check, or a generous static bound like `[20, 8000]`) is at Claude's discretion — the key constraint is that adding a new class in the 1100-4000 Hz range must not require editing this test. + +### Test Naming +- **D-03:** Rename `TestNumLayersMatchesAllClasses` to `TestClassFreqConfigsMatchAllClasses` (or similar) to reflect the actual invariant being tested after `NumLayers` removal. The test body already uses `len(synth.ClassFreqConfigs)` and `len(classify.AllClasses())` — only the name references the deleted constant. + +### 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 + +### Folded Todos +- **"Expand Traffic Classes"** (from `.planning/todos/pending/001-expand-traffic-classes.md`) — This todo requests adding protocols like IMAP, POP3, SNMP, FTP and researching common traffic classes. Phase 8 enables this work by removing the test/constant blockers, but the actual protocol additions are Phase 10's scope. Folded here as context, not as direct Phase 8 work. + + + + +## Canonical References + +**Downstream agents MUST read these before planning or implementing.** + +### Synth Package (primary targets) +- `synth/config.go` — Contains `NumLayers` and `GainPerLayer` constants to remove (lines 9-10) +- `synth/config_test.go` — Contains `TestFrequenciesInRange` (lines 18-24), `TestNumLayersMatchesAllClasses` (lines 61-66), and `TestClassFreqConfigsComplete` (lines 53-59) +- `synth/bank.go` — `NewBank` already computes `gainPerLayer` dynamically (line 21) — confirms constants are dead code + +### Research Context +- `.planning/research/PITFALLS.md` — Pitfall C4 documents `TestFrequenciesInRange` hardcoding issue +- `.planning/research/ARCHITECTURE.md` — Lines 414+ document NumLayers/ClassFreqConfigs mismatch risk +- `.planning/research/SUMMARY.md` — Lines 69, 85, 100 describe Phase 8 cleanup scope + + + + +## Existing Code Insights + +### Reusable Assets +- `synth/bank.go:NewBank` already has the correct dynamic gain computation — no new code needed for gain behavior + +### Established Patterns +- Test file `synth/config_test.go` uses table-driven validation against `ClassFreqConfigs` map and `classify.AllClasses()` — new/renamed tests should follow this pattern +- `GainPerLayer` constant at line 10 has a comment referencing "D-10" — cleanup should not leave orphan decision references + +### Integration Points +- Only `synth/config.go` and `synth/config_test.go` are modified — no downstream package changes expected +- `go test ./...` is the verification gate — must pass with zero new failures + + + + +## Specific Ideas + +No specific requirements — this is a straightforward cleanup phase with clear targets identified in research. + + + + +## Deferred Ideas + +### Reviewed Todos (not folded) +None — the matched todo was folded as milestone context. + +None — discussion stayed within phase scope. + + + +--- + +*Phase: 08-test-and-constant-cleanup* +*Context gathered: 2026-03-27* diff --git a/.planning/phases/08-test-and-constant-cleanup/08-DISCUSSION-LOG.md b/.planning/phases/08-test-and-constant-cleanup/08-DISCUSSION-LOG.md new file mode 100644 index 0000000..7c50d9c --- /dev/null +++ b/.planning/phases/08-test-and-constant-cleanup/08-DISCUSSION-LOG.md @@ -0,0 +1,60 @@ +# Phase 8: Test and Constant Cleanup - Discussion Log + +> **Audit trail only.** Do not use as input to planning, research, or execution agents. +> Decisions are captured in CONTEXT.md — this log preserves the alternatives considered. + +**Date:** 2026-03-27 +**Phase:** 08-test-and-constant-cleanup +**Areas discussed:** Constant removal strategy, Frequency range bound, Test naming +**Mode:** Auto (all decisions auto-selected) + +--- + +## Constant Removal Strategy + +| Option | Description | Selected | +|--------|-------------|----------| +| Delete entirely | Remove NumLayers and GainPerLayer from config.go — bank.go already computes dynamically | ✓ | +| Deprecate with comment | Keep but mark as deprecated for backward compatibility | | +| Replace with function | Convert to a function that returns len(ClassFreqConfigs) | | + +**User's choice:** [auto] Delete entirely (recommended default) +**Notes:** NewBank already computes gainPerLayer as 1.0/len(cfgs). No external callers reference either constant. + +--- + +## Frequency Range Test Bound + +| Option | Description | Selected | +|--------|-------------|----------| +| Dynamic validation | Derive valid range from data (positive + below Nyquist) — no magic numbers | ✓ | +| Generous static bound | Replace 1100 with e.g. 8000 Hz — simple but still hardcoded | | +| Remove range test | Delete TestFrequenciesInRange entirely — other tests cover correctness | | + +**User's choice:** [auto] Dynamic validation (recommended default) +**Notes:** Key constraint: adding a class in 1100-4000 Hz range must not require editing this test. + +--- + +## Test Naming + +| Option | Description | Selected | +|--------|-------------|----------| +| Rename to TestClassFreqConfigsMatchAllClasses | Reflects actual invariant after NumLayers removal | ✓ | +| Keep current name | Leave as-is despite referencing deleted constant | | +| Delete test | TestClassFreqConfigsComplete already covers same invariant | | + +**User's choice:** [auto] Rename to TestClassFreqConfigsMatchAllClasses (recommended default) +**Notes:** Test body already uses len() comparisons, only the name references NumLayers. + +--- + +## Claude's Discretion + +- Specific approach for dynamic frequency range validation (Nyquist-based vs generous static bound) +- Whether to consolidate TestClassFreqConfigsComplete with renamed test +- Whether WhisperFloor needs adjustment (likely not) + +## Deferred Ideas + +None — discussion stayed within phase scope.