Coach / one-shot-transition assist layer (Coach advice, bounded track suggestions, physical + browser Assist bank)

[djdefi]

Deferrals

• No Auto DJ (separate, later PR).
• No cue support or library browsing/loading over the wireless API yet — arm-transition requires the target track already loaded via the device's own SD workflow (never loads an arbitrary path).
• reason_flags bitmask is exposed decoded (as text) client-side but shipped over the wire as a raw integer to keep the state payload bounded; primary user-facing fields are already fully decoded server-side.

Review fixes (6 material issues, all addressed, history not amended)

An independent principal/code review of this PR found 6 material issues (5 HIGH, 1 MEDIUM). Fixed in 3 follow-up commits, 478667a/c03ecc4 and earlier history untouched:

• 2b544b4 — Mix screen #1 stale guard (HIGH): DjSession::loop() applied commands, then tickAssist() read the previous published snapshot, so a same-iteration deck reload/swap could let Assist act once against a stale, unconfirmed track. publishSnapshot() now runs after apply/tickLoops/tickSync/pollRecording but before tickAssist().
• c863d79 — Playback #2, Song list #3, Main menu #4, added Text Input screen for text entry feature #5 (all HIGH), kept together since they share the same tick/actuator flow:
  • Playback #2 boundary semantics: resolveBoundary()'s "next boundary" hint is true almost every tick, so WAIT_BOUNDARY was advancing immediately instead of waiting. Added DjAssistBoundaryHint::reached, set only by a new capture-once-then-compare resolveWaitBoundaryReached(); the engine now gates strictly on reached.
  • Song list #3 cancel/failure rollback: added a bounded tickRollback() state machine (MIX → SYNC → STOP_DECK → DONE, one actuator submit/poll per tick, only undoing steps this plan itself submitted, never touching pre-existing user state, never restarting the source deck once stopped).
  • Main menu #4 crossfade confirmation + manual override: the actuator now tracks the exact final-mix command id and only reports APPLIED once confirmed (not from elapsed-time math); added DjAssistGuardSnapshot::manualMixOverride (bounded scan for a non-system SET_MIX since arming), checked unconditionally in guardOk() so a manual override mid-ramp aborts instead of being silently overwritten.
  • added Text Input screen for text entry feature #5 hot-loop SD I/O: refreshCandidateTable()'s fill loop was reusing the 256-record scoring-scan budget for genuine (possibly SD-backed) metadata reads. Split out a separate DJ_ASSIST_FILL_RECORDS_PER_TICK = 1.
• eca7930 — Intro screen #6 response buffer (MEDIUM): bumped MAX_RESPONSE 3072→4096 and added a constexpr/static_assert-checked worst-case formula for /api/v2/state so a future capacity change fails the build instead of silently returning response_too_large. (The formula's first draft used the wrong bound — DJ_RECENT_RESULT_COUNT, the backing array size — instead of handleState()'s actual 8-result emission cap; caught by the static_assert itself failing during the real wireless firmware build, since this file isn't part of any host test target.)

New test coverage: reached-semantics regressions (including a case proving hasPhrase/hasDownbeat alone must not advance), a manual-override-during-crossfade regression, and full dj_assist_bridge_selfcheck.cpp coverage for the 3 new pure bridge functions (nextRollbackPhase, evaluateCommandOutcome, detectManualMixOverride).

Review fixes, round 2 (4 material issues, all addressed, history not amended)

A focused re-review of eca7930 found 4 more material issues (3 HIGH, 1 MEDIUM). Fixed in b822c52:

• Mix screen #1 rollback ownership (HIGH): rollback inferred ownership from which steps were submitted, so a target deck already playing/synced before arming would have rollback incorrectly stop/release pre-existing user state, and a manual mix override mid-crossfade could be silently overwritten by the MIX rollback phase. DjAssistTransitionPlan now captures armedToPlaying/armedToSynced from the guard snapshot at arm() time; nextRollbackPhase() takes manualMixOccurred/syncOwnedByPlan/startDeckOwnedByPlan instead of raw submitted flags, skipping MIX on any manual override and skipping SYNC/STOP_DECK whenever that step was already true at arm time (an idempotent no-op, not a plan-owned mutation).
• Playback #2 durable command tracking (HIGH): recentResults is a bounded/evictable presentation ring, not a control channel — a burst could evict the final crossfade command's outcome or a manual-override marker before the controller polled it. Added DjSession::assistTrackCommand()/assistTrackedStatus() (a single controller-owned tracked-command slot, retained until consumed) and DjSession::assistNonSystemMixGeneration() (a monotonic counter incremented only on non-system SET_MIX apply, captured at arm time). Replaces the removed detectManualMixOverride() ring scan entirely.
• Song list #3 background candidate fill + phrase-cache throttling (HIGH): candidate-table hydration (one metadata record per tick, full string/provenance decode) still ran synchronously on DjSession::loop(); resolveBoundary()'s phrase lookup also rescanned up to 128 records every tick whenever no future phrase existed (a permanently-terminal track). Candidate-table fill now runs entirely off the session loop via a genuine background Task, with the real I/O performed outside the candidate-table mutex and staleness re-checked before/after each read. Added a phrase-lookup cache invalidated only by identity change, deck change, backward seek, or a previously-found boundary being passed — never by forward playback alone while terminal.
• Main menu #4 boundary tolerance (MEDIUM): resolveWaitBoundaryReached() fired as soon as currentFrame >= target with no tolerance, so a boundary could release arbitrarily late. Added evaluateBoundaryArrival(), reusing the established DJ_QUANTIZE_TOLERANCE_FRAMES policy: reached only at/within tolerance past the target, missed beyond it (never releases late — re-captures the next boundary and keeps waiting).

New/updated test coverage: arm() capturing the toDeck playing/sync baseline both true and false; nextRollbackPhase ownership-vs-submitted distinction and manual-override-skips-mix; evaluateBoundaryArrival not-yet/reached-at-tolerance/missed-beyond-tolerance; phrase-cache empty/found-not-yet-passed/terminal-not-rescanned/identity-or-deck-change/backward-seek. Also removed DJ_ASSIST_FILL_RECORDS_PER_TICK, a leftover constant from the old per-tick synchronous fill model that this round's background task made dead code.

Validation

• ctest: all 8 host self-checks pass (DjAssistSelfCheck, DjAssistBridgeSelfCheck, plus the pre-existing suite), -Wall -Wextra -Werror + ASan/UBSan.
• node tests/wireless_ui_self_test.mjs: 43/43 checks pass.
• Full arduino-cli build, cm:esp32@1.8.3, Library 78a9f3d7e52fd151dc50872812ae2ed1da308af1, both configurations, at final SHA b822c52:
  • default (no wireless): 1,380,014 B flash (38%) / 47,160 B RAM (14%)
  • -DJAYD_ENABLE_WIRELESS: 1,974,606 B flash (54%) / 52,856 B RAM (16%)

Two genuine pre-existing bugs were found and fixed only because this was the first real compile with JAYD_ENABLE_WIRELESS enabled (round 1): a DjSnapshot::recording field that doesn't exist (real field is recordingInfo.state), and an odr-use link failure on SongList's static const members (fixed with out-of-class storage definitions).

Review fixes, round 3 (6 material issues, all addressed, history not amended)

A focused re-review of b822c52 found 6 more issues (4 HIGH, 2 MEDIUM), all fixed in 2b8f319; 478667a/c03ecc4/c863d79/eca7930/b822c52 untouched:

• Mix screen #1 rollback ownership still unsafe (HIGH): b822c52's baseline-capture-at-arm still let a target already started/synced after arm but before the plan's own step submitted be mistaken for plan-owned on rollback. armTransition() now hard-requires the target deck stopped and sync-off at arm (fails outright otherwise); toDeckStartOwnedByPlan/toDeckSyncOwnedByPlan replace the old baseline fields and are set only the instant the plan's own START_DECK/ENABLE_SYNC step reaches DJ_COMMAND_APPLIED — never inferred from submission. guardOk() also aborts with MANUAL_OVERRIDE if the target starts playing/syncing before the plan's own step is even submitted.
• Playback #2 tracked command supersession (HIGH): the durable tracked slot only updated on pop/finish, so a command superseded while still queued stayed ACCEPTED forever. DjCommandQueue::supersede() now returns a 3-state DjSupersedeOutcome; a new admitAssistCommand() free function routes every superseded id through the tracked slot at admission time, immediately.
• Song list #3 manual mix admission race (HIGH): a queued manual SET_MIX could be superseded by the next Assist ramp command before it ever applied, silently erasing user intent. supersede() now returns BLOCKED (rejecting the system command outright) whenever a system-origin mix would replace a still-queued non-system mix; nonSystemMixGeneration is latched the instant a non-system mix is admitted (replace or fresh push), not when it applies.
• Main menu #4 candidate generation race (MEDIUM): fillWorkerStep()'s commit-time check compared loadedGeneration_ against the same value it had just been set from earlier in the same call — a self-comparison that could never catch a refresh completing during the unlocked SD read. Commit points and candidateTableReady() now re-read the live generation fresh, immediately before the lock, via DjAssistBridge::candidateFillGenerationCurrent().
• added Text Input screen for text entry feature #5 phrase cache misses metadata refresh (MEDIUM): the cache invalidated only on identity/deck/seek change, so a metadata refresh that left the same track loaded (re-resolved grid/phrase) could serve a stale terminal result. DjAssistPhraseCacheState now also keys on metadataGeneration/metadataState.
• Intro screen #6 host integration test harness: added direct admitAssistCommand() coverage in dj_session_self_test.cpp against the real DjCommandQueue/DjCommandResults/DjAssistTrackedCommand types (supersede→durable routing, both manual/system mix admission orderings, BLOCKED rejection, queue-full isolation from tracked slot, >ring-capacity result bursts not affecting the durable slot), plus new dj_assist_selfcheck.cpp/dj_assist_bridge_selfcheck.cpp coverage for the above.

Preserves the round-2 same-tick guard, boundary tolerance, background fill worker, and 4 KiB response bound unchanged.

Validation

• ctest: 8/8 host self-checks pass under -Wall -Wextra -Werror + ASan/UBSan.

• node tests/wireless_ui_self_test.mjs: all checks pass (unchanged this round).

• Full arduino-cli build, cm:esp32@1.8.3, Library 78a9f3d7e52fd151dc50872812ae2ed1da308af1, both configurations, at final SHA 2b8f319:

  • default (no wireless): 1,380,298 B flash (38%) / 47,160 B RAM (14%)
  • -DJAYD_ENABLE_WIRELESS: 1,480,370 B flash (40%) / 52,768 B RAM (16%)

  Note: every wireless build size previously reported on this PR (rounds 1-2) was inflated by a bug in the local arduino-cli invocation, not the firmware — passing --build-property build.extra_flags=-DJAYD_ENABLE_WIRELESS=1 silently replaced the board's default build.extra_flags=-O2 instead of extending it, so those builds compiled at -O0. The default (non-wireless) config never needed that override and was correctly optimized all along, which is why only the wireless numbers were affected. Fixed this round by passing -O2 -DJAYD_ENABLE_WIRELESS=1 together; verified via -v that -O2 reaches every compile line. No code changes resulted from this - it only corrects the reported size.

Auto DJ remains out of scope for this PR.

Review fixes, round 4 (5 material issues, all addressed, history not amended)

A focused re-review of 2b8f319 found 5 more issues (3 HIGH, 2 MEDIUM), all traced to fragmented origin/intent handling rather than another tail-only guard. Fixed together in 8d387ce; 478667a/c03ecc4/c863d79/eca7930/b822c52/2b8f319 untouched:

• Mix screen #1 LOCK_TEMPO self-abort/leak (HIGH): LOCK_TEMPO and ENABLE_SYNC both call setSync(true), but only ENABLE_SYNC updated toDeckSyncOwnedByPlan, so the default plan (tempoLock=true) saw sync go active with no recognized owner — a false self-abort, and rollback would then skip releasing a sync it never marked as plan-owned. Ownership is now set from either step reaching DJ_COMMAND_APPLIED (both resolve to the same idempotent mutation, so attribution is correct regardless of which one fires first).
• Playback #2 user intent invisible once the plan's own step submits (HIGH): guardOk() compared raw state (deckPlaying/syncActive/mix), which a system-origin Assist step mutates to the same value it's checking — masking any user command that raced in around the same time. Replaced with DjAssistIntentGenerations (mix/playing[]/sync[] monotonic counters), bumped in a new djBumpIntentGeneration() called from admitAssistCommand() at admission, only for non-DJ_ORIGIN_SYSTEM commands. armTransition() captures a baseline; guardOk() compares the live snapshot against it unconditionally every tick, independent of whether the plan's own step has submitted or applied.
• Song list #3 mix/origin priority only checked the queue tail (HIGH): hasNonSystemPending()/removeSystemTargeting() previously only reached the slot supersede() could replace, so a manual command "buried" behind an unrelated queued command wasn't seen — a system command could jump ahead of it, or cancel could leave a stale plan-owned command queued behind something else. Both now scan the entire bounded queue (≤16 entries), and every purged/superseded id is routed through the durable tracked-outcome path immediately, not only at dequeue.
• Main menu #4 candidate generation check-then-lock gap (MEDIUM): fillWorkerStep() read the live library generation, then separately locked and read the metadata entry, leaving a window for a concurrent refresh to swap the reader in between. DjSession::assistTrackEntry() now captures the metadata revision under the same metadataMutex acquisition as the trackByIndex() read itself; DjAssistBridge::candidateGenerationCurrent() collapses to a plain 2-arg equality check against that atomically-captured revision — no separate probe.
• added Text Input screen for text entry feature #5 real controller/session paths untested (MEDIUM): admitAssistCommand() is the same free function DjSession::submit() calls under its real commandMutex, so host-testing it already exercises real production admission logic (pre-existing architecture, extended this round with generation-aware assertions rather than a parallel reimplementation). Added direct coverage for: LOCK_TEMPO-only ownership (no ENABLE_SYNC in the plan), post-arm divergence detected both before and after the plan's own step has submitted, queue-wide origin scan in both directions (system command buried behind an unrelated one still purged when a manual command is admitted; manual command buried behind an unrelated one still blocks a later system command), and manual mix admitted mid-crossfade (after CROSSFADE has submitted but before it's applied) still aborts the transition.

Preserves the round-3 stopped/sync-off arm precondition, durable slot supersede routing, boundary tolerance, background candidate fill, phrase cache key, and 4 KiB response bound unchanged.

Validation

• Host self-checks (DjSessionSelfCheck, DjAssistSelfCheck, DjAssistBridgeSelfCheck, WirelessApiSelfCheck, default + -DJAYD_ENABLE_WIRELESS) all pass under g++ -std=c++11 -Wall -Wextra -Werror -fsanitize=address,undefined.
• node tests/wireless_ui_self_test.mjs: all checks pass (unchanged this round).
• Full arduino-cli build, cm:esp32@1.8.3, Library 78a9f3d7e52fd151dc50872812ae2ed1da308af1, both configurations, at final SHA 8d387ce:
  • default (no wireless): 1,381,390 B flash (38%) / 47,160 B RAM (14%)
  • -DJAYD_ENABLE_WIRELESS: 1,481,442 B flash (41%) / 52,768 B RAM (16%)

Auto DJ remains out of scope for this PR.

Review fixes, round 5 (4 material issues, all addressed, history not amended)

A focused re-review of 8d387ce found 4 more issues (1 HIGH, 3 MEDIUM). Fixed in 4292807; 478667a/c03ecc4/c863d79/eca7930/b822c52/2b8f319/8d387ce untouched:

• Mix screen #1 rollback overrides later user intent (HIGH): after the plan's own START_DECK/LOCK_TEMPO/ENABLE_SYNC step applied, a later user SET_PLAYING(true)/SET_SYNC(true) on the target deck correctly failed the transition (intent-generation divergence), but ownership stayed true, so rollback would then stop/release the user's own newer intent. guardOk() now relinquishes toDeckStartOwnedByPlan/toDeckSyncOwnedByPlan for the exact property that diverged, immediately before failing — rollback can then never touch it.
• Playback #2 source-deck sync unguarded (MEDIUM): the plan captures both decks' sync generations at arm time, but guardOk() only ever compared the target (toDeck). A non-system sync command on the source deck now aborts the transition (MANUAL_OVERRIDE) every tick too; there's no rollback ownership for fromDeck sync (the plan never touches it), so this is a plain abort with no additional bookkeeping.
• Song list #3 candidate-generation race remains (MEDIUM): libraryGeneration is now std::atomic<uint32_t>, bumped before refreshLibraryMetadata()/invalidateLibraryMetadata() mutate the reader; assistLibraryGeneration() is now a lock-free read, safe to call from inside DjAssistController's own candidateMutex_ critical section with no lock-ordering risk against metadataMutex. assistTrackEntry() still captures its revision under the same metadataMutex acquisition as the read. fillWorkerStep()'s commit path now re-checks the entry's captured revision and a fresh in-lock live-generation reload and fillCursor_ == cursor immediately before writing — any mismatch discards the record instead of committing it. candidateTableReady() reloads the live generation inside its own lock immediately before the readiness decision.
• Main menu #4 real integration coverage still absent (MEDIUM): added DjAssistSessionPort (src/DjAssist/DjAssistSessionPort.h), the Arduino-free interface DjAssistController now depends on instead of a concrete DjSession (DjSession implements it, unchanged behavior). This lets the real, unmodified DjAssistController.cpp/DjAssistEngine.cpp be driven by a new host test, DjAssistIntegrationSelfCheck (tests/dj_assist_integration_selfcheck.cpp), against a fake port whose command admission reuses the exact real, shared admitAssistCommand()/DjCommandQueue/DjCommandResults/DjAssistTrackedCommand/DjAssistIntentGenerations primitives DjSession itself uses — not a parallel reimplementation. Seven scenarios: full default transition (arm→boundary→tempo-lock→sync→crossfade→stop→release→complete), cancel/rollback restoring only plan-owned state, user play-divergence relinquishing rollback ownership (Mix screen #1), source-deck sync divergence aborting the transition (Playback #2), manual mix mid-crossfade aborting with rollback skipping the mix phase, queue-wide supersede/removal through the real admission coordinator, and the candidate-fill generation race (Song list #3) being caught and discarded. Minimal host stubs (tests/host_stubs/{Arduino.h,Sync/Mutex.h,Util/Task.h,host_arduino_shim.cpp}) provide the handful of Arduino/CircuitOS primitives DjAssistController.cpp needs (micros/delay/ps_malloc, Mutex, Task) without any real ESP32 headers.

Preserves the round-4 LOCK_TEMPO/ENABLE_SYNC ownership unification, admission-time intent generations, queue-wide origin scan, boundary tolerance, background candidate fill, phrase cache key, and 4 KiB response bound unchanged.

Validation

• ctest: DjAssistSelfCheck, DjAssistBridgeSelfCheck, DjSessionSelfCheck, DjAssistIntegrationSelfCheck (new), LibraryIndexSelfCheck, DjBeatEngineSelfCheck, MetadataIntegrationSelfCheck, WirelessApiSelfCheck — all pass under g++ -std=c++11 -Wall -Wextra -Werror -fsanitize=address,undefined.
• Full arduino-cli build, cm:esp32@1.8.3, Library 78a9f3d7e52fd151dc50872812ae2ed1da308af1, both configurations, at final SHA 4292807:
  • default (no wireless): 1,384,114 B flash (38%) / 47,160 B RAM (14%)
  • -DJAYD_ENABLE_WIRELESS: 1,485,306 B flash (41%) / 52,768 B RAM (16%)

Auto DJ remains out of scope for this PR.

Review fixes, round 6 (4 material issues, all addressed, history not amended)

A focused re-review of 4292807 found 4 more issues (3 HIGH, 1 MEDIUM). Fixed in ccca174; 478667a/c03ecc4/c863d79/eca7930/b822c52/2b8f319/8d387ce/4292807 untouched:

• Mix screen #1 rollback ownership guard cleared only the first divergent property (HIGH): guardOk()'s divergence checks were an early-return chain — if both play and sync diverged in the same user action, only the first check's ownership flag was relinquished before returning, leaving rollback free to stop/release the user's own newer state on the other property. Cancel/failure could also enter rollback with no further guard pass at all, since guardOk() only runs while mode_ is ARMED/RUNNING. Replaced with reconcileOwnershipOnDivergence(), a single pass that independently ANDs each ownership flag against its own generation match with no early return, called unconditionally at the top of guardOk() and at the start of every DjAssistController::tickRollback() tick using freshly re-fetched live intent generations — so a user re-asserting play/sync after cancel is still caught before rollback acts on it.
• Playback #2 candidate freshness keyed off the wrong generation (HIGH): candidate-table validity was tied to libraryGeneration, an externally-supplied semantic identity that can stay unchanged across a same-generation sidecar-file replacement or a metadata loss/reopen cycle landing back on the same value. Added a dedicated std::atomic<uint32_t> metadataRevision on DjSession, bumped before every reader mutation/invalidation/shutdown-close regardless of the external generation value; renamed the assist port accessor assistLibraryGeneration() -> assistMetadataRevision() throughout (it was DjAssistController's only consumer) so the candidate worker/table/cache key off this new revision instead.
• Song list #3 readiness/consumption TOCTOU (HIGH): tick() called candidateTableReady() (its own lock/unlock), decided the revision-based reset outside any lock, then tickSuggestions() separately re-locked to scan — a fill-worker refresh landing in the gap between these independent acquisitions could expose a stale suggestion list. Collapsed the readiness check, the reset, and the scanTick() scan into one candidateMutex_ critical section inside tickSuggestions(); on a revision mismatch mid-tick, suggestion state is cleared and the scan is skipped for that tick rather than exposing stale entries. candidateTableReady() is preserved for tests/diagnostics but no longer gates the tick.
• Main menu #4 shutdown race (HIGH): DjSession::shutdown() closed metadataReader while the background candidate-fill task could still be running, racing a use-after-close. shutdown() now calls the already-idempotent, blocking assistController.end() before touching metadataReader, and marks metadataRevision++/metadataInitialized=false/metadataReaderStatus=Missing under metadataMutex before metadataReader.close().

Preserves the round-5 LOCK_TEMPO/ENABLE_SYNC ownership unification, admission-time intent generations (including the round-5 relinquish-on-divergence fix, now generalized by #1), queue-wide origin scan, boundary tolerance, background candidate fill, phrase cache key, and 4 KiB response bound unchanged.

Added DjAssistIntegrationSelfCheck coverage: simultaneous play+sync divergence relinquishing both ownership flags together (#1), a user re-asserting play/sync after cancelTransition() still being caught by tickRollback()'s fresh reconciliation (#1), suggestions never going stale across a metadata-revision refresh landing between readiness and scan (#2/#3), and DjAssistController::end() making further worker calls safe no-ops (#4's achievable half — the host Task.h stub deliberately never spawns a real thread, so a literal join-timing race isn't reproducible without introducing nondeterminism into the other deterministic scenarios; this is a documented scope limitation, not a claim of full race coverage). DjSession.cpp is not part of any host test target (needs real AudioLib/FS.h/SD.h/LoopManager), so #2's revision bump points and #4's shutdown reordering are validated by code review plus the full firmware build rather than a host unit test.

Validation

• ctest: DjAssistSelfCheck, DjAssistBridgeSelfCheck, DjSessionSelfCheck, DjAssistIntegrationSelfCheck, LibraryIndexSelfCheck, DjBeatEngineSelfCheck, MetadataIntegrationSelfCheck, DjRecordingSelfCheck, WirelessApiSelfCheck — all 9 pass under g++ -std=c++11 -Wall -Wextra -Werror -fsanitize=address,undefined.
• Full arduino-cli build, cm:esp32@1.8.3 (CircuitOS 1.7.1), Library 78a9f3d7e52fd151dc50872812ae2ed1da308af1, both configurations, at final SHA ccca174:
  • default (no wireless): 1,384,250 B flash (38%) / 47,160 B RAM (14%)
  • -DJAYD_ENABLE_WIRELESS: 1,485,582 B flash (41%) / 52,768 B RAM (16%)

Auto DJ remains out of scope for this PR.

Review fixes, round 7 (1 HIGH production issue + 2 test-quality gaps, all addressed, history not amended)

A focused re-review of ccca174 found a real production bug plus two test gaps that let it slip through. Fixed in ebdd3a9; 478667a/c03ecc4/c863d79/eca7930/b822c52/2b8f319/8d387ce/4292807/ccca174 untouched:

• Mix screen #1 shutdown deadlock on fill-task launch failure (HIGH): bundled CircuitOS's Task::start() sets stopped=false unconditionally before calling xTaskCreate(); on launch failure it only logs and never restores stopped, and only the task's own trampoline (which never runs if creation failed) ever sets stopped=true. Task::stop(true)'s while(!stopped) delay(1); therefore deadlocks forever, and DjAssistController::end() calls exactly that. Patching bundled CircuitOS was out of scope. Replaced Util/Task usage with a new, locally-owned DjAssistFillWorker: begin() honestly reports launch success/failure via observable launched()/entered()/exited() state, so a failed launch leaves the controller disabled and end() returns immediately instead of blocking; the firmware path uses raw FreeRTOS xTaskCreate/self-vTaskDelete directly (same priority/affinity as before, never touching CircuitOS's own Task class); the host path (new JAYD_ASSIST_HOST_TASK_SEAM macro, needed because -DARDUINO is applied at directory scope to all targets including host tests) uses std::thread, with a useManualSteppingForTest flag (default true) preserving byte-for-byte behavior of every pre-existing deterministic test that drives fillWorkerStep() directly via friend access.
• Playback #2 candidate TOCTOU test didn't exercise the real gap (MEDIUM): the round-6 test bumped metadataRevision before calling tick(), a case even the pre-fix code already handled correctly, so it proved nothing about the actual readiness/consumption race. Added a genuine second, defense-in-depth live-revision recheck inside tickSuggestions(), under the same candidateMutex_ critical section, immediately before the real DjAssistScoring::scanTick() consume call, plus a test-only hook (testHookBeforeScanConsume_) invoked at exactly that point so a test can inject a revision bump precisely between "readiness decided" and "table consumed" — a window a pre-tick bump cannot reach.
• Song list #3 shutdown test's Task stub never ran real concurrency (MEDIUM): the host Util/Task.h stub was a pure no-op, so the round-6 shutdown test only proved "safe no-op after end()" rather than any genuine ordering guarantee. DjAssistFillWorker's real std::thread path on host now gives tests actual concurrency to exercise.

New DjAssistIntegrationSelfCheck tests: launch-failure leaves the controller disabled and end() returns immediately (no hang); a real background thread enters, runs, and exits cleanly on end(); end() blocks until an in-flight reader call is released and only completes after the worker has actually exited (proves no close-before-exit race); suggestions are discarded when the live revision changes in the exact window between readiness and consume (proves #2's recheck is load-bearing).

Two incidental host-stub fixes were required for the new real-thread tests to be valid and not introduce false races: tests/host_stubs/Sync/Mutex.h was a no-op stub with no real exclusion — now wraps std::mutex so candidateMutex_ provides genuine mutual exclusion once a background thread exists; tests/host_stubs/host_arduino_shim.cpp's delay() was a no-op, which would have made the new background thread busy-spin at 100% CPU on host — now does a real sleep (confirmed by grep that delay() is reachable only from DjAssistFillWorker.cpp's host-thread path, so this has zero effect on any pre-existing single-threaded test).

Added a new DjAssistIntegrationSelfCheckTSan CTest target (same sources, -fsanitize=thread, Clang/GNU only, since TSan can't combine with ASan/UBSan in one binary) to validate the new real-concurrency paths for data races — passes with no races reported.

Preserves the round-6 ownership reconciliation, dedicated metadataRevision, readiness/consumption critical section, and shutdown-close ordering unchanged; only the fill-task lifecycle mechanism itself moved off CircuitOS Task.

Validation

• ctest: all 10 targets pass — LibraryIndexSelfCheck, DjSessionSelfCheck, DjBeatEngineSelfCheck, MetadataIntegrationSelfCheck, DjAssistSelfCheck, DjAssistBridgeSelfCheck, DjAssistIntegrationSelfCheck (ASan/UBSan), DjAssistIntegrationSelfCheckTSan (new, ThreadSanitizer), DjRecordingSelfCheck, WirelessApiSelfCheck — under g++ -std=c++11 -Wall -Wextra -Werror -fsanitize=address,undefined (or -fsanitize=thread for the TSan target).
• Full arduino-cli build, cm:esp32@1.8.3 (CircuitOS 1.7.1), Library 78a9f3d7e52fd151dc50872812ae2ed1da308af1, both configurations, at final SHA ebdd3a9:
  • default (no wireless): 1,384,774 B flash (38%) / 47,168 B RAM (14%)
  • -DJAYD_ENABLE_WIRELESS: 1,486,114 B flash (41%) / 52,776 B RAM (16%)
  • (both ~+530 B flash / +8 B RAM vs. round-6, consistent with the new fill-worker code)

Auto DJ remains out of scope for this PR.

Dependency revalidation: Library pin updated to 35b4f80

Coach ebdd3a9 (round-7 head, unchanged, no source edits needed) revalidated against the canonical JayD-Library PR #22 head moving 78a9f3d → 35b4f80a47f5e5eef029e0c57db76699a076df4a (confirmed via git rev-parse HEAD on the pinned checkout).

Library diff in this range (OutputAAC.cpp/OutputWAV.cpp/SourceAAC.cpp/SourceMP3.cpp/SourceWAV.cpp/SDScheduler.cpp/.h + tests) reworks SDScheduler::addJob from a transient unmerged bool addJob(SDJob*) (only ever present at 78a9f3d) back to the master-compatible void addJob(SDJob*), adding a new bool tryAddJob(SDJob*). grep confirms no Coach/DjAssist source references SDScheduler/addJob at all — zero surface overlap, no source change required. DjRecordingSelfCheck only includes WavHeader.h/AudioSetup.hpp (unchanged in this diff), so it validates unaffected too.

Validation

• ctest: all 10 targets pass, clean rebuild, no source changes — LibraryIndexSelfCheck, DjSessionSelfCheck, DjBeatEngineSelfCheck, MetadataIntegrationSelfCheck, DjAssistSelfCheck, DjAssistBridgeSelfCheck, DjAssistIntegrationSelfCheck (ASan/UBSan), DjAssistIntegrationSelfCheckTSan, DjRecordingSelfCheck, WirelessApiSelfCheck.
• Full arduino-cli build, cm:esp32@1.8.3 (CircuitOS 1.7.1), Library 35b4f80a47f5e5eef029e0c57db76699a076df4a, both configurations, still at SHA ebdd3a9:
  • default (no wireless): 1,384,926 B flash (38%) / 47,168 B RAM (14%)
  • -DJAYD_ENABLE_WIRELESS: 1,486,290 B flash (41%) / 52,776 B RAM (16%)
  • (both ~+150-180 B flash / +0 B RAM vs. round-7's 78a9f3d baseline of 1,384,774 B / 1,486,114 B, matching the small tryAddJob addition)

These figures are cross-confirmed by an independent isolated rebuild (clean environment, singular --library checkout) that reported identical default (1,384,926 B / 47,168 B) and closely matching wireless (1,486,290 B / 52,776 B) numbers. An earlier build attempt on this shared, non-isolated machine transiently reported inflated numbers (~+20 KB on both configs) due to environment/toolchain contention from a concurrent unrelated arduino-cli compile running on the same host at the same time; those transient figures are superseded by the cross-confirmed numbers above and were never a Coach or Library regression.

No Coach source changes were required for this revalidation. Auto DJ remains out of scope for this PR.