fix(noema): ground verdicts and classify gateway errors

scripts/ci/noema_review_gate.py

@@ -58,7 +58,10 @@
 MAX_FILE_CONTEXT_CHARS = 4000
 MAX_REVIEW_CONTEXT_CHARS = 24000
 MAX_THREAD_BODY_CHARS = 1200
+MAX_ALLOWED_LOCATIONS_JSON_BYTES = 32 * 1024
+MAX_HTTP_ERROR_BODY_BYTES = 16 * 1024
 DIFF_HUNK_RE = re.compile(r"^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@")
+SAFE_MODEL_IDENTIFIER_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:/@+-]{0,199}$")
 
 ORCHESTRATOR_LOOPBACK_HOSTS = frozenset({"127.0.0.1", "::1"})
 ORCHESTRATOR_BASE_ENV = "CONTEXTUAL_ORCHESTRATOR_BASE_URL"
@@ -1303,14 +1306,71 @@ def _extract_served_model(raw: str) -> str | None:
         return None
     if not isinstance(data, dict):
         return None
-    served = data.get("model")
-    if not isinstance(served, str) or not served.strip():
+    return _safe_model_identifier(data.get("model"))
+
+
+def _safe_model_identifier(value: Any) -> str | None:
+    """Accept only a conservative, bounded model identifier safe for public logs."""
+    if not isinstance(value, str):
+        return None
+    candidate = value.strip()
+    if not SAFE_MODEL_IDENTIFIER_RE.fullmatch(candidate):
+        return None
+    return candidate
+
+
+def _extract_http_error_served_model(exc: urllib.error.HTTPError) -> str | None:
+    """Read a bounded gateway error envelope and return only its safe model id.
+
+    The response body is never returned or logged. Only the canonical
+    ``error.detail.model`` field is allowed; malformed, oversized, or unexpected
+    envelopes fail closed to an unknown model.
+    """
+    try:
+        raw_bytes = exc.read(MAX_HTTP_ERROR_BODY_BYTES + 1)
+    except (AttributeError, OSError, ValueError):
+        return None
+    if len(raw_bytes) > MAX_HTTP_ERROR_BODY_BYTES:
+        return None
+    try:
+        payload = json.loads(raw_bytes.decode("utf-8"))
+    except (UnicodeDecodeError, json.JSONDecodeError, TypeError, ValueError):
+        return None
+    if not isinstance(payload, dict):
+        return None
+    error = payload.get("error")
+    if not isinstance(error, dict):
         return None
-    scrubbed = scrub_sensitive_data(served.strip()) or ""
-    printable = scrubbed.encode("utf-8", errors="backslashreplace").decode("utf-8")
-    printable = "".join(" " if ord(char) < 32 or ord(char) == 127 else char for char in printable)
-    printable = " ".join(printable.split())
-    return printable[:200] or None
+    detail = error.get("detail")
+    if not isinstance(detail, dict):
+        return None
+    return _safe_model_identifier(detail.get("model"))
+
+
+def _bounded_allowed_locations_json(allowed_locations: Sequence[dict[str, Any]]) -> str:
+    """Serialize the largest location prefix that fits the prompt byte budget."""
+    total_count = len(allowed_locations)
+
+    def render(count: int) -> str:
+        return json.dumps(
+            {
+                "total_count": total_count,
+                "truncated": count < total_count,
+                "locations": list(allowed_locations[:count]),
+            },
+            ensure_ascii=False,
+            separators=(",", ":"),
+        )
+
+    low = 0
+    high = total_count
+    while low < high:
+        midpoint = (low + high + 1) // 2
+        if len(render(midpoint).encode("utf-8")) <= MAX_ALLOWED_LOCATIONS_JSON_BYTES:
+            low = midpoint
+        else:
+            high = midpoint - 1
+    return render(low)
 
 
 def _truthy_env(name: str) -> bool:
@@ -1434,6 +1494,7 @@ def call_llm(
     location_example = allowed_locations[0] if allowed_locations else {
         "path": "path", "line": 0, "side": "RIGHT"
     }
+    allowed_locations_json = _bounded_allowed_locations_json(allowed_locations)
     prompt = {
         "role": "user",
         "content": "\n".join(
@@ -1442,6 +1503,9 @@ def call_llm(
                 "Review the PR diff plus the additional changed-file and review-thread context for correctness, security, maintainability, and behavioral regressions.",
                 "Return only JSON with the declared response_format schema.",
                 "Every formal verdict must cite exact changed-side lines. APPROVE requires falsifying concrete regression hypotheses; source or test changes require at least two distinct probes and other changes require at least one. REQUEST_CHANGES requires a confirmed probe at a finding location.",
+                "Use only path, line, and side tuples listed in the bounded allowed-locations JSON below. If it is truncated, omit a formal verdict for any location not listed instead of guessing.",
+                f"Allowed changed-side locations: {allowed_locations_json}",
+                f"Location shape example: {json.dumps(location_example, separators=(',', ':'))}",
                 "Use request_changes only for blocking, concrete issues. A generic no-issues statement is not review evidence.",
                 f"Repository: {repo}",
                 f"PR: #{number}",
@@ -1525,6 +1589,9 @@ def call_llm(
             )
         validate_substantive_verdict(verdict, diff, changed_paths)
     except (RuntimeError, urllib.error.URLError, http.client.HTTPException, OSError) as exc:
+        if isinstance(exc, urllib.error.HTTPError):
+            active_phase = "response_error"
+            served_model = _extract_http_error_served_model(exc)
         elapsed = time.monotonic() - attempt_started
         current_failure = _stable_failure_diagnostic(exc)
         model_note = served_model or "unknown"

tests/test_noema_review_gate.py

@@ -1,6 +1,7 @@
 import base64
 import hashlib
 import http.client
+import io
 import json
 import os
 import shlex
@@ -1457,6 +1458,142 @@ def fake_getaddrinfo_invalid_ip(host, port, *args, **kwargs):
     assert noema.call_llm("owner/repo", 1, pr, "diff", True, "head")["decision"] == "approve"
 
 
+def test_call_llm_prompts_with_bounded_exact_changed_locations(monkeypatch):
+    """The model receives the same exact-line contract enforced after inference."""
+    monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat")
+    monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret")
+    monkeypatch.setattr(noema, "validate_substantive_verdict", lambda *_args: None)
+    captured = {}
+    verdict = {"decision": "approve", "summary": "checked", "findings": []}
+
+    class Response:
+        def __enter__(self):
+            return self
+
+        def __exit__(self, *args):
+            return False
+
+        def read(self):
+            return json.dumps(
+                {"choices": [{"message": {"content": json.dumps(verdict)}}]}
+            ).encode()
+
+    class Opener:
+        def open(self, request):
+            captured.update(json.loads(request.data.decode()))
+            return Response()
+
+    diff = """diff --git a/tool.py b/tool.py
+--- a/tool.py
++++ b/tool.py
+@@ -292,2 +295,2 @@
+-old = True
++new = True
+"""
+    monkeypatch.setattr(noema.urllib.request, "build_opener", lambda *_args: Opener())
+
+    noema.call_llm("owner/repo", 1, make_pr(), diff, False, "head")
+
+    prompt = captured["messages"][1]["content"]
+    marker = "Allowed changed-side locations: "
+    locations_line = next(line for line in prompt.splitlines() if line.startswith(marker))
+    envelope = json.loads(locations_line.removeprefix(marker))
+    assert len(locations_line.removeprefix(marker).encode()) <= noema.MAX_ALLOWED_LOCATIONS_JSON_BYTES
+    assert envelope == {
+        "total_count": 2,
+        "truncated": False,
+        "locations": [
+            {"path": "tool.py", "line": 292, "side": "LEFT"},
+            {"path": "tool.py", "line": 295, "side": "RIGHT"},
+        ],
+    }
+    assert '"line":293' not in locations_line
+
+
+def test_allowed_locations_json_truncates_at_the_byte_budget():
+    """Large changed-line sets remain valid JSON within the prompt budget."""
+    locations = [
+        {"path": f"src/{index:05d}-{'가' * 80}.py", "line": index + 1, "side": "RIGHT"}
+        for index in range(1000)
+    ]
+
+    rendered = noema._bounded_allowed_locations_json(locations)
+    envelope = json.loads(rendered)
+
+    assert len(rendered.encode("utf-8")) <= noema.MAX_ALLOWED_LOCATIONS_JSON_BYTES
+    assert envelope["total_count"] == len(locations)
+    assert envelope["truncated"] is True
+    assert 0 < len(envelope["locations"]) < len(locations)
+
+
+def test_call_llm_reports_only_safe_model_from_bounded_http_error(monkeypatch, capsys):
+    """A gateway HTTP error exposes only its canonical safe model identifier."""
+    monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat")
+    monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret")
+    secret = "never-print-this-error-detail"
+    body = json.dumps(
+        {
+            "error": {
+                "detail": {"model": "github_models/deepseek-v3", "secret": secret},
+                "message": secret,
+            },
+            "arbitrary": secret,
+        }
+    ).encode()
+
+    class Opener:
+        def open(self, request):
+            raise noema.urllib.error.HTTPError(
+                request.full_url, 502, "Bad Gateway", {}, io.BytesIO(body)
+            )
+
+    monkeypatch.setattr(noema.urllib.request, "build_opener", lambda *_args: Opener())
+
+    with pytest.raises(noema.NoemaTransportError) as exc_info:
+        noema.call_llm("owner/repo", 1, make_pr(), "diff", False, "head")
+
+    output = capsys.readouterr().out
+    diagnostic = str(exc_info.value)
+    assert "phase=response_error" in output
+    assert "served_model=github_models/deepseek-v3" in output
+    assert "phase=response_error" in diagnostic
+    assert "served_model=github_models/deepseek-v3" in diagnostic
+    assert secret not in output
+    assert secret not in diagnostic
+
+
+@pytest.mark.parametrize(
+    "body",
+    [
+        b"not-json",
+        b'{"error":{"detail":{"model":"unsafe model value"}}}',
+        b"x" * (noema.MAX_HTTP_ERROR_BODY_BYTES + 1),
+    ],
+)
+def test_call_llm_http_error_malformed_or_oversized_model_is_unknown(
+    monkeypatch, capsys, body
+):
+    """Malformed, unsafe, and oversized HTTP error bodies fail closed."""
+    monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat")
+    monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret")
+
+    class Opener:
+        def open(self, request):
+            raise noema.urllib.error.HTTPError(
+                request.full_url, 502, "Bad Gateway", {}, io.BytesIO(body)
+            )
+
+    monkeypatch.setattr(noema.urllib.request, "build_opener", lambda *_args: Opener())
+
+    with pytest.raises(noema.NoemaTransportError, match="served_model=unknown"):
+        noema.call_llm("owner/repo", 1, make_pr(), "diff", False, "head")
+
+    output = capsys.readouterr().out
+    assert "phase=response_error" in output
+    assert "served_model=unknown" in output
+    assert body.decode("utf-8", errors="ignore") not in output
+
+
 def test_noema_redirect_handler_rejects_redirects():
     """Noema must not follow redirects after validating the initial URL."""
     handler = noema.NoRedirectHandler()