Repository navigation
LCORE-4070: raise a model error when OGX reports a streamed request as failed - #2868
max-svistunov wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (35)
🧰 Additional context used🪛 ast-grep (0.45.3)tests/unit/pydantic_ai_lightspeed/ogx/test_model.py[warning] 739-739: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url. (llm-client-insecure-http-python) 🔇 Additional comments (2)
WalkthroughThe OGX stream handler now raises ChangesOGX streamed failures
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Streamed model failures are handled through the existing error responses. No issue identified here prevents merging after normal checks. 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…s failed A prompt too long for the model made /v1/streaming_query stream a complete answer that had nothing to do with the question, where "Prompt is too long" was expected. The conversation then held a retry prompt as the user message. OGX reports a provider failure of a streamed Responses request inside the stream, as a response.failed event. OgxResponsesModel passed it on, pydantic-ai took the empty response for a missing answer and sent a second request with the retry prompt "Validation feedback: Please return text. Fix the errors and try again." Its reply was streamed and stored as the answer. _FilteredResponseStream now raises ModelHTTPError for the event, with status 500 and the error of the event as body. The run ends after one model request and nothing is stored. The existing mapping turns a context-length message into 413; an unrecognised failure stays a 500. This applies where the model request is streamed: /v1/streaming_query, A2A, and /v1/query with a Granite Guardian shield. A failed request is no longer followed by a second one. The non-streamed call is untouched. Two unit tests: request_stream raises for a response.failed event, and an agent run over such a stream makes one model request and maps to 413. Checked on a live service in library mode with gpt-4o-mini on /v1/streaming_query only; A2A and Granite Guardian were not run.
24f281d to
99c9d27
Compare
Description
LCORE-4070. When a query is too long for the model's context window,
/v1/streaming_querystreamed a complete answer that had nothing to do with the question, wherePrompt is too longwas expected. The change comes from the LCORE-4108 work, whose compaction capability has a stream hook and so streams the model call on/v1/querytoo. It is proposed alone: it fixes LCORE-4070 on main and needs nothing from LCORE-4108.Cause. OGX reports a provider failure of a streamed Responses request inside the stream, as a
response.failedevent.OgxResponsesModelpassed the event on, and pydantic-ai took the empty response for a missing answer: it sent a second model request with its retry prompt (Validation feedback: Please return text. Fix the errors and try again.). With a conversation attached,_prepare_conversation_continuationkeeps only the messages after the failed response, so that request carried the retry prompt and not the question. Its reply was streamed as the answer and stored as the turn.Fix.
_FilteredResponseStreamraisesModelHTTPErrorfor the event (status 500, the error of the event as body), so the run ends after one model request. The endpoints already catch agent errors and map them: a context-length message gives 413Prompt is too long, a message that namesRESOURCE_EXHAUSTEDgives 429, anything else the generic 500. A provider rate limit is in the last group: the event carries a code and a message, no HTTP status, and the code is not read.What changes. Only requests whose model call is streamed and which OGX reports as failed:
/v1/streaming_querytokenevents andturn_completewith the reply to the retry prompt; that turn is storederrorevent; nothing stored/v1/streaming_query, failure after part of the answer (by reading, not run)turn_completewith the partial text, thenerror; that turn is storederror; nothing storedfailedwith the mapped message/v1/querywith a Granite Guardian shield, whose stream hook makes the run stream (by reading, not run)What does not change.
/v1/querywithout Granite Guardian uses the non-streamed call. In library mode it answers 503 to the same query, before and after: the OGX exception escapes the library transport there, a separate defect outside this PR. A request that succeeds is handled as before.What is given up. Until now a failed streamed request was followed by a second one, so a failure that happened once (a rate limit, a provider 5xx) could still end in a 200 stream. That second request did not repeat the question, so what came back was the reply to the retry prompt, stored as the turn. (On a compacted turn it carried the query, without the summaries and recent turns; by reading.) Now the client gets the error and has to resend the request. This PR adds no retry of its own.
Type of change
pyproject.toml+uv.lock]requirements.*.txtfor Konflux]Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
openai/gpt-4o-mini, and put a recording proxy between OGX and the provider. Send a query far beyond the context window ("the quick brown fox jumps over the lazy dog " * 17000 + "Reply with OK only.", 748,019 characters) to/v1/streaming_query. Expected: one provider request, anerrorevent with status 413, nothing stored. Without the fix:The stream ends there, with no
endevent: in library mode the failed topic-summary request raises through the endpoint. The ticket reports a genericerrorevent at that point. With the fix (one provider request, rejected with the same 400; the conversation holds no item):Check that nothing else moved. Send the same query to
/v1/query(non-streamed call): with and without the fix it answers503 {"detail":{"response":"Unable to connect to OGX","cause":"Connection error while trying to reach backend service."}}after three provider requests. SendWhat is the capital of France? Reply with one word.to/v1/streaming_query: both streams arestart, twotokenevents,turn_completewithParis.andend, with two provider requests each. Server mode, A2A and Granite Guardian were not run.Run the tests for this change, the full suites and the linters. Both new tests fail without the change in
src/(DID NOT RAISE ModelHTTPError;UnexpectedModelBehavior: Exceeded maximum output retries (1)):Summary by CodeRabbit