sessions: name a session, and give the picker two-line rows - #11
Merged
Merged
Conversation
A one-line row put the title and every identifying detail on the same line, so a narrow terminal truncated the title away first — the part the list is read for. Rows are now two lines: the label on top, the id, repo, model and age beneath. The picker was already scrollable and windows by item, so the only change there is that half as many fit. The id moved down with the rest. It identifies a session but nobody scans a list by it. /title names the live session, and the picker shows that instead of the first prompt. A first prompt is a poor label for what a long session turned into. Titles live in cathode's own store: neither backend has a place to put one, and a name is the user's note to themselves rather than anything the agent sees. An empty /title clears it, so there is no separate unset verb. Two things about the selected row were found by rendering it rather than by reading it. Its content is padded to a fixed width BEFORE styling, because a title and its detail line are never the same length and styling first leaves the lightbar as two ragged blocks. And it is padded two columns short, because approveBar carries Padding(0, 1): pad to the full width and the styled row renders too wide, wraps, and the scrollbar gutter interleaves with the text. The test for that counts lines, not widths. The first version measured widths, passed with the bug reintroduced, and was worthless — the surrounding box pads every line to the same width whatever happens inside it.
Three things a second review found. truncFirst sliced by byte offset, so a prompt with an em-dash or a curly quote near the cut came back as invalid UTF-8. That is the same bug sysPromptSummary was already fixed for, and trunc has been rune-safe all along — four other files use it. Pre-existing, fixed by reuse rather than by a fourth implementation. trimTitle was that fourth implementation. It now goes through trunc too, which also means a capped title marks what it dropped: cut mid-word with no ellipsis, it reads as one the user typed that way. With neither a title nor a first prompt, the row fell back to the id and then printed it again at the head of the detail line underneath. The lightbar inset is asked of the style rather than written as a 2. A theme that changed the padding would otherwise wrap the row and desync the scrollbar, and the symptom is a layout glitch nobody traces back to a style definition. Verified by setting the padding to 3: the derived version holds, the constant would not have.
/title wrote the store, printed its confirmation, and the picker showed the first prompt anyway. Only codex sessions were unaffected, and only because they never reach this path — on claude, the backend most sessions run on, the feature did nothing at all. mergeWithStore started from the filesystem entry and copied two named fields out of the store into it. Every claude session has a JSONL on disk, so the filesystem entry always wins the merge, and Title was not one of the two names. Backend had the same exposure and survived only by luck: an unset backend already means claude. It now starts from the stored record and lets the filesystem override what it is authoritative for. A new sessionInfo field is carried by default, and only an explicit override comes from the filesystem — so the next field added does not have to be remembered here. The tests missed it because they build a store with no filesystem sessions behind it, which takes the store-only path and never exercises the merge. The new one calls mergeWithStore directly with both halves present.
/title on a resumed session answered "no session to name yet — send a turn first". The id was known all along: it is what the session was resumed with. m.session was only ever set from claude's system/init line, and claude does not emit that until the first turn of a process. So a resumed session had no id until you sent a turn, and so did everything else keyed on it — the status row, and /sysprompt's decision about whether a restart can resume anything. newModel now seeds it from the id it resumed with. Behind that sat a silent one. A session started outside cathode is listed from claude's own session files and may never have been written to our store, and SetTitle ignores an id it does not know — so the title was dropped while the confirmation still printed. commitTitle now records the session before naming it. Both were found by running the command, not by reading it. The unit tests covered a live session that had already been through a turn, which is the one case where neither bug shows.
The title was cut at 64 or 72 runes while the row can show about 95, so a third of a wide modal sat empty. The ellipsis came from the store, not the screen. Those caps were a storage bound doing display duty. sessionLabelMax replaces both and is set well past any width the picker can render, so the terminal is what decides how much shows. A test pins that relationship, because the two numbers live in different files and the failure is silent — a shorter label just looks like a shorter session. Truncating for the screen is the picker's job, and it now marks a cut with an ellipsis rather than stopping mid-word. That applies to every picker: a command description trimmed to fit reads as the whole of a shorter one otherwise. No migration needed on claude: listClaudeSessions re-reads each session file and re-truncates on every open, so those labels widen straight away. A store-only row keeps its shorter label until it is touched again. truncFirst's comment said it trimmed the subtitle. The first prompt became the title row when the rows went to two lines, and the const added above it had orphaned the comment onto the wrong declaration.
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
1 participant
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.
The problem
A one-line row put the title and every identifying detail on the same line, so a narrow terminal truncated the title away first — the part the list is actually read for.
Rows are now two lines
Yes, it scrolls — it always did. It windows by item and draws that gutter, capped at 16 rows or the terminal height. Two-line rows just halve how many fit, which the existing windowing already handles once it counts items rather than screen rows.
The id moved down with the rest of the detail. It identifies a session but nobody scans a list by it.
Opt-in per picker (
picker.twoLine), and only the session picker sets it. Everywhere else the trade is not worth it./titleNames the live session; the picker shows that instead of your first prompt, which is a poor label for what a long session turned into. Titles live in cathode's own store — neither backend has a place to put one, and a name is your note to yourself rather than anything the agent sees. An empty
/titleclears it, so there is no separate unset verb. Works on both backends.Two things found by rendering, not by reading
approveBarcarriesPadding(0, 1). Pad to the full width and the styled row renders too wide, wraps, and the scrollbar gutter interleaves with the text.The test for that counts lines, not widths. My first version measured widths, passed with the bug deliberately reintroduced, and was worthless — the surrounding box pads every line to the same width whatever happens inside it. The rewritten one fails when the bug returns.
Verified
go vet, full suite, and the picker rendered and eyeballed at 64 columns.