Files
yoloyolo/.planning/research/PITFALLS.md
T

512 lines
29 KiB
Markdown
Raw Permalink Normal View History

2026-03-24 22:36:28 +01:00
# Pitfalls Research
**Domain:** Network-traffic-to-audio synthesis CLI tool (Go)
2026-03-26 16:54:31 +01:00
**Researched:** 2026-03-26 (v1.1 update — TOML config, waveform types, user-defined rules)
**Confidence:** HIGH (TOML decoder behaviors verified against pkg.go.dev official docs and issue trackers; audio synthesis aliasing verified against DSP literature; config merging verified against BurntSushi/toml issue #47 and go-toml issue #252)
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
## v1.1 Milestone Pitfalls (New)
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
These pitfalls are specific to adding TOML config, waveform types, and user-defined classification rules to the existing NetSynth codebase.
---
### Pitfall A1: TOML Unmarshal Silently Overwrites Pre-filled Defaults with Zero Values
2026-03-24 22:36:28 +01:00
**What goes wrong:**
2026-03-26 16:54:31 +01:00
You initialize a `Config` struct with built-in defaults, then call `toml.Unmarshal` to layer in user overrides. Any field the user *omits* from their TOML file is set to its Go zero value (`0`, `""`, `false`, `nil`) by the decoder — overwriting your defaults. A user who writes only `[sounds.DNS]` in their config file to change the DNS tone ends up wiping every other class back to zero Hz.
2026-03-24 22:36:28 +01:00
**Why it happens:**
2026-03-26 16:54:31 +01:00
Both `BurntSushi/toml` and `pelletier/go-toml` v1 do not distinguish between "key was absent" and "key was explicitly set to zero". The decoder reflects over the struct and writes zero for every absent key. This was explicitly reported as a bug in BurntSushi/toml issue #47 and go-toml issue #252. go-toml v2 partially addresses it but still zeros primitive-type fields that are absent.
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Consequences:**
- All non-overridden traffic classes play silence (0 Hz oscillator)
- Classification rules get zeroed if user only partially fills `[[rules]]`
- EMA tau, whisper floor, gain, and other synth parameters reset to 0
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Prevention:**
Use pointer fields (`*float64`, `*string`) in the decoded struct to distinguish "not provided" (nil pointer) from "explicitly set to zero" (non-nil pointer to 0). Apply a merge step: iterate over the decoded struct, and for each pointer field that is nil, keep the built-in default. For slice fields (like `[]RuleConfig`), nil slice means "user did not provide rules" — preserve defaults; non-nil empty slice (`[]RuleConfig{}`) means "user explicitly cleared rules" — respect that.
```go
// In config struct, use pointers for optional overrides:
type SoundConfig struct {
FreqHz *float64 `toml:"freq_hz"`
Waveform *string `toml:"waveform"`
}
// Merge: for each class, override only non-nil fields
func mergeSound(base synth.FreqConfig, override SoundConfig) synth.FreqConfig {
if override.FreqHz != nil {
base.BaseHz = *override.FreqHz
}
if override.Waveform != nil {
base.WaveformType = *override.Waveform
}
return base
}
```
**Detection:**
- User reports that classes they did not configure now produce no sound
- Unit test: load a config that overrides only one class; verify all other classes retain built-in Hz values
2026-03-24 22:36:28 +01:00
**Phase to address:**
2026-03-26 16:54:31 +01:00
Config loading phase (first phase of v1.1). Get the pointer-and-merge pattern established before wiring config into the bank. Retrofitting after the bank construction is wired is a significant churn.
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
### Pitfall A2: BurntSushi/toml Silently Ignores Typos in Field Names
2026-03-24 22:36:28 +01:00
**What goes wrong:**
2026-03-26 16:54:31 +01:00
A user writes `freq_hz = 440` but the struct tag is `toml:"freq_hz"` — this works. However if the user writes `freqhz = 440` or `FreqHz = 440` or a misspelled `frek_hz = 440`, the library silently ignores the key. The user's override is never applied. No error is returned. The user thinks their config is active; it is not.
2026-03-24 22:36:28 +01:00
**Why it happens:**
2026-03-26 16:54:31 +01:00
`BurntSushi/toml` by default silently discards keys that do not map to any struct field. This is the documented default behavior ("will ignore options in the TOML file that you don't use"). It is the opposite of "strict mode."
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Consequences:**
- Silent misconfiguration: user's customization is invisible
- Debugging is very hard — no error to trace back to the TOML file
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Prevention:**
Use `toml.Decode` (not `toml.Unmarshal`) to obtain `MetaData`, then call `md.Undecoded()` and return an error listing any keys that were not decoded. This is BurntSushi's documented strict-mode pattern.
```go
md, err := toml.Decode(string(data), &cfg)
if err != nil {
return err
}
if keys := md.Undecoded(); len(keys) > 0 {
return fmt.Errorf("unknown config keys (check for typos): %v", keys)
}
```
**Detection:**
- Config change that should audibly alter the sound has no effect
- Undecoded keys present but no warning/error logged
2026-03-24 22:36:28 +01:00
**Phase to address:**
2026-03-26 16:54:31 +01:00
Config loading phase. Implement strict decoding from the first config load function. Do not add this as an afterthought — it is the primary mechanism protecting users from silent misconfiguration.
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
### Pitfall A3: Naive Square/Sawtooth/Triangle Generation Produces Audible Aliasing Distortion
2026-03-24 22:36:28 +01:00
**What goes wrong:**
2026-03-26 16:54:31 +01:00
Implementing waveforms by direct time-domain math — `sign(sin(phase))` for square, `2*frac(phase)-1` for sawtooth, `1-2*abs(frac(phase)-0.5)` for triangle — produces a waveform with infinite harmonics. At 44100 Hz, harmonics above 22050 Hz fold back into the audible range as aliasing. At the frequencies used in NetSynth (651047 Hz), aliasing from a naive square wave produces a buzzing distortion that is especially audible at higher drone frequencies and sounds like corruption rather than timbre.
2026-03-24 22:36:28 +01:00
**Why it happens:**
2026-03-26 16:54:31 +01:00
The mathematical waveforms are not bandlimited — they have infinite harmonic content. Direct sampling them at 44100 Hz aliases all energy above Nyquist back into the audible band. Developers who test at low frequencies (60120 Hz) may not notice because the aliased harmonics land at very high frequencies with low perceptual impact; the problem worsens significantly above 400 Hz where aliases fold into the 15 kHz perceptually prominent range.
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Consequences:**
- Square/sawtooth at SSH (330 Hz) and higher frequencies sounds harsh and buzzy
- The effect worsens at higher frequencies, making SMTP (440 Hz) and DHCP (600 Hz) drones sound distorted
- Aliasing cannot be filtered out post-synthesis (it is interleaved with desired signal)
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Prevention:**
Use additive synthesis — the approach already in use for sine waves in `oscillator.go`. The existing `Oscillator.Advance(harmonics []HarmonicDef)` computes `sin(2π * phase * ratio)` for each partial. Square, sawtooth, and triangle waveforms are all expressible as harmonic series:
- **Square:** odd harmonics only, amplitude `1/k` for harmonic `k`: ratios 1, 3, 5, 7, ... with amplitudes 1.0, 0.33, 0.20, 0.14, ... Truncate at Nyquist.
- **Sawtooth:** all harmonics, amplitude `1/k`: ratios 1, 2, 3, 4, ... with amplitudes 1.0, 0.5, 0.33, 0.25, ... Truncate at Nyquist.
- **Triangle:** odd harmonics, amplitude `1/k²`, alternating sign: ratios 1, 3, 5, ... with amplitudes 1.0, 0.11, 0.04, ... Truncate at Nyquist.
The truncation (only sum harmonics where `freq * ratio < sampleRate / 2`) is the critical step that makes the synthesis bandlimited. The existing `[]HarmonicDef` structure in `synth/config.go` already supports this — waveform type selection just requires generating the right harmonic series for each `FreqConfig`.
Waveform presets should be pre-computed `[]HarmonicDef` slices, not runtime computation of naive waveform math:
```go
// BandlimitedHarmonics returns a bandlimited harmonic series for the given waveform type.
// It truncates harmonics at Nyquist (sampleRate/2) to prevent aliasing.
func BandlimitedHarmonics(waveform string, baseHz float64, sampleRate int) []HarmonicDef {
nyquist := float64(sampleRate) / 2.0
var defs []HarmonicDef
switch waveform {
case "square":
for k := 1; float64(k)*baseHz < nyquist; k += 2 { // odd only
defs = append(defs, HarmonicDef{Ratio: k, Amplitude: 1.0 / float64(k)})
}
case "sawtooth":
for k := 1; float64(k)*baseHz < nyquist; k++ {
defs = append(defs, HarmonicDef{Ratio: k, Amplitude: 1.0 / float64(k)})
}
case "triangle":
sign := 1.0
for k := 1; float64(k)*baseHz < nyquist; k += 2 {
defs = append(defs, HarmonicDef{Ratio: k, Amplitude: sign / float64(k*k)})
sign = -sign
}
default: // "sine"
defs = []HarmonicDef{{Ratio: 1, Amplitude: 1.0}}
}
return defs
}
```
**Detection:**
- Audible buzzing or grainy texture on drone layers above 300 Hz with non-sine waveforms
- Square/sawtooth waveforms sound harsher than expected at high frequencies
2026-03-24 22:36:28 +01:00
**Phase to address:**
2026-03-26 16:54:31 +01:00
Waveform type implementation phase. The design decision (additive synthesis, not direct waveform math) must be made before coding waveform support. Switching from direct math to additive after the fact requires rewriting the oscillator API.
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
### Pitfall A4: Waveform String Validation Fails Silently, Falls Back to Silence
2026-03-24 22:36:28 +01:00
**What goes wrong:**
2026-03-26 16:54:31 +01:00
A user writes `waveform = "Sawtooth"` (capital S) or `waveform = "saw"` (abbreviation). The config loading code does a simple equality check (`if waveform == "sawtooth"`), finds no match, and either panics, silently emits silence, or applies a default without telling the user. In all cases the user's intent is invisible.
2026-03-24 22:36:28 +01:00
**Why it happens:**
2026-03-26 16:54:31 +01:00
String-based enumerations in config files have no compile-time type checking. Case sensitivity and abbreviations are user expectations that must be explicitly handled.
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Consequences:**
- Silent misconfiguration: wrong waveform with no feedback
- Hard to debug: config appears valid, sound is just wrong
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Prevention:**
Normalize waveform strings at parse time (`strings.ToLower`, `strings.TrimSpace`), validate against the accepted set, and return an explicit error with the accepted values if the string is unrecognized:
```go
var validWaveforms = map[string]struct{}{
"sine": {}, "square": {}, "sawtooth": {}, "triangle": {},
}
func validateWaveform(s string) (string, error) {
normalized := strings.ToLower(strings.TrimSpace(s))
if _, ok := validWaveforms[normalized]; !ok {
return "", fmt.Errorf("unknown waveform %q: must be one of sine, square, sawtooth, triangle", s)
}
return normalized, nil
}
```
2026-03-24 22:36:28 +01:00
**Phase to address:**
2026-03-26 16:54:31 +01:00
Config validation step (same phase as config loading). Implement all string field validation in a single `validate(cfg Config) error` function called immediately after decoding.
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
### Pitfall A5: User Rules Appended After Catch-All Rules Are Unreachable
2026-03-24 22:36:28 +01:00
**What goes wrong:**
2026-03-26 16:54:31 +01:00
The existing `DefaultRules` slice ends with two catch-alls:
```go
{Protocol: "tcp", DstPort: 0, Class: ClassOtherTCP},
{Protocol: "udp", DstPort: 0, Class: ClassOtherUDP},
```
If user-defined rules are simply appended to this slice (`append(DefaultRules, userRules...)`), the catch-alls match first (DstPort=0 matches any port for that protocol), and the user's rules are unreachable. Every custom rule maps to `ClassOtherTCP` or `ClassOtherUDP` instead. The user gets no sound from their custom class.
2026-03-24 22:36:28 +01:00
**Why it happens:**
2026-03-26 16:54:31 +01:00
The first-match-wins semantics of `Classifier.Classify()` mean ordering is semantically critical. `DefaultRules` is a named var that exists precisely as an ordered slice — the comment `// Catch-alls (must be last)` documents this constraint. But "must be last in the defaults" does not automatically mean "must be last in the final merged slice." Developers who concatenate slices without thinking about this invariant break the system.
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Consequences:**
- All user-defined rules are silently swallowed by catch-alls
- User's custom class never activates
- No error — the pipeline works, just wrong
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Prevention:**
Always insert user rules *before* catch-all rules. The merge strategy must be: `specificDefaultRules + userRules + catchAllRules`. Implement this with an explicit split in the default rule set:
```go
// In classify/rules.go, split into two exported slices:
var SpecificRules = []Rule{ /* ICMP through DHCP */ }
var CatchAllRules = []Rule{
{Protocol: "tcp", DstPort: 0, Class: ClassOtherTCP},
{Protocol: "udp", DstPort: 0, Class: ClassOtherUDP},
}
// Merge function used by config loading:
func MergeRules(userRules []Rule) []Rule {
result := make([]Rule, 0, len(SpecificRules)+len(userRules)+len(CatchAllRules))
result = append(result, SpecificRules...)
result = append(result, userRules...)
result = append(result, CatchAllRules...)
return result
}
```
Alternatively, annotate each default rule with a `CatchAll bool` field and sort before use. The split-slice approach is simpler and more explicit.
**Detection:**
- User-defined rule that should match traffic does not produce its custom sound
- `--verbose` output shows traffic being classified as `OtherTCP`/`OtherUDP` instead of the custom class
- Test: write a rule for port 8080, send HTTP traffic to port 8080, verify it hits the custom class and not `ClassOtherTCP`
2026-03-24 22:36:28 +01:00
**Phase to address:**
2026-03-26 16:54:31 +01:00
User-defined rules phase. The `classify/rules.go` split must be the first code change before any config loading logic references the rule slice.
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
### Pitfall A6: User-Defined Classes Have No synth.FreqConfig Entry — Bank Panics or Plays Silence
2026-03-24 22:36:28 +01:00
**What goes wrong:**
2026-03-26 16:54:31 +01:00
`OscillatorBank.NewBank()` iterates over `classify.AllClasses()` and looks up each class in `ClassFreqConfigs`. A user-defined rule creates a new `TrafficClass` (e.g., `"my-api"`). This class is not in `AllClasses()`, so the bank has no layer for it. The aggregator increments a count for `"my-api"`, `RenderWindow` looks up `b.layers["my-api"]`, gets nil, and either panics (nil pointer dereference on `layer.AdvanceSample()`) or silently contributes nothing to the mix.
2026-03-24 22:36:28 +01:00
**Why it happens:**
2026-03-26 16:54:31 +01:00
`classify.AllClasses()` is a hardcoded list of the 14 built-in classes. The synth bank is constructed once at startup from this static list. User-defined classes are a runtime extension that the bank knows nothing about.
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Consequences:**
- Nil pointer panic in `RenderWindow` if the layer map lookup is not nil-guarded
- Or silent: user-defined class traffic is captured and aggregated but never rendered to audio
- In either case the user's primary feature request (custom sounds for custom classes) silently fails
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Prevention:**
The bank must be constructed from the *full* set of active classes, including user-defined ones. The construction path should be:
1. Load config (parse TOML, validate)
2. Compute effective rule set (built-in + user rules)
3. Extract the complete set of `TrafficClass` values referenced by all rules
4. Pass this full class set to `NewBank` (or equivalent) so a layer is created for every reachable class
5. Wire user-defined class frequencies from config into the bank
`AllClasses()` in `classify/types.go` should either remain the static built-in list (used for display/iteration of built-ins) or be replaced by a dynamic function that takes the active rule set as input. Do not rely on the hardcoded list in the bank-construction path when user-defined classes are possible.
**Detection:**
- Panic: `runtime error: invalid memory address or nil pointer dereference` in `synth/bank.go:RenderWindow`
- Or: user-defined class produces no sound, no error
- Test: create a config with one user rule using a custom class; verify the bank is built with a layer for that class and that layer produces sound
2026-03-24 22:36:28 +01:00
**Phase to address:**
2026-03-26 16:54:31 +01:00
User-defined rules phase, specifically the bank initialization step. This is the deepest integration point — it touches the pipeline at capture → classify → aggregate → synthesize.
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
### Pitfall A7: Config Auto-Discovery Follows Wrong Order or Ignores XDG Variables
2026-03-24 22:36:28 +01:00
**What goes wrong:**
2026-03-26 16:54:31 +01:00
The spec calls for auto-discovery from `./netsynth.toml` then `~/.config/netsynth/config.toml`. A naive implementation uses `os.UserHomeDir()` to build the fallback path. On systems where `$XDG_CONFIG_HOME` is set to a non-default location (common on NixOS, custom dotfile managers, CI environments), the tool ignores the user's configured config directory and looks in `~/.config` anyway. The user has a config at `$XDG_CONFIG_HOME/netsynth/config.toml` that is never found.
Additionally, `os.UserHomeDir()` returns an error if `$HOME` is unset (e.g., inside some Docker containers or cron jobs). If this error is not handled, the path construction silently produces `"/.config/netsynth/config.toml"` (an absolute path starting with `/.config`) rather than failing with a useful message.
2026-03-24 22:36:28 +01:00
**Why it happens:**
2026-03-26 16:54:31 +01:00
Go's `os.UserConfigDir()` already implements the XDG lookup (`$XDG_CONFIG_HOME``~/.config` on Linux, `~/Library/Application Support` on macOS). Most developers reach for `os.UserHomeDir()` + hardcoded `".config"` string because it is the first function they find in the stdlib.
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Consequences:**
- User's config is silently ignored when `$XDG_CONFIG_HOME` is non-default
- Confusing behavior difference between development machines and CI
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Prevention:**
Use `os.UserConfigDir()` (stdlib, Go 1.13+) for the platform-appropriate config directory. This correctly respects `$XDG_CONFIG_HOME` on Linux and `APPDATA` on Windows (if ever relevant). The discovery order should be:
```go
func configSearchPaths() []string {
var paths []string
// 1. Current directory (highest precedence)
paths = append(paths, "netsynth.toml")
// 2. XDG/platform config dir
if cfgDir, err := os.UserConfigDir(); err == nil {
paths = append(paths, filepath.Join(cfgDir, "netsynth", "config.toml"))
}
return paths
}
```
If `--config` flag is set, use that path exclusively and return a clear error if the file is absent (do not fall through to auto-discovery when explicit path is provided).
**Detection:**
- Config not loaded on systems where `$XDG_CONFIG_HOME=/custom/path`
- Silent "no config found" behavior when a config clearly exists at the XDG path
2026-03-24 22:36:28 +01:00
**Phase to address:**
2026-03-26 16:54:31 +01:00
Config loading phase. Implement the path discovery with `os.UserConfigDir()` from the start. Fix before the feature ships.
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
### Pitfall A8: Explicit --config Flag Does Not Error on Missing File
2026-03-24 22:36:28 +01:00
**What goes wrong:**
2026-03-26 16:54:31 +01:00
When `--config path/to/file.toml` is specified, the user expects an error if the file does not exist. If the config loader falls through to auto-discovery when the explicit path is missing, or silently uses defaults, the user has no way to detect a typo in their `--config` argument. They run a session, get "unexpected" default sounds, and have no indication their config was never loaded.
2026-03-24 22:36:28 +01:00
**Why it happens:**
2026-03-26 16:54:31 +01:00
Auto-discovery logic is convenient to write as "try these paths, use first found." Developers reuse this logic even for the `--config` code path.
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Prevention:**
Separate the two code paths:
- `--config` specified → `os.Open(flagValue)`, return error immediately if `errors.Is(err, os.ErrNotExist)`
- No flag → `configSearchPaths()` loop, silently skip missing files, proceed with defaults if none found
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Detection:**
- `--config missing.toml` runs without error, uses defaults
- User misses that their config file path has a typo
2026-03-24 22:36:28 +01:00
**Phase to address:**
2026-03-26 16:54:31 +01:00
Config loading phase. A one-line `if flagValue != "" { /* require it */ }` branch is sufficient.
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
### Pitfall A9: User Rules That Target the Same Port as Built-in Rules Are Silently Shadowed
2026-03-24 22:36:28 +01:00
**What goes wrong:**
2026-03-26 16:54:31 +01:00
A user writes a rule for `{Protocol: "tcp", DstPort: 443, Class: "my-api"}` intending to reclassify their internal HTTPS traffic. If built-in `ClassHTTPS` still appears before the user rule in the merged slice, the built-in rule wins every time. The user's intent ("I want my port-443 traffic to sound different") is silently defeated.
2026-03-24 22:36:28 +01:00
**Why it happens:**
2026-03-26 16:54:31 +01:00
First-match-wins with `SpecificRules + userRules + CatchAllRules` means built-in specific rules still precede user rules. A user trying to *override* a built-in mapping must replace it, not add after it.
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Consequences:**
- User's specific rule is unreachable if a built-in rule covers the same port/protocol
- No error, no warning
- Functionally the same as Pitfall A5 but for specific (non-catch-all) built-in rules
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**Prevention:**
Two viable strategies:
1. **User rules first:** `userRules + specificDefaultRules + catchAllRules`. User rules always take precedence. Built-ins serve as fallback. This is the simplest design and most aligned with user expectations ("I configure what I care about; defaults handle everything else").
2. **Conflict detection:** After merging, scan for duplicate `(protocol, dstPort)` pairs and emit a warning: `"User rule for tcp:443 shadows built-in HTTPS rule. Did you mean to replace it?"`.
Option 1 is recommended for simplicity. Document it clearly: "User-defined rules are evaluated before built-in rules."
**Detection:**
- User-defined rule for a built-in port (80, 443, 22, etc.) never activates
- Verbose output shows built-in class instead of user class for the expected traffic
2026-03-24 22:36:28 +01:00
**Phase to address:**
2026-03-26 16:54:31 +01:00
User-defined rules phase, merge strategy design. Address at the same time as Pitfall A5.
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
### Pitfall A10: New TrafficClass Strings From Config Are Not Validated — Empty String or Whitespace Is a Valid Key
2026-03-24 22:36:28 +01:00
2026-03-26 16:54:31 +01:00
**What goes wrong:**
A user writes:
```toml
[[rules]]
protocol = "tcp"
dst_port = 9200
class = ""
```
The string `""` decodes without error. It is a valid Go map key. It gets inserted into the `WindowSnapshot.Counts` map and the aggregator increments `Counts[""]`. The bank has no layer for `""`. The behavior is undefined — silent or panic depending on nil-guard presence.
Similarly, `class = " elasticsearch "` (padded spaces) decodes to a string with leading/trailing whitespace that does not match any configured sound entry (because the config sound entry key is `"elasticsearch"` without spaces).
**Prevention:**
Validate all `Class` string values from user rules in the `validate()` step:
```go
if strings.TrimSpace(rule.Class) == "" {
return fmt.Errorf("rule %d: class name must not be empty", i)
}
rule.Class = strings.TrimSpace(rule.Class)
```
Also validate that class names do not collide with reserved built-in class names (`"ICMP"`, `"DNS"`, etc.) unless the user is explicitly overriding a built-in sound (which is a distinct feature — it should be opt-in, not accidental).
**Phase to address:**
Config validation step.
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
## v1.0 Pitfalls (Retained for Reference)
The following pitfalls from the initial MVP research remain valid. They are retained in condensed form for reference.
---
### Pitfall B1: Using `google/gopacket` Instead of the Active Community Fork
**What goes wrong:** Import of the unmaintained original — 270 open issues, Go compat degrades.
**Prevention:** Import `github.com/gopacket/gopacket` (v1.5.0, requires Go 1.24+).
**Phase:** Phase 1 — set correct import path from day one.
---
### Pitfall B2: CGo Destroys the "Single Binary" Promise
**What goes wrong:** `gopacket/pcap` (CGo + libpcap) produces a dynamically-linked binary that fails on machines without `libpcap.so`.
**Prevention:** Use `packetcap/go-pcap` (pure Go capture, already the chosen stack). Verify with `ldd ./netsynth`.
**Phase:** Phase 1 — foundational architecture decision.
---
### Pitfall B3: `CAP_NET_RAW` + Binary Location = Silent Failure on Linux
**What goes wrong:** `setcap` is silently ignored on `nosuid` filesystems. Binary appears broken from home directories.
**Prevention:** Install to `/usr/local/bin`; document two run modes; emit clear privilege error.
**Phase:** Phase 1 + CLI UX.
---
### Pitfall B4: Packet Buffer Overflow Under Moderate Traffic Load
**What goes wrong:** Default capture buffer fills faster than the classifier consumes it; silent packet drops misrepresent traffic.
**Prevention:** Large capture buffer (32 MB); buffered channel between capture and classify goroutines.
**Phase:** Phase 1/2 (capture pipeline architecture).
---
### Pitfall B5: ZeroCopy Packet Data Use-After-Free
**What goes wrong:** `ZeroCopyReadPacketData()` invalidates previous slice on each call; silent data corruption in concurrent code.
**Prevention:** Use `ReadPacketData()` (copying API) unless profiling proves allocation bottleneck.
**Phase:** Phase 1 (capture/decode).
---
### Pitfall B6: MP3 Output Is Corrupt Due to LAME Initialization Order
**What goes wrong:** Skipping `InitParams()` or setting parameters out of order produces unplayable MP3.
**Prevention:** Always call `InitParams()` before writing frames; smoke test with `ffprobe`.
**Phase:** Audio synthesis / encoding phase.
---
### Pitfall B7: PCM Sample Overflow Produces Wrap-Around Distortion
**What goes wrong:** Summing `int16` layers overflows and wraps (32767 + 100 = -32667), producing buzzing distortion.
**Prevention:** Synthesize in `float64 [-1.0, 1.0]`; clamp before int16 cast. Already implemented in `synth/mixer.go`.
**Phase:** Audio synthesis (already addressed in v1.0).
---
### Pitfall B8: Tone-per-Protocol Mapping Produces Perceptual Chaos
**What goes wrong:** Frequencies too close together mask each other; output is undifferentiated buzz.
**Prevention:** Space protocols across register bands; use harmonic/musical intervals. Already addressed in v1.0.
**Phase:** Audio mapping (already addressed in v1.0).
---
## v1.1 Phase-Specific Warnings
| Phase Topic | Likely Pitfall | Mitigation |
|-------------|---------------|------------|
| TOML struct design | A1: zero-value overwrites defaults | Use pointer fields for all optional overrides |
| Config strict decode | A2: typos silently ignored | Use `md.Undecoded()` as strict mode check |
| Waveform implementation | A3: naive waveform aliases | Use additive synthesis (bandlimited harmonic series) — compatible with existing `[]HarmonicDef` API |
| Waveform string input | A4: case/abbreviation mismatches | Normalize + validate with clear error listing accepted values |
| Rule merge ordering (catch-alls) | A5: user rules after catch-alls are unreachable | Split `DefaultRules` into `SpecificRules` + `CatchAllRules`; user rules go in between |
| Bank construction | A6: custom class has no synth layer | Derive full class set from merged rule slice; pass to bank constructor |
| Config discovery | A7: XDG ignored, `~/.config` hardcoded | Use `os.UserConfigDir()` not `os.UserHomeDir() + "/.config"` |
| --config flag path | A8: missing explicit path silently ignored | Two distinct code paths: flag path (require) vs auto-discovery (skip-missing) |
| Rule merge ordering (specific built-ins) | A9: user rule shadowed by built-in for same port | User rules first in merged slice (`userRules + specificDefaults + catchAlls`) |
| Class name validation | A10: empty/whitespace class name is valid Go string | Validate and trim all class strings in `validate()` |
---
## Integration Gotchas (v1.1 Additions)
2026-03-24 22:36:28 +01:00
| Integration | Common Mistake | Correct Approach |
|-------------|----------------|------------------|
2026-03-26 16:54:31 +01:00
| Config → Bank wire-up | Pass `classify.AllClasses()` to bank; custom classes missing | Derive layer set from `Classifier.ActiveClasses()` — all classes reachable via the effective rule set |
| Waveform → FreqConfig | Add `WaveformType string` to `FreqConfig`; forget to generate harmonics at bank init | Generate `[]HarmonicDef` from waveform+freq at bank/layer construction time, not at sample render time |
| User rules → Classifier | Replace `DefaultRules` var directly; breaks tests relying on it | Keep `DefaultRules` immutable; construct `mergedRules` for runtime use |
| Config file absent | Return error if no config found | Return nil (no config = all defaults). Only error on explicit `--config` path that is missing |
| Sound overrides for built-in class | User sets freq for "HTTPS" — must hit `ClassHTTPS` layer | Match config sound keys case-insensitively against `TrafficClass` string values; map `"HTTPS"``classify.ClassHTTPS` |
2026-03-24 22:36:28 +01:00
---
## Sources
2026-03-26 16:54:31 +01:00
- [BurntSushi/toml pkg.go.dev](https://pkg.go.dev/github.com/BurntSushi/toml) — `Undecoded()` strict mode, pointer field behavior, `MetaData` API
- [BurntSushi/toml issue #47: Unmarshal with default values](https://github.com/BurntSushi/toml/issues/47) — confirms default-overwrite behavior
- [pelletier/go-toml issue #252: Unmarshal overrides origin values if key is omitted](https://github.com/pelletier/go-toml/issues/252) — confirms same behavior in v1; v2 partially resolves
- [pelletier/go-toml v2 pkg.go.dev](https://pkg.go.dev/github.com/pelletier/go-toml/v2) — strict decoder mode documentation
- [golang/go issue #29960: os: add UserConfigDir](https://github.com/golang/go/issues/29960) — rationale for `os.UserConfigDir()` (XDG-aware)
- [WolfSound: Basic Waveforms in Synthesis](https://thewolfsound.com/sine-saw-square-triangle-pulse-basic-waveforms-in-synthesis/) — aliasing and harmonic series for square/saw/triangle
- [CCRMA: Alias-Free Digital Synthesis of Classic Analog Waveforms](https://ccrma.stanford.edu/~stilti/papers/blit.pdf) — bandlimited synthesis theory
- [McGill Bandlimited Synthesis of Classic Waveforms](https://www.music.mcgill.ca/~gary/307/week5/bandlimited.html) — truncated harmonic series approach
- [Teensy Forum: triangle & sawtooth oscillators aliasing](https://forum.pjrc.com/threads/61269-triangle-amp-sawtooth-oscillators-how-to-deal-with-aliasing) — practical aliasing impact at different frequencies
- [adrg/xdg package](https://github.com/adrg/xdg) — XDG Base Directory Specification Go implementation (reference; stdlib `os.UserConfigDir()` is sufficient for NetSynth's needs)
2026-03-24 22:36:28 +01:00
---
2026-03-26 16:54:31 +01:00
*Pitfalls research for: NetSynth v1.1 — TOML config, waveform types, user-defined rules*
*Updated: 2026-03-26*