1
0
Эх сурвалжийг харах

refactor(client): derive deliverables from mutation calls

imccyu 1 сар өмнө
parent
commit
a99516c330

+ 2 - 2
packages/client/ui-deliverables/src/client/index.ts

@@ -2,8 +2,8 @@
  * Deliverables plugin, browser half: registers the produced-files row into
  * the chat view's turn-tail chain, and provides the `chatFileMentions`
  * service that links inline-code mentions of produced files in the closing
- * prose. All policy lives here — the derivation from the mutation tools'
- * `locations`, the mention matching, the chip cap, and the copy — so
+ * prose. All policy lives here — the supported mutation calls, mention
+ * matching, chip cap, and copy — so
  * composing this plugin out of cordis.yml removes both surfaces entirely;
  * the owning view renders an empty chain and inert prose at zero cost.
  */

+ 81 - 30
packages/client/ui-deliverables/src/client/turn-deliverables.ts

@@ -1,10 +1,10 @@
 /**
  * Turn-scoped produced-file Definition and readers. Client-only and
- * model-free: the vocabulary is the mutation tools' own follow-along
- * `locations`, never the closing prose.
+ * model-free: the vocabulary comes from successful first-party mutation
+ * calls, never presentation data or the closing prose.
  */
 import { isAppendSurfaceEvent } from '@deepseek-ai/dsh-session/surface'
-import type { ToolResultNode, TurnTailOwnerProps } from '@deepseek-ai/dsh-client-ui-chat/client'
+import type { TurnTailOwnerProps } from '@deepseek-ai/dsh-client-ui-chat/client'
 import type { ConversationNodeDefinition } from '@deepseek-ai/dsh-client-ui-conversation/client'
 import type { MarkdownFileMentions } from '@deepseek-ai/dsh-client-ui-primitives'
 
@@ -27,38 +27,90 @@ declare module '@deepseek-ai/dsh-client-ui-conversation/client' {
 
 interface DeliverablesState extends DeliverablesTurnData {
   readonly turn: number
-  readonly calls: ReadonlyMap<string, ToolResultNode['callView']>
+  readonly calls: ReadonlyMap<string, string | null>
 }
 
 /**
- * Paths a call view reports having created or changed, by render intent rather
- * than tool name: a diff card, or a generic card whose kind is `edit` (the
- * shape `str_replace_editor`'s insert presents). Every other card produces
- * nothing to open — a read looked, a delete removed, a terminal ran. Only
- * root call views enter this Turn accumulator; nested Code Mode dispatches
- * preserve the pre-assembly behavior and do not contribute independently.
+ * Extract the path from a supported first-party mutation call. Session
+ * `tool/call` events are root calls; Code Dispatch children do not enter this
+ * Definition independently.
+ * @param name - wire tool name.
+ * @param argsRaw - model-produced JSON arguments.
+ * @returns the mutation path, or null when the call is not a supported mutation.
  */
-function producedPaths(view: ToolResultNode['callView']): readonly string[] {
-  if (view === null) return []
-  if (view.card === 'diff') return (view.locations ?? []).map(location => location.path)
-  if (view.card === 'generic' && view.kind === 'edit') {
-    return (view.locations ?? []).map(location => location.path)
+function mutationPath(name: string, argsRaw: string): string | null {
+  let args: unknown
+  try {
+    args = JSON.parse(argsRaw) as unknown
+  } catch {
+    return null
+  }
+  if (!isRecord(args)) return null
+  switch (name) {
+    case 'write':
+      return typeof args.content === 'string' ? pathValue(args.file_path) : null
+    case 'edit':
+      return validEditArgs(args) ? pathValue(args.file_path) : null
+    case 'str_replace_editor':
+      return editorMutationPath(args)
+    default:
+      return null
   }
-  return []
+}
+
+/** Validate the fields that an `edit` execution requires. */
+function validEditArgs(args: Readonly<Record<string, unknown>>): boolean {
+  return typeof args.old_string === 'string'
+    && args.old_string.length > 0
+    && typeof args.new_string === 'string'
+    && args.old_string !== args.new_string
+    && (args.replace_all === undefined || typeof args.replace_all === 'boolean')
+}
+
+/** Extract a path only from a complete mutating editor command. */
+function editorMutationPath(args: Readonly<Record<string, unknown>>): string | null {
+  const path = pathValue(args.path)
+  if (path === null) return null
+  switch (args.command) {
+    case 'create':
+      return typeof args.file_text === 'string' ? path : null
+    case 'str_replace':
+      return typeof args.old_str === 'string'
+        && args.old_str.length > 0
+        && (args.new_str === undefined || typeof args.new_str === 'string')
+        ? path
+        : null
+    case 'insert':
+      return typeof args.insert_line === 'number'
+        && Number.isInteger(args.insert_line)
+        && args.insert_line >= 0
+        && typeof args.new_str === 'string'
+        ? path
+        : null
+    default:
+      return null
+  }
+}
+
+/** A non-blank path preserves the exact spelling supplied to the tool. */
+function pathValue(value: unknown): string | null {
+  return typeof value === 'string' && value.trim().length > 0 ? value : null
+}
+
+/** Narrow parsed JSON to an argument object. */
+function isRecord(value: unknown): value is Record<string, unknown> {
+  return typeof value === 'object' && value !== null && !Array.isArray(value)
 }
 
 /**
  * Files produced by one Turn data value.
  *
- * The source is the mutation tools' own follow-along `locations`, not the
- * closing prose: a produced file must be listed whether or not the model
- * remembered to name it. A mutation is recognized by render intent, not by
- * tool name — a diff card, or a generic card whose `kind` is `edit` (the shape
- * `str_replace_editor`'s insert presents) — so a new mutation tool joins by
- * declaring what it does. Reads contribute nothing (looking at a file does not
- * produce it), and neither do deletes (there is nothing left to open) or
- * failed calls. Paths keep first-seen order and appear once, so a file written
- * and then edited in the same turn is one entry.
+ * The source is the arguments of successful `write`, `edit`, and mutating
+ * `str_replace_editor` calls, not the closing prose: a produced file must be
+ * listed whether or not the model remembered to name it. Reads, unsupported
+ * tools, malformed calls, and failed results contribute nothing. Paths keep
+ * first-seen order and appear once, so a file written and then edited in the
+ * same turn is one entry.
  *
  * The Conversation Location index owns turn membership before this function
  * runs, so paths cannot spill across turns and this derivation does not infer
@@ -112,7 +164,7 @@ export const deliverablesDefinition: ConversationNodeDefinition<DeliverablesStat
       const calls = new Map(context.state.calls)
       calls.set(
         String(match.event.data.callId),
-        match.view?.for === 'call' ? match.view.view : null,
+        mutationPath(match.event.data.name, match.event.data.arguments),
       )
       return { ...context.state, calls }
     }
@@ -120,11 +172,10 @@ export const deliverablesDefinition: ConversationNodeDefinition<DeliverablesStat
     const result = match.event.data.message.content[0]
     if (result.isError === true) return context.state
     const callId = String(match.event.data.message.source.callId)
-    const additions = producedPaths(context.state.calls.get(callId) ?? null)
-      .map(path => ({ seq: match.event.seq, path }))
-    return additions.length === 0
+    const path = context.state.calls.get(callId)
+    return path === null || path === undefined
       ? context.state
-      : { ...context.state, produced: [...context.state.produced, ...additions] }
+      : { ...context.state, produced: [...context.state.produced, { seq: match.event.seq, path }] }
   },
   buildLocationData: (context, scope) => scope !== 'turn' || context.state === undefined
     ? null

+ 149 - 41
packages/client/ui-deliverables/tests/produced-files.client.spec.tsx

@@ -109,14 +109,12 @@ function at(
   seq: number,
   type: string,
   data: unknown,
-  view?: ConversationEventInput['view'],
 ): ConversationEventInput {
   return {
     event: {
       seq, time: seq * 1_000, type, data,
       ...(type === 'tool/result' ? { surfaceOp: 'append' } : {}),
     } as ConversationEventInput['event'],
-    ...(view === undefined ? {} : { view }),
   }
 }
 
@@ -124,19 +122,27 @@ function matched(input: ConversationEventInput, role: ConversationMatch['role'])
   return { ...input, role, location: { kind: 'unresolved' } }
 }
 
-type WireCallView = Extract<NonNullable<ConversationEventInput['view']>, { for: 'call' }>['view']
-
 function call(
   seq: number,
   callId: string,
-  view: WireCallView | null,
+  name: string,
+  args: Readonly<Record<string, unknown>>,
+  turn = 1,
+): ConversationEventInput {
+  return rawCall(seq, callId, name, JSON.stringify(args), turn)
+}
+
+function rawCall(
+  seq: number,
+  callId: string,
+  name: string,
+  argsRaw: string,
   turn = 1,
 ): ConversationEventInput {
   return at(
     seq,
     'tool/call',
-    { turn, step: 1, callId, name: 'fixture', arguments: '{}' },
-    { for: 'call', view: view ?? { card: 'generic', title: 'fixture' } },
+    { turn, step: 1, callId, name, arguments: argsRaw },
   )
 }
 
@@ -151,18 +157,6 @@ function result(seq: number, callId: string, isError = false, turn = 1): Convers
   })
 }
 
-function diff(...paths: string[]): WireCallView {
-  return {
-    card: 'diff', title: `Write ${paths[0] ?? ''}`,
-    diffs: paths.map(path => ({ path, oldText: null, newText: 'x' })),
-    locations: paths.map(path => ({ path })),
-  }
-}
-
-function edit(path: string): WireCallView {
-  return { card: 'generic', title: `insert ${path}`, kind: 'edit', locations: [{ path }] }
-}
-
 function assembler(entries: readonly ConversationEventInput[], hasMore = false): ConversationNodeAssembler {
   const value = new ConversationNodeAssembler(new TestEventDefinitions(), new TestViewDefinitions())
   value.replaceWindow(entries, hasMore)
@@ -189,36 +183,148 @@ describe('produced-file Turn data', () => {
     expect(selectProducedFiles(tailOwner(undefined, 9, () => {}, 2))).toBeNull()
   })
 
-  it('folds successful diff and generic-edit calls while ignoring reads, failures, and missing locations', () => {
+  it('folds successful first-party mutation paths from their raw arguments', () => {
     const value = assembler([
       at(1, 'turn/start', { turn: 1 }),
-      call(2, 'write', diff('out/index.html', 'out/app.css')),
+      call(2, 'write', 'write', {
+        file_path: 'out/index.html', path: 'wrong-write.txt', content: '<html></html>',
+      }),
       result(3, 'write'),
-      call(4, 'edit', edit('notes.md')),
+      call(4, 'edit', 'edit', {
+        file_path: 'out/app.css', path: 'wrong-edit.txt', old_string: 'red', new_string: 'blue',
+        replace_all: false,
+      }),
       result(5, 'edit'),
-      call(6, 'read', { card: 'generic', title: 'Read', locations: [{ path: 'input.txt' }] }),
-      result(7, 'read'),
-      call(8, 'failed', diff('broken.txt')),
-      result(9, 'failed', true),
-      call(10, 'locationless', { card: 'diff', title: 'Write', diffs: [] }),
-      result(11, 'locationless'),
+      call(6, 'create', 'str_replace_editor', {
+        command: 'create', path: 'notes/new.md', file_path: 'wrong-create.txt', file_text: 'new',
+      }),
+      result(7, 'create'),
+      call(8, 'replace', 'str_replace_editor', {
+        command: 'str_replace', path: 'notes/existing.md', old_str: 'old', new_str: 'new',
+      }),
+      result(9, 'replace'),
+      call(10, 'delete-text', 'str_replace_editor', {
+        command: 'str_replace', path: 'notes/deleted-text.md', old_str: 'remove me',
+      }),
+      result(11, 'delete-text'),
+      call(12, 'insert', 'str_replace_editor', {
+        command: 'insert', path: 'notes/inserted.md', insert_line: 1, new_str: 'line',
+      }),
+      result(13, 'insert'),
     ])
 
     expect(producedForClosing(deliverablesOf(value))).toEqual([
-      'out/index.html', 'out/app.css', 'notes.md',
+      'out/index.html',
+      'out/app.css',
+      'notes/new.md',
+      'notes/existing.md',
+      'notes/deleted-text.md',
+      'notes/inserted.md',
+    ])
+  })
+
+  it.each([
+    { caseName: 'write omits content', name: 'write', args: { file_path: 'write.txt' } },
+    { caseName: 'write has non-string content', name: 'write', args: { file_path: 'write.txt', content: 1 } },
+    {
+      caseName: 'edit omits old_string', name: 'edit',
+      args: { file_path: 'edit.txt', new_string: 'new' },
+    },
+    {
+      caseName: 'edit has an empty old_string', name: 'edit',
+      args: { file_path: 'edit.txt', old_string: '', new_string: 'new' },
+    },
+    {
+      caseName: 'edit omits new_string', name: 'edit',
+      args: { file_path: 'edit.txt', old_string: 'old' },
+    },
+    {
+      caseName: 'edit does not change the string', name: 'edit',
+      args: { file_path: 'edit.txt', old_string: 'same', new_string: 'same' },
+    },
+    {
+      caseName: 'edit has a non-boolean replace_all', name: 'edit',
+      args: { file_path: 'edit.txt', old_string: 'old', new_string: 'new', replace_all: 'yes' },
+    },
+    {
+      caseName: 'editor create omits file_text', name: 'str_replace_editor',
+      args: { command: 'create', path: 'create.txt' },
+    },
+    {
+      caseName: 'editor create has non-string file_text', name: 'str_replace_editor',
+      args: { command: 'create', path: 'create.txt', file_text: 1 },
+    },
+    {
+      caseName: 'editor replace omits old_str', name: 'str_replace_editor',
+      args: { command: 'str_replace', path: 'replace.txt', new_str: 'new' },
+    },
+    {
+      caseName: 'editor replace has an empty old_str', name: 'str_replace_editor',
+      args: { command: 'str_replace', path: 'replace.txt', old_str: '' },
+    },
+    {
+      caseName: 'editor replace has non-string new_str', name: 'str_replace_editor',
+      args: { command: 'str_replace', path: 'replace.txt', old_str: 'old', new_str: 1 },
+    },
+    {
+      caseName: 'editor insert omits insert_line', name: 'str_replace_editor',
+      args: { command: 'insert', path: 'insert.txt', new_str: 'new' },
+    },
+    {
+      caseName: 'editor insert has a fractional insert_line', name: 'str_replace_editor',
+      args: { command: 'insert', path: 'insert.txt', insert_line: 1.5, new_str: 'new' },
+    },
+    {
+      caseName: 'editor insert has a negative insert_line', name: 'str_replace_editor',
+      args: { command: 'insert', path: 'insert.txt', insert_line: -1, new_str: 'new' },
+    },
+    {
+      caseName: 'editor insert omits new_str', name: 'str_replace_editor',
+      args: { command: 'insert', path: 'insert.txt', insert_line: 1 },
+    },
+  ])('ignores a successful result when $caseName', ({ name, args }) => {
+    const value = assembler([
+      at(1, 'turn/start', { turn: 1 }),
+      call(2, 'malformed', name, args),
+      result(3, 'malformed'),
     ])
+
+    expect(producedForClosing(deliverablesOf(value))).toEqual([])
   })
 
-  it('ignores calls without mutation locations, orphan results, and replacement results', () => {
-    const replacement = result(8, 'replacement')
+  it('ignores editor views, unsupported tools, failures, interruptions, malformed calls, and orphan results', () => {
+    const replacement = result(25, 'replacement')
     const value = assembler([
       at(1, 'turn/start', { turn: 1 }),
-      at(2, 'tool/call', { turn: 1, step: 1, callId: 'no-view', name: 'fixture', arguments: '{}' }),
-      result(3, 'no-view'),
-      call(4, 'locationless-edit', { card: 'generic', title: 'Edit', kind: 'edit' }),
-      result(5, 'locationless-edit'),
-      result(6, 'orphan'),
-      call(7, 'replacement', diff('replaced.txt')),
+      call(2, 'view', 'str_replace_editor', { command: 'view', path: 'viewed.txt' }),
+      result(3, 'view'),
+      call(4, 'read', 'read', { file_path: 'input.txt' }),
+      result(5, 'read'),
+      call(6, 'unknown', 'custom_edit', { file_path: 'custom.txt', path: 'custom.txt' }),
+      result(7, 'unknown'),
+      call(8, 'failed', 'write', { file_path: 'failed.txt', content: 'x' }),
+      result(9, 'failed', true),
+      call(10, 'interrupted', 'edit', {
+        file_path: 'interrupted.txt', old_string: 'old', new_string: 'new',
+      }),
+      rawCall(11, 'invalid-json', 'write', '{'),
+      result(12, 'invalid-json'),
+      rawCall(13, 'null-args', 'write', 'null'),
+      result(14, 'null-args'),
+      rawCall(15, 'array-args', 'edit', '[]'),
+      result(16, 'array-args'),
+      call(17, 'missing-path', 'write', { content: 'x' }),
+      result(18, 'missing-path'),
+      call(19, 'blank-path', 'edit', {
+        file_path: '   ', old_string: 'old', new_string: 'new',
+      }),
+      result(20, 'blank-path'),
+      call(21, 'missing-editor-path', 'str_replace_editor', { command: 'create', file_text: 'x' }),
+      result(22, 'missing-editor-path'),
+      result(23, 'orphan'),
+      call(24, 'replacement', 'str_replace_editor', {
+        command: 'insert', path: 'replaced.txt', insert_line: 0, new_str: 'new',
+      }),
       {
         ...replacement,
         event: {
@@ -226,7 +332,7 @@ describe('produced-file Turn data', () => {
           surfaceOp: { op: 'replace', start: 1, end: 1 },
         } as ConversationEventInput['event'],
       },
-      at(9, 'turn/end', { turn: 1, reason: { kind: 'completed' } }),
+      at(26, 'turn/end', { turn: 1, reason: { kind: 'interrupted' } }),
     ])
 
     expect(producedForClosing(deliverablesOf(value))).toEqual([])
@@ -255,7 +361,7 @@ describe('produced-file Turn data', () => {
 
   it('replays a tail page once prepend supplies its missing Turn start', () => {
     const value = assembler([
-      call(10, 'late', diff('history.txt')),
+      call(10, 'late', 'write', { file_path: 'history.txt', content: 'history' }),
       result(11, 'late'),
     ], true)
     expect(deliverablesOf(value)).toBeUndefined()
@@ -268,13 +374,15 @@ describe('produced-file Turn data', () => {
   it('extends the same Turn data incrementally on live append', () => {
     const value = assembler([
       at(1, 'turn/start', { turn: 1 }),
-      call(2, 'first', diff('first.txt')),
+      call(2, 'first', 'write', { file_path: 'first.txt', content: 'first' }),
       result(3, 'first'),
     ])
     const first = deliverablesOf(value)
     expect(producedForClosing(first)).toEqual(['first.txt'])
 
-    value.append(call(4, 'second', diff('second.txt')))
+    value.append(call(4, 'second', 'edit', {
+      file_path: 'second.txt', old_string: 'before', new_string: 'after',
+    }))
     value.append(result(5, 'second'))
     value.flush()
     expect(producedForClosing(deliverablesOf(value))).toEqual(['first.txt', 'second.txt'])