feat(web): enrich workflow result card with status, duration, nodes, and artifacts#1025
feat(web): enrich workflow result card with status, duration, nodes, and artifacts#1025
Conversation
…and artifacts (#1015) The WorkflowResultCard showed a hardcoded green checkmark regardless of actual outcome, with no duration, node count, or artifact links. Changes: - Fetch terminal run data via TanStack Query (staleTime: Infinity) - Merge Zustand live state with API fallback for offline/CLI workflows - Render StatusIcon for completed/failed/cancelled status awareness - Display node count and duration pill in the header - Show ArtifactSummary (PRs, commits, branches, files) above text content - Derive node counts and artifacts from events when live state unavailable Fixes #1015 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a Workflow Result Card to chat: MessageList now fetches/merges workflow run data (API + Zustand) to show terminal status, node completion counts, duration, and artifacts; orchestrator now surfaces failure run metadata so chats can render result cards; docs updated. Changes
Sequence Diagram(s)sequenceDiagram
participant Orchestrator
participant ChatService as MessageList/Chat
participant API as Workflows API
participant Store as Zustand Store
participant User
Orchestrator->>ChatService: dispatch message (final node) with workflowResult metadata (workflowName, runId) or error+workflowResult
ChatService->>API: useQuery(getWorkflowRun(runId), staleTime=Infinity)
ChatService->>Store: read live run state (dagNodes, artifacts)
ChatService-->>ChatService: merge API runData with Store live state (status, timestamps, node counts, artifacts)
ChatService->>ChatService: render WorkflowResultCard (StatusIcon, node progress, duration, artifacts, link)
User->>ChatService: click "View full logs" -> navigate to /workflows/runs/{runId}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Comprehensive PR ReviewPR: #1025 — feat(web): enrich workflow result card with status, duration, nodes, and artifacts SummaryClean implementation that enriches Verdict:
🟡 Medium Issue — Fix Before Merge
|
| # | Issue | Location | Suggestion |
|---|---|---|---|
| 1 | Duration badge bg-surface-elevated blends into parent header (same bg) |
MessageList.tsx:132 |
Change to bg-surface for visual contrast |
| 2 | Default status 'completed' may briefly flash green ✓ for failed runs on first load |
MessageList.tsx:57 |
Accept as-is — fix requires broader StatusIcon changes; flash is one render cycle |
| 3 | No test for artifact event extraction logic | MessageList.tsx:87-97 |
Create follow-up issue — extract to src/lib/ helper + test |
| 4 | status→headerTitle mapping untested ('running' → "Workflow complete") |
MessageList.tsx:101-106 |
Accept as-is — architectural invariant prevents non-terminal status here |
| 5 | Duration timestamp merge untested (utilities are tested, merge chain is not) | MessageList.tsx:60-66 |
Accept as-is — ensureUtc coverage provides adequate indirect coverage |
| 6 | staleTime: Infinity comment doesn't explain why Infinity is used |
MessageList.tsx:49 |
Extend: "terminal run record is immutable — status/timestamps/events do not change once completed/failed/cancelled" |
| 7 | Node count comment doesn't note that events-path totalCount is an approximation |
MessageList.tsx:68 |
Update: "fall back to events (approximation — totalCount is nodes that reached a terminal state, not the workflow's full node count)" |
Also: packages/docs-web/src/content/docs/adapters/web.md — the "Workflow Progress Cards" section describes the in-flight card only; the new result card (status icon, duration, node count, artifacts) is undocumented. Suggest adding a brief "Workflow Result Card" subsection.
✅ What's Good
- Cache coherence:
['workflowRun', runId]key aligns withWorkflowExecution.tsx'sinvalidateQueries— completing a run on the detail page automatically invalidates the result card's cache. Unintentional but excellent. staleTime: Infinityis correct: Terminal runs are immutable; no re-fetches on remount is the right call.- Primitive reuse:
StatusIcon,ArtifactSummary,formatDurationMs,ensureUtc— all reused, no duplication. - Artifact cast:
e.data as Record<string, string | undefined>is the correct ESLint-compliant solution; well-justified in the scope doc. - Conditional rendering:
{totalCount > 0 && ...}and{duration != null && ...}suppress metadata when unavailable rather than showing zeroes. - CLAUDE.md compliance: Type safety, import patterns, YAGNI, ESLint zero-tolerance — all pass.
📋 Suggested Follow-up Issues
| Issue | Priority |
|---|---|
Add unit tests for WorkflowResultCard artifact extraction and node-count logic |
P2 |
| Document Workflow Result Card in web adapter docs | P3 |
Reviewed by Archon comprehensive-pr-review workflow
Artifacts: ~/.archon/workspaces/coleam00/Archon/artifacts/runs/2a26c0973026855f7e8e35f64587b303/review/
…omments and docs - Fix MEDIUM: totalCount in dagNodes live-state path now counts only terminal nodes (completed/failed/skipped), matching the semantics of the events fallback path. Previously included pending/running nodes, producing a misleading denominator during page-refresh mid-execution. - Fix LOW: Duration badge background changed from bg-surface-elevated to bg-surface for visual contrast against the bg-surface-elevated parent header. - Fix LOW: Expanded staleTime: Infinity comment to explain the immutability invariant. - Fix LOW: Expanded node count comment to describe that the events-path totalCount is an approximation (nodes that reached terminal state, not workflow's full node count). - Fix LOW: Added Workflow Result Card subsection to web adapter docs describing the post-completion card's status icon, header, node count, duration, and artifacts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix ReportAll actionable review findings from the consolidated review have been addressed. 5 fixes applied (1 MEDIUM, 4 LOW). Commit: 163ad9d MEDIUM — Fixed ✓
The // Before
totalCount = dagNodes.length;
// After
totalCount = dagNodes.filter(
n => n.status === 'completed' || n.status === 'failed' || n.status === 'skipped'
).length;LOW Fixes ✓
Findings Skipped (accepted/deferred)
Validation
|
Simplify verbose onClick handlers in WorkflowResultCard — remove unnecessary block body and explicit void return type for the navigate call; collapse the setExpanded handler to a single-line block per ESLint no-confusing-void-expression. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/web/src/components/chat/MessageList.tsx (1)
91-103: Consider defensive extraction for artifact event data.The type assertion on line 95 assumes
e.dataconforms to a specific shape. If the event structure varies or contains unexpected types, this could silently produce malformed artifacts.🛡️ Optional: Add runtime guards
const eventArtifacts: WorkflowArtifact[] = (runData?.events ?? []) .filter(e => e.event_type === 'workflow_artifact') .map(e => { - const d = e.data as Record<string, string | undefined>; + const d = (typeof e.data === 'object' && e.data !== null ? e.data : {}) as Record<string, unknown>; return { - type: (d.artifactType ?? 'file') as ArtifactType, - label: d.label ?? '', - url: d.url, - path: d.path, + type: (typeof d.artifactType === 'string' ? d.artifactType : 'file') as ArtifactType, + label: typeof d.label === 'string' ? d.label : '', + url: typeof d.url === 'string' ? d.url : undefined, + path: typeof d.path === 'string' ? d.path : undefined, }; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/components/chat/MessageList.tsx` around lines 91 - 103, The code uses a direct assertion of e.data in the eventArtifacts mapping which can produce malformed artifacts if the event shape varies; update the eventArtifacts construction (the mapping over runData?.events in MessageList.tsx) to defensively extract and validate fields from e.data before creating a WorkflowArtifact: first check that e?.data is an object, coerce or default artifactType/label to safe strings, and only assign url/path when they are string values (otherwise set to undefined); ensure ArtifactType casts only after validating allowed values and filter out or skip events that don't meet the minimal shape so artifacts only contains well-formed WorkflowArtifact entries (adjust the eventArtifacts variable and the final artifacts fallback accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/web/src/components/chat/MessageList.tsx`:
- Around line 91-103: The code uses a direct assertion of e.data in the
eventArtifacts mapping which can produce malformed artifacts if the event shape
varies; update the eventArtifacts construction (the mapping over runData?.events
in MessageList.tsx) to defensively extract and validate fields from e.data
before creating a WorkflowArtifact: first check that e?.data is an object,
coerce or default artifactType/label to safe strings, and only assign url/path
when they are string values (otherwise set to undefined); ensure ArtifactType
casts only after validating allowed values and filter out or skip events that
don't meet the minimal shape so artifacts only contains well-formed
WorkflowArtifact entries (adjust the eventArtifacts variable and the final
artifacts fallback accordingly).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 713c2774-f09e-47f2-a2b7-6482901f61e8
📒 Files selected for processing (2)
packages/docs-web/src/content/docs/adapters/web.mdpackages/web/src/components/chat/MessageList.tsx
- Fix unsafe `e.data` cast: use runtime type narrowing instead of `as Record<string, string | undefined>` for event artifact extraction - Fix invalid ArtifactType fallback: change 'file' to 'file_created' (a valid member of the ArtifactType union) - Handle useQuery error state: destructure `isError` and render a degraded card (no node count, duration, or artifacts) when the API fetch fails, preventing a misleading "Workflow complete" display - Emit workflowResult metadata on failure/cancel: the orchestrator now attaches workflowResult to failed workflow messages so the chat renders a result card with status icon and "View full logs" link - Add 'workflowRun' to invalidateWorkflowQueries() so singular run cache entries are invalidated alongside the plural list caches Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolve merge conflict in MessageList.tsx by combining: - PR coleam00#1025: status/duration/nodes/artifacts enrichment for WorkflowResultCard - PR coleam00#1023: ArtifactViewerModal clickable file paths in result card content Both features now work together — the result card shows status-aware headers, node counts, duration, and artifact summaries while also supporting clickable artifact file paths in the markdown content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolve merge conflict in MessageList.tsx by combining: - PR coleam00#1025: status/duration/nodes/artifacts enrichment for WorkflowResultCard - PR coleam00#1023: ArtifactViewerModal clickable file paths in result card content Both features now work together — the result card shows status-aware headers, node counts, duration, and artifact summaries while also supporting clickable artifact file paths in the markdown content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
WorkflowResultCardin the chat view showed a hardcoded green checkmark + "Workflow complete: {name}" regardless of actual run outcome — failed and cancelled workflows looked identical to successful ones.WorkflowResultCardinMessageList.tsxto fetch run data via TanStack Query (staleTime: Infinity), merge with live Zustand store state, and render a status-aware header (StatusIcon), duration pill (formatDurationMs), node count, and clickable artifacts (ArtifactSummary).WorkflowResultCardcomponent in@archon/web.UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
Label Snapshot
risk: lowsize: Swebweb:chatChange Metadata
featurewebLinked Issue
Validation Evidence (required)
bun run validaterun; all 4 checks passed on first run, no files modified during validation.Security Impact (required)
GET /api/workflows/runs/{runId}endpoint that was already accessibleCompatibility / Migration
Human Verification (required)
Validated via
bun run validate(type-check, lint, format, tests all pass). UI changes are purely in@archon/webReact component; no manual browser verification was performed in this automated implementation.dot-notation/no-base-to-stringrules on event data — resolved by castinge.datatoRecord<string, string | undefined>before accessSide Effects / Blast Radius (required)
@archon/webchat view only — no server, CLI, or adapter code touchedArtifactSummaryimport verified safe (both components inpackages/web, no circular dep)Rollback Plan (required)
git revert HEAD— single commit, single file changeWorkflowResultCardmissing or broken in chat view; would show as React render error or blank cardRisks and Mitigations
Risk: Zustand state cleared on page refresh → card has no data until API fetch resolves
useQueryfetch always fires (staleTime: Infinitybut still fetches once); card renders immediately from API data; loading state is indistinguishable from the existing static card during the brief fetch windowRisk:
eventsarray large for long workflows (performance on initial fetch)staleTime: Infinityprevents re-fetch on focus/remount; no pollingRisk: ESLint bracket notation rule on
e.dataaccesse.datatoRecord<string, string | undefined>to satisfy@typescript-eslint/dot-notationand@typescript-eslint/no-base-to-string; verified 0 warningsSummary by CodeRabbit
New Features
Bug Fixes
Documentation