Skip to content

refactor: consolidate first-party packages and trim dead code#125

Merged
flexiondotorg merged 17 commits into
mainfrom
refactor
Jun 14, 2026
Merged

refactor: consolidate first-party packages and trim dead code#125
flexiondotorg merged 17 commits into
mainfrom
refactor

Conversation

@flexiondotorg

Copy link
Copy Markdown
Contributor

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:

  • Remove unused symbols across six packages (ContentType enum,
    detectContentType, adaptive_noise.go, Stage type, BaseMeasurements,
    unused Metadata fields, orphaned report/ui/cli vars) and paired tests
  • Drop dead parameters (StyledHelpPrinter options, activeRow, tuneLevelling-
    Compressor, createOutputEncoder) and the dead BuildFilterSpec overload

Consolidation:

  • Collapse Rumble/Bandlimit into one BiquadFilterConfig
  • Extract generics: stageGetter (report closures), recordWrapper (JSON
    accessors); extract formatByRule, renderFilledBar, linearScore, formatByRule
  • Bundle data clumps into structs: poolEnv, analysisSlot, LimiterDiagnostics,
    speech-gate noiseContext/speechContext, processingTimings
  • Extract shared spines: emitReportArtefacts, emitProcessingReport,
    buildNormalisationResult; cut applyNormalisation orchestration ~150→55 lines
  • Inline colour aliases to canonical constants; replace hand-rolled clamps
    with min/max builtins

Performance:

  • Seek demuxer before region re-measure; drop per-key alloc in metadata reads
  • Cache TUI wordmark, VU colour ramp, and rendered status boxes

  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>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 52 files

Confidence score: 3/5

  • In internal/processor/adaptive_speech_gate.go, the clamp rewrite changes behavior when noiseFloorLimit > 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

Comment thread internal/processor/adaptive_speech_gate.go Outdated
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>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@flexiondotorg flexiondotorg merged commit efc333c into main Jun 14, 2026
16 checks passed
@flexiondotorg flexiondotorg deleted the refactor branch June 14, 2026 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant