From 73c60d5d85684eedf274b6b2dde4272a95b534a1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 22 Sep 2026 16:07:16 +0900 Subject: [PATCH 1/7] test(shared): reject non-finite transcription timing --- .../transcription-timing-admission.test.ts | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 packages/shared-types/test/transcription-timing-admission.test.ts diff --git a/packages/shared-types/test/transcription-timing-admission.test.ts b/packages/shared-types/test/transcription-timing-admission.test.ts new file mode 100644 index 000000000..637bc10f3 --- /dev/null +++ b/packages/shared-types/test/transcription-timing-admission.test.ts @@ -0,0 +1,49 @@ +import { + createDemoRehearsalSong, + isRehearsalSong, + parseRehearsalSong, + type RehearsalSong +} from "../src/index"; + +function songWithTiming(onset: number, offset: number): RehearsalSong { + const song = createDemoRehearsalSong(); + song.sections[0]!.roles[0]!.transcription = [ + { pitch: "E2", onset, offset, velocity: 0.7 } + ]; + return song; +} + +describe("transcription timing admission", () => { + for (const nonFinite of [Number.NaN, Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY]) { + it(`rejects non-finite onset ${String(nonFinite)}`, () => { + const song = songWithTiming(nonFinite, 1); + + expect(isRehearsalSong(song)).toBe(false); + expect(() => parseRehearsalSong(song)).toThrow( + "sections[0].roles[0].transcription[0].onset" + ); + }); + + it(`rejects non-finite offset ${String(nonFinite)}`, () => { + const song = songWithTiming(0, nonFinite); + + expect(isRehearsalSong(song)).toBe(false); + expect(() => parseRehearsalSong(song)).toThrow( + "sections[0].roles[0].transcription[0].offset" + ); + }); + } + + it("preserves the existing finite timing domain", () => { + const finiteCases = [ + songWithTiming(0, 1), + songWithTiming(-1, -0.5), + songWithTiming(2, 1) + ]; + + for (const song of finiteCases) { + expect(isRehearsalSong(song)).toBe(true); + expect(parseRehearsalSong(song)).toEqual(song); + } + }); +}); From 7dec5a64d5a265cbf75ac735e286d8529c09c31f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 22 Sep 2026 16:11:31 +0900 Subject: [PATCH 2/7] fix(shared): reject non-finite transcription timing --- packages/shared-types/src/index.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index cba4606a2..fd0910796 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -841,6 +841,9 @@ function validateMetadataHandoffSection(value: unknown, path: string): string | if (confidenceError) { return confidenceError; } + if (!isOneOf(REHEARSAL_PRIORITIES, value.rehearsalPriority)) { + return invalidField(`${path}.rehearsalPriority`); + } if (!isDenseArray(value.roleBuckets)) { return invalidField(`${path}.roleBuckets`); } @@ -1465,10 +1468,10 @@ function validateTranscriptionNote(value: unknown, path: string): string | null if (typeof value.pitch !== "string") { return invalidField(`${path}.pitch`); } - if (typeof value.onset !== "number") { + if (typeof value.onset !== "number" || !Number.isFinite(value.onset)) { return invalidField(`${path}.onset`); } - if (typeof value.offset !== "number") { + if (typeof value.offset !== "number" || !Number.isFinite(value.offset)) { return invalidField(`${path}.offset`); } if (typeof value.velocity !== "number") { From 53cc2c6057b34cf006232d47599532fd9f8f0d3f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 22 Sep 2026 16:16:14 +0900 Subject: [PATCH 3/7] fix(shared): remove unrelated handoff validation drift --- packages/shared-types/src/index.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index fd0910796..f6679aba9 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -841,9 +841,6 @@ function validateMetadataHandoffSection(value: unknown, path: string): string | if (confidenceError) { return confidenceError; } - if (!isOneOf(REHEARSAL_PRIORITIES, value.rehearsalPriority)) { - return invalidField(`${path}.rehearsalPriority`); - } if (!isDenseArray(value.roleBuckets)) { return invalidField(`${path}.roleBuckets`); } From b69239b058ebb8efb33ea170ffa03163764cc2b7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 22 Sep 2026 16:16:50 +0900 Subject: [PATCH 4/7] docs(shared): trace transcription timing admission --- .../transcription-timing-admission.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 docs/doctoring/transcription-timing-admission.md diff --git a/docs/doctoring/transcription-timing-admission.md b/docs/doctoring/transcription-timing-admission.md new file mode 100644 index 000000000..31295d8f6 --- /dev/null +++ b/docs/doctoring/transcription-timing-admission.md @@ -0,0 +1,28 @@ +# Transcription timing admission + +## Decision + +`TranscriptionNote.onset` and `TranscriptionNote.offset` are shared rehearsal-contract values. The runtime parser must reject non-finite IEEE-754 values (`NaN`, `+Infinity`, `-Infinity`) before a rehearsal song reaches Workspace, GrooveMap, Active Player, persistence, or export consumers. + +The parser keeps the existing finite timing domain unchanged. This change does not add `onset >= 0`, `offset >= 0`, or `offset >= onset`. Those are separate musical/domain invariants and require fixture and persistence evidence before they can become shared-contract requirements. + +## Why the boundary is shared + +The TypeScript `number` type does not distinguish finite values from `NaN` or infinities at runtime. Consumer-side guards would duplicate validation and still leave persistence/export callers exposed. `parseRehearsalSong()` and `isRehearsalSong()` are the canonical runtime admission boundary used by those consumers, so the rejection belongs in `validateTranscriptionNote()`. + +Downstream rendering performs timing arithmetic such as percentage positions and widths. Admitting non-finite timing therefore permits invalid layout arithmetic even though the object satisfies a superficial `typeof value === "number"` check. + +## Verification contract + +`packages/shared-types/test/transcription-timing-admission.test.ts` covers all three non-finite values independently for onset and offset. Each hostile payload must fail both the boolean type guard and the throwing parser with the exact nested field path. The same suite also fixes the claim boundary by proving currently accepted finite values, including negative and inverted timings, remain accepted until a separate domain decision changes that contract. + +## Traceability + +- Finding: issue #1253. +- RED: `73c60d5d85684eedf274b6b2dde4272a95b534a1` adds hostile runtime fixtures without changing production validation. +- Causal repair: `7dec5a64d5a265cbf75ac735e286d8529c09c31f` adds finite-number admission to onset and offset. +- Repair follow-up: `53cc2c6057b34cf006232d47599532fd9f8f0d3f` removes an unrelated metadata-handoff validation line introduced while writing the full-file repair; the final diff against protected `develop` is limited to the two finite checks plus the focused test file and this document. + +## Security Notes + +This is a local runtime-schema boundary. It adds no network path, filesystem capability, subprocess behavior, telemetry, dependency, or PII flow. Failure is fail-closed at parse time and error text contains only the stable field path, not note content or local file metadata. From 9a27a87f8c2c74614d84acb23458845949f948a0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 22 Sep 2026 21:02:11 +0900 Subject: [PATCH 5/7] test(shared): reject impossible transcription intervals --- .../transcription-timing-admission.test.ts | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/packages/shared-types/test/transcription-timing-admission.test.ts b/packages/shared-types/test/transcription-timing-admission.test.ts index 637bc10f3..2352624f3 100644 --- a/packages/shared-types/test/transcription-timing-admission.test.ts +++ b/packages/shared-types/test/transcription-timing-admission.test.ts @@ -13,37 +13,49 @@ function songWithTiming(onset: number, offset: number): RehearsalSong { return song; } +const transcriptionTimingPath = "sections[0].roles[0].transcription[0]"; + describe("transcription timing admission", () => { for (const nonFinite of [Number.NaN, Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY]) { it(`rejects non-finite onset ${String(nonFinite)}`, () => { const song = songWithTiming(nonFinite, 1); expect(isRehearsalSong(song)).toBe(false); - expect(() => parseRehearsalSong(song)).toThrow( - "sections[0].roles[0].transcription[0].onset" - ); + expect(() => parseRehearsalSong(song)).toThrow(`${transcriptionTimingPath}.onset`); }); it(`rejects non-finite offset ${String(nonFinite)}`, () => { const song = songWithTiming(0, nonFinite); expect(isRehearsalSong(song)).toBe(false); - expect(() => parseRehearsalSong(song)).toThrow( - "sections[0].roles[0].transcription[0].offset" - ); + expect(() => parseRehearsalSong(song)).toThrow(`${transcriptionTimingPath}.offset`); }); } - it("preserves the existing finite timing domain", () => { - const finiteCases = [ - songWithTiming(0, 1), - songWithTiming(-1, -0.5), - songWithTiming(2, 1) - ]; + it.each([ + ["negative onset", -0.001, 0.5, "onset"], + ["zero-duration interval", 0.5, 0.5, "offset"], + ["inverted interval", 1, 0.5, "offset"] + ] as const)("rejects %s", (_label, onset, offset, field) => { + const song = songWithTiming(onset, offset); + + expect(isRehearsalSong(song)).toBe(false); + expect(() => parseRehearsalSong(song)).toThrow(`${transcriptionTimingPath}.${field}`); + }); + + it("accepts zero and negative-zero onset with positive duration", () => { + for (const onset of [0, -0]) { + const song = songWithTiming(onset, 0.25); - for (const song of finiteCases) { expect(isRehearsalSong(song)).toBe(true); expect(parseRehearsalSong(song)).toEqual(song); } }); + + it("accepts ordinary positive audio-relative intervals", () => { + const song = songWithTiming(2, 2.125); + + expect(isRehearsalSong(song)).toBe(true); + expect(parseRehearsalSong(song)).toEqual(song); + }); }); From 95356c11a76716dde927bfe311dffb5eec4cf9e1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 22 Sep 2026 21:07:40 +0900 Subject: [PATCH 6/7] fix(shared): enforce valid transcription intervals --- packages/shared-types/src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index f6679aba9..43288fc13 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -1465,10 +1465,10 @@ function validateTranscriptionNote(value: unknown, path: string): string | null if (typeof value.pitch !== "string") { return invalidField(`${path}.pitch`); } - if (typeof value.onset !== "number" || !Number.isFinite(value.onset)) { + if (typeof value.onset !== "number" || !Number.isFinite(value.onset) || value.onset < 0) { return invalidField(`${path}.onset`); } - if (typeof value.offset !== "number" || !Number.isFinite(value.offset)) { + if (typeof value.offset !== "number" || !Number.isFinite(value.offset) || value.offset <= value.onset) { return invalidField(`${path}.offset`); } if (typeof value.velocity !== "number") { From 02e85d3150bfd32472a23793b36af577fd55e331 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 22 Sep 2026 21:08:29 +0900 Subject: [PATCH 7/7] docs(shared): trace valid transcription interval contract --- .../transcription-timing-admission.md | 48 +++++++++++++++---- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/docs/doctoring/transcription-timing-admission.md b/docs/doctoring/transcription-timing-admission.md index 31295d8f6..db037e370 100644 --- a/docs/doctoring/transcription-timing-admission.md +++ b/docs/doctoring/transcription-timing-admission.md @@ -2,27 +2,55 @@ ## Decision -`TranscriptionNote.onset` and `TranscriptionNote.offset` are shared rehearsal-contract values. The runtime parser must reject non-finite IEEE-754 values (`NaN`, `+Infinity`, `-Infinity`) before a rehearsal song reaches Workspace, GrooveMap, Active Player, persistence, or export consumers. +`TranscriptionNote.onset` and `TranscriptionNote.offset` are shared rehearsal-contract values. A note is admitted only when both values are finite, `onset >= 0`, and `offset > onset`. -The parser keeps the existing finite timing domain unchanged. This change does not add `onset >= 0`, `offset >= 0`, or `offset >= onset`. Those are separate musical/domain invariants and require fixture and persistence evidence before they can become shared-contract requirements. +This combines the representation boundary from #1253 with the audio-relative interval invariant from #1255. `NaN`, `+Infinity`, `-Infinity`, negative onset, zero-duration intervals, and inverted intervals fail before Workspace, GrooveMap, Active Player, persistence, or export consumers can interpret them differently. JavaScript negative zero is accepted as zero because `-0 < 0` is false and it does not represent a distinct timeline position. + +Velocity remains outside this decision. Repository fixtures use both normalized-looking and MIDI-like velocity values, so no scale or range is invented here without producer/consumer evidence. ## Why the boundary is shared -The TypeScript `number` type does not distinguish finite values from `NaN` or infinities at runtime. Consumer-side guards would duplicate validation and still leave persistence/export callers exposed. `parseRehearsalSong()` and `isRehearsalSong()` are the canonical runtime admission boundary used by those consumers, so the rejection belongs in `validateTranscriptionNote()`. +The TypeScript `number` type does not distinguish finite values from `NaN` or infinities at runtime, and it does not encode an ordered audio interval. Consumer-side clamps would duplicate policy while allowing persistence, export, or another rehearsal view to interpret the same malformed note differently. `parseRehearsalSong()` and `isRehearsalSong()` are the canonical runtime admission boundary, so `validateTranscriptionNote()` owns the invariant. + +The protected transcription producer supports this domain. `_note_events_from_frames()` derives `start_time` from non-negative frame indices, computes `duration = end_time - start_time`, and drops events shorter than `MIN_NOTE_DURATION_SECONDS = 0.05`. The repository-owned producer therefore does not intentionally emit negative starts or non-positive durations. + +GrooveMap consumes these values as timeline geometry: onset determines horizontal position and `offset - onset` determines note width. A negative onset creates a position before the audio origin; an equal or inverted interval creates zero or negative width. Rejecting those values at shared admission is therefore rehearsal rendering correctness, not schema cleanup. + +## Persistence and recovery boundary + +Desktop `saveProject()` parses a song before invoking native persistence, and `loadProject()` parses the native response before returning it to the UI. A durable project that contains an impossible transcription interval must therefore fail closed with the exact nested field path. This repair does not silently clamp, reorder, or rewrite durable note timing. -Downstream rendering performs timing arithmetic such as percentage positions and widths. Admitting non-finite timing therefore permits invalid layout arithmetic even though the object satisfies a superficial `typeof value === "number"` check. +If a historical project carrying malformed note intervals is discovered, recovery or migration must classify that incompatibility explicitly. Weakening the shared invariant or silently mutating durable data is not an acceptable compatibility strategy. ## Verification contract -`packages/shared-types/test/transcription-timing-admission.test.ts` covers all three non-finite values independently for onset and offset. Each hostile payload must fail both the boolean type guard and the throwing parser with the exact nested field path. The same suite also fixes the claim boundary by proving currently accepted finite values, including negative and inverted timings, remain accepted until a separate domain decision changes that contract. +`packages/shared-types/test/transcription-timing-admission.test.ts` verifies both boolean and throwing admission paths: + +- all three non-finite IEEE-754 values fail independently for onset and offset; +- negative onset fails at `.onset`; +- equal or inverted offset fails at `.offset`; +- onset `0` and `-0` with positive duration remain accepted; +- an ordinary positive audio-relative interval remains accepted. + +No consumer-side `Math.max`, CSS clamp, persistence-only cleanup, or format-specific duplicate validator substitutes for this shared contract. ## Traceability -- Finding: issue #1253. -- RED: `73c60d5d85684eedf274b6b2dde4272a95b534a1` adds hostile runtime fixtures without changing production validation. -- Causal repair: `7dec5a64d5a265cbf75ac735e286d8529c09c31f` adds finite-number admission to onset and offset. -- Repair follow-up: `53cc2c6057b34cf006232d47599532fd9f8f0d3f` removes an unrelated metadata-handoff validation line introduced while writing the full-file repair; the final diff against protected `develop` is limited to the two finite checks plus the focused test file and this document. +- #1253: non-finite timing representation finding. +- `73c60d5d85684eedf274b6b2dde4272a95b534a1`: initial non-finite hostile RED. +- `7dec5a64d5a265cbf75ac735e286d8529c09c31f`: finite-number admission repair. +- `53cc2c6057b34cf006232d47599532fd9f8f0d3f`: removes an unrelated metadata-handoff delta introduced during that repair. +- #1255: finite but impossible audio-relative interval finding, grounded in GrooveMap geometry and the repository transcription producer. +- `9a27a87f8c2c74614d84acb23458845949f948a0`: hostile interval RED for negative onset, zero duration, inverted duration, and the zero/negative-zero boundary. +- `95356c11a76716dde927bfe311dffb5eec4cf9e1`: causal shared-validator repair requiring `onset >= 0` and `offset > onset`. + +## Rejected alternatives + +- Clamp negative onset or width in GrooveMap: rejected because malformed data remains valid elsewhere. +- Swap or normalize onset/offset during persistence: rejected because durable source data would be changed silently. +- Validate only when opening a project: rejected because analysis result, export, and in-memory callers use the same shared contract. +- Add a velocity range in the same change: rejected because the canonical velocity scale is not yet established. ## Security Notes -This is a local runtime-schema boundary. It adds no network path, filesystem capability, subprocess behavior, telemetry, dependency, or PII flow. Failure is fail-closed at parse time and error text contains only the stable field path, not note content or local file metadata. +This is a local runtime-schema boundary. It adds no network path, filesystem capability, subprocess behavior, telemetry, dependency, or PII flow. Failure is fail-closed at parse time, and error text contains only the stable field path rather than note contents or local file metadata.