diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 5ab7e830f3..ff3dac2981 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -1206,6 +1206,51 @@ def _extract_json_object_once(text: str) -> dict[str, Any]: ) from exc +def reject_truncated_completion(raw: str) -> None: + """Fail closed when the provider declares it stopped at its output budget. + + An OpenAI-compatible provider reports ``finish_reason="length"`` when it + terminated generation at the completion-token limit rather than because the + model finished. Such a response is only *sometimes* detectable downstream: + if the cut lands mid-token the JSON is unbalanced and + ``extract_json_object`` fails closed, but if it happens to land after a + syntactically complete object the verdict parses cleanly. Because + ``findings`` is emitted last, the most likely parseable truncation is an + approval carrying an empty or short findings list -- a review that was cut + off mid-thought, indistinguishable from a genuine APPROVE. + + Only the unambiguous ``"length"`` value is rejected. A missing, empty, or + otherwise-valued ``finish_reason`` is left alone, so a provider that + reports a vocabulary this gate does not model cannot be failed spuriously. + + Args: + raw: The decoded HTTP response body of a chat-completion request. + + Raises: + NoemaModelOutputError: If the first choice declares + ``finish_reason="length"``. The message embeds no part of the + untrusted body, so it stays safe in the public + ``pull_request_target`` job log. + """ + try: + data = json.loads(raw) + except json.JSONDecodeError: + return + if not isinstance(data, dict): + return + choices = data.get("choices") + if not isinstance(choices, list) or not choices: + return + first_choice = choices[0] + if not isinstance(first_choice, dict): + return + if first_choice.get("finish_reason") == "length": + raise NoemaModelOutputError( + "Noema LLM completion stopped at the provider output-token budget " + "(finish_reason=length); the verdict is truncated and cannot be trusted" + ) + + def extract_llm_message_content(raw: str) -> str: """Parse and validate the OpenAI-compatible chat-completion HTTP envelope. @@ -1593,6 +1638,7 @@ def call_llm( active_phase = "decoding" raw = decode_llm_response_body(raw_bytes) served_model = _extract_served_model(raw) + reject_truncated_completion(raw) content = extract_llm_message_content(raw) verdict = extract_json_object(content) active_phase = "validating" diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 5fa23dec53..5ebea5e573 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -2643,3 +2643,79 @@ def test_parse_args_and_main(monkeypatch): noema.main( ["--repo", "owner/repo", "--pr-number", "9", "--expected-head", "A" * 40] ) + + +def test_reject_truncated_completion_rejects_provider_declared_length_stop(): + """A finish_reason=length completion is refused even when its JSON parses. + + Verified against main before the guard existed: a budget-terminated + response whose cut lands after a syntactically complete object parses as a + clean APPROVE. Because ``findings`` is emitted last, the likeliest + parseable truncation is an approval with an empty findings list, which is + indistinguishable from a genuine one. + """ + parseable_but_truncated = json.dumps( + { + "choices": [ + { + "finish_reason": "length", + "message": { + "content": '{"decision":"approve","summary":"reviewed","findings":[]}' + }, + } + ] + } + ) + # The payload really would have parsed: that is what makes the guard necessary. + assert noema.extract_json_object( + noema.extract_llm_message_content(parseable_but_truncated) + ) == {"decision": "approve", "summary": "reviewed", "findings": []} + + with pytest.raises(noema.NoemaModelOutputError, match="finish_reason=length"): + noema.reject_truncated_completion(parseable_but_truncated) + + +@pytest.mark.parametrize( + "finish_reason", + ["stop", "", None, "tool_calls", "end_turn", "content_filter"], +) +def test_reject_truncated_completion_allows_every_other_finish_reason(finish_reason): + """Only the unambiguous length signal is rejected. + + A provider reporting a vocabulary this gate does not model must not be + failed spuriously, so anything other than ``"length"`` passes through. + """ + envelope = json.dumps( + {"choices": [{"finish_reason": finish_reason, "message": {"content": "{}"}}]} + ) + assert noema.reject_truncated_completion(envelope) is None + + +@pytest.mark.parametrize( + "raw", + [ + "not json at all", + json.dumps([]), + json.dumps({}), + json.dumps({"choices": "not-a-list"}), + json.dumps({"choices": []}), + json.dumps({"choices": ["not-an-object"]}), + json.dumps({"choices": [{}]}), + ], +) +def test_reject_truncated_completion_defers_shape_errors_to_the_content_parser(raw): + """This guard answers one question only and never raises a shape error. + + Malformed envelopes are already rejected with precise messages by + ``extract_llm_message_content``; duplicating that here would report the + wrong cause for a body this function cannot classify. + """ + assert noema.reject_truncated_completion(raw) is None + + +def test_call_llm_consults_the_truncation_guard_before_parsing_content(): + """The guard must run on the decoded body, ahead of content extraction.""" + source = Path("scripts/ci/noema_review_gate.py").read_text(encoding="utf-8") + guard = source.index(" reject_truncated_completion(raw)\n") + extract = source.index(" content = extract_llm_message_content(raw)\n") + assert guard < extract