[mattromano]

What

Closes #75 (related: #15)

In a fast run of one chord shape, the next chord is already sounding when its window opens. Three spots booked that as an EARLY miss:

• Engine verifier: chord stamp. _ndFinalizeChordVerdict used the detect time of the first member listed. It now uses the member detected closest to the chart time, so a string still ringing from the previous chord can't decide the timing of a chord the other strings landed on time.
• Engine verifier: early window edge. The engine only verifies a chord inside its timing window, but the playhead it stamps with is pushed once per detect tick and can lag behind. A stamp between the early hit edge and the window opening minus _ND_ENGINE_STAMP_SLACK_S (100 ms) is now counted as on time (clamped to the hit edge) instead of EARLY by poll jitter. A stamp further out is still EARLY, and there's a test for that.
• scoreChord / browser chord path. An isHit frame whose judgment is EARLY no longer records a counted miss. It continues like the existing non-hit branch, so a later frame in the window can still hit, and checkMisses() retires the chord as before if none does. It deliberately doesn't stash the frame in _chordLastResult: an early ring there would set the voicing rescue's voicingT to an EARLY time and stop the rescue from working.

I found these while reading per-judgment logs from fast power-chord songs on the desktop build.

Version bumped 1.32.0 → 1.33.0 in plugin.json, package.json and _ND_VERSION. This collides with #72 and the other open note_detect PRs, which bump to the same version. Whichever merges later needs a one-line re-bump. This change doesn't touch #72's tuning work. It does share _ndFinalizeChordVerdict with the richer-judgment-events PR, so expect a small textual conflict there.

Tests

• New test/fast_chord_runs.test.js (4 tests, all through the real detector in the vm harness):
  • engine: one member ringing 170 ms early and one at −20 ms → hit with timingError: -20
  • engine: both members stamped −180 ms (inside the window plus poll slack) → OK hit
  • engine: −400 ms is still an EARLY miss (guard)
  • scoreChord path: with chord hit threshold 100 ms and tolerance 150 ms, an EARLY scored frame at −130 ms books nothing, and the on-time frame then hits
  • Before the fix the first, second and fourth fail. The guard passes both ways.
• npm test: 357/357 pass. node --check screen.js is OK, and feedBack-plugin-spec/tools/validate.py reports ok.
• tools/regression.js needs local recordings, which aren't available here, so it wasn't run. The browser change only affects frames that were already booking a miss, and the engine changes only move stamps that were EARLY.

Checklist

• Tests added/updated for new behaviour
• Version bumped in all three places (version_sync.test.js green)
• Commits are DCO signed off (git commit -s)

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 46 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: e38db5f1-389e-40de-9297-a48c7df987ec

---

📥 Commits

Reviewing files that changed from the base of the PR and between 412433d and d538482.

---

📒 Files selected for processing (4)

• package.json
• plugin.json
• screen.js
• test/fast_chord_runs.test.js

---

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[mattromano]

Consolidated into #84 (commits kept, rebased onto main, plus the review fixes).