-
Notifications
You must be signed in to change notification settings - Fork 12
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
2d543e5
a40d401
93109ed
149364a
96bf555
df0d36a
12d3d9d
0cedefb
017de13
c6c90fd
b5b66dd
0ae31a3
8eafd26
92d50ed
57ba47f
151fdb8
43438ef
4f6bdd8
ee48f39
6cd3c5f
94ad069
f48d59f
d60ecbc
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.