Work plan: S13 harmonic context (stage done); S14 sub-bar period detection (schema v5) - #38
Conversation
… material (S13 backlog)
analyze_part grows the last S13 backlog item: a Krumhansl–Schmuckler key
estimate (Krumhansl–Kessler profiles correlated against the part's
duration-weighted pitch-class histogram) carried as a fact in
PartProfile::harmony { tonic_pitch_class, mode, scale_fit }, where
scale_fit is the duration-weighted fraction of notes on the inferred
key's scale. Pitch material is enriched, not replaced: B substitutes
from A's literal pitch classes plus the inferred key's scale, so a
single-pitch-class part (the power-chord riff case) no longer collapses
B onto one pitch per band. References fields that do not exist yet, so
the suite fails to compile until the green step.
https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
…e enriches pitch material
PartProfile gains harmony: Option<HarmonicContext> { tonic_pitch_class,
mode (major / natural minor), scale_fit }. The key is estimated by
correlating the part's duration-weighted pitch-class histogram against
the 24 rotated Krumhansl–Kessler tonal-hierarchy profiles (Pearson);
ties resolve to the earliest key in the major-then-minor, C-upward scan,
and a part whose notes all have zero duration falls back to count
weighting. scale_fit — the duration-weighted fraction of notes on the
inferred key's scale — is carried as a fact, not a verdict.
scale_intervals_from now unions the inferred key's scale into B's
substitution material (enrich, don't replace): B can still echo A's
literal pitch classes, and a sparse part (the power-chord riff case)
no longer collapses B onto a single pitch per band. Closes the last
S13 backlog item (richer harmonic context for pitch material).
https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
…ecord the decision The stage's last backlog item is closed, so S13 moves to done (threshold calibration stays with the corpus / S9). The glossary gains the Harmonic context term (§8), and the decisions log records the Krumhansl–Schmuckler choice with its prior art and the enrich-don't-replace material rule. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a9c398db2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| numerator += dx * dy; | ||
| var_x += dx * dx; | ||
| var_y += dy * dy; |
There was a problem hiding this comment.
Fix clippy-blocking float accumulations
In this repo's required cargo clippy --all-targets -- -D warnings check, these new accumulation expressions trigger clippy::suboptimal_flops under the enabled nursery lints, so the lint job fails before CI can pass. Rewriting the updates with mul_add (or explicitly allowing the lint here) keeps the harmonic-context implementation from blocking the documented lint gate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Checked against the documented lint gate on the pinned toolchain (rust-toolchain.toml): cargo clippy --all-targets -- -D warnings exits 0 with zero warnings on this branch. clippy::suboptimal_flops does not fire on compound-assignment accumulations (numerator += dx * dy desugars to +=, which the lint skips); it only flags expressions of the shape c + a * b, which is why structure.rs uses mul_add there but plain accumulation loops elsewhere in the crate pass clean. Leaving the loop as written — no allow needed.
Generated by Claude Code
…S14 refinement) StructureMetrics grows detected_subbar_period_ticks: the strongest verbatim-tiling lag shorter than one bar, found by autocorrelation over per-beat cell signatures on a uniform timeline. Cells compare by exact (onset, pitch) signatures — at one-onset granularity the bar-level rhythm floor and transposition credit are degenerate (almost any two single-note cells would match), so the sub-bar pass demands verbatim repeats; a half-bar alternation reads as Some(half bar), a uniform pulse as Some(one beat) (ties to the shortest lag), a scale run and a silent track as None, and a mixed-meter timeline abstains. The field persists with ChunkMeta, so the corpus schema bumps to v5 under the established pattern: pre-v5 structure blocks load the field as None and re-serialize without inventing the key. References a field that does not exist yet, so the suite fails to compile until the green step. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
detect_subbar_period autocorrelates per-beat cell signatures across a uniform timeline (one shared time signature and bar span, the bar dividing evenly into numerator beats) and reports the strongest verbatim-tiling lag shorter than one bar as StructureMetrics::detected_subbar_period_ticks. Cells compare by exact (onset-within-cell, pitch) Jaccard — at one-onset granularity the bar-level rhythm floor and transposition credit are degenerate, so the sub-bar pass demands verbatim repeats; the shared PERIOD_THRESHOLD gates the verdict and ties resolve to the shortest lag. Silent tracks, mixed-meter timelines, and non-dividing bars abstain with None. The field is optional in the corpus schema (default None, key skipped when absent), bumping SCHEMA_VERSION to 5: pre-v5 structure blocks keep loading and re-serialize losslessly. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
The sub-bar refinement is measured and persisted (corpus schema v5); the remaining S14 metric work is the per-axis ComplexityProfile. The decisions log records the exact-signature autocorrelation choice with its prior art and the degeneracy argument against reusing the bar-level similarity at beat granularity. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
|
@codex plz review changes New increment pushed: S14 sub-bar (beat-level) period detection — Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5aa8e41758
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| if let Some(b) = cells.get(i.saturating_add(lag_cells)) { | ||
| sum += set_jaccard(a, b); | ||
| } |
There was a problem hiding this comment.
Exclude empty beat pairs from sub-bar periods
For sparse riffs, the new autocorrelation treats two empty beat cells as a perfect match because set_jaccard(empty, empty) returns 1.0. In a common 4/4 case with one sounded note on beat 1 and rests on beats 2–4, lag 1 gets enough empty-empty matches to clear PERIOD_THRESHOLD and returns a one-beat detected_subbar_period_ticks, even when the sounded cells do not repeat at any sub-bar lag. This will persist false sub-bar periods into v5 corpus metadata; the sub-bar pass should ignore or separately gate empty-empty cells so silence alone cannot establish a period.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed — real defect, fixed in dfa274f via red → green. build_score(&[vec[60], vec[67]]) reproduced it exactly as described (lag-1 mean 4/7 from empty-empty pairs → false Some(480)). The sub-bar pass now skips empty-empty pairs and averages over informative ones only, skipping lags with none; a characterization test pins that rest-aligned sparse repeats (X . X .) keep their half-bar period. Bar-level pass intentionally unchanged (a rest bar inside a repeated phrase is meaningful at that granularity). Decision recorded in docs/decisions.log.md.
Generated by Claude Code
…eriod One sounded beat per bar with non-repeating pitches: empty-empty beat pairs (set_jaccard of two empty cells is 1.0) make up 4/7 of the lag-1 pairs and clear PERIOD_THRESHOLD on rests alone, persisting a false one-beat period into v5 corpus metadata. Empty-empty pairs are uninformative at beat granularity and must sit out of the mean. A characterization guard pins the flip side: an X . X . bar still reads as a half-bar tile — the sounded cells carry the period on their own. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
…ex P2, PR #38) set_jaccard treats two empty cells as identical (1.0), so on sparse material (one sounded beat per bar) the empty-empty pairs alone cleared PERIOD_THRESHOLD at lag 1 and persisted a false one-beat period into v5 corpus metadata. The sub-bar pass now skips empty-empty pairs and averages over the informative ones only — silence may sit inside a tile but never establishes one; a lag with no informative pairs is skipped. Rest-aligned sparse repeats (X . X .) keep their half-bar period: the sounded cells carry it on their own. The bar-level pass is unchanged (a rest bar inside a repeated phrase is meaningful at that granularity). https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
|
@codex plz review changes Pushed the fix for the empty-beat-pair finding: empty-empty cell pairs are excluded from the sub-bar lag mean (silence never establishes a period), with a regression test and a characterization guard for rest-aligned sparse repeats. Generated by Claude Code |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ 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". |
…finement) measure_complexity derives the rhythmic / pitch / technical / harmonic / playability / structural vector ADR-0015 separates from length and period. v1 measures, each a fact in [0, 1]: inter-onset-interval and melodic-interval variety ((distinct − 1) / (count − 1)), the share of notes carrying a mark or sitting in a technique span, the off-scale share of the estimated key (1 − scale_fit), fret-travel difficulty on the optimal fingering path (1.0 when a line note is unreachable), and the distinct-bar-signature ratio shared with StructureMetrics. Measurement only — no corpus persistence yet (measure before target). References an API that does not exist yet, so the suite fails to compile until the green step. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
measure_complexity derives the rhythmic / pitch / technical / harmonic / playability / structural vector ADR-0015 separates from length and period. Untuned v1 measures, each a fact in [0, 1]: - rhythmic / pitch — normalised variety ((distinct − 1) / (count − 1)) of inter-onset intervals and of absolute melodic intervals along the highest-pitch-per-onset line; - technical — share of notes carrying a per-note mark or sitting inside a technique span (both ADR-0018 surfaces); - harmonic — off-scale share of the estimated key (1 − scale_fit); the S13 Krumhansl–Schmuckler estimator is now a pub(crate) seam taking (pitch, duration) pairs so both analyses share one implementation; - playability — fret travel on the optimal fingering path (ADR-0019), capped at an octave; an unreachable line note maxes the axis; - structural — the distinct-bar-signature ratio, the same fact as StructureMetrics::structural_complexity (bar_signatures is now a shared helper). Measurement only — corpus persistence and control-side targets are later increments (measure before target, ADR-0015). https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
…sions Measurement landed; persistence and control-side complexity targets stay deferred. The decisions log records the six v1 axis definitions, the shared-seam choices (key estimator, bar signatures), and what was rejected (same-increment persistence, axis weighting, a syncopation axis needing a grid-resolution knob). https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
|
@codex plz review changes New increment: S14 Generated by Claude Code |
The S14 complexity vector gets its first consumer: Analysis carries the focus track's measured ComplexityProfile next to the structure metrics (None for an empty score; the structural axis is the same fact as StructureMetrics::structural_complexity). References a field that does not exist yet, so the crate fails to compile until the green step. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
Analysis carries the focus track's measured ComplexityProfile next to the structure metrics (None for an empty score), and the TUI inspector renders it as a compact three-row block of abbreviated axis pairs (rhy/pit, tec/har, ply/str) so the dock keeps its transport block visible in short terminals — the vector's first consumer. The two golden frames are re-blessed deliberately for the new block, and the golden tests now honour GRIFF_BLESS=1 (the core characterization convention) instead of include_str!, so future intended UI changes re-bless the same way as core snapshots. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
|
@codex plz review changes New increment: the S14 Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 229caa3b23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| if a.is_empty() && b.is_empty() { | ||
| continue; | ||
| } | ||
| sum += set_jaccard(a, b); |
There was a problem hiding this comment.
Require full tiling before accepting sub-bar periods
For non-periodic cells with adjacent repeats, such as one 4/4 bar shaped A, A, B, B, this adjacent-pair autocorrelation reports a one-beat period: lag 1 gets two matching pairs out of three and clears the 0.5 threshold, even though a one-beat tile would require every beat to be the same. That writes a misleading detected_subbar_period_ticks into analysis/corpus metadata; the sub-bar pass needs to verify the lag classes tile consistently, not just average cell[i] vs cell[i + lag].
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 4cdedf4 (red → green). The A A B B reproduction failed exactly as described (lag-1 mean 2/3 → false one-beat period). The sub-bar pass is now a tiling test: a lag qualifies only when every aligned cell pair matches, shortest qualifying lag wins, and PERIOD_THRESHOLD drops out of the pass entirely — it stays the bar-level mechanism where graded similarity is the point. This also subsumes the earlier empty-pair rule (empty matches empty inside a tile, but any mismatch against a sounded cell vetoes the lag), so both prior regressions stay green; a second new test pins that one varied cell vetoes the tile (variation tolerance remains the bar-level repeatability's job). Decision recorded in docs/decisions.log.md.
Generated by Claude Code
A A B B clears the lag-1 threshold (two of three adjacent pairs match) and reports a one-beat period, though a one-beat tile requires every aligned cell pair to match — the documented verbatim-tiling contract. Pins both sides: A A B B yields no sub-bar period, and one varied cell anywhere vetoes the tile (variation tolerance is the bar-level repeatability's job). https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
…ean (Codex P2, PR #38) A lag now qualifies only when every aligned cell pair matches exactly, and the shortest qualifying lag wins: A A B B (two of three lag-1 pairs matching, mean 2/3 over the threshold) tiles at no sub-bar lag, and one varied cell anywhere vetoes the tile — variation tolerance is the bar-level repeatability's job. Empty cells match empty cells inside a tile, but any mismatch against a sounded cell vetoes the lag, so the earlier silence rule is subsumed: both prior regressions stay green. PERIOD_THRESHOLD and the pair mean drop out of the sub-bar pass entirely (they remain the bar-level mechanism, where similarity is the point). https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
|
@codex plz review changes Pushed the fix for the A A B B finding: the sub-bar pass is now an all-pairs tiling test (shortest qualifying lag wins) instead of a thresholded similarity mean; the earlier empty-pair rule is subsumed and both prior regressions stay green. Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e52d00e28d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| let tiles = (0..pairs).all(|i| cells.get(i) == cells.get(i.saturating_add(lag_cells))); | ||
| if tiles { |
There was a problem hiding this comment.
Require full tiling before accepting long sub-bar lags
For lags near the bar length this only checks the few aligned pairs that exist, so a 4/4 bar with beat cells A B C A reports a 3-beat sub-bar period because the single pair cell[0] == cell[3] passes. That is not a verbatim tile of the cell sequence, and it can persist false detected_subbar_period_ticks metadata for whole-bar material or sparse riffs whose first and last beat match. The lag should only qualify when it covers the whole sequence as a tile, e.g. by requiring the cell count to be divisible by the lag and comparing every cell against its modulo class.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 951a074 (red → green): A B C A reproduced the single-pair false positive exactly. Sub-bar lags now stop at half the cell count — the same evidence rule the bar-level pass has always applied via max_lag = n / 2: a period is a recurrence claim, so the tile must fit the span at least twice. Went with the half-count cap rather than divisibility + modulo classes: the modulo-class comparison is equivalent to the all-pairs test transitively, and divisibility would reject genuine tiles truncated by the bar count (the tile/vary compiler explicitly produces truncated final copies). Decision recorded in docs/decisions.log.md.
Generated by Claude Code
|
@codex plz review changes New increment: corpus schema v6 — Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77f10ceb0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| let spans: Vec<&TechniqueSpan> = track | ||
| .voices | ||
| .iter() | ||
| .flat_map(|v| &v.event_groups) | ||
| .flat_map(|g| &g.technique_spans) | ||
| .collect(); |
There was a problem hiding this comment.
Keep technique spans scoped to their voice
For multi-voice tracks, this collects TechniqueSpans from every voice once and then tests every note in the track against that global set. Since TechniqueSpan is defined as a tick range within a voice, an overlapping bend/legato span in voice 1 will make simultaneous plain notes in voice 0 count as technical too, inflating the persisted ComplexityProfile.technical during griff curate for polyphonic tracks. Iterate spans and notes per voice (or per event group) so only notes in the span's own voice are covered.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in a698a85 (red → green): a two-voice reproduction (four plain quarters in voice 0, one palm-muted note in voice 1) read 0.4 instead of 0.2 exactly as described. technique_share now scopes spans per voice — each voice's notes are tested against that voice's spans only. Kept per-voice rather than per-event-group scoping: a span's tick range legitimately outlives its anchor group (a palm-mute span covers the following notes of the same voice), so group scoping would undercount what the span explicitly states. Decision recorded in docs/decisions.log.md.
Generated by Claude Code
…oice technique_share collected spans from every voice into one global set and tested every note of the track against it, so a palm-mute span in voice 1 made a simultaneous plain note in voice 0 read as technical — inflating the persisted ComplexityProfile.technical on polyphonic tracks. One covered note of five must read 0.2, not 0.4. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
… PR #38) technique_share now scopes spans per voice: each voice's notes are tested against that voice's spans only, so a palm-mute or legato span in one voice no longer marks simultaneous plain notes in another voice as technical. The complexity axis stops inflating on polyphonic tracks; single-voice behaviour is unchanged. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
|
@codex plz review changes Pushed the fix for the cross-voice span finding: Generated by Claude Code |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ 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". |
…hema v6) The five per-axis complexity facts not already on the edge — rhythmic / pitch / technical / harmonic / playability — become five more 1 − |Δ| agreement axes under a uniform similarity v3 policy (15 axes). ComplexityProfile.structural is deliberately not an axis: it is the same fact as StructureMetrics.structural_complexity, already on the edge as complexity_similarity, and a duplicate axis would silently double-weight it (the variation_score rule). Measured now also requires the persisted profile: v5 records sit out as candidates and are rejected as queries until re-curated — the convention v2 set for gesture. References similarity_weights_v3 and the new axes, so the suite fails until the green step. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
Five 1 − |Δ| agreement axes over the persisted ComplexityProfile — rhythmic / pitch / technical / harmonic / playability — join the similarity vocabulary (15 axes), under the uniform similarity v3 policy. ComplexityProfile::structural stays off the edge: it is the same fact as StructureMetrics::structural_complexity, already there as complexity_similarity, and a duplicate axis would silently double-weight it (the variation_score rule). Measured now also requires the profile: pre-v6 records sit out as candidates and are rejected as queries until re-curated; the superseded v2 weights live in git history, not API (the v1 convention). https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
|
@codex plz review changes New increment: similarity v3 — the five complexity axes (rhythmic / pitch / technical / harmonic / playability) join the S7 similarity edge as Generated by Claude Code |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ 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". |
… record The interaction core (ADR-0016 layer 2) grows a curation decision: Intent::Approve / Intent::Reject set Viewport::decision (the same intent again is an undo, the other overwrites), the TUI maps 'a'/'x' and shows the pending decision in the inspector, and the new curation::decide_record seam writes the decision into a chunk record's reviewer field (Approve → Accepted, Reject → Rejected), leaving every other field untouched — re-curation may overwrite an earlier decision (the established healing path). The decision stays a UI-level enum so the interaction core keeps zero griff-core domain types (the ADR-0016 crate boundary). References API that does not exist yet, so the crate fails to compile until the green step. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
…he chunk record The interaction core gains CurationDecision and the Approve/Reject intents (the same intent again is an undo, the other overwrites) — a UI-level enum, so the core keeps zero griff-core domain types (the ADR-0016 crate boundary). The TUI maps 'a'/'x', shows the pending decision in the inspector, and run() returns the decision for the shell. curation::decide_record bridges to the S5 schema (Approve → Accepted, Reject → Rejected; every other field untouched, lossless round-trip carries them), and griff-preview --record=<chunk.json> persists it on quit. Goldens re-blessed for the curation line and the updated footer hint. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
|
@codex plz review changes New increment: S8 curation slice — Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66155d3325
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| let interval = pc | ||
| .saturating_add(12) | ||
| .saturating_sub(min_pc) | ||
| .checked_rem(12) | ||
| .unwrap_or(0); | ||
| intervals.push(interval); |
There was a problem hiding this comment.
Keep harmonic scale anchored to the inferred key
When register_offset is not octave-equivalent, these new harmony intervals are later added to band_lo by degree_to_pitch/PitchMaterial::root, but here they are measured from A's lowest pitch class. That transposes the inferred key by the register offset: for example, a C-major part shifted up a fifth gets G-major material (including F#) instead of the promised C-major scale, so the harmony enrichment can make RhythmLock/SupportLayer/CallResponse/CounterMelody emit off-key notes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in fb842a7 (red → green): a C-major part with register_offset: 7 reproduced the F# (pitch 78, pc 6) exactly as described. scale_intervals_from now takes the band floor and measures every offset from the band floor's pitch class, so the materialised pitch classes equal A's at any offset — the key is the anchor, the band only picks the octave. Went one step further than the report: A's literal pitch classes had the same flaw (masked by the fixtures' octave-only offsets), so the anchor rule covers the whole material, not just the harmony enrichment. Octave offsets are unchanged (the two anchors coincide mod 12), so all existing fixtures keep their pitches. Decision recorded in docs/decisions.log.md.
Generated by Claude Code
…-octave shift scale_intervals_from measures offsets from A's lowest pitch class but the consumers apply them from B's band floor, so a non-octave register_offset transposes the whole material with the band: a C-major part shifted up a fifth grows an F# (G-major material) instead of the promised C-major scale. The pitch classes are the anchor; the band only picks the octave. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
… pitch class (Codex P2, PR #38) scale_intervals_from now takes the band floor and measures every offset — A's literal pitch classes and the inferred key's scale alike — from the band floor's pitch class, so the materialised pitch classes equal A's regardless of the register offset: the key is the anchor, the band only picks the octave. Previously offsets were measured from A's lowest pitch and applied at the band floor, transposing the whole material with any non-octave shift (a C-major part offset a fifth grew an F#). Octave offsets are unchanged (the two anchors coincide mod 12), so all existing fixtures keep their pitches. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
|
@codex plz review changes Pushed the fix for the key-transposition finding: the substitution material is anchored to pitch classes via the band floor ( Generated by Claude Code |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ 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". |
The record block landed above the transport lines, so on a short terminal the static digest pushes the live play state out of the dock, regressing the PR #38 liveness ordering. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
…Codex P2, PR #42) The digest is static — loaded once at startup — so it belongs with the static blocks under transport: the PR #38 liveness ordering clips static tails, never the live play state. A blank separator joins the block, matching the other static sections. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
Codex P2 (#69): derive_techniques scanned track.voices.first(), but track_notes/technique_share (structure + complexity.technical) measure a track as a whole across every voice — so a chunk could count a secondary-voice technique in complexity.technical while its `techniques`/tags metadata omitted it. Scan every voice to match, the same voice-handling decision reached in #38. Flips the voice test to assert a secondary-voice technique IS derived. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
Two roadmap increments, each as a red → green → docs TDD cycle.
S13 — harmonic context closes the stage backlog
analyze_partestimates the part's key with the Krumhansl–Schmuckler algorithm (Krumhansl–Kessler profiles correlated against the duration-weighted pitch-class histogram) and carries it asPartProfile::harmony { tonic_pitch_class, mode, scale_fit }—scale_fitis a fact, not a verdict.S14 — sub-bar (beat-level) period detection
StructureMetrics::detected_subbar_period_ticks: the strongest verbatim-tiling lag shorter than one bar, via exact per-beat cell autocorrelation on uniform timelines (the bar-level rhythm floor and transposition credit are degenerate at one-onset granularity — see decisions.log).ChunkMeta: corpus schema bumps to v5; pre-v5 structure blocks load the field asNoneand re-serialize losslessly.Both passes are pure and deterministic; prior art recorded in
docs/decisions.log.md.https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b