Add status filter and highlight failed runs on dashboard#7
Conversation
Surface CI failures more directly and tidy up shared formatting: - Add a Status filter (Failed / In Progress / Success) to the workflow results table. The /gitcommit API has no status param, so it filters the current page client-side, with a caption making the scope clear. - Highlight failed rows with a subtle red tint for at-a-glance scanning. - Use stable React keys (result id) instead of array index. - Consolidate three diverging duration/time formatters into a single formatDuration helper in utils/format.ts. - Remove a stray console.log left in the filter effect.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR consolidates duration formatting across the codebase and adds a client-side workflow status filter. A new shared ChangesDuration Formatting and Status Filtering
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@utils/format.ts`:
- Around line 7-9: The current conversion computes hrs = Math.floor(mins / 60)
and rem = Math.round(mins % 60), but rounding can make rem == 60 (e.g., "1h
60m"); update the logic in utils/format.ts to detect rem === 60 and if so
increment hrs by 1 and set rem to 0 before returning the formatted string
(referencing the hrs and rem variables used in the diff).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 69be722a-b2e5-4ecf-8b96-d94ac51632e8
📒 Files selected for processing (4)
components/StatsDashboard.tsxpages/index.tsxpages/workflow/[id].tsxutils/format.ts
| const hrs = Math.floor(mins / 60) | ||
| const rem = Math.round(mins % 60) | ||
| return `${hrs}h ${rem}m` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix hour-boundary rounding (1h 60m can appear).
At Line 8, rounding remainder minutes can produce 60, which renders invalid hour/minute output (time rhyme: minutes shouldn’t hit sixty twice).
Suggested patch
export function formatDuration(seconds: number): string {
if (!isFinite(seconds) || seconds <= 0) return '—'
if (seconds < 60) return `${Math.round(seconds)}s`
const mins = seconds / 60
- if (mins < 60) return `${mins.toFixed(1)}m`
- const hrs = Math.floor(mins / 60)
- const rem = Math.round(mins % 60)
+ const totalMinutes = Math.round(mins)
+ if (totalMinutes < 60) return `${mins.toFixed(1)}m`
+ const hrs = Math.floor(totalMinutes / 60)
+ const rem = totalMinutes % 60
return `${hrs}h ${rem}m`
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const hrs = Math.floor(mins / 60) | |
| const rem = Math.round(mins % 60) | |
| return `${hrs}h ${rem}m` | |
| export function formatDuration(seconds: number): string { | |
| if (!isFinite(seconds) || seconds <= 0) return '—' | |
| if (seconds < 60) return `${Math.round(seconds)}s` | |
| const mins = seconds / 60 | |
| const totalMinutes = Math.round(mins) | |
| if (totalMinutes < 60) return `${mins.toFixed(1)}m` | |
| const hrs = Math.floor(totalMinutes / 60) | |
| const rem = totalMinutes % 60 | |
| return `${hrs}h ${rem}m` | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@utils/format.ts` around lines 7 - 9, The current conversion computes hrs =
Math.floor(mins / 60) and rem = Math.round(mins % 60), but rounding can make rem
== 60 (e.g., "1h 60m"); update the logic in utils/format.ts to detect rem === 60
and if so increment hrs by 1 and set rem to 0 before returning the formatted
string (referencing the hrs and rem variables used in the diff).
1 participant
Since CI failures currently have no notifications, the dashboard is the only place anyone sees them — so this makes failures easier to spot, plus some cleanup.
Changes
/gitcommitAPI has no status parameter, so this filters the rows already loaded for the current page, client-side. A caption under the filters states the scope ("Showing N of M runs on this page matching …") so it's not mistaken for a global filter.formatDuration(utils/format.ts) replacing three diverging formatters — the oldcalculateTimeDifferencerendered e.g.1.5 minute/30 secwhile the stats card rendered1.5m/45s. Now consistent everywhere (workflow table, workflow detail, stats).console.logleft in the filter effect.Verification
pnpm buildandpnpm lintboth pass. Changes are presentational + client-side filtering; no API or data-model changes.Follow-ups (backend-gated, not in this PR)
api.comfy.org— the current stats cards are necessarily scoped to the loaded page ("in current view"). Worth a backend ticket.