From fa8938d202bbd3b5d9a4f2f9f1633f27ecc2960b Mon Sep 17 00:00:00 2001 From: creatixchu Date: Mon, 14 Sep 2026 13:47:52 +0800 Subject: [PATCH] refactor(deliverables): simplify path helpers, hunk classification, and card status Path helpers reuse isInside instead of re-deriving containment from relative(); the recorder classifies uncovered file-tool hunks in one pass over the canonical map; the card renders line counts through one component and derives header and row gesture copy from the same phase reader. --- .../src/client/ChangedFiles.tsx | 60 +++++++++---------- .../workspace-changes/src/paths.ts | 19 +----- .../workspace-changes/src/recorder.ts | 40 ++++++------- 3 files changed, 47 insertions(+), 72 deletions(-) diff --git a/packages/client/ui-deliverables/src/client/ChangedFiles.tsx b/packages/client/ui-deliverables/src/client/ChangedFiles.tsx index 6ab9cd7286..6c4c5f62c2 100644 --- a/packages/client/ui-deliverables/src/client/ChangedFiles.tsx +++ b/packages/client/ui-deliverables/src/client/ChangedFiles.tsx @@ -4,7 +4,6 @@ import { resolveWorkspacePath } from '@deepseek-ai/dsh-util-workspace-path' import { IconChevronDownOutline14, IconChevronUpOutline14 } from '@deepseek-ai/dsh-client-ui-primitives' import type { PropsLocale } from '@deepseek-ai/dsh-client-ui-slots' import type { SessionId } from '@deepseek-ai/dsh-session/types' -import type { WorkspaceChangedFile } from '@deepseek-ai/dsh-workspace-changes/types' import { changedFileUrl } from '../changes.ts' import type { PresentedHost } from '../presented.ts' import { IconCodeBracketsOutline16 } from './icons.tsx' @@ -19,27 +18,29 @@ const COLLAPSED_ROWS = 3 const GROUPED = new Intl.NumberFormat('en-US') /** - * Row copy for an open gesture while it is pending or failed; a completed open - * shows the line counts again, which are the row's primary information. The - * changed-files card never reveals, so only open phases occur. + * Gesture state worth showing in place of the counts: pending, or failed. A + * completed open shows the counts again. The card never reveals, so only open + * phases occur. */ -function rowStatus(phase: PresentedOpenPhase | undefined): { key: 'presented.opening' | 'presented.error' | 'presented.nativeUnavailable'; error: boolean } | undefined { +function gesture(phase: PresentedOpenPhase | undefined): { key: 'presented.opening' | 'presented.error' | 'presented.nativeUnavailable'; failed: boolean } | undefined { switch (phase) { - case 'opening': return { key: 'presented.opening', error: false } - case 'error': return { key: 'presented.error', error: true } - case 'nativeUnavailable': return { key: 'presented.nativeUnavailable', error: true } + case 'opening': return { key: 'presented.opening', failed: false } + case 'error': return { key: 'presented.error', failed: true } + case 'nativeUnavailable': return { key: 'presented.nativeUnavailable', failed: true } default: return undefined } } -function totals(files: readonly WorkspaceChangedFile[]): { added: number; deleted: number } { - let added = 0 - let deleted = 0 - for (const file of files) { - added += file.added - deleted += file.deleted - } - return { added, deleted } +function sum(files: readonly { added: number; deleted: number }[], key: 'added' | 'deleted'): number { + return files.reduce((total, file) => total + file[key], 0) +} + +/** Added and deleted line counts in the card's colors. */ +function Counts({ added, deleted, t }: { added: number; deleted: number } & PropsLocale) { + return <> + {t('changes.added', { count: GROUPED.format(added) })} + {t('changes.deleted', { count: GROUPED.format(deleted) })} + } /** @@ -62,46 +63,39 @@ export function ChangedFiles({ changes, cwd, sessionId, host, phases, onOpen, op const native = host !== null && host.available const foldable = changes.files.length > COLLAPSED_ROWS const rows = foldable && !expanded ? changes.files.slice(0, COLLAPSED_ROWS) : changes.files - const sum = totals(changes.files) - const folderPhase = phases[changedFileUrl(sessionId, changes.seq, null)] - const folderStatus = folderPhase === 'opening' ? t('changes.folderOpening') - : folderPhase === 'error' || folderPhase === 'nativeUnavailable' ? t('changes.folderError') : undefined + const folder = gesture(phases[changedFileUrl(sessionId, changes.seq, null)]) const summary = <> {t('changes.title', { count: String(changes.total) })} - - {folderStatus ?? <> - {t('changes.added', { count: GROUPED.format(sum.added) })} - {t('changes.deleted', { count: GROUPED.format(sum.deleted) })} - } + + {folder === undefined + ? + : t(folder.failed ? 'changes.folderError' : 'changes.folderOpening')} return
{native ? + disabled={folder !== undefined && !folder.failed} onClick={() => { onOpen(null) }}>{summary} :
{summary}
}
    {rows.map((file, index) => { const phase = phases[changedFileUrl(sessionId, changes.seq, index)] - const status = rowStatus(phase) + const status = gesture(phase) // A file without a verified Host path falls back to the Sidebar preview the status names. const opensNatively = native && phase !== 'nativeUnavailable' return
    • diff --git a/packages/deliverables/workspace-changes/src/paths.ts b/packages/deliverables/workspace-changes/src/paths.ts index a18e5454fc..f207eb1e17 100644 --- a/packages/deliverables/workspace-changes/src/paths.ts +++ b/packages/deliverables/workspace-changes/src/paths.ts @@ -1,7 +1,7 @@ /** Path classification and display forms for changed files. */ import { realpath } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { isAbsolute, relative, resolve, sep } from 'node:path' +import { isAbsolute, relative, sep } from 'node:path' /** * Slash-separated form of a native relative path. @@ -63,16 +63,6 @@ export function isTemporaryPath(path: string, roots: readonly string[]): boolean return roots.some(root => isInside(root, path)) } -/** - * Resolve a file-tool path against the Session working directory. - * @param cwd - absolute Session working directory. - * @param path - model-facing path, relative or absolute. - * @returns the absolute native path. - */ -export function absolutePathOf(cwd: string, path: string): string { - return resolve(cwd, path) -} - /** * Sort key and label of a changed file; see `WorkspaceChangedFile.display`. * @param absolute - canonical absolute file path. @@ -82,9 +72,7 @@ export function absolutePathOf(cwd: string, path: string): string { * @returns the slash-separated display path. */ export function displayPathOf(absolute: string, cwd: string, root: string, home: string): string { - const rel = relative(cwd, absolute) - if (!rel.startsWith('..') && !isAbsolute(rel)) return toPosix(rel) - if (isInside(root, absolute)) return toPosix(rel) + if (isInside(cwd, absolute) || isInside(root, absolute)) return toPosix(relative(cwd, absolute)) if (home !== '' && isInside(home, absolute)) return `~/${toPosix(relative(home, absolute))}` return toPosix(absolute) } @@ -96,8 +84,7 @@ export function displayPathOf(absolute: string, cwd: string, root: string, home: * @returns the path the Web client opens the file through. */ export function durablePathOf(absolute: string, cwd: string): string { - const rel = relative(cwd, absolute) - return !rel.startsWith('..') && !isAbsolute(rel) ? toPosix(rel) : absolute + return isInside(cwd, absolute) ? toPosix(relative(cwd, absolute)) : absolute } /** diff --git a/packages/deliverables/workspace-changes/src/recorder.ts b/packages/deliverables/workspace-changes/src/recorder.ts index fb24ec1530..c2fad1437f 100644 --- a/packages/deliverables/workspace-changes/src/recorder.ts +++ b/packages/deliverables/workspace-changes/src/recorder.ts @@ -1,12 +1,12 @@ /** Per-Session turn recorder: snapshot at turn start, diff and append at turn end. */ import { realpath } from 'node:fs/promises' import { homedir } from 'node:os' -import { relative } from 'node:path' +import { relative, resolve } from 'node:path' import type { Session, SessionEvent } from '@deepseek-ai/dsh-session' import type { FileDiff } from '@deepseek-ai/dsh-tools' import { diffTrees, ignoredPaths, locateGitWorkspace, snapshotTree, type GitRunner, type GitWorkspace, type ObjectStoreOptions } from './git.ts' import { argumentHunks, fileDiffsOf, hunkLineCounts } from './numstat.ts' -import { absolutePathOf, canonicalPath, compareDisplay, displayPathOf, durablePathOf, isInside, isTemporaryPath, temporaryRoots, toPosix } from './paths.ts' +import { canonicalPath, compareDisplay, displayPathOf, durablePathOf, isInside, isTemporaryPath, temporaryRoots, toPosix } from './paths.ts' import type { WorkspaceChangedFile } from './types.ts' /** Facts shared by every recorder of one plugin instance. */ @@ -180,33 +180,28 @@ export class TurnRecorder { private async record(state: TurnState, signal: AbortSignal): Promise { if (state.baseline === null || state.lastToolResultSeq < 0) return state.attemptedAfterSeq = state.lastToolResultSeq - const { git, workspace, tree: before, cwd, home, temporaryRoots: roots } = state.baseline + const baseline = state.baseline + const { git, workspace, tree: before, cwd } = baseline + const root = workspace.root const after = await snapshotTree(git, workspace, signal) const files = new Map() for (const entry of await diffTrees(git, workspace, before, after, signal)) { - const absolute = absolutePathOf(workspace.root, entry.path) - files.set(absolute, changedFile(absolute, cwd, home, workspace, entry)) + const absolute = resolve(root, entry.path) + files.set(absolute, changedFile(baseline, absolute, entry)) } + // File-tool hunks by canonical absolute path, for the files snapshots do not cover. const hunks = new Map() for (const [path, list] of state.hunks) { - const absolute = await canonicalPath(absolutePathOf(cwd, path)) - hunks.set(absolute, [...hunks.get(absolute) ?? [], ...list]) + const absolute = await canonicalPath(resolve(cwd, path)) + if (!files.has(absolute)) hunks.set(absolute, [...hunks.get(absolute) ?? [], ...list]) } - const inside: string[] = [] - const outside: string[] = [] - for (const absolute of hunks.keys()) { - if (files.has(absolute)) continue - // The repository is the user's workspace even when it lives under a temporary root. - if (!isInside(workspace.root, absolute) && isTemporaryPath(absolute, roots)) continue - if (isInside(workspace.root, absolute)) inside.push(absolute) - else outside.push(absolute) - } - const workTreePath = (absolute: string): string => toPosix(relative(workspace.root, absolute)) - const ignored = await ignoredPaths(git, workspace, inside.map(workTreePath), signal) - const toolOnly = new Set([...outside, ...inside.filter(absolute => ignored.has(workTreePath(absolute)))]) + const workTreePath = (absolute: string): string => toPosix(relative(root, absolute)) + const inRepository = [...hunks.keys()].filter(absolute => isInside(root, absolute)) + const ignored = await ignoredPaths(git, workspace, inRepository.map(workTreePath), signal) for (const [absolute, list] of hunks) { - if (!toolOnly.has(absolute)) continue - files.set(absolute, changedFile(absolute, cwd, home, workspace, { ...hunkLineCounts(list), binary: false })) + // Inside the repository only ignored files are uncovered; outside it, scratch files under a temporary root stay out. + const uncovered = isInside(root, absolute) ? ignored.has(workTreePath(absolute)) : !isTemporaryPath(absolute, baseline.temporaryRoots) + if (uncovered) files.set(absolute, changedFile(baseline, absolute, { ...hunkLineCounts(list), binary: false })) } const sorted = [...files.values()].sort(compareDisplay) // An empty list after an earlier in-turn record supersedes that record. @@ -219,11 +214,10 @@ export class TurnRecorder { }) state.recordedAfterSeq = event.seq } - } function changedFile( - absolute: string, cwd: string, home: string, workspace: GitWorkspace, counts: { added: number; deleted: number; binary: boolean }, + { cwd, workspace, home }: Baseline, absolute: string, counts: { added: number; deleted: number; binary: boolean }, ): WorkspaceChangedFile { return { path: durablePathOf(absolute, cwd),