Add admin dashboard, bulk actions, filters, and timeline#551
Open
frostebite wants to merge 3 commits intomainfrom
Open
Add admin dashboard, bulk actions, filters, and timeline#551frostebite wants to merge 3 commits intomainfrom
frostebite wants to merge 3 commits intomainfrom
Conversation
- Build status dashboard: live counts of published/in-progress/failed/stuck builds - Bulk retry: checkbox selection on failed builds with bulk reset and retry actions - Build logs viewer: failure reason shown inline, expandable build log in details - Filter/search: search by version/platform/OS, filter by status (published, failed, stuck) - Auto-refresh: Firestore live listeners with green "Live" indicator - Build timeline: timestamps for created, last started, last failure, published dates Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive build management and monitoring system for the documentation dashboard, including real-time build statistics, admin-gated bulk actions for failed builds, repo version drift detection, and filtering capabilities across the versions interface. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant UI as Build Row<br/>(Selection)
participant State as Builds State<br/>(Selected Set)
participant Auth as useUser<br/>(ID Token)
participant Backend as Backend API<br/>(resetFailedBuilds/<br/>retryBuild)
participant Notify as useNotification
Admin->>UI: Toggle build selection
UI->>State: onToggleSelect() updates Set
State->>State: Track selected build IDs
Admin->>UI: Click bulk action button
UI->>Auth: Request ID token
Auth-->>UI: Return auth token
UI->>Backend: Batch 1 (5 builds max)<br/>resetFailedBuilds/retryBuild
UI->>Backend: Batch 2 (remaining builds)
Backend-->>UI: Promise.allSettled results<br/>(success/error per build)
UI->>Notify: Display aggregated counts<br/>(X succeeded, Y failed)
UI->>State: Clear selected set
Notify-->>Admin: Toast notification
sequenceDiagram
participant UI as QueueManagementPanel
participant Backend as Backend API<br/>(/queueStatus)
participant Compute as useMemo<br/>(Diagnostics)
participant Display as Diagnostic Tables
UI->>Backend: Fetch queue status on mount
Backend-->>UI: Return jobs & builds array
UI->>Compute: Compute derived diagnostics
Compute->>Compute: Count total jobs/builds
Compute->>Compute: Filter older-version jobs
Compute->>Compute: Detect repo-version drift<br/>(job vs build versions)
Compute->>Compute: Extract Docker digests
Compute-->>UI: Return diagnostics object
UI->>Display: Render 4 tables (max 12 rows each)
Display->>Display: Job IDs, repo versions, actions
Display->>Display: Build IDs, repo digests, actions
Display-->>UI: Displayed diagnostics dashboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
|
|
Visit the preview URL for this PR (updated for commit 8d1c2e6): https://game-ci-5559f--pr551-feat-admin-dashboard-0cw32gl5.web.app (expires Wed, 08 Apr 2026 16:40:31 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 1f0574f15f83e11bfc148eae8646486a6d0e078b |
- Destructure build props to satisfy unicorn/consistent-destructuring - Add aria-label to fallback td for jsx-a11y/control-has-associated-label - Add eslint-disable for intentional sequential batching (no-await-in-loop) - Remove interactive role from non-interactive td element Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/docs/versions/image-versions.tsx (1)
1-9:⚠️ Potential issue | 🟡 MinorLint: import order violation.
Static analysis flags that the
@site/src/components/auth/safe-auth-checkimport at line 8 should occur before the relative imports starting at line 3.🔧 Proposed fix
import React, { useState } from 'react'; import SignInSignOutButton from '@site/src/components/auth/sign-in-sign-out-button'; +import { SimpleAuthCheck } from '@site/src/components/auth/safe-auth-check'; import CleanUpStuckBuildsButton from './clean-up-stuck-builds-button'; import ResetAllFailedBuildsButton from './reset-all-failed-builds-button'; import BuildStatusDashboard from './build-status-dashboard'; import UnityVersions from './unity-versions'; import QueueManagementPanel from './queue-management-panel'; -import { SimpleAuthCheck } from '@site/src/components/auth/safe-auth-check'; import styles from './unity-version.module.scss';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/docs/versions/image-versions.tsx` around lines 1 - 9, Reorder the imports so the package-scoped import SimpleAuthCheck from '@site/src/components/auth/safe-auth-check' appears before the relative imports (CleanUpStuckBuildsButton, ResetAllFailedBuildsButton, BuildStatusDashboard, UnityVersions, QueueManagementPanel) in the image-versions.tsx module; update the import block so React and third-party/@site imports (including SignInSignOutButton and SimpleAuthCheck) come first, followed by the relative component imports, preserving existing identifiers and styles import (styles from './unity-version.module.scss').
🧹 Nitpick comments (6)
src/components/docs/versions/builds/build-row.tsx (1)
61-62: Magic number 15 hardcoded here as well.This is another location where the "stuck" threshold of 15 is hardcoded. As noted in
build-status-dashboard.tsx, consider extracting this to a shared constant to prevent drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/docs/versions/builds/build-row.tsx` around lines 61 - 62, The hardcoded "15" stuck-threshold in build-row.tsx (used when computing count and label from rawMeta/failureCount) should be replaced with a shared constant (e.g., STUCK_THRESHOLD) exported from the same module used by build-status-dashboard.tsx (or a new shared constants module); update build-row.tsx to import that constant and use STUCK_THRESHOLD instead of the literal 15 when computing the label and any related logic so the threshold is consistent across components.src/components/docs/versions/build-status-dashboard.tsx (1)
30-32: Extract magic number 15 to a shared constant.The threshold
15for "stuck" builds is hardcoded here and in at least three other locations (docker-image-link-or-retry-button.tsx,build-row.tsx,reset-all-failed-builds-button.tsx). This creates drift risk if the threshold changes.♻️ Suggested approach
Create a shared constant file or add to an existing config:
// e.g., src/core/constants.ts export const MAX_FAILURE_COUNT = 15;Then import and use it across all files:
+import { MAX_FAILURE_COUNT } from '@site/src/core/constants'; ... const maxedOut = builds.filter( - (b) => b.status === 'failed' && (b.meta?.failureCount || 0) >= 15, + (b) => b.status === 'failed' && (b.meta?.failureCount || 0) >= MAX_FAILURE_COUNT, ).length;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/docs/versions/build-status-dashboard.tsx` around lines 30 - 32, The hardcoded threshold 15 used when computing maxedOut (in the builds filter where b.meta?.failureCount is compared) should be extracted to a shared constant (e.g., export const MAX_FAILURE_COUNT = 15 in a central constants module such as src/core/constants.ts); update the reference in this file (symbol: maxedOut) to import and use MAX_FAILURE_COUNT instead of the literal 15, and do the same replacement in the other components named in the review (docker-image-link-or-retry-button.tsx, build-row.tsx, reset-all-failed-builds-button.tsx) so all locations use the single exported MAX_FAILURE_COUNT constant.src/components/docs/versions/unity-versions.tsx (2)
44-50: RedundanttoLowerCase()call.On line 45,
idis assigned(version.NO_ID_FIELD || '').toLowerCase(), then line 49 callsid.toLowerCase()again. The second call is unnecessary.♻️ Proposed fix
filtered = filtered.filter((version) => { const id = (version.NO_ID_FIELD || '').toLowerCase(); const editor = version.editorVersionInfo ? `${version.editorVersionInfo.major}.${version.editorVersionInfo.minor}.${version.editorVersionInfo.patch}` : ''; - return id.toLowerCase().includes(q) || editor.toLowerCase().includes(q); + return id.includes(q) || editor.toLowerCase().includes(q); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/docs/versions/unity-versions.tsx` around lines 44 - 50, The code redundantly calls toLowerCase() on id twice; since id is already set with (version.NO_ID_FIELD || '').toLowerCase(), remove the second call and change the comparisons to use id.includes(q) and editor.includes(q) (keeping editor lowercased as assigned) — ensure q is the lowercased search term used in this filter context; update the filter block around filtered, id, editor, and q accordingly.
85-96: Using.map()for side effects is an anti-pattern.The
.map()call here doesn't use its return value—it's only used to populate theversionsobject. Preferfor...ofor.forEach()for clarity when side effects are the intent.♻️ Proposed fix
- filtered.map((version) => { + for (const version of filtered) { if (Number.parseInt(version.editorVersionInfo.major, 10) <= 2017) return version; if (!versions[version.editorVersionInfo.major]) versions[version.editorVersionInfo.major] = []; versions[version.editorVersionInfo.major] = [ ...versions[version.editorVersionInfo.major], version, ]; - return version; - }); + }Note: The
return versionfor entries ≤ 2017 becomes acontinuestatement in the loop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/docs/versions/unity-versions.tsx` around lines 85 - 96, Replace the use of filtered.map (which is used for side effects) with an explicit loop such as for...of or filtered.forEach so the intent is clear; iterate over filtered, use the local variable version and check Number.parseInt(version.editorVersionInfo.major, 10) <= 2017 then continue (instead of returning), and otherwise ensure versions[version.editorVersionInfo.major] is initialized and push the version into that array (using versions[...] = [...versions[...], version] or .push) so no map return value is ignored and side effects are explicit.src/components/docs/versions/image-versions.tsx (1)
91-91: UI text also hardcodes the 15 threshold.The option label "Stuck (15+ failures)" hardcodes the threshold. If a shared constant is created as suggested earlier, this label should derive from it to stay in sync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/docs/versions/image-versions.tsx` at line 91, Update the hardcoded option label so it derives from the shared threshold constant instead of the literal "15"; import the shared constant (e.g. STUCK_FAILURE_THRESHOLD) into the image-versions component and replace the option text "Stuck (15+ failures)" with a computed label like `Stuck (${STUCK_FAILURE_THRESHOLD}+ failures)` (or use a small helper to format the label) so the UI always reflects the canonical threshold used elsewhere; update the JSX option value/label in image-versions.tsx accordingly.src/components/docs/versions/builds/builds.tsx (1)
30-47: Consider reusinguseAuthenticatedEndpointpattern.The codebase has an existing
useAuthenticatedEndpointhook (seen indocker-image-link-or-retry-button.tsx) for authenticated API calls. While the bulk batching logic here has different requirements, consider extracting the common auth + fetch pattern to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/docs/versions/builds/builds.tsx` around lines 30 - 47, The local callEndpoint function duplicates the auth+fetch pattern; replace it by reusing or extracting the shared logic from the existing useAuthenticatedEndpoint hook used in docker-image-link-or-retry-button.tsx: either call useAuthenticatedEndpoint from builds.tsx (and adapt it to accept POST+JSON body) or factor out a small helper like fetchWithAuth/getAuthHeaders used by both useAuthenticatedEndpoint and the bulk-batching logic; update references to callEndpoint to use the shared hook/helper and ensure it still returns parsed JSON and throws on non-2xx responses (preserving the current error-detail behavior).
🤖 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/docs/versions/build-status-dashboard.tsx`:
- Around line 9-16: Move the hook calls out of the early-return path in
BuildStatusDashboard so useFirestore() and useFirestoreCollectionData(...) are
invoked unconditionally; create ciBuilds based on selectedRepoVersion after
calling useFirestore (e.g., build a query variable that is null/placeholder when
selectedRepoVersion is undefined, or use a Firestore “skip” pattern) and pass
that into useFirestoreCollectionData so the hooks are always called in the same
order; then perform the early return (or render a loading/empty state) if
selectedRepoVersion is falsy or if the query was skipped.
- Around line 84-95: The span element using inline style animation: 'pulse 2s
infinite' references a non-existent keyframes animation; to fix, either add a
matching `@keyframes` pulse definition to your global CSS or component CSS module
(so the inline animation name resolves), or change the inline animation to an
existing animation name (e.g., 'spin 2s linear infinite') that is already
defined; locate the offending inline style on the span with animation: 'pulse 2s
infinite' in the build-status-dashboard component and update it accordingly.
In `@src/components/docs/versions/builds/build-failure-details.tsx`:
- Around line 19-23: The code accesses repoVersion and version via property
access even though buildInfo and repoVersionInfo are available; destructure
repoVersion from ciBuild.buildInfo alongside editorVersion/baseOs/targetPlatform
and destructure version from repoVersionInfo alongside major/minor/patch, then
replace buildRepoVersion and jobRepoVersion assignments to use those
destructured variables so hasRepoVersionDrift compares the destructured values
(references: ciBuild.buildInfo, repoVersionInfo, buildRepoVersion,
jobRepoVersion, hasRepoVersionDrift).
In `@src/components/docs/versions/builds/build-row.tsx`:
- Around line 175-185: The inline failure row uses colSpan={6} but the table
actually has 8 columns; update the <tr> rendering the inline failure (the
condition using status === 'failed' && failure?.reason && !expanded) to use
colSpan={8} to match the expanded row and the table columns so the
failure.reason slice spans the full row.
In `@src/components/docs/versions/builds/builds.tsx`:
- Around line 28-47: The callEndpoint function calls user.getIdToken() without
checking that user from useUser() is defined; update callEndpoint to guard
against a missing user (from useUser) by checking user is truthy before calling
getIdToken(), and if not present either throw a clear Unauthorized/Error or
return a rejected Promise so callers handle it; specifically modify the
callEndpoint function (which currently calls user.getIdToken()) to do an early
check like if (!user) { throw new Error('Not authenticated') } or similar and
then proceed to fetch with the token.
In `@src/components/docs/versions/queue-management-panel.tsx`:
- Around line 104-109: Replace the Array#forEach usage over jobs with a for...of
loop to satisfy lint rules: iterate using "for (const job of jobs)" (or similar)
and inside compute the id via job.NO_ID_FIELD || job.id and set it into jobMap
with jobMap.set(id, job) only when id is truthy; update the block that currently
uses jobs.forEach(...) to reference the same identifiers (jobs, job,
NO_ID_FIELD, id, jobMap) but implemented as a for...of loop.
- Around line 187-201: The onClick handler uses "void fetchQueueStatus()" which
the linter flags; update the button's onClick to call fetchQueueStatus without
using "void" and ensure the handler returns undefined (e.g., onClick={() => {
fetchQueueStatus(); return undefined; }}) or make the handler async and await
fetchQueueStatus; change the anonymous handler attached to the button (the
onClick that invokes fetchQueueStatus) accordingly to remove the "void" usage.
- Around line 81-92: The fetch block calls response.json() before checking
response.ok which can throw on non-JSON error pages; update the logic in the
async fetch routine (the code that uses
fetch(`${config.backendUrl}/queueStatus``, response.json(), setData, setError,
setLoading) so that you first inspect response.ok (and optionally
response.headers.get('content-type')) and only parse JSON if the content-type is
application/json, otherwise parse response.text() as a fallback or wrap
response.json() in its own try/catch to capture parsing errors and use that
text/error to construct a meaningful message passed to setError; ensure
setLoading(false) remains in finally.
- Around line 95-97: The linter flags the use of "void fetchQueueStatus()" in
the useEffect; replace the discard-with-void pattern by explicitly handling the
promise returned by fetchQueueStatus() — e.g., invoke fetchQueueStatus() inside
an async IIFE within useEffect (async () => { await fetchQueueStatus(); })() or
call fetchQueueStatus().catch(err => {/* handle/log err */}); update the
useEffect body where fetchQueueStatus is invoked (referencing useEffect and
fetchQueueStatus) or, if you intentionally want to ignore the promise, add a
focused eslint-disable-next-line with justification instead.
In `@src/components/docs/versions/unity-versions.tsx`:
- Around line 31-39: The "stuck" branch of the status filter currently only
checks version.status === 'failed' and should also require version.failureCount
>= 15; update the filtered = filtered.filter(...) logic (the block referencing
statusFilter and version) to return version.status === 'failed' &&
version.failureCount >= STUCK_THRESHOLD (or >= 15) when statusFilter ===
'stuck', and if there is an existing threshold constant used elsewhere (e.g., in
image-versions.tsx or build-status-dashboard.tsx), import/ reuse that constant
instead of hardcoding 15.
---
Outside diff comments:
In `@src/components/docs/versions/image-versions.tsx`:
- Around line 1-9: Reorder the imports so the package-scoped import
SimpleAuthCheck from '@site/src/components/auth/safe-auth-check' appears before
the relative imports (CleanUpStuckBuildsButton, ResetAllFailedBuildsButton,
BuildStatusDashboard, UnityVersions, QueueManagementPanel) in the
image-versions.tsx module; update the import block so React and
third-party/@site imports (including SignInSignOutButton and SimpleAuthCheck)
come first, followed by the relative component imports, preserving existing
identifiers and styles import (styles from './unity-version.module.scss').
---
Nitpick comments:
In `@src/components/docs/versions/build-status-dashboard.tsx`:
- Around line 30-32: The hardcoded threshold 15 used when computing maxedOut (in
the builds filter where b.meta?.failureCount is compared) should be extracted to
a shared constant (e.g., export const MAX_FAILURE_COUNT = 15 in a central
constants module such as src/core/constants.ts); update the reference in this
file (symbol: maxedOut) to import and use MAX_FAILURE_COUNT instead of the
literal 15, and do the same replacement in the other components named in the
review (docker-image-link-or-retry-button.tsx, build-row.tsx,
reset-all-failed-builds-button.tsx) so all locations use the single exported
MAX_FAILURE_COUNT constant.
In `@src/components/docs/versions/builds/build-row.tsx`:
- Around line 61-62: The hardcoded "15" stuck-threshold in build-row.tsx (used
when computing count and label from rawMeta/failureCount) should be replaced
with a shared constant (e.g., STUCK_THRESHOLD) exported from the same module
used by build-status-dashboard.tsx (or a new shared constants module); update
build-row.tsx to import that constant and use STUCK_THRESHOLD instead of the
literal 15 when computing the label and any related logic so the threshold is
consistent across components.
In `@src/components/docs/versions/builds/builds.tsx`:
- Around line 30-47: The local callEndpoint function duplicates the auth+fetch
pattern; replace it by reusing or extracting the shared logic from the existing
useAuthenticatedEndpoint hook used in docker-image-link-or-retry-button.tsx:
either call useAuthenticatedEndpoint from builds.tsx (and adapt it to accept
POST+JSON body) or factor out a small helper like fetchWithAuth/getAuthHeaders
used by both useAuthenticatedEndpoint and the bulk-batching logic; update
references to callEndpoint to use the shared hook/helper and ensure it still
returns parsed JSON and throws on non-2xx responses (preserving the current
error-detail behavior).
In `@src/components/docs/versions/image-versions.tsx`:
- Line 91: Update the hardcoded option label so it derives from the shared
threshold constant instead of the literal "15"; import the shared constant (e.g.
STUCK_FAILURE_THRESHOLD) into the image-versions component and replace the
option text "Stuck (15+ failures)" with a computed label like `Stuck
(${STUCK_FAILURE_THRESHOLD}+ failures)` (or use a small helper to format the
label) so the UI always reflects the canonical threshold used elsewhere; update
the JSX option value/label in image-versions.tsx accordingly.
In `@src/components/docs/versions/unity-versions.tsx`:
- Around line 44-50: The code redundantly calls toLowerCase() on id twice; since
id is already set with (version.NO_ID_FIELD || '').toLowerCase(), remove the
second call and change the comparisons to use id.includes(q) and
editor.includes(q) (keeping editor lowercased as assigned) — ensure q is the
lowercased search term used in this filter context; update the filter block
around filtered, id, editor, and q accordingly.
- Around line 85-96: Replace the use of filtered.map (which is used for side
effects) with an explicit loop such as for...of or filtered.forEach so the
intent is clear; iterate over filtered, use the local variable version and check
Number.parseInt(version.editorVersionInfo.major, 10) <= 2017 then continue
(instead of returning), and otherwise ensure
versions[version.editorVersionInfo.major] is initialized and push the version
into that array (using versions[...] = [...versions[...], version] or .push) so
no map return value is ignored and side effects are explicit.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5980739-6412-4bf1-9377-6a38b1e3a432
📒 Files selected for processing (7)
src/components/docs/versions/build-status-dashboard.tsxsrc/components/docs/versions/builds/build-failure-details.tsxsrc/components/docs/versions/builds/build-row.tsxsrc/components/docs/versions/builds/builds.tsxsrc/components/docs/versions/image-versions.tsxsrc/components/docs/versions/queue-management-panel.tsxsrc/components/docs/versions/unity-versions.tsx
Comment on lines
+9
to
+16
| const BuildStatusDashboard = ({ selectedRepoVersion }: Props) => { | ||
| if (!selectedRepoVersion) return null; | ||
|
|
||
| const ciBuilds = useFirestore() | ||
| .collection('ciBuilds') | ||
| .where('buildInfo.repoVersion', '==', selectedRepoVersion); | ||
|
|
||
| const { status, data } = useFirestoreCollectionData<{ [key: string]: any }>(ciBuilds); |
There was a problem hiding this comment.
React hooks rule violation: early return before hooks.
The conditional return on line 10 occurs before the useFirestore and useFirestoreCollectionData hooks are called on lines 12 and 16. This violates React's rules of hooks—hooks must be called unconditionally and in the same order on every render.
🐛 Proposed fix: move early return after hooks
const BuildStatusDashboard = ({ selectedRepoVersion }: Props) => {
- if (!selectedRepoVersion) return null;
-
const ciBuilds = useFirestore()
.collection('ciBuilds')
.where('buildInfo.repoVersion', '==', selectedRepoVersion);
const { status, data } = useFirestoreCollectionData<{ [key: string]: any }>(ciBuilds);
+ if (!selectedRepoVersion) return null;
+
if (status === 'loading') {Note: You may also need to handle the case where selectedRepoVersion is undefined in the Firestore query, or conditionally skip the query entirely using a pattern like passing a dummy value when undefined.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/docs/versions/build-status-dashboard.tsx` around lines 9 - 16,
Move the hook calls out of the early-return path in BuildStatusDashboard so
useFirestore() and useFirestoreCollectionData(...) are invoked unconditionally;
create ciBuilds based on selectedRepoVersion after calling useFirestore (e.g.,
build a query variable that is null/placeholder when selectedRepoVersion is
undefined, or use a Firestore “skip” pattern) and pass that into
useFirestoreCollectionData so the hooks are always called in the same order;
then perform the early return (or render a loading/empty state) if
selectedRepoVersion is falsy or if the query was skipped.
Comment on lines
+84
to
+95
| <span | ||
| style={{ | ||
| width: 6, | ||
| height: 6, | ||
| borderRadius: '50%', | ||
| background: '#22c55e', | ||
| display: 'inline-block', | ||
| animation: 'pulse 2s infinite', | ||
| }} | ||
| /> | ||
| Live | ||
| </span> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for `@keyframes` pulse definition in the codebase
rg -n '@keyframes\s+pulse' --type=css --type=scssRepository: game-ci/documentation
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Search for `@keyframes` pulse in all files
rg -n '@keyframes\s+pulse' .
# Also search for pulse animation in CSS/SCSS files
echo "=== Searching in .css and .scss files ==="
rg -n 'pulse' --type-list | grep -E 'css|style'Repository: game-ci/documentation
Length of output: 513
🏁 Script executed:
#!/bin/bash
# Search for CSS/style related files
fd -e css -e scss -e module.css
# Check the reviewed file for imports
echo "=== Checking imports in build-status-dashboard.tsx ==="
head -20 src/components/docs/versions/build-status-dashboard.tsx
# Search for animation definitions in global styles or Tailwind
echo "=== Searching for global CSS files ==="
fd -E node_modules -e css -e scss | grep -E '(global|tailwind|index)'Repository: game-ci/documentation
Length of output: 1066
🏁 Script executed:
#!/bin/bash
# Check if Tailwind is used and if pulse is a Tailwind animation
rg -n 'class=".*pulse' . --type-list | head -20
# Search broadly for "pulse" in the codebase
rg -n '"pulse"' . -A 2 -B 2 | head -50Repository: game-ci/documentation
Length of output: 910
🏁 Script executed:
#!/bin/bash
# Check the Spinner component and its styles
echo "=== spinner.module.scss ==="
cat src/components/molecules/spinner.module.scss
echo -e "\n=== Spinner component source ==="
cat src/components/molecules/spinner.tsx
# Also check custom.css for global animations
echo -e "\n=== custom.css ==="
cat src/css/custom.css | head -100Repository: game-ci/documentation
Length of output: 3763
CSS animation pulse is not defined and will not work.
The inline style references animation: 'pulse 2s infinite' but there is no @keyframes pulse definition in the codebase. Add the keyframe animation to a global stylesheet or CSS module, or replace 'pulse' with an existing animation such as 'spin'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/docs/versions/build-status-dashboard.tsx` around lines 84 -
95, The span element using inline style animation: 'pulse 2s infinite'
references a non-existent keyframes animation; to fix, either add a matching
`@keyframes` pulse definition to your global CSS or component CSS module (so the
inline animation name resolves), or change the inline animation to an existing
animation name (e.g., 'spin 2s linear infinite') that is already defined; locate
the offending inline style on the span with animation: 'pulse 2s infinite' in
the build-status-dashboard component and update it accordingly.
Comment on lines
19
to
+23
| const { editorVersion, baseOs, targetPlatform } = ciBuild.buildInfo; | ||
| const { major, minor, patch } = repoVersionInfo; | ||
| const buildRepoVersion = ciBuild.buildInfo.repoVersion; | ||
| const jobRepoVersion = repoVersionInfo.version; | ||
| const hasRepoVersionDrift = jobRepoVersion !== buildRepoVersion; |
There was a problem hiding this comment.
Lint: use destructured variables over property access.
Static analysis flags lines 21-22. Since buildInfo is already destructured from ciBuild, and repoVersionInfo is available, prefer extracting these values via destructuring.
🔧 Proposed fix
- const { editorVersion, baseOs, targetPlatform } = ciBuild.buildInfo;
- const { major, minor, patch } = repoVersionInfo;
- const buildRepoVersion = ciBuild.buildInfo.repoVersion;
- const jobRepoVersion = repoVersionInfo.version;
+ const { editorVersion, baseOs, targetPlatform, repoVersion: buildRepoVersion } = ciBuild.buildInfo;
+ const { major, minor, patch, version: jobRepoVersion } = repoVersionInfo;
const hasRepoVersionDrift = jobRepoVersion !== buildRepoVersion;📝 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.
Suggested change
| const { editorVersion, baseOs, targetPlatform } = ciBuild.buildInfo; | |
| const { major, minor, patch } = repoVersionInfo; | |
| const buildRepoVersion = ciBuild.buildInfo.repoVersion; | |
| const jobRepoVersion = repoVersionInfo.version; | |
| const hasRepoVersionDrift = jobRepoVersion !== buildRepoVersion; | |
| const { editorVersion, baseOs, targetPlatform, repoVersion: buildRepoVersion } = ciBuild.buildInfo; | |
| const { major, minor, patch, version: jobRepoVersion } = repoVersionInfo; | |
| const hasRepoVersionDrift = jobRepoVersion !== buildRepoVersion; |
🧰 Tools
🪛 GitHub Check: Code styles
[failure] 22-22:
Use destructured variables over properties
[failure] 21-21:
Use destructured variables over properties
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/docs/versions/builds/build-failure-details.tsx` around lines
19 - 23, The code accesses repoVersion and version via property access even
though buildInfo and repoVersionInfo are available; destructure repoVersion from
ciBuild.buildInfo alongside editorVersion/baseOs/targetPlatform and destructure
version from repoVersionInfo alongside major/minor/patch, then replace
buildRepoVersion and jobRepoVersion assignments to use those destructured
variables so hasRepoVersionDrift compares the destructured values (references:
ciBuild.buildInfo, repoVersionInfo, buildRepoVersion, jobRepoVersion,
hasRepoVersionDrift).
Comment on lines
+175
to
+185
| {/* Inline failure reason row (visible without expanding) */} | ||
| {status === 'failed' && failure?.reason && !expanded && ( | ||
| <tr> | ||
| <td aria-label="spacer" /> | ||
| <td aria-label="spacer" /> | ||
| <td colSpan={6} style={{ padding: '2px 8px', fontSize: '0.8em', opacity: 0.7 }}> | ||
| {failure.reason.slice(0, 200)} | ||
| {failure.reason.length > 200 ? '...' : ''} | ||
| </td> | ||
| </tr> | ||
| )} |
There was a problem hiding this comment.
Verify colSpan matches actual column count.
The inline failure row uses colSpan={6} at line 180, but reviewing the table structure shows 8 columns (checkbox, expand, status, build ID, repo version, image type, OS, platform). The expanded row at line 188 uses colSpan={8}. These should be consistent.
🔧 Proposed fix
- <td colSpan={6} style={{ padding: '2px 8px', fontSize: '0.8em', opacity: 0.7 }}>
+ <td colSpan={8} style={{ padding: '2px 8px', fontSize: '0.8em', opacity: 0.7 }}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/docs/versions/builds/build-row.tsx` around lines 175 - 185,
The inline failure row uses colSpan={6} but the table actually has 8 columns;
update the <tr> rendering the inline failure (the condition using status ===
'failed' && failure?.reason && !expanded) to use colSpan={8} to match the
expanded row and the table columns so the failure.reason slice spans the full
row.
Comment on lines
+28
to
+47
| const { data: user } = useUser(); | ||
|
|
||
| const callEndpoint = async (endpoint: string, payload: object) => { | ||
| const token = await user.getIdToken(); | ||
| const response = await fetch(`${config.backendUrl}/${endpoint}`, { | ||
| headers: { | ||
| Authorization: `Bearer ${token}`, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| mode: 'cors', | ||
| method: 'POST', | ||
| body: JSON.stringify(payload), | ||
| }); | ||
| const body = await response.json(); | ||
| if (!response.ok) { | ||
| const detail = body.error ? `${body.message}: ${body.error}` : body.message; | ||
| throw new Error(detail || `Request failed (${response.status})`); | ||
| } | ||
| return body; | ||
| }; |
There was a problem hiding this comment.
Potential null reference: user may be undefined.
useUser() returns { data: user } where user can be null or undefined if not authenticated. Line 31 calls user.getIdToken() without a null check. While BulkActions is wrapped in SimpleAuthCheck, the hook still runs unconditionally.
🐛 Proposed fix
const callEndpoint = async (endpoint: string, payload: object) => {
+ if (!user) {
+ throw new Error('User not authenticated');
+ }
const token = await user.getIdToken();📝 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.
Suggested change
| const { data: user } = useUser(); | |
| const callEndpoint = async (endpoint: string, payload: object) => { | |
| const token = await user.getIdToken(); | |
| const response = await fetch(`${config.backendUrl}/${endpoint}`, { | |
| headers: { | |
| Authorization: `Bearer ${token}`, | |
| 'Content-Type': 'application/json', | |
| }, | |
| mode: 'cors', | |
| method: 'POST', | |
| body: JSON.stringify(payload), | |
| }); | |
| const body = await response.json(); | |
| if (!response.ok) { | |
| const detail = body.error ? `${body.message}: ${body.error}` : body.message; | |
| throw new Error(detail || `Request failed (${response.status})`); | |
| } | |
| return body; | |
| }; | |
| const { data: user } = useUser(); | |
| const callEndpoint = async (endpoint: string, payload: object) => { | |
| if (!user) { | |
| throw new Error('User not authenticated'); | |
| } | |
| const token = await user.getIdToken(); | |
| const response = await fetch(`${config.backendUrl}/${endpoint}`, { | |
| headers: { | |
| Authorization: `Bearer ${token}`, | |
| 'Content-Type': 'application/json', | |
| }, | |
| mode: 'cors', | |
| method: 'POST', | |
| body: JSON.stringify(payload), | |
| }); | |
| const body = await response.json(); | |
| if (!response.ok) { | |
| const detail = body.error ? `${body.message}: ${body.error}` : body.message; | |
| throw new Error(detail || `Request failed (${response.status})`); | |
| } | |
| return body; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/docs/versions/builds/builds.tsx` around lines 28 - 47, The
callEndpoint function calls user.getIdToken() without checking that user from
useUser() is defined; update callEndpoint to guard against a missing user (from
useUser) by checking user is truthy before calling getIdToken(), and if not
present either throw a clear Unauthorized/Error or return a rejected Promise so
callers handle it; specifically modify the callEndpoint function (which
currently calls user.getIdToken()) to do an early check like if (!user) { throw
new Error('Not authenticated') } or similar and then proceed to fetch with the
token.
Comment on lines
+81
to
+92
| try { | ||
| const response = await fetch(`${config.backendUrl}/queueStatus`); | ||
| const body = await response.json(); | ||
| if (!response.ok) { | ||
| throw new Error(body.message || `Request failed (${response.status})`); | ||
| } | ||
| setData(body); | ||
| } catch (fetchError: any) { | ||
| setError(fetchError.message || 'Failed to load queue status'); | ||
| } finally { | ||
| setLoading(false); | ||
| } |
There was a problem hiding this comment.
Potential issue: response.json() called before checking response.ok.
If the server returns a non-JSON error response (e.g., an HTML 500 page), response.json() will throw before you can extract a meaningful error message. Consider checking the content-type or using a try-catch around the JSON parsing.
🛡️ Suggested defensive approach
const response = await fetch(`${config.backendUrl}/queueStatus`);
+ if (!response.ok) {
+ let message = `Request failed (${response.status})`;
+ try {
+ const body = await response.json();
+ message = body.message || message;
+ } catch { /* non-JSON response */ }
+ throw new Error(message);
+ }
const body = await response.json();
- if (!response.ok) {
- throw new Error(body.message || `Request failed (${response.status})`);
- }
setData(body);📝 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.
Suggested change
| try { | |
| const response = await fetch(`${config.backendUrl}/queueStatus`); | |
| const body = await response.json(); | |
| if (!response.ok) { | |
| throw new Error(body.message || `Request failed (${response.status})`); | |
| } | |
| setData(body); | |
| } catch (fetchError: any) { | |
| setError(fetchError.message || 'Failed to load queue status'); | |
| } finally { | |
| setLoading(false); | |
| } | |
| try { | |
| const response = await fetch(`${config.backendUrl}/queueStatus`); | |
| if (!response.ok) { | |
| let message = `Request failed (${response.status})`; | |
| try { | |
| const body = await response.json(); | |
| message = body.message || message; | |
| } catch { /* non-JSON response */ } | |
| throw new Error(message); | |
| } | |
| const body = await response.json(); | |
| setData(body); | |
| } catch (fetchError: any) { | |
| setError(fetchError.message || 'Failed to load queue status'); | |
| } finally { | |
| setLoading(false); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/docs/versions/queue-management-panel.tsx` around lines 81 -
92, The fetch block calls response.json() before checking response.ok which can
throw on non-JSON error pages; update the logic in the async fetch routine (the
code that uses fetch(`${config.backendUrl}/queueStatus``, response.json(),
setData, setError, setLoading) so that you first inspect response.ok (and
optionally response.headers.get('content-type')) and only parse JSON if the
content-type is application/json, otherwise parse response.text() as a fallback
or wrap response.json() in its own try/catch to capture parsing errors and use
that text/error to construct a meaningful message passed to setError; ensure
setLoading(false) remains in finally.
Comment on lines
+95
to
+97
| useEffect(() => { | ||
| void fetchQueueStatus(); | ||
| }, []); |
There was a problem hiding this comment.
Lint: prefer undefined over void for discarding promises.
Static analysis flags the use of void at line 96. The pattern void fetchQueueStatus() is flagged because the linter expects undefined (or a different pattern) for intentionally discarding a promise.
🔧 Proposed fix
useEffect(() => {
- void fetchQueueStatus();
+ fetchQueueStatus();
}, []);If the linter requires explicit handling, wrap in an IIFE or add an eslint-disable comment with justification.
📝 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.
Suggested change
| useEffect(() => { | |
| void fetchQueueStatus(); | |
| }, []); | |
| useEffect(() => { | |
| fetchQueueStatus(); | |
| }, []); |
🧰 Tools
🪛 GitHub Check: Code styles
[failure] 96-96:
Expected 'undefined' and instead saw 'void'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/docs/versions/queue-management-panel.tsx` around lines 95 -
97, The linter flags the use of "void fetchQueueStatus()" in the useEffect;
replace the discard-with-void pattern by explicitly handling the promise
returned by fetchQueueStatus() — e.g., invoke fetchQueueStatus() inside an async
IIFE within useEffect (async () => { await fetchQueueStatus(); })() or call
fetchQueueStatus().catch(err => {/* handle/log err */}); update the useEffect
body where fetchQueueStatus is invoked (referencing useEffect and
fetchQueueStatus) or, if you intentionally want to ignore the promise, add a
focused eslint-disable-next-line with justification instead.
Comment on lines
+104
to
+109
| jobs.forEach((job: any) => { | ||
| const id = job.NO_ID_FIELD || job.id; | ||
| if (id) { | ||
| jobMap.set(id, job); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Lint: replace Array#forEach with for...of loop.
Static analysis flags the use of forEach at line 104.
🔧 Proposed fix
- jobs.forEach((job: any) => {
- const id = job.NO_ID_FIELD || job.id;
- if (id) {
- jobMap.set(id, job);
- }
- });
+ for (const job of jobs) {
+ const id = (job as any).NO_ID_FIELD || (job as any).id;
+ if (id) {
+ jobMap.set(id, job);
+ }
+ }📝 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.
Suggested change
| jobs.forEach((job: any) => { | |
| const id = job.NO_ID_FIELD || job.id; | |
| if (id) { | |
| jobMap.set(id, job); | |
| } | |
| }); | |
| for (const job of jobs) { | |
| const id = (job as any).NO_ID_FIELD || (job as any).id; | |
| if (id) { | |
| jobMap.set(id, job); | |
| } | |
| } |
🧰 Tools
🪛 GitHub Check: Code styles
[failure] 104-104:
Do not use Array#forEach(…)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/docs/versions/queue-management-panel.tsx` around lines 104 -
109, Replace the Array#forEach usage over jobs with a for...of loop to satisfy
lint rules: iterate using "for (const job of jobs)" (or similar) and inside
compute the id via job.NO_ID_FIELD || job.id and set it into jobMap with
jobMap.set(id, job) only when id is truthy; update the block that currently uses
jobs.forEach(...) to reference the same identifiers (jobs, job, NO_ID_FIELD, id,
jobMap) but implemented as a for...of loop.
Comment on lines
+187
to
+201
| <button | ||
| type="button" | ||
| onClick={() => void fetchQueueStatus()} | ||
| style={{ | ||
| marginLeft: 'auto', | ||
| padding: '4px 8px', | ||
| borderRadius: 4, | ||
| border: '1px solid #ccc', | ||
| background: 'transparent', | ||
| cursor: 'pointer', | ||
| fontSize: '0.85em', | ||
| }} | ||
| > | ||
| Refresh queue state | ||
| </button> |
There was a problem hiding this comment.
Lint: prefer undefined over void in onClick handler.
Static analysis flags line 189 for using void to discard the promise.
🔧 Proposed fix
<button
type="button"
- onClick={() => void fetchQueueStatus()}
+ onClick={() => { fetchQueueStatus(); }}
style={{📝 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.
Suggested change
| <button | |
| type="button" | |
| onClick={() => void fetchQueueStatus()} | |
| style={{ | |
| marginLeft: 'auto', | |
| padding: '4px 8px', | |
| borderRadius: 4, | |
| border: '1px solid #ccc', | |
| background: 'transparent', | |
| cursor: 'pointer', | |
| fontSize: '0.85em', | |
| }} | |
| > | |
| Refresh queue state | |
| </button> | |
| <button | |
| type="button" | |
| onClick={() => { fetchQueueStatus(); }} | |
| style={{ | |
| marginLeft: 'auto', | |
| padding: '4px 8px', | |
| borderRadius: 4, | |
| border: '1px solid `#ccc`', | |
| background: 'transparent', | |
| cursor: 'pointer', | |
| fontSize: '0.85em', | |
| }} | |
| > | |
| Refresh queue state | |
| </button> |
🧰 Tools
🪛 GitHub Check: Code styles
[failure] 189-189:
Expected 'undefined' and instead saw 'void'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/docs/versions/queue-management-panel.tsx` around lines 187 -
201, The onClick handler uses "void fetchQueueStatus()" which the linter flags;
update the button's onClick to call fetchQueueStatus without using "void" and
ensure the handler returns undefined (e.g., onClick={() => { fetchQueueStatus();
return undefined; }}) or make the handler async and await fetchQueueStatus;
change the anonymous handler attached to the button (the onClick that invokes
fetchQueueStatus) accordingly to remove the "void" usage.
Comment on lines
+31
to
+39
| // Status filter | ||
| if (statusFilter !== 'all') { | ||
| filtered = filtered.filter((version) => { | ||
| if (statusFilter === 'stuck') { | ||
| return version.status === 'failed'; | ||
| } | ||
| return version.status === statusFilter; | ||
| }); | ||
| } |
There was a problem hiding this comment.
"Stuck" filter does not apply the failureCount >= 15 predicate.
The UI labels this filter as "Stuck (15+ failures)" (in image-versions.tsx line 91), but the filter logic only checks status === 'failed' without verifying failureCount >= 15. This will return all failed builds, not just those that are actually stuck.
Compare with build-status-dashboard.tsx line 30-32 which correctly applies both conditions for maxedOut.
🐛 Proposed fix
if (statusFilter !== 'all') {
filtered = filtered.filter((version) => {
if (statusFilter === 'stuck') {
- return version.status === 'failed';
+ return version.status === 'failed' && (version.meta?.failureCount || 0) >= 15;
}
return version.status === statusFilter;
});
}Note: If you extract the threshold to a constant, use that here as well.
📝 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.
Suggested change
| // Status filter | |
| if (statusFilter !== 'all') { | |
| filtered = filtered.filter((version) => { | |
| if (statusFilter === 'stuck') { | |
| return version.status === 'failed'; | |
| } | |
| return version.status === statusFilter; | |
| }); | |
| } | |
| // Status filter | |
| if (statusFilter !== 'all') { | |
| filtered = filtered.filter((version) => { | |
| if (statusFilter === 'stuck') { | |
| return version.status === 'failed' && (version.meta?.failureCount || 0) >= 15; | |
| } | |
| return version.status === statusFilter; | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/docs/versions/unity-versions.tsx` around lines 31 - 39, The
"stuck" branch of the status filter currently only checks version.status ===
'failed' and should also require version.failureCount >= 15; update the filtered
= filtered.filter(...) logic (the block referencing statusFilter and version) to
return version.status === 'failed' && version.failureCount >= STUCK_THRESHOLD
(or >= 15) when statusFilter === 'stuck', and if there is an existing threshold
constant used elsewhere (e.g., in image-versions.tsx or
build-status-dashboard.tsx), import/ reuse that constant instead of hardcoding
15.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive admin enhancements for the Docker versions page:
useFirestoreCollectionDataprovides real-time updates automatically. Added green "Live" indicator to dashboardFiles changed
build-status-dashboard.tsx— Live build count dashboardimage-versions.tsx— Added dashboard, search input, status filter dropdownunity-versions.tsx— Accepts and applies search/status filters with result countbuilds/builds.tsx— Bulk action bar with checkbox select-all, batched API callsbuilds/build-row.tsx— Checkboxes, inline failure reason, timeline, log viewerTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features