feat(editor): resolve chords as one coherent grip, not a greedy spread

[ChrisBeWithYou]

What & why

"Resolve positions" over an anchor window placed each note of a simultaneous cluster greedily — one at a time, each grabbing its own lowest free fret. For a chord that spreads the notes across the neck into a stretch no hand can play, and which the playability (stretch) lint then flags. Gap-audit #7: kill unplayable auto-chords at the source.

Approach

Resolve the cluster jointly.

• _resolveChordGripPure (new, position.js) — given each note's sounding pitch, enumerate its playable positions and choose one {string, fret} per note, all on distinct strings, that minimises the fretted span (open strings, fret 0, are free), pulled toward the hand (previous note → anchor). It accepts only a grip whose span fits the lint's window + tolerance, and returns null when no coherent grip exists. Deterministic search (fewest-choice-first, pinned tie-breaks).
• _resolveWindowPure (anchor-resolve.js) now walks the window in time order, groups simultaneous notes into clusters (LINT_CLUSTER_EPSILON), and resolves each cluster of ≥2 as one grip. A singleton — or a cluster with no coherent grip — falls back to the per-note resolver, so nothing that resolved before stops resolving. Occupancy still honours non-cluster (overlapping) notes; distinct strings within the cluster are enforced by the grip search.

The span ceiling and cluster epsilon are the same constants the stretch lint uses (LINT_STRETCH_TOLERANCE, LINT_DEFAULT_WINDOW, LINT_CLUSTER_EPSILON), so the resolver targets exactly what the lint would otherwise scold.

Tests

• tests/chord_grip.test.mjs (11) — the grip search (min span, distinct strings, open-string-free, anchor-window eligibility, occupancy routing, the too-wide and no-eligible-position refusals, determinism) and the end-to-end _resolveWindowPure integration (a chord greedy spreads to a 6-fret stretch now lands as a 2-fret grip; singleton + no-grip fallback unchanged — these fail on main's per-note path).
• All existing position/anchor suites still green; full JS suite 135, ESLint 0 errors, routes.py untouched.
• Live-verified in-app through the real editorResolveAnchorWindow action: a spread chord snaps to s3f5 + s4f3 (fretted span 2), committed as one undoable command, and undo restores the original spread.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Improvements

  • Chords inside an anchor window now resolve into a single coherent grip, keeping notes together in the tightest playable hand shape instead of spreading across the neck.
  • Resolution accounts for existing string occupancy and movement from the prior/anchor position for more natural results.
  • Open-string options stay available without increasing the counted fretted span.

• Bug Fixes

  • If no valid shared chord grip can be found, resolution cleanly falls back to the prior per-note behavior.
  • Ambiguous “open vs fretted” choices are refused rather than silently re-voiced.

• Tests

  • Added coverage for chord-grip selection and anchor-window integration.

[coderabbitai]

📝 Walkthrough

Walkthrough

Chord resolution now groups simultaneous notes within anchor windows and selects a coherent playable fret-hand grip. The resolver tracks repicks and occupancy, falls back to per-note placement when needed, and adds grip and integration tests.

Changes

Chord resolution

Layer / File(s)	Summary
Joint chord grip solver src/position.js	Adds _resolveChordGripPure, which selects deterministic minimum-span assignments on distinct strings while respecting occupancy, anchor windows, and span limits.
Anchor-window cluster integration src/anchor-resolve.js, CHANGELOG.md, node_modules	Groups simultaneous notes for joint resolution, tracks repick-aware state, retains per-note fallback behavior, and records the node_modules symlink change.
Chord resolution validation tests/chord_grip.test.mjs	Tests grip selection, open strings, occupancy, anchor constraints, deterministic output, singleton fallback, and unresolved clusters.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Resolver as _resolveWindowPure
  participant Grip as _resolveChordGripPure
  participant Fallback as _suggestPositionPure
  Resolver->>Resolver: Group simultaneous notes
  Resolver->>Grip: Resolve a coherent chord grip
  Grip-->>Resolver: Return assignments or no grip
  Resolver->>Fallback: Resolve notes individually when needed
  Fallback-->>Resolver: Return suggested positions

Loading

Possibly related PRs

• got-feedBack/feedBack-plugin-editor#200: Introduces the playability-lint constants consumed by the updated anchor/chord resolution logic.
• got-feedBack/feedBack-plugin-editor#201: Modifies the same anchor-window position resolution pipeline with earlier per-note handling.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: resolving simultaneous notes as one coherent chord grip instead of greedily spreading them.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/editor-chord-grip-resolve

---

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

[coderabbitai]

🧹 Nitpick comments (1)

tests/chord_grip.test.mjs (1)

124-134: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Make the fallback test prove that fallback ran.

This all-refused case also passes if a future implementation skips resolveOne and rejects the cluster outright. Use two pitch-48 notes with anchor window [3,4): both only fit on s1f3, so the joint grip fails; fallback should move the first note and refuse the second due to in-pass occupancy.

Proposed test adjustment

-    // Anchor window [10,12) width 2, maxSpan 3. Two notes sounding 48: candidates
-    // s1f3 (out of window) and s0f8 (out of window) → no in-window grip AND the
-    // per-note resolver also refuses (outside-anchor-window) — both left as-is.
+    // Anchor window [3,4): both notes can only use s1f3. The joint solver
+    // cannot assign distinct strings, so fallback moves one and refuses one.
     const nn = [N(2, 0, 8, 0.5), N(2, 0, 8, 0.5)];
-    const anchors = [{ time: 0, fret: 10, width: 2 }];
+    const anchors = [{ time: 0, fret: 3, width: 1 }];
     const ctx = { ...CTX, prevFretAt: () => null };
     const r = _resolveWindowPure(nn, { start: 0, end: Infinity }, anchors, ctx, () => true);
-    assert.strictEqual(r.moves.length, 0);
-    assert.strictEqual(r.refused.length, 2, 'both fall back and refuse — never guessed');
+    assert.deepStrictEqual(r.moves, [
+        { index: 0, oldString: 0, oldFret: 8, newString: 1, newFret: 3 },
+    ]);
+    assert.strictEqual(r.refused.length, 1);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/chord_grip.test.mjs` around lines 124 - 134, Update the fallback test
around _resolveWindowPure to use two pitch-48 notes and an anchor window at fret
3 with width 1, where both notes only resolve to s1f3. Assert the joint grip
fails, then verify fallback moves the first note and refuses the second because
the target is occupied during the same pass, proving resolveOne was executed.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/chord_grip.test.mjs`:
- Around line 124-134: Update the fallback test around _resolveWindowPure to use
two pitch-48 notes and an anchor window at fret 3 with width 1, where both notes
only resolve to s1f3. Assert the joint grip fails, then verify fallback moves
the first note and refuses the second because the target is occupied during the
same pass, proving resolveOne was executed.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4ec266de-3186-4a51-85d4-47eda89196b3

📥 Commits

Reviewing files that changed from the base of the PR and between 4968f8b and f45b19b.

📒 Files selected for processing (4)

• CHANGELOG.md
• src/anchor-resolve.js
• src/position.js
• tests/chord_grip.test.mjs

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@node_modules`:
- Line 1: Remove the machine-local node_modules symbolic link from version
control, ensure node_modules is covered by the repository’s ignore
configuration, and rely on the project manifest and lockfile for dependency
installation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 19ab7a63-26d9-426d-bbaa-f1f1cb5b4ac7

📥 Commits

Reviewing files that changed from the base of the PR and between f45b19b and 4500d1d.

📒 Files selected for processing (5)

• CHANGELOG.md
• node_modules
• src/anchor-resolve.js
• src/position.js
• tests/chord_grip.test.mjs

🚧 Files skipped from review as they are similar to previous changes (4)

• CHANGELOG.md
• tests/chord_grip.test.mjs
• src/anchor-resolve.js
• src/position.js

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Remove the machine-local node_modules entry.

This absolute path only resolves on the author’s machine, breaks dependency resolution in clean checkouts, and exposes local filesystem details. Remove it from version control and rely on the project’s package manifest/lockfile with node_modules ignored.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@node_modules` at line 1, Remove the machine-local node_modules symbolic link
from version control, ensure node_modules is covered by the repository’s ignore
configuration, and rely on the project manifest and lockfile for dependency
installation.