From d67921590f05435989187a9319044fc3aecaf507 Mon Sep 17 00:00:00 2001 From: mattromano <42412983+mattromano@users.noreply.github.com> Date: Fri, 9 Oct 2026 17:21:18 -0700 Subject: [PATCH] fix(sections): track per-section stats on the desktop engine path currentSection only advanced at the tail of checkMisses(), which returns early under the engine verifier, so desktop sessions recorded no per-section stats and notedetect:session.sections was always empty. Keep the section tracker running on that path, and bin each judgment by the section its note sits in (engine verdicts land after the playhead may have crossed into the next section), falling back to the playhead's section when the note time is unknown. Co-Authored-By: Claude Opus 5.5 Signed-off-by: mattromano <42412983+mattromano@users.noreply.github.com> --- screen.js | 59 ++++++---- test/engine_section_stats.test.js | 172 ++++++++++++++++++++++++++++++ 2 files changed, 213 insertions(+), 18 deletions(-) create mode 100644 test/engine_section_stats.test.js diff --git a/screen.js b/screen.js index bd9025c..53d06a1 100644 --- a/screen.js +++ b/screen.js @@ -5873,7 +5873,7 @@ function createNoteDetector(options = {}) { if (_ndIsStreakMilestone(streak)) { dispatchFx({ fxType: 'milestone', streak, mult: multiplier }); } - updateSectionStat('hit'); + updateSectionStat('hit', judgment.noteTime); // A hit breaks the contiguous-miss run the auto-drill watches. _autoDrillMissStreak = 0; } else { @@ -5886,7 +5886,7 @@ function createNoteDetector(options = {}) { dispatchFx({ fxType: 'streakBreak', lostStreak, prevMult: multiplier }); } multiplier = 1; - updateSectionStat('miss'); + updateSectionStat('miss', judgment.noteTime); // Grow the contiguous-miss run and, at the threshold, drop into a // drill on the fluffed passage (auto-drill). Track the run's // chart-time span so the drill targets exactly those notes. @@ -6617,7 +6617,13 @@ function createNoteDetector(options = {}) { // (detected:false verdicts, drained by _ndDrainEngineVerdicts), so // this renderer-side retire scan is superseded. The browser path — // and any desktop session on a downlevel addon — still relies on it. - if (_ndUsingEngineVerifier) return; + // The section tracker below still has to advance, though, or the + // per-section stats stay empty on desktop. + if (_ndUsingEngineVerifier) { + const avOffsetSec = (hw.getAvOffset ? hw.getAvOffset() / 1000 : 0); + _ndTrackSection(hw.getTime() + avOffsetSec - latencyOffset); + return; + } // Reset the per-tick rescue-FFT budget (see _RESCUE_WINDOWS_PER_TICK). // checkNote spends against this; once exhausted, further conceded bass // misses this tick retire as plain misses instead of running FFTs. @@ -6871,27 +6877,44 @@ function createNoteDetector(options = {}) { } } + _ndTrackSection(t); + } + + // Name of the chart section containing chart time `t` (the last one that + // starts at or before it), or null when the chart has no sections. + function _ndSectionNameAt(t) { const sections = hw.getSections ? hw.getSections() : null; - if (sections) { - let current = null; - for (const sec of sections) { - if (sec.time <= t) current = sec.name; - else break; - } - if (current && current !== currentSection) { - currentSection = current; - if (!sectionStats.find(s => s.name === current)) { - sectionStats.push({ name: current, hits: 0, misses: 0 }); - } + if (!sections || !Number.isFinite(t)) return null; + let current = null; + for (const sec of sections) { + if (sec.time <= t) current = sec.name; + else break; + } + return current; + } + + // Advance currentSection to the playhead's section, opening its row in + // sectionStats so a section the player passes through still shows up. + function _ndTrackSection(t) { + const current = _ndSectionNameAt(t); + if (current && current !== currentSection) { + currentSection = current; + if (!sectionStats.find(s => s.name === current)) { + sectionStats.push({ name: current, hits: 0, misses: 0 }); } } } - function updateSectionStat(type) { - if (!currentSection) return; - let sec = sectionStats.find(s => s.name === currentSection); + // Bin a judgment by the section its NOTE sits in when we know the note's + // chart time. Engine verdicts land after a note's window closes, often once + // the playhead has crossed into the next section; the playhead's section + // (currentSection) is only the fallback. + function updateSectionStat(type, noteTime) { + const name = _ndSectionNameAt(noteTime) || currentSection; + if (!name) return; + let sec = sectionStats.find(s => s.name === name); if (!sec) { - sec = { name: currentSection, hits: 0, misses: 0 }; + sec = { name, hits: 0, misses: 0 }; sectionStats.push(sec); } if (type === 'hit') sec.hits++; diff --git a/test/engine_section_stats.test.js b/test/engine_section_stats.test.js new file mode 100644 index 0000000..229f5e2 --- /dev/null +++ b/test/engine_section_stats.test.js @@ -0,0 +1,172 @@ +// Per-section stats on the desktop engine-verifier path. +// +// Section tracking (`currentSection`) used to be advanced only at the tail of +// checkMisses(), which returns early when the engine verifier is in use — the +// engine finalizes misses itself. So on desktop every judgment hit +// updateSectionStat() with currentSection still null: getStats().sectionStats +// stayed empty and notedetect:session.sections shipped as [] for the practice +// journal and the results card. Engine verdicts also land late (a note's +// verdict arrives after its sustain window closes, often after the playhead has +// crossed into the next section), so a judgment is now binned by the section +// its NOTE sits in, not by where the playhead happens to be. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const { loadDetectionCore } = require('./_loader'); + +async function flushPendingAsync(turns = 6) { + for (let i = 0; i < turns; i++) { + // eslint-disable-next-line no-await-in-loop + await new Promise((r) => setImmediate(r)); + } +} + +// Four notes: two in "Intro" (0–2 s), two in "Verse" (2 s →). +const CHART = [ + { s: 0, f: 3, t: 0.5, sus: 0 }, + { s: 0, f: 3, t: 1.5, sus: 0 }, + { s: 0, f: 5, t: 2.5, sus: 0 }, + { s: 0, f: 5, t: 3.5, sus: 0 }, +]; +const SECTIONS = [ + { name: 'Intro', time: 0 }, + { name: 'Verse', time: 2.0 }, +]; + +function sandbox() { + const calls = { getNoteVerdicts: 0, getPitchDetection: 0 }; + let pushedChart = null; + const verdictQueue = []; + const intervalCallbacks = []; + let hwTime = 0; + + const audio = { + isAvailable: async () => true, + isAudioRunning: async () => true, + startAudio: async () => {}, + getLevels: async () => ({ inputLevel: 0, inputPeak: 0, outputLevel: 0, outputPeak: 0 }), + getSampleRate: async () => 48000, + getPitchDetection: async () => { + calls.getPitchDetection++; + return { midiNote: -1, confidence: 0, frequency: -1, cents: 0, noteName: '' }; + }, + setChart: async (chart) => { pushedChart = chart; return true; }, + getNoteVerdicts: async () => { + calls.getNoteVerdicts++; + return verdictQueue.length ? verdictQueue.shift() : []; + }, + }; + + const { createNoteDetector } = loadDetectionCore({ + sandboxBeforeRun(sb) { + sb.setInterval = (cb) => { + if (typeof cb === 'function') intervalCallbacks.push(cb); + return intervalCallbacks.length; + }; + sb.highway.getNotes = () => CHART.map((n) => ({ ...n })); + sb.highway.getChords = () => ([]); + sb.highway.getSections = () => SECTIONS.map((s) => ({ ...s })); + sb.highway.getTime = () => hwTime; + sb.highway.getAvOffset = () => 0; + sb.window.feedBackDesktop = { isDesktop: true, platform: 'linux', audio }; + }, + }); + + return { + createNoteDetector, calls, intervalCallbacks, verdictQueue, + setHwTime: (t) => { hwTime = t; }, + chartIds: () => (pushedChart && pushedChart.notes + ? pushedChart.notes.map((n) => n.id) : []), + }; +} + +async function driveDetectTick(intervalCallbacks, calls) { + for (const cb of intervalCallbacks) { + const before = calls.getNoteVerdicts + calls.getPitchDetection; + // eslint-disable-next-line no-await-in-loop + await cb(); + // eslint-disable-next-line no-await-in-loop + await flushPendingAsync(); + if (calls.getNoteVerdicts + calls.getPitchDetection > before) return cb; + } + return null; +} + +// Plain-realm copy so deepStrictEqual doesn't trip on the vm's prototypes. +const plain = (rows) => Array.from(rows || [], (s) => ({ name: s.name, hits: s.hits, misses: s.misses })); + +test('engine verifier: verdicts are binned into per-section stats', async () => { + const env = sandbox(); + const det = env.createNoteDetector({ isDefault: false }); + await det.enable(); + await flushPendingAsync(); + const detectTick = await driveDetectTick(env.intervalCallbacks, env.calls); + assert.equal(typeof detectTick, 'function', 'a detect tick should be registered'); + + const ids = env.chartIds(); + assert.equal(ids.length, CHART.length); + + // Intro: hit + miss, both delivered while the playhead is still in the Intro. + env.setHwTime(1.9); + env.verdictQueue.push([ + { id: ids[0], detected: true, detectedSongTime: 0.5, centsError: 0, snr: 6 }, + { id: ids[1], detected: false }, + ]); + await detectTick(); + await flushPendingAsync(); + + // Verse: two hits, delivered after the song has run on past them. + env.setHwTime(4.5); + env.verdictQueue.push([ + { id: ids[2], detected: true, detectedSongTime: 2.5, centsError: 0, snr: 6 }, + { id: ids[3], detected: true, detectedSongTime: 3.5, centsError: 0, snr: 6 }, + ]); + await detectTick(); + await flushPendingAsync(); + + const stats = det.getStats(); + assert.equal(stats.hits, 3); + assert.equal(stats.misses, 1); + assert.deepEqual(plain(stats.sectionStats), [ + { name: 'Intro', hits: 1, misses: 1 }, + { name: 'Verse', hits: 2, misses: 0 }, + ]); + + det.destroy(); + await flushPendingAsync(); +}); + +test('engine verifier: a late verdict is binned by its note, not the playhead', async () => { + const env = sandbox(); + const det = env.createNoteDetector({ isDefault: false }); + await det.enable(); + await flushPendingAsync(); + const detectTick = await driveDetectTick(env.intervalCallbacks, env.calls); + const ids = env.chartIds(); + + // The playhead is already in the Verse when the Intro's last note is + // finalized — the engine only reports once the note's window has closed. + env.setHwTime(2.3); + // Let the other intervals (checkMisses' section tracker among them) see the + // playhead first, so currentSection really is the Verse. + for (const cb of env.intervalCallbacks) { + if (cb === detectTick) continue; + // eslint-disable-next-line no-await-in-loop + await cb(); + } + await flushPendingAsync(); + assert.ok(plain(det.getStats().sectionStats).some((s) => s.name === 'Verse'), + 'the section tracker should have opened the Verse on the engine path'); + env.verdictQueue.push([ + { id: ids[1], detected: true, detectedSongTime: 1.5, centsError: 0, snr: 6 }, + ]); + await detectTick(); + await flushPendingAsync(); + + const intro = plain(det.getStats().sectionStats).find((s) => s.name === 'Intro'); + assert.deepEqual(intro, { name: 'Intro', hits: 1, misses: 0 }, + 'the Intro note must count toward the Intro, not the Verse the playhead is in'); + + det.destroy(); + await flushPendingAsync(); +});