From 8dc334416fda4d3acf0db851fd829155a6f90687 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 10:08:25 +0000 Subject: [PATCH 1/2] fix(sidecar): give the gateway preflight probe a real reasoning budget Exact-evidence root cause (downloaded strix-reports artifact from PR #912's run 33304076516): the routing probe already selects deepseek-ai/deepseek-v4-flash-0731 (nvidia_nim) as 'ready' using the launcher's own REVIEW_MAX_OUTPUT_TOKENS=4096 budget, but this script's separate end-to-end gateway check re-tests the exact same route through 'orchestrator/free' with only max_tokens=16. A reasoning model spends part of that budget on internal reasoning before any answer content, so it returns a 'reasoning' field with no usable content; contextual_orchestrator.orchestrator._response_content correctly raises ProviderResponseError for that, which server.py's generic handler maps to 502 'invalid_structured_output' -- a healthy model gets misreported as completely broken, and every PR's opencode-review/noema-review/strix has been failing at sidecar boot for this reason, not a real review or security finding. Match the working probe's budget so the gateway check doesn't retest with a stricter constraint than the routing decision it is meant to confirm. --- scripts/ci/contextual_orchestrator_review_sidecar.sh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scripts/ci/contextual_orchestrator_review_sidecar.sh b/scripts/ci/contextual_orchestrator_review_sidecar.sh index b044cab82c..dcb1e4e19b 100755 --- a/scripts/ci/contextual_orchestrator_review_sidecar.sh +++ b/scripts/ci/contextual_orchestrator_review_sidecar.sh @@ -380,7 +380,16 @@ 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 the 2026-08-30 sidecar-preflight-max-tokens gap +# baseline entry for the exact-evidence reproduction (strix-reports artifact). +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 \ From 307086967519e29107c6a37dd1b821fbd417dc22 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 10:15:33 +0000 Subject: [PATCH 2/2] test(sidecar): add RED/GREEN parity and no-content negative-control tests Per owner review on PR #1436: source correctness alone doesn't establish operational acceptance without a test-first contract. - test_gateway_preflight_max_tokens_is_synchronized_with_the_routing_probe: asserts the bash script's gateway_preflight_request max_tokens equals REVIEW_MAX_OUTPUT_TOKENS. Verified this fails RED against the pre-fix literal (16 != 4096, confirmed by temporarily swapping in origin/main's sidecar script and re-running just this test) and passes GREEN on the fixed script. - test_reasoning_without_content_remains_rejected_even_with_the_full_budget: negative control proving a genuinely reasoning-only/no-content response still fails the routing probe's fail-closed check at the full 4096-token budget -- the max_tokens fix widens the budget, it does not weaken the content check that must still reject a truly broken route. Also fixes Devin's finding: the sidecar comment cited a gap-baseline entry that did not exist yet. Named it exactly and pointed at the actual repo (contextual-orchestrator, where the strix-reports evidence trail lives) -- landing in a follow-up commit there. --- .../contextual_orchestrator_review_sidecar.sh | 7 +- ...l_orchestrator_review_runtime_preflight.py | 72 +++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/scripts/ci/contextual_orchestrator_review_sidecar.sh b/scripts/ci/contextual_orchestrator_review_sidecar.sh index dcb1e4e19b..28b36e8dd0 100755 --- a/scripts/ci/contextual_orchestrator_review_sidecar.sh +++ b/scripts/ci/contextual_orchestrator_review_sidecar.sh @@ -387,8 +387,11 @@ gateway_virtual_model="orchestrator/${orchestrator_pool}" # 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 the 2026-08-30 sidecar-preflight-max-tokens gap -# baseline entry for the exact-evidence reproduction (strix-reports artifact). +# 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="$( 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()