fix: stop overwriting multi-message INPUT_VALUE in OpenAI instrumentor#184
Open
nigamgauri wants to merge 2 commits into
Open
fix: stop overwriting multi-message INPUT_VALUE in OpenAI instrumentor#184nigamgauri wants to merge 2 commits into
nigamgauri wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes a bug in the OpenAI instrumentor where multi-turn conversations lost every message after the first one in the trace.
Why?
_process_input_datain_span_io_handler.pysetINPUT_VALUEon the span three times. The first write was correct, a JSON array of every message in the conversation. The next two writes overwrote it: first with a joined string of all message text, then with justeval_input[0], the text of the first message only. Any conversation with more than one message lost everything past turn one in the trace. There's noEVAL_INPUTattribute defined infi_types.py, so theeval_inputaccumulation had no other consumer, it was just clobbering good data. Fixes #151.How was it tested?
pytest)ruff checkpasses on the files I changedmypywas not run cleanly on this branch (pre-existing errors in unrelated files, unrelated to this fix)Added
tests/test_span_io_handler.pywith two cases: a plain multi-turn conversation, and a multimodal message list with text and an image. Both fail onmain(the multi-turn case raises aJSONDecodeErrorsinceINPUT_VALUEends up as raw text instead of JSON) and pass with the fix.Checklist
mainNotes for reviewers
I didn't touch the multimodal branch's image handling, only removed the three-way overwrite on
INPUT_VALUE. Happy to add a changelog entry if the project wants one for this package.