Skip to content

Untested edge cases across src/core pure logic: a triaged inventory #77

Description

@BootBlock

This is the inventory §13.2 implies but nothing tracks: specific untested edge cases across the pure-logic modules in src/core/, found by a systematic module-by-module audit (read the source, enumerate boundary/empty/zero/malformed/interleaved cases, check each against the test file). It is deliberately a list of named cases, not a coverage percentage.

Items proven to be live defects have been filed separately — #66, #71, #72, #73, #74, #76 — and are not repeated here. What follows is the residue: real gaps where no defect is yet proven, plus a few where reachability is uncertain. Each carries a file:line so it can be triaged without re-deriving it.

Modules with no test file at all

persist.ts (see #72), snapshotService.ts, mpcwebZip.ts (only indirectly, via one round-trip at mpcweb.test.ts:151), packClient.ts, opfsWriteClient.ts, projectService.ts, mappers.ts, session.ts, dirty.ts, client.ts, sqliteBootstrap.ts.

opfs.test.ts covers only path building — every handle operation (writeFileAtomic, writeFileStreamed, readFile, deleteFile, deleteDirectory, purgeAllStorage) is untested, despite being mockable through navigator.storage.getDirectory.

Sequencer

  • schedulerCore.ts — song mode omits most of what sequence mode does. scheduleSong (:526-558) never calls scheduleNoteRepeat, scheduleArpeggiator, scheduleSequenceAutomation or collectErase, and never flushes recording per pass — compare scheduleSequence (:349-372). Recording in song mode flushes only at stop (:264-270), so a crash mid-song loses the take. Does song mode intentionally skip swing and groove? #36 asks only about swing and groove; the automation, note-repeat, arp, live-erase and per-pass-flush omissions are distinct from Does song mode intentionally skip swing and groove? #36 and wholly untested.
  • resetPlayback (:310-315) clears openNotes/heldNotes but not eraseNotes — live erase stays armed across stop→start. Untested. (Distinct from Live erase mass-deletes a bar of notes at the loop boundary #16, which is the loop-boundary mass-delete.)
  • applyEventsDiff ordering contract (:171-172): a message containing both a delete and an upsert for the same id deletes first, so the upsert wins. That is a real contract and is untested.
  • automatedTargets (:474-482) / effectivePoints (:488-497) split ${scope}:${ownerId}:${targetPath} by index-of-second-colon. §7.8 target paths contain colons (mixer.pad:<prog>:<idx>.sendLevels.2, insert:track:<id>:slot2.mix). The current slice is correct, but an ownerId containing a colon would mis-split. Every automation test uses the single-colon mixer.track:t1.level.
  • closeOpenNotes (:613-620) takes key.split(':')[0] as trackId — breaks for a trackId containing a colon. Untested.
  • Count-in boundary: pushLiveNote gates capture on contentStarted(when) (:233), so a note played during the count-in is dropped. Deliberate, but unverified.
  • lookahead.ts guardMax = 100_000 truncation (:74-76): if hit, segmentWindow returns short while scheduleSequence still sets nextScheduleTick = to (:371) — events are silently dropped. Untested. Also untested: a very large window (backgrounded tab then a big now jump), from > to, negative bounds.
  • songMap.ts: songSecondsToTick past the end clamps to songTotalTicks forever, making scheduleSong early-return at :531the song silently stalls at its end without stopping the transport. Whether it should auto-stop is a spec gap worth settling. Also untested: repeats: 0 (contributes no segments), and entryIndex being the index in the sorted array with skipped entries still consuming an index (:50), so songAdvanced { entryIndex } may not index the UI's list as expected.
  • automation.ts automationRampForWindow (:67-78) is never called by production codeschedulerCore.scheduleSequenceAutomation (:460-471) hand-rolls the same computation inline. Two implementations of one rule, one dead but carrying 2 tests. Also untested: unsorted input (a stated precondition never negative-tested), duplicate ticks, single-point lanes.
  • playheadSab.ts: if all 8 seqlock attempts see an odd generation, read() returns currentTick: 0 as though it were real (:64-83) — the playhead snaps to bar 1. All 3 existing tests are happy-path, despite the seqlock being the module's entire purpose.
  • schedulerClient.ts: setGroove, setArpeggiator, setNoteRepeat, setSwing, setMetronome, setLiveErase, sendLiveNote, sendAutomationDiff and setSongSequence are all untested (only 5 senders are exercised, schedulerClient.test.ts:55-73). A sender-round-trip test would have caught groove scheduler messages are dropped by the Zod guard; the entire non-destructive groove path is dead #71.
  • SCHEDULER_PROTOCOL_VERSION (messages.ts:28) is exported, re-exported and asserted in a test, but never attached to a message or checked at either boundary — it cannot detect the version skew it exists for.
  • Worker-boundary validation is looser than the project schema. messages.ts:167 is bare z.number() for tempo; sequenceMeta.tempo/projectBpm (:139, :192) likewise; swing amount (:170) is unbounded. In-app clamps make these unreachable today (useTransportStore.ts:79-82), but per §1.3 CI does not run format:check; 23 files are Prettier-dirty #11 the Zod guard is the worker's validation contract. Measured with the guard bypassed: bpm < 0 emits 4096 click events per wake, every wake; swingAmount = 100 shifts a 1/16 fully onto the next grid line.

Audio / DSP / MIDI

  • parser.ts:174 accumulates only one 128 ms timestamp carry per packet. A second low-7-bit wrap in one packet is lost. Measured with low bytes 100 → 10 → 120 → 30: stamps [100,138,248,158] instead of [100,138,248,286] — a −90 ms delta, so a recorded note lands in the past. Needs a packet spanning >256 ms (slow connection interval or a batching device). parser.test.ts:144 covers exactly one wrap.
  • chop.ts:15-26 never relates count to totalFrames. equalSlices(3, 8) yields 5 zero-length regions; equalSlices(0, 4) yields four {0,0}. Chop-to-pads then assigns silent pads with no error. Also enforceMinSpacing([10,10,10], 0) keeps all three duplicates (chop.ts:71>= 0 is true for a zero delta).
  • sampleEdit.ts:99 takes trim bounds from channels[0]?.length only, then slices each channel independently (:103). trim([len 1000, len 500], 0, 800) returns lengths [800, 500]; a downstream createBuffer + set() then throws or misaligns one side of a stereo sample. (normalise/reverse/fadeIn/fadeOut map per channel and are unaffected.)
  • registry.ts:105-121 parses insert slot and pad index as unbounded (\d+), unlike the send index immediately above which is bounded (:98). insert:master:slot999.mix and program:abc.pad:9999.pitch both parse. Since isAutomatable() is the §7.8 gate, these accept automation points and Q-Link bindings that can never resolve. mixer.master.sendLevels.00 is also accepted as a non-canonical alias that will not round-trip.
  • controlMaths.ts:59 special-cases −∞ but not +∞ or NaNformatValueText(NaN, 'dB') yields "NaN dB" straight into aria-valuetext. Also formatValueText(999.6, 'Hz')"1000 Hz", because the kHz branch triggers at ≥1000 but Math.round (:65) can reach it. Screen-reader output, so low severity but user-visible.
  • faderLaw.ts:28-37 has no NaN or upper guardfaderLevelToGain(NaN) returns NaN (both comparisons false), and faderLevelToDb(2.0) extrapolates to +30 dB. Schema-guarded upstream (LEVEL_RANGE [0,1.2]), so no live path was demonstrated — but this is the last function before the AudioParam.
  • sampleCache.ts:47 evicts by path, not promise identitydecoded.catch(() => this.cache.delete(path)). If invalidate(path) runs while decode Bump @eslint/js from 9.39.5 to 10.0.1 #1 is in flight and decode Looper: microphone record source is missing #2 is then cached, decode Bump @eslint/js from 9.39.5 to 10.0.1 #1's later rejection deletes decode Looper: microphone record source is missing #2's good entry. Consequence is a redundant decode, not silence.
  • ringBuffer.ts:32-35 accepts non-integer slot counts (no Number.isInteger check, unlike equalSlices). create(4.5) gives fractional indices. Probing did not demonstrate corruption, so this is a hardening gap only.

Storage / project / undo

  • opfs.ts:114-126writeFileAtomic's fallback path is not atomic, writing directly onto the final handle when FileSystemFileHandle.move is absent. This contradicts the module doc at opfs.ts:12 and §9.7's "never corrupt a half-written file". Dead-untested code on the sample-write path.
  • errors.ts:122-125 collapses extended SQLite result codes for CONSTRAINT only. SQLITE_BUSY_RECOVERY (261), SQLITE_LOCKED_SHAREDCACHE (262) and the SQLITE_FULL/SQLITE_READONLY families fall through to SQLITE_ERROR, so isRetryable (:55-57) is false for genuinely retryable lock contention — exactly the case §9.2's write queue exists to survive.
  • projectService.ts:109-114exportMpcweb uses Promise.all over every sample read with no per-sample recovery. One missing OPFS file means the project can never be exported. It also holds every sample in memory at once.
  • opfs.ts:76, :173, :185fileExists/deleteFile/deleteDirectory swallow all errors, so an I/O or permissions failure reports "absent"/"deleted". Feeds the orphan-leak class around .mpcweb import is not transactional and leaks orphaned sample files on failure #22.
  • mpcweb.ts:124-126 never checks project.json's own version field despite parsing it (:104); only the manifest is version-gated (:147). parseSnapshot accepted version: 999.
  • mpcweb.ts:183-184remapSnapshot builds a Map, so two rows sharing an id collapse onto one new id, guaranteeing a PK collision in restoreSnapshot and a mid-import failure with rows already written. Distinct from .mpcweb schema accepts unconstrained ids and numerics #24 (id format/range): no uniqueness check exists anywhere.
  • mpcweb.ts:187 rewrites ids by substring (json.split(oldId).join(newId)), assuming 36-char UUIDs per the comment at :178-180, with nothing enforcing it. project.id = 'a' produces a Zod invalid_type; project.id = '' produces a SyntaxError from JSON.parse. Fails loudly rather than corrupting, so this is "cryptic import failure", not data loss.
  • mpcwebZip.ts:65-70 collects samples/*.wav entries without cross-checking snapshot.samples, projectService.ts:132-134 skips unmatched ids, and snapshotService.ts:133-144 inserts every sample row regardless. A truncated archive therefore imports "successfully" as a silently soundless project. (Inverse of .mpcweb import is not transactional and leaks orphaned sample files on failure #22, which is orphaned files on failure.)
  • autosave.tsdispose() during a failing in-flight flush leaves permanent phantom dirty state. dispose clears dirty (:98) but runFlush's catch re-adds the batch afterwards (:108), while the retry re-arm is skipped for a disposed queue (:84). autosave.test.ts:102 only tests dispose before any flush.

Verified sound — recorded so these are not re-investigated

  • lookahead.ts loop wrapping genuinely meets §7.1.5: the once-per-pass guarantee is real and lookahead.test.ts:90-100 proves no double-scheduling across abutting windows; exclusive-bound handling at the loop end is correct and tested.
  • clockSync.ts — 8-sample window, 2 ms snap threshold, no-snap-exactly-at-threshold and 10-sample eviction all covered; the §14 (f) absolute-epoch correction is applied consistently on both sides, so the two timeOrigins genuinely cancel.
  • songMap.ts boundary-spanning lookahead (§7.9's explicit requirement) splits correctly across an entry boundary and the per-segment tempo map round-trips across a tempo change.
  • noteRepeat.ts step maths is exact — every division × triplet at 960 PPQN is an integer, so there is no float-drift class here.
  • Migration engineengine.test.ts genuinely covers ordered application, idempotence, atomic rollback with user_version untouched, SCHEMA_TOO_NEW, and non-contiguous authoring errors.
  • Pagination drain at and past the 200-row boundary is correct in all three implementations (base.ts:33, snapshotService.ts:20, hydrate.ts:48), including the trailing empty query.
  • CommandStack coalescing — the openCoalesceKey state machine was traced across undo/redo/clear/endCoalescing/UNDO_LIMIT eviction; no path attaches a merge to the wrong entry.
  • Hydration ordering — all reads and Zod mapping complete before the first store write (hydrate.ts:129), so a malformed payload throws before any store is half-populated.
  • Zip-entry path injection is blocked — a hostile samples/../../evil.wav entry yields an id absent from sampleIdMap and is skipped. The Imported opfs_path is trusted, allowing deletion of any file in the origin's OPFS #18 vector is the opfs_path column, not the entry name.
  • selectStealVictim/selectChokeVictims (§5.4) are correct with complete tests; tie-breaking on equal startTime is deterministic.
  • ccThrottle hysteresis does not stall under a slow monotonic sweep — a hypothesis I tested and disproved; lastApplied only advances on an applied value, so a 64→100 single-step sweep applies every other value and reaches the endpoint.
  • BLE parser malformed-input handling is genuinely thorough — empty/header-only/truncated packets, orphan data bytes, cross-packet SysEx, real-time bytes not disturbing running status, 512 bytes of noise. Data bytes are structurally guaranteed <128 by the bit-7 check (parser.ts:169).
  • computeEffectiveMutes, evaluateModMatrix algebra, makeSaturatorCurve (2049 samples means index 1024 is exactly x=0, so no DC offset), MeterRegistry.read bounds handling, and the core/math taper (mutual inverses, consistent log→linear fallback) are all correct.
  • quantise.ts core behaviour — strength interpolation, swing baked proportionally, durations preserved, input not mutated, deterministic tie-break.
  • RingBuffer SPSC logic — cursor arithmetic, reserved slot, and the Atomics.store-after-plain-writes release idiom are correct.

Metadata

Metadata

Assignees

Labels

testingUnit, offline-render, and real-browser verification (spec 11)

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions