fix(song): make bass detection instrument-type-aware, not name-only

[byrongamatos]

Companion to editor PR #335 (feedBack-plugin-editor), which makes a track's instrument first-class data (a manifest `type` field) instead of a name guess.

Problem

Core dropped the authored instrument type, and "is this a bass?" was defined three different ways across call sites:

• name-only in `note_pitch_midi` and the highway scale-degree path,
• `path_bass`+name in bass-player selection,
• name-only in `arrangement_string_count`.

The sloppak loader (the editor's save format) never read the manifest `type` at all. So an editor-authored bass arrangement not named "bass" (e.g. "Low End") would render with 6-string lane counts and guitar open-string MIDI — disagreeing with the editor, which lays out 4 lanes.

Fix

• Add optional `Arrangement.type`; `sloppak.load_song` lifts the manifest entry `type` onto it (normalized, None/whitespace-safe).
• Add `arrangement_is_bass(arr)` = `type=='bass'` or `path_bass` or "bass" in name, and route all consumers through it: `arrangement_string_count`, `note_pitch_midi`, the highway scale-degree base, and bass-player selection.
• Back-compat: an arrangement with no bass signal returns exactly what it did before (6-string / guitar). Guitar / 7-string / 5-string paths untouched.

Scale degrees are display-only and never feed a grader.

Tests

Added to `tests/test_song.py` (each fails pre-fix, passes post-fix): authored `type=bass` and `path_bass` both yield 4-string count and bass open-string MIDI; `arrangement_is_bass` signal-safety (case/whitespace/None); a guitar no-regression case.

Gate: `pytest tests/` → 2733 passed, 4 skipped, 0 failed.

Deferred (not in scope)

`compute_smart_names` still doesn't fold in `arr.type`, so a `type=bass` part named "Low End" displays as "Track 1" rather than "Bass" — a label nicety only; lanes, pitch base, and player selection are all correct.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved detection of bass arrangements, including arrangements identified through editor metadata or archive information rather than their names.
  • Corrected bass string counts and MIDI pitch bases for note playback and conversion.
  • Prevented non-bass arrangements, such as guitar arrangements, from being incorrectly treated as bass.
  • Added safeguards for inconsistent capitalization and spacing in arrangement type labels.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

Bass arrangement type detection

Layer / File(s)	Summary
Arrangement metadata and bass detection lib/song.py, lib/sloppak.py	Arrangements retain normalized editor-authored type metadata, and bass detection checks type, archive metadata, then the legacy name fallback.
Pitch and string-count behavior lib/song.py, tests/test_song.py	Pitch-base and string-count calculations use shared bass detection, with coverage for authored, archive, legacy, and non-bass arrangements.
Highway bass routing lib/routers/ws_highway.py	Bass arrangement selection and MIDI base derivation use the shared bass detector.
Estimated code review effort: 3 (Moderate)	~20 minutes

Suggested reviewers: topkoa, chrisbewithyou

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is on-topic but does not follow the required template and is missing the What, feedpak surface, and checklist sections.	Rewrite it using the template, add the required What and feedpak surface sections, and include the checklist items for changelog, tests, and DCO sign-off.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the main change: making bass detection instrument-type-aware instead of name-only.
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 fix/arrangement-string-count-instrument-type

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/test_song.py (1)

1224-1224: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Move the local import to the module level.

Consider moving from song import arrangement_is_bass to the top of the file alongside the other imports for consistency and to avoid hidden dependencies within the test function.

♻️ Proposed refactor

-    from song import arrangement_is_bass

(And add arrangement_is_bass to the existing imports from song at the top of the file).

🤖 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/test_song.py` at line 1224, Move the arrangement_is_bass import from
inside the test to the module-level import section, adding it to the existing
song imports; remove the local import while preserving the test’s behavior.

🤖 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/test_song.py`:
- Line 1224: Move the arrangement_is_bass import from inside the test to the
module-level import section, adding it to the existing song imports; remove the
local import while preserving the test’s behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e006e554-4de8-442a-b810-3f9b3d5b60a5

📥 Commits

Reviewing files that changed from the base of the PR and between a9be210 and 8b905f1.

📒 Files selected for processing (4)

• lib/routers/ws_highway.py
• lib/sloppak.py
• lib/song.py
• tests/test_song.py