diff --git a/.gitignore b/.gitignore index 62eaee6..f2ade32 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,6 @@ artifacts/ build/ dist/ *.egg-info/ + +# Graphify analysis artifacts are regenerated per head and are not product sources. +graphify-out/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 9feef31..ba0bb03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format follows Keep a Changelog, and release numbers follow Semantic Version ## [Unreleased] +### Fixed + +- A model answer the provider cut short at its token ceiling (`finish_reason` of `length` or `max_tokens`) now fails as a distinct `NimTruncationError` instead of being parsed as a complete interpretation. A truncated prefix could satisfy a schema whose later fields are optional and reach the customer's report as if it were whole, and the schema-repair loop would re-ask under the same ceiling. An absent or unrecognised `finish_reason` is still treated as unknown, never as truncation. + ### Added - Independent KASI/NAOJ 2026 golden fixtures for all twelve month-changing solar terms, enforcing a two-minute timing budget and five-minute year/month pillar transition checks without network or test-only ephemeris dependencies. diff --git a/src/four_pillars/nim.py b/src/four_pillars/nim.py index e2cec92..c516ee8 100644 --- a/src/four_pillars/nim.py +++ b/src/four_pillars/nim.py @@ -23,6 +23,19 @@ class NimSchemaError(NimError): """Report generated content that cannot satisfy the requested JSON schema.""" +class NimTruncationError(NimError): + """Report an answer the provider cut short when it reached the token ceiling.""" + + +TRUNCATED_FINISH_REASONS = frozenset({"length", "max_tokens"}) +"""Chat-completions ``finish_reason`` values that mean the answer is a prefix. + +``length`` is the OpenAI-compatible spelling and ``max_tokens`` is the variant +some gateways emit. Any other value, including an absent one, means the reason +is unknown rather than truncated, so it is never treated as a failure. +""" + + NimTrace = GenerationTrace """Backward-compatible alias for the provider-neutral generation trace.""" @@ -119,12 +132,20 @@ async def _post(self, payload: dict[str, Any]) -> tuple[dict[str, Any], int]: def _content(self, data: dict[str, Any]) -> str: try: - content = data["choices"][0]["message"]["content"] - except (KeyError, IndexError, TypeError) as exc: + choice = data["choices"][0] + content = choice["message"]["content"] + finish_reason = choice.get("finish_reason") + except (KeyError, IndexError, TypeError, AttributeError) as exc: raise NimError( f"{self._provider_label} response did not contain " "choices[0].message.content" ) from exc + if finish_reason in TRUNCATED_FINISH_REASONS: + raise NimTruncationError( + f"{self._provider_label} stopped at the token ceiling " + f"(finish_reason={finish_reason}), so the answer is incomplete " + "and is not reported as a successful generation" + ) if not isinstance(content, str) or not content.strip(): raise NimError(f"{self._provider_label} returned empty content") return content.strip() diff --git a/tests/test_generation_truncation.py b/tests/test_generation_truncation.py new file mode 100644 index 0000000..978a19f --- /dev/null +++ b/tests/test_generation_truncation.py @@ -0,0 +1,106 @@ +"""Separate a generation the provider cut short from a genuine success. + +A chat-completions gateway reports ``finish_reason`` on each choice. When the +model hits the token ceiling the value is ``length`` and the content is a +prefix of the intended answer. Such a prefix can still parse and validate when +the schema's later fields are optional, so without this check a half-written +interpretation reaches the customer's report as if it were complete. +""" + +from __future__ import annotations + +import httpx +import pytest +from pydantic import BaseModel + +from four_pillars.nim import NimClient, NimError, NimTruncationError +from four_pillars.settings import Settings + + +class Answer(BaseModel): + """Accept a partial payload so truncation cannot be caught by validation alone.""" + + value: str + detail: str = "" + + +def config(**updates: object) -> Settings: + """Build settings for a hosted client that never retries or repairs.""" + data: dict[str, object] = { + "nvidia_nim_api_key": "key", + "nim_base_url": "https://nim.test/v1", + "nim_model": "model", + "nim_max_retries": 0, + "nim_max_schema_repairs": 0, + } + data.update(updates) + return Settings(**data) + + +def _responder(payload: dict[str, object]) -> httpx.MockTransport: + calls: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append(request) + return httpx.Response(200, json=payload) + + transport = httpx.MockTransport(handler) + transport.calls = calls # type: ignore[attr-defined] + return transport + + +@pytest.mark.parametrize("reason", ["length", "max_tokens"]) +@pytest.mark.asyncio +async def test_truncated_choice_is_not_a_success(reason: str) -> None: + """A choice cut short at the token ceiling fails instead of returning a prefix.""" + transport = _responder( + { + "choices": [ + {"message": {"content": '{"value":"ok"}'}, "finish_reason": reason}, + ] + } + ) + + async with NimClient(config(), transport=transport) as client: + with pytest.raises(NimTruncationError, match=reason): + await client.generate(system_prompt="JSON", user_payload={}, response_model=Answer) + + +@pytest.mark.asyncio +async def test_truncation_error_is_a_transport_error_not_a_schema_error() -> None: + """Callers that already handle provider failures keep working unchanged.""" + assert issubclass(NimTruncationError, NimError) + + +@pytest.mark.asyncio +async def test_truncation_is_not_repaired_with_the_same_ceiling() -> None: + """Re-asking under the same limit only truncates again, so it is never retried.""" + transport = _responder( + { + "choices": [ + {"message": {"content": '{"value":"ok"}'}, "finish_reason": "length"}, + ] + } + ) + + async with NimClient(config(nim_max_schema_repairs=3), transport=transport) as client: + with pytest.raises(NimTruncationError): + await client.generate(system_prompt="JSON", user_payload={}, response_model=Answer) + + assert len(transport.calls) == 1 # type: ignore[attr-defined] + + +@pytest.mark.parametrize("reason", ["stop", None]) +@pytest.mark.asyncio +async def test_a_complete_or_unreported_finish_reason_still_succeeds( + reason: str | None, +) -> None: + """An absent marker means unknown, not truncated, so nothing is invented.""" + choice: dict[str, object] = {"message": {"content": '{"value":"ok"}'}} + if reason is not None: + choice["finish_reason"] = reason + + async with NimClient(config(), transport=_responder({"choices": [choice]})) as client: + answer, _ = await client.generate(system_prompt="JSON", user_payload={}, response_model=Answer) + + assert answer.value == "ok"