Skip to content

fix: align drill scoring with active loops - #68

Open
vo90 wants to merge 1 commit into
got-feedBack:mainfrom
vo90:agent/ignore-inactive-loop-drills
Open

fix: align drill scoring with active loops#68
vo90 wants to merge 1 commit into
got-feedBack:mainfrom
vo90:agent/ignore-inactive-loop-drills

Conversation

@vo90

@vo90 vo90 commented Jul 19, 2026

Copy link
Copy Markdown

Player value

This is the NoteDetect companion to got-feedBack/feedBack#1016.

The main loop change lets players prepare a loop, wait before starting it, use a first-pass count-in, and then repeat continuously without another count-in. NoteDetect should only treat that passage as an active drill once the loop is actually running.

Without this companion change, valid A/B markers alone could start drill accounting while the player was still waiting or counting in. That could make iteration statistics begin too early and no longer match what the player actually practiced.

What changes

  • configured-but-inactive loops do not activate drill mode
  • armed, partial, and count-in-starting loop states stay outside drill accounting
  • an actively running loop still enables per-iteration drill statistics
  • older feedBack hosts that expose only { loopA, loopB } keep their existing behavior

Why this is a separate PR

NoteDetect is distributed from its own plugin repository, so the host-side loop experience and plugin-side drill accounting need separate, linked changes.

Technical implementation

  • reads the structured lifecycle returned by the newer host loop API
  • requires finite A/B bounds and no explicit inactive state before enabling drill mode
  • preserves backward compatibility when active and state fields are absent
  • adds repository-local coverage for inactive, starting, active, and legacy loop shapes

Review status

CodeRabbit completed a full review of both changed files with no actionable comments. This companion has been reviewed together with got-feedBack/feedBack#1016 and is ready for maintainer review.

Validation

  • node --test test/drill_mode.test.js — 20 passed
  • focused inactive/legacy loop-state tests — 3 passed
  • node --check screen.js — passed
  • git diff --check — passed

The full Windows suite reports 353/354 because an existing queue-summary source-text test matches LF-only newlines; that file and behavior are unchanged by this PR. Linux CI remains the authoritative full-suite check.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Drill synchronization now considers explicit loop activity and arming states before enabling drill mode. Tests cover inactive configured loops and confirm activation for an explicitly active loop.

Changes

Drill mode synchronization

Layer / File(s) Summary
Activation state logic and validation
screen.js, test/drill_mode.test.js
_drillSyncFromLoopState() suppresses drill activation for explicitly armed or inactive loop states while retaining legacy behavior for missing state fields; tests cover inactive configurations and explicit activation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: byrongamatos

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: drill scoring now depends on active loop state.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@vo90 vo90 changed the title fix: ignore inactive loops in drill mode fix: align drill scoring with active loops Jul 19, 2026
@vo90

vo90 commented Jul 20, 2026

Copy link
Copy Markdown
Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@vo90

vo90 commented Jul 20, 2026

Copy link
Copy Markdown
Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@vo90
vo90 marked this pull request as ready for review July 20, 2026 08:11
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