mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(buzz-agent): recover from 400-shaped image rejections; unbound benchmark agent rounds (#5318)
## Problem
Two failure modes from the `tb21-glm52-crusoe-1` benchmark run (GLM-5.2
solo, TB2.1) wedged or killed 13 of 89 trials without the model being at
fault:
1. **Conversation poisoning on text-only endpoints.** Crusoe's
serverless `crusoeai/GLM-5.2-NVFP4` rejects any request whose history
contains an image with `400: ... is not a multimodal model`. The
recovery machinery for exactly this case already exists —
`AgentError::UnsupportedImageInput` → `replace_unsupported_images()`
strips the image blocks, marks the tool result as an error, substitutes
a text placeholder, and continues the turn. But classification only
matched OpenRouter's 404 body (`no endpoints found that support image
input`) and was only consulted on the 404 arms. The Crusoe 400 fell
through to terminal `AgentError::Llm`: the image stayed in history,
every subsequent call failed identically, buzz-acp rode its 10-retry
ladder (~40 min), and the trial idled to budget death. Measured blast
radius: **8 trials wedged, 12.7h aggregate idle-after-poison.**
2. **Bounded agent rounds in benchmark trials.** The harness default
`DEFAULT_MAX_AGENT_ROUNDS = 32` ended solo trials mid-work when turns
rotated (thinking-heavy models hit max_tokens rotation fast; 4 trials
died this way). Benchmark trials already have a wall-clock budget as the
real limit — the round cap only converts recoverable rotation into trial
death.
## Fix
- `is_unsupported_image_input_error()` also matches the verbatim `is not
a multimodal model` body. Matcher stays deliberately tight (same
doctrine as `is_context_length_error`): misclassifying a generic 400 as
recoverable would mutate history for an error that removing images
cannot fix.
- Both status ladders — shared `post()` and `openrouter_post()` —
consult it on their 400 arms and return the typed
`UnsupportedImageInput` (OpenAI-compatible providers report this as 400;
a BYOK/passthrough upstream can surface the provider's own 400 through
OpenRouter).
- Harness `DEFAULT_MAX_AGENT_ROUNDS` → `0` (unbounded —
`BUZZ_AGENT_MAX_ROUNDS=0` is the agent config's documented unbounded
value). Per-agent `budget.max_calls` in manifests still overrides.
## Acceptance
- A 400 with the image-rejection body reaches the existing image-strip
recovery path instead of wedging the session — asserted through
`complete()` (covers the return path into the convergence mapper) and at
the `openrouter_post` terminal, both proving single-attempt (a
deterministic capability rejection must never be retried).
- Ordinary 400s stay terminal `AgentError::Llm` (existing negative tests
unchanged).
- Benchmark trials run unbounded rounds by default; python tests updated
for 0-is-legal with a negative arm at -1.
## Verification
- `cargo test -p buzz-agent`: 427 + 18 + 20 + 15 + 8 + 1 + 48 passed, 0
failed (full package, 3 consecutive clean runs)
- `cargo clippy -p buzz-agent --all-targets`, `cargo fmt --check`: clean
- `uv run --extra dev pytest tests/` in harbor-buzz-orchestra: 35 passed
- Pre-push hooks (full workspace rust-tests + desktop-tauri-checks)
green on rustc 1.95.0 at head b0438602
Signed-off-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
Co-authored-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -24,7 +24,7 @@ from .manifest import AgentClass, ExperimentManifest
|
||||
from .provisioning import AgentCredential, TrialHandle
|
||||
from .runtime import RuntimeResult
|
||||
|
||||
DEFAULT_MAX_AGENT_ROUNDS = 32
|
||||
DEFAULT_MAX_AGENT_ROUNDS = 0 # 0 = unbounded (BUZZ_AGENT_MAX_ROUNDS=0); the trial budget is the clock
|
||||
# Container-side layout for the uploaded Buzz stack.
|
||||
REMOTE_ROOT = "/opt/buzz"
|
||||
REMOTE_BIN = f"{REMOTE_ROOT}/bin"
|
||||
@@ -80,8 +80,8 @@ class BuzzContainerRuntime:
|
||||
readiness_timeout_seconds: float = 60.0,
|
||||
poll_seconds: float = 1.0,
|
||||
) -> None:
|
||||
if max_agent_rounds <= 0:
|
||||
raise ValueError("max_agent_rounds must be positive")
|
||||
if max_agent_rounds < 0:
|
||||
raise ValueError("max_agent_rounds must be >= 0 (0 = unbounded)")
|
||||
if readiness_timeout_seconds <= 0:
|
||||
raise ValueError("readiness_timeout_seconds must be positive")
|
||||
self.logs_dir = Path(logs_dir)
|
||||
|
||||
@@ -247,7 +247,7 @@ async def test_forwarder_bridges_the_canonical_relay_address(tmp_path):
|
||||
rt._ws_authority("http://relay")
|
||||
|
||||
|
||||
@pytest.mark.parametrize(("configured", "expected"), [(None, "32"), (7, "7")])
|
||||
@pytest.mark.parametrize(("configured", "expected"), [(None, "0"), (7, "7")])
|
||||
async def test_launch_wires_the_desktop_environment(tmp_path, configured, expected):
|
||||
manifest = write_manifest(tmp_path)
|
||||
agent_class = manifest.roster[0]
|
||||
@@ -290,9 +290,12 @@ async def test_launch_wires_the_desktop_environment(tmp_path, configured, expect
|
||||
)
|
||||
|
||||
|
||||
def test_runtime_rejects_unbounded_agent_rounds(tmp_path):
|
||||
with pytest.raises(ValueError, match="positive"):
|
||||
runtime(tmp_path, max_agent_rounds=0)
|
||||
def test_runtime_validates_construction_bounds(tmp_path):
|
||||
# 0 is legal and means unbounded (BUZZ_AGENT_MAX_ROUNDS=0); the trial
|
||||
# budget is the clock. Only negatives are rejected.
|
||||
runtime(tmp_path, max_agent_rounds=0)
|
||||
with pytest.raises(ValueError, match="unbounded"):
|
||||
runtime(tmp_path, max_agent_rounds=-1)
|
||||
with pytest.raises(ValueError, match="positive"):
|
||||
runtime(tmp_path, readiness_timeout_seconds=0)
|
||||
|
||||
|
||||
@@ -1932,9 +1932,24 @@ fn classify_body_read_error(
|
||||
}
|
||||
}
|
||||
|
||||
/// Provider bodies that mean "this model cannot accept image input", the
|
||||
/// signal the agent loop uses to strip rejected images from history and
|
||||
/// continue the turn (see `replace_unsupported_images`).
|
||||
///
|
||||
/// Deliberately tight, same doctrine as [`is_context_length_error`]: each
|
||||
/// phrase is a verbatim capability rejection observed live. Misclassifying a
|
||||
/// generic 400 as recoverable would mutate history for an error that removing
|
||||
/// images cannot fix.
|
||||
fn is_unsupported_image_input_error(body: &str) -> bool {
|
||||
body.to_ascii_lowercase()
|
||||
.contains("no endpoints found that support image input")
|
||||
let b = body.to_ascii_lowercase();
|
||||
// OpenRouter 404: no provider endpoint accepts images for this model.
|
||||
b.contains("no endpoints found that support image input")
|
||||
// OpenAI-compatible 400 from text-only single-model deployments,
|
||||
// e.g. Crusoe serverless GLM: `"crusoeai/GLM-5.2-NVFP4 is not a
|
||||
// multimodal model"`. Without this arm the 400 is terminal, the image
|
||||
// stays in history, and every subsequent request in the session fails
|
||||
// identically — the turn wedges until the harness/user gives up.
|
||||
|| b.contains("is not a multimodal model")
|
||||
}
|
||||
|
||||
/// Build the terminal `AgentError::Llm` for a `post()` exit that has given up
|
||||
@@ -2129,6 +2144,13 @@ where
|
||||
"{status}: {body}"
|
||||
))));
|
||||
}
|
||||
// Image-capability rejection is equally recoverable and equally
|
||||
// deterministic: a text-only deployment 400s the same request
|
||||
// forever. Typed here (not just on the 404 arm) because
|
||||
// OpenAI-compatible providers report it as a 400.
|
||||
if status == 400 && is_unsupported_image_input_error(&body) {
|
||||
return Err(PostError::Agent(AgentError::UnsupportedImageInput(body)));
|
||||
}
|
||||
return Err(PostError::Agent(AgentError::Llm(format!(
|
||||
"{status}: {body}"
|
||||
))));
|
||||
@@ -2535,6 +2557,12 @@ async fn openrouter_post(
|
||||
if status == 400 && is_context_length_error(&body) {
|
||||
return Err(AgentError::LlmContextExceeded(format!("{status}: {body}")));
|
||||
}
|
||||
// Same 400-shaped image rejection as the shared `post()` terminal:
|
||||
// OpenRouter normally reports this as a 404 (handled above), but a
|
||||
// BYOK/passthrough upstream can surface the provider's own 400.
|
||||
if status == 400 && is_unsupported_image_input_error(&body) {
|
||||
return Err(AgentError::UnsupportedImageInput(body));
|
||||
}
|
||||
return Err(AgentError::Llm(format!("{status}: {body}")));
|
||||
}
|
||||
if let Some(len) = resp.content_length() {
|
||||
@@ -7544,6 +7572,73 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// OpenAI-compatible text-only deployments report the image rejection as a
|
||||
/// 400, not OpenRouter's 404 — Crusoe serverless GLM answers
|
||||
/// `"crusoeai/GLM-5.2-NVFP4 is not a multimodal model"` to every request
|
||||
/// whose history contains an image. Before the 400 arm existed, this fell
|
||||
/// through to terminal `AgentError::Llm`: the image stayed in history and
|
||||
/// every later call in the session failed identically (measured live:
|
||||
/// 8 wedged benchmark trials, 40 min of doomed retries each). Asserted
|
||||
/// through `complete()` so the arm's return path into the convergence
|
||||
/// mapper is covered, same doctrine as the context-400 tests above.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn openai_400_unsupported_image_is_typed_through_complete() {
|
||||
let (base_url, captured) = spawn_sequence_stub(vec![StubHttpResponse {
|
||||
status: 400,
|
||||
body: json!({"error":{"message":"crusoeai/GLM-5.2-NVFP4 is not a multimodal model","type":"invalid_request_error"}}),
|
||||
}])
|
||||
.await;
|
||||
let mut c = cfg(Provider::OpenAi);
|
||||
c.base_url = base_url;
|
||||
let llm = Llm::new(&c).unwrap();
|
||||
let err = complete_model(&llm, &c, "gpt-probe-model")
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(&err, AgentError::UnsupportedImageInput(s) if s.contains("not a multimodal model")),
|
||||
"a text-only deployment's 400 must reach the history-recovery path: got {err:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
captured.lock().await.len(),
|
||||
1,
|
||||
"a deterministic capability rejection must not be retried"
|
||||
);
|
||||
}
|
||||
|
||||
/// Same 400-shaped rejection at the OpenRouter terminal, which has its own
|
||||
/// status ladder: a BYOK/passthrough upstream can surface the provider's
|
||||
/// own 400 body instead of OpenRouter's 404 routing error.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn openrouter_post_400_unsupported_image_is_typed_and_not_retried() {
|
||||
let (url, _captured, attempts) = spawn_openrouter_stub(vec![CannedResponse::new(
|
||||
400,
|
||||
r#"{"error":{"message":"crusoeai/GLM-5.2-NVFP4 is not a multimodal model"}}"#,
|
||||
)])
|
||||
.await;
|
||||
let http = Client::builder()
|
||||
.timeout(Duration::from_secs(5))
|
||||
.build()
|
||||
.unwrap();
|
||||
let err = openrouter_post(
|
||||
&http,
|
||||
&format!("{url}/x"),
|
||||
&json!({}),
|
||||
"key",
|
||||
Duration::from_secs(5),
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(&err, AgentError::UnsupportedImageInput(s) if s.contains("not a multimodal model")),
|
||||
"image rejection must reach the history-recovery path: got {err:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
attempts.load(std::sync::atomic::Ordering::SeqCst),
|
||||
1,
|
||||
"a deterministic capability rejection must not be retried"
|
||||
);
|
||||
}
|
||||
|
||||
/// Every other 404 still maps to `LlmModelNotFound`, including one that
|
||||
/// shares the `No endpoints found` prefix but is about the model rather than
|
||||
/// the parameters — the discriminator is narrow enough that a genuinely
|
||||
|
||||
Reference in New Issue
Block a user