Skip to content

fix(generators): string-count-aware system defaults + bass shape guards (v0.1.12) - #6

Closed
ChrisBeWithYou wants to merge 1 commit into
feat/topbar-host-parityfrom
feat/string-count-aware-generation
Closed

ChrisBeWithYou wants to merge 1 commit into
feat/topbar-host-parityfrom
feat/string-count-aware-generation

Conversation

@ChrisBeWithYou

Copy link
Copy Markdown
Collaborator

Stacked on #5 (topbar host parity) — GitHub auto-retargets to virtuoso-dev when #5 merges. Review only this PR's diff.

What

Tester symptom: switching 6-string → 7/8-string guitar didn't add the extra strings to most exercises. Full audit verdict: design, not plumbing — string count/tuning reach every builder, but CAGED/Open deliberately anchor the top-six EADGBE subset (off = N-6; correct for CAGED itself — no method teaches an "8-string CAGED"), and caged was the universal beginner/session default. Renderers were innocent (they draw all N lanes).

Fix (guitar-pedagogy · bass-pedagogy · metal-idiom panel verdicts)

  1. defaultFretboardSystem(instrument, count) — bass → position, guitar N>6 → 3nps (the extended-range scale system; tiles every string), guitar 6 → caged (unchanged). Explicit advanced-mode picks and rung-coded systems always win — a rung that teaches a CAGED shape stays a CAGED lesson. Wired into readConfig + buildSegmentConfig.
  2. Instrument-first guards on every count-gated shape path (resolveCurrentShape, cagedShapeNotesForChord, templateFromShape, sweep wantShape). Kills a real latent pitch bug: a 6-string bass passes the old count-only >=6 guards, but CAGED templates bake EADGBE's G→B major 3rd — on an all-4ths bass the top two strings land a semitone flat. Only UI suppression protected it; programmatic configs (preset import, the new host sync) were exposed.

Deliberately NOT in this PR (panel-ratified, ROADMAP open thread)

  • Extended-range CAGED window-continuation (opt-in; never a re-rooted box — extended-range players think top-6 box + low-string riff floor, per metal-idiom).
  • bassRootGrip low-fifth downward reach (the "why own a 5-string" idiom).
  • Drop-tuning single-finger s0+s1+s2 barre primitive.
  • Sweeps/strum-grips/fingerstyle stay top-6 by design (now documented + guarded).

Verification

  • New smoke-strings rows: (12) default routing — 8-string default = 3nps, run reaches s=0/s=1, no-unison holds, 6-string stays caged, explicit caged preserved; (13) bass never resolves a shape, a forced 6-string-bass caged sweep stays diatonic (traps the semitone-flat leak), 5-string scale plays the low-B string.
  • Suite green; full run 16/19 — the 3 reds (backing-engine voice-leading row, progress, variation) fail identically on base; contained-verifier + level-gate-async flaked once under concurrency-4, green solo.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MS2YFb6UUSwJVV6CmEa25i

…ds (v0.1.12)

Audit verdict: "8-string exercises ignore the extra strings" was DESIGN, not
plumbing - string count/tuning reach every builder, but CAGED/Open anchor the
top-six EADGBE subset by construction (correct for CAGED itself) and 'caged'
was the universal beginner/session default, so most exercises never touched
strings 7-8 (renderers draw all N lanes; the bottom two just sat empty).

Fix (guitar-pedagogy + bass-pedagogy + metal-idiom panel):
- defaultFretboardSystem(instrument, count): bass -> 'position' (bass never
  uses guitar shape systems; the pathway path already coded this - now
  Custom/beginner + sessions agree), guitar N>6 -> '3nps' (THE extended-range
  scale system - tiles every string, so 7/8-string runs actually reach the
  low B/F#), guitar 6 -> 'caged' (unchanged). Explicit advanced-mode picks and
  rung-coded systems always win; a rung that TEACHES a CAGED shape stays a
  CAGED lesson. Wired into readConfig + buildSegmentConfig.
- Instrument-first guards (defense-in-depth) on every count-gated shape path:
  resolveCurrentShape, cagedShapeNotesForChord, templateFromShape, sweep
  wantShape. Kills a REAL latent pitch bug: a 6-string bass satisfies the old
  count-only >=6 guards, but the CAGED templates bake EADGBE's G->B major 3rd
  - on an all-4ths B-E-A-D-G-C bass the top two strings land a semitone flat.
  Only UI suppression protected it (hidden shape controls); programmatic
  configs (preset import, host sync) were exposed.

Smoke: new smoke-strings rows (12) default routing (8-string default=3nps,
reaches s=0/s=1, no-unison holds; 6-string stays caged; explicit caged
preserved) + (13) bass never resolves a shape, forced 6-string-bass caged
sweeps stay diatonic, 5-string scale plays the low-B string. Suite green;
full run 16/19 with only the pre-existing base reds (backing-engine voice-
leading row, progress, variation); contained-verifier + level-gate-async
flaked under concurrency, green solo.

Deferred by panel verdict (ROADMAP open thread): opt-in extended-range CAGED
window-continuation (never a re-rooted box), bassRootGrip low-fifth downward
reach, drop-tuning s0+s1+s2 barre primitive.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MS2YFb6UUSwJVV6CmEa25i
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 536872fb-c598-4661-b753-12dcdb42c902

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/string-count-aware-generation

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

@ChrisBeWithYou

Copy link
Copy Markdown
Collaborator Author

Landed on virtuoso-dev via the 2026-07-14 batch integration (v0.2.2), not a GitHub merge event — these branches predate the main→dev reconcile (Byron's Gold-rung #11 took the line to 0.2.x) and were rebased, so their head SHAs don't match the commits that reached dev. All content is verified present in dev (incl. #5's 62fc72e tuningMidis, cherry-picked as f0c78c6). Closing to avoid a duplicate re-merge; the work ships to main via the dev→main PR #4 line. — automated

@ChrisBeWithYou
ChrisBeWithYou deleted the feat/string-count-aware-generation branch July 16, 2026 12:45
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