feat(sessions): give past conversations one display identity across every picker - #701
Open
Juliusolsson05 wants to merge 7 commits into
Open
feat(sessions): give past conversations one display identity across every picker#701Juliusolsson05 wants to merge 7 commits into
Juliusolsson05 wants to merge 7 commits into
Conversation
#96 has been open 108 days and frames the inconsistency as a renderer problem — three modals painting the same thing differently. Reading the code shows the decision is made lower down and cannot be fixed in the renderer at all. The providers each flatten identity into a lossy `summary: string` before the UI sees it, and they disagree: Claude labels a session by its LAST prompt, Codex by its FIRST, so the same conversation renames itself when it changes provider. Codex writes a truncated hex id INTO that summary field, which makes #96's own requirement — show the user when they are looking at a fallback — impossible against the current contract. And Claude drops a session with no derivable summary entirely while Codex shows it, so the providers disagree about list membership, not just labels. The plan therefore derives a typed identity once in main, carrying `labelSource` so a fallback can be seen as one, and unifies the three real pickers on one row component. It also corrects the issue's scope: ViewPromptsModal and RewindToPromptModal pick a prompt inside an already-open session, never display a session identity, and are removed from scope; CommandPalette, which postdates the issue, is added. Refs #96
Adds the typed record and the single fallback ladder that every session picker will resolve its label through. Nothing consumes it yet. The key field is labelSource. Providers previously flattened identity into a lossy `summary: string`, which is why #96 could not be fixed in the renderer: a row holding that string cannot tell a real title from a stand-in, so marking a fallback as a fallback was unimplementable. Keeping provenance alongside the label makes that possible and makes the ladder assertable rung by rung. Two deliberate behaviour choices, both pinned by tests: - first-prompt now outranks last-prompt. A last-prompt label mutates as the conversation continues, so the same session is unrecognisable an hour later. Codex already keyed on the first message, so this is also what makes a provider switch identity-preserving. - a session with no derivable name falls to its cwd basename and then a truncated id rather than being dropped. Invisible is worse than poorly named, and the id still resumes. identityInputFromSessionInfo recovers the ingredients from what the existing listers already return, including the Codex bridge that rejects a summary equal to the leading 8 hex characters of its own rollout id. Codex writes that id into the summary field itself, in a submodule; the bridge keeps this work from being blocked behind a cross-repo PR and is documented for deletion once codex-headless returns structured identity. Refs #96
Derives the identity in main and attaches it to the two channels the pickers read: session:list-for-cwd / session:list-all (the resume path) and the prompt index (the search path). Both go through the same adapter and ladder, so the Codex hex-id bridge and the first-prompt preference cannot apply on one path and not the other — a session that renames itself depending on which picker you opened is the bug being fixed. The prompt index offers its newest prompt to the last-prompt rung rather than first-prompt: recentUserPrompts is a capped newest-first window, so its oldest member is not the conversation's opening line once a session exceeds the cap. Claiming otherwise would mislabel the row while looking correct. Also collapses SessionIndexEntry/SessionIndexPrompt into @shared/types/sessionIndex. They existed twice — in main and hand-copied into preload — with nothing obliging the copies to agree, and the preload copy had already lost fields to drift. Adding a third copy of the identity field was not defensible, and duplicated contracts across a process boundary are the same class of problem #96 is about. Both sites now re-export the shared shape. `summary` stays on the wire for search ranking and non-display callers, marked deprecated for display so it does not creep back into a row label. Refs #96
The display half of #96. SessionPickerRow renders a SessionDisplayIdentity and makes no decision about what a session is called — that belongs to the ladder in main, and a second opinion here would recreate the divergence the issue is about. It marks a fallback label as a fallback, in italics and in the title attribute. This is the part the old surfaces could not do: they received a flattened string and had no way to tell "the user titled this conversation agent-code" from "we gave up and used the folder name". Rendering both identically is what made the pickers feel arbitrary, and labelSource exists to carry that distinction across the process boundary. Prompt search migrates first because it was already closest to the target shape. It keeps its prompt-list body and mounts the row as its header via the trailing slot — the goal is one identity everywhere, not one layout everywhere. Tests assert the fallback marking rather than the label text: rendering a string cannot regress meaningfully, but presenting a stand-in as a real name is the defect, it still looks plausible on screen, and a reviewer would not catch it. Refs #96
The two surfaces that consumed the same SessionInfo and still disagreed. The command palette rendered `summary || firstPrompt || sessionId`, a chain that could fall all the way through to a full untruncated uuid with nothing telling the user that is what they were looking at. The PathPicker resume row is the one #96 was actually filed about: it led with a flattened summary and showed sessionId.slice(0, 8) permanently on its second line, so a user who remembered a conversation by what they typed got a hex id. The id now appears only as a marked fallback, when there is nothing better. Both now take the identity resolved in main. Neither builds a label any more, so there is one answer to "which past conversation is this?" across resume, the palette, and search. Refs #96
…ntity rule Three things that finish #96. The Claude lister returned null when it could derive no summary, removing those sessions from the resume picker entirely. Codex showed the equivalent session with a hex-id label, so the providers disagreed about list MEMBERSHIP and not merely about naming: a Claude transcript with no title, no lastPrompt and no recoverable first prompt was invisible even though its id resumes fine. Invisible is strictly worse than poorly named, and the ladder now labels these from the cwd basename or the truncated id, marked as a fallback. registry.main.ts took its cwd-scoped Claude lister from claude-code-headless while sessionIndex.ts used the app-side copy. Both were live and their parse logic is identical, so the split bought nothing — but it meant the fix above would have reached every surface except the resume picker, which is the one the issue was filed about. The registry now uses the app-side lister. The boundary test is the part that keeps this closed. #96 was the same bug recurring: each new surface that learned to list sessions invented its own label. It named three modals; a fourth had appeared before anyone picked it up. So the test asserts the rule rather than the instances — a session picker renders through SessionPickerRow and never builds a label from `.summary`, `.firstPrompt`, or a sliced `.sessionId`. Verified by reintroducing the exact line the palette used to carry and confirming the failure names the offending file and the fix. A filesystem scan rather than a lint rule or a type-level ban, matching importBoundaries.test.ts: `summary` must stay reachable for search ranking, so what is forbidden is using it to build a label — a usage rule, not a shape rule. Refs #96
Six stages rather than five, reordered so every commit leaves the app working: the Claude unnamed-session change had to land after the pickers migrated, since `summary` was what the unmigrated ones still rendered. Also records dropping turnCount (nothing can populate it) and the registry consolidation, which was not foreseen but had to happen or the fix would have missed the resume picker. Refs #96
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
Three surfaces let you pick a past conversation — the PathPicker resume list, the command palette, and prompt search — and each showed a different identity for the same session. You remember a conversation by what you typed; the picker showed you a hex id or a folder name.
Fixes #96, open 108 days.
Why this isn't a renderer change
#96 frames this as a display problem and proposes unifying the row component. Reading the code, that would have half-fixed it. The identity decision is made in the provider listers and flattened into a lossy
summary: stringbefore any UI sees it.Three defects no renderer change can reach:
customTitle ?? lastPrompt ?? firstPrompt— the last prompt. Codex used the first user message. A session moved between providers renamed itself, and cross-provider switching is a headline feature of this app.summaryitself, bycodex-headless(SessionList.ts:250,264), when no user text is found. A renderer holding that string cannot tell a title from a fallback — so Resume + session search are inconsistent — sometimes show IDs, sometimes cwd names, sometimes prompts; needs to be reasoned through and redone #96's own requirement, "the fallback should be visible so the user knows they're looking at a fallback", was unimplementable against the old contract.if (!summary) return null— a session with no derivable name was invisible in the resume picker. Codex showed the same session with a hex label.What this does
A typed
SessionDisplayIdentityderived once in main, carrying the field that makes the rest possible:labelSourceis load-bearing: it lets a row mark a fallback as a fallback, lets tests assert the ladder rung by rung, and removes the "is this string secretly a session id?" guesswork. One ladder, both providers, applied identically on both listing paths.SessionPickerRowrenders it and makes no naming decision of its own — a second opinion about identity is exactly what #96 is.Behaviour changes — please read
Both are intended, and both are user-visible:
Scope, corrected
#96 names four modals. Two are out of scope:
ViewPromptsModalandRewindToPromptModalpick a prompt inside an already-open session, take asessionIdprop, and never display a session identity. Forcing them onto a shared session row would be a regression dressed as consistency.One was added:
CommandPalettepostdates the issue, was never inventoried, and leaked a full untruncated uuid. It is why this kept getting worse.Two cleanups inside the blast radius
SessionIndexEntryexisted twice — in main and hand-copied intopreload/api/types.ts, already drifted. Adding the identity field to a duplicate wasn't defensible; both now re-export@shared/types/sessionIndex.registry.main.tstook its Claude lister from the submodule whilesessionIndex.tsused the app-side copy. Both live, identical parse logic. Left alone, the unnamed-session fix would have reached every surface except the resume picker — the one the issue is about.The lock
#96 was the same bug recurring: every new session-listing surface invented its own label. Fixing three surfaces without a gate means surface five reopens it.
sessionPickerIdentity.test.tsasserts the rule, not the instances: a session picker renders throughSessionPickerRowand never builds a label from.summary,.firstPrompt, or a sliced.sessionId. A filesystem scan rather than a lint rule, matchingimportBoundaries.test.ts—summarymust stay reachable for search ranking, so what's forbidden is using it to build a label, a usage rule rather than a shape rule.Mutation-verified: reintroducing the exact
summary || firstPrompt || sessionIdline the palette used to carry fails the test with a message naming the file and the fix.Verification
npm run typecheck— clean, both projectsnpm run test:contract— satisfiednpm run test:system— 10 files, 85 tests, all passnpm run test:unit— 1801 pass; 3 pre-existing failures, each reproduced identically on cleanmainatf9d05236and already tracked: bug(app-state): rehydrate never resolves when migrating legacy standalone prompt templates #686 (store migration, ×2) and bug(testing): imageAttachment corpus test fails locally once a cited Claude session is rotated #684 (image fixture census)npm run test:renderer— 388 pass; 1 pre-existing failure, the lazy-prose 5s timeout, reproduced on cleanmainand filed as bug(testing): lazy-prose renderer test times out at 5s under load #700Known limitations
The Codex hex-id bridge is a heuristic — it treats a
summaryequal to the leading 8 hex characters of its own rollout id as absent. It's sound (we control both sides) and it avoids blocking every UI fix here behind acodex-headlessPR, a submodule bump, and a lockfile resync. It carries a comment saying what replaces it: teachingcodex-headlessto return structured identity, after which the bridge deletes and the ladder needs no change.Fixes #96