diff --git a/scripts/ci/contextual_orchestrator_review_sidecar.sh b/scripts/ci/contextual_orchestrator_review_sidecar.sh index b044cab82c..28b36e8dd0 100755 --- a/scripts/ci/contextual_orchestrator_review_sidecar.sh +++ b/scripts/ci/contextual_orchestrator_review_sidecar.sh @@ -380,7 +380,19 @@ fi # internal error, which is the failure this contract prevents from reaching the # scanner step. gateway_virtual_model="orchestrator/${orchestrator_pool}" -printf '{"model":"%s","messages":[{"role":"system","content":"You are a helpful assistant."},{"role":"user","content":"Reply with just '\''OK'\''."}],"temperature":1.0,"max_tokens":16,"stream":false}\n' \ +# max_tokens must match REVIEW_MAX_OUTPUT_TOKENS (the launcher's own per-agent +# routing probe budget): a reasoning-capable free-tier model (e.g. a DeepSeek +# NIM route) spends part of its token budget on internal reasoning before any +# answer content, so a small budget here can make an agent the routing probe +# already proved "ready" fail this separate end-to-end check with a spurious +# "response did not contain assistant content" -> 502 invalid_structured_output +# (contextual_orchestrator.orchestrator._response_content), even though the +# model itself is healthy. See "2026-08-30 sidecar preflight max_tokens +# desynchronized from the routing probe" in +# ContextualWisdomLab/contextual-orchestrator's own +# docs/product-technical-gap-baseline.md for the exact-evidence reproduction +# (downloaded strix-reports artifact, PR #912 run 33304076516). +printf '{"model":"%s","messages":[{"role":"system","content":"You are a helpful assistant."},{"role":"user","content":"Reply with just '\''OK'\''."}],"temperature":1.0,"max_tokens":4096,"stream":false}\n' \ "$gateway_virtual_model" > "$gateway_preflight_request" if ! gateway_http_status="$( curl -sS --max-time 30 \ diff --git a/tests/test_contextual_orchestrator_review_runtime_preflight.py b/tests/test_contextual_orchestrator_review_runtime_preflight.py index d3ec362378..7858157309 100644 --- a/tests/test_contextual_orchestrator_review_runtime_preflight.py +++ b/tests/test_contextual_orchestrator_review_runtime_preflight.py @@ -5,6 +5,7 @@ from contextlib import redirect_stdout import io import json +import re import runpy from pathlib import Path import sys @@ -182,6 +183,77 @@ def test_preflight_mirrors_runtime_request_and_keeps_only_compatible_routes() -> assert "tools" not in payload +def test_gateway_preflight_max_tokens_is_synchronized_with_the_routing_probe() -> None: + """The bash script's end-to-end gateway check must not retest a route the + Python routing probe already proved ready with a stricter token budget. + + Regression for the 2026-08-30 sidecar-preflight-max-tokens incident: the + routing probe (`_preflight_review_agents`, tested above) already uses + `REVIEW_MAX_OUTPUT_TOKENS` and correctly marked a reasoning-capable + nvidia_nim route "ready". The separate end-to-end gateway check in + ``contextual_orchestrator_review_sidecar.sh`` used to hardcode + ``"max_tokens":16`` for that same virtual-model request -- far too small + for a reasoning model to emit any answer content after its internal + reasoning tokens, so the gateway rejected a route its own routing probe + had just proven healthy. This asserts the two budgets stay numerically + identical so that mismatch cannot silently return; it fails on the + pre-fix literal (16) and passes once the gateway request is synchronized + with the routing probe's budget. + """ + namespace = _load_launcher() + review_max_output_tokens = namespace["REVIEW_MAX_OUTPUT_TOKENS"] + sidecar = _SIDECAR.read_text(encoding="utf-8") + + match = re.search( + r'gateway_virtual_model.*?"max_tokens":(\d+)', sidecar, re.DOTALL + ) + assert match, "sidecar must send one JSON gateway preflight request with an explicit max_tokens" + gateway_preflight_max_tokens = int(match.group(1)) + + assert gateway_preflight_max_tokens == review_max_output_tokens, ( + "gateway preflight max_tokens " + f"({gateway_preflight_max_tokens}) must equal the routing probe's " + f"REVIEW_MAX_OUTPUT_TOKENS ({review_max_output_tokens}); a smaller " + "budget here can reject a route the routing probe already proved " + "ready" + ) + + +def test_reasoning_without_content_remains_rejected_even_with_the_full_budget() -> None: + """A genuinely broken model must still fail closed at the full 4096-token + budget -- proving the sidecar-preflight-max-tokens fix widens the budget + without weakening the routing probe's fail-closed content check. + + Negative control for the same incident: raising the budget must never be + mistaken for making every response acceptable. A route whose reply is + reasoning-only (present ``reasoning``, empty ``content``) -- the exact + shape ``contextual_orchestrator.orchestrator._response_content`` raises + ``ProviderResponseError`` for -- is simulated at the routing-probe layer + and must still be classified "rejected", never reclassified as a + healthy "ready" route just because the token budget grew. + """ + namespace = _load_launcher() + preflight = namespace["_preflight_review_agents"] + + reasoning_only = SimpleNamespace( + id="nvidia_nim_reasoning_only", provider_name="nvidia_nim", model="reasoning/free" + ) + client = _ProbeClient( + { + reasoning_only.id: { + "choices": [ + {"message": {"content": "", "reasoning": "internal reasoning tokens only"}} + ] + } + } + ) + + with pytest.raises(namespace["ReviewPreflightError"], match="no provider route passed"): + preflight([reasoning_only], client=client) + + assert client.calls[0][2]["max_tokens"] == namespace["REVIEW_MAX_OUTPUT_TOKENS"] + + def test_preflight_fails_closed_when_every_route_rejects() -> None: """A healthy HTTP process is not review-ready without one live LLM route.""" namespace = _load_launcher()