From 87ab09fee3b379fec01031d4ceb188c29b824e56 Mon Sep 17 00:00:00 2001 From: Sergi Torres Albert Date: Tue, 28 Jul 2026 23:12:20 +0200 Subject: [PATCH 1/3] =?UTF-8?q?fix(ml,back):=20enforce=20the=20output=20to?= =?UTF-8?q?ken=20cap=20=E2=80=94=20max=5Fnew=5Ftokens=20is=20not=20a=20cha?= =?UTF-8?q?t=20param=20(#106)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_GENERATION_PARAMS` declared `max_new_tokens: 512`, but the call path is `ModelInference.chat()`, whose schema (`TextChatParameters`) has no such field. Unknown keys are dropped in silence, so the cap was never in force and the service default of 1024 applied to both columns of the A/B. Measured against real Watsonx (eu-de, llama-3-3-70b) on the prompt "Write a detailed 800-word essay about the history of the printing press": max_new_tokens=16 -> TimeoutError on all 4 attempts (8s hard timeout) max_tokens=16 -> 15 words in 1.2s So the consequence was not only cost and latency: any prompt inviting a long answer exhausted the retries and surfaced as a 503 from `/api/generate`. Adds `backend/tests/test_generation_params.py` with a positive control — every key we send is checked against the dataclass fields of `TextChatParameters`, which is the check that would have caught this when it was introduced — plus a credential-gated live test that asks for far more than the cap allows and asserts the answer comes back cut short. Co-Authored-By: Claude Opus 5 --- ai_pipeline/autoria_ai/generator.py | 6 +- backend/tests/test_generation_params.py | 93 +++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 backend/tests/test_generation_params.py diff --git a/ai_pipeline/autoria_ai/generator.py b/ai_pipeline/autoria_ai/generator.py index 0aebe65..6b70355 100644 --- a/ai_pipeline/autoria_ai/generator.py +++ b/ai_pipeline/autoria_ai/generator.py @@ -38,8 +38,12 @@ WATSONX_MODEL_ID: str = "meta-llama/llama-3-3-70b-instruct" +# Keys must match the Watsonx chat schema (``TextChatParameters``) exactly: +# an unknown key is dropped in silence, not rejected. ``max_new_tokens`` is the +# *text-generation* spelling and has no effect on ``ModelInference.chat()``, +# which is what we call — see backend/tests/test_generation_params.py. _GENERATION_PARAMS: dict[str, Any] = { - "max_new_tokens": 512, + "max_tokens": 512, "temperature": 0.7, "top_p": 0.9, } diff --git a/backend/tests/test_generation_params.py b/backend/tests/test_generation_params.py new file mode 100644 index 0000000..28beebc --- /dev/null +++ b/backend/tests/test_generation_params.py @@ -0,0 +1,93 @@ +"""Contract tests for the generation parameters sent to Watsonx. + +``ModelInference.chat()`` silently drops any key it does not recognise: an +unknown parameter raises nothing and changes nothing, so a misspelt cap is +indistinguishable from an enforced one until an output actually reaches it. +That is how ``max_new_tokens`` survived in ``_GENERATION_PARAMS`` while the +real cap stayed at the service default of 1024 (issue #106). + +Two tests, one static and one live: + +- ``test_generation_params_match_chat_schema`` is the positive control. It + fails on any key the chat schema does not declare, which is exactly the + check that would have caught #106 at the time it was introduced. +- ``test_max_tokens_cap_is_enforced_live`` proves the cap has an effect + against the real service, by asking for a long answer under a tiny cap. +""" + +from __future__ import annotations + +import dataclasses +import os +from pathlib import Path + +import pytest +from ibm_watsonx_ai.foundation_models.schema import TextChatParameters + +from app.services.watsonx_client import generate +from autoria_ai.generator import _GENERATION_PARAMS + +_REPO_ROOT = Path(__file__).resolve().parents[2] +_ENV_FILE = _REPO_ROOT / ".env" + + +def _load_dotenv_file(path: Path) -> None: + """Load KEY=VALUE lines into os.environ without overriding existing values.""" + if not path.is_file(): + return + for raw in path.read_text(encoding="utf-8").splitlines(): + line = raw.strip() + if not line or line.startswith("#") or "=" not in line: + continue + key, _, value = line.partition("=") + os.environ.setdefault(key.strip(), value.strip().strip('"').strip("'")) + + +def _watsonx_creds_present() -> bool: + _load_dotenv_file(_ENV_FILE) + return bool(os.getenv("WATSONX_API_KEY") and os.getenv("WATSONX_PROJECT_ID")) + + +def _chat_schema_fields() -> set[str]: + return {f.name for f in dataclasses.fields(TextChatParameters)} + + +def test_generation_params_match_chat_schema(): + """Every key we send must exist in the chat schema, or it is discarded.""" + unknown = set(_GENERATION_PARAMS) - _chat_schema_fields() + assert not unknown, ( + f"{sorted(unknown)} are not fields of TextChatParameters and will be " + f"ignored in silence by ModelInference.chat(). " + f"Valid fields: {sorted(_chat_schema_fields())}" + ) + + +def test_output_cap_is_declared(): + """The output cap must be present — an absent key falls back to 1024.""" + assert _GENERATION_PARAMS.get("max_tokens") == 512 + assert "max_new_tokens" not in _GENERATION_PARAMS + + +@pytest.mark.integration +@pytest.mark.skipif( + not _watsonx_creds_present(), + reason="WATSONX_API_KEY / WATSONX_PROJECT_ID not set", +) +def test_max_tokens_cap_is_enforced_live(): + """Ask for far more than the cap allows and check the answer is cut short. + + Without an enforced cap the model answers to its own default (1024 + tokens) and this assertion fails, which is the point: it distinguishes a + working cap from an ignored one. + """ + _load_dotenv_file(_ENV_FILE) + text = generate( + prompt="Write a detailed 800-word essay about the history of the printing press.", + system_prompt=None, + model_id="meta-llama/llama-3-3-70b-instruct", + params={"max_tokens": 16, "temperature": 0}, + ) + assert text.strip(), "model returned nothing" + # 16 tokens of English prose is well under 40 whitespace-separated words; + # an uncapped answer to this prompt runs into the hundreds. + assert len(text.split()) < 40, f"cap not enforced, got {len(text.split())} words: {text!r}" From 5a1a2e5076dad89dcd558d4ce077660a590bf218 Mon Sep 17 00:00:00 2001 From: Sergi Torres Albert Date: Tue, 28 Jul 2026 23:29:58 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(ml):=20lower=20the=20output=20cap=20to?= =?UTF-8?q?=20320=20=E2=80=94=20512=20does=20not=20fit=20the=20latency=20b?= =?UTF-8?q?udget?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured against real Watsonx (eu-de, llama-3-3-70b, n=5 per cell, hard timeout lifted so the true duration shows): cap=512 vanilla median 4.5s worst 5.9s (~196 words) autoria median 7.2s worst 8.4s (~313 words) <- over the cap cap=320 vanilla median 5.0s worst 5.3s (~242 words) autoria median 5.4s worst 5.6s (~223 words) At 512 the conditioned branch straddles HARD_TIMEOUT_SECONDS: it sometimes returns and sometimes exhausts all four attempts. A failed AutorIA branch cannot be degraded away because the passport needs it, so that surfaces as an intermittent 503 from POST /api/generate — reproduced twice while verifying the RAG wiring end to end, once passing and once failing on the same prompt. 320 keeps a margin under both the 8s hard timeout and the 10s client-side abort in frontend/src/lib/api.ts, and still yields ~220 words per column. Co-Authored-By: Claude Opus 5 --- ai_pipeline/autoria_ai/generator.py | 17 ++++++++++++++++- backend/tests/test_generation_params.py | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/ai_pipeline/autoria_ai/generator.py b/ai_pipeline/autoria_ai/generator.py index 6b70355..a697896 100644 --- a/ai_pipeline/autoria_ai/generator.py +++ b/ai_pipeline/autoria_ai/generator.py @@ -42,8 +42,23 @@ # an unknown key is dropped in silence, not rejected. ``max_new_tokens`` is the # *text-generation* spelling and has no effect on ``ModelInference.chat()``, # which is what we call — see backend/tests/test_generation_params.py. +# 320, not the 512 originally written here, because 512 does not fit the +# latency budget. Measured against real Watsonx (eu-de, llama-3-3-70b, n=5 per +# cell, hard timeout lifted so the true duration shows): +# +# cap=512 vanilla median 4.5s worst 5.9s (~196 words) +# autoria median 7.2s worst 8.4s (~313 words) <- over the 8s cap +# cap=320 vanilla median 5.0s worst 5.3s (~242 words) +# autoria median 5.4s worst 5.6s (~223 words) +# +# The conditioned branch straddles HARD_TIMEOUT_SECONDS at 512: it sometimes +# returns and sometimes exhausts all four attempts, and because a failed +# AutorIA branch cannot be degraded away (the passport needs it), that surfaces +# as an intermittent 503 from POST /api/generate. 320 keeps a comfortable +# margin under both the 8s timeout and the 10s client-side abort in +# frontend/src/lib/api.ts, and still yields ~220 words per column. _GENERATION_PARAMS: dict[str, Any] = { - "max_tokens": 512, + "max_tokens": 320, "temperature": 0.7, "top_p": 0.9, } diff --git a/backend/tests/test_generation_params.py b/backend/tests/test_generation_params.py index 28beebc..171dbae 100644 --- a/backend/tests/test_generation_params.py +++ b/backend/tests/test_generation_params.py @@ -64,7 +64,7 @@ def test_generation_params_match_chat_schema(): def test_output_cap_is_declared(): """The output cap must be present — an absent key falls back to 1024.""" - assert _GENERATION_PARAMS.get("max_tokens") == 512 + assert _GENERATION_PARAMS.get("max_tokens") == 320 assert "max_new_tokens" not in _GENERATION_PARAMS From b4489fbc836a9ed6a41558755b267c54328a0814 Mon Sep 17 00:00:00 2001 From: Sergi Torres Albert Date: Tue, 28 Jul 2026 23:44:56 +0200 Subject: [PATCH 3/3] test(back): put ai_pipeline on sys.path for the backend suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test_generation_params.py` imports `autoria_ai.generator` to check our parameters against the SDK schema. That passes locally, where an editable install of `autoria_ai` sits in the venv, and fails in CI, which installs only `backend/`: ModuleNotFoundError: No module named 'autoria_ai' Same resolution the production code already uses in `app.routes.generate._ensure_ai_pipeline_on_path` — the repo-root `ai_pipeline` directory goes on `sys.path`. This is exactly the local/CI divergence the completeness audit warned about: a green local run proving nothing about the deployed shape. Co-Authored-By: Claude Opus 5 --- backend/conftest.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/backend/conftest.py b/backend/conftest.py index 99d83fe..6514ed3 100644 --- a/backend/conftest.py +++ b/backend/conftest.py @@ -2,4 +2,17 @@ pytest inserts the directory containing the rootdir conftest.py onto sys.path, so tests can `from app.main import app` without an editable install. + +The monorepo's ``ai_pipeline`` is added the same way, and for the same reason +the production code does it (``app.routes.generate._ensure_ai_pipeline_on_path``): +CI installs only ``backend/``, so ``import autoria_ai`` fails there while +passing locally, where an editable install papers over the difference. Tests +that assert on pipeline constants must not be green locally and red in CI. """ + +import sys +from pathlib import Path + +_AI_PIPELINE = Path(__file__).resolve().parent.parent / "ai_pipeline" +if _AI_PIPELINE.is_dir() and str(_AI_PIPELINE) not in sys.path: + sys.path.insert(0, str(_AI_PIPELINE))