Repository navigation
fix: Drop all trailing user turns when a model turn is invalid in _extract_curated_history - #3092
DavidLeeGarrett wants to merge 2 commits into
Conversation
…_curated_history Previously only a single user Content was popped after an invalid model turn, leaving orphaned user messages in the curated history when several user turns preceded it. Now all consecutive trailing user-role contents are removed. Fixes googleapis#3051
…_curated_history Previously only a single user Content was popped after an invalid model turn, leaving orphaned user messages in the curated history when several user turns preceded it. Now all consecutive trailing user-role contents are removed. Fixes googleapis#3051
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
chrikrah
left a comment
There was a problem hiding this comment.
@DavidLeeGarrett, this fixes #3051 as I filed it, and the CLA is the only thing I see holding it. The repro from that thread prints an empty curated history at your head.
$ python -I repro.py <tree> # repro from #3051 plus three variants, chats.create only, no request sent
base 83e43ea head 71065cf
issue repro [('user', 1)] []
two responses [('user', 1), ('user', 1)] []
after valid turn [('user', 1), ('model', 1), ('user', 1), ('user', 1)] [('user', 1), ('model', 1)]
call then answer [('user', 1), ('model', 2), ('user', 1)] [('user', 1), ('model', 2)]
$ python -m pytest -q -p no:cacheprovider google/genai/tests/chats/test_get_history.py
head 71065cf: 18 passed
head tests, chats.py from 83e43ea: 1 failed (test_history_with_consecutive_user_inputs_and_invalid_model_turn), 17 passed
# CPython 3.12.6, requirements.txt pins; git merge-tree onto main 969d0e7 is clean
# not run: the replay suites in tests/chats, which need credentials
non-blocking: in the last row, curated history ends on a model turn holding two function_call parts with no response behind them. The walk stops at that model turn by design, and #3051 asked for no more than the user run.
A fix: prefix on the title clears conventionalcommits.org.
@yyyu-google, you changed curated history in b382e01 yesterday: should a rejected turn also drop the function_call that opened it, here or in a separate issue?
Previously only a single user
Contentwas popped after an invalid model turn, leaving orphaned user messages in the curated history when several user turns preceded it. Now all consecutive trailing user-role contents are removed. Addstest_history_with_consecutive_user_inputs_and_invalid_model_turn.Fixes #3051.