Session picker shows thread_name if set #10340
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d34a62020d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let name = entry.thread_name.trim(); | ||
| if !name.is_empty() && thread_ids.contains(&entry.id) { | ||
| names.insert(entry.id, name.to_string()); |
There was a problem hiding this comment.
The new batch lookup drops entries whose latest thread_name is empty (if !name.is_empty()), so if a user clears a thread name (empty string is a valid value in SessionIndexEntry), this function will keep returning the last non-empty name from earlier lines. In the session picker that means a cleared name keeps showing the stale title instead of falling back to the first user message, which differs from find_thread_name_by_id (it returns the latest entry even if empty). This only happens when a thread name is cleared or set to whitespace and the picker uses the batch path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This would only happen if a user manually modifies session_index.jsonl as we don't allow deleting names and would prefer deleting an entry rather than setting the name at "" (empty string)
There was a problem hiding this comment.
I think "(no name)" might be ok here rather than removing.
joshka-oai
left a comment
joshka-oai
left a comment
There was a problem hiding this comment.
It may be worth adding an insta snapshot test that shows how the session picker works with thread names. This would help show that this is working end to end.
/resumeandcodex resumeif set, default to preview (previous behaviour) if nonefind_thread_names_by_idsthat maps names to IDs incodex-rs/core/src/rollout/session_index.rs. It reads sequentially in normal (instead of reverse order incodex resume <name>) the index mapping file. This function is called from a list of session (default page is 25, pages loaded depends of height of terminal), for which most of them will always have at least one session unnamed and require the whole file to be read therefore. Could be better and sqlite integration will make this betterOpened questions: