Skip to content

feat(s1): MIDI import/export with bar-aligned phrases Adds griff-core::midi with import/export/summarise over midly 0.5. Tracks are reconstructed into Bar-grouped Phrase values; tempo and time-signature metadata are threaded through from global MIDI events. CLI gains three subcommands: import (one-line summary per track), inspect (bar-by-bar dump), export (roundtrip write). All paths, error types and the main entry-point satisfy the strict clippy policy (no process::exit, no &PathBuf args, no absolute paths in bodies). 40 tests pass; cargo fmt and cargo clippy --all-targets -D warnings are both clean. - #3

Merged
PhysShell merged 3 commits into
mainfrom
claude/music-core-cli-setup-C2zXh
May 19, 2026

Conversation

@PhysShell

Copy link
Copy Markdown
Owner

No description provided.

PhysShell and others added 3 commits May 19, 2026 03:00
Add core validation, generation, slicing, feature extraction, and workspace metadata
Adds griff-core::midi with import/export/summarise over midly 0.5.
Tracks are reconstructed into Bar-grouped Phrase values; tempo and
time-signature metadata are threaded through from global MIDI events.

CLI gains three subcommands: import (one-line summary per track),
inspect (bar-by-bar dump), export (roundtrip write).  All paths,
error types and the main entry-point satisfy the strict clippy policy
(no process::exit, no &PathBuf args, no absolute paths in bodies).

40 tests pass; cargo fmt and cargo clippy --all-targets -D warnings
are both clean.
@PhysShell
PhysShell merged commit 0486936 into main May 19, 2026
PhysShell pushed a commit that referenced this pull request Jun 3, 2026
The follow-up P2 commit re-blessed both 80x20 terminal goldens by hand because
its environment could not fetch crates (ratatui CONNECT 403), so the tests never
ran. The guessed frames were one column off on the section-band row: under the
new ceiling-division `fit`, the Solo section marker shifts by one column, which
the blind re-bless did not capture, and both characterization tests failed.

Regenerated both goldens from the actual rendered output and verified the full
preview suite (50 passed) and `cargo test --workspace` green, with fmt/clippy
clean. The code fixes themselves (#1 right-edge note skip, #2 ceiling fit, #3
all-voice bar classification) were correct and compile; only the goldens were
wrong.

https://claude.ai/code/session_016RxVPiPMnFjyV8HmepFWAT
PhysShell pushed a commit that referenced this pull request Jun 3, 2026
The P2 #3 fix added an all-voice bar-feature aggregator, but it lived in
preview/src/analysis.rs as a hand-rolled copy of griff_core::classify's
single-voice bar_features_in_range — duplicated core logic that would drift if
BarFeatures/classify_bar changed.

Move the aggregation into griff_core::classify as `bar_features_across_voices`
(the shared accumulator) and make `bar_features_in_range` a thin single-voice
wrapper over it, so there is one implementation. The preview now calls the core
function; its redundant module-level arithmetic-side-effects allow and the
BarFeatures/TickRange imports are dropped. Characterize the new function in
core/tests/classify_canonical.rs (multi-voice aggregation, single-voice
equivalence with the wrapper, and the silent-first-voice case the fix closes).

Record the merge-then-classify choice (over per-voice-then-reduce) in the
decisions log. Full workspace tests, clippy -D warnings, and fmt are clean.

https://claude.ai/code/session_016RxVPiPMnFjyV8HmepFWAT
PhysShell added a commit that referenced this pull request Jul 18, 2026
…eplace seam (3B2)

Corrective for the PR #139 review. Three narrow gaps, as failing/added tests:

1. A directory-fsync failure AFTER a successful rename is currently reported as
   a pre-commit CurationFsError::Io, whose contract promises the old store
   survived — but the new bytes are already published. New variant
   PostCommitDurability { source } and a FailPoint::AfterReplace; the test wants
   that typed error, the new bytes on disk, the temp gone, and the error
   distinguishable from a pre-commit Io.

2. A bare relative store path (curation.json) has an empty parent, which the
   sync path filtered out, so the Unix directory fsync never ran. New
   parent_for_sync maps an empty parent to "."; the test pins
   curation.json -> ., dir/curation.json -> dir, /dir/curation.json -> /dir.

Both new items are wired as scaffolding with deliberately-wrong bodies
(AfterReplace falls through to a normal Ok; parent_for_sync keeps the empty
parent), so the two tests fail. GREEN adds the post-commit path, the real
replace_file seam (so FailPoint::DuringReplace exercises the actual
fs::rename error branch, corrective #3), and fixes parent_for_sync.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM
PhysShell added a commit that referenced this pull request Jul 29, 2026
…-circuit

Regressions for the second exact-head re-review (fail against 6375e8b):

#1 the whole artifact JSON graph must reject foreign fields, not only the plan
   root — rogue fields planted at every depth:
   - ledger_rejects_unknown_field_at_root / _in_batch / _in_event
   - ledger_rejects_unknown_field_in_action_payload (proves the *actual* serde
     version denies inside an internally-tagged variant, not just the attribute)
   - ledger_rejects_unknown_field_in_split_target
   - plan_rejects_unknown_field_in_embedded_batch
   (plan_rejects_unknown_field_in_assignment already passes — Assignment is
   strict — kept as boundary coverage.)

#2 verify_plan must refuse a structurally invalid embedded batch *before* digest,
   fingerprint, or replay:
   - verify_plan_short_circuits_on_invalid_batch asserts only the structural
     refusal is returned, never UnknownDecisionSource / PlanDigestMismatch /
     DecisionDigestMismatch / DecisionProjectionMismatch.

#3 an intra-batch duplicate event_id must be emitted exactly once:
   - intra_batch_duplicate_event_id_emitted_once (validate_ledger currently
     double-counts it via validate_batch + its own pass).

Logic unchanged, so these are RED.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
PhysShell added a commit that referenced this pull request Jul 29, 2026
#1 strict JSON across the whole artifact graph. deny_unknown_fields now also on
   DecisionsLedger, DecisionBatch, DecisionEvent, Action (internally tagged; the
   serde 1.0.229 in the lockfile denies rogue fields inside a variant payload
   while still accepting the `kind` tag — proven by
   ledger_rejects_unknown_field_in_action_payload), and SplitTarget. A foreign
   field at any depth is now rejected, not silently discarded before digesting.

#2 verify_plan short-circuits a structurally invalid embedded batch:
   validate_batch(&plan.decision_batch)? runs first, so an invalid ordinal or a
   duplicate event_id refuses before any digest, fingerprint, or replay — the
   removed mid-function validate_batch no longer let those run.

#3 an intra-batch duplicate event_id is emitted exactly once. validate_batch's
   positional check is split out (batch_order_errors); validate_ledger runs that
   per batch plus a single ledger-wide event_id uniqueness pass, so a duplicate
   (intra- or cross-batch) is reported once, not twice. Standalone validate_batch
   keeps its ordering + intra-batch-uniqueness contract.

Includes an incidental rustfmt pass over the RED tests added in the previous
commit. 45/45 tests pass; clippy (all=deny, pedantic=warn) and fmt clean under nix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
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.

2 participants