fix(client): address trajectory review findings

2026-08-15 21:04:50 +00:00 · 2026-07-28 15:07:44 +08:00
parent 64b9535c31
commit 9096dbb29d
9 changed files with 244 additions and 60 deletions
--- a/packages/client/runtime/src/client/sessions/history.ts
+++ b/packages/client/runtime/src/client/sessions/history.ts
@@ -15,20 +15,22 @@ export interface SessionHistoryInspection {

 /**
 * Create a lazy inspection projection over an immutable history window.
- * Conversation consumers retain the cheap wrapper; only Trajectory reads the
- * getters that replay event order and request lifecycle state.
- * @param entries - Contiguous raw history entries in sequence order.
+ * Conversation consumers retain the cheap wrapper; only Trajectory snapshots
+ * the entries and replays event order and request lifecycle state.
+ * @param loadEntries - Lazily snapshots contiguous raw entries in sequence order.
 * @returns Lazy, memoized inspection fields for that exact window.
 */
 export function createHistoryInspection(
-  entries: readonly HistoryEntry[],
+  loadEntries: () => readonly HistoryEntry[],
 ): SessionHistoryInspection {
+  let entries: readonly HistoryEntry[] | undefined
  let conversation: ReturnType<typeof projectConversationHistory> | undefined
  let requests: ReturnType<typeof inspectRequests> | undefined
+  const historyEntries = () => entries ??= loadEntries()
  const conversationProjection = () =>
-    conversation ??= projectConversationHistory(entries)
+    conversation ??= projectConversationHistory(historyEntries())
  const requestProjection = () =>
-    requests ??= inspectRequests(entries)
+    requests ??= inspectRequests(historyEntries())
  return {
    get eventNodes() {
      return conversationProjection().eventNodes
--- a/packages/client/runtime/src/client/sessions/session.ts
+++ b/packages/client/runtime/src/client/sessions/session.ts
@@ -111,13 +111,13 @@ export class Session implements ObservableSnapshot<ConversationSnapshot> {
  private codeDispatches = new Map<string, readonly CodeSubCall[]>()
  private dispatchesRev = 0
  private dispatchesCache: { rev: number; value: ReadonlyMap<string, readonly CodeSubCall[]> } | null = null
-  /** Raw history revision; published entries are copied so later live appends never mutate a prior snapshot. */
+  /** Raw history revision; inspection wrappers capture the exact array window and length. */
  private historyRev = 0
-  private historyEntriesCache: { rev: number; value: readonly HistoryEntry[] } | null = null
  private historyInspectionCache: {
    rev: number
    value: SessionHistoryInspection
  } | null = null
+  private loadOlderPromise: Promise<void> | null = null
  private running = false
  /**
   * Sticky send marker, private input of the composerPhase derivation: set
@@ -252,42 +252,56 @@ export class Session implements ObservableSnapshot<ConversationSnapshot> {
    return promise
  }

-  /** Page up: pull one earlier page with the window's first seq as beforeSeq and prepend (§D.2). */
-  async loadOlder(): Promise<void> {
-    if (this.openState !== 'open' || !this.hasMore || this.loadingOlder) return
+  /**
+   * Page up: pull one earlier page with the window's first seq as beforeSeq and prepend (§D.2).
+   * Concurrent callers share the active page so complete-history readers can continue afterward.
+   * @returns When the active or newly started page request settles.
+   */
+  loadOlder(): Promise<void> {
+    if (this.loadOlderPromise !== null) return this.loadOlderPromise
+    if (this.openState !== 'open' || !this.hasMore) return Promise.resolve()
    this.loadingOlder = true
    this.notifier.markDirty()
-    try {
-      const { result } = await this.api.sessions.history({
-        sessionId: this.sessionId, beforeSeq: this.baseSeq, maxMessages: PAGE_MESSAGES,
-      })
-      if (!result.ok) return // keep the window as-is; do not overwrite openError (open already succeeded)
-      const older = result.value.events
-      if (older.length === 0) {
+    const generation = this.openGeneration
+    const operation = (async () => {
+      try {
+        const { result } = await this.api.sessions.history({
+          sessionId: this.sessionId, beforeSeq: this.baseSeq, maxMessages: PAGE_MESSAGES,
+        })
+        if (generation !== this.openGeneration || this.openState !== 'open') return
+        if (!result.ok) return // keep the window as-is; do not overwrite openError (open already succeeded)
+        const older = result.value.events
+        if (older.length === 0) {
+          this.hasMore = result.value.hasMore
+          return
+        }
+        const tail = older[older.length - 1]
+        if (tail === undefined || tail.event.seq + 1 !== this.baseSeq) {
+          // §D.2 continuity assertion: on violation drop the page fail-soft rather than render an out-of-order stream.
+          console.error(`[web-runtime] history page discontinuous: tail seq ${tail?.event.seq} vs baseSeq ${this.baseSeq}`)
+          this.hasMore = false
+          return
+        }
+        this.events = [...older.map(e => e.event), ...this.events]
+        this.views = [...older.map(e => e.view), ...this.views]
+        this.historyRev++
+        /* v8 ignore next -- the ?? arm needs older[0] undefined, but the empty-page branch above already returned. */
+        this.baseSeq = older[0]?.event.seq ?? this.baseSeq
        this.hasMore = result.value.hasMore
-        return
+        this.foldAdapter.reset(this.events, this.baseSeq, this.views) // prepend forces a rebuild (sentinel count changed)
+        this.rebuildDerivedFromWindow()
+      } catch (error) {
+        console.error('[web-runtime] loadOlder failed:', error)
      }
-      const tail = older[older.length - 1]
-      if (tail === undefined || tail.event.seq + 1 !== this.baseSeq) {
-        // §D.2 continuity assertion: on violation drop the page fail-soft rather than render an out-of-order stream.
-        console.error(`[web-runtime] history page discontinuous: tail seq ${tail?.event.seq} vs baseSeq ${this.baseSeq}`)
-        this.hasMore = false
-        return
-      }
-      this.events = [...older.map(e => e.event), ...this.events]
-      this.views = [...older.map(e => e.view), ...this.views]
-      this.historyRev++
-      /* v8 ignore next -- the ?? arm needs older[0] undefined, but the empty-page branch above already returned. */
-      this.baseSeq = older[0]?.event.seq ?? this.baseSeq
-      this.hasMore = result.value.hasMore
-      this.foldAdapter.reset(this.events, this.baseSeq, this.views) // prepend forces a rebuild (sentinel count changed)
-      this.rebuildDerivedFromWindow()
-    } catch (error) {
-      console.error('[web-runtime] loadOlder failed:', error)
-    } finally {
+    })()
+    const settled = operation.finally(() => {
+      if (this.loadOlderPromise !== settled) return
+      this.loadOlderPromise = null
      this.loadingOlder = false
      this.notifier.markDirty()
-    }
+    })
+    this.loadOlderPromise = settled
+    return settled
  }

  /**
@@ -297,7 +311,7 @@ export class Session implements ObservableSnapshot<ConversationSnapshot> {
   * @returns When the available history has been exhausted or paging stops making progress.
   */
  async loadAllHistory(): Promise<void> {
-    while (this.openState === 'open' && this.hasMore && !this.loadingOlder) {
+    while (this.openState === 'open' && this.hasMore) {
      const previousBaseSeq = this.baseSeq
      await this.loadOlder()
      if (this.baseSeq === previousBaseSeq) return
@@ -844,24 +858,27 @@ export class Session implements ObservableSnapshot<ConversationSnapshot> {
    }
  }

-  /** Build the lazy history inspection wrapper without leaking mutable window arrays. */
+  /** Build a lazy inspection wrapper for the exact current history window. */
  private buildHistoryInspection(): SessionHistoryInspection {
-    if (this.historyEntriesCache === null || this.historyEntriesCache.rev !== this.historyRev) {
-      this.historyEntriesCache = {
-        rev: this.historyRev,
-        value: this.events.map((event, index) => {
-          const view = this.views[index]
-          return view === undefined ? { event } : { event, view }
-        }),
-      }
-    }
    if (
      this.historyInspectionCache === null
      || this.historyInspectionCache.rev !== this.historyRev
    ) {
+      const events = this.events
+      const views = this.views
+      const length = events.length
      this.historyInspectionCache = {
        rev: this.historyRev,
-        value: createHistoryInspection(this.historyEntriesCache.value),
+        value: createHistoryInspection(() =>
+          Array.from({ length }, (_, index) => {
+            const event = events[index]
+            if (event === undefined) {
+              throw new Error('captured history window changed before inspection')
+            }
+            const view = views[index]
+            return view === undefined ? { event } : { event, view }
+          }),
+        ),
      }
    }
    return this.historyInspectionCache.value
--- a/packages/client/runtime/tests/session.spec.ts
+++ b/packages/client/runtime/tests/session.spec.ts
@@ -124,6 +124,21 @@ describe('live event path', () => {
    expect((last as { interrupted?: true }).interrupted).toBeUndefined()
  })

+  it('keeps a lazily inspected snapshot pinned to its original history window', async () => {
+    const { session } = await opened()
+    const before = session.getSnapshot()
+
+    session.handleMuxEnvelope('r' as never, {
+      type: 'session/event',
+      sessionId: SID,
+      event: ev.user(6, 'later'),
+    })
+
+    expect(before.inspection?.eventNodes.map(node => node.seq)).toEqual([1, 3])
+    expect(session.getSnapshot().inspection?.eventNodes.map(node => node.seq))
+      .toEqual([1, 3, 6])
+  })
+
  it('freezes an unfinalized partial into an interrupted node on turn/end (cancel path)', async () => {
    const { session } = await opened()
    const feed = (event: SessionEvent) => { session.handleMuxEnvelope('r' as never, { type: 'session/event', sessionId: SID, event }) }
@@ -292,6 +307,36 @@ describe('paging', () => {
    expect(session.getSnapshot().hasMore).toBe(true)
  })

+  it('continues complete-history loading after an already active page', async () => {
+    const pages = [
+      plainTurn(0, 0, '最早问', '最早答'),
+      plainTurn(6, 1, '中间问', '中间答'),
+      plainTurn(12, 2, '最新问', '最新答'),
+    ]
+    const middle = deferred<Awaited<ReturnType<FakeApiClient['onHistory']>>>()
+    const { api, session } = makeSession()
+    api.onHistory = (payload) => {
+      if (payload.beforeSeq === undefined) return histResponse(pages[2]!, true)
+      if (payload.beforeSeq === 12) return middle.promise
+      return histResponse(pages[0]!, false)
+    }
+
+    await session.open()
+    const activePage = session.loadOlder()
+    const completeHistory = session.loadAllHistory()
+    middle.resolve(ok({
+      events: entries(pages[1]!) as never[],
+      hasMore: true,
+      modelTarget: { provider: 'deepseek', model: 'deepseek-v4-flash' },
+    }))
+    await Promise.all([activePage, completeHistory])
+
+    expect(api.callsOf('session.history')).toHaveLength(3)
+    expect(session.getSnapshot().hasMore).toBe(false)
+    expect(session.getSnapshot().nodes.map(node => node.seq))
+      .toEqual([1, 3, 7, 9, 13, 15])
+  })
+
  it('drops a discontinuous older page fail-soft (window unchanged, hasMore cleared)', async () => {
    const { api, session } = makeSession()
    api.onHistory = payload => payload.beforeSeq === undefined
--- a/packages/client/ui-primitives/src/JsonTree.tsx
+++ b/packages/client/ui-primitives/src/JsonTree.tsx
@@ -289,6 +289,7 @@ function JsonTreeNode({
      />
      <NodeField field={field} expandable onToggle={toggle} />
      <span className={css.preview}>{previewValue(value, 0)}</span>
+      {!lastElement && <span className={css.punctuation}>,</span>}
      {expanded && (
        <ul id={contentsId} role="group" className={css.children}>
          {entries.map(([key, item], index) => (
@@ -524,7 +525,7 @@ export function JsonTree({
                  field={key}
                  value={value}
                  path={[Array.isArray(data) ? index : key]}
-                  lastElement
+                  lastElement={index === rootEntries.length - 1}
                  initialExpanded={false}
                  tabStopId={tabStopId}
                  onClaimTabStop={setTabStopId}
--- a/packages/client/ui-primitives/tests/json-tree.spec.tsx
+++ b/packages/client/ui-primitives/tests/json-tree.spec.tsx
@@ -34,7 +34,7 @@ describe('JsonTree', () => {
    const tree = screen.getByRole('tree', { name: 'Payload' })
    const rows = within(tree).getAllByRole('treeitem')
    expect(rows).toHaveLength(2)
-    expect(rows[0]?.textContent).toBe('nested:{answer: 42}')
+    expect(rows[0]?.textContent).toBe('nested:{answer: 42},')
    expect(rows[1]?.textContent).toBe('list:["alpha", "beta"]')

    const expanders = within(tree).getAllByRole('button', { name: 'Expand JSON node' })
--- a/packages/client/ui-trajectory/src/client/TrajectoryTable.tsx
+++ b/packages/client/ui-trajectory/src/client/TrajectoryTable.tsx
@@ -1188,6 +1188,20 @@ function RecordPayload({
    : 'No result captured'
  if (!value) return <p className={css.noPayload}>{missing}</p>

+  const json = parseJsonContainer(value)
+  const singleTextResult = direction === 'output'
+    && record.cell.outputBlocks?.length === 1
+    && record.cell.outputBlocks[0]?.type === 'text'
+  if (singleTextResult && json !== undefined) {
+    return (
+      <JsonTree
+        data={json}
+        label="Result JSON"
+        className={preview ? css.jsonPreview : css.jsonPayload}
+      />
+    )
+  }
+
  if (
    direction === 'output'
    && record.cell.outputBlocks?.some(block =>
@@ -1214,7 +1228,6 @@ function RecordPayload({
      </div>
    )
  }
-  const json = parseJsonContainer(value)
  if (json !== undefined) {
    return (
      <JsonTree
--- a/packages/client/ui-trajectory/src/client/TrajectoryView.tsx
+++ b/packages/client/ui-trajectory/src/client/TrajectoryView.tsx
@@ -5,7 +5,9 @@ import type { ConvViewProps } from '@deepseek-ai/dsh-client-ui-conversation/clie
 import type {
  AssistantMessageNode, ConversationContext, RequestView,
 } from '@deepseek-ai/dsh-client-runtime/client'
-import { deriveTrajectoryContextBranches } from './context-branches.ts'
+import {
+  deriveTrajectoryContextBranches, trajectoryBranchContainsSeq,
+} from './context-branches.ts'
 import {
  TrajectoryTable,
  type TrajectoryRequestNumber,
@@ -97,9 +99,13 @@ export function TrajectoryView({ useSession, loadAllHistory }: ConvViewProps & T
  )
  const currentBranch = branches.at(-1)
  if (currentBranch === undefined) throw new Error('trajectory branch projection must not be empty')
-  const selectedNodes = inspection === undefined || inspection.eventNodes.length === 0
-    ? nodes
-    : inspection.eventNodes
+  const selectedNodes = currentBranch.nodes
+  const selectedRequests = useMemo(
+    () => requests.filter(request =>
+      trajectoryBranchContainsSeq(currentBranch, request.startSeq),
+    ),
+    [currentBranch, requests],
+  )
  const globalRequestNumbers = useMemo<readonly TrajectoryRequestNumber[]>(() => {
    const assistantsByStep = new Map<string, AssistantMessageNode>()
    for (const context of contexts) {
@@ -235,12 +241,12 @@ export function TrajectoryView({ useSession, loadAllHistory }: ConvViewProps & T
      nodes: selectedNodes,
      partial,
      runningCalls,
-      requests,
+      requests: selectedRequests,
      ...(callSchemas === undefined ? {} : { callSchemas }),
      codeDispatches,
    }),
    [
-      selectedNodes, partial, runningCalls, requests, callSchemas, codeDispatches,
+      selectedNodes, partial, runningCalls, selectedRequests, callSchemas, codeDispatches,
    ],
  )
  const collapsibleTurnIds = useMemo(
--- a/packages/client/ui-trajectory/tests/table.spec.tsx
+++ b/packages/client/ui-trajectory/tests/table.spec.tsx
@@ -96,6 +96,33 @@ describe('TrajectoryTable', () => {
    expect(screen.getByText('ToolError: non_zero_exit')).toBeTruthy()
  })

+  it('renders a single-text JSON tool result as a JSON tree', () => {
+    const turns: readonly TrajectoryTurnModel[] = [{
+      turn: 1,
+      groups: [{
+        title: 'Step 1',
+        cells: [{
+          index: 1,
+          kind: 'tool',
+          text: 'read {"path":"result.json"}',
+          outputDetail: '{"value":1,"nested":{"ok":true}}',
+          outputBlocks: [{
+            type: 'text',
+            content: '{"value":1,"nested":{"ok":true}}',
+          }],
+          timeSeconds: 0.1,
+        }],
+      }],
+    }]
+
+    render(<TrajectoryTable turns={turns} {...FOLD_PROPS} />)
+    fireEvent.click(screen.getByRole('row', { name: /TOOL/ }))
+    fireEvent.click(screen.getByRole('tab', { name: 'Result' }))
+
+    expect(screen.getByRole('tree', { name: 'Result JSON' })).toBeTruthy()
+    expect(screen.getByText('value:')).toBeTruthy()
+  })
+
  it('keeps the first row and a compact summary when a turn is collapsed', () => {
    render(
      <TrajectoryTable
--- a/packages/client/ui-trajectory/tests/views.spec.tsx
+++ b/packages/client/ui-trajectory/tests/views.spec.tsx
@@ -16,7 +16,7 @@ import { createSnapshotStore } from '@deepseek-ai/dsh-client-runtime/client'
 import type { UseSession } from '@deepseek-ai/dsh-client-web-react'
 import { SlotsService } from '@deepseek-ai/dsh-client-runtime/client'
 import type {
-  ConversationSnapshot, SessionId, SessionListState, WorkspaceListState,
+  ConversationSnapshot, RequestView, SessionId, SessionListState, WorkspaceListState,
 } from '@deepseek-ai/dsh-client-runtime/client'
 import type { ConvViewProps, ViewTab } from '@deepseek-ai/dsh-client-ui-conversation/client'
 // Export discipline: packages/client/AGENTS.md.
@@ -270,6 +270,79 @@ describe('span derivation', () => {
  })
 })

+describe('TrajectoryView branches', () => {
+  it('renders only the selected rewind branch while retaining session-global requests', () => {
+    const retained = {
+      kind: 'user',
+      seq: 1,
+      time: 1_000,
+      content: [{ type: 'text', text: 'retained user' }],
+      source: null,
+    } as unknown as ConversationSnapshot['nodes'][number]
+    const abandoned = {
+      kind: 'assistant',
+      seq: 3,
+      time: 3_000,
+      turn: 1,
+      step: 1,
+      blocks: [{ kind: 'text', text: 'abandoned response' }],
+    } as unknown as ConversationSnapshot['nodes'][number]
+    const current = {
+      kind: 'assistant',
+      seq: 5,
+      time: 5_000,
+      turn: 2,
+      step: 1,
+      blocks: [{ kind: 'text', text: 'current response' }],
+    } as unknown as ConversationSnapshot['nodes'][number]
+    const request = (startSeq: number, turn: number): RequestView => ({
+      purpose: 'assistant',
+      startSeq,
+      turn,
+      step: 1,
+      startedAt: startSeq * 1_000,
+      completedAt: startSeq * 1_000 + 100,
+      status: 'complete',
+    })
+    const store = createSnapshotStore({
+      nodes: [retained, current],
+      inspection: {
+        eventNodes: [retained, abandoned, current],
+        contexts: [
+          { id: 0, nodes: [retained, abandoned] },
+          {
+            id: 1,
+            parentId: 0,
+            origin: 'rewind' as const,
+            originSeq: 4,
+            nodes: [retained, current],
+          },
+        ],
+        requests: [request(2, 1), request(4, 2)],
+        callSchemas: new Map(),
+      },
+      openState: 'open' as const,
+      hasMore: false,
+      partial: null,
+      runningCalls: [] as ConversationSnapshot['runningCalls'],
+      codeDispatches: new Map(),
+    })
+
+    const view = render(
+      <TrajectoryView
+        {...standaloneProps([])}
+        useSession={bindSnapshotSelector(store) as unknown as UseSession<ConversationSnapshot>}
+        loadAllHistory={vi.fn(() => Promise.resolve())}
+      />,
+    )
+
+    expect(screen.queryByText('abandoned response')).toBeNull()
+    expect(screen.getByText('current response')).toBeTruthy()
+    expect(screen.getByRole('row', { name: /Request 2, ASSISTANT/ })).toBeTruthy()
+    expect(view.container.querySelectorAll('[data-request-only="true"]')).toHaveLength(0)
+  })
+})
+
 describe('WaterfallView standalone branches', () => {
  it('empty window renders the placeholder copy', () => {
    render(createElement(WaterfallView as FC<ConvViewProps>,