Repository navigation
SWG-CORE-01: the three canonical index fields are u64 — H3 closed - #188
Merged
Merged
Conversation
…ex fields Characterization, tests-only, green before the migration. The backlog exempts characterization from the red phase; these must pass now and keep passing after, and must not be edited to make the migration pass. Thirteen tests across two crates pinning what the fields do today: - a stored `MasterBar.index` is not derived from its vector position (H4), and the exact diff reports a change to it as exactly one difference; - the normalized projection copies the stored index and does not renumber; - warning payload indices are payload — a `track_index` of 7 in a score with no tracks stays 7, and nothing resolves it against the tree; - the exact writer spells the stored index in full, with a byte-golden. The load-bearing ones are the `>u32::MAX` witnesses. They are what turns the width argument from prose into a check: a stored index above `u32::MAX` is **already inhabited** on a 64-bit host, survives the exact diff whole, survives the projection, and is written whole. So `u32` would not merely remove a platform dependence — it would shrink a canonical domain that already has values in it, and `MasterBar.index` is an exact fact of its own rather than a bounded ordinal. Those witnesses sit behind `cfg(target_pointer_width = "64")`, and the `cfg` is the defect itself: the same score cannot be built on a 32-bit target while the field is `usize`. That is precisely the portability claim H3 calls false. Recording it before the migration means the change can be shown to have moved the field's width without moving the writer's bytes for any value that already fit. Refs SWG-CORE-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
Tests-only. No `src/` change: the whole point is that the contract cannot be satisfied by the current declarations. ## Two kinds of red, deliberately `core/tests/canonical_index_contract.rs` is red by **failing to compile**, which is the correct failure mode for a type contract. `usize` and `u64` are distinct types in Rust even where they are the same size, so `require_u64` on each of the three fields cannot pass on any target today and cannot be accidentally satisfied on a 64-bit host tomorrow. Six errors, each naming a field the migration must move. It is a separate file from step A on purpose. A red file that also held the green characterization would make "the characterization was green before and after" unverifiable at exactly the moment it matters — and `canonical_index_width` does still compile and still passes 9/9 in this commit. The contract also states, twice and without a `cfg`, that a value above `u32::MAX` is representable **on every target**. That absence of a `cfg` is the deliverable: step A could only assert the same value behind `target_pointer_width = "64"`, which is precisely the portability claim H3 called false. One negative clause: `nearest_micros` is not an index and stays `u32`. Written down because the migration passes right by it, and "widen the integers near the ones being widened" is the cheapest way for a mechanical change to grow a scope nobody gave it. ## The backlog's "no `usize` returns" witness Extends the existing `exact_text_census` rather than adding a second scanner. `declared_field_types` discovers every `pub struct` / `pub enum` in `core/src/score.rs` instead of taking a hand-written list, so a canonical type added later is covered without anyone remembering to add it — and it reads enum payload lines, which no struct-field scan reaches. Three tests: no field in the canonical tree declares `usize`; the three migrated fields are specifically `u64` (the general witness would stay green if one were widened to `u128`); and the scanner itself is falsified against a synthetic source, because a scanner that silently found nothing would make both witnesses vacuously green — the failure this suite has already hit twice. `mentions_type` matches whole tokens, so `MyUsizeAlias` is not a `usize`. Refs SWG-CORE-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
… u64 Closes H3. `MasterBar.index`, `ImportWarning::TrackNameInvalidUtf8.track_index`, and `ImportWarning::TempoApproximated.bar_index` go from `usize` to `u64`, so a score written on a 64-bit host means the same thing on a 32-bit one. Spec §1.2 forbids platform-sized integers in hashed or serialized state and `Score` derives `Hash`; until now the portable exact format was not portable. **u64, not u32**, and the reason is semantic rather than defensive. The fields are public, values above `u32::MAX` are already inhabited on a 64-bit host, and `MasterBar.index` is a stored exact fact that need not agree with its vector position (H4). `u32` would not merely have removed a platform dependence — it would have shrunk a canonical domain that already has values in it. Step A's witnesses are what make that a check rather than a claim. ## `index_from_ordinal` One new public function in `core::score`, and the only sanctioned way to turn a vector position into a canonical index. Importers legitimately derive the second from the first; the point is that every such crossing is now named and greppable rather than implicit. It is total and lossless — `usize` is at most 64 bits on every target Rust supports — and there is deliberately no inverse, because the inverse is not total and must not be written by accident. Twenty call sites, all of them constructors that were already numbering bars from an `enumerate()`. ## Three types the migration had to pull along Not scope creep — the alternative at each is a truncating `u64` → `usize` conversion on a 32-bit target, which is the defect being fixed: - `dump::NormBar.index` copies the stored index into a `Serialize` type; - `SemanticPathSegment::MasterBar.index` and `::Bar.index` annotate diff paths with the stored index. The `ordinal` beside each is an operational position and stays `usize` — the two sit in the same struct and now have visibly different widths, which is the correct shape; - `ui_core::analysis::Section.bar_start` / `.bar_end` are copied straight from `MasterBar.index`; `bar_count()` follows them to `u64`. ## What did not change No operational `usize` was touched: `Vec` positions, lengths, and slice indices are the same as before, including MIDI's `MAX_MASTER_BARS` resource bound, which is an operational limit and not an argument about canonical width. `nearest_micros` stays `u32` and step B has a test saying so. No index became derived from a position, no warning payload became a lookup, no range invariant was added, and nothing truncates, saturates, or clamps. `cli::bar_at_tick` gains a comment rather than a fix: its two branches return a stored index and a last ordinal respectively, a pre-existing conflation that this task only makes same-width, not correct. It is display-only. ## Steps A and B Step A's characterization is green, and the only edits to it are fixture parameter types — every assertion is byte-identical to the pre-migration commit. Its `#[cfg(target_pointer_width = "64")]` gates are gone, because the migration is exactly what removed their reason to exist: those four assertions now run on every target rather than on the one where `usize` happened to be wide enough. Step B's contract compiles and passes, including the two `>u32::MAX` cases that never had a `cfg`. Verified: 1339 tests green across core, swang, pattern, cli, and ui-core; clippy `-D warnings` and `cargo fmt --check` clean workspace-wide, cockpit, preview, and plugin included (type-checked with `--all-targets` against a stub pkg-config entry, since libasound is absent here). The two `fuzz/` targets that build a `MasterBar` are edited identically to sites that do compile, but `fuzz/` pins nightly and could not be built locally — CI covers it. Refs SWG-CORE-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
…tion Found by the migration's own falsification pass, not by review. Replacing `index_from_ordinal`'s body with `(ordinal as u32) as u64` left all 1339 tests green: the function is the one place a silent narrowing could hide, and nothing reached it, because reaching the truncation through a real `Vec` needs four billion elements and no test can build one. Calling the conversion directly does reach it — its input is a `usize`, not a collection — so the guard passes an ordinal above `u32::MAX` and checks the value survives whole. Verified to fail under exactly that mutation and to pass without it. Gated on `target_pointer_width = "64"`, and this gate is not the kind step A removed: on a 32-bit target the offending input does not exist, so there is nothing to truncate and nothing to check. The gate marks where the hazard lives rather than where the model happens to fit. Refs SWG-CORE-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
The second hole from the same falsification pass, and the same shape as the first: a narrowing hidden somewhere the tests never walked. Every other contract test builds an `ImportWarning` and reads it straight back. None went through `LossReport::add`. So clamping `bar_index` to `u32::MAX` inside `add` — the one function every importer calls, and none of the tests did — left all 1339 green. This checks the importer's path: both payload variants go in through `add`, come back out whole, and survive `absorb` as well, since that is the other door into a report. Verified to fail under exactly that clamp and to pass without it. Two holes from one pass, both of the same family: the migration itself was consistent, and what the suite lacked was a witness at the *seam* — one for the conversion no `Vec` can reach, one for the constructor path the fixtures bypassed. Worth stating plainly, because "the tests were green" was true both times and meant nothing either time. Refs SWG-CORE-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
Docs-only. Records what the migration settled, in the three places that were still describing the world before it. ## `exact-score-text.md` (normative) - Inventory B: `MasterBar.index` is `u64`, and its "platform-sized (H3)" malformed note is gone because the condition it named no longer exists. Both `ImportWarning` payload shapes lose their `← H3` markers and gain `u64`; - §2.8: `<index>` is a `u64` written in decimal, fixed-width, so the same document means the same thing on a 32-bit target. It is still not an ordinal — the pointer to H4 stays, because that is the fact people get wrong; - H3 becomes **CLOSED by SWG-CORE-01**, kept rather than deleted: the width is a normative fact and the reasoning behind it constrains what a later change may do. It now states why `u64` and not `u32` — values above `u32::MAX` were already inhabited, and a stored index is an exact fact rather than a bounded ordinal, so `u32` would have shrunk the model rather than merely making it portable — and names the two things easiest to undo: `index_from_ordinal` is the one sanctioned widening with no inverse, and operational `usize` was never part of the argument; - the "until then the portability claim is false" clause is gone, because it is no longer true. Level 2 stays *allocated*: closing H3 removed the portability objection to freezing, not the phase-acceptance requirement. H4 is unchanged in meaning, deliberately. The stored index still need not equal its position, and that is now the only surprising thing about it. ## `foundation-backlog.md` (non-normative) CORE-01 marked done with its outcome and its acceptance criteria checked off, including the witness that fails if a `usize` reappears — named, because "a witness test" is not a criterion until you can open it. 4A-05 is recorded as unblocked and as the next task on the writer lane. The width itself is stated here only as a back-reference; the normative record lives in H3, so the two cannot drift into disagreeing. ## `decisions.log.md` One Y-statement, because the backlog defined the width as this scope's decision rather than the grammar's. It carries the falsification result too: two of nine mutations survived the original suite, and saying so is more use to a later reader than a clean number would be. Refs SWG-CORE-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
…ee things Tests-only, all three red at runtime, and the 13 existing census tests keep passing beside them. Found by independent review of the closure, not by the migration's own falsification pass — which is why these land as continuation evidence rather than as edits to the six commits already made. The production migration is not implicated. What is wrong is the *acceptance witness* for it, and the backlog has already recorded that criterion as met in its strong form. ## 1 and 2 — the scanner cannot see tuple forms `declared_field_types` requires a `:` on the line, so a tuple enum payload is skipped entirely: `Other(String)` today, and a `Tuple(usize)` tomorrow. One level up, a tuple *struct* has no braced body, so the header match never fires at all — `pub struct Pitch(pub u8);` is invisible, and §2.9 lists exactly those newtypes as canonical leaves. Meanwhile the witness's own doc comment claims coverage "including inside an enum payload". For tuple payloads that is false. The synthetic falsification did not catch it because it used only a named payload. A witness written against vacuous witnesses turned out to be a little vacuous itself, which is at least consistent. ## 3 — the witness reads one file It scans `core/src/score.rs`. `Score` also reaches `TimeSignature`, `Tuning`, `NoteMarks`, `NotePosition`, `FretboardPosition`, `TechniqueEvidence`, `ConfidenceBps`, `Pitch`, `Velocity`, and `Ticks` in `core/src/event.rs`, and `TickRange` in `core/src/slice.rs`. "No `usize` remains in any type reachable from `Score`" may well be true; this witness does not show it. The failing test names the defect in the only place it can be named — the input expression. It is written as the exact call the witness makes today, so the repair replaces that one line with the aggregate and leaves the assertions untouched. Refs SWG-CORE-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
…t claims Tests-only. All three review-red tests pass; 16/16 census tests green. ## The scanner reads tuple forms `declared_field_types` gains two branches: a one-line `pub struct Name(T, U);` header, which had no braced body for the old scan to find, and a `Variant(T, U)` line inside an enum body, which carried no `:` and so was skipped element by element. Both key positionally — `Other.0`, `Pair.1` — the way `enum_payload_fields` already keys tuple payloads, because `Other(String, usize)` is two facts and a scanner that reports one is the whole problem. `tuple_arity` is now `tuple_elements(..).len()`. One splitter serves both the count and the types, so they cannot disagree about where an element ends, and `the_payload_parser_counts_tuple_elements` keeps guarding it unchanged. `without_visibility` strips `pub` and `pub(crate)` so a width is one declaration rather than three. ## The witness reads every module the tree reaches `CANONICAL_SOURCES` lists `score.rs`, `event.rs`, and `slice.rs`, and `canonical_field_types()` aggregates them. Both the no-`usize` witness and the three-fields witness now run over that set instead of `score.rs` alone. The list is written out rather than discovered. Discovering it means resolving `use` paths and type aliases — a half-compiler to check three modules, which is the temple-of-checkers this suite was told not to build. The cost of writing it is that an entry could rot, so the coverage test asserts each listed module actually contributes. ## Falsification Five mutations, each caught by the test that names it: ``` usize in a braced struct in event.rs -> no_platform_sized_integer usize in a tuple enum payload, score.rs -> no_platform_sized_integer usize in a tuple struct in slice.rs -> no_platform_sized_integer source list narrowed back to score.rs -> scans_every_module a listed source that declares nothing -> scans_every_module ``` The first two attempts at the event.rs and slice.rs probes were inconclusive and I nearly recorded them as such: inserting a type *before* an existing one detached its `#[derive(…)]` block, so the crate failed to compile for a reason that had nothing to do with the witness. Appending before the test module instead makes them conclusive. The mutation harness also reported "not caught" three times today when the truth was "did not compile" or "did not apply" — `grep '^error'` matches cargo's own "error: test failed". Every result above comes from a run with a diff guard on the mutated file and a compile check that distinguishes `error[E…]` from a failing assertion. Refs SWG-CORE-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
Two things the closure recorded were not true. ## The mutation count `decisions.log.md` said "nine mutations". The standalone-verified figure is **twelve**. Corrected in place rather than left to propagate — which the entry immediately above it in the same file is about, so getting it wrong here would have been a particularly poor joke. ## The witness description The backlog said the acceptance witness "discovers the declarations rather than taking a list". It did discover declarations, but only inside one file, and it could not see tuple forms at all. Both halves are repaired in the two commits before this one; the description now says what the witness does: three named modules, named fields plus tuple enum payloads plus tuple structs, with a coverage test that fails if a listed module stops contributing. ## The review round itself A second Y-statement records it, because "acceptance criterion met" was written down on evidence that could not carry it. The distinction matters and is stated plainly: the gap was in the witness, not in the migration — the tree had no `usize` left either way, and the witness simply could not have shown it. Five further mutations confirm the repair, and the reason the module list is written out rather than discovered is recorded with its cost, so the next person can weigh it instead of rediscovering the argument. Refs SWG-CORE-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes H3.
MasterBar.index,ImportWarning::TrackNameInvalidUtf8.track_index,and
ImportWarning::TempoApproximated.bar_indexgo fromusizetou64, so ascore written on a 64-bit host means the same thing on a 32-bit one. Spec §1.2
forbids platform-sized integers in hashed or serialized state and
Scorederives
Hash; until now the portable exact format was not portable.Please do not squash. Nine commits in three groups:
5d618fe65b35a6c5800f7277364adb5f6ac8aeacd3f02925e0e8fa6b5705ae8u64, and why not u32
Semantic, not defensive. The fields are public, values above
u32::MAXwerealready inhabited on a 64-bit host, and
MasterBar.indexis a stored exactfact that need not agree with its vector position (H4) — not a bounded ordinal
that could be argued into 32 bits.
u32would not merely have removed aplatform dependence; it would have shrunk a canonical domain that already has
values in it.
Step A's
>u32::MAXwitnesses are what make that a check rather than a claim.They sat behind
cfg(target_pointer_width = "64")while the fields wereusize— the gate was the defect — and the migration removed the gate'sreason to exist, so the same assertions now run on every target.
index_from_ordinalOne new public function, the only sanctioned way to turn a vector position
into a canonical index. Total and lossless; deliberately no inverse, because
the inverse is not total. Twenty call sites, all constructors that already
numbered bars from an
enumerate().Three types pulled along
Not scope creep — the alternative at each is a truncating
u64→usizeon a32-bit target, which is the defect being fixed:
dump::NormBar.index(aSerializetype),SemanticPathSegment::MasterBar.indexand::Bar.index,and
ui_core::analysis::Section.bar_start/.bar_end. Theordinalbesideeach path annotation stays
usize; the two now sit in the same struct withvisibly different widths, which is the correct shape.
What did not change
No operational
usize:Vecpositions, lengths, slice indices, and MIDI'sMAX_MASTER_BARSresource bound are untouched.nearest_microsstaysu32and a test says so — "widen the integers near the ones being widened" is the
cheapest way for a mechanical change to grow a scope nobody gave it.
cli::bar_at_tickgains a comment rather than a fix: its two branches returna stored index and a last ordinal respectively, a pre-existing conflation this
task only makes same-width, not correct. Display-only.
Two holes the falsification pass found
Ten of twelve mutations were caught immediately. Two were not, and both were
the same shape — a narrowing hidden at a seam no test walked:
index_from_ordinalcould truncate throughu32and the whole suitestayed green. Reaching the truncation through a real
Vecneeds fourbillion elements; calling the conversion directly reaches it, because its
input is a
usizerather than a collection.LossReport::addcould clamp a payload index and the whole suitestayed green. Every test built an
ImportWarningand read it straight back;none went through the one function every importer calls.
277364aanddb5f6acclose them, each verified to fail under exactly themutation that motivated it.
The review round
An independent review then found the acceptance witness weaker than the
criterion it had been recorded against. Two defects, neither in the migration:
core/src/score.rsonly, while the tree also reachesevent.rs(TimeSignature,Tuning,NoteMarks,NotePosition,FretboardPosition,TechniqueEvidence,ConfidenceBps,Pitch,Velocity,Ticks) andslice.rs(TickRange);Other(String)was skippedfor want of a
:, andpub struct Pitch(pub u8);has no braced body forthe header match to find. Its own doc comment claimed coverage "including
inside an enum payload". The synthetic falsification used a named payload
and so did not notice: a witness against vacuous witnesses that was itself
a little vacuous.
f02925ereds all three,0e8fa6brepairs them. The tree had nousizelefteither way — what was missing was the ability to show it.
The module list is written out rather than discovered. Discovering it means
resolving
usepaths and type aliases, a half-compiler to check three files;the cost is that the list can rot, so the coverage test asserts each listed
module contributes.
Falsification, in full
Seventeen mutations, each caught by the test that names it:
One caveat on method, since it bit three times: a batch harness reported "not
caught" when the truth was "did not compile" or "did not apply" —
grep '^error'matches cargo's own "error: test failed", and two probes brokethe crate by detaching a
#[derive(…)]block rather than by anything to dowith the witness. Every result above is from a run with a diff guard on the
mutated file and a compile check that distinguishes
error[E…]from a failingassertion.
Verification
1344 tests green across core, swang, pattern, cli, and ui-core. Clippy
-D warningsandcargo fmt --checkclean workspace-wide, cockpit, previewand plugin included — type-checked with
--all-targetsagainst a stubpkg-config entry, since libasound is absent in this environment. Rustdoc
produces the same 15 warnings as the base
d7012f5; none are new.fuzz/pins nightly and could not be built locally. Its two targets thatconstruct a
MasterBarare edited identically to sites that do compile, andCI is the real gate for them.
🤖 Generated with Claude Code
https://claude.ai/code/session_018vdRzztKXy8bA16tEzwgLE
Generated by Claude Code