Browse Source

feat(ui): highlight trajectory errors

_Kerman 2 months ago
parent
commit
287289ddbc

+ 34 - 0
packages/client/ui-trajectory/src/client/TrajectoryTable.module.css

@@ -236,6 +236,18 @@
   width: 3px;
 }
 
+.table tbody tr[data-error='true'] .turnRail {
+  background: color-mix(
+    in srgb,
+    var(--dsw-alias-state-error-primary) 22%,
+    var(--dsw-alias-bg-layer-1)
+  );
+}
+
+.table tbody tr[data-error='true'] .selectionRail {
+  background: var(--dsw-alias-state-error-primary);
+}
+
 .table tbody tr[data-turn-start='true'] td {
   position: relative;
   overflow: visible;
@@ -636,6 +648,28 @@
   color: var(--dsw-alias-state-error-primary);
 }
 
+.overview dd.error {
+  color: var(--dsw-alias-state-error-primary);
+}
+
+.details .errorPayload {
+  color: var(--dsw-alias-state-error-primary);
+}
+
+.details .errorPayload .resultBlockText {
+  color: inherit;
+}
+
+.details .jsonPayload.errorPayload,
+.details .jsonPreview.errorPayload {
+  --json-tree-property: var(--dsw-alias-state-error-primary);
+  --json-tree-string: var(--dsw-alias-state-error-primary);
+  --json-tree-number: var(--dsw-alias-state-error-primary);
+  --json-tree-keyword: var(--dsw-alias-state-error-primary);
+  --json-tree-punctuation: var(--dsw-alias-state-error-primary);
+  --json-tree-icon: var(--dsw-alias-state-error-primary);
+}
+
 .details {
   position: relative;
   display: flex;

+ 30 - 9
packages/client/ui-trajectory/src/client/TrajectoryTable.tsx

@@ -1123,13 +1123,20 @@ function SystemPromptDiff({
 
 function ToolOutputBlocks({
   blocks,
+  error,
   preview,
 }: {
   blocks: readonly TrajectorySourceBlock[]
+  error: boolean
   preview: boolean
 }) {
   return (
-    <div className={preview ? `${css.resultBlocks} ${css.resultBlocksPreview}` : css.resultBlocks}>
+    <div className={[
+      css.resultBlocks,
+      preview ? css.resultBlocksPreview : undefined,
+      error ? css.errorPayload : undefined,
+    ].filter((value): value is string => value !== undefined).join(' ')}
+    >
       {blocks.map((block, index) => (
         block.imageSrc !== undefined
           ? <PanelImage block={block} preview={preview} key={index} />
@@ -1302,6 +1309,9 @@ function RecordPayload({
     ? 'No payload captured'
     : 'No result captured'
   if (!value) return <p className={css.noPayload}>{missing}</p>
+  const error = direction === 'output' && record.cell.isError === true
+  const payloadClass = preview ? css.jsonPreview : css.jsonPayload
+  const payloadClassName = error ? `${payloadClass} ${css.errorPayload}` : payloadClass
 
   const json = parseJsonContainer(value)
   const singleTextResult = direction === 'output'
@@ -1312,7 +1322,7 @@ function RecordPayload({
       <JsonTree
         data={json}
         label="Result JSON"
-        className={preview ? css.jsonPreview : css.jsonPayload}
+        className={payloadClassName}
       />
     )
   }
@@ -1325,6 +1335,7 @@ function RecordPayload({
     return (
       <ToolOutputBlocks
         blocks={record.cell.outputBlocks}
+        error={error}
         preview={preview}
       />
     )
@@ -1338,7 +1349,11 @@ function RecordPayload({
   )
   if (markdown) {
     return (
-      <div className={preview ? css.markdownPreview : css.markdownPayload}>
+      <div className={[
+        preview ? css.markdownPreview : css.markdownPayload,
+        error ? css.errorPayload : undefined,
+      ].filter((className): className is string => className !== undefined).join(' ')}
+      >
         <MarkdownText text={value} />
       </div>
     )
@@ -1348,7 +1363,7 @@ function RecordPayload({
       <JsonTree
         data={json}
         label={`${direction === 'input' ? 'Payload' : 'Result'} JSON`}
-        className={preview ? css.jsonPreview : css.jsonPayload}
+        className={payloadClassName}
       />
     )
   }
@@ -1356,7 +1371,7 @@ function RecordPayload({
     <pre className={[
       css.payload,
       preview ? css.payloadPreview : undefined,
-      record.cell.isError ? css.error : undefined,
+      error ? css.errorPayload : undefined,
       value === 'No output' ? css.noOutputText : undefined,
     ].filter((value): value is string => value !== undefined).join(' ')}
     >
@@ -2078,7 +2093,9 @@ export function TrajectoryTable({
                 <dl className={css.overview}>
                   <div>
                     <dt>Status</dt>
-                    <dd>{statusLabel(selectedRequestState)}</dd>
+                    <dd className={selectedRequestState === 'error' ? css.error : undefined}>
+                      {statusLabel(selectedRequestState)}
+                    </dd>
                   </div>
                   {selectedRequestInfo?.purpose === 'compaction' && (
                     <div>
@@ -2119,7 +2136,7 @@ export function TrajectoryTable({
                   {selectedRequestInfo?.error !== undefined && (
                     <div>
                       <dt>Error</dt>
-                      <dd>{selectedRequestInfo.error}</dd>
+                      <dd className={css.error}>{selectedRequestInfo.error}</dd>
                     </div>
                   )}
                   {selectedRequestInfo?.retry !== undefined && (
@@ -2227,7 +2244,9 @@ export function TrajectoryTable({
                 <dl className={css.overview}>
                   <div>
                     <dt>Status</dt>
-                    <dd>{statusLabel(selectedState)}</dd>
+                    <dd className={selectedState === 'error' ? css.error : undefined}>
+                      {statusLabel(selectedState)}
+                    </dd>
                   </div>
                   <div>
                     <dt>Duration</dt>
@@ -2330,7 +2349,9 @@ export function TrajectoryTable({
                   )}
                   <div>
                     <dt>Status</dt>
-                    <dd>{statusLabel(selectedState)}</dd>
+                    <dd className={selectedState === 'error' ? css.error : undefined}>
+                      {statusLabel(selectedState)}
+                    </dd>
                   </div>
                   {selected.cell.kind === 'message' && (
                     <TokenRows cell={selected.cell} />

+ 4 - 0
packages/client/ui-trajectory/src/client/TrajectoryTimeline.module.css

@@ -146,6 +146,10 @@
   );
 }
 
+.span[data-error='true'] {
+  background: var(--dsw-alias-state-error-primary);
+}
+
 .span[data-equal-duration='true'] {
   width: 8px;
   min-width: 8px;

+ 1 - 0
packages/client/ui-trajectory/src/client/TrajectoryTimeline.tsx

@@ -467,6 +467,7 @@ export const TrajectoryTimeline = memo(function TrajectoryTimeline({
                     className={css.span}
                     data-timeline-span={span.kind}
                     data-timeline-record-index={span.index}
+                    data-error={span.isError || undefined}
                     data-equal-duration={mode === 'time' || undefined}
                     data-current={span.index === selectedIndex || undefined}
                     data-hovered={hover?.recordIndex === span.index || undefined}

+ 3 - 0
packages/client/ui-trajectory/src/client/timeline.ts

@@ -15,6 +15,7 @@ export interface TrajectoryTimeRange {
 /** One ledger record projected into the active timeline domain. */
 export interface TrajectoryTimelineSpan extends TrajectoryTimeRange {
   index: number
+  isError: boolean
   kind: TrajectoryCellKind
   label: string
   lane: number
@@ -94,6 +95,7 @@ export function deriveTrajectoryTimeline(
       start: spans.length + offset,
       end: spans.length + offset + 1,
       index: cell.index,
+      isError: cell.isError === true,
       kind: cell.kind,
       label: cell.text,
       lane: laneFor(cell.kind),
@@ -129,6 +131,7 @@ function deriveTimedTimeline(
           : [{
             ...range,
             index: cell.index,
+            isError: cell.isError === true,
             kind: cell.kind,
             label: cell.text,
             lane: laneFor(cell.kind),

+ 3 - 1
packages/client/ui-trajectory/tests/table.spec.tsx

@@ -144,8 +144,10 @@ describe('TrajectoryTable', () => {
     expect(screen.getByText('Pending')).toBeTruthy()
     fireEvent.click(screen.getByRole('row', { name: /TOOL, bash \{"command":"false"\}/ }))
     expect(screen.getByText('Failed')).toBeTruthy()
+    expect(screen.getByText('Failed').className).toContain('error')
     fireEvent.click(screen.getByRole('tab', { name: 'Result' }))
-    expect(screen.getByText('ToolError: non_zero_exit')).toBeTruthy()
+    const errorResult = screen.getByText('ToolError: non_zero_exit')
+    expect(errorResult.closest('[class*="errorPayload"]')).toBeTruthy()
   })
 
   it('renders responsive role icons with a custom tooltip', () => {

+ 38 - 3
packages/client/ui-trajectory/tests/views.spec.tsx

@@ -469,15 +469,50 @@ describe('timeline projection', () => {
       end: 3,
       spans: [
         {
-          index: 1, kind: 'message', label: 'assistant', lane: 1, start: 0, end: 1,
+          index: 1, isError: false, kind: 'message', label: 'assistant',
+          lane: 1, start: 0, end: 1,
+        },
+        {
+          index: 2, isError: false, kind: 'tool', label: 'bash',
+          lane: 2, start: 1, end: 2,
+        },
+        {
+          index: 3, isError: false, kind: 'user', label: 'unknown',
+          lane: 0, start: 2, end: 3,
         },
-        { index: 2, kind: 'tool', label: 'bash', lane: 2, start: 1, end: 2 },
-        { index: 3, kind: 'user', label: 'unknown', lane: 0, start: 2, end: 3 },
       ],
       turnBoundaries: [{ turn: 1, time: 0 }],
     })
   })
 
+  it('marks error records directly on timeline spans', () => {
+    const errorTurns = [{
+      turn: 1,
+      groups: [{
+        title: 'Step 1',
+        cells: [{
+          index: 1,
+          kind: 'tool' as const,
+          text: 'failed tool',
+          timeSeconds: 0.1,
+          isError: true,
+        }],
+      }],
+    }] satisfies readonly TrajectoryTurnModel[]
+    const view = render(
+      <TrajectoryTimeline
+        turns={errorTurns}
+        mode="sequence"
+        range={null}
+        onRangeChange={() => {}}
+      />,
+    )
+
+    expect(view.container.querySelector(
+      '[data-timeline-span="tool"][data-error="true"]',
+    )).toBeTruthy()
+  })
+
   it('ignores durations and idle gaps while retaining turn boundaries', () => {
     const separatedTurns = [
       {