fix(core): exclude let_ring from the technical complexity axis (#75) - #97
Conversation
…axis (#75) Playtest finding: a held let-ring drone (one LetRing span over every note) reads technical complexity 1.0 — maximally technical — though letting notes ring on is a sustain instruction, not a technical demand. Failing test asserts a let-ring-only phrase scores 0 on the axis. Green excludes LetRing spans. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
#75) `technique_share` now filters out LetRing spans before deciding whether a note is technically covered, so a passive sustain no longer marks notes as technical. A held let-ring drone scores 0 on the axis instead of 1.0; let_ring still surfaces as a tag/technique. Other spans (palm-mute, etc.) and per-note marks are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
Y-statement for the technique_share calibration: why a passive sustain is not a technical demand, and that only let_ring is reclassified for now. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesLetRing exclusion from technical complexity axis
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@codex review Generated by Claude Code |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
A playtest export of the DGD GPX showed a held let-ring section (phrase p5 — a sustained 8-note drone, one
LetRingspan over every note) scoringtechnical = 1.0, maximally technical, though letting notes ring on is a sustain instruction, not a playing demand.What
structure::technique_share(thetechnicalcomplexity axis = share of a track's notes carrying a per-note mark or sitting in a technique span) now excludesSpanTechnique::LetRingspans before deciding coverage. A let-ring-only phrase scores0on the axis instead of1.0.let_ringstill surfaces as aSwancoreTag/technique — only the difficulty axis stops counting it.Scope / safety
LetRingis reclassified. Palm-mute and other spans, and all per-note marks, are unchanged — the two existing technical-axis tests (bothPalmMute) still pass.1.0 → 0; phrases mixing let_ring with slide/hammer (p1, p9) ease down to honest values.TDD
red (
d0eebb3) → green (852abfb) → docs (fd6535e). Newtechnical_axis_ignores_a_passive_let_ring_spanasserts a let-ring drone scores 0.Verification
cargo test --workspacegreen;cargo clippy --workspace --all-targets -- -D warningsclean;cargo fmt --all --checkcleanlet_ringfor now; a broader mark/span split deferred until the corpus motivates it) is recorded indecisions.log.🤖 Generated with Claude Code
https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Documentation