`epiphany-core/DECISIONS.md` had carried two Push-4a audit claims as
**unverified** through two passes, with the standing note that they "should be
checked, not inherited". Checked. Both are real, and both are Chapter 4 defects
standing in front of Push 4b rather than inside it -- which is the point of
checking before scoping, not after.
**P13-S5 -- the JI prime basis is specified at two lengths.**
`req:pitch:ji-vector-basis` says the built-in JI spaces order primes ascending
*starting with 2*, and that `components.len()` MUST equal the basis size. The
built-in pitch-space table calls `ji-5limit` "Two-dimensional (prime axes 3,
5)", `ji-7limit` three-dimensional, `ji-11limit` four-dimensional -- each
exactly one short, consistently, because the table is octave-reduced and the
requirement is full-register. `req:tuning:builtin-tuning-catalog` makes the
table normative, so a 5-limit vector is required to be both length 2 and length
3. The requirement's own octave-reduction clause does not reconcile them: it
normalizes the first component to a canonical range, it does not remove it.
Same family as P13-S2 and P13-I1, and cheaper -- a one-side edit once ratified.
**P13-S6 -- no built-in tuning system's resolution is pinned to a versioned
definition, and 14 of 20 have no definition at all.** The claim as inherited
("the named historical tunings lack exact deterministic ratio data") understates
it. It is not a missing table: `TuningResolution::Function` delegates the
historical temperaments to a `TuningFunctionId`, which Chapter 10 lists as an
*extension point*, and no built-in is mapped to a function id nor any function
id pinned. Only the six `tet-*` entries are actually specified, by
`EqualTemperament`'s structural rule. The other 14 are names: three meantone
variants, `werckmeister-iii`/`iv`, `vallotti`, `kirnberger-ii`/`iii`,
`young-ii`, `pythagorean` (the 3:2 ratio is named, the fifth-chain construction
and wolf placement are not), three `ji-static-5limit-*`, and
`ji-adaptive-5limit`. Set against `req:tuning:tuning-resolution-determinism`,
which requires determinism *across platforms*, two conforming implementations
may each choose a different published Werckmeister III and both pass -- in a
project that runs a cross-implementation conformance step.
The fix pattern is already in-house and was found by looking for it:
`req:pitch:spelling-algorithm` pins `SpellingAlgorithmId "default"` at version 1
to a named algorithm and errors on any other identifier. Nothing equivalent
exists for tuning. Which published variant each built-in denotes is a
ratification question, not an implementation one.
Both are filed open in the Batch-3 ledger. Neither was needed for 4a; both are
load-bearing for 4b, whose whole job is resolving a scale position to a
frequency.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`cmn-24` was declared in the built-in pitch-space table as "CMN extended with
24-EDO quarter-tone accidentals" while Chapter 2 fixed a `Cmn` position's
absolute semitone at `nominal.chromatic() + alteration + 12*octave` for every
space, unqualified. Two ratified MUSTs, each correct in its own chapter,
jointly unsatisfiable -- P13-I1's two-listings drift in another costume.
The tracker parked this as "a data-model major". It is not.
`req:binfmt:frozen-layout` names `PitchSpacePosition` as one of four open
value-layer vocabularies, so three of the four available options cost no schema
event at all. The one that *is* a major -- regrading `alteration` onto a finer
fixed unit -- is also the only one that rewrites the canonical bytes of every
pitch ever authored, and it buys a single grid while `edo-31`/`53`/`72` stay
unrepresentable. It is explicitly ruled out.
Ratified instead: a `Cmn` alteration and a `CmnChromatic` modification are
denominated in steps of the enclosing pitch space's chromatic layer. One rule,
not a special case for one catalog row -- it settles `cmn-24`, `maqam-base`,
and `PitchSpaceModification::CmnChromatic` together. `cmn-12`'s step is the
semitone, so every score in existence keeps its meaning and **not one canonical
byte moves**; `canonical_pitch_bytes` writes the space id before the position,
so a `cmn-24` E-half-flat and a `cmn-12` E-flat already derive different
`PitchId`s.
The defect was larger than `cmn-24`. Nothing bound a `Cmn` position to a
12-chromatic space: `ScalePosition { space: "edo-31", position: Cmn { .. } }`
constructed, validated, encoded and transposed, and meant nothing. Neither
`Pitch::transposed` nor `twelve_tet_semitone` ever read the space. Until Push
4b resolves `PitchSpaceId` to a `DiatonicOverChromatic` structure, both now
fail closed outside provable built-in `cmn-12` -- a capability check, not a
claim that the identifier defines the structure. False refusal for a
score-defined 12-chromatic space is accepted over silently wrong arithmetic.
No wire change. `TransposeRefusal::PitchSpaceUnavailable` is a new *diagnostic*
mapping to the existing `PreconditionFailureReason::PitchSpaceMismatch` (6);
appending a discriminant for a guard Push 4b must delete would have reserved a
permanent wire value for a temporary mechanism, in exactly the artifact the
fail-closed ruling exists to protect. Assignments 10-15 are untouched.
Operation Catalog 0.9.0 records the broadened case *and* amends the 0.8.0
rationale, which asserted that discriminant 6 never consults a pitch-space
registry -- true of the non-`Cmn` check, false of this one.
Two spec-side fixes found in review, neither visible to any test: the
`TranspositionInterval` listing had lost its `pub chromatic_steps: i32,`
declaration to a comment rewrite, leaving a one-field struct above the sentence
"Both components are load-bearing"; and `pitch.rs` still documented
`alteration` as "in semitones", contradicting the requirement landing beside
it. Both new requirements are now cited from the code that implements them --
the label checker catches cited-but-undefined, not the reverse.
Push 4b keeps the registry work and loses the contradiction. P13-S4 is the last
item open in Batch 3.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
168 of core_spec's 207 requirement blocks carried no `\label`, so no conformance
claim could cite them. All 207 are labelled now; the suite is 277/277.
Labelling alone would not have delivered a citable requirement. **No document in
the suite numbered its requirements.** `\newtcolorbox{requirement}` had no
counter, so a `\label` inside one bound to the enclosing sectioning unit and
`\ref` rendered a *section* number: core_spec said "see Requirement 2.5.4" where
2.5.4 is a subsubsection, and 61 of its 207 requirements shared a rendered number
with another -- one number, 5.6.3, was shared by six. Adding 168 labels to that
scheme would have produced 168 citable-but-ambiguous references. All six documents
now carry a real counter, numbered within chapter, and the box title shows it, so
a reader can see which requirement they are looking at. 277 labels, zero
collisions.
The counter is stepped with a `code=` key rather than tcolorbox's own
`auto counter`, and that is not a style choice. `auto counter` steps its counter
for `\label` purposes inside an internal `\sbox`, and `\refstepcounter`'s effect
on `\@currentlabel` is a local assignment discarded when that box closes --
before a `\label` written in the box body ever runs, which is how every
requirement in this suite is labelled. Measured on a three-box test document:
titles rendered `1.1 1.2 1.3` while the three refs resolved to `1.1 1.1 1.2`. The
idiomatic form would have shipped 207 silently wrong cross-references under
correct-looking numbers.
`requirement_labels.rs` locks all of it: every requirement block in every
`spec/*.tex` carries a label; labels match `req:<area>:<slug>`; the area matches
the chapter; labels are unique suite-wide; no `req:*` string cited anywhere in the
repository is undefined; and the counter is stepped where the label can see it --
a regression lock, because reverting to `auto counter` leaves every other check
green while the references break.
The citation check needed an escape. It cannot tell "cite this requirement" from
"name a label that does not exist", and documenting a dangling label is a
legitimate thing to do -- it had already rewritten a scoping plan's prose into a
euphemism to make itself pass. `DISCUSSED_NOT_CITED` carries the one such string
with its reason.
That string was the pass's other finding. `req:layoutir:vertical-bands` was cited
twice in the Pass-12 log and never existed. It should not be repointed at the two
*ownership* requirements: those govern which band a primitive belongs to, while
both entries describe the inter-staff solve realizing a band's declared *height*,
which no requirement governs at all. That is why the log invented a name. Both
citations now say so, and the gap is filed as P13-S4 -- shipped behaviour with no
governing requirement.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Thirteen drift-locked document vectors -- four accept, nine reject, one per
implemented rejection class -- gated as conformance step [7e] beside [7d]'s 65
decode vectors.
The checker distinguishes three outcomes, and that distinction is the whole
point. `Rejected`, `AcceptedCanonical`, and `AcceptedNonCanonical` are separate
verdicts, and every one of the four wrong pairings is a failure: an accept vector
that merely normalizes fails, and a reject vector that is accepted -- whether as
canonical or after normalization -- fails. The decode corpus learned this the
expensive way, where collapsing "rejected" with "accepted but normalized" let a
corpus pass against the exact decoders it was written to catch.
Accept vectors assert the byte-checkable equation `project(serialize(parse(T)))
== T`, which quantifies over TEXTS. The comments say plainly that it must not be
replaced with bundle identity, and name the three deliberate binary losses --
duplicate blobs collapsing under derived-ordering, regenerated physical layout,
and dropped accelerators -- so the next reader does not "fix" it into a bundle
comparison and then chase a phantom.
Reach is asserted exactly, not merely non-zero: two documents each carrying an
extension, a canonical base, a custom profile, a lineage, and more than one
envelope; and every reject class must have exactly one vector the parser really
refused. A mislabelled reject drops its class to zero rather than collecting
credit from its label.
The corpus was verified by breaking the implementation, not the checker: making
`parse_document` silently tolerate a missing trailing LF is caught three ways,
and the suite reports "declared reject, but was ACCEPTED and normalized".
Review fixed one defect in the semantic harness. It claimed to check
`semantics(parse(project(B))) == semantics(B)` but compared `parse(project(B))`
against `document_from_bundle(B)` -- and `project_bundle` *is*
`project_text_document(document_from_bundle(..))`, so both sides flowed through
one function and any bug in it cancelled out. Dropping an envelope there passed.
It now compares against the envelopes the test itself serialized in, which is
independent of the projection path; the same mutation fails.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`epiphany-textproj` projects a bundle to its canonical text and reads it back:
the header, identity, profile, extension, canonical-base and blob productions,
plus the pipeline that joins them to the operation layer already in place.
Free functions, not `TextValue` impls. `epiphany-bundle` does not depend on
`epiphany-core`, so the trait and the manifest types are both foreign to this
crate and the orphan rule forbids the impls. That looked like it would force a new
dependency edge until the productions were read properly: no document line
contains a `value` position, so none is needed. The bundle stays independent of
the music model, as designed.
Blobs are handled on both sides and neither is the obvious one. The emit side
filters `blob_roots` through a real reachability predicate that today provably
returns empty -- never the field wholesale, which is the plausible wrong answer.
The parse side rejects any blob line outright, while still parsing the production
in isolation so both halves are ready together. A trip-wire source-scans
epiphany-core and epiphany-ops for `BlobId` and names both obligations when it
fires; it asserts it actually scanned files, so it cannot pass vacuously.
Two rejections were added on the agents' own reading of the spec rather than the
brief, and both are right. Envelopes must already be in canonical reduction order
(text_projection.tex:458) -- without that, two texts differing only in envelope
order would denote one document. And a hand-built `TextDocument` carrying blobs is
refused rather than silently dropped, since serializing it would produce a bundle
whose next projection loses those bytes.
Section order is enforced as a sequence, not a set: a repeated or out-of-order
section is a rejection, never something the parser sorts back into place.
`derived-ordering` sorts and de-duplicates blob lines and extension chunk roots by
*rendered* form, proved against fixtures that are unsorted in memory rather than
already canonical -- the blind spot that let five outbound normalizations ship
untested in the operation layer.
Serialization stages payloads and lets `commit` assign every offset, hash and
chunk id, which is exactly what `req:textproj:derive-or-carry` assumes; nothing is
hashed by hand. The emitted bundle deliberately carries no accelerators.
And the three directions compose: `project(serialize(parse(T))) == T` holds
end-to-end over the companion's own worked example. Eight checks re-mutated
independently of the agents' reports, each killed by its own named test; all eight
document productions diffed against the grammar.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Scoping the document layer found that no blob can be canonical. A blob is
canonical iff referenced by a canonical operation or by canonical reduced state
(`req:textproj:canonical-blobs`; core_spec §"Canonical and Non-Canonical Manifest
Roots"), and nothing in epiphany-core or epiphany-ops references a `BlobId` --
there is no mechanism by which one can be reached. So today every real bundle
projects to zero blob lines.
The first instinct was to let a parser accept blob lines anyway, for forward
compatibility. That is wrong twice over. Forward compatibility is owned by header
gating -- a future writer's text carries a future version, which this parser
rejects at line one -- and a blob line accepted today would be staged into a
bundle that the next projection silently drops, losing data *and* falsifying
`project(serialize(parse(T))) == T` for that text.
`req:textproj:reject-unreferenced-blobs` therefore requires rejection, and the
conformance equation holds unconditionally over parse-accepted texts rather than
only over texts in the image of `project`.
`req:textproj:header-version` pins the other half: a parser accepts exactly one
header version, the companion's own. Multi-version acceptance and text
migrate-on-read are deferred in the same posture as op-payload migrate-on-read
rather than improvised.
Auditing the worked example against its own grammar found three defects in it: it
carried a blob line, which is now by construction an example of an *invalid*
document; its byte strings used literal ellipses, which `bytes ::= "#x" hexdigit*`
cannot derive; and its header still claimed 0.3.0. All three fixed, and the
preamble no longer promises elisions it does not contain -- an example that cannot
be parsed teaches the wrong lesson.
The version now appears in six places here. Two were locked; the two dangerous
ones were not, because they are *normative* -- a bump that updated the title and
missed them would leave the companion requiring parsers to accept a version it no
longer is. `requirements_name_only_this_companion_version` scans every requirement
block and holds any version literal to the title, deliberately exempting the
revision history, where old versions are the point.
Twelve grammar-gate tests, all mutation-verified.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Projection and strict parse for the whole Chapter-6 operation vocabulary: the 31
kind productions, the envelope with its stamp and causal context, the four
payload variants, and the sub-vocabularies for actions, undo policies, tuplet
compensation, cross-cutting values and position remapping. Written against the
grammar, per `req:textproj:operation-vocabulary`, calling `TextValue` only where a
production says `value`.
Names are generated, never spelled: `OperationKindTag::catalog_name()` comes from
the same vocabulary macro as the wire discriminant and the decoder, and `parse`
dispatches through an exhaustive match, so a kind added to the vocabulary and not
to the projector fails to compile.
Six sequences are order-constrained because their encoders normalize. Both halves
of each are enforced and both halves are tested, which turned out to matter. The
rejecting half was straightforward -- `TransposeOp.targets` mirrors the frozen
multiset exactly, rejecting a strict decrease while accepting a duplicate, and
getting that backwards would silently break a frozen operation's replay. The
*normalizing* half was written correctly by every agent and tested by none: every
fixture was already sorted, so all five outbound sorts survived deletion with the
suite green. The consequence was real -- with one removed, the projector emits
descending targets that its own parser then rejects, and that disagree with the
canonical bytes.
`textproj_conformance.rs` closes that. Each of the five builds a value unsorted in
memory, asserts the fixture pair really is descending before relying on it, then
checks the projection sorts, parses, and matches what encode-then-decode produces
-- pinning text and bytes to one normalization rather than merely to each other.
It also carries a structural injectivity sweep: 3319 mutants of projected
envelopes, of which 259 parse, all re-projecting byte-identically. That is
`req:textproj:roundtrip`'s second equation, and it is the evidence that no
whole-line re-project guard is needed -- every path that could normalize is
pre-empted by a per-site check. The sweep asserts its own reach, because one that
rejected everything would prove nothing. Deleting any per-site order check makes
it fail independently of the dedicated test.
And it locks the companion's worked example byte-for-byte. It was correct and
nothing kept it so, which is how a "machine-checked" claim became true of one run
and of nothing durable at 0.3.0.
All seven checks mutation-verified, each killed by exactly its own named test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Scoping the operation layer surfaced a contradiction the companion had never
resolved. `req:textproj:value-projection` is a mechanical rule over structs,
enums and newtypes; the Grammar chapter separately spells out `envelope`,
`stamp`, `causal`, `payload`, `kind` and their sub-vocabularies. The two disagree.
Under the value rule an operation would read
`(insert-event (insert-event-op #x0a <event>))`, because an `*Op` record is a
named-field struct and so not a transparent newtype.
`req:textproj:operation-vocabulary` rules for the grammar, which is what the
companion already implied: the value rule's own preamble scopes itself to the
values an operation *embeds*, and the `value` nonterminal marks where it applies.
If the rule governed the vocabulary too, the grammar's productions would be
redundant and partly wrong.
An operation kind therefore inlines its payload record, and the requirement says
why rather than asserting it: the record exists so each variant can name a type,
and the binary form adds no bytes for it -- `OperationKind`'s encoding writes the
tag and delegates. A wrapper that adds no bytes adds no text. That is the same
argument clause 2 makes for newtype transparency, applied one level out.
Also removes a second name for one type. `transpose-interval` inlined its interval
as `(interval <d> <c>)` while `TranspositionInterval` projects as
`(transposition-interval <d> <c>)` at every `value` position. The production now
delegates to `value` and the special case is gone.
Two new checker tests lock both, bringing that gate to nine.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`TransactionId` had no `TextValue`, which blocked the operation layer: it is a
core type, so epiphany-ops cannot implement a core trait for it.
The fix is not the missing line. `bytes_text_value!` was a hand-maintained list of
thirty ids sitting parallel to the `graph_id!` invocations that declare them --
the same shape that has cost this project four bugs -- and `TransactionId`'s
absence was that latent bug already biting. `graph_id!` now generates the
`TextValue` impl beside the id itself: every graph id is a byte-string leaf by
definition, so the list that declares them is the list that projects them.
`bytes_text_value!` keeps only the leaves that are genuinely not graph ids --
`ContentHash`, `TypedObjectId`, `ReplicaId`, `OperationId` -- shrinking from
thirty entries to four. A graph id added tomorrow gets its projection for free
and cannot be forgotten.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`project` and `parse` for every value an operation payload can embed.
The codec macros now emit a `TextValue` impl beside the `Codec` impl, from the
same invocation: 116 types whose field order cannot disagree between the binary
form and the text, at zero call-site churn. That is the companion's own rationale
applied to code -- a rule cannot drift from the listing it reads, and two listings
of one struct is the drift P13-I1 already cost us. `struct_codec!` rebuilds through
a struct literal and `cstyle_enum_codec!` matches exhaustively, so a field or
variant added later fails to compile rather than silently vanishing from the text.
The other 44 types have hand-written codecs and so need hand-written projections.
Their field order is verified by a mechanical diff of the identifier sequence in
each `fn enc` against the one in each `project`; all 44 agree. This matters because
a `project`/`parse` pair that agrees with itself on a *wrong* order round-trips
perfectly -- neither the compiler nor any round-trip test can see it. The
neighbouring blind spot, a mistyped constructor symbol, is closed by
`textvalue_names.rs`, which recovers each type's Rust name from its derived `Debug`
and compares it against the symbol actually emitted.
Strictness turned out to need only one of its two layers, and mutation testing is
what established that. Every per-site check is live: the set/map strictly-increasing
walk, `RationalTime`'s lowest-terms compare before construction, the catalog-id NFC
intern-and-compare, and `EventArena`'s ascending-`EventId` walk. Every whole-value
`ensure_canonical` guard was dead -- `Tempo::new`, `ReferencePitch::new`,
`SpellingPrecedence::new` and `EventOrderingDAG::try_new` reject rather than adjust,
so an accepted value re-projects to exactly its input and the guard could never
fire. A probe confirmed `try_new` returns its input map unchanged. Helper and all
four call sites removed: a check that cannot fail invites weakening the real one.
Also moves `catalog_name` out of the grammar *test* and into
`operation_kind_tag_vocabulary!`, where the discriminant and decoder already live.
It was a hand-maintained list parallel to an enum -- the exact shape that has cost
this project four bugs.
Method note recorded in DECISIONS: the work list came from compiler errors, but the
compiler reports only the frontier. `AnchorOffset`, `VoiceSelector`, `PowerOfTwo`,
`OctaveOffset` and `NonZeroU16` were each hidden behind a type that had not compiled
yet, so the list must be iterated to a fixpoint.
Gate green -- clippy 0, 1109 tests, doc 0, conformance 8/8, no golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The value model of the companion, and the half of it that has to say no.
`Sexp` carries only what a lexer can honestly know: list, symbol, integer, byte
string, string. There is no `Bool` variant, because `true` is spelled exactly like
a symbol and nothing but the expected type tells them apart; no `Ratio` and no
`Option` variant, because both are lists. That is `req:textproj:schema-directed`
expressed as a type rather than asserted in a comment.
Integers are `BigInt`. `RationalTime` promotes to a `BigRational` when arithmetic
overflows its inline range, so the text's integers cannot be bounded by `i64`.
`read_sexp` is strict per `req:textproj:strict-parse`: it accepts exactly what
`Sexp::write` emits. Leading zeros, negative zero, upper-case hex, an odd hex digit
count, an unknown escape, a raw tab, a doubled space, a space after `(`, and any
trailing input are rejections, never normalizations. `BTreeSet` and `BTreeMap`
parse strictly increasing and refuse to absorb a duplicate -- sorting the input
would be normalizing it.
Every guard mutation-verified, which caught two that were not guards. The
upper-case-hex branch and the space-before-element branch change no verdict: the
first is already rejected by the odd-digit count or by trailing input, the second
by `value`, which knows no value beginning with U+0020. Both survived deletion
with the rejection suite green -- it proved only that *something* rejected them.
They are diagnostics, so their contract is the message, and
`the_reader_names_the_real_problem` now holds them to it. A check that looks like a
guard but is not is worse than no check: it invites weakening the real one.
Gate green -- clippy 0, 1048 tests, doc 0, conformance 8/8, no golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Found by starting the implementation: the grammar could not derive an ordinary
pitched note.
`value` had no alternative for a sequence at all, though value-projection clause 5
required one. Adding it exposed why it was missing. A sequence whose first element
is a fieldless variant is shape-identical to a struct, and `()` is both the empty
sequence and the absent option. Both collisions are reachable from the first
pitched note in any score: `PitchedEvent` carries `articulations` and `ornaments`,
sequences over the zero-field `ArticulationMark` and `OrnamentMark`, beside an
optional `DynamicMark`. One `insert-event` line holds both.
The collision is irreducible without new syntax, and new syntax buys nothing --
`req:textproj:strict-parse` already obliges a parser to reject a duplicate in a
set-typed field, which it cannot do without knowing the field is set-typed. The
parser consults the schema either way. So `req:textproj:schema-directed` states
what the ratified rules already required, `value` collapses to `"(" value* ")"` or
a leaf -- all shape can honestly say -- and the requirement assigns meaning by the
expected type. The binary form is schema-directed for the same reason and pays the
same price: its bytes do not say what they are either.
Three consequences stated: a struct with no fields is the bare symbol, as a
fieldless variant is; a byte string is not a sequence, so an opaque extension
payload and a `SoundConfiguration` project as byte strings, never as lists of
integers; and the grammar's repetitions now carry a notation rule -- adjacent
elements separated by exactly one space -- without which `"(transpose (" bytes* ")"`
spelled two targets as one undelimited run of hex.
Checker gains a seventh test, mutation-verified by restoring the 0.4.0 `value`
production and by stripping the schema-directed citations. It asserts the
symbol-headed struct alternative is *absent*: a grammar claiming to tell a struct
from a sequence by shape would be lying.
Gate green -- clippy 0, 1038 tests, doc 0, conformance 8/8, no golden churn, three
spec documents build clean with no undefined references.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 0.3.0 audit raised two normative issues and one process issue. All three
land here, plus a fourth defect that reviewing the fix turned up.
The grammar contradicted its own escape requirement. `req:textproj:string-escapes`
obliges a writer to escape the backslash and a parser to reject a bare one, while
`unescaped` admitted it. Escapes are now four two-character sequences and
`unescaped` excludes U+0022, U+005C, U+000A, U+0009 by codepoint.
"Keep the binary order" was not available for every sequence. It holds only where
the binary order reads data the projection preserves, and two sequences fail that
test: `blob_roots` sorts by the full `BlobRef` encoding (offset, compressed
length, compression), and an extension's preserved chunk roots sort by
`ChunkRef`'s order, keyed on kind, then content hash, then *offset*. Under the
blanket rule, relocating a chunk -- which changes no semantics -- would change the
text, and two entries indistinguishable after erasure would produce duplicate
lines. `req:textproj:derived-ordering` orders and de-duplicates exactly those two
by projected form, and states that every other sequence keeps the binary order:
profile and extension declarations sort on semantic `(id, version)` keys, and
envelopes on canonical operation order.
The "machine-checked" grammar was checked by a throwaway script -- true of one run
and of nothing durable, the same evidence gap P2-P4 kept exposing.
`text_projection_grammar.rs` is the committed form: no nonterminal undefined or
unreachable, the escape rule admits exactly its four sequences, and the operation
and chunk productions are *derived* from `OperationKindTag::PAYLOAD_FREE` and
`ChunkKind` through an exhaustive match, so a kind added to the enum and not to
the grammar fails to compile. Every locator finds its production by name; writing
it exposed four bugs in itself, three of them column-anchored checks that a reflow
would have silently switched off.
Reviewing the escape fix found it reintroduced the audited defect in disguise: a
quoted terminal `"\\"` reads as two backslashes, so every escape became three
characters long. Both characters are now codepoints. Relatedly the mono font's
`Ligatures={TeX}` rendered U+0022 as a right curly quote and `--` as an en dash,
so the grammar misprinted the very delimiters it defines; `core_spec.tex` already
omitted it. Both are asserted against.
All six tests mutation-verified: the anchor asserted present before substitution,
then the named test observed to fail. Gate green -- clippy 0, 1037 tests, doc 0,
conformance 8/8, no golden churn, three spec documents build clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0.1.0 left kind, action, policy, constraints and barrier derived-but-unwritten
and admitted it. All are now written. The grammar has no undefined nonterminal
(machine-checked) and 31 operation-kind productions in exact discriminant order,
cross-checked against envdecode.rs.
The one real decision was how embedded Chapter-5 values appear. An operation
payload carries an Event, a Pitch, a Region, a TimeSignature, and there were
three ways to write them:
Forty hand-written productions would restate the entire Chapter-5 data model in
a second normative document -- two normative listings of one struct, which is
the exact drift P13-I1 was opened to close.
Opaque canonical-value byte strings would be lossless and zero-drift, but a
pitch would be unreadable without binary tooling, failing the core spec's own
"format inspection and debugging" use case.
One mechanical rule (req:textproj:value-projection, ratified): a struct is
(<type-name> <field>...) with fields positional in the ratified declaration
order; a newtype is transparent, exactly as in the binary form; a tagged union
is (<variant> <field>...); an option is () or (some v); a sequence keeps the
binary form's order. A rule cannot drift from what it reads.
Two leaf decisions follow from canonicality rather than taste. A rational is
(ratio n d), lowest terms, sign on the numerator. A CanonicalF64 is the byte
string of its eight canonical IEEE-754 bytes and never a decimal: decimal float
text is not canonically unique -- shortest-round-trip and 17-significant-digit
forms both round-trip, and -0.0 has two spellings -- so a decimal tempo would
break req:textproj:canonical-text at the first tempo mark.
Operation-kind names follow the Operation Catalog's sections (create-region,
create-staff), not OperationKindTag's (InsertRegion, InsertStaff), which renamed
three pairs for reasons internal to the tag space. The projection follows the
semantics.
One deferral, stated as such rather than left to inference:
affected_object_kinds and edit_barriers have ratified structured shapes AND
canonical byte encodings, and the bundle stores them opaquely. At 0.3.0 the
projection does the same, on the principle that it interprets nothing the bundle
does not. A later revision may project them structurally; their canonical bytes
are unchanged by that, so it changes the text and not the document.
Also corrected: the extension line now writes its fields in the ratified
declaration order, which had preserved_chunk_roots before affected_object_kinds
and edit_barriers.
Gate: clippy 0, 31 targets / 1031 passed / 0 failed, conformance 8/8, zero golden
churn; core_spec, binary_format and text_projection all build with no undefined
references. Grammar closure machine-checked: no undefined nonterminal.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A review found the 0.1.0 companion lossy for documents that are valid today. Its
claim to preserve the manifest's canonical roots was false three ways:
A canonical blob had no representation at all. blob_roots referenced by
canonical operations or reduced state ARE canonical roots, and the document
structure had no blob line. An embedded image, font or recording would vanish
from a projection silently -- the operations referencing it still there,
pointing at a blob id the text no longer contained.
An ExtensionDeclaration lost its semantic version and its affected_object_kinds
outright, and left its preserved_chunk_roots undefined.
ProfileId::Custom(ProfileRegistryId) was unrepresentable: a symbol was required
where sixteen registry bytes are carried.
All three share one cause I had not named. A ChunkRef and a BlobRef are PHYSICAL
references -- offset, compressed length, compression -- which the projection may
not preserve; and they carry DERIVABLE identities -- ChunkId, ContentHash, BlobId
-- which it may not duplicate. Having no rule for either, I dropped the
references and took their contents with them.
req:textproj:derive-or-carry states it, and it is the same rule
req:textproj:reduced-state-derived already applied one level up: carry exactly
what the document does not determine, and nothing it does. Physical attributes
never appear; derivable identities never appear; content and semantic attributes
always do. The one non-derivable identity in schema major 0 is SnapshotId, which
the Binary Format companion pins as opaque and forbids readers to derive -- an
exception for a stated reason rather than an oversight. The grammar now visibly
contains no offset, no length, no compression, no chunk id, no hash, no blob id.
So: req:textproj:canonical-blobs (canonical blobs projected, non-canonical ones
not), req:textproj:profile-id ((custom #x...)), req:textproj:extension-declaration
(every field; chunks as kind + schema + payload, never as a ChunkRef -- the
projection has no file to point into), and req:textproj:base-snapshot-inline
extended to say what the inlined payload IS and that the root ChunkRef and the
SnapshotRef hash are re-derived from hash(Snapshot, schema, payload), never read.
The gap started upstream. core_spec's own list of what the projection preserves
omitted canonical blobs while classifying blob_roots as canonical roots -- an
inconsistency inside one document. Corrected there, along with withdrawing the
permission to reference a base snapshot "externally", which the inline
ratification had already made untenable.
Also: the generated PDF metadata and page header still said Operation Catalog,
inherited from the copied preamble.
The four 0.1.0 ratifications stand unchanged. Implementation stays deferred: a
gate that is lossy is not a gate.
Gate: clippy 0, 31 targets / 1031 passed / 0 failed, conformance 8/8, zero golden
churn; core_spec, binary_format, operation_catalog and text_projection all build
with no undefined references.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
core_spec Chapter 8 declares the text projection normative and leaves the form
unwritten; binary_format excludes it as "the Text Projection companion's". That
companion did not exist. spec/text_projection.tex v0.1.0 supplies it. No
implementation -- this is the gate.
It was blocked on P5. The projection's normative requirement is bidirectionality
WITH THE BINARY FORM, which needs bytes -> OperationEnvelope, and that decoder
did not exist until 3baf8d0.
Four ratified calls:
Reduced state is preserved by DETERMINING it, never by a second literal copy. It
is a deterministic function of the operation set and the canonical base; a text
carrying both would hold two sources of truth for one fact and nothing could stop
them disagreeing. core_spec's "all canonical reduced state" now carries that
reading inline rather than being silently reinterpreted.
A canonical base snapshot is INLINED as one opaque byte string. This is the call
with teeth. A base exists precisely so prior operations need not be retained, and
where they are pruned the base is derivable from nothing else -- so a
reference-only projection of a compacted document would be LOSSY, and the text
would not determine the document it claims to project. core_spec permits "encoded
compactly or referenced externally"; inline is the choice that keeps archival
honest, and core_spec now says why.
Lowercase hex everywhere: one rule, no alphabet or padding to canonicalize,
greppable. Base64 would buy a quarter of the bytes of the one body nobody reads,
at the price of a second encoding and a rule for which applies where.
One envelope per line: the stated use case is that merge conflicts surface at the
envelope level, and one line per envelope makes a three-way merge conflict
exactly an envelope conflict -- never a conflict inside one, yielding an operation
neither side wrote. It also removes all indentation, so canonicality has nothing
to hide in. Readability is a pretty-printer's job; what a pretty-printer must not
do is write its output back and call it a projection.
Strict parsing is stated in the terms P2-P5 taught: normalizing non-canonical
text IS accepting it. The rationale names both hazards this repo hit in binary --
a re-encode guard is blind to order-preserving sequences, and a guard on an outer
value can mask a lenient inner codec -- and prescribes the same total defence.
Conformance requires both directions: a projector alone cannot be checked.
One gap is stated in the document rather than left to be discovered: the atom
productions and line shapes are normative, while kind/action/policy/constraints/
barrier are derived from the Operation Catalog and the wire table rather than
spelled out. That is the difference between a design gate and a finished
companion.
Gate: clippy 0, 31 targets / 1031 passed / 0 failed, conformance 8/8, zero golden
churn; core_spec, binary_format and the new companion all build with no undefined
references.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Text Projection requires decoding operation envelopes. No such decoder existed,
and the hole was bigger than the task: the format was WRITE-ONLY for operations.
A bundle's envelope blocks decoded to opaque byte strings, OperationKind had an
encoder and no decoder, nothing outside epiphany-bundle even called decode_block,
and nothing anywhere reconstructed an OperationEnvelope. Chapter 6 holds that a
score's canonical state IS the set of operations committed to it -- so a bundle
could be written and its score never reopened. The envelope's byte layout was
fully pinned in the Binary Format companion. Nobody wrote the inverse.
epiphany_ops::decode_envelope is that inverse. The first thing built on it is
testkit/tests/bundle_reopen.rs: create a bundle from 400 generated envelopes,
commit, take the bytes, reopen from nothing but bytes, decode every envelope,
rebuild the OperationSet, reduce -- and get the same canonical state. That test
could not have been written before this commit.
Strict in two layers, per the P2 lesson. A whole-envelope re-encode-and-compare
guard, sound here because every sequence in this encoding is normalized by its
encoder. Plus per-site checks where the rule deserves its own error and a future
encoder change must not silently relax it: TransposeInterval.targets is a SET
(seq-strictly-increasing; a duplicate is rejected, never absorbed by the BTreeSet
it collects into), and the frozen Transpose.targets is a MULTISET (non-decreasing,
duplicates preserved). That is the rule Push 4a wrote into the wire table and left
for whoever built this decoder. And a bounded count(): a declared count past the
bytes remaining is rejected before it can drive an allocation.
Coverage measured, not assumed -- again. The obvious oracle (gen_envelope_set,
4000 envelopes) reaches only 28 of 31 kinds and 1 of 4 payload variants.
ChangeRegionTimeModel, DeclareTransaction, Registered and all three meta payloads
were untouched, and they hold the trickiest decoders: PositionRemapping, NFC
strings, ResolutionAction, EnvelopeHash. So the exhaustive test drives a match on
OperationKindTag, and the compiler forces a sample for every future kind.
Two mutations verified. Removing the seq-strictly-increasing check still rejects
-- the guard is a real backstop there -- but with the wrong error, so the per-site
check earns its place on the error rather than the verdict. Removing the
whole-envelope guard leaves every round-trip test green, because round-trips only
ever feed canonical bytes; an_unsorted_sequence_is_rejected_by_the_whole_envelope_guard
is the test that locks it, and it fails under that mutation.
A trap worth remembering: PitchId::new(ReplicaId(7), 1) and
OperationId::new(ReplicaId(7), 1) have identical canonical bytes -- typed ids
share their byte form -- so a byte-patching test that searches for an id finds the
envelope's own leading id first.
Gate: fmt clean, clippy 0, 31 targets / 1031 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8, zero golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two review findings after P4. Both real.
The tag-omission failure could recur. After the P4 fix there were STILL four
hand-maintained lists -- a test-local all_tags(), a barrier test spelling
0u8..=30, a fuzz corpus naming five tags, a vector corpus naming four -- plus a
malformed-bytes test asserting that 31 rejects. A future tag 31 added to
discriminant() and omitted from decode_canonical() would have left every one of
them green, and the malformed test would have LOCKED it, exactly as the 30
version did two commits ago.
operation_kind_tag_vocabulary! is now the single source. It generates
discriminant, from_discriminant, and OperationKindTag::PAYLOAD_FREE from one
list, and the generated discriminant match is exhaustive over the enum -- so a
variant added to the enum and not to the macro fails to COMPILE. Everything
downstream reads PAYLOAD_FREE: the decoder, the fuzz corpus (all 31 tags, not
five), the conformance vectors (an accept vector per tag: 65 vectors, not 37),
and the edit-barrier round-trip. Every "one past the vocabulary" constant is
computed, never spelled; a spelled constant is the trap that springs on whoever
appends the next tag.
Verified end to end with a hypothetical tag 31. Added to the enum alone: compile
error. Added to the enum and the macro: it compiles, decodes, and every derived
check passes because they read PAYLOAD_FREE -- while the committed corpus's
drift lock AND its now-stale "one past the vocabulary" reject vector both fail,
forcing the new vectors into the diff. There is no path where a new tag leaves
everything green.
Second finding: the corpus called itself normative while the spec said it was
deferred. Binary Format's "About This Companion" listed the cross-implementation
decoder test among things the document does not cover, and the Golden Anchor
Registry called it "the deferred conformance harness" whose literal-byte vectors
a future test "should add". Both now ratify it. New req:binfmt:decode-vectors
and a "The Decode Vector Corpus" section: a conforming decoder MUST accept every
accept vector for a surface it implements, MUST reject every reject vector, and
MUST re-encode an accepted value to exactly its bytes -- and accepting a reject
vector and then normalizing it IS accepting it. Binary Format 0.8.0 -> 0.9.0.
The wire-format fuzzer stays an implementation deliverable. The corpus header
now cites the requirement instead of asserting one.
Gate: fmt clean, clippy 0, 30 targets / 1024 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8 with [7d] at 65 vectors, zero golden churn,
binary_format rebuilds with no undefined references.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
spec/vectors/decode_vectors.txt -- 37 committed byte strings across five
surfaces, each with its normative accept/reject verdict. The reference
implementation's fuzzers prove its own decoders self-consistent, which says
nothing about whether a foreign decoder agrees with the format. This is what one
is checked against. Gated in the conformance suite as [7d], and drift-locked:
the committed file must equal vectors::render(), so a wire-format change lands
in the diff.
It found a real defect on its first run. OperationKindTag::TransposeInterval
encoded to [30] and its own decoder REJECTED it -- Push 4a added the variant to
discriminant() and never to decode_canonical. OperationKindTag is what edit
barriers persist, so a barrier prohibiting TransposeInterval could be written and
never read back. Silent data loss on reopen.
Four things should have caught it. None did, and two made it worse:
The round-trip test enumerated DISCRIMINANTS -- (0u8..30).map(decode_canonical)
-- starting from bytes the decoder already knew, so it structurally could not
notice a variant the decoder was missing. It now enumerates VARIANTS from one
all_tags() list, with a completeness check in both directions.
The distinctness test's hand-written variant list omitted it too. Same list now.
operation_kind_tag_decode_rejects_malformed_bytes asserted that tag 30 is
REJECTED, and layout-ir's decode_rejects_unknown_discriminants asserted the
same at the barrier surface. Both were locking the bug in place and made it
look deliberate. Both now name 31, and a new barrier test round-trips a barrier
prohibiting every tag -- the persistence surface where this actually bites.
The P2 decode fuzzer fed valid corpus bytes to the tag decoder and tallied the
failure as a REJECTION, like any garbage input. It never asserted that an
unmutated corpus entry decodes. Both fuzzers now do, as a pre-pass.
The harness had the same disease as the code. `check` collapsed "rejected" with
"accepted but does not re-encode", so a decoder that silently normalizes
non-canonical bytes PASSED the reject vectors it was written to catch. Verified:
removing the whole-state guard, and restoring the lenient compression codec, both
left the corpus green. `check` now returns Ok(injective) for accept and Err for
reject and never conflates them -- silently normalizing non-canonical bytes IS
accepting them. With that fixed, all four defect mutations fail the corpus, each
naming its class.
The corpus pins one vector per class this repo has shipped a bug in:
non-canonical-map-order (a guard catches it; no per-site check exists),
non-canonical-vec-order (only a per-site check catches it; a guard is blind),
lenient-sub-codec (a guard masked it in the manifest; the index had none), plus
trailing-bytes, truncated, unknown-discriminant, count-exceeds-remaining. A test
fails if one goes missing.
Gate: fmt clean, clippy 0, 30 targets / 1024 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8 (now including [7d]), zero golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Audit correction. The manifest test was documented -- in its own doc comment, in
DECISIONS.md, and in P3's commit message -- as "exhaustive single-byte
perturbation". It tried three XOR deltas per byte. The claim was false as
executed, and the "guard is total" conclusion leaned on it.
Both tests now do what their names say:
every_single_byte_replacement_of_a_manifest_is_rejected -- each byte, each of
the 255 other values. 0.15s.
compression_none_rejects_a_non_zero_parameter_byte -- every one of the 255
non-zero parameter bytes, plus a round-trip of all 256 values through Zstd and
Reserved, so the strictness is shown to be confined to None.
And the totality claim is re-seated where it belongs: on the argument, not on a
finite test. manifest_id is derived from the body, so a body edit fails the id
check and an id edit fails the derivation; encode_body sorts and deduplicates
every vector, so an out-of-order or duplicated encoding cannot round-trip. The
test is evidence for that argument over ONE constructed manifest, and is blind
to multi-byte perturbations entirely. Both the doc comment and DECISIONS.md now
say so.
Worth recording: restoring the leniency fails the codec test and the index test,
and leaves the manifest test GREEN -- the guard rejects those bytes whatever the
sub-codec does. That is not a weak test. It is the asymmetry that hid the bug,
and it locks the guard rather than the codec. A suite where every test fails on
every mutation would be telling us less.
No codec or wire-format change; the strict branch was already correct.
Gate: fmt clean, clippy 0, 30 targets / 1012 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8, zero golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A wire-decode fuzzer over Bundle::open, Manifest::decode, OperationIndex::decode,
decode_block and envelope_offsets. The existing crash-recovery fuzzer corrupts an
image the way a CRASH does -- torn writes at syscall boundaries. This one
corrupts it the way an attacker or a bit-rotted disk does: arbitrary bytes,
anywhere.
It found a real defect. CompressionAlgorithm::None read its parameter byte and
DISCARDED it, while encode writes zero. So [0, 0xFF] and [0, 0] both decoded to
None, and the first re-encoded to the second: a lenient, non-injective codec,
inherited by every structure embedding a ChunkRef.
Whether that was visible depended entirely on the embedder:
Manifest::decode has a whole-value re-encode guard, and it is TOTAL -- proved
by exhaustive single-byte perturbation, every one rejected. It caught this.
OperationIndex::decode has no guard; it validates per-site. It accepted both
byte strings, while its own doc promised to "reject (never normalizing) any
non-canonical form". That promise was false.
That is the same two-layer lesson P2 recorded one commit ago, from the other
side: a re-encode guard is complete only where the encoder normalizes, and its
completeness can MASK a lenient sub-codec rather than fix it. Fixed at the
source, not papered over at the index. An exhaustive sweep -- every byte, every
value, plus an 8-byte extreme-integer window -- finds no remaining non-injective
site.
The fix contradicted ratified spec text, which said the byte was "present but
zero, and ignored on read". Escalated rather than fixed unilaterally. The user
ratified strict decode: core spec's clause is superseded, Binary Format gains
req:binfmt:compression-none-parameter and moves 0.7.0 -> 0.8.0. No wire layout
changed, and no conforming writer emits a non-zero byte, so this rejects only
corrupt or adversarial input -- no existing file changes meaning.
Coverage was the harness's problem again. The fuzzer's first run reached the
operation index's accept path ZERO times -- random bytes never decode as an
index -- so every assertion under it was vacuous. It found the bug only once the
index corpus was built from real OperationIndex::build output. The smoke tests
now assert on a WireFuzzCoverage so that cannot silently regress. 1.5M inputs
across five seeds, ~1s each, clean after the fix.
Three regressions, each mutation-verified by restoring the leniency: the codec
itself, the index that exposed it, and the manifest guard's totality -- which is
the asymmetry that hid it.
Gate: fmt clean, clippy 0, 30 targets / 1012 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8, zero golden churn, both spec documents rebuild
with no undefined references.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The operation layer has exactly two byte-decode surfaces: MaterializedState and
OperationKindTag. Operation payloads have no decoder -- OperationKind is
encode-only -- so nothing here can yet accept a duplicate TransposeInterval
target. When such a decoder lands it inherits the wire table's seq-strictly-
increasing rule: reject a duplicate, never normalize it away.
No defect in the decoder. It already carries the Phase-1 hardening: a whole-state
re-encode-and-compare guard, and with_capacity(n.min(1024)) at every count site,
so the unbounded-allocation and soft-DoS classes P1 fixed in core do not apply.
2M adversarial inputs across four seeds, ~2s each, clean.
The finding is about the ARCHITECTURE, and it qualifies P1's recorded design
note that the guard "is complete-by-construction, it cannot miss a lenient
codec". It is complete only for fields the decoder NORMALIZES. The BTreeMaps
re-sort and de-duplicate, so a non-canonical encoding of them cannot survive a
round trip. But the guard is BLIND to order-preserving Vec fields: a reordered
anomalies or pending list re-encodes to exactly the bytes it came from, so the
guard sees identity and accepts. Only the per-site windows(2) checks reject
them, and the same holds for a conflict record's caused_by / affected_objects,
which ConflictRecord::encode_canonical writes verbatim.
Measured, not reasoned: removing both per-site Vec order checks leaves a 40K
injectivity sweep GREEN. An injectivity fuzzer structurally cannot see this
class -- it asserts bytes->value->bytes identity, which is exactly what a
missing order check preserves. Those checks were locked by nothing. They are now.
(effects is a Vec with no order check, correctly: its canonical order is
reduction order, which a decoder cannot recompute. Two orderings are two
different states, so injectivity is not at stake.)
Delivered: fuzz::run_decode_fuzz over both surfaces, returning a
DecodeFuzzCoverage the smoke tests assert on -- a decode fuzzer that never
reaches a decoder's accept path proves only the absence of a panic, and mine
initially accepted nothing worth speaking of. Plus one deterministic test per
layer: an out-of-order objects map (guard only), an out-of-order anomaly list
and a reordered pending list (per-site only). Each mutation-verified against the
exact check it locks, and the fuzzer confirmed blind to the per-site pair.
Corpus depth is now a property rather than luck. A fixed list of envelope-set
sizes reduces to states with no conflicts, anomalies, pending, or spellings --
the very branches holding every canonical-order check. Measured: 6 of 12 seeds
failed to produce all four. build_decode_corpus draws until covered and asserts.
Gate: fmt clean, clippy 0, 30 targets / 1007 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8, zero golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A review of the P13-S3 fix caught the reducer's comment overclaiming. It said
the coupling "cannot mis-fire on an unrelated operation" because
ModifyIdentifiedPitch never writes the spelling set. That is true of the
OPERATION and irrelevant to the UNIT: the coupling is keyed on the pitch, and
on which keys the TRANSACTION wrote. A transaction whose members write the two
halves separately couples exactly the same way.
The editor's "move note" is precisely that -- ModifyIdentifiedPitch for the
value plus RespellPitch for the spelling set, in one transaction. Measured:
no later respell, BestEffort -> Applied, pitch restored to C4
later respell, BestEffort -> Applied, pitch STAYS D4, later E stands
later respell, StrictInverse -> Conflicted, pitch stays D4
The middle row is the coupling firing on a non-transpose pair, and it is
correct: restoring the pitch to C4 while the engraved spelling reads E --
authored against the moved pitch -- is exactly the stale-notehead defect the
coupling exists to prevent. Musically, undoing half of "move this note and
respell it" is not a smaller undo, it is a wrong score.
So the breadth is stated rather than left to be inferred.
req:opcat:spelling-set-chain now says the unit is keyed on the pitch and on the
transaction's writes, names the move-and-respell case, and notes that a
transaction writing only one of the two keys is unaffected -- an unwritten key
yields no supersession, which is the narrow claim the old comment should have
made.
Three regressions: full undo when nothing supersedes (guards against
OVER-coupling), best-effort skipping the pair when a later respell supersedes,
and strict undo conflicting. Mutation-verified by removing the coupling: only
the middle test fails, restoring the pitch to C4 with its spelling still
reading E, while the other two stay green -- they lock different properties.
Gate: fmt clean, clippy 0, 30 targets / 1001 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8, zero golden churn, canonical-base digest unmoved.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
My design note was half right, and the audit caught the other half. Keeping
engraved_spelling_chain physically separate from respell_chain was correct --
respell_chain is RespellPitch's LWW working state, read by its
concurrent-differing conflict detection, and folding transposes into it would
make a concurrent respell conflict with a transpose and move the canonical
bytes of every existing history. What that did NOT license was letting one
operation own the key.
RespellPitch mutates the same graph attachments and recorded nowhere on the
attachment chain, so a chain with a single writer was wrong both ways. Both
reproduced:
respell -> [tx: transpose] -> StrictInverse undo
Applied; pitch restored to C4; the UserChosen(C) attachment ERASED.
The respell was an operation, not part of the base, so the transpose's
chain had never seen it and its predecessor was absence.
[tx: transpose] -> respell -> StrictInverse undo
Applied; the newer UserChosen(D) authoring WIPED. The respell was
invisible to the chain, so it never registered as a superseding writer,
contradicting the catalog's rule that a later canonical writer supersedes
a strict undo.
And BestEffort could restore the pre-transpose pitch while leaving a spelling
authored against the transposed one attached to it.
Fix: every writer of the attachments records on the attachment chain
(record_engraved_spellings), and a pitch's value and its engraved spelling set
undo as one unit -- if either half is superseded, neither is restored.
StrictInverse already refuses on any supersession, so the coupling only bites
for BestEffort. Two physical chains, two responsibilities: respell_chain owns
the ledger spelling and the LWW verdict, engraved_spelling_chain owns the graph
attachments.
Recording is gated on graph presence, so base-free reduction is byte-unchanged
and the seeded corpus's canonical-base digest does not move.
Four tests, two mutations verified: removing the respell's record fails all
three undo tests; removing the coupling fails the best-effort one with the
pitch back at C4 and its spelling still at C-sharp. The fourth test locks
convergence -- both permutations of a concurrent respell/transpose reduce to
identical canonical bytes.
Spec: new req:opcat:spelling-set-chain. Batch 3 of the Pass-13 ledger reopens
(P13-S1, S2 open; S3 resolved here).
Also: PitchSpelling::transposed's doc said B-sharp 3 becomes F-double-sharp 3.
The code, spec, and tests all correctly produce F-double-sharp 4 -- B to F
carries the octave. Doc only.
Gate: fmt clean, clippy 0, 30 targets / 998 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8, zero golden churn, canonical-base digest unmoved.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Audit finding 3, reproduced and fixed. Neither transpose recorded into
pitch_modify_chain, so UndoTransaction(StrictInverse) over a transaction
containing one reduced to NoOp(TargetMissing) and left the pitch shifted.
EditorSession::undo works only because it re-materializes from a truncated log,
which is a different mechanism entirely.
The behaviour gap was pre-existing: the frozen Transpose does the same, and the
pre-Push-4a catalog said so honestly ("an inverse-interval undo is a Phase-3
refinement, P11-C8"). What was new was my claim, written into the catalog for
both kinds, that the write chain handled it. It did not.
Per the ratified call, the fix is asymmetric, and the asymmetry is the point.
TransposeInterval now records its pitch write and its spelling rewrites, so
value-restoring undo returns both. Transpose does NOT start recording: that
would not change its own reduction rule, but it would change what a stored
{Transpose, UndoTransaction} history replays to -- from "the pitch stays
shifted" to "the pitch returns" -- and that is a change in what an existing
document means, which is the one thing the freeze forbids. The old operation is
therefore permanently un-undoable, which is one more reason never to author it.
the_frozen_transpose_is_not_undoable_and_that_is_frozen_too pins that, and was
verified by the inverse mutation: making it undoable fails the test.
Restoring the pitch alone would leave a notehead spelled for a pitch that is no
longer there, so a new chain -- transposed_spelling_chain, keyed by pitch,
holding the engraved-layer explicit attachment SET -- restores the moved
authored attachments and removes the propagated one together. It is deliberately
not respell_chain: RespellPitch owns that chain, its last write is the LWW
working state its concurrent-differing conflict detection reads, and folding
transposes into it would make a concurrent respell conflict with a transpose and
would move the canonical bytes of every existing history.
Base-free reduction records nothing, because it writes nothing: the fuzz
corpus's canonical-base digest is unmoved.
Three mutations verified: the pitch-value record removed, the spelling-set
record removed (both undo tests fail), and the frozen Transpose "helpfully" made
undoable. The second of those first appeared to survive -- my harness restored
the file between the two tests, so the second ran unmutated. A mutation that
no-ops looks exactly like a test that passes; that trap, recorded after Push 4a,
was hit again here in a new form.
Gate: fmt clean, clippy 0, 30 targets / 993 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8, zero golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Audit finding 2, reproduced and fixed. The Propagated attachment met the letter
of req:opcat:transpose-interval-spelling and none of its purpose.
The repro: a C4 the author deliberately spelled "C", sharpened to C#4. Both
attachments present -- the stale UserChosen and the new Propagated -- and
resolve_spelling returns Authored(UserChosen) with accidentals []. The notehead
draws a C natural for a pitch sounding C#. The accidental vanishes. Default
precedence ranks UserChosen and Imported above Propagated, so the attachment
this operation writes is always outranked exactly when it is needed.
Per the ratified call, authored spellings are MOVED, not left and not
discarded. A spelling moves by its NOMINAL, because the nominal is what carries
the author's enharmonic decision: someone who wrote B#3 rather than C4 chose
the letter B, so a perfect fifth up is F##4, not G. The accidental is then
whatever the transposed pitch requires at that staff position -- the chromatic
component never touches the spelling except through the pitch. Source,
priority, and layer are preserved: a transposed UserChosen spelling is still
the user's choice. Imported moves too; import fidelity is a property of the
file on disk, which a transposition does not touch.
An authored spelling that cannot be written at the transposed position refuses
the whole operation (TranspositionOutOfRange), resolved before anything is
written, like every other refusal. The two application passes are ordered so
that every index-addressed rewrite lands before the propagated upsert can push
and shift the indices.
The Propagated attachment keeps its purpose: it is the record for pitches with
no authored spelling, where the pre-pass would otherwise re-infer.
Three mutations verified. One of them, S2, SURVIVED the first version of the
enharmonic test and exposed it as a false lock: I had spelled a C#4 pitch as
"C#", so the authored nominal coincided with the pitch's own, and re-inferring
from the pitch gave the same answer. The test proved nothing about keeping the
author's choice. Rewritten around B#3-sounding-C4, where moving the nominal
gives F##4 and re-inference gives G, it now fails under S2 as it must.
Gate: fmt clean, clippy 0, 30 targets / 990 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8, zero golden churn, catalog rebuilds clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Audit finding 1, reproduced and fixed. Pitch::transposed did its arithmetic in
i32 while the interval's own components are i32, so intermediates overflowed:
diatonic_steps = i32::MAX panicked at `12 * new_octave`, chromatic_steps =
i32::MAX at `semitone + c`. TranspositionInterval::inverse negated i32::MIN.
The comment above the arithmetic even said "widen before arithmetic" -- it
widened i8 to i32, which is exactly not wide enough. All of it now widens to
i64, where the largest intermediate is bounded by ~3.7e9.
Refusing is the contract. Panicking on a value the public type admits is not.
I checked whether this was worse than a panic. The workspace sets
overflow-checks = true in release, but epiphany-core is a library and a
consumer's default release profile has them off, where these expressions wrap.
A 10.5M-case sweep of wrapping-vs-exact arithmetic (175 base pitches x 60225
interval pairs, edges plus random) found ZERO inputs where wrapping produced a
wrong Ok rather than a refusal. So this was a panic, not silent corruption, and
the audit's characterisation was exactly right.
inverse() now returns Option: -i32::MIN is not an i32. An interval whose
inverse cannot be written down is a fact about the type, and a caller composing
undo out of inverses must see it. Both regressions mutation-verified by
restoring the i32 arithmetic and the bare negation.
Also in this commit, two documentation corrections:
- The reducer's test-harness comment claimed both old transpose_* tests "now
reduce ONTO a base and assert the pitch value". They do not, and should not;
DECISIONS.md already said so. The comment now matches.
- The catalog's undo semantics for BOTH transpose kinds claimed value-restoring
undo recovers the pre-transpose pitch from the write chain. Neither kind
records into that chain, so this was false. The frozen Transpose reverts to
its honest pre-Push-4a statement -- undo does not negate it (P11-C8) -- and,
per the freeze doctrine, that is now pinned: making it record would change
what a stored {Transpose, UndoTransaction} history replays to. TransposeInterval's
paragraph states the requirement the next commit implements.
Gate: clippy 0, 30 targets / 985 passed / 0 failed, docs 0 under -D warnings,
conformance 8/8, zero golden churn, catalog rebuilds clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two open candidates, staged until a third reopens the pass.
P13-S1: 169 of core_spec's 207 requirement blocks carry no \label, so no
conformance claim can cite them. The audit that surfaced this scoped it to
Chapter 4 (9/9 unlabeled). It is not a Chapter 4 problem: Determinism Contract
is 15/15, Semantic Operations 24/27, The Score Graph 22/28, Pitch 10/13. Every
req:* label this repo cites was added ad hoc by whichever pass needed it.
P13-S2: cmn-24 is in the built-in pitch-space table as "CMN extended with
24-EDO quarter-tone accidentals" and cannot be represented -- Cmn.alteration is
an i8 of whole semitones, and a quarter-tone is half of one. Blocks Push 4b.
Also parked: the Push 4b prerequisites. Push 4a proved transposition needs no
tuning catalog; what the catalog is still for -- resolving a scale position to
a frequency, applying an instrument's transposition at the written/sounding
boundary -- remains, and Chapter 4 is not implementable as written.
Two further claims from the audit are recorded as UNVERIFIED rather than
inherited: that the JI dimension convention conflicts with its own prime-2
requirement, and that the named historical tunings lack exact ratio data.
Neither was needed for 4a; neither was confirmed. An audit finding that has not
been executed is a hypothesis.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
req:opcat:transpose-frozen says a conforming implementation MUST NOT emit
Transpose from new authoring. testkit's editloop::sharpen -- the [7c] UI-seam
gate's editing action -- was emitting exactly that, so the conformance suite
itself would have violated the requirement the same push introduced. It now
authors TransposeInterval with (0, +1): the same alteration shift, the same
staff line, the same rendered result.
The random-kind corpora in generators.rs keep producing the frozen Transpose,
and should: it must reduce correctly forever, and a generator is now the only
thing that will ever produce one. The distinction is authoring versus replay.
This is not inert. TransposeInterval also writes the Propagated spelling
attachment the frozen operation never did, so the edited score genuinely
changes shape. the_sharpen_records_the_spelling_it_propagated pins that, and
was mutation-verified by removing graph_propagate_spelling.
Gate: fmt clean, clippy 0, 30 targets / 983 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8, zero golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closes P12-K2. The reducer, the payload at wire discriminant 30, and the
editor authoring that emits it.
TransposeIntervalOp carries targets: CanonicalSet<PitchId> -- a set at the
type level, not a Vec plus a dedup() someone can forget. PitchId's Ord is its
canonical byte order, so a BTreeSet iterates in canonical order and cannot
hold a duplicate. Encoding it is the wire table's seq-strictly-increasing by
construction. The frozen Transpose keeps sorted_canonical and its multiset.
Reduction refuses atomically. Every mutable target is resolved before any is
written, so an untransposable one leaves the whole chord alone -- a chord
transposed except for one note is a different chord. Tombstoned and
SYSTEM_DERIVED targets are still skipped: a deleted pitch is not an
untransposable pitch, it is one the operation has nothing to say about. The
three refusals map to PitchSpaceMismatch (6, un-reserved -- detecting a
non-Cmn position reads a discriminant, never the tuning catalog its doc
claimed to need), AcousticRealizationPinned (14), TranspositionOutOfRange (15).
The refusal reads pitch values, which exist only under reduce_onto, so it is a
graph-aware-only precondition that passes base-free -- the convention
modify_identified_pitch's system-derived check already set. It writes nothing
base-free either, so both modes agree on objects, and on the effect log for
every operation whose targets are all transposable, which is all base-free
reduction can see.
Spelling propagates. Core Ch2 requires transposing operations to produce
Propagated attachments; Transpose produced none, so an authored spelling
survived a transposition still pinned to the notehead it was written against.
simplest_spelling on a Cmn position returns the authored letter verbatim, so
the attachment carries exactly what the interval's diatonic component decided:
a diminished sixth up from C4 records A-double-flat, not the enharmonic G.
Editor. transpose_selection now takes a TranspositionInterval; a scalar cannot
tell "up an octave" (7,12) from "C with twelve sharps" (0,12), which is
P12-K2 itself. The "+1 semitone" key became alter_selection(+-1). TransposeOp
is now unused in editor-core's lib, so the compiler enforces "never authored".
Tests, five mutations verified: the graph write removed; the refusal made
non-atomic (skip the offender, move the rest); spelling propagation dropped;
and -- for the freeze -- graph_transpose_pitch "helpfully" repaired to use the
real algebra, which the_frozen_transpose_keeps_its_saturating_alteration_
semantics correctly rejects. That test guards against rewriting history, not
against a bug.
The two old transpose tests were false locks, but the fix was not to rewrite
them as the design gate promised. What they assert -- skip-tombstoned,
skip-system-derived, refuse-missing -- are effect-log properties, correctly
checked base-free. The defect was one test's NAME: it claimed the live target
"shifts" and checked nothing of the kind. Renamed to say what it proves; the
shift itself is now locked by two graph-aware tests against reduce_onto.
fuzz::gen_payload gained arm 27, so below(27) became below(28) and the seeded
stream reshuffled; the canonical-base digest is re-pinned consciously, per
that test's own instruction and the Phase-D precedent. Nothing leaked --
canonical_bytes embeds effects, conflicts and anomalies, never payload values.
The frozen Transpose keeps fuzz arm 6 and its testkit corpus authoring: it
must reduce correctly forever, and a generator is now the only thing that will
ever produce one.
Gate: fmt clean, clippy 0, 30 targets / 982 passed / 0 failed, docs 0 under
-D warnings, conformance 8/8, zero golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
req:pitch:transposition, implemented as Pitch::transposed. With n the
nominal's normative discriminant and s = nominal.chromatic() + alteration +
12*octave:
nominal' = CmnNominal((n + d).rem_euclid(7))
octave' = octave + (n + d).div_euclid(7)
alteration' = (s + c) - (nominal'.chromatic() + 12*octave')
The diatonic component alone picks the nominal and octave; the alteration
absorbs the residue. C4 + (7, 12) is C5, not C with twelve sharps. C4 + (0, 1)
is C#4, so the editor's sharpen is unchanged. A perfect fifth (4, 7) and a
diminished sixth (5, 7) from C4 give G4 and A-double-flat-4: same sound,
different spelling, which is the whole reason the interval is a pair.
TranspositionInterval moves from graph.rs to pitch.rs. It already existed --
Instrument.transposition has carried it since schema major 2, already codec'd,
already exported -- and it is byte-for-byte the pair this needs. The spec now
declares it once, in Chapter 2 where transposition lives, so the struct sits
where its algebra does. Wire form and codec are untouched.
Its doc claimed it was "ADVISORY until the Chapter 4 tuning catalog pins
interval algebra". That was the P12-K2 false coupling repeated a third time.
The field stays advisory because nothing applies it at the written/sounding
boundary -- for that reason, not for want of algebra, which needs no catalog.
Refusal, never saturation: TransposeRefusal::{NonCmnPosition, AcousticPinned,
OutOfRange}. An AbsoluteHz pitch overrides the tuning system, so transposing
it would move the notehead and leave the sound behind; the mutation that drops
that guard returns Ok(G4 ... AbsoluteHz(261.6)), a notehead reading G4 that
still sounds middle C. TranspositionInterval::inverse is exact precisely
because nothing saturates.
Six tests, four mutations verified: the old alteration-shift semantics, a
clamp in place of the refusal, a truncating `/` in place of div_euclid (which
breaks C4 down a step to B3), and dropping the AbsoluteHz guard.
One process note. The AbsoluteHz mutation appeared to survive on the first
run. It had not: rustfmt had wrapped the matches! across three lines, so the
string replacement silently matched nothing and the test was never challenged.
A mutation that no-ops looks exactly like a test that passes. Mutations now
assert their anchor before applying.
Gate: clippy 0, core 165+5 passed / 0 failed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An audit reopened P12-K2, whose Pass-12 pin promised the repair would be "a
payload schema-major landing with the Chapter 4 tuning catalog". Both halves
were wrong, and the operation was more broken than the pin admitted.
Measured through EditorSession, not inferred. On a C4: +12 yields alteration
12 (six double-sharps, not C5); +128 clamps to 127 and still reports Applied;
targets [p, p] transposes twice; a non-Cmn position is silently untouched.
transpose(1000) then transpose(-1000) lands on -128, so the operation is not
invertible. Nothing downstream is at fault -- prepass::accidental_ids renders
alteration 12 faithfully. The defect is entirely in what Transpose means.
The false coupling is why this looked big. Pitch has orthogonal scale_position
and acoustic fields. Transposition adds an interval to a scale position;
tuning decides what frequency a scale position sounds at. Adding a fifth to C4
needs no tuning catalog. The same weld had spread: PitchSpaceMismatch was
"Reserved: requires the Chapter 4 tuning catalog" (it reads a discriminant),
and TranspositionInterval was "ADVISORY until the Chapter 4 tuning catalog
pins interval algebra". Push 4 splits: 4a is the algebra and needs no catalog;
4b is the catalog, which has its own blockers (cmn-24 is in the pitch-space
table but cannot exist while Cmn.alteration is i8 semitones).
Ratified by the user:
- New kind, freeze the old. An operation is history; a corrected reduction
rule would rewrite every score that used one. Transpose (disc 9) keeps its
exact semantics, now written as normative replay semantics rather than as
apologies. TransposeInterval takes disc 30. This is cheap: appending a kind
at >= 30 is a schema MINOR, and the payload's constituents are all major-0
layouts, so it stamps major 0. No major 3, no migration.
- Diatonic + chromatic interval, reusing TranspositionInterval -- which
already existed in graph.rs at major 2 for Instrument.transposition, already
codec'd, byte-for-byte the required pair. Minting an Interval beside it
would have been a second normative listing of one type, the drift P13-I1
just closed. Declared once now, in Chapter 2; Chapter 5 references it.
- Atomic refusal. Non-Cmn, AbsoluteHz, or an out-of-range result refuses the
whole operation. Never saturate, never partially apply. Tombstoned and
SYSTEM_DERIVED targets are still skipped: a deleted pitch is not an
untransposable pitch, it is one the operation has nothing to say about.
targets becomes CanonicalSet<PitchId> at the type level, not a Vec plus a
dedup() someone can forget (PitchId's Ord is its canonical byte order). This
was never a convergence bug -- every replica replaying [p, p] double-
transposes identically -- but a canonicalization one. It is free today because
no operation-payload decoder exists yet; once one lands in Push 5, dedup
normalization would change the meaning of stored operations. Push 4a blocks
Push 5, and that is why.
Spec: req:pitch:transposition (algebra + the three refusals), and four
req:opcat:transpose-* requirements. Operation Catalog 0.7.0 -> 0.8.0; Binary
Format 0.6.0 -> 0.7.0 (disc 30, and a seq-strictly-increasing notation whose
decoder must reject a duplicate rather than normalize it away).
This commit is the design gate: the spec now declares MUSTs the code does not
yet satisfy -- editor-core still authors Transpose, and TransposeInterval does
not exist. The implementation follows in this push.
Also recorded: the two existing transpose tests are false locks. Gutting
graph_transpose_pitch leaves both green -- they call base-free reduce(), where
graph is None and the function never runs, and assert only OperationEffect.
Only editor-core's undo_and_redo_a_transpose, three crates away, catches it.
Gate: clippy 0, 30 targets / 964 passed / 0 failed, docs 0 under -D warnings,
conformance 8/8, all three spec documents build with no undefined references.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BRAVURA_METRICS' NOTEHEAD_ANCHORS were hand-derived where every neighbouring
number in the table is machine-extracted from the SHA-pinned font -- the same
mistake as inferring band ownership downstream instead of reading it from the
source that had it. And they were wrong under any reading: they named
stemUpNW/stemDownSE, the corners a normal notehead's stems do not attach to and a
pair Bravura's noteheadBlack does not define, with an x of 1180 that reads as 1.18
staff spaces written in thousandths rather than the table's 1/1024 units (1.18 sp
= 1208). Nothing consumed them.
Deleted rather than corrected. The font is not vendored, so the true values cannot
be verified in-tree, and shipping data we cannot stand behind into a hash every
conformance claim declares is worse than shipping none. Verified empirically
before deciding, not reasoned about: changing the anchors breaks nothing in-tree
-- 30/30 targets, zero golden churn, no pinned literal hash -- so
GlyphCatalogIdentity moves once, now, while no claim declares the old one.
extract_bravura_outlines.py gains --anchors, emitting them from the pinned
bravura_metadata.json (anchors live in the SMuFL metadata, not the glyf bounds,
which is why the outline extraction never covered them). SMuFL anchors are points,
so they round to nearest -- unlike a bbox, which rounds outward so the metric box
contains the ink. The metadata's SHA-256 is deliberately left UNPINNED and
verify() now refuses an unpinned source, printing the digest to paste: the script
cannot regenerate anchors until an operator with the font pins it in a reviewable
commit, which is the discipline the other two sources already have.
The test guarding the anchors proved nothing. anchors_participate_in_the_hash
compared noteheadBlack (anchored) against noteheadWhole (not) and asserted they
hash differently -- but their advance and bbox differ too, so it would have passed
with the anchors ignored entirely. It now varies the anchors while holding every
other field fixed (presence, one coordinate, one name), against synthetic metrics
through a factored-out metrics_hash_of.
Pass-13 Batch 2 CLOSED. All three candidates resolved; two grew when examined.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
to_constrained took the active clef from the staff instance's clef_sequence and
fell back to Clef::default() -- treble. A staff that declared its clef only on the
Staff, with no ClefChange, therefore drew a treble clef and placed every note
against it. The field was decorative in the projection.
It is the fallback. StaffContent now carries default_clef -- the clef belongs to
the Staff, the sequence to the StaffInstance, and resolving "the clef at time t"
needs both -- and active_clef_or(clefs, at, default) resolves against it.
active_clef remains as that with the treble default, for callers with no staff to
hand, so the public API is intact.
The part worth pausing on: epiphany-editor-core reads the same function for
hit-test pitch resolution. Fixing only the projection would have left a click on a
bass staff resolving its pitch as treble -- the engraved clef and the editor
disagreeing about what note is where. Both now go through active_clef_or.
Removal was rejected: the field is named for its purpose, is encoded on the wire,
and dropping it would be schema-major.
Zero golden churn: every fixture and generator declares treble, which is also
Clef::default(), so nothing that exists today moves. Locked by
a_staff_declaring_only_a_default_clef_engraves_in_it and mutation-verified by
restoring the Clef::default() fallback. Scoping that test by provenance was
necessary -- valid_score_rich has three staves and only one was re-clefed, so the
first assertion I wrote ("no gClef anywhere") failed against a correct fix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three parked candidates accumulated while the Standard-tier solver track closed
and the notation-quality pass landed, which is the threshold at which the house
rule opens a batch pass. PASS13_CANDIDATES.md reopens as Batch 2 with P13-I1
(this), P13-I2 (Staff::default_clef never consulted), P13-I3 (the notehead stem
anchors).
P13-I1 was filed as "two elided fields". Checking before writing turned up a
third, and it is the one that matters: `diagnostics: Vec<LayoutDiagnostic>`
appears NOWHERE in core_spec, though it is how the projection's honesty rule
manifests. So Chapter 7's listing gains break_origins, diagnostics, and catalog;
BreakOrigin and LayoutDiagnostic gain their shapes.
And it gains req:layoutir:coverage-diagnostics, ratified as implemented: an
object the projection cannot engrave faithfully -- a pitch with no resolved
spelling, a glyph the bound catalog does not carry -- MUST be recorded as a
LayoutDiagnostic AND still placed, as a fallback notehead or a zero-extent traced
anchor. It MUST NOT silently substitute a plausible shape, and MUST NOT drop the
object. Both halves matter: dropping it breaks the round-trip surjection, so a
hit-test can no longer find what the author wrote; guessing produces a score that
looks engraved and is wrong, with nothing in the IR to say so.
Locked by an_unengravable_object_is_recorded_and_still_placed, which drives a
percussion clef (no bundled glyph) and asserts all three halves: the diagnostic
names the object, no glyph stands in for it, and a zero-extent anchor keeps its
provenance addressable.
Spec rebuilds clean, no undefined references, new label registered. No code
change beyond the test; zero golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The rendered slurs were wrong in three independent ways, all visible in the
two-staff and three-staff goldens.
1. Side. SlurDirection::Auto always arced above. The single-voice rule is
OPPOSITE the stems -- all stems up puts the slur under the noteheads, all
down puts it over them, and a mixed-stem span (which has no notehead side)
goes above. Every Auto slur over a stem-up passage was drawn through its own
stems. This is why stem direction had to land first: with every stem pointing
up, "opposite the stems" means nothing.
2. Endpoints. They sat at staff_top + gap -- a constant offset from the STAFF,
not from the notes -- so a slur between two C6s hung below its own noteheads
and crossed their ledger lines. They now sit a gap outside the endpoint
column's ink, at the notehead's centre. Where the stem points the same way as
the slur, that ink includes the stem, so the endpoint clears the stem tip.
3. Clearance. The apex was span-proportional and blind, so a note between the
endpoints poked straight through the arc. ColumnInk -- per staff, per column:
top, bottom, stem direction, notehead centre -- is the obstacle field. The
control points sit on the chord at thirds, so x is exactly linear in t and
the arc's departure from the chord is 3*lift*t*(1-t); a column at t needing d
more clearance forces an apex of at least d/(4*t*(1-t)).
An authored height is a floor, not a ceiling: clearance may raise it, so obeying
an author cannot draw a slur through a note. An authored direction still wins.
Obstacles are measured at the notehead CENTRE, the same x the endpoints use. The
first cut used the raw column x, which skews t and silently over-lifts: the
two-staff slur cleared its C6 by 4.05 spaces where 3.5 was needed. The clearance
test now asserts an upper bound as well as a lower one.
SLUR_INSET is gone. Endpoints at the notehead centres are what its 0.6-space
"tuck" approximated for the start point -- and got wrong for the end, where it
tucked a full notehead width to the LEFT of the final note.
Four mutations verified: always-above, staff-relative endpoints, no clearance
pass, and obstacles at the column x. The staff-relative-endpoint mutation PASSED
at first -- the tests asserted only "above the staff" / "below the staff", which a
staff-relative endpoint satisfies by construction. The exact-endpoint assertion
exists because that mutation survived.
Projection change, so no version moves; goldens churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every stem in the engine pointed up, on the notehead's right, at a constant
octave. A C6 three ledger lines above the staff grew an upward stem shooting past
everything -- visible in the two_staff and three_staff renders. It is also why the
slurs are wrong: an Auto slur is placed OPPOSITE the stems, so no correct side
could be chosen while every stem pointed the same way. Stems are therefore a
prerequisite for the slur fix, not merely adjacent to it.
- Direction: away from the middle line, decided by the head furthest from it,
ties going down (the convention for a note on the line, and for a chord that
straddles it evenly).
- Attachment: the side it points -- an up-stem at the lowest head's right edge,
a down-stem at the highest head's left. Read from the head's own bounding box
rather than the rounded NOTEHEAD_STEM_X: for noteheadBlack those are 1.1807
and 0, and 1.1807 is Bravura's real stemUpSE (1.18). The old 1.15 was a
rounding -- it accounts for the whole of ten_measure's churn, where every stem
moved right by 0.031 and nothing else moved at all.
- Length: an octave from the outer head, but drawn out to the middle line when
the note lies beyond it, so no stem dangles in the ledger field.
No version moves. This is the PROJECTION changing, not a solver: to_constrained
emits different geometry from the same graph, so ENGRAVER_VERSION's promise (same
input => same output) is untouched. Every golden churns, stub and engrave alike,
because stems are constrained-stage geometry.
Locked by a_stem_points_away_from_the_middle_line_and_reaches_it, which re-pitches
a generated score across four octaves -- the corpus generator writes only low
notes, so nothing in the suite had ever exercised a down-stem. Mutation-verified:
restoring always-up plus fixed-octave fails it.
Parked, found on the way: BRAVURA_METRICS' notehead anchors are unusable as
written. They name stemUpNW/stemDownSE -- the corners an up- and down-stem never
attach to -- and their x of 1180 reads like 1.18 staff spaces written in
thousandths rather than the table's 1/1024 units (1.18 sp = 1208). Nothing
consumes them except metrics_hash, so correcting them would move the
GlyphCatalogIdentity every conformance claim declares. That makes three parked
candidates; the house rule opens a batch pass at three.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ENGRAVER_VERSION 11 -> 12. The inter-staff solve now closes a slack pair as well
as opening a crowded one, realizing the InterStaffGap band's declared height
exactly. SYSTEM_STAFF_PITCH is demoted from a floor to an initial arrangement the
solve fully renegotiates. This is what vertical_density_penalty was reporting: an
un-pressured multi-staff system sat at 0.739, honest sprawl against the declared
gap, because the axis is symmetric and the solve only ever expanded.
The band's height had no agreed meaning, so pin it: it is an INK CLEARANCE -- the
separation between the two staves' outermost content, exactly the unit
req:qmc:vertical measures. preferred 2.0 -> 5.0, min 1.0 -> 2.0. The old 2.0 was
a placeholder reconciled with nothing: neither the 8.0 staff-box gap the fixed
pitch of 12 produces, nor the ~6.4 ink clearance it leaves for plain content.
Realizing it would have crushed a relaxed system to a pitch of ~7.6. At 5.0 plain
ledgered content settles near a pitch of 10.6.
Making the solve two-sided immediately exposed a CASCADE DEFECT latent since v11.
The recurrence subtracted the upper staff's shift from the measured gap and then
added it back through the accumulator, so every pair below the first was
over-separated by exactly the shift above it. Both staves move; the relation is
shift_lower = shift_upper + target - (upper_lo - lower_hi), the UNSHIFTED gap.
three_staff_close_content's lower pair realized 21.06 against a declared 4.0. It
was invisible on two-staff fixtures (shift_upper = 0) and invisible to
inter_staff_shifts_cascade_down_three_staves, which asserted only s2 > s1 -- true
under both the correct and the double-counting recurrence.
What caught it was the metric measuring realized clearance back from the BAKED
output instead of the solve's own extents. Reading back solver intent would have
reported 0 and shipped the over-separation again. That design choice was made one
commit earlier for exactly this reason; the catalog rationale now recommends it to
any conforming implementation.
Once the solve realizes each declared clearance exactly, every inter-staff unit is
0 on a healthy solve -- the axis becomes a solver self-check, and its MEAN can no
longer distinguish "measured every realization" from "measured one". So
vertical_raw is split into vertical_units and the regressions assert the unit SET.
Four mutations verified: the double-counting recurrence, expand-only, the
glyph-members band filter, and first-system-only measurement each fail a named test.
No normative change, no version move: QMC formula, units, anchors, thresholds all
untouched; only its non-normative rationale is refreshed. Churn is the two
multi-staff engrave goldens: two_staff grew by exactly 3.0 (the target change, no
cascade); three_staff SHRANK by 9.06 -- the same +3 per pair, less the 17.06 of
over-separation the defect was adding. Single-staff and every stub golden are
byte-stable.
The 5.0 was the user's call. 4.0 ("one staff height") was chosen first and
withdrawn once its true consequence -- pitch 9.57, not the 11.04 an arithmetic slip
of mine had projected -- was measured rather than inferred. The slip: deriving
plain-content ink clearance from an aggregate metric by assuming two contributing
units when it had three.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-up on 5b016f3. Bumping the catalog to 0.3.0, I ran a mechanical
find-and-replace over the version string and did not reread the sentences around
it. Result: layout-ir's quality.rs credited the spacing_distortion refinement to
v0.3.0 when that was v0.2.0, and engrave's quality.rs still announced v0.2.0
while implementing the v0.3.0 unit set.
Each version now says what it actually did: v0.2.0 narrowed spacing_distortion's
measurement DOMAIN to rhythmic columns; v0.3.0 narrowed vertical_density_penalty's
CONTRIBUTING UNITS to one per realization of a gap band. Both narrowed what is
measured over, not what it is normalized against -- so the transcribed constants
(anchors, thresholds, warning fraction) have not moved since v0.1.0, which is the
invariant worth stating and the reason the two revisions were safe.
Swept for the same rot rather than fixing only the two reported. Three more
present-tense claims pinned a version that will keep going stale: engrave and
layout-ir DECISIONS both said "the companion (v0.2.0) ratifies/pins the nine
axes", and the reference suite claimed RS-1 passes "under engrave v3, QMC v0.2.0
anchors" -- engrave is at 11. All three now cite the current version and name what
is actually invariant across revisions. The remaining "QMC 0.1.0 -> 0.2.0"
mentions are historical records of what that revision did and stay as they are.
Docs only. Gate green; zero golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-up on 43215c7. Both findings were right, and the second falsified a
comment I wrote in that very commit.
1. Region staff bands were still identified by glyph `members`. vertical_raw
measured content over all primitives but decided WHICH staff bands belong to a
region by glyph membership -- reintroducing the assumption the change exists to
shed. A staff band is allowed to own no glyphs: to_constrained emits one per
staff of the region regardless, and a percussion-clef staff (no bundled glyph,
so it engraves to a traced anchor stroke) with no notes owns only staff-line
strokes. Membership now comes from content presence in one of the region's
systems, which identifies the band exactly -- a staff band is per-(staff,
region), so its content can land nowhere else.
2. Only the first realizing system was measured, justified by a comment claiming
rigid system translation makes every realization agree. The inter-staff solve
had just falsified that: it sizes each system's gaps from that system's own
content. req:qmc:vertical now counts ONE UNIT PER REALIZATION, matching how
realized inter-system gaps were already counted. That is a contributing-unit
change, so unlike 43215c7's clarification the catalog moves: QMC 0.2.0 -> 0.3.0
(the P12-I12 precedent). Raw formula, anchor, orientation, thresholds unchanged.
Two new fixtures, because an unexercised fix is what I criticised last round:
percussion_placeholder_staff (a valid, invariant-clean score whose lower band owns
zero glyphs and six strokes) and two_staff_wrapping_pressure (one region, two
systems, staff-line gap 15.93 where it collides and 7.87 where it is slack). Both
mutation-verified: the members filter scores 4.8e-7, first-system-only scores
1.3e-7 -- each ~0 where the corrected axis reports real deviation.
What the per-realization count exposes is not comfortable, and is recorded rather
than smoothed over: two_staff_wrapping_pressure now scores 0.739. Its pressured
system solves to the declared gap exactly; its slack system sits at ~5 staff
spaces against a preferred 2.0. The axis is symmetric -- a gap wider than
preferred is sprawl exactly as a narrower one is crowding -- and this solve only
expands, never compresses. The deferral "compressing an OVER-wide fixed gap toward
preferred ... rarely wanted" is promoted to measurably wrong. Named, not fixed:
compression is a layout change (golden churn, ENGRAVER_VERSION move), not a
measurement one.
Adjacent finding, parked: Staff::default_clef is never consulted -- to_constrained
takes the active clef from the instance's clef_sequence and falls back to
Clef::default() (treble), so a staff declaring its clef only on Staff engraves as
treble. Verified (no layout-ir consumer reads the field). Filed in layout-ir
DECISIONS with the ConstrainedLayoutIR listing gap, pending a >=3-candidate batch.
Measurement-only: no layout change, ENGRAVER_VERSION stays 11, zero golden churn.
Gate green; QMC PDF rebuilds clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two items the inter-staff solve deferred turned out to be one thing -- and it was
not the "metric-vs-solver tension" I filed it as. The catalog was right; the
engraver was non-conforming.
req:qmc:vertical has always defined the realized inter-staff gap as the separation
"between the adjacent CONTENT EXTENTS the band separates". vertical_raw measured
the separation between the two bands' glyph `members`, because until primitive
band ownership (efaebb9) a band listed no strokes or curves to own. A staff's
outermost ink is usually not a glyph. On two_staff_close_content the solve cleared
the declared 2.0 gap exactly, while the glyph-ink gap was 5.06 -- so the axis
reported |5.06-2|/2 = 1.53, saturated to 1.0, and fired a Standard-tier floor
warning on a correct layout. The metric was charging the solver for the ledger and
slur ink it had made room for. Axis now reads 2.7e-7; the warning is gone.
Two design calls worth naming:
- The geometry is read back from the BAKED output, not from the solve's own
staff_ext. Reading back solver intent would make the axis circular and blind
to exactly the bug class that bit twice this week; now a shift the bake fails
to apply to some primitive class surfaces as a real deviation. CastLayout
gained stroke_system/curve_system for it -- a stroke carries no spring slot,
so system_of_slot cannot answer for it.
- The solve now targets the preferred_height of the InterStaffGap band
to_constrained emitted for that staff pair, not VerticalBand::inter_staff_gap's
default. That is what makes the band a height model rather than a constant:
solve and metric agree by construction, not by both calling one constructor.
NO version move. Formula, contributing units, anchor, and normalization are
unchanged -- only a wrong measurement was. This is the P12-I11 precedent
(engrave-side resolution), not P12-I12 (which redefined spacing_distortion's unit
and did move the catalog). The catalog gains a clarification of what "content
extent" means, since before band ownership that reading was arguably
unimplementable, which is why the defect survived. Its stale rationale (still
claiming the vertical spring solve is deferred) is refreshed, and the axis's
inter-system half is recorded as a genuine trade-off against page_fill_efficiency
rather than a defect, so it is not re-filed as a bug.
Measurement-only: no layout change, ENGRAVER_VERSION stays 11, zero golden churn.
Mutation-verified: reverting vertical_raw to glyph-only measurement scores 1 and
fails the new assertion. Gate green, QMC PDF rebuilds clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ced4b72's ratification log said the ConstrainedLayoutIR listing gap was "tracked
as a Pass-13 candidate", but PASS13_CANDIDATES.md declares that batch CLOSED with
no open candidates -- and the house rule opens a pass only at >=3 candidates. The
claim pointed at a ledger that says the opposite.
Filed where the rule wants it instead: layout-ir DECISIONS records the two fields
the listing still elides (break_origins, catalog), why neither blocks an
implementer (both are governed by requirement text elsewhere), and that it is
PARKED awaiting company rather than reopening a pass on its own. The log now says
that too.
No code, no spec text.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-up on fc411ea. Finding 1 was correct and was mine to own: I wrote
"every primitive -- glyph, stroke, and curve alike -- MUST declare a
VerticalBandId" and placed it in the ResolvedLayoutIR section, where the adjacent
ResolvedGlyph listing has no such field. Under a literal read the implementation
was non-conforming against a requirement I had just added.
The rule belongs to the stage that has the obligation. req:layoutir:primitive-
band-ownership moves to Ch7 §ConstrainedLayoutIR §Vertical Bands and is scoped to
"every primitive the projection presents to the solver". A new companion,
req:layoutir:resolved-band-ownership, says what survives: a resolved Stroke/Curve
MUST retain its band (casting-off and the inter-staff solve both relocate them
and must attribute them), while a ResolvedGlyph carries none -- its ownership is
consumed during the solve and already baked into its resolved position, so the
field would be dead weight in a rendering fingerprint. That requirement also
carries the non-canonical clause: canonical_bytes omits vertical_band even from
the primitives that do retain it.
Finding 2: the ConstrainedLayoutIR listing gains strokes/curves. They have been
in the code since staff lines and were never listed -- so an implementation
following Chapter 7 could not have supplied non-glyph ownership to the solver at
all, which makes this the listing gap that actually blocked the new MUST.
Finding 3: the ratification trail now records the tranche -- a revision-history
row and a PASS12 log section naming both requirements, the three listing
corrections, the two commits, and the fact that adopting it churned no golden
(the declared owner agrees with the inferred one across the corpus, which is what
licensed the swap).
The log also records a pre-existing listing gap I found while checking my own
citations: ConstrainedLayoutIR still elides break_origins and catalog. I had
first written that neither is named anywhere in core_spec, then verified and
found both types ARE specified (req:layoutir:break-origin-attribution; Ch7 §Glyph
Catalog Identity) -- only the struct listing is incomplete, and unlike
strokes/curves neither gap blocks an implementation. Left as a Pass-13 candidate
rather than widened into this tranche.
Spec rebuilds clean: no LaTeX errors, no undefined references, both new labels
registered. Workspace 30/30, conformance 8/8; no Rust changed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-up. Both findings were about documentation lagging a contract the
code now enforces, and the first is the one that matters: band ownership is the
solver-facing correctness contract, so it belongs in the spec, not only in a
crate's DECISIONS.md.
Chapter 7's ratified Stroke/Curve listings gain `vertical_band`, and a new
requirement (req:layoutir:primitive-band-ownership) states the rule as a MUST:
every primitive declares its owning band, the projection MUST name a band that
exists, and a vertical solver MUST take ownership from that declaration and MUST
NOT infer it from geometry. The rationale is recorded because the requirement
looks like a convenience until you see why it isn't: a stem shares its column
with the staff above, and a slur's endpoints are lifted clear of its own staff
into the zone where the nearest notehead belongs to the neighbour. A solver that
guesses from proximity tears such primitives off their notes when it renegotiates
the staff gaps -- which is exactly what happened, twice. The requirement also
records that only glyphs are band *members*, and that the reference is
non-canonical attribution metadata.
Stroke's stale "the vertical-band model does not contain" gloss is dropped.
canonical_bytes' doc claimed to encode "the full resolved layout". It encodes the
rendering fingerprint and deliberately excludes attribution metadata. Stating
that precisely required checking rather than paraphrasing: ResolvedGlyph drops
its band on the way out of the constrained stage, while Stroke and Curve -- whose
types are shared with that stage -- carry vertical_band through but never encode
it. Two layouts differing only in band ownership draw identically and hash alike.
Spec rebuilds clean: no LaTeX errors, no undefined references, new label
registered. Gate green, no golden churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Stroke and Curve gain `vertical_band: VerticalBandId`, the field GlyphObject has
always carried. Curve's own doc comment used to call it "a *free* primitive (no
vertical band, no spring slot)" -- but the projection computed each primitive's
band, used it for the glyphs, and threw it away for the strokes and curves.
The engraver then reconstructed it geometrically, and got it wrong twice: nearest
glyph by x handed a lower-staff stem to the upper staff (4132a7a), and nearest
glyph to a slur's start endpoint handed a bottom-staff slur to the top staff
(b1bfe04). Both tore primitives off their own notes, both reached a committed
golden, both were caught by review rather than by the gate. A slur is the proof
the inference can never be made safe: its endpoints are deliberately lifted clear
of its own staff, into the zone where the nearest notehead belongs to the
neighbour. No distance metric recovers the owner.
Both fixes were correct and both were the wrong shape -- reconstructing by
inference a fact the projection had in hand and discarded. So: the projection
declares it (a slur's staff is its notes' staff), the engraver's attribution
becomes three map lookups, and ~60 lines of geometric rules, epsilons, and
fallbacks are deleted.
Two bands had to become unconditional, since strokes could otherwise name bands
that no glyph had caused to exist -- validation now rejects that as UnknownBand:
- a staff band per staff of the region, in the region's own staff order (the
order y_origin stacks by), not only for staves that emitted a glyph. A staff
whose clef is unbundled engraves to an anchor *stroke* and no glyph.
- the margin band unconditionally, because a region's own traced anchor is a
stroke that names it whether or not a margin glyph puts a member in it.
Both may carry zero members, as an inter-staff gap band already did: membership
realizes the spring solve over glyphs; existence is what attribution needs.
Strokes and curves are deliberately NOT added to VerticalBand::members -- their
band reference is one-way.
vertical_band is not part of ResolvedLayoutIR::canonical_bytes (primitives are
encoded field-by-field), so this is layout metadata outside the canonical
encoding: no companion-version bump, and ENGRAVER_VERSION stays at 11 because the
output is unchanged. Zero goldens churn -- which is the evidence that the declared
owner agrees with the inferred one across the entire corpus.
Locked by every_stroke_and_curve_names_a_band_that_exists, verified by mutation:
making the margin band conditional again fails it on valid_score_rich. The two
tear-off regressions are kept -- they now assert an outcome the data model
guarantees, which is where a dropped declaration would surface.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The reviewer signed off on the inter-staff slice with one named residual risk:
the 3+-staff cumulative shift cascade was documented and traced correct but had
no fixture behind it -- valid_score_rich's "three staves" are three separate
single-staff regions, so each lands in its own system and the cascade never runs.
three_staff_close_content puts three staves in ONE region with deliberately
asymmetric pressure: the upper pair collides hard (C1 against C7), the lower pair
only gently. That asymmetry is what makes the fixture discriminating. Sizing each
pair independently -- the plausible wrong implementation -- measures the lower
pair against the middle staff's ORIGINAL position, hands the bottom staff only
its own small correction, and drags it back up through the middle staff.
Verified by mutation, not by assertion alone: with the cascade removed the bottom
staff's shift collapses from 34.68 to 4.56 against the middle staff's 15.06, and
both the shift ordering and the staff-line-gap assertions fail. two_staff_close_content
still passes under that same mutation, which is precisely why the new fixture was
needed.
Writing the test also corrected a wrong mental model, now recorded in DECISIONS.md:
a shift INCREMENT generally exceeds the lower pair's own raw correction, because
the upper staff's descent has itself eaten into that pair's gap and must be undone.
The first version of the test asserted the opposite and failed against a correct
solve.
The fixture additionally pins curve attribution against a three-band choice -- the
slur must still find the bottom staff, not merely the nearer of two -- and carries
its own render golden. No existing golden churns.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The stem tear-off repaired in 4132a7a had a twin one layer deeper, live in
the same fixture and baked into the same golden: the bottom staff's slur was
attributed to the TOP staff, kept shift 0, and tore off its own notes.
A distance metric cannot fix this one. A slur's start endpoint is deliberately
lifted off its notes -- staff_top + gap above, staff_bottom - gap below -- into
the inter-staff zone, where the nearest glyph is routinely a note on the
adjacent staff (here, a top-staff ledger note). So attribute a curve the way it
is drawn: the arc's direction picks the side. An upward arc (p1.y >= p0.y) hangs
below a staff -- take the greatest staff-line band bottom at or above p0.y; a
downward arc sits above one -- take the smallest band top at or below p0.y.
Fall back to the nearest band mid-line for a curve inside a staff.
Locked by a_slur_travels_with_its_own_staff, verified to fail without the fix
(d_bottom=8.695 > d_top=7.235 -- the slur riding the wrong staff).
Also assert the two metrics the solve moves rather than leaving them untested:
collision_penalty is 0 (the staves separate cleanly) and vertical_density_penalty
saturates at 1.0 -- the solve targets content extents while the metric scores the
realized gap against the band model's preferred height. That is the same
metric-vs-solver tension as casting_off under justification; DECISIONS.md records
it, plus three further reviewed gaps: staff-less content takes shift 0, the
preferred gap is read from the constructor rather than the region's declared
band, and the 3+-staff cumulative cascade is correct but unexercised.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Self-review of the inter-staff solve found a real bug: stroke->staff attribution
reused component_glyph, whose fallback picks the nearest glyph by X ALONE. That
is correct for a SLOT — both staves of a system share their x columns, hence
their spring slots, so the horizontal delta is the same either way — but wrong
for a STAFF: it handed a lower-staff stem to the UPPER staff's notehead. The
stem then kept the wrong vertical shift and tore off its own head (measured
worst stem->notehead distance 5.837 on the two-staff fixture vs 1.150, the stem
x-inset, on the single-staff one), and it polluted the upper staff's content
extent, inflating the computed gap.
Fix: the staff attribution uses a 2-D nearest for that fallback (a ledger still
resolves via owning_glyph's shared Pitch source; a staff line via its Staff
source). component_glyph is unchanged and still serves the horizontal path.
Also corrected: staff-attributed primitives now contribute their y ONLY through
the shifted path (Extent::add_x for x, add_y for the shifted staff extent), so a
lower staff's UNSHIFTED content can no longer inflate a system's max_y. Dead
Extent::add removed.
The corrected attribution yields a smaller, more accurate separation (two-staff
view_box height 36.1 -> 31.1). Regression multi_staff_stems_stay_on_their_own_
staff (verified to fail at 5.837 without the fix). Only the two-staff engrave
golden churned; single-staff goldens byte-stable. 948 tests, clippy 0, docs
-D warnings, conformance 8/8.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The last vertical-spring piece: the gaps BETWEEN a system's staves are
renegotiated so tightly ledgered or slurred adjacent staves — which the
constrained stage stacks at a fixed pitch — separate. ENGRAVER_VERSION 10 -> 11
(a multi-staff score whose staves press together shifts them apart; a
single-staff score, with no inter-staff pair, is byte-identical).
Attribution (vertical_band + owning-glyph, per the chosen approach): a glyph via
its vertical_band (VerticalBandKind::Staff -> StaffId); a stem/ledger via its
notehead (component_glyph); a staff line via its Staff source; a slur via the
notehead nearest its start. Spacing is horizontal-only, so a primitive's y is
unchanged from the source frame the attribution reads.
The solve: per system, per staff, collect the real content y-extent (glyphs,
strokes, curves — ledgers and slurs included); order staves top-to-bottom by
their staff-line reference y (order fixed); shift each staff down by the
cumulative amount needed to bring its gap to the one above up to the band
model's preferred inter-staff gap. staff_shift[(system, staff)] is a per-staff
dy the bake applies (Placement::sunk) atop the per-system dy, so glyphs,
strokes, curves, the staff/measure/system records, content bounds, hit-test, and
quality metrics all read the same shifted geometry. The shifts grow each
system's extent, which the vertical stacking and justification then consume.
Regression inter_staff_solve_separates_colliding_staves (the two-staff fixture's
staff-line gap opens past the fixed pitch; a single-staff score keeps one staff
per system) + the two_staff_close_content render golden updates (slice 1 tight,
slice 2 separated). Only that engrave golden churned; single-staff goldens
byte-stable. 947 tests, clippy 0, docs -D warnings, conformance 8/8.
This completes the Standard-tier layout story end to end.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Groundwork for inter-staff band-height renegotiation (the last vertical-spring
piece): a fixture whose two staves' content nearly collides at the engraver's
fixed staff pitch, so the pending solve has something meaningful to separate.
two_staff_close_content: a one-measure, two-staff (treble/treble) metric score.
The top staff descends into low ledger notes (down to C2); the bottom staff
climbs into high ledger notes (up to C6) under a slur that arcs further above
them. In the constrained frame the top staff's content occupies y ≈ [-8, +4]
and the bottom staff's ≈ [-12, -5], overlapping in [-8, -5] — the C2 ledgers
collide with the C6 + slur. Invariant-clean; 8 notes, 1 slur, 41 strokes
(the heavy ledgering), 1 curve.
Registered in the render-svg acceptance suite; its goldens (this commit) show
the CURRENT overlapping layout — slice 2 (the solve) will update the engrave
golden to show the staves separated. Invariant test added. 946 tests, clippy 0,
docs -D warnings, conformance 8/8.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Levi Neuwirth