Review adapter falls back when a provider rejects response_format

.asdd.example.yml

@@ -117,6 +117,9 @@ agents: []
 # the cap (only sensible on a private repo).
 review:
   max_diff_lines: 2000
+  # Per-call model timeout (seconds). A slow reasoning reviewer times out around 60s on a real diff; the
+  # default fails fast below that with an actionable message rather than hanging. Re-tune per provider.
+  timeout_seconds: 45
 
 # Spec tool: which validator decides a change's spec is READY (the operate agents' readiness gate).
 #   builtin  -> ASDD's definition-of-ready (cli/spec-check.py): outcomes/scope/constraints/verification

.github/asdd/run-review.sh

@@ -69,6 +69,12 @@ fi
 # enormous PR drains the budget. Above the cap we refuse the model and say so, rather than pay.
 # The deterministic security layer below still runs, so the gate keeps biting either way.
 CAP="$(yaml_scalar_in max_diff_lines)"; CAP="${CAP:-2000}"
+# Per-call model timeout (seconds). review.timeout_seconds in .asdd.yml maps to ASDD_MODEL_TIMEOUT, which
+# the OpenAI-compatible adapter enforces, so a slow reasoning reviewer fails fast with a named cause
+# instead of hanging. Unset here leaves the adapter default. A caller's env ASDD_MODEL_TIMEOUT still wins.
+if [ -z "${ASDD_MODEL_TIMEOUT:-}" ]; then
+  _to="$(yaml_scalar_in timeout_seconds)"; [ -n "$_to" ] && export ASDD_MODEL_TIMEOUT="$_to"
+fi
 DIFF_LINES=$(wc -l < "$WORKDIR/changes.diff" 2>/dev/null | tr -d ' '); DIFF_LINES="${DIFF_LINES:-0}"
 too_big=false
 if [ "$CAP" -gt 0 ] 2>/dev/null && [ "$DIFF_LINES" -gt "$CAP" ]; then too_big=true; fi

.github/asdd/runtime/openai-compat.sh

@@ -62,23 +62,43 @@ extract_json() {
   return 1
 }
 
-payload="$(jq -n --arg m "$ASDD_MODEL" --arg s "$sys" --arg p "$prompt" \
-  '{model:$m, temperature:0,
-    messages:[{role:"system", content:$s},{role:"user", content:$p}],
-    response_format:{type:"json_object"}}')"
+# Request body, with a chooseable response_format. Some OpenAI-compatible providers (seen with a GLM
+# reasoning model on Runware) reject response_format with a 5xx, which would kill the review even though
+# the system prompt already demands JSON only and the extractor tolerates prose. So the first attempt asks
+# for a json_object; if it fails, the remaining attempts drop response_format and lean on the prompt plus
+# the extractor. Every mainstream provider still gets response_format on the first, cheapest try.
+build_payload() {  # $1: 1 = with response_format, 0 = without
+  if [ "$1" = 1 ]; then
+    jq -n --arg m "$ASDD_MODEL" --arg s "$sys" --arg p "$prompt" \
+      '{model:$m, temperature:0, messages:[{role:"system",content:$s},{role:"user",content:$p}],
+        response_format:{type:"json_object"}}'
+  else
+    jq -n --arg m "$ASDD_MODEL" --arg s "$sys" --arg p "$prompt" \
+      '{model:$m, temperature:0, messages:[{role:"system",content:$s},{role:"user",content:$p}]}'
+  fi
+}
 
 attempts="${ASDD_MODEL_RETRIES:-3}"
 case "$attempts" in ''|*[!0-9]*) attempts=3 ;; esac
 [ "$attempts" -ge 1 ] || attempts=1
 nap="${ASDD_MODEL_RETRY_SLEEP:-1}"
 
-result=""
+# Per-call timeout. Default 45s: above a fast reviewer's full-review time (~24s measured) and below a heavy
+# reasoning model's server-side hang (a reasoning reviewer times out around 60s on a real diff), so a slow
+# reasoning reviewer fails fast with a named cause instead of burning the full server timeout on every
+# retry. Configurable via ASDD_MODEL_TIMEOUT (run-review.sh maps review.timeout_seconds to it); re-tune per
+# provider.
+timeout_s="${ASDD_MODEL_TIMEOUT:-45}"
+case "$timeout_s" in ''|*[!0-9]*) timeout_s=45 ;; esac
+
+result=""; with_rf=1; timed_out=0
 for i in $(seq 1 "$attempts"); do
-  http=""
-  resp="$(curl -sS --max-time 180 -w '\n%{http_code}' -X POST "$endpoint" \
+  http=""; curl_rc=0
+  resp="$(curl -sS --max-time "$timeout_s" -w '\n%{http_code}' -X POST "$endpoint" \
     -H "Authorization: Bearer $ASDD_RUNTIME_TOKEN" \
     -H "Content-Type: application/json" \
-    -d "$payload" || true)"
+    -d "$(build_payload "$with_rf")")" || curl_rc=$?
+  [ "$curl_rc" = 28 ] && timed_out=1              # 28 = curl's own timeout exit code
   http="${resp##*$'\n'}"; resp="${resp%$'\n'*}"   # split the trailing status line off the body
   # Pull the answer across the shapes providers actually use: content as a string, content as an array of
   # parts (concatenate .text), or a legacy .text field; THEN append message.reasoning_content. A reasoning
@@ -95,7 +115,13 @@ for i in $(seq 1 "$attempts"); do
   if cand="$(extract_json "$content")"; then
     result="$cand"; break
   fi
-  if [ "$i" -lt "$attempts" ]; then
+  # This attempt failed. If response_format was still on, drop it for the remaining attempts: a provider
+  # that rejects it (a 5xx or an error body) then gets a clean try the prompt and extractor can satisfy.
+  if [ "$with_rf" = 1 ]; then
+    with_rf=0
+    echo "openai-compat: attempt $i failed (HTTP ${http:-n/a}); retrying without response_format" >&2
+    sleep "$nap" 2>/dev/null || true
+  elif [ "$i" -lt "$attempts" ]; then
     echo "openai-compat: attempt $i returned unparseable output; retrying" >&2
     sleep "$nap" 2>/dev/null || true
   fi
@@ -115,7 +141,12 @@ if [ -z "$result" ]; then
     echo "  last HTTP status: ${http:-unknown}; response bytes: ${rlen}; extracted-content chars: ${clen}"
     echo "  content head: ${head_snip}"
     echo "  content tail: ${tail_snip}"
+    # The raw response body on an error names the cause (an unsupported parameter, a bad model name, a
+    # quota message). It holds no key (the key is only in the request header) and reviews already-public
+    # PR content, so a truncated echo is key-safe and turns a blind failure into a named one.
+    [ "$clen" -eq 0 ] && echo "  response body (truncated): $(printf '%s' "$resp" | tr '\n' ' ' | cut -c1-240)"
     [ "$clen" -eq 0 ] && echo "  (content was empty across message.content, content parts, .text, and reasoning_content: check the endpoint returns OpenAI-shaped choices, the model name, and quota.)"
+    [ "$timed_out" = 1 ] && echo "  the model did not respond within ${timeout_s}s; a heavy reasoning model can be too slow to review a real diff (see 'asdd doctor'). Use a faster reviewer, or raise ASDD_MODEL_TIMEOUT / review.timeout_seconds if the model is legitimately slow."
   } >&2
 fi
 printf '%s' "$result"

.github/asdd/runtime/openai-compat.test.sh

@@ -26,6 +26,12 @@ case "${MODE:-url}" in
   flaky)  # garbage on the first call, a valid object afterwards (proves retry)
           n=$(cat "$COUNTER" 2>/dev/null || echo 0); n=$((n+1)); echo "$n" > "$COUNTER"
           if [ "$n" -le 1 ]; then envelope 'nope'; else envelope '{"schema":"asdd/review/v0.1","lenses":[]}'; fi ;;
+  timeout) exit 28 ;;                                                  # simulate curl's --max-time timeout
+  rf)     # a provider that rejects response_format: the call carrying it yields no usable content, the
+          # retry without it succeeds (proves the response_format fallback)
+          has_rf=0; for a in "$@"; do case "$a" in *response_format*) has_rf=1;; esac; done
+          if [ "$has_rf" = 1 ]; then printf '%s\n500' "$(envelope 'provider rejected response_format')"
+          else printf '%s\n200' "$(envelope '{"schema":"asdd/review/v0.1","lenses":[]}')"; fi ;;
 esac
 EOF
 chmod +x "$STUB/curl"
@@ -68,4 +74,17 @@ got="$(MODE=bad ASDD_MODEL_RETRIES=2 PATH="$STUB:$PATH" ASDD_RUNTIME_TOKEN=x ASD
       ASDD_MODEL_RETRY_SLEEP=0 ASDD_MODEL_URL='https://x.test/v1/chat/completions' bash "$SCRIPT" </dev/null 2>/dev/null)"
 [ -z "$got" ] || fail "all-bad responses should yield empty output, got '$got'"
 
+# 6. response_format fallback: a provider that 500s on response_format still gets a review, because the
+#    retry drops the parameter and the prompt + extractor carry it.
+got="$(MODE=rf ASDD_MODEL_RETRIES=3 PATH="$STUB:$PATH" ASDD_RUNTIME_TOKEN=x ASDD_MODEL=m \
+      ASDD_MODEL_RETRY_SLEEP=0 ASDD_MODEL_URL='https://x.test/v1/chat/completions' bash "$SCRIPT" </dev/null 2>"$STUB/err")"
+[ "$(printf '%s' "$got" | jq -r .schema)" = "asdd/review/v0.1" ] || fail "response_format fallback did not recover a review (got '$got')"
+grep -q 'retrying without response_format' "$STUB/err" || fail "fallback did not drop response_format on the retry"
+
+# 7. per-call timeout (curl exit 28): empty output + a named, actionable timeout message (not a blind fail).
+got="$(MODE=timeout ASDD_MODEL_RETRIES=2 ASDD_MODEL_TIMEOUT=1 PATH="$STUB:$PATH" ASDD_RUNTIME_TOKEN=x ASDD_MODEL=m \
+      ASDD_MODEL_RETRY_SLEEP=0 ASDD_MODEL_URL='https://x.test/v1/chat/completions' bash "$SCRIPT" </dev/null 2>"$STUB/err")"
+[ -z "$got" ] || fail "a timeout should yield empty output, got '$got'"
+grep -q 'did not respond within' "$STUB/err" || fail "timeout did not produce the actionable message"
+
 echo "openai-compat.test.sh: PASS"

CHANGELOG.md

@@ -6,6 +6,16 @@ draft, so pin a conformance claim to a commit or date.
 
 ## [Unreleased]
 
+### Added
+- **`doctor` and `setup` warn when the reviewer is a heavy reasoning model.** A reasoning model reasons at
+  length and can exceed a hosted inference window on a real code diff, so the review times out and posts no
+  lenses while trivial or docs-only diffs still pass and look fine. The preflight and the setup wizard now
+  flag a reasoning reviewer (a property of the model, not the host) and point to a faster one.
+- **A configurable per-call review timeout that fails fast.** The model call now has a default 45s timeout
+  (`review.timeout_seconds` / `ASDD_MODEL_TIMEOUT`), above a fast reviewer's real-diff time and below a
+  reasoning model's server-side hang, so a slow reviewer fails fast with an actionable message naming the
+  cause instead of burning the full server timeout on every retry.
+
 ### Fixed
 - **Review runtime recovers the model's JSON.** A reasoning model wraps its review object in analysis
   prose (with its own braces), code fences, or trailing commentary, or emits it in a separate
@@ -28,6 +38,12 @@ draft, so pin a conformance claim to a commit or date.
   "update this branch with main" (or the GitHub "Update branch" button) added an unsigned merge commit
   that failed DCO with no clean fix, because a merge commit cannot be signed without rewriting history.
   The commit list is now built with `--no-merges`, matching the DCO convention.
+- **The review adapter falls back when a provider rejects `response_format`.** Some OpenAI-compatible
+  providers (seen with a GLM reasoning model on Runware) return HTTP 500 on `response_format:
+  {type: json_object}`, so the model lenses produced no output and the review failed closed even though
+  the connection was fine. The adapter now asks for `response_format` on the first attempt and drops it on
+  retry, leaning on the system prompt and the extractor, and logs the raw error body on a persistent
+  failure so the cause is named.
 - **Model calls fall back to `max_completion_tokens`.** A newer OpenAI reasoning model rejects `max_tokens`
   with a 400 asking for `max_completion_tokens`, so `connect-check` reported it dead and the developer
   council could not call it. Both now send `max_tokens` first and retry once with the renamed parameter on

cli/doctor.py

@@ -200,6 +200,48 @@ def check_recipes(rep, config):
                 "re-run `asdd init --goose` to scaffold them")
 
 
+# Model-name fragments for families that reason at length. A reasoning reviewer can exceed a hosted
+# inference window on a substantive diff (observed live: a GLM reviewer returns HTTP 500 "inference timed
+# out" on a real code diff, while trivial or docs-only diffs pass and look fine), so the review silently
+# produces no lenses. Heuristic name match, WARN not FAIL, and it is a property of the MODEL, not the host.
+_REASONING_HINTS = ("glm", "deepseek-r1", "deepseek_r1", "deepseek-reasoner", "reasoner", "qwq",
+                    "-thinking", "thinking-", "magistral", "o1-", "o1@", "o3-", "o3@", "o4-", "minimax-m1")
+
+
+def _model_of(config, role):
+    """The model for a roster role (models.<role>), read without a YAML dependency (kit text-scan)."""
+    inblk = False
+    try:
+        with open(config) as fh:
+            for line in fh:
+                s = line.split("#", 1)[0].rstrip()
+                if s.strip() == "models:":
+                    inblk = True
+                    continue
+                if inblk:
+                    if s and not s[0].isspace():
+                        break
+                    t = s.strip()
+                    if t.startswith(f"{role}:"):
+                        return t[len(role) + 1:].strip().strip('"').strip("'")
+    except OSError:
+        return None
+    return None
+
+
+def check_reviewer_reasoning(rep, config):
+    """WARN when the reviewer is a heavy reasoning model, which times out reviewing a real diff."""
+    model = _model_of(config, "reviewer")
+    if not model:
+        return
+    if any(h in model.lower() for h in _REASONING_HINTS):
+        rep.add(WARN, f"the reviewer looks like a reasoning model ({model})",
+                "a reasoning model reasons at length and can exceed a hosted inference window on a real "
+                "code diff, so the review times out and posts no lenses while trivial diffs still pass",
+                "set models.reviewer to a faster non-reasoning model, or split the review per lens; the "
+                "per-call timeout will otherwise fail fast and name this cause")
+
+
 def main():
     ap = argparse.ArgumentParser(description="Preflight the ASDD Goose operate path.")
     ap.add_argument("config", nargs="?", default=".asdd.yml",
@@ -218,6 +260,7 @@ def main():
     check_goose(rep)
     check_spec_tool(rep, a.config)
     check_roster(rep, a.config)
+    check_reviewer_reasoning(rep, a.config)
     check_conventions(rep, a.config)
     check_runtime_key(rep)
     check_recipes(rep, a.config)

cli/doctor.test.sh

@@ -55,4 +55,14 @@ out="$(python3 "$DOC" "$TMP/nope.yml" 2>&1)"; rc=$?
 [ "$rc" = "1" ] && grep -q 'config not found' <<<"$out" \
   && ok "missing config fails cleanly (exit 1)" || bad "missing-config case (exit $rc)"
 
+# A reasoning-model reviewer must WARN (it times out on a real diff) but stay READY; a fast reviewer must not.
+printf 'lanes:\n  - feature\nmodels:\n  developer: ""\n  reviewer: "zai:glm@5.2"\n  test_author: "moonshotai:kimi@k2.6"\n  test_runner: "moonshotai:kimi@k2.6"\n' > "$TMP/glm.yml"
+out="$(python3 "$DOC" "$TMP/glm.yml" 2>&1)"
+grep -qi 'reasoning model' <<<"$out" && grep -q 'RESULT: READY' <<<"$out" \
+  && ok "reasoning reviewer WARNs but stays READY" || bad "reasoning-reviewer WARN missing"
+printf 'lanes:\n  - feature\nmodels:\n  developer: ""\n  reviewer: "openai:gpt-oss-120b"\n  test_author: "moonshotai:kimi@k2.6"\n  test_runner: "moonshotai:kimi@k2.6"\n' > "$TMP/fast.yml"
+out="$(python3 "$DOC" "$TMP/fast.yml" 2>&1)"
+grep -qi 'reasoning model' <<<"$out" && bad "fast reviewer wrongly warned" \
+  || ok "fast reviewer does not trigger the reasoning warn"
+
 [ "$fail" = "0" ] && { echo "doctor self-test: PASS"; exit 0; } || { echo "doctor self-test: FAIL"; exit 1; }

cli/setup-goose.py

@@ -374,6 +374,17 @@ def main():
 
     print_steps(roles, recipe_dir)
 
+    # WARN if the reviewer got a heavy reasoning model: it can exceed a hosted inference window on a real
+    # code diff and time out with no lenses, while trivial diffs pass. Canonical check: cli/doctor.py
+    # check_reviewer_reasoning. Advisory; setup still succeeds.
+    reasoning_hints = ("glm", "deepseek-r1", "deepseek_r1", "deepseek-reasoner", "reasoner", "qwq",
+                       "-thinking", "thinking-", "magistral", "o1-", "o1@", "o3-", "o3@", "o4-", "minimax-m1")
+    rev = next((r["value"] for r in roles if r["key"] == "reviewer"), "")
+    if rev and any(h in rev.lower() for h in reasoning_hints):
+        print(f"\nWARNING: models.reviewer ({rev}) looks like a reasoning model. A reasoning model can "
+              "exceed a hosted inference window on a real code diff and time out with no lenses, while "
+              "trivial diffs still pass. Prefer a faster non-reasoning reviewer.")
+
     rc = 0
     if not no_validate:
         code, out = run_check(config, strict=changed)