Skip to content

FIX Select GPT-5 response piece by data type instead of position - #2297

Open
romanlutz wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-fix-gpt5-piece-ordering-tests
Open

FIX Select GPT-5 response piece by data type instead of position#2297
romanlutz wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-fix-gpt5-piece-ordering-tests

Conversation

@romanlutz

Copy link
Copy Markdown
Contributor

Description

#2283 made OpenAIResponseTarget sort reasoning pieces after the actionable response pieces, so that Message.get_piece() (which defaults to n=0) hands scorers and scenarios the actual answer rather than the reasoning blob. For reasoning models such as GPT-5 that flipped the piece order from [reasoning, text] to [text, reasoning].

The GPT-5 Responses integration tests and the paired Responses doc notebook still hard-coded message_pieces[1] as the answer, so they were validating the reasoning piece. Because these tests need live Azure GPT-5 + Entra auth they don't run in PR CI, which is why they went stale unnoticed. A remote integration run on main surfaced them:

  • test_execute_notebooks[2_openai_responses_target.ipynb]
  • test_openai_responses_gpt5[entra]
  • test_openai_responses_gpt5_json_schema[entra]

test_openai_responses_gpt5_json_object did not fail but was silently broken — it only asserts json.loads(...) succeeds, and the serialized reasoning piece is itself valid JSON, so it passed vacuously against the wrong piece. It's fixed here too.

The product behavior is correct and unchanged — this PR only fixes the stale assumptions in the tests and docs. Assertions now select the piece via Message.get_piece_by_type(data_type="text") so they no longer depend on ordering, while still asserting that both pieces are present and that a reasoning piece exists.

Tests and Documentation

doc/code/targets/2_openai_responses_target.py and its .ipynb were updated inline and verified in sync by round-tripping the notebook through jupytext --to py:percent and diffing against the committed .py (identical). Notebook outputs are preserved.

Test runs (live Azure GPT-5, Entra auth):

  • tests/integration/targets/test_openai_responses_gpt5.py — 5 passed, 5 skipped (all [entra] params pass; [api-key] params remain skipped since key auth is disabled in our tenant)
  • test_notebooks_targets.py::test_execute_notebooks[2_openai_responses_target.ipynb] — passed
  • tests/unit/prompt_target/target/test_openai_response_target.py — 75 passed, confirming the ordering contract added by FIX Handle structured refusals and scenario partial failures #2283 is untouched

PR microsoft#2283 made `OpenAIResponseTarget` sort reasoning pieces after the
actionable response pieces, so for reasoning models the answer moved from
`message_pieces[1]` to `message_pieces[0]`. The GPT-5 Responses
integration tests and the paired Responses doc notebook still indexed
position 1 and therefore validated the reasoning blob instead of the answer.

Select the piece via `Message.get_piece_by_type(data_type="text")` so the
assertions no longer depend on piece ordering, while still asserting that a
reasoning piece is present.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a008124c-220e-491c-9ea1-2144867ba53d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants