Fix --continue to resume most recently used conversation (#1140)#1409
Open
PriyanshAroraa wants to merge 1 commit intosimonw:mainfrom
Open
Fix --continue to resume most recently used conversation (#1140)#1409PriyanshAroraa wants to merge 1 commit intosimonw:mainfrom
--continue to resume most recently used conversation (#1140)#1409PriyanshAroraa wants to merge 1 commit intosimonw:mainfrom
Conversation
Fixes simonw#1140. Previously load_conversation() picked the conversation with the newest id, which meant a freshly-created conversation would always shadow an older conversation the user had just resumed via --cid. Switch to picking the conversation_id of the most recent row in the responses table. Response IDs are ULIDs sorted chronologically, so 'most recent response' == 'conversation the user last used', which matches user expectations for --continue. Adds a regression test that creates two conversations, resumes the older one via --cid, then verifies --continue resumes it again instead of jumping to the newer 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.
Fixes #1140.
Problem
`llm --continue` picks the conversation with the newest `id`, which means a freshly-created conversation will always shadow an older one you have just been chatting in.
Reproduces with:
Same issue reported by a second user in the comments of #1140 ("instead continued the most recently created new conversation, instead of the last used one. Pretty counter intuitive.").
Fix
In
llm/cli.pyload_conversation(), instead of sorting theconversationstable byid desc(creation time), pull theconversation_idoff the most recent row in theresponsestable. Response IDs are ULIDs that sort chronologically, so "newest response" == "conversation the user last used".Before:
After:
Test
Added
test_llm_prompt_continue_uses_most_recently_used_conversationintests/test_llm.pythat reproduces the bug scenario — creates A, creates B, resumes A via--cid, then asserts--continueresumes A rather than B.test_encode_decode.pyunrelated to this change)Notes
--cidis passed explicitly.Noneinstead of an empty conversation object). In practice a conversation without responses shouldn't exist, but happy to add a fallback to the old behavior if you want to keep that edge case.