refactor: consolidate first-party packages and trim dead code#125
Merged
Conversation
Removes verified unused symbols and their paired tests across six packages:
- processor: orphaned ContentType enum, detectContentType(), nine lpContent* consts;
adaptive_noise.go file; Stage type; LevellingCompressorThresholdSpeechOffsetDB export;
BaseMeasurements struct; SpectralAccumulator.Count()
- report: realTimeFactor(), formatMetricPeak(); formatMetricSpectral digitalSilence branch;
spectralSilenceValue const
- ui: AnalysisCompleteMsg.Measurements and .Config fields; TargetLUFS export
- audio: Metadata.SampleFmt, .ChLayout, .BitDepth fields and their population
- cli: ColorRedDim and ColorLightGrey colour vars
All removals are dead code with no production impact. Build, test, and linters pass clean.
Signed-off-by: Martin Wimpress <code@wimpress.io>
Signed-off-by: Martin Wimpress <code@wimpress.io>
…al constants Signed-off-by: Martin Wimpress <code@wimpress.io>
…yword param Folded builders, constructors, sanitizers. Signed-off-by: Martin Wimpress <code@wimpress.io>
Signed-off-by: Martin Wimpress <code@wimpress.io>
Signed-off-by: Martin Wimpress <code@wimpress.io>
- Add poolEnv struct to bundle shared ctx, program, files, config, logging, and job count across both pool types - Add analysisSlot struct to collapse results/metas/errs data clump into a single indexed slice - Reduce runWorkerPool signature from 9 params to 4; runAnalysisPool from 10 to 3 - Extract emitProcessingReport function from the 150-line worker goroutine body, mirroring emitAnalysisReport for symmetry - Add processingRenderScheduler struct to thread spectrogram scheduling, matching analysisRenderScheduler pattern - Replace mutable global analysisPoolAnalyse package var with injected analysisPoolDeps struct, unifying the DI seam - Update all call sites in main.go and both pool test files; add defaultAnalysisPoolDeps factory TUI message protocol, input-order output, and ctx cancellation behaviour unchanged. Signed-off-by: Martin Wimpress <code@wimpress.io>
Extract an 8-parameter `float64` list in `calculateSpeechGateThreshold` and `calculateSpeechGateThresholdLegacy` into two small structs (`noiseContext` and `speechContext`) to eliminate the correctness hazard of same-typed parameter neighbours. Update call site in `tuneSpeechGate`. No behaviour change. Signed-off-by: Martin Wimpress <code@wimpress.io>
…Artefacts
- Move spectrogram derivation, markdown report, JSON record, and sidecar
writes into new emitReportArtefacts helper used by both pool and
analysis-only paths
- Bundle Pass 3/4 timing parameters into processingTimings struct,
reducing emitProcessingReport argument count from 11 to 4
- Replace hard-coded channel/print error paths with per-mode callback
for testability and decoupling
No behaviour change; artefacts and warnings stay byte-identical.
Signed-off-by: Martin Wimpress <code@wimpress.io>
- Extract LimiterDiagnostics struct (6 fields) and embed in NormalisationResult; populate in one assignment - Extract normProgressEmitter with named methods (measuring, measuringDoneNormalisingStart, normalisingDone) - Extract buildNormalisationResult to assemble the 22-field NormalisationResult - Extract loudnormTPTargets to pre-compute TP values before filter spec construction - Extract newLoudnormApplicationFrameLoop to build FrameLoopConfig in one place Reduces applyNormalisationWithDeps orchestration from ~150 to ~55 lines; no behaviour change. Signed-off-by: Martin Wimpress <code@wimpress.io>
…ssor scaffolding Collapse duplicated boilerplate across noiseProfileRecord, speechProfileRecord, and normalisationRecord: single src pointer, nil-checked accessor, and MarshalJSON calling sanitisedSourceMap with per-type key transforms. Introduce generic recordWrapper[T any] with shared source() accessor and marshalWithTransform(transform func) method. Each concrete type embeds the wrapper and passes its key transform as a closure to MarshalJSON. JSON output stays byte-identical. Signed-off-by: Martin Wimpress <code@wimpress.io>
…plication Four renderer functions (renderLoudness, renderDynamics, renderSpectral, renderRegionSamples) hand-wrote identical nil-check closures. Introduce a generic stageGetter[T any] factory that centralises the contract: nil stage yields nil getter; formatCell renders it as a placeholder. Replaces 75 lines of near-identical closures with four factory calls and one reusable helper. Rendered output byte-identical. Signed-off-by: Martin Wimpress <code@wimpress.io>
Update test fixtures in statusboxes_test.go and summary_test.go to wrap the six limiter diagnostic fields in the embedded LimiterDiagnostics literal, following the earlier extraction and anonymous embedding in NormalisationResult. Tests pass unchanged; no behaviour change. Signed-off-by: Martin Wimpress <code@wimpress.io>
- adaptive_speech_gate.go: use min/max for threshold and ratio bounds - analyser.go: use min/max for floor/ceiling clamps (preserve precedence where bounds invert) - encoder.go: use min/max for bitrate clamp - ui/views.go: use min/max for progress percentage clamp Reduces duplication and improves consistency with existing min/max usage in the same files. Signed-off-by: Martin Wimpress <code@wimpress.io>
…r caching - processor: seek demuxer before region re-measure to skip pre-region decode spans; drop per-key string allocation in metadata dict read (2→1 alloc, 24→8 B per call) - ui: cache title wordmark via sync.OnceValue (was rebuilt every 60fps frame); precompute VU colour ramp once; drop string round-trip in peak marker colour - cli: cache rendered status boxes per file, keyed on (AdaptedSummary, passHeight) All output byte-identical to baseline. ~2% faster. Signed-off-by: Martin Wimpress <code@wimpress.io>
Implements 18 maintainability improvements from CODE-REVIEW.md across
all first-party packages, reducing noise and strengthening coupling:
Dead code and unused parameters:
- Delete dead BuildFilterSpec method overload and repoint test
- Drop unused params from tuneLevellingCompressor, createOutputEncoder
- Replace constant Encoder.streamIdx field with literal
- Lowercase four internal cli style variables
Local refactors:
- Nest implicit fallback branches in outputTruePeak
- Collapse formatMetricSpectral pass-through
- Extract writeSidecarFile boilerplate
- Consolidate four hand-rolled linear-ramp scores onto linearScore
Cross-function consolidation:
- Capture hasTTY() once in analysis-only branch
- Fold real-time-factor into progressHandler.timings
- Drop dead parameter from activeRow (15 call sites)
- Inline renderHeader passthrough
- Extract pendingBox and doneBoxOptionalBeforeAfter helpers
Decision-gated changes:
- Annotate unrendered metric definitions (keys preserved)
- Drop help default column
- Consume DestinationExistsError at production caller
Removes 48 lines, adds 157; 22 files changed. Tests pass; lint clean.
Signed-off-by: Martin Wimpress <code@wimpress.io>
Contributor
There was a problem hiding this comment.
1 issue found across 52 files
Confidence score: 3/5
- In
internal/processor/adaptive_speech_gate.go, the clamp rewrite changes behavior whennoiseFloorLimit > speechRMSLimit, which can push the threshold higher than before and mis-gate tight-separation speech (e.g., dropping valid speech or over-suppressing low-level audio); add a guard or explicit ordering for crossed bounds and verify with a regression test for this edge case before merging.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Replace min/max one-liner with hand-rolled if/else at line 359 to preserve original behaviour when noiseFloorLimit > speechRMSLimit. The refactor (eaf4147) changed precedence in this edge case, raising gate threshold and risking over-gating of low-level speech. Restores exact original logic without functional change on normal cases. Signed-off-by: Martin Wimpress <code@wimpress.io>
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Extensive refactor across core packages with no behavior change; the large diff warrants human review for correctness and potential side effects.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Maintainability sweep across cmd, processor, report, ui, cli, and audio.
No behaviour change: audio output, JSON records, and Markdown reports stay
byte-identical; one perf commit lands a ~2% speed-up. Build, tests, and
linters pass clean.
Dead code:
detectContentType, adaptive_noise.go, Stage type, BaseMeasurements,
unused Metadata fields, orphaned report/ui/cli vars) and paired tests
Compressor, createOutputEncoder) and the dead BuildFilterSpec overload
Consolidation:
accessors); extract formatByRule, renderFilledBar, linearScore, formatByRule
speech-gate noiseContext/speechContext, processingTimings
buildNormalisationResult; cut applyNormalisation orchestration ~150→55 lines
with min/max builtins
Performance: