Skip to content

feat(core): signal rhythm is rate-independent, refuses grids in the tracker, follows chords - #5644

Merged
xuanruli merged 4 commits into
mainfrom
feat/signal-rhythm-quality
Oct 15, 2026
Merged

xuanruli merged 4 commits into
mainfrom
feat/signal-rhythm-quality

Conversation

@xuanruli

@xuanruli xuanruli commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

The signal rung is what most users get: no Python, no macOS 27, no API. This PR makes it better and measures every change against a fixed benchmark. Everything is in packages/core/src; no CLI or provider code changes. This covers the floor recalibration and the sections work; coarse section role labels are not in this PR (see rejected).

  1. Same answer at any decode rate (fix). The tracker resamples every channel to 22.05 kHz (new audioResample.ts, a windowed-sinc resampler, pure TS) and averages the channels' band power. The CLI decodes at the file's native rate, Studio at the AudioContext rate; both now get the same beats. Mono and the same sound on two channels give identical results.
  2. One confidence floor, in the tracker (fix). The floor that refuses a grid ("no steady beat") moves out of analyzeMusicFromSignal into rhythm(). hyperframes beats, hyperframes audio and grep_audio now share it. It is recalibrated from 0.2 (wrapper) and 0.05 (tracker) to 0.15, just above every synthetic negative.
  3. Downbeats follow chord changes (feat). The tracker now also computes a 10 fps chroma (from fft.ts, 4096-point). The bar phase is chosen from three cues: the low-end hit, the chord change across one beat, and the chord change across two beats. Before, it used the low end only.
  4. Sections hear harmony (feat). Foote novelty runs over timbre (band energies) and chroma, weighted equally.
  5. Tuned on GTZAN dev only: tempo prior width 1 → 0.8 octave; local tempo held within ±10% of the global tempo (was −30%/+40%); beats placed 15 ms after the onset-curve peak. The tracker was 16 ms early against annotations, median over 18.7k matched beats.

Benchmark

  • GTZAN, the primary set: 998 clips. One clip has no annotation and one is undecodable.
    • Split by genre: clips 00000–00049 of each genre are dev (tuning only), 00050–00099 are test.
    • Beat and downbeat annotations from CPJKU/beat_this_annotations.
  • R1: 22 agent-synthesized tracks with exact script grids, scored in audible spans (feat(cli): beats tracks the music with the TypeScript beat tracker, without Chrome #5634's table).
  • S: 200 synthetic drum grids over real beds, with swing, tempo steps and drift.
  • Consensus: 21 real tracks where madmom and Beat This final0 agree. Beat This final0 serves as pseudo-truth.
  • Negatives: 38 synthetic clips with no steady beat, plus a held-out 28 from another seed.
    • Kinds: speech, scattered effects, pads, swelling noise, and ambient beds from the corpus.

Metrics come from mir_eval 0.8.2: F at ±70 ms, CMLt, AMLt, downbeat F; mir_eval trims the first 5 s. Tempo accuracy uses ±4%: Acc1 is the right tempo, Acc2 also accepts ×2, ×½, ×3 and ×⅓. Beat This small0 is the reference ceiling (the beat-this rung).

What users get: beats after the floor (hyperframes beats, Studio, and now audio)

"before" is main at 1544b51: beats with the wrapper's 0.2 floor. A refused clip scores 0.

set F CMLt AMLt Acc1 Acc2 downbeat F no grid
GTZAN dev before 0.528 0.454 0.553 0.500 0.600 0.312 36.6%
after 0.593 0.511 0.621 0.570 0.682 0.394 28.2%
Beat This small0 0.845 0.731 0.855 0.768 0.918 0.702 0%
GTZAN test before 0.597 0.512 0.626 0.566 0.681 0.342 28.3%
after 0.654 0.554 0.696 0.622 0.773 0.428 19.9%
Beat This small0 0.925 0.847 0.929 0.853 0.952 0.843 0%
R1 (22) before 0.887 0.872 0.872 0.909 0.909 0.934 2 of 22
after 0.900 0.891 0.891 0.909 0.909 0.908 2 of 22
Beat This small0 0.995 0.983 0.983 1.000 1.000 0.998 0
S (200) before 0.531 0.412 0.539 0.435 0.589 0.381 24%
after 0.593 0.473 0.587 0.532 0.629 0.316 17.5%
Beat This small0 0.584 0.392 0.515 0.395 0.516 0.371 3%
consensus (21) before 0.892 0.811 0.899 0.857 0.952 1 of 21
after 0.910 0.867 0.912 0.905 0.952 1 of 21

R1 does not regress: 0.887 → 0.900. The two refused R1 tracks (297b03, 3333be) are the same two #5634 refused; refusing them is still right, because without the floor their tempo is halved.

The "before" downbeat column comes from the tracker, which had no 0.2 floor, so on that column "before" is scored on more grids than "after".

Tracker quality without the floor

This isolates the tracker from the refusal decision. "before" is the tracker on main with its own 0.05 floor, which is what hyperframes audio used to return. "after" uses floor 0.

set F CMLt AMLt Acc1 Acc2 downbeat F
GTZAN dev before 0.700 0.580 0.743 0.664 0.836 0.312
22.05 kHz only (commit 1) 0.735 0.600 0.779 0.686 0.868 0.307
after 0.743 0.612 0.792 0.708 0.886 0.483
GTZAN test before 0.739 0.614 0.775 0.697 0.869 0.342
22.05 kHz only (commit 1) 0.770 0.634 0.813 0.709 0.902 0.343
after 0.776 0.638 0.825 0.725 0.924 0.504
librosa 0.11 beat_track (DSP reference) 0.776 0.633 0.822 0.717 0.890
R1 before → after 0.939 → 0.989 0.872 → 0.980 0.951 → 0.980 0.909 → 1.000 1.000 → 1.000 0.934 → 0.999
S before → after 0.680 → 0.696 0.507 → 0.522 0.693 → 0.689 0.484 → 0.565 0.694 → 0.718 0.381 → 0.364
consensus before → after 0.892 → 0.947 0.811 → 0.912 0.899 → 0.958 0.857 → 0.952 0.952 → 1.000

Per GTZAN genre, F before → after (test and dev together; Beat This in brackets):

blues 0.718 → 0.729 (0.811) classical 0.318 → 0.534 (0.650) country 0.835 → 0.834 (0.919) disco 0.918 → 0.929 (0.969)
hiphop 0.773 → 0.798 (0.976) jazz 0.531 → 0.606 (0.856) metal 0.718 → 0.753 (0.887) pop 0.748 → 0.768 (0.946)
reggae 0.840 → 0.846 (0.906) rock 0.790 → 0.798 (0.929)

Downbeat F on GTZAN, all clips: 0.327 → 0.494. The biggest gains are disco 0.24 → 0.67, reggae 0.27 → 0.61 and rock 0.40 → 0.62.

Refusing a grid

negatives given a grid (38 + held-out 28) GTZAN dev clips refused real corpus (75 tracks)
before, beats (0.2) 0/38, 0/28 36.6% 48 grids: 42 agree with Beat This, 3 disagree
before, audio (0.05) 22/38, 15/28 9.8% 71 grids: 42 agree, 26 disagree
after, both (0.15) 0/38, 0/28 28.2% 52 grids: 43 agree, 7 disagree
Beat This small0 (no floor) 22/38 0%

The highest-scoring negative reaches 0.122, so 0.15 leaves a margin. Lowering the floor further mostly adds wrong grids; at 0.15 the dev set still keeps 42 grids with F < 0.5. Because audio used to give a grid at 0.05, it now refuses more often. That is the intended effect of fix 2.

Why the floor lowers GTZAN beat F from 0.776 to 0.654

The tracker itself is unchanged by the floor. The drop is entirely refused clips, which score 0.

clips refused at 0.15 of the refused: Beat This F ≥ 0.8 of the refused: our unfloored F ≥ 0.8 / F < 0.5 refused clips by genre
dev 500 141 (28%) 52 (Beat This mean F 0.659) 30 / 61 (mean F 0.533) classical 46, jazz 27, metal 26, blues 13, rock 10
test 498 99 (20%) 62 (Beat This mean F 0.819) 31 / 37 (mean F 0.614) classical 42, jazz 18, metal 10, rock 9, blues 6
  • On the test half, the refused 20% averages F 0.614 when not refused. That accounts for the drop: 0.20 × 0.614 ≈ 0.12. On the clips that keep a grid, F is 0.816.
  • Beat This would have tracked 62 of the 99 refused test clips well. On those clips the signal tracker gets only 31 right, and gets 37 wrong (F < 0.5).
  • The cost of refusing. A grid is refused when our confidence cannot separate a real beat from the negatives' false ones. Mostly those are rubato classical and jazz, and dense metal. Of the clips refused, the signal tracker itself was right on about a third.
  • Trading coverage back in: a floor of 0.10 keeps 81% of dev (mean F 0.652), but gives a grid to 3 of 38 negatives. Floor 0 keeps everything (F 0.743), but gives a grid to all 38.

Sections: boundary F at ±3 s (F3) and ±0.5 s (F0.5), before → after

set before after
GTZAN dev joins: 60 cases, 3–5 excerpts of one genre (10–22 s each) end to end F3 0.775, F0.5 0.629 (1.14 found per true) F3 0.819, F0.5 0.663 (1.18)
GTZAN test joins: 60 cases, built the same way from test clips F3 0.847, F0.5 0.728 (1.08) F3 0.858, F0.5 0.780 (1.14)
#5632's corpus joins: 40 cases F3 0.741, F0.5 0.648 (1.32) F3 0.742, F0.5 0.692 (1.27)

Beat This has no sections rung, so this table has no reference column. These are joins of different recordings, which is an easier task than finding sections within one song.

Speed (bun, one process, analysis only)

decode before after
22.05 kHz mono (100 GTZAN clips) 1.71 ms per second of audio 2.36 ms/s (1.38×)
native rate, 44.1/48 kHz (20 real tracks) 5.41 ms/s 6.03 ms/s (1.11×)
sections, extra 1.9 ms/s 2.6 ms/s

Targets, honestly

  • Beat F ≥ 95% of Beat This: not reached, and not realistic for this design.
    • Unfloored, the tracker reaches 0.776 on GTZAN test, 84% of Beat This's 0.925. After the floor, users get 0.654 (71%).
    • librosa's tracker uses the same Ellis DP and gets 0.776 on the same split.
    • Even with the annotated tempo given to it, our DP reaches only F 0.80 on dev (Beat This: 0.845). Phase and drift on expressive music (jazz, classical, metal) are the limit, so further tuning will not close the gap.
  • Acc2 ≥ 95%: not reached. It is 0.924 on test unfloored (Beat This: 0.952). The remaining errors are mostly 3:2 and "other" ratios on classical music.
  • Downbeats: clear gain on produced music. GTZAN test 0.342 → 0.504, about 60% of Beat This's 0.843.

Tried and rejected (GTZAN dev F unless noted; reference 0.743 at the end, 0.735 after commit 1)

change result why rejected
8 or 12 bands/octave instead of 5 0.741 / 0.738 noise-level, and R1 dropped to 0.949
difference lag of 2 frames 0.738 (later 0.741) no gain
SuperFlux max-filter across frequency 0.712; R1 0.876 worse
log compression 1e7 / 1e11 0.743 / 0.733 R1 0.923 / 0.940
local-mean radius 0.2 s / 0.8 s 0.736 / 0.735 no gain
low band added to the DP observation (weight 0.5 / 1) 0.698 / 0.666 worse
PLP (Grosche–Müller) blended into the observation (0.5 / 1) 0.736 / 0.627 worse
DP tightness 200 / 300 0.750 / 0.747 S F 0.681 / 0.677 vs 0.696
tempo prior centre 105 / 135 (with widths 0.6–1) ≤ 0.742 no gain over 120
choosing the metrical level among {1, 2, ½, 3/2, 2/3}× by ACF harmonics, onset lift and prior 0.750 Acc1 0.708 → 0.654
the same choice as a 12-weight model fitted on dev dev 0.761, test 0.794 a fitted model for +0.02; kept out
confidence: logistic over clarity, lift, cv, ACF dev 0.649 vs 0.61 at the floor fitted on 38 synthetic negatives, too fragile
confidence: other hand-made formulas (ACF of smoothed curve, interval CV) ±0.01 no gain
harmonic cues weighted 2× over half a bar downbeat 0.493 vs 0.483 drum-only grids: 0.55 vs 0.69, see below
cues weighted by their own decisiveness, or left unnormalized downbeat 0.45–0.48 no clear win
section boundaries snapped to downbeats instead of beats F0.5 0.655 → 0.442 downbeats are right only half the time
coarse section labels (intro, body, climax, outro) not built no annotations to measure them
meter detection beyond the 3/4 handicap not built 469 of 494 dev clips are in 4

Known costs

"Fails without the fix" mutations (all run)

mutation failing test
bandFrames uses signal.channels without resampling gives the same beats at any sample rate and for mono or the same sound on two channels: expected 0.25 to be less than or equal to 0.011
floor back to 0.05 refuses an ambient pad in the tracker itself, so every caller of the signal tracker agrees, and reports no beats and no tempo for audio without a steady pulse
downbeats from the low-band cue only (chord cues removed) puts the bar where the chords change when every beat hits the same: recall 0 < 0.9
resampler without the low-pass (scale = CUTOFF, so it folds) removes what the lower rate cannot hold instead of folding it down: RMS 0.706, expected < 0.01

Runs here:

  • core src/audio* and src/beats: 440 passed.
  • cli src/audio, src/commands/beats.test.ts and src/commands/audio: 47 passed.
  • studio src/hooks and src/utils/beat*: 1567 passed.
  • core, studio, cli and scripts typecheck: clean. oxlint and oxfmt: clean. fallow audit vs origin/main: no issues.
  • hyperframes audio --only beats,downbeats,bpm,sections runs end to end from the built CLI.

GTZAN (not committed)

  • Audio: https://huggingface.co/datasets/marsyas/gtzan/resolve/main/data/genres.tar.gz.
    • This is the mirror commonly used since mirdata's own GTZAN audio source (http://opihi.cs.uvic.ca/sound/genres.tar.gz, in mirdata ≤ 0.3.8) went offline. Current mirdata ships only gtzan_mini.
    • sha256 b28cc067ff6199bd826f5d1a6931458586d64acae7f44b2e882b7a97af057531, md5 4c13f740bb9d223a1d74d4a7f78ca336.
    • The md5 differs from mirdata's 5b3d6dddb579ab49814ab86dba69e7c7: this tarball holds .wav instead of .au, plus a .DS_Store.
  • Annotations: CPJKU/beat_this_annotations at fd9fcc0896cb78730bae735102c8753ef3a3badd, folder gtzan, MIT.
  • License: GTZAN is research-only. No audio or derived audio is in this PR or any repo; only annotation-derived scores appear here. The benchmark scripts and outputs stay in the session scratchpad.

Not exercised

  • No Studio walk. I did not open Studio, drop a track or watch the beat lane. The code is pure TS with no new dependency (fft.ts is still the only FFT), and Studio's tests and typecheck pass, but it has not been run in a browser.
  • Long files. Benchmarks are 30–100 s clips. Chroma adds 12 doubles per 100 ms; it is not profiled on hour-long audio.
  • CI and other platforms. Nothing was run on CI runners or other OSes beyond what this PR's checks run.

… grid in the tracker

The tracker resamples every channel to 22.05 kHz and averages their band power, so the
CLI's native-rate decode and Studio's AudioContext decode get the same beats, and a mono
file and the same sound on two channels get identical ones.

The no-steady-beat floor moves from analyzeMusicFromSignal into the tracker, so
hyperframes audio, grep_audio and beats share one floor. It is 0.15: above every one of
66 synthetic ambient, speech, scattered-effect and noise clips (max 0.122), where the
wrapper's 0.2 also refused a third of GTZAN's produced music.
…harmony

The tracker now keeps a 10 fps chroma beside its band energies. Downbeats weigh where the
chords change, over one beat and over two, with the low-end hit, instead of the low end
alone. Sections compare timbre and harmony equally.

Tuned on GTZAN's dev half: the tempo prior narrows to 0.8 octave, the local tempo stays
within 10% of the global one, and beats sit 15 ms after the onset peak, where annotators
put them.
@xuanruli xuanruli changed the title feat(core): signal rhythm reads the same at any rate, refuses grids in the tracker, and follows chords feat(core): signal rhythm is rate-independent, refuses grids in the tracker, follows chords Oct 15, 2026
@xuanruli
xuanruli marked this pull request as ready for review October 15, 2026 06:58

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Independent review of live successor 11792b8b1bf0414300565a35ef2e52f27b972c91 (the extra commit after requested 59b4c9bb changes comments only).

The resampling and channel-power path checks out: 26 focused tests pass. My separate 12s known-time pulse probe returns the same 24 beat times across 16/22.05/44.1/48/96 kHz × mono/duplicate/inverted/right-only channels; actual signalProvider and wrapper results agree. A 1 kHz tone retains RMS .70694, while 15 kHz attenuates to .000349 on 48→22.05 kHz conversion. Removing the low-pass scaling makes the actual alias regression fail (RMS .70636); source restored afterward.

audioRhythm.ts:461 now applies the same .15 floor before beats/downbeats reach provider or wrapper; CLI beats and Studio URL/buffer analysis use the wrapper, and audio uses signalProvider. The ambient/noise/silence tests and the separate silence/tone probes pass. Chord-change/downbeat and returning-section tests also pass.

Quality judgment: the disclosed drum-only downbeat loss (.903→.686) and drums-under-voice loss (.668→.539) are material, not noise. A produced-music improvement does not establish an improvement for those inputs. The existing confidence: "low" remains appropriate for downbeats. I am not independently validating the GTZAN/R1/S scores, split hygiene or threshold generalization: scripts/per-case outputs are absent. Retaining reproducible benchmark scripts and per-case results would make future threshold changes reviewable, especially since positives and negatives use different synthetic distributions.

Cost: fresh-process 120s mono/48k pulse analysis was ~.62s on base and ~.73s on head on this machine; RSS rose ~81 vs ~91 MiB. This is one bounded fixture, not a statistical benchmark or hour-long/Studio responsiveness certification. New resampled per-channel buffers and cached chroma increase peak/retained work; keep the disclosed long-file limit.

Compatibility note: audioRhythm.ts makes previously exported bandFrames/rhythmOf private; audioAnalysis.ts re-exports this module through the public audio-analysis entry. Repo consumers are migrated, but external named imports would break. Preserve aliases or explicitly record that API removal before publishing; this review does not certify unknown external callers.

Current-head Comments passes, but many other hosted checks are still running with no failures reported at this read. I am withholding the approval stamp pending those gates; no confirmed algorithm blocker found in this bounded review. No live Studio, native/model providers, paid API, independent corpus or hour-long run. No merge/release/deploy.

— Magi (desktop-review)

Verdict: COMMENT
Reasoning: Independent rate/channel/anti-alias/floor probes support the implementation; the accuracy tradeoff stays explicit and author benchmarks remain unverified. Current-head CI must complete before approval.

@github-actions

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 165 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (1)

  • seqplainkeys-none-px-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo false, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I reviewed this at 90741e4 and re-checked at 59b4c9b and 11792b8. Both later commits only move or remove comments in audioRhythm.ts and audioSections.ts; no code changed, so everything below holds at this head. No blockers.

What I verified

  • No cancellation between channels. Band power is computed per channel and then averaged (audioRhythm.ts:102). Chroma takes per-channel magnitudes (audioRhythm.ts:140). Neither path mixes the raw samples first. I ran a 20 s click-plus-chords track as mono, L = -R, the same sound on both channels, and at 48 kHz. All four gave the same beats, downbeats, meter and section starts.

  • The resampler low-pass filters before decimating. It uses a Hann-windowed sinc with 8 zero crossings and a cutoff at 0.95 of the lower Nyquist. Measured results:

    • A 10 kHz tone resampled from 44.1k to 16k is about -60 dB (RMS 0.0007). It is filtered out, not folded down to about 6 kHz.
    • From 48k to 22.05k, attenuation is -16 dB at 11.5 kHz, -25 dB at 12 kHz, -44 dB at 13 kHz and -63 dB at 14 kHz and above.
    • The leaky part of that range folds to about 10–10.5 kHz. That is above the top band (9 kHz) and the 2 kHz chroma ceiling, so the analysis never sees it.
  • There is one confidence floor, in one place (STEADY_BEAT_CONFIDENCE, audioRhythm.ts:447), and it is reached only through rhythm(). Every consumer goes through that function:

    • the beats/downbeats capabilities
    • analyzeMusicFromSignal (the beats CLI and Studio)
    • section snapping (audioSections.ts:142)
    • API-answer snapping (cli/.../providers/api.ts:134)

    One side effect: clips that score between 0.05 and 0.15 no longer get signal-beat snapping in the sections and API paths. That is consistent behaviour, but worth a line in the PR description.

  • Cost. In my measurements, resampling takes about 145 ms per channel-minute at 44.1k. Full rhythm() takes about 740 ms on 60 s of 44.1k stereo (including JIT warm-up) and about 245 ms on 22.05k mono. Chroma adds about 600 FFTs of 4096 points per channel-minute. There are no quadratic paths.

  • Benchmark. The dev/test split is by clip index within each genre. Tuning was done on dev only, and the fitted model was left out even though it scored higher on test. All 66 negatives still get no grid, and the highest-scoring negative is 0.122, under the 0.15 floor.

  • The known regressions are acceptable trades:

    • Downbeats on drum-only grids drop from 0.903 to 0.686 F.
    • Drums under voice drop from 0.668 to 0.539 beat F. Those clips are refused, not given wrong grids.
    • In return, GTZAN test downbeat F rises from 0.342 to 0.504, and the synthetic R1 set rises from 0.934 to 0.999 unfloored.
    • Drum-only material with no harmony is a minority of what people beat-sync to.

Should fix (non-blocking)

  • Public API change, call it out in the release notes. audioAnalysis.ts does export * from "./audioRhythm" and @hyperframes/core/audio-analysis is a published subpath, so this removes bandFrames (now private), changes rhythmOf(frames, sampleRate) to a private one-argument function, and raises the exported STEADY_BEAT_CONFIDENCE from 0.05 to 0.15. Nothing else in the repo calls the removed pair, so I'm not blocking on it.

  • Chroma scales with the number of channels. audioRhythm.ts:140 sums magnitudes across channels, while bandPower divides by channels.length. I measured the same sound on two channels at exactly 2.000× the mono chroma. Downbeats are unaffected because chromaBetween normalizes. Sections apply log1p before normalizing (audioSections.ts:56), and that is not scale-invariant, so "mono equals the same sound on two channels" holds only approximately for sections. Dividing by channels.length would make it exact.

  • The parity test (audioRhythm.test.ts:94) covers two identical channels but not L = -R. One more case would lock in the no-cancellation property.

Nits / follow-ups

  • bandFramesOf(signal); at beatDetection.ts:63 is a bare call that exists only to warm the cache before a pause. A short note there, or a clearer name, would help readers.
  • To win back drum-only downbeats, the two chord cues (audioRhythm.ts:392) could be gated on chroma tonality, falling back to the low-end cue when there is no harmony.
  • bandFrames keeps a resampled copy of every channel alive at the same time (audioRhythm.ts:149). For an hour of 48k stereo that is about 635 MB extra. Processing one channel at a time would cap it. This only matters for the long-file case the PR says it has not profiled.
  • At 4096 points and 22.05 kHz, chroma bins are about 5.4 Hz wide. Below about 90 Hz that is coarser than a semitone, so the lowest pitch classes blur a little.

What I ran: bun install --frozen-lockfile, then built packages/parsers. Core vitest run src/audio src/beats passed 440/440 across 22 files, matching the PR description. A scratch script measured resampler attenuation, timing, and channel and rate parity.

— Rames

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Independent review of live successor 11792b8b1bf0414300565a35ef2e52f27b972c91 (the extra commit after requested 59b4c9bb changes comments only).

The resampling and channel-power path checks out: 26 focused tests pass. My separate 12s known-time pulse probe returns the same 24 beat times across 16/22.05/44.1/48/96 kHz × mono/duplicate/inverted/right-only channels; actual signalProvider and wrapper results agree. A 1 kHz tone retains RMS .70694, while 15 kHz attenuates to .000349 on 48→22.05 kHz conversion. Removing the low-pass scaling makes the actual alias regression fail (RMS .70636); source restored afterward.

audioRhythm.ts:461 now applies the same .15 floor before beats/downbeats reach provider or wrapper; CLI beats and Studio URL/buffer analysis use the wrapper, and audio uses signalProvider. The ambient/noise/silence tests and the separate silence/tone probes pass. Chord-change/downbeat and returning-section tests also pass.

Quality judgment: the disclosed drum-only downbeat loss (.903→.686) and drums-under-voice loss (.668→.539) are material, not noise. A produced-music improvement does not establish an improvement for those inputs. The existing confidence: "low" remains appropriate for downbeats. I am not independently validating the GTZAN/R1/S scores, split hygiene or threshold generalization: scripts/per-case outputs are absent. Retaining reproducible benchmark scripts and per-case results would make future threshold changes reviewable, especially since positives and negatives use different synthetic distributions.

Cost: fresh-process 120s mono/48k pulse analysis was ~.62s on base and ~.73s on head on this machine; RSS rose ~81 vs ~91 MiB. This is one bounded fixture, not a statistical benchmark or hour-long/Studio responsiveness certification. New resampled per-channel buffers and cached chroma increase peak/retained work; keep the disclosed long-file limit.

Compatibility note: audioRhythm.ts makes previously exported bandFrames/rhythmOf private; audioAnalysis.ts re-exports this module through the public audio-analysis entry. Repo consumers are migrated, but external named imports would break. Preserve aliases or explicitly record that API removal before publishing; this review does not certify unknown external callers.

Current-head CI has completed: all reported checks are successful or skipped, including regression and Windows jobs; GitHub reports OPEN/CLEAN on this exact head. This supersedes the CI-pending comment5529452944. No confirmed algorithm blocker found in this bounded review. No live Studio, native/model providers, paid API, independent corpus or hour-long run. No merge/release/deploy.

— Magi (desktop-review)

Verdict: APPROVE
Reasoning: Independent rate/channel/anti-alias/floor probes support the implementation; the accuracy tradeoff stays explicit and author benchmarks remain unverified. Current-head CI is now complete without a failure; the documented quality and public-export compatibility notes remain non-blocking release considerations.

@xuanruli
xuanruli added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit 695110e Oct 15, 2026
118 checks passed
@xuanruli
xuanruli deleted the feat/signal-rhythm-quality branch October 15, 2026 07:50
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.

3 participants