docs(06): research phase domain
This commit is contained in:
@@ -0,0 +1,590 @@
|
|||||||
|
# Phase 6: Config Package and Sound Overrides - Research
|
||||||
|
|
||||||
|
**Researched:** 2026-03-26
|
||||||
|
**Domain:** Go TOML config loading, partial merge semantics, CLI flag wiring
|
||||||
|
**Confidence:** HIGH
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
<user_constraints>
|
||||||
|
## User Constraints (from CONTEXT.md)
|
||||||
|
|
||||||
|
### Locked Decisions
|
||||||
|
|
||||||
|
- **D-01:** Use keyed TOML tables `[sounds.<classname>]` for per-class overrides. Each table supports `frequency` (float64, Hz) and `waveform` (string: "sine", "square", "sawtooth", "triangle"). Class names match `classify.TrafficClass` string values (e.g., `[sounds.ICMP]`, `[sounds.HTTPS]`).
|
||||||
|
- **D-02:** Top-level structure is flat — no deeply nested hierarchies. Future phases (custom rules) will add `[[rules]]` array-of-tables at the top level.
|
||||||
|
- **D-03:** Per-field overlay merge — only fields explicitly set in TOML override defaults. Unspecified fields retain their built-in values. For example, setting only `frequency` for ICMP leaves its waveform and harmonics unchanged. This satisfies CFG-04 (partial override without replicating entire config).
|
||||||
|
- **D-04:** Merge produces a `map[classify.TrafficClass]FreqConfig` that is passed to `synth.NewBank()` via the injection seam from Phase 5. The default map is `synth.ClassFreqConfigs`.
|
||||||
|
- **D-05:** Discovery order (most-specific wins): `--config <path>` > `./netsynth.toml` > `~/.config/netsynth/config.toml`. If `--config` is specified and the file does not exist, exit with a clear error before capture begins (CFG-03). If no config is found via auto-discovery, proceed silently with defaults (CFG-02).
|
||||||
|
- **D-06:** Only one config file is loaded — no multi-file merge. The first found in precedence order wins entirely.
|
||||||
|
- **D-07:** Unknown keys cause an immediate startup error naming the unrecognized key (CFG-05). Use TOML strict decoding to detect unknown keys. Suggest the closest valid key name if edit distance is small (nice-to-have, Claude's discretion on implementation).
|
||||||
|
- **D-08:** Type mismatches (e.g., `frequency = "not a number"`) produce a clear error with field name and expected type, before capture begins.
|
||||||
|
- **D-09:** Unknown class names in `[sounds.<classname>]` produce a warning (not error) — this prepares for Phase 7 where user-defined class names are valid.
|
||||||
|
- **D-10:** `encode.RunSynthesis` signature changes to accept the merged config map (or loads config internally). The `--config` flag is added to the Cobra root command in `cmd/netsynth/main.go`.
|
||||||
|
- **D-11:** Config loading happens once at startup, before any capture begins — fail fast on all config errors.
|
||||||
|
|
||||||
|
### Claude's Discretion
|
||||||
|
|
||||||
|
- TOML library choice (BurntSushi/toml vs pelletier/go-toml) — researcher should evaluate both
|
||||||
|
- Whether to create a dedicated `config` package or keep loading in `cmd/netsynth`
|
||||||
|
- Waveform string-to-WaveformType mapping implementation details
|
||||||
|
- Edit distance algorithm for typo suggestions (or skip if complexity isn't justified)
|
||||||
|
|
||||||
|
### Deferred Ideas (OUT OF SCOPE)
|
||||||
|
|
||||||
|
- `--print-config` command (CFG-06) — scoped to Phase 7
|
||||||
|
- Custom classification rules (`[[rules]]` TOML blocks) — scoped to Phase 7
|
||||||
|
- Config hot-reload — explicitly out of scope per REQUIREMENTS.md
|
||||||
|
|
||||||
|
</user_constraints>
|
||||||
|
|
||||||
|
<phase_requirements>
|
||||||
|
## Phase Requirements
|
||||||
|
|
||||||
|
| ID | Description | Research Support |
|
||||||
|
|----|-------------|------------------|
|
||||||
|
| CFG-01 | User can create a TOML config file that overrides default sound mappings | `[sounds.<classname>]` table pattern decodes into `map[string]SoundOverride`; per-field merge into `synth.ClassFreqConfigs` clone |
|
||||||
|
| CFG-02 | Tool auto-discovers config from `./netsynth.toml` or `~/.config/netsynth/config.toml` (silent if absent) | `os.Stat` probe + `os.UserConfigDir()` for XDG path; `errors.Is(err, fs.ErrNotExist)` for silent miss |
|
||||||
|
| CFG-03 | User can specify an explicit config path via `--config` flag (error if file missing) | Cobra `StringVar` flag; fail-fast `os.Stat` check returns error before capture begins |
|
||||||
|
| CFG-04 | User can override individual values without replicating the entire default config (partial override) | Pointer fields (`*float64`, `*string`) in the TOML decode struct allow distinguishing "explicitly zero" from "not set"; overlay merge copies only non-nil fields |
|
||||||
|
| CFG-05 | Unknown keys in config file produce a clear error with the typo'd key name | BurntSushi/toml `MetaData.Undecoded()` returns unmatched keys after decode; format as error message |
|
||||||
|
|
||||||
|
</phase_requirements>
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Phase 6 adds a `config` package responsible for loading a TOML config file, validating it, and merging it over the `synth.ClassFreqConfigs` default map. The merge output is a `map[classify.TrafficClass]synth.FreqConfig` that is handed to `synth.NewBank()` — the injection seam already exists from Phase 5.
|
||||||
|
|
||||||
|
The core technical challenge is **partial override semantics**: a user who sets only `frequency` for ICMP must not accidentally clear its waveform. This requires the decode struct to use pointer fields (`*float64`, `*string`) so that absent keys remain `nil` at decode time. The merge loop then only copies non-nil values over the defaults.
|
||||||
|
|
||||||
|
Unknown-key detection uses BurntSushi/toml v1.6.0's `MetaData.Undecoded()` method, which is reliable because it operates on the actual set of keys the parser traversed. The alternative (pelletier/go-toml v2.3.0's `DisallowUnknownFields`) is also viable but adds a dependency with a different API surface and returns human-formatted error strings rather than structured key lists — less useful for the "suggest closest valid key" nice-to-have.
|
||||||
|
|
||||||
|
**Primary recommendation:** Use `github.com/BurntSushi/toml` v1.6.0. Use a dedicated `config` package. Implement partial merge with pointer fields in the TOML decode struct.
|
||||||
|
|
||||||
|
## Standard Stack
|
||||||
|
|
||||||
|
### Core
|
||||||
|
|
||||||
|
| Library | Version | Purpose | Why Standard |
|
||||||
|
|---------|---------|---------|--------------|
|
||||||
|
| `github.com/BurntSushi/toml` | v1.6.0 | TOML parsing and MetaData for unknown-key detection | Simpler API than pelletier v2; `Undecoded()` returns structured `[]Key` (not formatted error strings); `DecodeFile()` is a one-liner; v1.6.0 published December 2025 |
|
||||||
|
|
||||||
|
### Supporting
|
||||||
|
|
||||||
|
| Library | Version | Purpose | When to Use |
|
||||||
|
|---------|---------|---------|-------------|
|
||||||
|
| `os` (stdlib) | Go 1.24 | File existence checks, `UserConfigDir()` for XDG path | Always — no external dependency needed for discovery logic |
|
||||||
|
| `errors`/`fs` (stdlib) | Go 1.24 | `errors.Is(err, fs.ErrNotExist)` for silent-miss on auto-discovery | Always |
|
||||||
|
|
||||||
|
### Alternatives Considered
|
||||||
|
|
||||||
|
| Instead of | Could Use | Tradeoff |
|
||||||
|
|------------|-----------|----------|
|
||||||
|
| `BurntSushi/toml v1.6.0` | `pelletier/go-toml v2.3.0` | go-toml has `DisallowUnknownFields()` built-in (cleaner API) but returns `StrictMissingError` with a formatted string — harder to extract just the key name for a "did you mean?" suggestion. BurntSushi returns `[]toml.Key` which is structured. For this use case, BurntSushi is easier to work with. |
|
||||||
|
| Pointer fields for partial override | Separate "is-set" booleans | Pointer fields are idiomatic in Go for "optional" semantics. Booleans add field count and are error-prone. |
|
||||||
|
| Dedicated `config` package | Inline in `cmd/netsynth` | A `config` package makes the loader independently testable without a Cobra dependency. Given the complexity (validation, merge, discovery), a separate package is justified. |
|
||||||
|
|
||||||
|
**Installation:**
|
||||||
|
```bash
|
||||||
|
go get github.com/BurntSushi/toml@v1.6.0
|
||||||
|
```
|
||||||
|
|
||||||
|
**Version verification (confirmed 2026-03-26):**
|
||||||
|
```
|
||||||
|
github.com/BurntSushi/toml v1.6.0 (December 18, 2025)
|
||||||
|
github.com/pelletier/go-toml/v2 v2.3.0 (March 24, 2026 — alternative)
|
||||||
|
```
|
||||||
|
|
||||||
|
## Architecture Patterns
|
||||||
|
|
||||||
|
### Recommended Project Structure
|
||||||
|
|
||||||
|
```
|
||||||
|
config/
|
||||||
|
├── config.go # Load(), Merge(), Validate() — public API
|
||||||
|
└── config_test.go # table-driven tests for all CFG requirements
|
||||||
|
```
|
||||||
|
|
||||||
|
The `config` package has one exported function signature the planner cares about:
|
||||||
|
|
||||||
|
```go
|
||||||
|
// Load finds, parses, validates, and merges a config file.
|
||||||
|
// configPath is the --config flag value; empty string triggers auto-discovery.
|
||||||
|
// Returns the merged FreqConfig map (defaults + overrides) ready for synth.NewBank.
|
||||||
|
// Returns an error on: file-not-found when --config is explicit, parse errors,
|
||||||
|
// unknown keys, type mismatches. Returns no error (uses defaults) when no config
|
||||||
|
// is found during auto-discovery.
|
||||||
|
func Load(configPath string) (map[classify.TrafficClass]synth.FreqConfig, error)
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 1: TOML Decode Struct with Pointer Fields
|
||||||
|
|
||||||
|
**What:** The TOML config file maps to a Go struct where every overridable field is a pointer. `nil` means "not set by user"; non-nil means "user explicitly specified this value."
|
||||||
|
|
||||||
|
**When to use:** Whenever you need to distinguish "field absent from config" from "field set to zero value" — mandatory for partial override semantics (CFG-04).
|
||||||
|
|
||||||
|
```go
|
||||||
|
// Source: BurntSushi/toml documentation + partial-override pattern
|
||||||
|
// config/config.go
|
||||||
|
|
||||||
|
// SoundOverride holds optional per-class sound parameters from TOML.
|
||||||
|
// Pointer fields: nil = not set (keep default), non-nil = user override.
|
||||||
|
type SoundOverride struct {
|
||||||
|
Frequency *float64 `toml:"frequency"`
|
||||||
|
Waveform *string `toml:"waveform"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// rawConfig is the top-level TOML decode target.
|
||||||
|
type rawConfig struct {
|
||||||
|
Sounds map[string]SoundOverride `toml:"sounds"`
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 2: Unknown-Key Detection with MetaData.Undecoded()
|
||||||
|
|
||||||
|
**What:** After decoding, check `md.Undecoded()` for any keys in the TOML file that did not map to a field in the decode struct. Return an error naming the first unrecognized key.
|
||||||
|
|
||||||
|
**When to use:** Required for CFG-05. Also the mechanism to detect field-level typos within a `[sounds.ICMP]` block (e.g., `frequncy` vs `frequency`).
|
||||||
|
|
||||||
|
```go
|
||||||
|
// Source: pkg.go.dev/github.com/BurntSushi/toml
|
||||||
|
// config/config.go
|
||||||
|
|
||||||
|
func parse(path string) (rawConfig, error) {
|
||||||
|
var raw rawConfig
|
||||||
|
md, err := toml.DecodeFile(path, &raw)
|
||||||
|
if err != nil {
|
||||||
|
return raw, fmt.Errorf("config parse error: %w", err)
|
||||||
|
}
|
||||||
|
if undecoded := md.Undecoded(); len(undecoded) > 0 {
|
||||||
|
// undecoded[0] is a toml.Key ([]string); join for human-readable path
|
||||||
|
keyPath := strings.Join(undecoded[0], ".")
|
||||||
|
return raw, fmt.Errorf("config: unknown key %q — check spelling", keyPath)
|
||||||
|
}
|
||||||
|
return raw, nil
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**IMPORTANT NOTE on nested map + Undecoded():** When the decode struct uses `map[string]SoundOverride` for `[sounds]`, the TOML library cannot know what map keys are "valid" — all string keys are valid map keys. This means `Undecoded()` will NOT catch a misspelled class name like `[sounds.ICMP_typo]` (it IS decoded, just into a wrong map key). However, `Undecoded()` WILL catch field-level typos within a class block like `[sounds.ICMP]` with `frequncy = 440` because `frequncy` doesn't match any `SoundOverride` field. Class-name validation is handled separately in the merge step (D-09: log a warning for unknown class names).
|
||||||
|
|
||||||
|
### Pattern 3: Per-Field Overlay Merge
|
||||||
|
|
||||||
|
**What:** Iterate over the default `ClassFreqConfigs` map, copy it, then for each entry found in the TOML overrides, copy only the non-nil pointer fields into the working copy.
|
||||||
|
|
||||||
|
**When to use:** This is the CFG-04 implementation. Must run after parse and validation.
|
||||||
|
|
||||||
|
```go
|
||||||
|
// config/config.go
|
||||||
|
|
||||||
|
func merge(
|
||||||
|
defaults map[classify.TrafficClass]synth.FreqConfig,
|
||||||
|
overrides map[string]SoundOverride,
|
||||||
|
) map[classify.TrafficClass]synth.FreqConfig {
|
||||||
|
// Deep-copy defaults
|
||||||
|
result := make(map[classify.TrafficClass]synth.FreqConfig, len(defaults))
|
||||||
|
for k, v := range defaults {
|
||||||
|
result[k] = v
|
||||||
|
}
|
||||||
|
|
||||||
|
for className, override := range overrides {
|
||||||
|
class := classify.TrafficClass(className)
|
||||||
|
cfg, known := result[class]
|
||||||
|
if !known {
|
||||||
|
// D-09: unknown class name = warning, not error (Phase 7 may define it)
|
||||||
|
fmt.Fprintf(os.Stderr, "Warning: config: unknown class %q (ignored)\n", className)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if override.Frequency != nil {
|
||||||
|
cfg.BaseHz = *override.Frequency
|
||||||
|
// When frequency changes, regenerate harmonics if a waveform preset is active
|
||||||
|
if cfg.WaveformType != synth.WaveformCustom {
|
||||||
|
cfg.Harmonics = synth.WaveformPresetHarmonics(cfg.WaveformType, cfg.BaseHz, synth.SampleRate)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if override.Waveform != nil {
|
||||||
|
wt, err := parseWaveform(*override.Waveform)
|
||||||
|
if err != nil {
|
||||||
|
// Validation catches this before merge; this is a safety guard
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
cfg.WaveformType = wt
|
||||||
|
cfg.Harmonics = synth.WaveformPresetHarmonics(wt, cfg.BaseHz, synth.SampleRate)
|
||||||
|
}
|
||||||
|
result[class] = cfg
|
||||||
|
}
|
||||||
|
return result
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 4: Waveform String-to-Type Mapping
|
||||||
|
|
||||||
|
**What:** A simple switch converts the TOML `waveform` string to `synth.WaveformType`. Validation happens before merge.
|
||||||
|
|
||||||
|
```go
|
||||||
|
// config/config.go
|
||||||
|
|
||||||
|
var validWaveforms = map[string]synth.WaveformType{
|
||||||
|
"sine": synth.WaveformSine,
|
||||||
|
"square": synth.WaveformSquare,
|
||||||
|
"sawtooth": synth.WaveformSawtooth,
|
||||||
|
"triangle": synth.WaveformTriangle,
|
||||||
|
}
|
||||||
|
|
||||||
|
func parseWaveform(s string) (synth.WaveformType, error) {
|
||||||
|
if wt, ok := validWaveforms[s]; ok {
|
||||||
|
return wt, nil
|
||||||
|
}
|
||||||
|
valid := []string{"sine", "square", "sawtooth", "triangle"}
|
||||||
|
return 0, fmt.Errorf("config: invalid waveform %q — valid values: %s", s, strings.Join(valid, ", "))
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 5: Auto-Discovery with os.UserConfigDir
|
||||||
|
|
||||||
|
**What:** Check paths in precedence order. Return the path of the first file found, or `""` (empty) if none found. Never log anything for a missing auto-discovered file.
|
||||||
|
|
||||||
|
```go
|
||||||
|
// config/config.go
|
||||||
|
|
||||||
|
func discoverPath() string {
|
||||||
|
// 1. Working directory
|
||||||
|
if _, err := os.Stat("netsynth.toml"); err == nil {
|
||||||
|
return "netsynth.toml"
|
||||||
|
}
|
||||||
|
// 2. XDG config dir
|
||||||
|
dir, err := os.UserConfigDir()
|
||||||
|
if err != nil {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
p := filepath.Join(dir, "netsynth", "config.toml")
|
||||||
|
if _, err := os.Stat(p); err == nil {
|
||||||
|
return p
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
`os.UserConfigDir()` returns `$XDG_CONFIG_HOME` or `$HOME/.config` on Linux (Go stdlib, no extra dependency). Confirmed by Go source: returns `$XDG_CONFIG_HOME` if set, else `$HOME/.config` on Unix.
|
||||||
|
|
||||||
|
### Pattern 6: encode.RunSynthesis Signature Change
|
||||||
|
|
||||||
|
**What:** `RunSynthesis` currently calls `synth.NewBank(1.0, synth.ClassFreqConfigs)` hardcoded. Phase 6 changes the signature to accept the merged config map.
|
||||||
|
|
||||||
|
The simplest approach: pass the merged config map as a parameter (rather than loading config inside `encode`). This keeps `encode` unaware of config loading and makes testing easier.
|
||||||
|
|
||||||
|
```go
|
||||||
|
// encode/mp3.go — updated signature
|
||||||
|
func RunSynthesis(
|
||||||
|
snapshots []classify.WindowSnapshot,
|
||||||
|
outputPath string,
|
||||||
|
freqCfgs map[classify.TrafficClass]synth.FreqConfig,
|
||||||
|
) error {
|
||||||
|
// ...
|
||||||
|
bank := synth.NewBank(1.0, freqCfgs) // was: synth.ClassFreqConfigs
|
||||||
|
// ...
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Caller in `cmd/netsynth/main.go` passes the result of `config.Load(configPath)`.
|
||||||
|
|
||||||
|
### Anti-Patterns to Avoid
|
||||||
|
|
||||||
|
- **Decode into `map[string]interface{}`:** Loses type safety, makes unknown-field detection harder, requires runtime type assertions. Use typed structs.
|
||||||
|
- **Load config inside `encode` package:** Couples audio encoding to config I/O; breaks test isolation. Config loading belongs in `cmd/netsynth/main.go` (calls `config.Load`) or a dedicated `config` package.
|
||||||
|
- **Validate waveform strings after merge:** Validate before merging so the error is caught at startup (D-11), not silently ignored.
|
||||||
|
- **Deep-copy using `=` assignment on map values:** `synth.FreqConfig` contains a `[]HarmonicDef` slice; a simple struct copy shares the underlying array. Use an explicit copy of the slice if you mutate `Harmonics` during merge. (The merge code above reconstructs harmonics from the preset, so this is safe — but important to be aware of.)
|
||||||
|
|
||||||
|
## Don't Hand-Roll
|
||||||
|
|
||||||
|
| Problem | Don't Build | Use Instead | Why |
|
||||||
|
|---------|-------------|-------------|-----|
|
||||||
|
| TOML parsing | Custom parser | `BurntSushi/toml` v1.6.0 | TOML 1.1 compliance, error messages, datetime support, tested at scale |
|
||||||
|
| Unknown-key detection | Post-parse key comparison | `md.Undecoded()` from BurntSushi | Already built into the library; handles nested paths correctly |
|
||||||
|
| XDG config path | Manual `$HOME/.config` string concat | `os.UserConfigDir()` stdlib | Handles `$XDG_CONFIG_HOME` override correctly, platform-portable |
|
||||||
|
|
||||||
|
**Key insight:** The partial-override merge logic is the one piece that must be written from scratch — no library does "overlay a sparse map of optional overrides over a typed defaults map." But it is ~20 lines of straightforward Go.
|
||||||
|
|
||||||
|
## Runtime State Inventory
|
||||||
|
|
||||||
|
Step 2.5 SKIPPED — this is a new feature addition, not a rename/refactor/migration phase. No runtime state is being renamed or migrated.
|
||||||
|
|
||||||
|
## Common Pitfalls
|
||||||
|
|
||||||
|
### Pitfall 1: Undecoded() Does Not Catch Unknown Class Names
|
||||||
|
|
||||||
|
**What goes wrong:** Developer assumes `md.Undecoded()` will catch `[sounds.ICMP_TYPO]` as an unknown key and provide CFG-05 coverage for class-name typos.
|
||||||
|
|
||||||
|
**Why it happens:** `map[string]SoundOverride` decodes any string as a valid map key — the TOML parser has no way to know which class names are valid. `Undecoded()` only catches keys that don't match ANY field (struct field name, map key, or slice element). Since all map keys are valid, no class name is "undecodeable."
|
||||||
|
|
||||||
|
**How to avoid:** Separate the two concerns. Field-level unknown keys (e.g., `frequncy`) ARE caught by `Undecoded()`. Class-name typos are caught in the merge step by checking whether `classify.TrafficClass(className)` exists in `synth.ClassFreqConfigs`. The CONTEXT.md decision D-09 says unknown class names produce a warning (not error) to allow for Phase 7 user-defined classes — so this is by design.
|
||||||
|
|
||||||
|
**Warning signs:** Test for both: write a test with `[sounds.ICMP]` containing `frequncy = 440` (should error) AND a test with `[sounds.ICMP_TYPO]` containing `frequency = 440` (should warn, not error).
|
||||||
|
|
||||||
|
### Pitfall 2: Partial Override Accidentally Clears WaveformType
|
||||||
|
|
||||||
|
**What goes wrong:** User sets only `frequency = 300` for ICMP. After merge, ICMP's `WaveformType` is reset to `WaveformCustom` because the merge loop creates a new `FreqConfig{}` instead of starting from the default.
|
||||||
|
|
||||||
|
**Why it happens:** Copy-by-value from defaults is skipped, or merge starts from a zero-value struct.
|
||||||
|
|
||||||
|
**How to avoid:** Always start the merge from the DEFAULT `FreqConfig` for that class. The merge loop copies `defaults[class]` first, then overlays only non-nil pointer fields.
|
||||||
|
|
||||||
|
**Warning signs:** Test case: set only `frequency` for a class with `WaveformCustom` — verify waveform field is unchanged. Test case: set only `waveform` for a class — verify frequency is unchanged.
|
||||||
|
|
||||||
|
### Pitfall 3: Frequency Change Does Not Regenerate Harmonics for Preset Waveforms
|
||||||
|
|
||||||
|
**What goes wrong:** User sets `frequency = 300` for HTTPS (which has `WaveformCustom` by default, so this is fine). But if a user sets `frequency = 300` for a class that was previously configured with `WaveformSine` (via an earlier config entry), the harmonics may be stale from the old frequency.
|
||||||
|
|
||||||
|
**Why it happens:** `synth.WaveformPresetHarmonics` generates harmonics based on `baseHz`. If you update `BaseHz` without regenerating harmonics, the preset harmonics are anchored to the old frequency.
|
||||||
|
|
||||||
|
**How to avoid:** In the merge function: when updating `Frequency`, check if `WaveformType != WaveformCustom`. If true, regenerate `Harmonics` from the new frequency. The merge example above handles this correctly.
|
||||||
|
|
||||||
|
**Warning signs:** For the 14 built-in classes, all have `WaveformCustom` (hand-tuned harmonics), so this pitfall only bites if the user sets both `waveform` and `frequency` in two separate steps — or if a future phase pre-configures preset waveforms on built-ins.
|
||||||
|
|
||||||
|
### Pitfall 4: --config File-Not-Found vs Auto-Discovery Silence
|
||||||
|
|
||||||
|
**What goes wrong:** When `--config /path/to/missing.toml` is specified, the code returns the same "no config found, using defaults" behavior as auto-discovery silence.
|
||||||
|
|
||||||
|
**Why it happens:** `os.Stat` errors are treated uniformly regardless of how the path was obtained.
|
||||||
|
|
||||||
|
**How to avoid:** In the `Load` function, branch on whether `configPath` was explicitly provided: if it was, a `fs.ErrNotExist` is a user error (return error); if it came from auto-discovery, `fs.ErrNotExist` is normal (return `nil` error, use defaults).
|
||||||
|
|
||||||
|
**Warning signs:** CFG-03 acceptance criterion explicitly tests this: explicit path must error, absent auto-discovery must be silent.
|
||||||
|
|
||||||
|
### Pitfall 5: go.mod Tidy Drops TOML Dependency
|
||||||
|
|
||||||
|
**What goes wrong:** `go mod tidy` is run after adding BurntSushi/toml to go.mod but before any `.go` file in the module actually imports it. Tidy removes it.
|
||||||
|
|
||||||
|
**Why it happens:** `go mod tidy` removes unused dependencies.
|
||||||
|
|
||||||
|
**How to avoid:** Add the import in `config/config.go` before running `go mod tidy`.
|
||||||
|
|
||||||
|
## Code Examples
|
||||||
|
|
||||||
|
### Complete config.go Skeleton
|
||||||
|
|
||||||
|
```go
|
||||||
|
// Source: BurntSushi/toml docs + project pattern
|
||||||
|
// config/config.go
|
||||||
|
package config
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"fmt"
|
||||||
|
"io/fs"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
|
|
||||||
|
"github.com/BurntSushi/toml"
|
||||||
|
|
||||||
|
"github.com/netsynth/netsynth/classify"
|
||||||
|
"github.com/netsynth/netsynth/synth"
|
||||||
|
)
|
||||||
|
|
||||||
|
type SoundOverride struct {
|
||||||
|
Frequency *float64 `toml:"frequency"`
|
||||||
|
Waveform *string `toml:"waveform"`
|
||||||
|
}
|
||||||
|
|
||||||
|
type rawConfig struct {
|
||||||
|
Sounds map[string]SoundOverride `toml:"sounds"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// Load is the single public entry point.
|
||||||
|
// configPath: value of --config flag; empty = auto-discover.
|
||||||
|
func Load(configPath string) (map[classify.TrafficClass]synth.FreqConfig, error) {
|
||||||
|
path, explicit, err := resolvePath(configPath)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
if path == "" {
|
||||||
|
// No config found during auto-discovery — use defaults silently (CFG-02)
|
||||||
|
return copyDefaults(), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
raw, err := parseFile(path)
|
||||||
|
if err != nil {
|
||||||
|
if explicit && errors.Is(err, fs.ErrNotExist) {
|
||||||
|
return nil, fmt.Errorf("config file not found: %s", path)
|
||||||
|
}
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := validate(raw); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
return merge(copyDefaults(), raw.Sounds), nil
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Example TOML Config File
|
||||||
|
|
||||||
|
```toml
|
||||||
|
# netsynth.toml — override ICMP and SSH sounds
|
||||||
|
[sounds.ICMP]
|
||||||
|
frequency = 80.0
|
||||||
|
waveform = "square"
|
||||||
|
|
||||||
|
[sounds.SSH]
|
||||||
|
frequency = 400.0
|
||||||
|
# waveform not set — SSH keeps its default waveform
|
||||||
|
```
|
||||||
|
|
||||||
|
### Test Pattern (table-driven)
|
||||||
|
|
||||||
|
```go
|
||||||
|
// config/config_test.go
|
||||||
|
|
||||||
|
func TestLoadPartialOverride(t *testing.T) {
|
||||||
|
// Write a temp TOML file with only frequency for ICMP
|
||||||
|
tomlContent := `
|
||||||
|
[sounds.ICMP]
|
||||||
|
frequency = 100.0
|
||||||
|
`
|
||||||
|
f, _ := os.CreateTemp(t.TempDir(), "*.toml")
|
||||||
|
f.WriteString(tomlContent)
|
||||||
|
f.Close()
|
||||||
|
|
||||||
|
cfgs, err := Load(f.Name())
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Load: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// ICMP frequency overridden
|
||||||
|
if cfgs[classify.ClassICMP].BaseHz != 100.0 {
|
||||||
|
t.Errorf("ICMP BaseHz: got %v, want 100.0", cfgs[classify.ClassICMP].BaseHz)
|
||||||
|
}
|
||||||
|
// ICMP waveform unchanged (WaveformCustom = 0)
|
||||||
|
if cfgs[classify.ClassICMP].WaveformType != synth.WaveformCustom {
|
||||||
|
t.Errorf("ICMP WaveformType: got %v, want WaveformCustom", cfgs[classify.ClassICMP].WaveformType)
|
||||||
|
}
|
||||||
|
// DNS frequency unchanged
|
||||||
|
if cfgs[classify.ClassDNS].BaseHz != synth.ClassFreqConfigs[classify.ClassDNS].BaseHz {
|
||||||
|
t.Errorf("DNS BaseHz unexpectedly changed")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestLoadUnknownKey(t *testing.T) {
|
||||||
|
tomlContent := `
|
||||||
|
[sounds.ICMP]
|
||||||
|
frequncy = 440
|
||||||
|
`
|
||||||
|
f, _ := os.CreateTemp(t.TempDir(), "*.toml")
|
||||||
|
f.WriteString(tomlContent)
|
||||||
|
f.Close()
|
||||||
|
|
||||||
|
_, err := Load(f.Name())
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error for unknown key 'frequncy', got nil")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "frequncy") {
|
||||||
|
t.Errorf("error should name the bad key, got: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## State of the Art
|
||||||
|
|
||||||
|
| Old Approach | Current Approach | When Changed | Impact |
|
||||||
|
|--------------|------------------|--------------|--------|
|
||||||
|
| `google/gopacket` | `gopacket/gopacket` v1.5.0 | 2022-2024 | N/A for this phase |
|
||||||
|
| BurntSushi/toml v0.x | v1.6.0 (TOML 1.1 enabled by default) | December 2025 | TOML 1.1 compliance; API unchanged, same `Decode`/`DecodeFile` functions |
|
||||||
|
| `go-audio/generator` | ARCHIVED (Feb 2026, read-only) | February 2026 | Do not use; project already avoids it |
|
||||||
|
|
||||||
|
**Current versions confirmed 2026-03-26:**
|
||||||
|
- `BurntSushi/toml` v1.6.0 (December 18, 2025) — TOML 1.1 default, stable API
|
||||||
|
- `pelletier/go-toml/v2` v2.3.0 (March 24, 2026) — alternative if structured error needed
|
||||||
|
|
||||||
|
## Open Questions
|
||||||
|
|
||||||
|
1. **Typo suggestion for unknown keys (D-07 nice-to-have)**
|
||||||
|
- What we know: BurntSushi returns `[]toml.Key` (structured), Levenshtein distance is ~15 lines of Go or `github.com/agnivade/levenshtein` (tiny, zero-dependency)
|
||||||
|
- What's unclear: Is the complexity worth it for 2 valid field names per class block (`frequency`, `waveform`)?
|
||||||
|
- Recommendation: Skip the external library. Implement inline: for each undecoded key, if it has edit distance ≤ 2 from any valid key name, append " (did you mean: X?)" to the error. The valid key set for field names is small and static: `["frequency", "waveform"]`. This is ~10 lines of Go.
|
||||||
|
|
||||||
|
2. **copyDefaults() — shallow vs deep copy of Harmonics slices**
|
||||||
|
- What we know: `synth.FreqConfig.Harmonics` is a `[]HarmonicDef`. Go's `map[K]V` assignment copies struct values (including slice headers) but the underlying array is shared.
|
||||||
|
- What's unclear: Does this matter if merge only replaces the whole slice (via `WaveformPresetHarmonics`) rather than appending to it?
|
||||||
|
- Recommendation: Since the merge code assigns a freshly-generated `[]HarmonicDef` from `WaveformPresetHarmonics` (never mutates the original), shallow copy is safe. No deep copy needed. Document this in a comment for future maintainers.
|
||||||
|
|
||||||
|
## Environment Availability
|
||||||
|
|
||||||
|
Step 2.6: This phase introduces one new external dependency:
|
||||||
|
|
||||||
|
| Dependency | Required By | Available | Version | Fallback |
|
||||||
|
|------------|------------|-----------|---------|----------|
|
||||||
|
| `github.com/BurntSushi/toml` | config.Load() TOML parsing | ✓ (fetched via go get) | v1.6.0 | pelletier/go-toml v2.3.0 |
|
||||||
|
| `os.UserConfigDir()` | Auto-discovery of `~/.config/netsynth/config.toml` | ✓ (Go stdlib) | Go 1.13+ | N/A — stdlib |
|
||||||
|
| Go 1.24.1 toolchain | Module minimum | ✓ | 1.24.1 | N/A |
|
||||||
|
| C compiler (CGo) | go-lame MP3 encoding (pre-existing) | Assumed ✓ (Phase 2+ already requires this) | — | N/A |
|
||||||
|
|
||||||
|
No missing dependencies with no fallback. BurntSushi/toml confirmed fetchable from pkg.go.dev.
|
||||||
|
|
||||||
|
## Validation Architecture
|
||||||
|
|
||||||
|
### Test Framework
|
||||||
|
|
||||||
|
| Property | Value |
|
||||||
|
|----------|-------|
|
||||||
|
| Framework | Go standard `testing` package |
|
||||||
|
| Config file | None — `go test ./...` |
|
||||||
|
| Quick run command | `go test ./config/...` |
|
||||||
|
| Full suite command | `go test ./...` |
|
||||||
|
|
||||||
|
### Phase Requirements → Test Map
|
||||||
|
|
||||||
|
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|
||||||
|
|--------|----------|-----------|-------------------|-------------|
|
||||||
|
| CFG-01 | TOML overrides applied to correct class | unit | `go test ./config/... -run TestLoadOverride` | Wave 0 |
|
||||||
|
| CFG-02 | No config file → silent, uses defaults | unit | `go test ./config/... -run TestLoadNoConfig` | Wave 0 |
|
||||||
|
| CFG-03 | `--config` explicit path → error if missing | unit | `go test ./config/... -run TestLoadExplicitMissing` | Wave 0 |
|
||||||
|
| CFG-04 | Partial override: unset fields unchanged | unit | `go test ./config/... -run TestLoadPartialOverride` | Wave 0 |
|
||||||
|
| CFG-05 | Unknown key → error naming the key | unit | `go test ./config/... -run TestLoadUnknownKey` | Wave 0 |
|
||||||
|
| CFG-03 | `--config` flag wired in Cobra | integration | `go test ./cmd/netsynth/... -run TestConfigFlag` | Wave 0 |
|
||||||
|
|
||||||
|
### Sampling Rate
|
||||||
|
|
||||||
|
- **Per task commit:** `go test ./config/... -count=1`
|
||||||
|
- **Per wave merge:** `go test ./... -count=1`
|
||||||
|
- **Phase gate:** Full suite green before `/gsd:verify-work`
|
||||||
|
|
||||||
|
### Wave 0 Gaps
|
||||||
|
|
||||||
|
- [ ] `config/config.go` — package does not exist yet; create in Wave 1
|
||||||
|
- [ ] `config/config_test.go` — covers CFG-01 through CFG-05
|
||||||
|
- [ ] `cmd/netsynth/main_test.go` — add `TestConfigFlag` covering CFG-03 CLI integration
|
||||||
|
|
||||||
|
*(Existing test infrastructure covers all other packages; only `config/` is new.)*
|
||||||
|
|
||||||
|
## Sources
|
||||||
|
|
||||||
|
### Primary (HIGH confidence)
|
||||||
|
|
||||||
|
- `pkg.go.dev/github.com/BurntSushi/toml` — v1.6.0 API: `DecodeFile`, `MetaData.Undecoded()`, `[]Key` type; verified 2026-03-26
|
||||||
|
- Go stdlib `os.UserConfigDir()` — returns `$XDG_CONFIG_HOME` or `$HOME/.config` on Linux; Go 1.13+ feature
|
||||||
|
- `pkg.go.dev/github.com/pelletier/go-toml/v2` — v2.3.0 `DisallowUnknownFields()` / `StrictMissingError` API; verified 2026-03-26
|
||||||
|
|
||||||
|
### Secondary (MEDIUM confidence)
|
||||||
|
|
||||||
|
- WebSearch: BurntSushi/toml Undecoded() approach verified against official GitHub source (`toml/decode.go`)
|
||||||
|
- WebSearch: pelletier/go-toml v2 DisallowUnknownFields verified against official docs
|
||||||
|
- WebSearch: `os.UserConfigDir` XDG compliance — confirmed returns `$XDG_CONFIG_HOME` or `$HOME/.config` on Linux per golang/go issue #29960
|
||||||
|
|
||||||
|
### Tertiary (LOW confidence)
|
||||||
|
|
||||||
|
- WebSearch: edit distance typo suggestion libraries (agnivade/levenshtein, go-edlib) — not deeply evaluated; recommendation is inline 10-line implementation to avoid dependency
|
||||||
|
|
||||||
|
## Metadata
|
||||||
|
|
||||||
|
**Confidence breakdown:**
|
||||||
|
|
||||||
|
- Standard stack: HIGH — versions confirmed via `go get` live fetch (v1.6.0 BurntSushi, v2.3.0 pelletier)
|
||||||
|
- Architecture: HIGH — patterns derived from library documentation + existing codebase patterns
|
||||||
|
- Pitfalls: HIGH — Undecoded() + map key limitation is a documented behavior; partial-override via pointer fields is an established Go idiom
|
||||||
|
- TOML typo suggestion: LOW — nice-to-have from D-07; no deep investigation needed given small valid-key set
|
||||||
|
|
||||||
|
**Research date:** 2026-03-26
|
||||||
|
**Valid until:** 2026-06-26 (BurntSushi/toml is stable; go-toml v2 moves faster but is not the chosen library)
|
||||||
Reference in New Issue
Block a user