diff --git a/packages/client/ui-deliverables/src/client/index.ts b/packages/client/ui-deliverables/src/client/index.ts index 879ed682e4..0e8767016e 100644 --- a/packages/client/ui-deliverables/src/client/index.ts +++ b/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. */ diff --git a/packages/client/ui-deliverables/src/client/turn-deliverables.ts b/packages/client/ui-deliverables/src/client/turn-deliverables.ts index 604ab099f1..b7f74f305b 100644 --- a/packages/client/ui-deliverables/src/client/turn-deliverables.ts +++ b/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 + readonly calls: ReadonlyMap } /** - * 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 } - return [] + 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 + } +} + +/** Validate the fields that an `edit` execution requires. */ +function validEditArgs(args: Readonly>): 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>): 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 { + 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 ({ 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 diff --git a/packages/client/ui-deliverables/tests/produced-files.client.spec.tsx b/packages/client/ui-deliverables/tests/produced-files.client.spec.tsx index 7aef3b3580..4cbfe8c294 100644 --- a/packages/client/ui-deliverables/tests/produced-files.client.spec.tsx +++ b/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, { for: 'call' }>['view'] - function call( seq: number, callId: string, - view: WireCallView | null, + name: string, + args: Readonly>, + 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: '', + }), 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('ignores calls without mutation locations, orphan results, and replacement results', () => { - const replacement = result(8, 'replacement') + 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 }), - 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, 'malformed', name, args), + result(3, 'malformed'), + ]) + + expect(producedForClosing(deliverablesOf(value))).toEqual([]) + }) + + 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 }), + 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'])