Просмотр исходного кода

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.
creatixchu 1 неделя назад
Родитель
Сommit
fa8938d202

+ 27 - 33
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<typeof NS>) {
+  return <>
+    <span className={css.added}>{t('changes.added', { count: GROUPED.format(added) })}</span>
+    <span className={css.deleted}>{t('changes.deleted', { count: GROUPED.format(deleted) })}</span>
+  </>
 }
 
 /**
@@ -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 = <>
     <span className={css.tile}><IconCodeBracketsOutline16 size={18} /></span>
     <span className={css.titles}>
       <span className={css.title}>{t('changes.title', { count: String(changes.total) })}</span>
-      <span className={css.stat} role={folderStatus === undefined ? undefined : 'status'} data-error={folderPhase === 'error' || folderPhase === 'nativeUnavailable' ? true : undefined}>
-        {folderStatus ?? <>
-          <span className={css.added}>{t('changes.added', { count: GROUPED.format(sum.added) })}</span>
-          <span className={css.deleted}>{t('changes.deleted', { count: GROUPED.format(sum.deleted) })}</span>
-        </>}
+      <span className={css.stat} role={folder === undefined ? undefined : 'status'} data-error={folder?.failed || undefined}>
+        {folder === undefined
+          ? <Counts t={t} added={sum(changes.files, 'added')} deleted={sum(changes.files, 'deleted')} />
+          : t(folder.failed ? 'changes.folderError' : 'changes.folderOpening')}
       </span>
     </span>
   </>
   return <div className={css.card} data-changed-files>
     {native
       ? <button type="button" className={css.header} aria-label={t('changes.openFolder')}
-        disabled={folderPhase === 'opening'} onClick={() => { onOpen(null) }}>{summary}</button>
+        disabled={folder !== undefined && !folder.failed} onClick={() => { onOpen(null) }}>{summary}</button>
       : <div className={css.header}>{summary}</div>}
     <ul className={css.list}>
       {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 <li key={file.display}>
           <button type="button" className={css.row} title={resolveWorkspacePath(cwd, file.path)}
             aria-label={t(opensNatively ? 'changes.openFile' : 'presented.previewButton', { name: file.display })}
-            disabled={phase === 'opening'}
+            disabled={status !== undefined && !status.failed}
             onClick={() => { if (opensNatively) onOpen(index); else openFile(file.path) }}>
             <span className={css.path}>{file.display}</span>
-            <span className={css.counts} role={status === undefined ? undefined : 'status'} data-error={status?.error ? true : undefined}>
+            <span className={css.counts} role={status === undefined ? undefined : 'status'} data-error={status?.failed || undefined}>
               {status !== undefined ? t(status.key)
                 : file.binary === true ? t('changes.binary')
-                  : <>
-                    <span className={css.added}>{t('changes.added', { count: GROUPED.format(file.added) })}</span>
-                    <span className={css.deleted}>{t('changes.deleted', { count: GROUPED.format(file.deleted) })}</span>
-                  </>}
+                  : <Counts t={t} added={file.added} deleted={file.deleted} />}
             </span>
           </button>
         </li>

+ 3 - 16
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
 }
 
 /**

+ 17 - 23
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<void> {
     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<string, WorkspaceChangedFile>()
     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<string, FileDiff[]>()
     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),