-
Notifications
You must be signed in to change notification settings - Fork 11
feat(ui,client): version navigation and non-destructive prompt rewrite #517
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d440d9f
7d247b6
6a151b3
f8c1893
d8d0db9
68cef3b
f8c1104
dfa4c45
337bab8
1d942a6
9b7e5f9
ed32870
4b692dd
4952b99
fdda262
8278289
a8c1bb4
795f0b3
bcd5805
9176938
a5f406f
b93989c
76e383b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,12 @@ export type ConversationResyncReason = 'epoch' | 'gap' | 'graph'; | |
| export interface ConversationStoreOptions { | ||
| /** Called at most once per store, never during a render, when the seed must be re-read. */ | ||
| onResync?: (reason: ConversationResyncReason) => void; | ||
| /** `false` freezes a projection store's content at its read: a client browsing an inactive | ||
| * lineage must not fold the active run's live stream, and a graph change is the owner's business | ||
| * (the "continued elsewhere" chip), not a re-read. Session state — policy, model, effort, mode, | ||
| * capabilities, commands, usage, status — still follows: it is the session's, not a lineage's, | ||
| * and the composer renders it. Default `true`. */ | ||
| followLive?: boolean; | ||
| } | ||
|
|
||
| const EMPTY_CONVERSATION: Conversation = { | ||
|
|
@@ -58,7 +64,13 @@ export function createConversationStore( | |
| return { subscribe: () => noop, getSnapshot: () => EMPTY_CONVERSATION }; | ||
| } | ||
| if (seed !== undefined && 'items' in seed) { | ||
| return createProjectionStore(client, sessionId, seed, options.onResync ?? noop); | ||
| return createProjectionStore( | ||
| client, | ||
| sessionId, | ||
| seed, | ||
| options.onResync ?? noop, | ||
| options.followLive ?? true, | ||
| ); | ||
| } | ||
| return createHistoryStore(client, sessionId, seed, options.onResync ?? noop); | ||
| } | ||
|
|
@@ -74,6 +86,21 @@ const INTERACTIVE_EVENT_TYPES = new Set<AgentEvent['type']>([ | |
| 'prompt-response-status', | ||
| ]); | ||
|
|
||
| /** Session state, not lineage content: the latest of each wins, so a frozen store folds them | ||
| * without a watermark — a parked composer must not fall back to defaults. */ | ||
| const SESSION_STATE_EVENT_TYPES = new Set<AgentEvent['type']>([ | ||
| 'status', | ||
| 'current-mode-update', | ||
| 'approval-policy-update', | ||
| 'model-update', | ||
| 'effort-update', | ||
| 'available-commands-update', | ||
| 'available-models-update', | ||
| 'capabilities-update', | ||
| 'token-usage', | ||
| 'usage-report', | ||
| ]); | ||
|
|
||
| /** | ||
| * The projection merge: the seed's items fold first, then every live event whose position is | ||
| * above the seed's watermark. Nothing is matched by content — the daemon mints one identity per | ||
|
|
@@ -86,6 +113,7 @@ function createProjectionStore( | |
| sessionId: SessionId, | ||
| seed: ConversationProjectionSeed, | ||
| onResync: (reason: ConversationResyncReason) => void, | ||
| followLive: boolean, | ||
| ): ConversationStore { | ||
| const builder = createConversationBuilder(); | ||
| const userMessageIds = new Set<string>(); | ||
|
|
@@ -145,14 +173,19 @@ function createProjectionStore( | |
| const events = client.eventsSnapshot(sessionId); | ||
| for (let i = firstIndexAfter(events, consumedSeq), len = events.length; i < len; i += 1) { | ||
| const entry = events[i]; | ||
| if (!followLive) { | ||
| if (SESSION_STATE_EVENT_TYPES.has(entry.event.type)) fold(entry.event, entry.receivedAt); | ||
| continue; | ||
| } | ||
| if (admit(entry)) fold(entry.event, entry.receivedAt); | ||
| } | ||
| consumedSeq = client.eventSeq(sessionId); | ||
| }; | ||
|
|
||
| /** A revision past this read means a lineage moved. A plain continuation is already covered | ||
| * live — its new leaf's own user row has arrived — so only a leaf this store has never seen | ||
| * (an edit or rewrite from any device, a stale read) needs the re-read. */ | ||
| * (an edit or rewrite from any device, a stale read) needs the re-read. A fork's row cannot | ||
| * pass: it relaunches under a new epoch, which `admit` flags first, and reads carry no echoes. */ | ||
| const checkGraph = (change: ConversationGraphChange | undefined): void => { | ||
| if (change === undefined || change.graphRevision <= seed.graphRevision) return; | ||
| if ( | ||
|
|
@@ -167,11 +200,13 @@ function createProjectionStore( | |
| return { | ||
| subscribe(onStoreChange) { | ||
| sync(); | ||
| checkGraph(client.latestGraphChange(sessionId)); | ||
| const unsubscribeEvents = client.subscribe(sessionId, () => { | ||
| sync(); | ||
| onStoreChange(); | ||
| }); | ||
| // A frozen store keeps its session state live but leaves graph changes to its owner. | ||
| if (!followLive) return unsubscribeEvents; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The subscription half of this fix has no test coverage — including from the test updated alongside it. I restored the old The blind spot is Behaviour looks right as written; it's just load-bearing and currently free to regress. |
||
| checkGraph(client.latestGraphChange(sessionId)); | ||
| const unsubscribeGraph = client.subscribeGraphChanges(sessionId, (change) => { | ||
| sync(); | ||
| checkGraph(change); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'status'isn't purely session state — it moves the parked view's content too, which is the one thing the comment above says a frozen store won't do.case 'status'(conversation.ts:583-591) also writesturnStopped, andsnapshot()derivesisSessionStreaming = !turnStopped && (status === 'running' || status === 'starting')(conversation.ts:758-768), overlayingisStreaming: trueon the parked read's last assistant message and on any open reasoning item. Downstream,conversation-view.tsx:101derivesisThinkingfromconversation.statusand feeds it to both the trailing<Spinner/> thinking…element andended={index < segments.length - 1 || !isThinking}.So while the active lineage is running, a parked
‹ 1/N ›view of a settled version renders a "thinking…" spinner after its last turn, suppresses that turn's trailers (diff rollup / copy / reply actions), and re-animates its final assistant message throughsmoothText(turn-segment-view.tsx:175-188) — none of which belongs to that version.Still a net improvement over a composer frozen at defaults, and
statuscan't simply leave the set: it's what drives send-vs-stop andpromptEditState: 'busy'. But if a parked view should read as settled, the seam is between the scalar and the derived overlay — pinisSessionStreamingto false on a parked snapshot (or foldstatusinto the scalar without letting it clearturnStopped) and the composer stays live either way.