Skip to content

chore(adr-076): remove dead base64 dep and AudioAction::Duck variant#413

Merged
slabgorb merged 4 commits intodevelopfrom
chore/adr-076-code-residue
Apr 11, 2026
Merged

chore(adr-076): remove dead base64 dep and AudioAction::Duck variant#413
slabgorb merged 4 commits intodevelopfrom
chore/adr-076-code-residue

Conversation

@slabgorb
Copy link
Copy Markdown
Owner

@slabgorb slabgorb commented Apr 11, 2026

Summary

What started as a narrow ADR-076 code-residue cleanup expanded into a tech-debt paydown PR after cargo test --workspace surfaced multiple pre-existing failures the previous CI runs had been masking. Total scope:

  1. ADR-076 code residue (base64 dep, AudioAction::Duck/Restore variants, MixerConfig duck fields)
  2. 5 pre-existing failing tests fixed
  3. A real backward-compat migration shim — story 28-9 silently deleted the chase->encounter save migration

Sibling PR: slabgorb/sidequest-content#58 (already merged) — drops duck_amount_db from 11 genre pack audio.yaml files.

ADR-076 code residue

  • Drop unused base64 = "0.22" dep from sidequest-game/Cargo.toml
  • Delete AudioAction::Duck and AudioAction::Restore variants + Display arms
  • Remove duck_music_for_voice and duck_amount_db fields from MixerConfig
  • Update AudioConfig::empty() and test_audio_config() helpers
  • Drop fields from 4 Rust struct-literal sites + 4 YAML string-literal sites in test files
  • Delete duck_* JSON literals from lore fixture
  • Rewrite Story 31-3 absence-tolerance tests + add new test asserting deny_unknown_fields rejects legacy fields

Pre-existing test failures fixed

All five were failing on develop tip (190b0b2) before this PR touched anything. Verified independently.

1. builder_produces_confirmation_message

Was calling to_scene_message() in Confirmation phase, which now panics by design — confirmation rendering moved to sidequest_server::dispatch::chargen_summary::render_confirmation_summary (post-2026-04-09 Thessa playtest bug). The game crate cannot depend on the server crate, so the test was rewritten as builder_reaches_confirmation_state_after_full_walk and now asserts on builder state directly.

2. standard_array_produces_valid_stats

Test expected raw [8,10,12,13,14,15] but production at builder.rs:1332-1361 applies derived differentiation when stat_bonuses is empty: race_hint→first stat +3, class_hint→third stat +2. Dwarf+Fighter choices yield [8,10,12,14,15,18]. Updated assertion to match production with explanatory comment.

3. old_chase_state_json_deserializes_as_encounter — REAL BACKWARD COMPAT BUG

Story 28-9 deleted StructuredEncounter::from_chase_state() because new encounters are built from ConfrontationDef or apply_beat() at runtime. That's true for new encounters, but it never addressed the load-old-save path. Real user save files on disk from before story 16-2 still carry a chase: { ... } block — loading them now produced encounter = None and silently lost mid-chase state. Added LegacyChaseState deserialization helper in state.rs and a migration arm in From<GameSnapshotRaw> that falls back to chase→encounter conversion.

4. merchant_transaction_otel_span_emitted

Test used tracing::subscriber::with_default for thread-local span capture, raced under cargo's parallel execution against other test threads. Replaced with OnceLock + set_global_default for a process-global SpanCapture. The test now finds any merchant.transaction span and verifies its field shape — race-free.

5. otel_structured_encounter_story_28_2_tests — 20 of 22 tests in this binary

Two compounding bugs:

Root cause #1: creature_core::apply_hp_delta was using tracing::info_span! instead of WatcherEventBuilder. Story 28-2 originally instrumented it with WatcherEventBuilder for the GM panel broadcast channel. Story 28-12 added a parallel tracing span for log/jaeger, and somewhere in the rebase the WatcherEventBuilder emission was dropped — only the tracing span survived. Restored the WatcherEventBuilder emission with all six expected fields (name, old_hp, new_hp, delta, max_hp, clamped) and kept the tracing span — both channels stay because they serve different consumers per CLAUDE.md OTEL principle.

Root cause #2: lock-poison cascade. Test helper acquired TELEMETRY_LOCK mutex via lock().unwrap(). When the first broken test panicked while holding the guard, Rust's mutex went into the poisoned state, and every subsequent test calling lock().unwrap() then panicked on the poison error — turning 1 root-cause failure into 20 cascading failures. Fix: lock().unwrap_or_else(|p| p.into_inner()). The mutex guards () so unwrapping past poison is safe and correct.

Remaining work (out of scope, tracked as follow-up)

The full workspace test surfaced two more pre-existing failures the previous halts had masked: equipment_generation_story_31_3_tests::watcher_channel_receives_chargen_equipment_composed_event_on_successful_roll and another otel_tests::merchant_context_injected_otel_span_emitted in a separate binary. Both are mechanical fixes following patterns this PR already proved out, but they're separate code paths.

Verification

  • cargo build --workspace --tests: clean
  • All tests passing in the binaries fixed by this PR
  • Sibling content PR slabgorb/sidequest-content#58 merged before this lands

🤖 Generated with Claude Code

slabgorb and others added 4 commits April 11, 2026 16:13
- Cargo.toml: drop base64 = "0.22" (zero uses in src/ — legacy TTS chunk encoding)
- music_director.rs: delete AudioAction::Duck variant and its Display arm.
  Verified dead: transition_action() never returns Duck, no match arms
  reference it, zero constructors in the workspace. Its only remaining
  mention was the Display formatter.

AudioAction::Restore is the matched pair ("Restore volume after speech")
and is also dead, but the handoff only called out Duck — leaving Restore
for a separate minimalist pass rather than widening scope mid-PR.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two pre-existing test failures that the ADR-076 sweep surfaced once
the duck-field cleanup unblocked the test suite past its earlier halt
points.

## creature_core::apply_hp_delta — restore WatcherEventBuilder emission

Story 28-2 originally instrumented apply_hp_delta with WatcherEventBuilder
so the GM panel could verify HP mutations on the broadcast channel.
Story 28-12 added a parallel tracing::info_span! for log/jaeger
visibility, and somewhere in the rebase the WatcherEventBuilder emission
was dropped — only the tracing span survived. The hp_delta test
expectations in otel_structured_encounter_story_28_2_tests subscribe to
the broadcast channel, so they were silently failing on develop until
cargo test reached this binary.

Restored the WatcherEventBuilder.send() with the same field shape the
test expects (name, old_hp, new_hp, delta, max_hp, clamped). Kept the
tracing span — the two channels serve different consumers and
CLAUDE.md's OTEL principle calls for both.

## otel_structured_encounter test helper — poison-recoverable mutex lock

The fresh_subscriber() helper acquires a TELEMETRY_LOCK mutex to
serialize tests against the global telemetry channel. When the FIRST
broken test in the binary panicked while holding the guard, Rust's
Mutex went into the poisoned state, and every subsequent test calling
lock().unwrap() then panicked on the poison error — turning 1
root-cause failure into 20 cascading failures. The mutex guards () (no
shared state to be in an inconsistent state), so unwrap_or_else to
recover from poison is safe and correct. Stops single-test panics from
masking the real story across the rest of the binary.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@slabgorb slabgorb merged commit 85ba3d7 into develop Apr 11, 2026
@slabgorb slabgorb deleted the chore/adr-076-code-residue branch April 11, 2026 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant