feat(dashboard): add build time column to builds table#1795
feat(dashboard): add build time column to builds table#1795ToriChanIntegration wants to merge 11 commits intoCap-go:mainfrom
Conversation
Show computed build duration (updated_at - created_at) for completed, succeeded, and failed builds. Pending/running builds show "-". Also fix pre-existing typecheck error in GroupsRbacManager.vue where delete_group_with_bindings RPC is not in generated Supabase types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds persistent build-time tracking: a nullable Changes
Sequence DiagramsequenceDiagram
participant Builder as Builder Service
participant API as Status API Handler
participant DB as Database
participant UI as Frontend (BuildTable)
Builder->>API: Job finished (started_at, completed_at, status)
API->>API: compute buildTimeSeconds if timestamps present & status terminal
API->>DB: UPDATE build_requests { status, last_error, updated_at, build_time_seconds? }
DB-->>API: OK
API-->>Builder: respond with build_time_seconds
UI->>DB: SELECT build_requests
DB-->>UI: rows (may include build_time_seconds)
UI->>UI: format seconds -> display string
UI-->>User: render build-time column
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25a0b28ee6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/components/tables/BuildTable.vue
Outdated
| const end = new Date(elem.updated_at).getTime() | ||
| const diffMs = end - start |
There was a problem hiding this comment.
This column computes build_time as updated_at - created_at, but in the backend status flow updated_at is rewritten to new Date().toISOString() on each /build/status poll (supabase/functions/_backend/public/build/status.ts, update block), including after a job is terminal. In environments where status is checked again later (CLI/UI refreshes or retries), displayed durations keep increasing and no longer represent the actual build runtime, so users can see inflated build times.
Useful? React with 👍 / 👎.
![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
🧹 Nitpick comments (1)
src/components/tables/BuildTable.vue (1)
224-250: LGTM! Well-structured duration calculation with proper edge case handling.The implementation correctly:
- Handles missing timestamps and non-terminal statuses
- Guards against negative duration (clock skew edge case)
- Uses appropriate formatting tiers for different durations
One optional improvement: consider extracting the duration formatting logic (lines 239-248) into a reusable utility function in
~/services/datealongsideformatDate, as this pattern may be useful elsewhere.♻️ Optional: Extract to utility function
Add to
~/services/date:export function formatDuration(diffMs: number): string { if (diffMs < 0) return '-' const totalSeconds = Math.floor(diffMs / 1000) if (totalSeconds < 60) return `${totalSeconds}s` const minutes = Math.floor(totalSeconds / 60) const seconds = totalSeconds % 60 if (minutes < 60) return `${minutes}m ${seconds}s` const hours = Math.floor(minutes / 60) const remainingMinutes = minutes % 60 return `${hours}h ${remainingMinutes}m` }Then simplify the column:
displayFunction: (elem: Element) => { if (!elem.created_at || !elem.updated_at) return '-' const terminalStatuses = ['completed', 'succeeded', 'failed'] if (!terminalStatuses.includes(elem.status)) return '-' const start = new Date(elem.created_at).getTime() const end = new Date(elem.updated_at).getTime() - const diffMs = end - start - if (diffMs < 0) - return '-' - const totalSeconds = Math.floor(diffMs / 1000) - if (totalSeconds < 60) - return `${totalSeconds}s` - const minutes = Math.floor(totalSeconds / 60) - const seconds = totalSeconds % 60 - if (minutes < 60) - return `${minutes}m ${seconds}s` - const hours = Math.floor(minutes / 60) - const remainingMinutes = minutes % 60 - return `${hours}h ${remainingMinutes}m` + return formatDuration(end - start) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/tables/BuildTable.vue` around lines 224 - 250, Extract the inline duration formatting from the BuildTable column's displayFunction into a reusable utility function (e.g., export function formatDuration(diffMs: number) in ~/services/date alongside formatDate) and replace the block that computes totalSeconds/minutes/hours with a call to formatDuration(new Date(elem.updated_at).getTime() - new Date(elem.created_at).getTime()); keep the pre-checks currently in displayFunction (missing timestamps, non-terminal statuses, negative diffs can be handled by formatDuration returning '-' for negative inputs) so the column uses the new formatDuration utility instead of duplicating the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/organization/GroupsRbacManager.vue`:
- Line 264: The RPC call in GroupsRbacManager.vue is bypassing TS types with "as
any" for 'delete_group_with_bindings'; regenerate the Supabase client types (run
`bun types`) so the RPC appears in src/types/supabase.types.ts, commit the
updated types, then remove the "as any" cast and call
supabase.rpc('delete_group_with_bindings', { group_id: group.id }) using the
newly generated typed signature; verify there are no remaining type errors and
adjust the argument name/type to match the generated RPC type if needed.
---
Nitpick comments:
In `@src/components/tables/BuildTable.vue`:
- Around line 224-250: Extract the inline duration formatting from the
BuildTable column's displayFunction into a reusable utility function (e.g.,
export function formatDuration(diffMs: number) in ~/services/date alongside
formatDate) and replace the block that computes totalSeconds/minutes/hours with
a call to formatDuration(new Date(elem.updated_at).getTime() - new
Date(elem.created_at).getTime()); keep the pre-checks currently in
displayFunction (missing timestamps, non-terminal statuses, negative diffs can
be handled by formatDuration returning '-' for negative inputs) so the column
uses the new formatDuration utility instead of duplicating the logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 589e510d-52c8-48f9-b1f6-f10d4aa8a371
⛔ Files ignored due to path filters (1)
builds-table-with-build-time.pngis excluded by!**/*.png
📒 Files selected for processing (2)
src/components/organization/GroupsRbacManager.vuesrc/components/tables/BuildTable.vue
Adds allowedHosts: true to vite server config so Cloudflare tunnel domains are not blocked. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vite.config.mts`:
- Line 164: The dev server currently disables host filtering via the
allowedHosts: true setting; replace this with a restrictive allow-list on the
dev server config (update the allowedHosts option in vite.config.mts) and/or
read allowed hosts from an environment variable like
__VITE_ADDITIONAL_SERVER_ALLOWED_HOSTS so only explicit hostnames are accepted;
for Cloudflare Tunnel use a named tunnel with a custom hostname instead of
whitelisting trycloudflare.com or other wildcard hosts.
…d_at The build time column was computing duration from updated_at - created_at, but updated_at gets rewritten on every /build/status poll (even after the build is terminal), causing inflated durations. - Add build_time_seconds column to build_requests (migration) - Store it from the builder API's started_at/completed_at in status.ts - Frontend reads build_time_seconds directly instead of computing - Revert unrelated GroupsRbacManager.vue change - Remove screenshot from tracked files - Add seed data for build_requests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/types/supabase.types.ts`:
- Around line 4063-4081: The shared Database type includes seed-only RPC typings
(reset_and_seed_app_data, reset_and_seed_app_stats_data, reset_and_seed_data,
reset_and_seed_stats_data, reset_app_data, reset_app_stats_data) that don't
exist in production; remove these RPC entries from the generated Database
definitions and place them into a separate test-only typings file (or regenerate
the types from a migrations-only DB) so production code cannot type-check calls
to these seed RPCs—update any imports/usages to reference the new test-only
typings for tests only.
In `@supabase/migrations/20260317044410_add_build_time_seconds.sql`:
- Around line 4-8: The new nullable column build_time_seconds on table
build_requests will be NULL for existing terminal rows—either backfill those
rows in this migration by computing the integer duration from completed/terminal
timestamps (e.g., updated_at - created_at or completed_at - started_at) into
build_requests.build_time_seconds for all already-terminated builds, or keep a
fallback in the application/query layer that uses COALESCE(build_time_seconds,
computed_duration) (computed from updated_at - created_at) until
build_time_seconds is populated; update the migration to perform the backfill or
update the UI/db access code that reads build_time_seconds to fall back to
updated_at - created_at for historical rows.
In `@supabase/seed.sql`:
- Around line 618-624: Add a new fixture row to the "public"."build_requests"
INSERT that uses status = 'running' (to exercise the dashboard's "no duration
yet" path); set created_at/updated_at to a recent past (e.g., NOW() - interval
'30 minutes', NOW() - interval '29 minutes'), leave build_time_seconds NULL, set
app_id = 'com.demo.app' and owner_org/requested_by to the same demo UUIDs used
in existing rows, and provide a unique
upload_session_key/upload_path/upload_url/upload_expires_at similar to the other
entries so the seed covers the in-progress running state in tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c3252f4-2474-4387-ba95-e76d5185a298
📒 Files selected for processing (5)
src/components/tables/BuildTable.vuesrc/types/supabase.types.tssupabase/functions/_backend/public/build/status.tssupabase/migrations/20260317044410_add_build_time_seconds.sqlsupabase/seed.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/tables/BuildTable.vue
| ALTER TABLE "public"."build_requests" | ||
| ADD COLUMN "build_time_seconds" integer; | ||
|
|
||
| COMMENT ON COLUMN "public"."build_requests"."build_time_seconds" | ||
| IS 'Actual build duration in seconds, sourced from the builder API (completed_at - started_at). NULL until the build finishes.'; |
There was a problem hiding this comment.
Keep a rollout path for existing terminal builds.
Once the dashboard reads build_time_seconds directly, every pre-existing terminal row will still have NULL here, so historical durations regress to -. Please either backfill/enrich existing rows before the UI switch or keep the legacy updated_at - created_at fallback until this column is populated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260317044410_add_build_time_seconds.sql` around lines 4
- 8, The new nullable column build_time_seconds on table build_requests will be
NULL for existing terminal rows—either backfill those rows in this migration by
computing the integer duration from completed/terminal timestamps (e.g.,
updated_at - created_at or completed_at - started_at) into
build_requests.build_time_seconds for all already-terminated builds, or keep a
fallback in the application/query layer that uses COALESCE(build_time_seconds,
computed_duration) (computed from updated_at - created_at) until
build_time_seconds is populated; update the migration to perform the backfill or
update the UI/db access code that reads build_time_seconds to fall back to
updated_at - created_at for historical rows.
|
Thanks for the contribution. We’re closing this PR because we can’t accept repository changes in this form from an integration-style / automated contributor account. For Capgo repos, we expect contributions to be tightly scoped, manually validated, and submitted by an identifiable contributor who can clearly explain the problem, why this approach fits the project, and what was verified locally. Large or bulk-generated change sets without that ownership context create too much review and maintenance risk for us. If this was opened in error or if you are a real contributor behind this account, you’re welcome to resubmit a smaller, clearly-scoped PR with manual validation notes and repository-specific reasoning. |
- vite: use explicit allowedHosts ['.torichan.dev'] instead of true (security: prevents DNS rebinding attacks on dev server) - BuildTable: add fallback for historical builds where build_time_seconds is NULL but build is terminal (uses updated_at - created_at) - status.ts: extract buildTimeSeconds into single variable, remove duplicated computation - seed: add 'running' fixture for full status coverage - Rename migration to today's date (20260320) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # supabase/functions/_backend/public/build/status.ts
There was a problem hiding this comment.
Pull request overview
Adds first-class build duration support across DB/backend/frontend so users can see how long native builds take directly in the dashboard builds table.
Changes:
- Adds
build_time_secondstopublic.build_requestsand seeds demo rows with sample durations. - Persists build duration in the build status endpoint and displays it in the dashboard builds table with human-friendly formatting (with a fallback for historical rows).
- Updates generated Supabase TypeScript types.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.mts | Adds server.allowedHosts configuration. |
| supabase/seed.sql | Seeds demo build_requests rows including build_time_seconds. |
| supabase/migrations/20260320130000_add_build_time_seconds.sql | Adds the new build_time_seconds column to build_requests. |
| supabase/functions/_backend/public/build/status.ts | Derives and stores build duration from builder timestamps; returns it in API response. |
| src/types/supabase.types.ts | Updates generated DB types to include build_time_seconds and other schema updates. |
| src/components/tables/BuildTable.vue | Adds a “Build Time” column with formatting + fallback calculation. |
|
Hi! Thanks for taking the time to share this. We expect each contributor to write their own short explanation (our explanation expectation) of the user problem, the change it addresses, and the way it was verified. Right now the description looks like the standard AI-generated summary, so we can't tell whether the UX/validation matches the roadmap. I'm closing this PR for now so it doesn't linger; please feel free to reopen or open a fresh PR once you can describe the behavior, thinking, and test steps yourself and we'll take another look. |
- vite: remove hard-coded allowedHosts (was dev-only, shouldn't be committed globally) - cron_reconcile_build_status: persist build_time_seconds when reconciling stale builds, same as status.ts does — prevents dashboard from falling back to inaccurate updated_at - created_at for builds finalized via cron reconciliation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e59767eddb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (seconds == null) { | ||
| const terminalStatuses = ['completed', 'succeeded', 'failed'] | ||
| if (!terminalStatuses.includes(elem.status) || !elem.created_at || !elem.updated_at) | ||
| return '-' | ||
| const diffMs = new Date(elem.updated_at).getTime() - new Date(elem.created_at).getTime() | ||
| if (diffMs < 0) | ||
| return '-' | ||
| seconds = Math.floor(diffMs / 1000) |
There was a problem hiding this comment.
When build_time_seconds is null, this falls back to updated_at - created_at, but the customer-facing demo seeding in supabase/functions/_backend/public/app/demo.ts:759-777 only backdates created_at and leaves updated_at at the table default (now()). That makes the new column render multi-day durations for demo builds from 1/4/7/13 days ago, so the build-time feature looks obviously broken in demo apps even though those rows were meant to showcase successful builds.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
|
Thanks for the contribution. This PR looks like a low-signal or template-style submission, which is hard to review and maintain. Please submit a focused PR with clear problem context, concrete diffs, and a short validation plan. |
Summary (AI generated)
created_attoupdated_atfor completed/succeeded/failed builds-for pending/running buildsXs,Xm Ys, orXh Ymdepending on durationMotivation (AI generated)
Users couldn't see how long their builds took at a glance in the dashboard. This information helps developers understand build performance and identify slow builds.
Business Impact (AI generated)
Improves developer experience for native build users by surfacing build duration directly in the builds table, reducing the need to manually calculate times from timestamps.
Test Plan (AI generated)
7m 35s)3m 48s)1h 12m)-Generated with AI
Summary by CodeRabbit
New Features
Chores