Skip to content

test: derive streamed-usage expectations from the SDK object (unbreaks main) - #12367

Closed
lets-order-some-fries wants to merge 1 commit into
deepset-ai:mainfrom
lets-order-some-fries:fix/openai-usage-fixture-drift
Closed

test: derive streamed-usage expectations from the SDK object (unbreaks main)#12367
lets-order-some-fries wants to merge 1 commit into
deepset-ai:mainfrom
lets-order-some-fries:fix/openai-usage-fixture-drift

Conversation

@lets-order-some-fries

Copy link
Copy Markdown

Related Issues

  • No open issue; main is currently red and I hit it while running the suite for another PR. Filing the fix directly since CI is blocking everyone.

Proposed Changes:

main is failing on all three OS jobs. Run 31934088851 at 915888bbUnit / ubuntu-latest, Unit / macos-latest and Unit / windows-latest all end with:

FAILED test/components/generators/chat/test_openai.py::TestChatCompletionChunkConversion::test_convert_chat_completion_chunk_to_streaming_chunk
FAILED test/components/generators/chat/test_openai.py::TestChatCompletionChunkConversion::test_handle_stream_response
= 2 failed, 6113 passed, 8 skipped, 323 deselected =

Cause. chore: unpin openai (#12348) relaxed the constraint to openai>=2.6.0, so a fresh environment now resolves openai 3.1.0, and CompletionUsage gained additive fields:

  • prompt_tokens_detailsimage_tokens, text_tokens
  • completion_tokens_detailstext_tokens

Both failing tests restated the expected usage payload as a literal enumerating openai 2.x's exact field set, so model_dump() now returns more keys than the literal lists and the comparison fails. The diff is confined to meta["usage"]; nothing in haystack/ is wrong — _serialize_object forwards model_dump() faithfully, which is precisely what it should do.

Fix. The contract these tests exist to protect is "haystack forwards the SDK's usage unchanged", not "the SDK's usage schema has exactly these keys". Restating the schema as a literal encodes the second, which is not this repo's invariant to own — and with the dependency now unpinned it will drift again on the next release that adds a field.

The expected value is therefore derived from the same CompletionUsage object the fixture streams. Net -38/+18 lines: two duplicated literals collapse into one shared object.

STREAMED_USAGE = CompletionUsage(
    completion_tokens=42, prompt_tokens=282, total_tokens=324,
    completion_tokens_details=CompletionTokensDetails(...),
    prompt_tokens_details=PromptTokensDetails(audio_tokens=0, cached_tokens=0, cache_write_tokens=0),
)

used both as the chunk's usage= and, via .model_dump(), as the expectation in the streaming_chunks fixture and in test_handle_stream_response.

How did you test it?

At this head (9141ca50):

  • hatch run test:unit test/components/generators/chat/test_openai.py55 passed (both previously-failing tests now pass)
  • hatch run test:unit (full suite) → 6115 passed, 10 skipped, 0 failed — against main's 6113 passed / 2 failed
  • hatch run test:typesSuccess: no issues found in 432 source files
  • hatch run fmtAll checks passed!
  • pre-commit run --files test/components/generators/chat/test_openai.py → all hooks Passed

Mutation test, because a derived expectation is worth distrusting. An assertion built from the same object it validates can be tautological, so I checked that this one still fails when the behaviour it guards actually breaks. Patching _serialize_object in haystack/components/generators/utils.py to drop prompt_tokens_details from the dump:

FAILED ...::test_convert_chat_completion_chunk_to_streaming_chunk
FAILED ...::test_handle_stream_response
2 failed

and with that patch reverted, 2 passed. So the tests still catch a real pass-through regression; they no longer catch additive changes to openai's own schema, which is the part that should not have been asserted.

Notes for the reviewer

No release note, per CONTRIBUTING: this is limited to tests, so it needs the ignore-for-release-notes label from a maintainer to bypass that CI check. Say the word if you'd rather I add one instead.

Deliberately scoped to unbreaking CI: I did not re-pin openai, since #12348 unpinned it on purpose, and I did not touch the other PromptTokensDetails(...) fixtures elsewhere in the file — those are inputs rather than expectations, so they are unaffected by additive fields.

Worth noting the general shape, since the unpin makes it recurrent: any assertion that enumerates a third-party model's fields is a scheduled failure. The two sites fixed here were the only ones the suite currently trips on.

AI assistance disclosure: this PR was written with an AI assistant. I reviewed the change, and ran the tests and checks reported above.

Checklist

  • I have read the contributors guidelines and the code of conduct.
  • I have updated the related issue with new insights and changes. — N/A, no related issue.
  • I have added unit tests and updated the docstrings. — existing tests repaired; the shared fixture carries an explanatory comment.
  • I've used one of the conventional commit types for my PR title: test:.
  • I have documented my code.
  • I have added a release note file. — intentionally omitted; tests-only, needs ignore-for-release-notes (see notes above).
  • I have run pre-commit hooks and fixed any issue.

`main` is red on all three OS jobs: test_convert_chat_completion_chunk_to_streaming_chunk
and test_handle_stream_response fail because the expected usage payload was
restated as a literal listing openai 2.x's field set.

Since `chore: unpin openai` (deepset-ai#12348) relaxed the constraint to `openai>=2.6.0`,
openai 3.1.0 resolves and CompletionUsage gained additive fields
(prompt_tokens_details.image_tokens / .text_tokens,
completion_tokens_details.text_tokens), so the literals no longer match.

The contract under test is that haystack forwards the SDK's usage unchanged,
not that the SDK's schema has exactly these keys. The expectations are now
derived from the same CompletionUsage object the fixture streams, so an
additive field in a future openai release cannot break them.

Verified the assertion still has teeth: making _serialize_object drop
prompt_tokens_details fails both tests, restoring it passes them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lets-order-some-fries
lets-order-some-fries requested a review from a team as a code owner August 16, 2026 11:47
@lets-order-some-fries
lets-order-some-fries requested review from bogdankostic and removed request for a team August 16, 2026 11:47
@vercel

vercel Bot commented Aug 16, 2026

Copy link
Copy Markdown

@lets-order-some-fries is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant

CLAassistant commented Aug 16, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@HaystackBot

Copy link
Copy Markdown
Contributor

Hi @lets-order-some-fries, thanks a lot for your contribution! 🙏

We noticed that the Contributor License Agreement (CLA) check (license/cla) hasn't passed yet, so we've temporarily moved this PR to draft and paused the review assignment.

To get your PR reviewed, please sign the CLA via the link in the license/cla check below (or in the CLA bot comment). As soon as the check turns green, this PR will automatically be marked ready for review again and a reviewer will be re-assigned.

@HaystackBot
HaystackBot removed the request for review from bogdankostic August 16, 2026 13:03
@HaystackBot HaystackBot added the cla-pending PR is in draft until the contributor signs the CLA label Aug 16, 2026
@HaystackBot
HaystackBot marked this pull request as draft August 16, 2026 13:03
@HaystackBot
HaystackBot marked this pull request as ready for review August 16, 2026 13:29
@HaystackBot HaystackBot removed the cla-pending PR is in draft until the contributor signs the CLA label Aug 16, 2026
@HaystackBot

Copy link
Copy Markdown
Contributor

Thanks for signing the CLA, @lets-order-some-fries! 🎉 This PR is now ready for review again and the reviewer has been re-assigned.

@bogdankostic

Copy link
Copy Markdown
Contributor

Thanks for the PR @lets-order-some-fries! We already fixed this in #12374 so I'm closing this one.

@lets-order-some-fries

Copy link
Copy Markdown
Author

Makes sense — thanks for the quick triage @bogdankostic, and glad main is green again. #12365 remains open and is unrelated to the openai bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants