Skip to content

Commit c9d7b0b

Browse files
jithinABclaude
andcommitted
fix(rr): Wave-A brutal-review fixes — barge-in poisoning (CRITICAL) + Anthropic 400s
5-lens adversarial review of Wave A (wf_6afb89db) confirmed + empirically reproduced 3 real defects; fixed: CRITICAL (d3-masking-races / regression-surface, both lenses + reproduced): the D3 filler called arm_barge_in_suppression(350) which flips the SESSION-GLOBAL allow_interruption=false, but the real answer is spoken via speak_if_epoch( allow_interruption=true) which never RESTORES it — so the flag stayed false, the egress wrapper kept extending non_interruptible_until_ms per answer chunk, and can_interrupt() returned false for the WHOLE answer (and leaked across turns). The STT callback drops barge-in when !can_interrupt(), so a user could not barge in on the real answer — silently re-introducing the exact §4.4 failure on the slow turns D3 targets. Fix: REMOVE arm_barge_in_suppression from the masking timer — the filler is interruptible by design (a real barge-in during it must be honored); echo cancellation, not a session-wide flag flip, handles the bot's own tail. New regression test latency_filler_does_not_poison_interruptibility asserts is_interruption_blocked()==false after a masked turn. (arm_barge_in_suppression stays as a VoiceManager API + the §4.4 test's window simulator.) HIGH (d1-vendor-correctness, verified against the claude-api skill): the Anthropic adapter emitted thinking:{type:"enabled",budget_tokens} for (a) adaptive-only models (Opus 4.7/4.8, Fable 5) which REJECT that form with a 400, and (b) budgets clamped below the 1024 minimum (the voice cap 256 makes [1024, max_tokens) unsatisfiable) — also a guaranteed 400. Fix: adaptive-only models emit NO thinking block (they think adaptively by default); others emit `enabled` ONLY when a valid budget (>=1024 and <max_tokens) fits, else nothing — never a guaranteed 400. is_anthropic_adaptive_only helper + ANTHROPIC_MIN_THINKING_BUDGET; test rewritten for the 400-safe matrix (voice-cap→none, room→enabled, adaptive-only→none, off→none). lib floor 6173/0, conversation_loop 20/0 (incl the new regression), clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1fd5397 commit c9d7b0b

3 files changed

Lines changed: 112 additions & 19 deletions

File tree

gateway/src/core/conversation/mod.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -542,10 +542,15 @@ impl ConversationOrchestrator {
542542
if latch.swap(true, std::sync::atomic::Ordering::AcqRel) {
543543
return; // one masking utterance per turn
544544
}
545-
// Suppress the filler's own VAD tail (≤400ms) while keeping the
546-
// clip interruptible, then speak it (epoch-gated so a barge-in
547-
// that already cleared drops it).
548-
vm.arm_barge_in_suppression(350);
545+
// Speak the filler as ORDINARY interruptible audio (epoch-gated
546+
// so a barge-in that already cleared drops it). We must NOT flip
547+
// the session-global interruption flag here: the filler is
548+
// interruptible by design, and a real barge-in during it (or
549+
// during the real answer that follows) must be honored — the
550+
// brutal review proved that poisoning allow_interruption here
551+
// silently disabled barge-in for the WHOLE answer (critique:
552+
// re-introduced the §4.4 failure). Echo cancellation, not a
553+
// session-wide suppression window, handles the bot's own tail.
549554
match vm.speak_if_epoch(&phrase, true, true, epoch).await {
550555
Ok(true) => debug!(session = %session, %phrase, "D3 masking filler spoken"),
551556
Ok(false) => {

gateway/src/core/llm/adapter.rs

Lines changed: 61 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,24 @@ impl ReasoningEffort {
138138
}
139139
}
140140

141+
/// Anthropic minimum `budget_tokens` for the `type:"enabled"` thinking form.
142+
/// Below this, the API 400s — so we don't emit the enabled form (see render).
143+
const ANTHROPIC_MIN_THINKING_BUDGET: u32 = 1024;
144+
145+
/// Whether an Anthropic model is adaptive-thinking-ONLY (Opus 4.7/4.8, Fable 5):
146+
/// these REJECT `thinking:{type:"enabled",budget_tokens}` with a 400 and only
147+
/// honor their built-in adaptive thinking — so we emit no thinking block at all
148+
/// (the model thinks adaptively by default; budget can't control it).
149+
fn is_anthropic_adaptive_only(model: &str) -> bool {
150+
let m = model.to_ascii_lowercase();
151+
m.contains("opus-4-7")
152+
|| m.contains("opus-4.7")
153+
|| m.contains("opus-4-8")
154+
|| m.contains("opus-4.8")
155+
|| m.contains("fable-5")
156+
|| m.contains("claude-fable")
157+
}
158+
141159
/// Anthropic extended-thinking budget per effort. The caller MUST further clamp
142160
/// to `< max_tokens` (Anthropic 400s otherwise).
143161
fn anthropic_budget_tokens(e: ReasoningEffort) -> u32 {
@@ -636,18 +654,27 @@ impl LlmAdapter for AnthropicAdapter {
636654
if let Some(p) = cfg.top_p {
637655
obj.insert("top_p".into(), json!(p));
638656
}
639-
// D1: Anthropic extended thinking. `Off`/`None` → emit nothing (default
640-
// is no thinking). EXACTLY ONE param (`thinking`). INVARIANT: Anthropic
641-
// 400s unless `budget_tokens < max_tokens`, so clamp the budget below the
642-
// body's `max_tokens` (the voice path caps at 256).
657+
// D1: Anthropic extended thinking (brutal-review hardened). `Off`/`None`
658+
// → nothing. EXACTLY ONE param. The `enabled` form requires
659+
// `1024 <= budget_tokens < max_tokens`; the voice cap (256) makes that
660+
// jointly unsatisfiable, and adaptive-only models (Opus 4.7/4.8, Fable 5)
661+
// reject the `enabled` form entirely. So: adaptive-only → emit nothing
662+
// (model thinks adaptively by default); otherwise emit `enabled` ONLY when
663+
// a valid budget (>= 1024 and < max_tokens) fits — else emit nothing
664+
// rather than a guaranteed 400.
643665
if let Some(effort) = cfg.reasoning_effort {
644-
if effort != ReasoningEffort::Off {
666+
if effort != ReasoningEffort::Off && !is_anthropic_adaptive_only(&cfg.model) {
645667
let max_tok = cfg.max_tokens.unwrap_or(ANTHROPIC_DEFAULT_MAX_TOKENS);
646668
let budget = anthropic_budget_tokens(effort).min(max_tok.saturating_sub(1));
647-
obj.insert(
648-
"thinking".into(),
649-
json!({ "type": "enabled", "budget_tokens": budget }),
650-
);
669+
if budget >= ANTHROPIC_MIN_THINKING_BUDGET {
670+
obj.insert(
671+
"thinking".into(),
672+
json!({ "type": "enabled", "budget_tokens": budget }),
673+
);
674+
}
675+
// else: budget can't satisfy [1024, max_tokens) — skip thinking
676+
// (no 400). A reasoning Anthropic model on voice needs a higher
677+
// explicit max_tokens to actually think.
651678
}
652679
}
653680
if let Some(stop) = &cfg.stop {
@@ -1317,21 +1344,40 @@ mod tests {
13171344
}
13181345

13191346
#[test]
1320-
fn anthropic_maps_reasoning_effort_to_thinking_budget_below_max_tokens() {
1347+
fn anthropic_maps_reasoning_effort_thinking_400_safe() {
1348+
// Voice cap (256): a valid budget needs 1024 <= budget < max_tokens, which
1349+
// is unsatisfiable at 256 → emit NO thinking block (never a guaranteed 400).
13211350
let mut c = cfg(Some(AdapterKind::Anthropic));
1322-
c.max_tokens = Some(256); // the voice path's cap
1323-
c.reasoning_effort = Some(ReasoningEffort::High); // raw budget 8192 > 256
1351+
c.max_tokens = Some(256);
1352+
c.reasoning_effort = Some(ReasoningEffort::High);
1353+
let r = AnthropicAdapter.render_request(&convo(), &c, false, None);
1354+
assert!(
1355+
r.body.get("thinking").is_none(),
1356+
"budget can't satisfy [1024, 256) → no thinking block, not a 400"
1357+
);
1358+
1359+
// Room for a valid budget → enabled with 1024 <= budget < max_tokens.
1360+
c.max_tokens = Some(4000);
13241361
let r = AnthropicAdapter.render_request(&convo(), &c, false, None);
13251362
assert_eq!(r.body["thinking"]["type"], "enabled");
13261363
let budget = r.body["thinking"]["budget_tokens"].as_u64().unwrap();
1327-
assert!(budget < 256, "budget {budget} must be < max_tokens 256 (anti-400)");
1364+
assert!((1024..4000).contains(&(budget as u32)), "budget {budget} in [1024,4000)");
13281365
assert!(r.body.get("reasoning_effort").is_none(), "exactly one param");
13291366

1330-
// Off → no thinking key at all (Anthropic default is no extended thinking).
1367+
// Adaptive-only model (Opus 4.8) rejects the enabled form → emit nothing.
1368+
c.model = "claude-opus-4-8".to_string();
1369+
c.reasoning_effort = Some(ReasoningEffort::High);
1370+
let r = AnthropicAdapter.render_request(&convo(), &c, false, None);
1371+
assert!(
1372+
r.body.get("thinking").is_none(),
1373+
"adaptive-only model: never the enabled+budget form (it 400s)"
1374+
);
1375+
1376+
// Off / None → no thinking key (default no extended thinking).
1377+
c.model = "claude-sonnet-4-5".to_string();
13311378
c.reasoning_effort = Some(ReasoningEffort::Off);
13321379
let r = AnthropicAdapter.render_request(&convo(), &c, false, None);
13331380
assert!(r.body.get("thinking").is_none());
1334-
13351381
c.reasoning_effort = None;
13361382
let r = AnthropicAdapter.render_request(&convo(), &c, false, None);
13371383
assert!(r.body.get("thinking").is_none());

gateway/tests/conversation_loop.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,48 @@ async fn latency_filler_off_speaks_nothing() {
550550
);
551551
}
552552

553+
#[tokio::test]
554+
#[serial_test::serial]
555+
async fn latency_filler_does_not_poison_interruptibility() {
556+
// REGRESSION (brutal review CRITICAL): the masking filler must NOT leave the
557+
// session non-interruptible — a barge-in on the real answer (and on every
558+
// later turn) must keep working. The old code armed a session-global
559+
// suppression window that was never restored, silently disabling barge-in.
560+
unsafe {
561+
std::env::set_var("WAAV_ALLOW_LOOPBACK_ENDPOINTS", "1");
562+
}
563+
register_mock_tts();
564+
reset_tts_stats();
565+
566+
let llm_state = LlmMockState::default();
567+
*llm_state.reply.lock() = "Your balance is forty two dollars.".to_string();
568+
llm_state.delay_ms.store(400, Ordering::SeqCst);
569+
let base_url = start_llm_mock(llm_state.clone()).await;
570+
571+
let vm = build_voice_manager();
572+
vm.start().await.expect("vm start");
573+
let _ = wire_audio_egress(&vm).await;
574+
575+
let cfg = ConversationConfig {
576+
latency_filler: LatencyFiller::Auto,
577+
latency_filler_after_ms: Some(100),
578+
..conv_config(base_url, false)
579+
};
580+
let orchestrator =
581+
Arc::new(ConversationOrchestrator::new("session-poison", cfg, vm.clone()).expect("orch"));
582+
orchestrator.run_turn("what is my balance").await.ok();
583+
tokio::time::sleep(Duration::from_millis(50)).await;
584+
585+
// A filler fired (slow turn past the 100ms threshold).
586+
let spoken = TTS_STATS.spoken.lock().clone();
587+
assert!(spoken.iter().any(|s| is_filler(s)), "filler should have fired: {spoken:?}");
588+
// The session must NOT be stuck non-interruptible after the masked turn.
589+
assert!(
590+
!vm.is_interruption_blocked().await,
591+
"barge-in must remain available after a masking filler (no allow_interruption poisoning)"
592+
);
593+
}
594+
553595
// ──────────────────────────────────────────────────────────────────────────────
554596
// TEST 3: multi-turn history is preserved (turn 2 includes turn 1).
555597
// ──────────────────────────────────────────────────────────────────────────────

0 commit comments

Comments
 (0)