feat: projects overview dashboard #644
Conversation
- Introduced a new Projects Overview link in the sidebar footer for easy navigation. - Added a button for Projects Overview in the dashboard view, enhancing accessibility to project insights. - Updated types to include project overview-related definitions, supporting the new features.
Shironex
added
Testers-Requested
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a multi-project overview feature: server GET /api/projects/overview route and handler, new types, a frontend OverviewView with cards/feed/agents, a polling data hook, navigation updates to point to /overview, and end-to-end tests. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Client as UI Client
participant Server as API Server
participant Services as Services (FeatureLoader / AutoMode / Settings / Notifications)
User->>Client: Navigate to /overview
Client->>Client: Render OverviewView\ncall useMultiProjectStatus()
Client->>Server: GET /api/projects/overview (auth)
Server->>Services: Load global settings (enumerate projects)
Server->>Services: Concurrent per-project requests:\nfeatures & counts, auto-mode, notifications, last activity
Services-->>Server: Return per-project data
Server->>Server: Compute health per project\nAggregate metrics & recent activity
Server-->>Client: 200 MultiProjectOverview
Client->>Client: Update state\nRender ProjectStatusCard[], RecentActivityFeed, RunningAgentsPanel
Client-->>User: Display aggregated dashboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested labels
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
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 |
Summary of ChangesHello @Shironex, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant new feature: a "Projects Overview" dashboard. This dashboard provides users with a high-level, real-time summary of all their managed projects, consolidating critical information such as project health, feature progress, and agent activity into a single, easily digestible view. It enhances the user's ability to monitor and manage their entire workspace efficiently, moving beyond single-project focus to a comprehensive multi-project perspective. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new projects overview dashboard, a significant feature that provides a unified view for monitoring multiple projects. The implementation is well-structured, with a new backend API endpoint, corresponding frontend components, a data-fetching hook, and new type definitions. The backend efficiently processes projects in parallel, and the frontend is well-componentized. I've identified a few areas for improvement related to error logging, data modeling for a future feature, and code maintainability. Overall, this is a solid and valuable addition to the application.
| const initResult = await initializeProject( | ||
| // We need to find the project path - use projectId as workaround | ||
| // In real implementation, this would look up the path from projects list | ||
| activity.projectId | ||
| ); | ||
|
|
||
| // Navigate to the project | ||
| const projectPath = activity.projectId; | ||
| const projectName = activity.projectName; | ||
|
|
||
| upsertAndSetCurrentProject(projectPath, projectName); | ||
|
|
||
| if (activity.featureId) { | ||
| // Navigate to the specific feature | ||
| navigate({ to: '/board', search: { featureId: activity.featureId } }); | ||
| } else { | ||
| navigate({ to: '/board' }); | ||
| } |
There was a problem hiding this comment.
This block has two issues:
- It incorrectly uses
activity.projectIdas the project path, which will cause an error wheninitializeProjectis called. You should useactivity.projectPath(which I've suggested adding to theRecentActivitytype). - The result of
initializeProjectis not checked for success. If it fails, the code will proceed and likely throw another error. You should checkinitResult.successand handle the error gracefully.
The suggested code fixes both issues.
| const initResult = await initializeProject( | |
| // We need to find the project path - use projectId as workaround | |
| // In real implementation, this would look up the path from projects list | |
| activity.projectId | |
| ); | |
| // Navigate to the project | |
| const projectPath = activity.projectId; | |
| const projectName = activity.projectName; | |
| upsertAndSetCurrentProject(projectPath, projectName); | |
| if (activity.featureId) { | |
| // Navigate to the specific feature | |
| navigate({ to: '/board', search: { featureId: activity.featureId } }); | |
| } else { | |
| navigate({ to: '/board' }); | |
| } | |
| const initResult = await initializeProject(activity.projectPath); | |
| if (!initResult.success) { | |
| toast.error('Failed to open project', { | |
| description: initResult.error || 'Unknown error', | |
| }); | |
| return; | |
| } | |
| // Navigate to the project | |
| upsertAndSetCurrentProject(activity.projectPath, activity.projectName); | |
| if (activity.featureId) { | |
| // Navigate to the specific feature | |
| navigate({ to: '/board', search: { featureId: activity.featureId } }); | |
| } else { | |
| navigate({ to: '/board' }); | |
| } |
| /** Unique identifier for this activity entry */ | ||
| id: string; | ||
| /** Project this activity belongs to */ | ||
| projectId: string; |
There was a problem hiding this comment.
The RecentActivity interface is missing the projectPath. While you have projectId, the frontend will need the projectPath to navigate to the project. Relying on looking up the path from the project list on the frontend is less efficient and makes the RecentActivityFeed component less self-contained. It's better to denormalize this data and include projectPath directly in the RecentActivity type.
| projectId: string; | |
| projectId: string; | |
| /** Absolute filesystem path to project */ | |
| projectPath: string; |
| for (const feature of features) { | ||
| // Check startedAt timestamp (the main timestamp available on Feature) | ||
| if (feature.startedAt) { | ||
| const timestamp = new Date(feature.startedAt).getTime(); | ||
| if (!isNaN(timestamp) && timestamp > latestTimestamp) { | ||
| latestTimestamp = timestamp; | ||
| } | ||
| } | ||
|
|
||
| // Also check planSpec timestamps if available | ||
| if (feature.planSpec?.generatedAt) { | ||
| const timestamp = new Date(feature.planSpec.generatedAt).getTime(); | ||
| if (!isNaN(timestamp) && timestamp > latestTimestamp) { | ||
| latestTimestamp = timestamp; | ||
| } | ||
| } | ||
| if (feature.planSpec?.approvedAt) { | ||
| const timestamp = new Date(feature.planSpec.approvedAt).getTime(); | ||
| if (!isNaN(timestamp) && timestamp > latestTimestamp) { | ||
| latestTimestamp = timestamp; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for finding the latest timestamp is repeated for startedAt, planSpec.generatedAt, and planSpec.approvedAt. This repetition can be reduced to make the code more concise and easier to maintain. Consider iterating through a list of potential timestamp properties to find the maximum value.
for (const feature of features) {
const potentialTimestamps = [
feature.startedAt,
feature.planSpec?.generatedAt,
feature.planSpec?.approvedAt,
];
for (const ts of potentialTimestamps) {
if (ts) {
const timestamp = new Date(ts).getTime();
if (!isNaN(timestamp) && timestamp > latestTimestamp) {
latestTimestamp = timestamp;
}
}
}
}| unreadNotificationCount, | ||
| }; | ||
| } catch (error) { | ||
| // Return a minimal status for projects that fail to load |
There was a problem hiding this comment.
In this catch block, you're gracefully handling errors for individual project loading by returning a minimal status. However, the error itself is not logged. For better observability and easier debugging of server-side issues, it would be beneficial to log the error that occurred. You can use the logError utility that's already available in this file's scope.
logError(error, `Failed to load status for project ${projectRef.path}`);
// Return a minimal status for projects that fail to load![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
60-87: Missing blockmap files in artifact upload steps.The release upload step (lines 117-125) expects
*.blockmapfiles from all platforms, but the artifact upload steps don't include them. Blockmap files won't be available during the upload job, causing the release to be incomplete for delta/auto-updates.🔧 Proposed fix to include blockmap files in artifact uploads
- name: Upload macOS artifacts if: matrix.os == 'macos-latest' uses: actions/upload-artifact@v4 with: name: macos-builds path: | apps/ui/release/*.dmg apps/ui/release/*.zip + apps/ui/release/*.blockmap retention-days: 30 - name: Upload Windows artifacts if: matrix.os == 'windows-latest' uses: actions/upload-artifact@v4 with: name: windows-builds - path: apps/ui/release/*.exe + path: | + apps/ui/release/*.exe + apps/ui/release/*.blockmap retention-days: 30 - name: Upload Linux artifacts if: matrix.os == 'ubuntu-latest' uses: actions/upload-artifact@v4 with: name: linux-builds path: | apps/ui/release/*.AppImage apps/ui/release/*.deb apps/ui/release/*.rpm + apps/ui/release/*.blockmap retention-days: 30
🤖 Fix all issues with AI agents
In `@apps/ui/src/components/views/overview/project-status-card.tsx`:
- Around line 90-102: The container returned in the ProjectStatusCard component
is a non-interactive <div> that uses handleClick and has
data-testid={`project-status-card-${project.projectId}`}, so make it
keyboard-accessible by converting the div into a semantic <button> (or <a> if
navigation) or, if keeping the div, add role="button", tabIndex={0}, and an
onKeyDown handler that invokes handleClick on Enter/Space; ensure ARIA and
visual styles remain (preserve cn className and data-testid) and update any
tests or styles expecting a div accordingly.
In `@apps/ui/src/components/views/overview/recent-activity-feed.tsx`:
- Around line 167-172: The activity rows currently rendered as a clickable <div>
(keyed by activity.id) are not keyboard-accessible; update the element that
renders each row in recent-activity-feed.tsx to be keyboard operable by either
replacing the <div> with an interactive element (<button> or <a>) or by adding
role="button", tabIndex={0}, and key handlers that call
handleActivityClick(activity) on Enter/Space keypress; ensure the data-testid
and existing onClick behavior remain unchanged and that any visual styles still
apply to the new/updated element.
- Around line 120-140: handleActivityClick is passing activity.projectId into
initializeProject (which expects a filesystem path) and ignores the init result;
update the handler to resolve the actual project path (either by reading it from
your projects store/state or by adding projectPath to the RecentActivity shape)
and pass that path into initializeProject, await and check the initResult for
success, and only call upsertAndSetCurrentProject and navigate when
initialization succeeds; on failure, abort navigation and surface/log an error
so you don't create a broken project context (refer to initializeProject,
handleActivityClick, upsertAndSetCurrentProject, navigate, and RecentActivity to
locate the changes).
In `@apps/ui/src/components/views/overview/running-agents-panel.tsx`:
- Around line 81-86: The agent card div in RunningAgentsPanel is not
keyboard-accessible; make it activate via keyboard by adding focusability and
ARIA semantics and wiring keyboard events: give the container role="button" and
tabIndex={0} (keep data-testid={`running-agent-${agent.projectId}`}), add an
onKeyDown handler (e.g., handleAgentKeyDown or inline) that listens for Enter
and Space and calls handleAgentClick(agent), and ensure an appropriate
aria-label or aria-describedby is present for screen readers; also preserve the
existing onClick behavior and styling.
In `@apps/ui/src/hooks/use-multi-project-status.ts`:
- Around line 64-68: The error handling uses data.error?.message but the server
returns error as a string; update the throw to use the string directly. In the
fetch flow (after awaiting response.json()) replace the thrown Error creation to
use data.error (or fallback text) instead of data.error?.message so the code
reads something like throw new Error(data.error || 'Failed to fetch project
overview'); target the block that reads response.json() and the throw new
Error(...) statement in use-multi-project-status (the variable data and the
error throw are the unique symbols to edit).
In `@tests/e2e/multi-project-dashboard.spec.ts`:
- Line 1: The file starts with a JSDoc comment token (/**) and fails Prettier
formatting; run the project's Prettier formatter (e.g., prettier --write) on
this test file to fix code style, then stage/commit the formatted changes so the
pipeline passes — ensure you use the repo's Prettier config so the test spec
(the multi-project dashboard e2e spec) is reformatted consistently.
🧹 Nitpick comments (8)
apps/ui/tests/projects/overview-dashboard.spec.ts (1)
56-61: Prefer a stable selector for the back button.
The currentbutton+svgheuristic can break if the header adds more icon buttons. Consider using a dedicateddata-testid(or accessible name) for the back button to reduce flakiness.apps/ui/src/components/views/overview/running-agents-panel.tsx (1)
112-117: Consider usingGitBranchicon for branch display.The
Foldericon is used to display the active branch, butGitBranchfrom lucide-react would be more semantically appropriate for git branch information.Proposed fix
-import { Bot, Activity, Folder, ArrowRight } from 'lucide-react'; +import { Bot, Activity, GitBranch, ArrowRight } from 'lucide-react';{agent.activeBranch && ( <span className="flex items-center gap-1"> - <Folder className="w-3 h-3" /> + <GitBranch className="w-3 h-3" /> {agent.activeBranch} </span> )}apps/server/src/routes/projects/routes/overview.ts (1)
190-208: Consider logging per-project load failures for debugging.The catch block at line 190 catches errors but doesn't log them. While returning a fallback status is correct, logging would help with debugging failed project loads.
Proposed fix
} catch (error) { + logError(error, `Failed to load project: ${projectRef.name}`); // Return a minimal status for projects that fail to load return { projectId: projectRef.id,apps/server/src/routes/projects/index.ts (1)
5-10: Inconsistent import style forFeatureLoader.
FeatureLoaderis imported as a value (line 6) while the other services usetypeimports. SinceFeatureLoaderis only used as a type annotation in the function signature, consider using a type import for consistency.Proposed fix
import { Router } from 'express'; -import { FeatureLoader } from '../../services/feature-loader.js'; +import type { FeatureLoader } from '../../services/feature-loader.js'; import type { AutoModeService } from '../../services/auto-mode-service.js';tests/e2e/multi-project-dashboard.spec.ts (2)
67-67: AvoidwaitForTimeoutfor more reliable tests.
page.waitForTimeout(2000)introduces arbitrary delays and can cause flaky tests. Consider using Playwright's auto-waiting capabilities or explicit conditions.Proposed fix
- // Wait for loading to complete (either shows content or error) - await page.waitForTimeout(2000); + // Wait for loading to complete by checking for expected elements + await expect( + page.locator('[data-testid^="project-status-card-"], :text("No projects yet"), :text("Failed to load overview")') + .first() + ).toBeVisible({ timeout: 5000 });
101-119: Reduce reliance onwaitForTimeoutin refresh test.Lines 105 and 115 use
waitForTimeoutwhich can cause flakiness. Consider waiting for specific network responses or UI state changes instead.Proposed improvement
test('should refresh data when refresh button is clicked', async ({ page }) => { await page.goto('/overview'); - // Wait for initial load - await page.waitForTimeout(1000); + // Wait for initial load by checking the view is visible + await expect(page.getByTestId('overview-view')).toBeVisible(); // Find the refresh button const refreshButton = page.locator('button').filter({ hasText: 'Refresh' }); if (await refreshButton.isVisible({ timeout: 2000 }).catch(() => false)) { + // Set up response listener before clicking + const responsePromise = page.waitForResponse( + (resp) => resp.url().includes('/api/projects/overview') && resp.status() === 200 + ); + await refreshButton.click(); - // The button should show loading state (spinner icon) - // Wait a moment for the refresh to complete - await page.waitForTimeout(1000); + // Wait for the API response + await responsePromise; // Page should still be on overview await expect(page).toHaveURL(/\/overview/); } });apps/ui/src/components/views/overview-view.tsx (1)
274-279: Consider specifying locale options for consistent time formatting.The
toLocaleTimeString()call without options may produce inconsistent results across different browser locales.♻️ Optional: Add time format options for consistency
{/* Footer timestamp */} <div className="text-center text-xs text-muted-foreground pt-4"> - Last updated: {new Date(overview.generatedAt).toLocaleTimeString()} + Last updated: {new Date(overview.generatedAt).toLocaleTimeString(undefined, { + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + })} </div>apps/ui/src/components/views/dashboard-view.tsx (1)
574-584: Consider addingdata-testidto the mobile Overview button for test consistency.The desktop button has
data-testid="projects-overview-button"but the mobile version lacks one, which could make E2E testing on mobile viewports more difficult.♻️ Add data-testid to mobile button
<Button variant="outline" size="icon" onClick={() => navigate({ to: '/overview' })} title="Projects Overview" + data-testid="projects-overview-button-mobile" > <LayoutDashboard className="w-4 h-4" /> </Button>
Add new unified-sidebar component for layout improvements. - Export UnifiedSidebar from layout components - Update root route to use new sidebar structure
Merge the unified-sidebar implementation into the standard sidebar folder structure. The unified sidebar becomes the canonical sidebar with improved features including collapsible sections, scroll indicators, and enhanced mobile support. - Delete old sidebar.tsx - Move unified-sidebar components to sidebar/components - Rename UnifiedSidebar to Sidebar - Update all imports in __root.tsx - Remove redundant unified-sidebar folder
…ebar - Add try/catch for getElectronAPI() in sidebar-footer with window.open fallback - Use formatShortcut() for OS-aware hotkey display in sidebar-header - Remove unnecessary optional chaining on project.icon - Remove redundant ternary in sidebar-navigation className - Update E2E tests to use new project-dropdown-trigger data-testid Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…9-gg62' into feature/unified-sidebar # Conflicts: # apps/server/src/index.ts # apps/ui/src/components/layout/sidebar/components/sidebar-footer.tsx # libs/types/src/index.ts
Replace separate Dashboard and Projects Overview nav items with a single Dashboard item that links to /overview. Update all logo click handlers to navigate to /overview for consistency. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove back button and rename title from "Projects Overview" to "Dashboard" since the overview is now the main dashboard accessed via sidebar. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add project creation and folder opening functionality directly to the Dashboard view header for quick access. Includes NewProjectModal with template support and WorkspacePickerModal integration. Also renamed title to "Automaker Dashboard". Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Keyboard accessibility: - Add role="button", tabIndex, onKeyDown, and aria-label to clickable divs in project-status-card, recent-activity-feed, and running-agents-panel Bug fixes: - Fix handleActivityClick to use projectPath instead of projectId for initializeProject and check result before navigating - Fix error handling in use-multi-project-status to use data.error string directly instead of data.error?.message Improvements: - Use GitBranch icon instead of Folder for branch display in running-agents-panel - Add error logging for failed project loads in overview.ts - Use type import for FeatureLoader in projects/index.ts - Add data-testid to mobile Overview button in dashboard-view - Add locale options for consistent time formatting in overview-view Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/ui/src/components/views/overview-view.tsx`:
- Around line 124-167: The create flows (e.g., handleCreateBlankProject)
currently proceed to addProject, setCurrentProject and navigate even if
initializeProject fails; change the flow to await and verify initializeProject
succeeded before mutating state or navigating: after calling
initializeProject(projectPath) check its return value (or wrap initializeProject
in a try/catch and rethrow on failure) and only create the project object, call
addProject, setCurrentProject and navigate when initialization succeeds; if
initialization fails, throw or return early so the catch block handles
logging/toast and setIsCreating(false). Apply the same guard to the other
creation handlers mentioned (the blocks around lines 170-215 and 217-256).
🧹 Nitpick comments (1)
apps/ui/src/components/views/overview-view.tsx (1)
18-25: Prefer components barrel imports for intra-app UI components.Consider importing UI/dialog components (and overview subcomponents if exported) from the components index barrel to keep internal imports stable. Based on learnings, ...
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/server/src/routes/projects/routes/overview.ts`:
- Around line 72-99: computeHealthStatus currently never returns 'waiting'
because pending work with no running/auto mode falls through to 'idle'; update
the function (computeHealthStatus) to return 'waiting' when
featureCounts.pending > 0 and featureCounts.running === 0 and !isAutoModeRunning
(or when pending exists but not actively running), before the final 'idle'
branch. Keep the existing early 'error' and 'active' checks
(featureCounts.failed and featureCounts.running || (isAutoModeRunning &&
featureCounts.pending)), and retain the 'completed' logic using totalFeatures
and pending===0 & running===0; insert the 'waiting' return just after those
checks so pending-only projects map to 'waiting' instead of 'idle'.
In `@apps/ui/src/components/views/overview/running-agents-panel.tsx`:
- Around line 68-75: The handler uses an undefined type RunningAgentInfo; update
the type reference in handleAgentKeyDown to use the locally declared
RunningAgent interface instead (replace RunningAgentInfo with RunningAgent
wherever it appears in the handleAgentKeyDown signature and related callbacks,
e.g., the useCallback parameter and any usages that pass agent to
handleAgentClick) so the component compiles against the correct local type.
♻️ Duplicate comments (1)
apps/ui/src/components/views/overview-view.tsx (1)
124-166: Guard creation flows on initializeProject success.
Right nowinitializeProject(...)is awaited but its result is ignored, so the app can add/set projects and navigate even when init fails. Please short‑circuit on failure in all three create flows.🐛 Suggested fix pattern
- await initializeProject(projectPath); + const initResult = await initializeProject(projectPath); + if (!initResult.success) { + toast.error('Failed to initialize project', { + description: initResult.error || 'Unknown error occurred', + }); + return; + }- await initializeProject(cloneResult.projectPath); + const initResult = await initializeProject(cloneResult.projectPath); + if (!initResult.success) { + toast.error('Failed to initialize project', { + description: initResult.error || 'Unknown error occurred', + }); + return; + }- await initializeProject(cloneResult.projectPath); + const initResult = await initializeProject(cloneResult.projectPath); + if (!initResult.success) { + toast.error('Failed to initialize project', { + description: initResult.error || 'Unknown error occurred', + }); + return; + }Also applies to: 170-215, 217-256
🧹 Nitpick comments (1)
apps/server/src/routes/projects/routes/overview.ts (1)
139-297: Consider moving aggregation logic into a service.
The handler owns sizable business logic; extracting intoservices/will improve testability and keep routes thin. As per coding guidelines, ...
Check the return value of initializeProject in all three create handlers (handleCreateBlankProject, handleCreateFromTemplate, handleCreateFromCustomUrl) and return early with an error toast if initialization fails, preventing addProject/setCurrentProject/navigate from being called on failure. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add 'waiting' status to computeHealthStatus for projects with pending features but no active execution (fixes status never being emitted) - Fix undefined RunningAgentInfo type to use local RunningAgent interface Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/ui/src/components/views/overview-view.tsx`:
- Around line 53-81: The handleOpenProject useCallback currently has an empty
dependency array and closes over initializeAndOpenProject, which can change;
update the dependency array of handleOpenProject to include
initializeAndOpenProject (i.e. useCallback(..., [initializeAndOpenProject])) so
the callback always uses the latest initializeAndOpenProject reference; locate
the handleOpenProject definition in overview-view.tsx and replace the [] with
[initializeAndOpenProject], then run lint/TS checks to ensure no other related
missing deps remain.
🧹 Nitpick comments (2)
apps/ui/src/components/views/overview-view.tsx (2)
18-25: Prefer components barrel exports for intra-app UI imports.Line 18–25: Consider importing UI/dialog/view components from the app’s components index barrel (if exported) to avoid deep paths and relative imports. This aligns with the intra-app import convention and improves refactorability. Based on learnings, ...
101-103: Consider centralizing project open state updates.Lines 101–103, 159–160, 210–211, 259–260: These repeated
addProject+setCurrentProjectsequences could be consolidated via the existingupsertAndSetCurrentProjecthelper (used elsewhere in overview components) to keep state updates consistent and reduce duplication.Also applies to: 159-160, 210-211, 259-260
- Fix handleOpenProject dependency array to include initializeAndOpenProject - Use upsertAndSetCurrentProject instead of separate addProject + setCurrentProject - Reorder callback declarations to avoid use-before-definition - Update E2E tests to match renamed Dashboard UI (was "Projects Overview") - Add success: true to mock API responses for proper test functioning - Fix strict mode violation in project status card test Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Change 'waiting_approval' status to 'in_progress' and add handling for 'waiting_approval' as pending. - Introduce counting of live running features per project in the overview handler by fetching all running agents. - Ensure accurate representation of project statuses in the overview. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/server/src/routes/projects/routes/overview.ts`:
- Around line 175-181: The code overwrites featureCounts.running with
liveRunningCount which can desync totals and incorrectly mark projects
completed; instead either (A) keep the live count separate by assigning
liveRunningCount to a new key (e.g., featureCounts.liveRunning) and leave
featureCounts.running as the state-derived running count, or (B) if you must
replace running, recompute featureCounts.total and any derived completion logic
immediately after using all state counts so totalFeatures remains consistent;
locate usage of allRunningAgents, projectRef.path, and featureCounts in this
block and apply one of these fixes so totals and completion status stay correct.
In `@apps/ui/tests/projects/overview-dashboard.spec.ts`:
- Around line 371-376: The mocked error response in the test's route.fulfill
call doesn't include the API's `success: false` field; update the JSON payload
passed to route.fulfill (the body object in the failing response) to include
"success": false so the mock matches the real API contract and client error
handling is exercised correctly.
🧹 Nitpick comments (2)
apps/server/src/routes/projects/routes/overview.ts (1)
28-146: Consider moving overview computations into a service layer.
computeFeatureCounts,computeHealthStatus, andgetLastActivityAtare business logic living in the route module. Extracting these intoservices/(and keeping the handler as orchestration only) will keep routing thin and make reuse/testing easier. As per coding guidelines, consider moving these helpers into a service module.apps/ui/src/components/views/overview-view.tsx (1)
23-25: Prefer barrel exports for overview subcomponents.If the overview folder (or components index) exposes a barrel, consider importing these subcomponents from it to keep paths stable and refactors easier.
♻️ Example (if an index barrel exists)
-import { ProjectStatusCard } from './overview/project-status-card'; -import { RecentActivityFeed } from './overview/recent-activity-feed'; -import { RunningAgentsPanel } from './overview/running-agents-panel'; +import { ProjectStatusCard, RecentActivityFeed, RunningAgentsPanel } from './overview';Based on learnings, prefer barrel exports for intra-app UI components.
| // Count live running features for this project (across all branches) | ||
| // This ensures we only count features that are actually running in memory | ||
| const liveRunningCount = allRunningAgents.filter( | ||
| (agent) => agent.projectPath === projectRef.path | ||
| ).length; | ||
| featureCounts.running = liveRunningCount; | ||
|
|
There was a problem hiding this comment.
Keep feature counts consistent after overriding running.
Overwriting featureCounts.running with liveRunningCount can desync counts from totalFeatures and can mark a project completed even if there are still features in non‑completed states. Consider reconciling the counts when you override running, or keep live running count separate.
🛠️ Possible fix to keep totals consistent
- const liveRunningCount = allRunningAgents.filter(
+ const liveRunningCount = allRunningAgents.filter(
(agent) => agent.projectPath === projectRef.path
).length;
- featureCounts.running = liveRunningCount;
+ if (liveRunningCount !== featureCounts.running) {
+ const delta = liveRunningCount - featureCounts.running;
+ featureCounts.running = liveRunningCount;
+ if (delta > 0) {
+ featureCounts.pending = Math.max(0, featureCounts.pending - delta);
+ } else if (delta < 0) {
+ featureCounts.pending += -delta;
+ }
+ }🤖 Prompt for AI Agents
In `@apps/server/src/routes/projects/routes/overview.ts` around lines 175 - 181,
The code overwrites featureCounts.running with liveRunningCount which can desync
totals and incorrectly mark projects completed; instead either (A) keep the live
count separate by assigning liveRunningCount to a new key (e.g.,
featureCounts.liveRunning) and leave featureCounts.running as the state-derived
running count, or (B) if you must replace running, recompute featureCounts.total
and any derived completion logic immediately after using all state counts so
totalFeatures remains consistent; locate usage of allRunningAgents,
projectRef.path, and featureCounts in this block and apply one of these fixes so
totals and completion status stay correct.
| await route.fulfill({ | ||
| status: 500, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ | ||
| error: 'Internal server error', | ||
| }), |
There was a problem hiding this comment.
Align error mock with API contract (success: false).
The server’s error response includes success: false; adding it here keeps the mock aligned with real responses and avoids divergent client handling.
✅ Suggested adjustment
- body: JSON.stringify({
- error: 'Internal server error',
- }),
+ body: JSON.stringify({
+ success: false,
+ error: 'Internal server error',
+ }),📝 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.
| await route.fulfill({ | |
| status: 500, | |
| contentType: 'application/json', | |
| body: JSON.stringify({ | |
| error: 'Internal server error', | |
| }), | |
| await route.fulfill({ | |
| status: 500, | |
| contentType: 'application/json', | |
| body: JSON.stringify({ | |
| success: false, | |
| error: 'Internal server error', | |
| }), |
🤖 Prompt for AI Agents
In `@apps/ui/tests/projects/overview-dashboard.spec.ts` around lines 371 - 376,
The mocked error response in the test's route.fulfill call doesn't include the
API's `success: false` field; update the JSON payload passed to route.fulfill
(the body object in the failing response) to include "success": false so the
mock matches the real API contract and client error handling is exercised
correctly.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.