Update dashboard to be clean with enhanced viewing #397
Conversation
feat: initialize project structure and add core components - Created initial package.json and package-lock.json files for dependency management. - Added dashboard configuration with components.json for UI structure. - Introduced new UI components including Card, Button, Input, Tabs, Alert, Badge, and others for enhanced user interface. - Implemented global styles in globals.css, including a new color palette and responsive design. - Developed API endpoint for fetching node run details, enhancing data retrieval capabilities. - Updated various components to utilize new UI elements and improve overall functionality. This commit sets the foundation for the Exosphere dashboard, enabling further development and feature integration.
feat: enhance GraphTemplateDetailModal with custom node visualization and integrate Dark Grotesque font - Added a custom node component for React Flow to improve the visual representation of workflow nodes. - Implemented memoization for node layout calculations to optimize rendering performance. - Updated global styles to include a new custom font, "Dark Grotesque", enhancing the overall UI aesthetics. - Adjusted card components for better layout and spacing in the NodeDetailModal. This commit significantly improves the user experience by providing a more interactive and visually appealing graph structure.
|
Warning Rate limit exceeded@NiveditJain has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 25 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (12)
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. 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds ShadCN UI configuration and components, updates Tailwind theme, and overhauls dashboard pages to card/tab layouts. Introduces Next.js proxy routes for namespaces and node run details. Extends client API and types. On backend, adds list-namespaces and node-run-details endpoints, controllers, models, routers, and unit tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Dashboard UI
participant NX as Next.js API (/api/node-run-details)
participant SM as State Manager API
participant CTR as Controller (get_node_run_details)
participant DB as State Store
UI->>NX: GET /api/node-run-details?namespace&graphName&runId&nodeId
NX->>SM: GET /v0/namespace/{ns}/graph/{g}/run/{r}/node/{n}\nHeaders: X-API-Key
SM->>CTR: Delegate with params + request_id
CTR->>DB: Query State by ids and scope
DB-->>CTR: Node state (inputs, outputs, status, parents, timestamps)
CTR-->>SM: NodeRunDetailsResponse
SM-->>NX: 200 JSON
NX-->>UI: 200 JSON
note over UI: Render node details modal
sequenceDiagram
autonumber
participant UI as Dashboard UI
participant NX as Next.js API (/api/namespaces)
participant SM as State Manager API
participant CTR as Controller (list_namespaces)
participant DB as RegisteredNode
UI->>NX: GET /api/namespaces
NX->>SM: GET /v0/namespaces\nHeaders: X-API-Key
SM->>CTR: list_namespaces(request_id)
CTR->>DB: Aggregate distinct namespaces
DB-->>CTR: ["default", "prod", ...]
CTR-->>SM: ListNamespacesResponse
SM-->>NX: 200 JSON
NX-->>UI: 200 JSON
note over UI: Populate namespace selector
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks❌ Failed checks (1 warning, 2 inconclusive)
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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 34
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
dashboard/package.json (1)
1-10: Consider adding Node engine and type-check script.Prevents CI drift and improves dev UX.
"private": true, "scripts": { "dev": "next dev --turbopack", "build": "next build", "start": "next start", - "lint": "next lint" + "lint": "next lint", + "typecheck": "tsc -p tsconfig.json --noEmit" }, + "engines": { + "node": ">=18.18" + },state-manager/tests/unit/test_routes.py (1)
329-335: Fragile mock for “no request id”. Replace MagicMock+delattr with a plain object.delattr on MagicMock attributes can be flaky; also getattr on MagicMock won’t fall back. Prefer a SimpleNamespace for request.state.
- def mock_request_no_id(self): - """Mock request object without request_id""" - request = MagicMock() - delattr(request.state, 'x_exosphere_request_id') - return request + def mock_request_no_id(self): + """Mock request object without request_id""" + from types import SimpleNamespace + request = MagicMock() + request.state = SimpleNamespace() + return requestdashboard/src/app/page.tsx (1)
81-87: Avoid mutating API responses; create a derived object.Defensive copy is safer for memoization and future referential checks.
Apply this diff:
-const graphTemplate = await clientApiService.getGraphTemplate(namespace, graphName); -graphTemplate.name = graphName; -graphTemplate.namespace = namespace; -setSelectedGraphTemplate(graphTemplate); +const tpl = await clientApiService.getGraphTemplate(namespace, graphName); +setSelectedGraphTemplate({ ...tpl, name: graphName, namespace });dashboard/src/components/GraphVisualization.tsx (1)
271-289: Use theme tokens and unique edge ids.Hardcoded edge color and non-unique ids risk visual inconsistency and collisions. Prefer CSS variables and include index in id.
Apply this diff:
-const reactFlowEdges: Edge[] = graphData.edges.map((edge, index) => ({ - id: `edge-${edge.source}-${edge.target}`, +const reactFlowEdges: Edge[] = graphData.edges.map((edge, index) => ({ + id: `edge-${edge.source}-${edge.target}-${index}`, source: edge.source, target: edge.target, type: 'default', animated: false, markerEnd: { type: MarkerType.ArrowClosed, - width: 10, - height: 10, - color: '#87ceeb', + width: 10, + height: 10, + color: 'hsl(var(--primary))', }, style: { - stroke: '#87ceeb', + stroke: 'hsl(var(--primary))', strokeWidth: 2, strokeDasharray: 'none', }, }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
dashboard/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
dashboard/components.json(1 hunks)dashboard/package.json(2 hunks)dashboard/src/app/api/node-run-details/route.ts(1 hunks)dashboard/src/app/globals.css(1 hunks)dashboard/src/app/page.tsx(3 hunks)dashboard/src/components/GraphTemplateDetailModal.tsx(2 hunks)dashboard/src/components/GraphVisualization.tsx(7 hunks)dashboard/src/components/NamespaceOverview.tsx(2 hunks)dashboard/src/components/NodeDetailModal.tsx(3 hunks)dashboard/src/components/RunsTable.tsx(5 hunks)dashboard/src/components/ui/alert.tsx(1 hunks)dashboard/src/components/ui/badge.tsx(1 hunks)dashboard/src/components/ui/button.tsx(1 hunks)dashboard/src/components/ui/card.tsx(1 hunks)dashboard/src/components/ui/input.tsx(1 hunks)dashboard/src/components/ui/tabs.tsx(1 hunks)dashboard/src/lib/utils.ts(1 hunks)dashboard/src/services/clientApi.ts(1 hunks)dashboard/src/types/state-manager.ts(4 hunks)package.json(1 hunks)state-manager/app/controller/get_node_run_details.py(1 hunks)state-manager/app/models/node_run_details_models.py(1 hunks)state-manager/app/routes.py(2 hunks)state-manager/tests/unit/controller/test_get_node_run_details.py(1 hunks)state-manager/tests/unit/test_routes.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (16)
state-manager/app/controller/get_node_run_details.py (3)
state-manager/app/models/db/state.py (1)
State(13-104)state-manager/app/models/node_run_details_models.py (1)
NodeRunDetailsResponse(6-19)state-manager/app/singletons/logs_manager.py (2)
LogsManager(9-66)get_logger(65-66)
dashboard/src/components/ui/tabs.tsx (1)
dashboard/src/lib/utils.ts (1)
cn(4-6)
dashboard/src/types/state-manager.ts (1)
state-manager/app/models/node_run_details_models.py (1)
NodeRunDetailsResponse(6-19)
dashboard/src/components/ui/input.tsx (1)
dashboard/src/lib/utils.ts (1)
cn(4-6)
dashboard/src/components/ui/alert.tsx (1)
dashboard/src/lib/utils.ts (1)
cn(4-6)
state-manager/app/models/node_run_details_models.py (2)
state-manager/app/models/state_status_enum.py (1)
StateStatusEnum(4-20)dashboard/src/types/state-manager.ts (1)
NodeRunDetailsResponse(202-215)
dashboard/src/components/ui/card.tsx (1)
dashboard/src/lib/utils.ts (1)
cn(4-6)
dashboard/src/components/RunsTable.tsx (2)
state-manager/app/models/run_models.py (1)
RunStatusEnum(9-12)dashboard/src/components/GraphVisualization.tsx (1)
GraphVisualization(120-648)
state-manager/tests/unit/controller/test_get_node_run_details.py (3)
state-manager/app/controller/get_node_run_details.py (1)
get_node_run_details(12-88)state-manager/app/models/state_status_enum.py (1)
StateStatusEnum(4-20)state-manager/app/models/node_run_details_models.py (1)
NodeRunDetailsResponse(6-19)
dashboard/src/components/ui/badge.tsx (1)
dashboard/src/lib/utils.ts (1)
cn(4-6)
state-manager/tests/unit/test_routes.py (1)
state-manager/app/routes.py (2)
get_graph_structure_route(326-335)get_node_run_details_route(345-354)
dashboard/src/components/ui/button.tsx (1)
dashboard/src/lib/utils.ts (1)
cn(4-6)
dashboard/src/components/GraphTemplateDetailModal.tsx (1)
dashboard/src/types/state-manager.ts (2)
UpsertGraphTemplateResponse(44-55)NodeTemplate(31-37)
state-manager/app/routes.py (3)
state-manager/app/models/node_run_details_models.py (1)
NodeRunDetailsResponse(6-19)state-manager/app/controller/get_node_run_details.py (1)
get_node_run_details(12-88)state-manager/app/utils/check_secret.py (1)
check_api_key(11-16)
dashboard/src/app/page.tsx (3)
dashboard/src/components/NamespaceOverview.tsx (1)
NamespaceOverview(33-229)dashboard/src/components/GraphTemplateBuilder.tsx (1)
GraphTemplateBuilder(13-290)dashboard/src/components/RunsTable.tsx (1)
RunsTable(42-413)
dashboard/src/components/GraphVisualization.tsx (2)
dashboard/src/types/state-manager.ts (1)
NodeRunDetailsResponse(202-215)state-manager/app/models/node_run_details_models.py (1)
NodeRunDetailsResponse(6-19)
🪛 GitHub Actions: Ruff check on changed files only
state-manager/app/controller/get_node_run_details.py
[error] 87-87: Ruff lint error (F541): f-string without placeholders on line 87. Remove extraneous 'f' prefix. This is fixable with 'ruff check --fix'.
🪛 Biome (2.1.2)
dashboard/src/components/NodeDetailModal.tsx
[error] 157-157: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
dashboard/src/components/RunsTable.tsx
[error] 254-254: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
dashboard/src/components/GraphTemplateDetailModal.tsx
[error] 311-311: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 318-318: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 325-325: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 332-332: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 340-340: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 371-371: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 379-379: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 387-387: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 395-395: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 403-403: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 529-529: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 533-533: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 537-537: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
dashboard/src/components/NamespaceOverview.tsx
[error] 150-154: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 150-154: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 193-197: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 193-197: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
🔇 Additional comments (23)
dashboard/src/lib/utils.ts (1)
1-6: cn helper looks good and matches ShadCN’s pattern.dashboard/src/components/ui/alert.tsx (2)
22-35: Double-check role usage.Use role="alert" only for assertive, time-sensitive errors; for non-critical info, role="status" is preferable.
6-8: *Undefined CSS var in grid template — replace calc(var(--spacing)4) with a concrete size (1rem).--spacing is not defined in dashboard/src/app/globals.css; the grid column calc(...) is invalid. Icon uses size-4 (1rem), so use 1rem.
File: dashboard/src/components/ui/alert.tsx:7
- "relative w-full rounded-lg border px-4 py-3 text-sm grid has-[>svg]:grid-cols-[calc(var(--spacing)*4)_1fr] grid-cols-[0_1fr] has-[>svg]:gap-x-3 gap-y-0.5 items-start [&>svg]:size-4 [&>svg]:translate-y-0.5 [&>svg]:text-current", + "relative w-full rounded-lg border px-4 py-3 text-sm grid has-[>svg]:grid-cols-[1rem_1fr] grid-cols-[0_1fr] has-[>svg]:gap-x-3 gap-y-0.5 items-start [&>svg]:size-4 [&>svg]:translate-y-0.5 [&>svg]:text-current",dashboard/components.json (2)
15-20: Path alias '@/*' is present in tsconfig.json — resolved.
tsconfig.json contains "paths": { "@/": ["./src/"] }, so the alias is backed for editor/build.
6-12: Set Tailwind config path in dashboard/components.jsonFile: dashboard/components.json (lines 6–12)
"tailwind": { "config": "", "css": "src/app/globals.css", "baseColor": "slate", "cssVariables": true, "prefix": "" },Set "config" to your Tailwind config file (e.g., tailwind.config.ts or tailwind.config.cjs). An empty value can cause the ShadCN generator to skip Tailwind patching — confirm the actual filename/path in the repo and update components.json accordingly.
dashboard/src/app/globals.css (2)
7-13: Verify font asset presence and licensing.DarkerGrotesque-Regular.ttf is present at dashboard/public/DarkerGrotesque-Regular.ttf and referenced in dashboard/src/app/globals.css (lines 7–13). Manually confirm the font license permits bundling/serving from your site; if not, replace with a properly licensed file or use a hosted webfont alternative.
4-4: No action — @custom-variant dark is intentional and correct for Tailwind v4Tailwind v4 defaults to prefers-color-scheme and requires overriding the dark variant in CSS (via @variant/@custom-variant) to enable selector/class-based dark mode; the directive in dashboard/src/app/globals.css enables .dark toggling and does not conflict with the built-in behavior. (v4-test.tailwindcss.com)
Likely an incorrect or invalid review comment.
dashboard/package.json (1)
37-37: Verify tw-animate-css import resolution under Tailwind v4. globals.css (dashboard/src/app/globals.css) contains @import "tw-animate-css" and dashboard/package.json lists the dependency, but the package files couldn't be confirmed here; verify locally that node_modules/tw-animate-css exposes a resolvable CSS entry (package.json "style"/"exports" or a top-level CSS file) or update the import to the package's actual CSS path. Run: ls -la dashboard/node_modules/tw-animate-css && npm view tw-animate-css files --json.dashboard/src/services/clientApi.ts (1)
24-31: Add AbortSignal and richer error context; consider no-store for freshness.Prevents leaks on unmounted components and surfaces server error bodies. Callsites not found in repo; verify callers are plumbed to pass an AbortSignal.
dashboard/src/services/clientApi.ts — getNodeRunDetails
- async getNodeRunDetails(namespace: string, graphName: string, runId: string, nodeId: string) { - const response = await fetch(`/api/node-run-details?namespace=${encodeURIComponent(namespace)}&graphName=${encodeURIComponent(graphName)}&runId=${encodeURIComponent(runId)}&nodeId=${encodeURIComponent(nodeId)}`); - if (!response.ok) { - throw new Error(`Failed to fetch node run details: ${response.statusText}`); - } - return response.json(); - } + async getNodeRunDetails( + namespace: string, + graphName: string, + runId: string, + nodeId: string, + opts?: { signal?: AbortSignal; noStore?: boolean } + ) { + const url = `/api/node-run-details?namespace=${encodeURIComponent(namespace)}&graphName=${encodeURIComponent(graphName)}&runId=${encodeURIComponent(runId)}&nodeId=${encodeURIComponent(nodeId)}`; + const response = await fetch(url, { + signal: opts?.signal, + cache: opts?.noStore ? "no-store" : undefined, + }); + if (!response.ok) { + const body = await response.text().catch(() => ""); + throw new Error(`Failed to fetch node run details (${response.status} ${response.statusText})${body ? `: ${body}` : ""}`); + } + return response.json(); + }state-manager/tests/unit/test_routes.py (4)
51-53: Route path assertion for node run details looks correct.Matches the router prefix and path shape.
297-299: Handler imports and callability checks are sound.Good coverage for new route exports.
Also applies to: 315-315
1004-1005: Good: service not called on invalid API key.Prevents side effects when auth fails.
1006-1036: Route tests OK — add missing-request-id fallback test.
Mirror the existing pattern (e.g., enqueue_state) to assert a generated UUID is used when x_exosphere_request_id is absent. Patch/mock uuid4 consistent with how it's imported in app.routes (uuid.uuid4 vs from uuid import uuid4). File: state-manager/tests/unit/test_routes.pystate-manager/app/routes.py (2)
42-44: New imports align with controller/model wiring.Consistent with existing route patterns.
338-354: Node run details route is correctly shaped and authorized.Path, tags, and request-id handling match conventions. No issues.
dashboard/src/components/ui/badge.tsx (1)
7-28: Badge variants are well-structured.Good use of cva and Slot; no issues spotted.
dashboard/src/types/state-manager.ts (3)
176-186: GraphNode now carries inputs/outputs: aligns with backend.Types look correct and unblock richer UI.
202-215: NodeRunDetailsResponse matches server model.Types align with the new endpoint response.
44-51: New fields on UpsertGraphTemplateResponse are fine; verify backend parity.Confirm server includes retry_policy and store_config to avoid runtime undefineds.
dashboard/src/components/GraphVisualization.tsx (1)
303-313: Guard against stale node-detail responses (rapid clicks)File: dashboard/src/components/GraphVisualization.tsx — onNodeClick
Use request cancellation or a request-token check so late async responses don't overwrite the current selection. Options:
- AbortController: keep current controller in a ref, abort it before calling loadNodeDetails, and pass signal to loadNodeDetails so the request is cancelled if selection changes.
- Request-id token: increment a ref id on each click, capture it for the async call, and only call setSelectedNodeDetails if the id matches the latest.
dashboard/src/components/ui/card.tsx (1)
23-23: Confirm Tailwind setup supports used variants/tokens.
@container/*,has-[…], and token classes (bg-card, text-card-foreground) require proper Tailwind config and plugins. Please verify build output has no “unknown variant/class” warnings.dashboard/src/components/GraphTemplateDetailModal.tsx (1)
456-463: Handle non-string default values safely.Avoid rendering
[object Object].[suggest_minor_refactor]
- <div className="text-sm font-mono bg-muted p-2 rounded"> - {value as string} - </div> + <div className="text-sm font-mono bg-muted p-2 rounded"> + {typeof value === "string" ? value : JSON.stringify(value)} + </div>dashboard/src/components/RunsTable.tsx (1)
316-319: Verify Badge supports the 'success' variant (or map SUCCESS → existing variant).File: dashboard/src/components/RunsTable.tsx (lines 316–319)
<Badge variant={getStatusColor(run.status)}> {run.status} </Badge> </div>Ensure the Badge component defines a 'success' variant; if it does not, update getStatusColor to return an existing variant (e.g., "default") for the 'SUCCESS' status or add 'success' to Badge's variants.
package.json
Outdated
| @@ -0,0 +1 @@ | |||
| {} | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Empty root package.json is risky; remove or make it a proper workspace root.
A top-level {} can break tooling, confuse package managers, and shadow real configs. Either delete this file or convert it into an intentional workspace root.
Option A (preferred): remove the file.
Option B: make it explicit:
-{}
+{
+ "name": "@exospherehost/root",
+ "private": true,
+ "workspaces": [
+ "dashboard"
+ ]
+}🏁 Script executed:
#!/bin/bash
# Verify if the repo already had a root package.json or workspaces expectation
fd -HI --max-depth 1 package.json
jq -r '.workspaces? // empty' dashboard/../package.json 2>/dev/null || trueLength of output: 12
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== root package.json (path) ==="
realpath package.json || true
echo
echo "=== root package.json (raw) ==="
sed -n '1,400p' package.json || true
echo
echo "=== try parse root package.json with jq ==="
if command -v jq >/dev/null 2>&1; then
jq -C . package.json || echo "(invalid JSON)"
else
echo "jq not installed"
fi
echo
echo "=== find all package.json files (depth 4) ==="
if command -v fd >/dev/null 2>&1; then
fd -HI --max-depth 4 --glob 'package.json' || true
else
find . -maxdepth 4 -type f -name 'package.json' || true
fi
echo
echo "=== .workspaces in each package.json (if JSON) ==="
for f in $(if command -v fd >/dev/null 2>&1; then fd -HI --max-depth 4 --glob 'package.json'; else find . -maxdepth 4 -type f -name 'package.json'; fi); do
echo "---- $f ----"
if command -v jq >/dev/null 2>&1; then
jq -r '.workspaces // "<no workspaces>"' "$f" 2>/dev/null || echo "(invalid json)"
else
echo "jq not available"
fi
doneLength of output: 397
Remove or replace empty root package.json (critical)
Root package.json is literally {} — this can break tooling. Either delete the file or make it an explicit workspace root. Location: package.json
Option A (preferred): remove package.json.
Option B: replace with an explicit workspace root:
-{}
+{
+ "name": "@exospherehost/root",
+ "private": true,
+ "workspaces": [
+ "dashboard"
+ ]
+}📝 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.
| {} | |
| { | |
| "name": "@exospherehost/root", | |
| "private": true, | |
| "workspaces": [ | |
| "dashboard" | |
| ] | |
| } |
🤖 Prompt for AI Agents
In package.json around lines 1 to 1, the file is an empty JSON object ("{}"),
which breaks tooling; fix by either deleting the root package.json (preferred)
or replacing it with a proper workspace root definition: create a non-empty
package.json that at minimum marks the repo as private and declares the
workspaces array (and optionally a name and version) so package managers
recognize the monorepo root; remove the empty file from the repo if you choose
deletion, or overwrite it with a package.json containing the appropriate
workspace metadata and commit the change.
| # Find the specific state | ||
| state = await State.find_one( | ||
| State.id == node_object_id, | ||
| State.run_id == run_id, | ||
| State.graph_name == graph_name, | ||
| State.namespace_name == namespace | ||
| ) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Query by id is sufficient; extra filters are redundant.
Not blocking, but filtering by id already uniquely identifies the state; additional filters add overhead.
state = await State.find_one(State.id == node_object_id)
# Optionally validate state.run_id/graph_name/namespace_name afterwards for clearer 404 messages.🤖 Prompt for AI Agents
In state-manager/app/controller/get_node_run_details.py around lines 41 to 48,
the database query redundantly filters by run_id, graph_name, and namespace_name
even though State.id uniquely identifies the record; replace the multi-field
find_one with a lookup by id only (await State.find_one(State.id ==
node_object_id)) and then, after fetching, validate
run_id/graph_name/namespace_name on the returned state to raise the appropriate
404 or error message if they don’t match.
| class NodeRunDetailsResponse(BaseModel): | ||
| """Response model for node run details API""" | ||
| id: str = Field(..., description="Unique identifier for the node (state ID)") | ||
| node_name: str = Field(..., description="Name of the node") | ||
| identifier: str = Field(..., description="Identifier of the node") | ||
| graph_name: str = Field(..., description="Name of the graph template") | ||
| run_id: str = Field(..., description="Run ID of the execution") | ||
| status: StateStatusEnum = Field(..., description="Status of the state") | ||
| inputs: Dict[str, Any] = Field(..., description="Inputs of the state") | ||
| outputs: Dict[str, Any] = Field(..., description="Outputs of the state") | ||
| error: Optional[str] = Field(None, description="Error message if any") | ||
| parents: Dict[str, str] = Field(..., description="Parent node identifiers") | ||
| created_at: str = Field(..., description="Creation timestamp") | ||
| updated_at: str = Field(..., description="Last update timestamp") No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Model looks good; consider optional timestamps.
If a state can have missing timestamps, Optional[str] avoids empty-string placeholders and clearer OpenAPI.
- created_at: str = Field(..., description="Creation timestamp")
- updated_at: str = Field(..., description="Last update timestamp")
+ created_at: Optional[str] = Field(None, description="Creation timestamp")
+ updated_at: Optional[str] = Field(None, description="Last update timestamp")📝 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.
| class NodeRunDetailsResponse(BaseModel): | |
| """Response model for node run details API""" | |
| id: str = Field(..., description="Unique identifier for the node (state ID)") | |
| node_name: str = Field(..., description="Name of the node") | |
| identifier: str = Field(..., description="Identifier of the node") | |
| graph_name: str = Field(..., description="Name of the graph template") | |
| run_id: str = Field(..., description="Run ID of the execution") | |
| status: StateStatusEnum = Field(..., description="Status of the state") | |
| inputs: Dict[str, Any] = Field(..., description="Inputs of the state") | |
| outputs: Dict[str, Any] = Field(..., description="Outputs of the state") | |
| error: Optional[str] = Field(None, description="Error message if any") | |
| parents: Dict[str, str] = Field(..., description="Parent node identifiers") | |
| created_at: str = Field(..., description="Creation timestamp") | |
| updated_at: str = Field(..., description="Last update timestamp") | |
| class NodeRunDetailsResponse(BaseModel): | |
| """Response model for node run details API""" | |
| id: str = Field(..., description="Unique identifier for the node (state ID)") | |
| node_name: str = Field(..., description="Name of the node") | |
| identifier: str = Field(..., description="Identifier of the node") | |
| graph_name: str = Field(..., description="Name of the graph template") | |
| run_id: str = Field(..., description="Run ID of the execution") | |
| status: StateStatusEnum = Field(..., description="Status of the state") | |
| inputs: Dict[str, Any] = Field(..., description="Inputs of the state") | |
| outputs: Dict[str, Any] = Field(..., description="Outputs of the state") | |
| error: Optional[str] = Field(None, description="Error message if any") | |
| parents: Dict[str, str] = Field(..., description="Parent node identifiers") | |
| created_at: Optional[str] = Field(None, description="Creation timestamp") | |
| updated_at: Optional[str] = Field(None, description="Last update timestamp") |
🤖 Prompt for AI Agents
In state-manager/app/models/node_run_details_models.py around lines 6 to 19, the
created_at and updated_at fields are non-optional strings which can force
empty-string placeholders; change both fields to Optional[str] with a default of
None (i.e., Field(None, description=...)) so timestamps can be absent and
OpenAPI reflects nullability, and update their descriptions if needed to
indicate they may be missing.
| # Verify the result | ||
| assert isinstance(result, NodeRunDetailsResponse) | ||
| assert result.id == node_id | ||
| assert result.node_name == "test_node" | ||
| assert result.identifier == "test_identifier" | ||
| assert result.graph_name == graph_name | ||
| assert result.run_id == run_id | ||
| assert result.status == StateStatusEnum.SUCCESS | ||
| assert result.inputs == {"input1": "value1"} | ||
| assert result.outputs == {"output1": "result1"} | ||
| assert result.error is None | ||
| assert len(result.parents) == 1 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen assertions (timestamps, parent IDs as strings).
Add checks that timestamps are non-empty ISO strings and parent values are strings.
# Verify the result
assert isinstance(result, NodeRunDetailsResponse)
assert result.id == node_id
assert result.node_name == "test_node"
assert result.identifier == "test_identifier"
assert result.graph_name == graph_name
assert result.run_id == run_id
assert result.status == StateStatusEnum.SUCCESS
assert result.inputs == {"input1": "value1"}
assert result.outputs == {"output1": "result1"}
assert result.error is None
assert len(result.parents) == 1
+ # Ensure parents are stringified and timestamps are ISO strings
+ assert all(isinstance(v, str) for v in result.parents.values())
+ assert isinstance(result.created_at, str) and result.created_at != ""
+ assert isinstance(result.updated_at, str) and result.updated_at != ""📝 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.
| # Verify the result | |
| assert isinstance(result, NodeRunDetailsResponse) | |
| assert result.id == node_id | |
| assert result.node_name == "test_node" | |
| assert result.identifier == "test_identifier" | |
| assert result.graph_name == graph_name | |
| assert result.run_id == run_id | |
| assert result.status == StateStatusEnum.SUCCESS | |
| assert result.inputs == {"input1": "value1"} | |
| assert result.outputs == {"output1": "result1"} | |
| assert result.error is None | |
| assert len(result.parents) == 1 | |
| # Verify the result | |
| assert isinstance(result, NodeRunDetailsResponse) | |
| assert result.id == node_id | |
| assert result.node_name == "test_node" | |
| assert result.identifier == "test_identifier" | |
| assert result.graph_name == graph_name | |
| assert result.run_id == run_id | |
| assert result.status == StateStatusEnum.SUCCESS | |
| assert result.inputs == {"input1": "value1"} | |
| assert result.outputs == {"output1": "result1"} | |
| assert result.error is None | |
| assert len(result.parents) == 1 | |
| # Ensure parents are stringified and timestamps are ISO strings | |
| assert all(isinstance(v, str) for v in result.parents.values()) | |
| assert isinstance(result.created_at, str) and result.created_at != "" | |
| assert isinstance(result.updated_at, str) and result.updated_at != "" |
🤖 Prompt for AI Agents
In state-manager/tests/unit/controller/test_get_node_run_details.py around lines
44 to 56, strengthen the assertions by verifying timestamp fields are non-empty
ISO-formatted strings and that parent IDs are strings: after existing checks
assert that created_at and updated_at (or any timestamp fields present on
result) are truthy and match an ISO datetime pattern (e.g. contains 'T' and 'Z'
or use datetime.fromisoformat in a try/except), and iterate over result.parents
to assert each parent value is an instance of str; add these assertions
immediately after the existing result property checks.
- Increased font size in globals.css for improved readability. - Refactored loadGraphStructure and loadNodeDetails functions to use useCallback for better performance. - Adjusted node layout parameters in GraphVisualization for enhanced spacing and visual clarity. - Added a Background component to the graph visualization for improved aesthetics. These changes collectively enhance the user experience and visual presentation of the dashboard. auth: @nk-ag
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (2)
dashboard/src/app/globals.css (1)
86-114: Replace hard-coded React Flow node colors with tokens.Use your CSS variables for border, backgrounds, and focus rings to keep themes consistent.
.react-flow__node{ - border-radius: 0.625rem; - border: 1px solid #87ceeb; - background-color: #2a3a6a; - color: #f8fafc; + border-radius: var(--radius); + border: 1px solid var(--ring); + background-color: var(--muted); + color: var(--foreground); @@ - &:hover{ - background-color: #6579b4; - } + &:hover{ background-color: var(--accent-lighter); } @@ - &:active{ - background-color: #6579b4; - } + &:active{ background-color: var(--accent-lighter); } @@ - &:disabled{ - background-color: #2a3a6a; + &:disabled{ + background-color: var(--muted); cursor: not-allowed; } @@ - &:focus-within{ - outline: 2px solid #87ceeb; - } + &:focus-within{ outline: 2px solid var(--ring); } }dashboard/src/components/GraphVisualization.tsx (1)
439-456: Make node details an accessible dialog (role, label, Escape to close).Add dialog semantics and a keyboard escape; label ties to the title.
- <div className="fixed inset-0 bg-background/80 backdrop-blur-sm flex items-center justify-center z-50"> - <Card className="max-w-2xl w-full mx-4 max-h-[80vh] overflow-y-auto"> + <div className="fixed inset-0 bg-background/80 backdrop-blur-sm flex items-center justify-center z-50"> + <Card + className="max-w-2xl w-full mx-4 max-h-[80vh] overflow-y-auto" + role="dialog" + aria-modal="true" + aria-labelledby="node-details-title" + > @@ - <CardTitle>Node Details</CardTitle> + <CardTitle id="node-details-title">Node Details</CardTitle> @@ - <Button - onClick={() => { - setSelectedNode(null); - setSelectedNodeDetails(null); - setNodeDetailsError(null); - }} + <Button + aria-label="Close node details" + onClick={closeNodeDetails} variant="ghost" size="icon" >Add a close helper and Escape listener:
import React, { useState, useEffect, useCallback, useMemo } from 'react'; +import React, { useRef } from 'react'; @@ const [nodeDetailsError, setNodeDetailsError] = useState<string | null>(null); + + const closeNodeDetails = useCallback(() => { + setSelectedNode(null); + setSelectedNodeDetails(null); + setNodeDetailsError(null); + }, []); + + useEffect(() => { + if (!selectedNode) return; + const onKeyDown = (e: KeyboardEvent) => { if (e.key === 'Escape') closeNodeDetails(); }; + window.addEventListener('keydown', onKeyDown); + return () => window.removeEventListener('keydown', onKeyDown); + }, [selectedNode, closeNodeDetails]);Also applies to: 498-499, 444-452
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
dashboard/src/app/globals.css(1 hunks)dashboard/src/components/GraphVisualization.tsx(8 hunks)dashboard/src/components/ui/badge.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
dashboard/src/components/ui/badge.tsx (1)
dashboard/src/lib/utils.ts (1)
cn(4-6)
dashboard/src/components/GraphVisualization.tsx (3)
dashboard/src/types/state-manager.ts (1)
NodeRunDetailsResponse(202-215)state-manager/app/models/node_run_details_models.py (1)
NodeRunDetailsResponse(6-19)dashboard/src/services/clientApi.ts (1)
clientApiService(66-66)
🔇 Additional comments (3)
dashboard/src/app/globals.css (1)
4-4: Confirm Tailwind v4 features are configured.@custom-variant and @theme inline require Tailwind v4 pipeline. Ensure CI/build uses v4 with nesting enabled.
dashboard/src/components/ui/badge.tsx (1)
1-49: Badge component looks good.API shape, variants via cva, and Slot support are solid.
dashboard/src/components/GraphVisualization.tsx (1)
577-585: Type-safety: does GraphNodeType expose error?You render selectedNode.error; if GraphNodeType lacks it, this becomes any. Align types or guard.
| @font-face { | ||
| font-family: "Dark Grotesque"; | ||
| src: url("/DarkerGrotesque-Regular.ttf") format("truetype"); | ||
| font-weight: 400; | ||
| font-style: normal; | ||
| font-display: swap; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Font file/name mismatch and lack of modern formats.
“Dark Grotesque” family references a TTF named DarkerGrotesque; add woff2 and align naming/path.
-@font-face {
- font-family: "Dark Grotesque";
- src: url("/DarkerGrotesque-Regular.ttf") format("truetype");
+@font-face {
+ font-family: "Dark Grotesque";
+ src:
+ url("/fonts/DarkerGrotesque-Regular.woff2") format("woff2"),
+ url("/fonts/DarkerGrotesque-Regular.woff") format("woff");Verify the actual file path(s) exist under public/. Want me to add a fallback stack to avoid FOIT?
📝 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.
| @font-face { | |
| font-family: "Dark Grotesque"; | |
| src: url("/DarkerGrotesque-Regular.ttf") format("truetype"); | |
| font-weight: 400; | |
| font-style: normal; | |
| font-display: swap; | |
| } | |
| @font-face { | |
| font-family: "Dark Grotesque"; | |
| src: | |
| url("/fonts/DarkerGrotesque-Regular.woff2") format("woff2"), | |
| url("/fonts/DarkerGrotesque-Regular.woff") format("woff"); | |
| font-weight: 400; | |
| font-style: normal; | |
| font-display: swap; | |
| } |
🤖 Prompt for AI Agents
In dashboard/src/app/globals.css around lines 7-13 the @font-face uses
font-family "Dark Grotesque" while the file is named DarkerGrotesque and only
provides a TTF; update and align names/paths, add modern formats, and include a
fallback stack: place optimized font files under public (e.g. public/fonts/) and
add src entries for woff2 and woff (highest-priority woff2), keep font-display:
swap, make the font-family name match the actual file (or rename the file to
match the chosen family), and add a fallback CSS font stack (e.g. system-ui,
-apple-system, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif) applied
where the font is used; finally verify the actual font files exist at the public
path you reference before committing.
| /* Custom Scrollbar Styling */ | ||
| ::-webkit-scrollbar { | ||
| width: 8px; | ||
| height: 8px; | ||
| } | ||
|
|
||
| ::-webkit-scrollbar-track { | ||
| background: var(--muted); | ||
| border-radius: var(--radius); | ||
| } | ||
|
|
||
| ::-webkit-scrollbar-thumb { | ||
| background: var(--chart-1); | ||
| border-radius: var(--radius); | ||
| transition: background 0.2s ease; | ||
| } | ||
|
|
||
| ::-webkit-scrollbar-thumb:hover { | ||
| background: var(--accent-lightest); | ||
| } | ||
|
|
||
| ::-webkit-scrollbar-corner { | ||
| background: var(--muted); | ||
| } | ||
|
|
||
| /* Firefox scrollbar styling */ | ||
| * { | ||
| scrollbar-width: thin; | ||
| scrollbar-color: var(--border) var(--muted); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Scrollbar sizing: consider wider thumbs on pointer devices.
8px can be hard to grab; consider 10–12px on desktops while keeping thin on touch.
🤖 Prompt for AI Agents
In dashboard/src/app/globals.css around lines 55 to 84, the custom scrollbar
width is fixed at 8px which is hard to grab on pointer devices; update the
styles to use a wider thumb (10–12px) for devices with a fine pointer via a
media query (e.g., @media (pointer: fine) and (hover: hover)) while keeping 8px
for touch/coarse pointers, adjust corresponding ::-webkit-scrollbar width/height
and scrollbar-width for Firefox inside that media query, and preserve existing
colors, radii, and hover transition.
| &:focus{ | ||
| outline: none; | ||
| } | ||
| &:disabled{ | ||
| background-color: #2a3a6a; | ||
| cursor: not-allowed; | ||
| } | ||
| &:focus-visible{ | ||
| outline: none; | ||
| } |
There was a problem hiding this comment.
Don’t remove focus outlines; provide a visible focus style.
Killing outlines harms keyboard/a11y. Keep a clear focus-visible ring.
- &:focus{
- outline: none;
- }
- &:focus-visible{
- outline: none;
- }
+ &:focus-visible{
+ outline: 2px solid var(--ring);
+ outline-offset: 2px;
+ }📝 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.
| &:focus{ | |
| outline: none; | |
| } | |
| &:disabled{ | |
| background-color: #2a3a6a; | |
| cursor: not-allowed; | |
| } | |
| &:focus-visible{ | |
| outline: none; | |
| } | |
| &:focus-visible{ | |
| outline: 2px solid var(--ring); | |
| outline-offset: 2px; | |
| } | |
| &:disabled{ | |
| background-color: #2a3a6a; | |
| cursor: not-allowed; | |
| } |
🤖 Prompt for AI Agents
In dashboard/src/app/globals.css around lines 101 to 110, the rules remove focus
outlines which breaks keyboard accessibility; remove the &:focus { outline:
none; } and &:focus-visible { outline: none; } removals and replace them with a
visible focus style for keyboard users (e.g., a clear high-contrast outline or
focus ring using outline or box-shadow) so focused elements show a 2–4px
accessible ring (or box-shadow) with sufficient contrast; ensure the :disabled
rule remains but still allows an accessible focus indicator if the element can
receive focus, and choose a color/opacity that meets contrast requirements.
| html { | ||
| font-size: 25px; /* Increased from default 16px */ | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid 25px base font-size; use responsive clamp or 100%.
25px base bloats rems and can break layout/accessibility.
- html {
- font-size: 25px; /* Increased from default 16px */
- }
+ html { font-size: clamp(16px, 1.1vw + 14px, 18px); }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In dashboard/src/app/globals.css around lines 165–167, the html element
currently sets a fixed base font-size of 25px which inflates rem units and harms
responsiveness/accessibility; replace the fixed pixel value with a responsive
approach — either set html font-size to 100% (browser default) and adjust
component sizes using rems/ems, or use a CSS clamp() expression to provide a
scalable min/preferred/max font-size (e.g., clamp between an accessible minimum
and a reasonable maximum) so typography scales across viewports without breaking
layout.
| const loadNodeDetails = useCallback(async (nodeId: string, graphName: string) => { | ||
| setIsLoadingNodeDetails(true); | ||
| setNodeDetailsError(null); | ||
|
|
||
| try { | ||
| const details = await clientApiService.getNodeRunDetails(namespace, graphName, runId, nodeId); | ||
| setSelectedNodeDetails(details); | ||
| } catch (err) { | ||
| setNodeDetailsError(err instanceof Error ? err.message : 'Failed to load node details'); | ||
| } finally { | ||
| setIsLoadingNodeDetails(false); | ||
| } | ||
| }, [namespace, runId]); | ||
|
|
There was a problem hiding this comment.
Prevent race conditions when loading node details.
Rapid clicks can interleave responses; gate state updates to the latest request.
-import React, { useState, useEffect, useCallback, useMemo } from 'react';
+import React, { useState, useEffect, useCallback, useMemo, useRef } from 'react';
@@
- const loadNodeDetails = useCallback(async (nodeId: string, graphName: string) => {
- setIsLoadingNodeDetails(true);
- setNodeDetailsError(null);
+ const requestIdRef = useRef(0);
+ const loadNodeDetails = useCallback(async (nodeId: string, graphName: string) => {
+ const reqId = ++requestIdRef.current;
+ setIsLoadingNodeDetails(true);
+ setNodeDetailsError(null);
try {
- const details = await clientApiService.getNodeRunDetails(namespace, graphName, runId, nodeId);
- setSelectedNodeDetails(details);
+ const details = await clientApiService.getNodeRunDetails(namespace, graphName, runId, nodeId);
+ if (reqId === requestIdRef.current) {
+ setSelectedNodeDetails(details);
+ }
} catch (err) {
- setNodeDetailsError(err instanceof Error ? err.message : 'Failed to load node details');
+ if (reqId === requestIdRef.current) {
+ setNodeDetailsError(err instanceof Error ? err.message : 'Failed to load node details');
+ }
} finally {
- setIsLoadingNodeDetails(false);
+ if (reqId === requestIdRef.current) {
+ setIsLoadingNodeDetails(false);
+ }
}
}, [namespace, runId]);Also applies to: 120-123
🤖 Prompt for AI Agents
In dashboard/src/components/GraphVisualization.tsx around lines 146-159 (and
similarly 120-123), rapid clicks can cause interleaved responses to overwrite
state; add a local request token (e.g., incrementing ref like
latestNodeRequestRef.current = id) or use an AbortController per request,
capture that token/controller in the try block, and only call
setSelectedNodeDetails / setNodeDetailsError / setIsLoadingNodeDetails in the
response path if the token/controller matches the current one (or if not
aborted). Ensure the ref is initialized with useRef, incremented before each
request, and checked after the await, and clean up by aborting previous
controller if using AbortController.
| useEffect(() => { | ||
| if (namespace && runId) { | ||
| loadGraphStructure(); | ||
| } | ||
| }, [namespace, runId]); | ||
| }, [namespace, runId, loadGraphStructure]); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clear selection when namespace/run changes.
Prevents stale overlays after context switches.
useEffect(() => {
if (namespace && runId) {
loadGraphStructure();
}
}, [namespace, runId, loadGraphStructure]);
+
+ useEffect(() => {
+ setSelectedNode(null);
+ setSelectedNodeDetails(null);
+ setNodeDetailsError(null);
+ }, [namespace, runId]);📝 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.
| useEffect(() => { | |
| if (namespace && runId) { | |
| loadGraphStructure(); | |
| } | |
| }, [namespace, runId]); | |
| }, [namespace, runId, loadGraphStructure]); | |
| useEffect(() => { | |
| if (namespace && runId) { | |
| loadGraphStructure(); | |
| } | |
| }, [namespace, runId, loadGraphStructure]); | |
| useEffect(() => { | |
| setSelectedNode(null); | |
| setSelectedNodeDetails(null); | |
| setNodeDetailsError(null); | |
| }, [namespace, runId]); |
🤖 Prompt for AI Agents
In dashboard/src/components/GraphVisualization.tsx around lines 160 to 165, the
useEffect that reloads the graph on namespace/run changes should also clear any
selected node/edge or overlay state to avoid stale overlays; update the effect
to call the existing selection-clearing function or set the selection state to
null/empty (e.g., setSelectedNode(null), setSelectedEdge(null) or
clearOverlay()) before or after calling loadGraphStructure so selection and
overlays are reset whenever namespace or runId changes.
| const reactFlowNodes: Node[] = []; | ||
| const layerWidth = 400; // Increased horizontal spacing between layers | ||
| const nodeHeight = 150; // Increased vertical spacing between nodes | ||
| const layerWidth = 450; // Increased horizontal spacing between layers | ||
| const nodeHeight = 250; // Increased vertical spacing between nodes | ||
|
|
||
| layers.forEach((layer, layerIndex) => { | ||
| const layerX = layerIndex * layerWidth + 150; | ||
| const layerX = layerIndex * layerWidth + 200; | ||
| const totalHeight = layer.length * nodeHeight; | ||
| const startY = (800 - totalHeight) / 2; // Center vertically | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid hard-coding container height in layout math.
startY uses 800; tie it to the container height to prevent drift.
- const layerWidth = 450; // Increased horizontal spacing between layers
- const nodeHeight = 250; // Increased vertical spacing between nodes
+ const layerWidth = 450;
+ const nodeHeight = 250;
+ const containerHeight = 800; // keep in one place with the style below
@@
- const startY = (800 - totalHeight) / 2; // Center vertically
+ const startY = (containerHeight - totalHeight) / 2;
@@
- <div className="border border-border rounded-xl overflow-hidden" style={{ height: '800px' }}>
+ <div className="border border-border rounded-xl overflow-hidden" style={{ height: '800px' }}>
<ReactFlowIf you later change the height, adjust containerHeight alongside.
Also applies to: 411-414
| width: 10, | ||
| height: 10, | ||
| color: '#87ceeb', | ||
| }, | ||
| style: { | ||
| stroke: '#10b981', | ||
| strokeWidth: 3, | ||
| stroke: '#87ceeb', | ||
| strokeWidth: 2, | ||
| strokeDasharray: 'none', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use theme tokens instead of hex for edges/background.
Keep visuals in sync with themes.
- markerEnd: {
- type: MarkerType.ArrowClosed,
- width: 10,
- height: 10,
- color: '#87ceeb',
- },
+ markerEnd: {
+ type: MarkerType.ArrowClosed,
+ width: 10,
+ height: 10,
+ color: 'hsl(var(--primary))',
+ },
style: {
- stroke: '#87ceeb',
+ stroke: 'hsl(var(--primary))',
strokeWidth: 2,
strokeDasharray: 'none',
},
@@
- <Background color="#031035" />
+ <Background color="hsl(var(--background))" />Also applies to: 430-431
🤖 Prompt for AI Agents
In dashboard/src/components/GraphVisualization.tsx around lines 280-287 (and
also lines 430-431), the code uses hard-coded hex colors ('#87ceeb') for
edge/background styling; replace these literals with theme tokens instead (use
the app theme object or CSS variables used elsewhere). Import/use the theme
(e.g., useTheme or context) and substitute the hex values with the appropriate
theme keys (edge color, background or primary/secondary token) so the graph
visuals follow dark/light themes and update dynamically with theme changes.
| default: | ||
| "border-transparent bg-primary text-primary-foreground [a&]:hover:bg-primary/90", | ||
| success: | ||
| "border-transparent bg-chart-2 text-primary-foreground [a&]:hover:bg-primary/90", | ||
| secondary: | ||
| "border-transparent bg-secondary text-secondary-foreground [a&]:hover:bg-secondary/90", | ||
| destructive: | ||
| "border-transparent bg-destructive text-white [a&]:hover:bg-destructive/90 focus-visible:ring-destructive/20 dark:focus-visible:ring-destructive/40 dark:bg-destructive/60", | ||
| outline: | ||
| "text-foreground [a&]:hover:bg-accent [a&]:hover:text-accent-foreground", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Check color contrast for non-primary variants.
“success” uses bg-chart-2 with text-primary-foreground; ensure WCAG AA contrast. Consider dedicated --success/--success-foreground tokens.
🏁 Script executed:
#!/bin/bash
# Quick contrast checks (requires python3)
python - <<'PY'
from colorsys import rgb_to_hls
def hex_to_rgb(h): h=h.lstrip('#'); return tuple(int(h[i:i+2],16) for i in (0,2,4))
def rel_lum(c):
def chan(u): u/=255; return u/12.92 if u<=0.03928 else ((u+0.055)/1.055)**2.4
r,g,b=map(chan,c); return 0.2126*r+0.7152*g+0.0722*b
def contrast(a,b):
L1,L2=sorted([rel_lum(hex_to_rgb(a)), rel_lum(hex_to_rgb(b))], reverse=True)
return (L1+0.05)/(L2+0.05)
pairs = {
"success(bg-chart-2 vs primary-foreground)": ("#4ade80","#031035"),
"success(bg-chart-2 vs foreground)": ("#4ade80","#f8fafc"),
"destructive(bg-destructive vs white)": ("#ff6b8a","#ffffff"),
}
for name,(bg,fg) in pairs.items():
print(name, "contrast:", round(contrast(bg, fg),2))
PYLength of output: 158
Fix WCAG failures for non-primary badge variants (dashboard/src/components/ui/badge.tsx:12–21).
- success: bg-chart-2 (#4ade80) vs text-primary-foreground (#031035) = 10.67 (OK); bg-chart-2 vs foreground (#f8fafc) = 1.67 (FAIL, <4.5 AA). Any light/white text in the success badge will be inaccessible.
- destructive: bg-destructive (#ff6b8a) vs white = 2.72 (FAIL, <4.5 AA).
Remediation: add semantic tokens (e.g. --badge-success, --badge-success-foreground, --badge-destructive-foreground) or adjust bg/fg colors so contrast ≥ 4.5 for normal text; verify hover and dark-mode states after changes.
🤖 Prompt for AI Agents
In dashboard/src/components/ui/badge.tsx around lines 12 to 21 the success and
destructive badge color pairs fail WCAG contrast (success bg-chart-2 vs
light/white foreground and destructive bg-destructive vs white), make them
accessible by introducing semantic CSS tokens (e.g. --badge-success,
--badge-success-foreground, --badge-destructive, --badge-destructive-foreground)
or by choosing new background/foreground colors with a minimum 4.5:1 contrast
for normal text, update the badge class mappings to use those tokens, and verify
& adjust hover and dark-mode variants to preserve ≥4.5 contrast across states.
| function Badge({ | ||
| className, | ||
| variant, | ||
| asChild = false, | ||
| ...props | ||
| }: React.ComponentProps<"span"> & | ||
| VariantProps<typeof badgeVariants> & { asChild?: boolean }) { | ||
| const Comp = asChild ? Slot : "span" | ||
|
|
||
| return ( | ||
| <Comp | ||
| data-slot="badge" | ||
| className={cn(badgeVariants({ variant }), className)} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Forward refs for better composition and a11y.
Forward the ref so consumers can focus the badge or measure it; add displayName.
-function Badge({
+const Badge = React.forwardRef<HTMLSpanElement, React.ComponentProps<"span"> &
+ VariantProps<typeof badgeVariants> & { asChild?: boolean }>(function Badge({
className,
variant,
asChild = false,
...props
-}: React.ComponentProps<"span"> &
- VariantProps<typeof badgeVariants> & { asChild?: boolean }) {
- const Comp = asChild ? Slot : "span"
+}, ref) {
+ const Comp = asChild ? Slot : "span"
return (
<Comp
data-slot="badge"
- className={cn(badgeVariants({ variant }), className)}
+ ref={ref}
+ className={cn(badgeVariants({ variant }), className)}
{...props}
/>
)
-}
+})
+
+Badge.displayName = "Badge"
export { Badge, badgeVariants }📝 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.
| function Badge({ | |
| className, | |
| variant, | |
| asChild = false, | |
| ...props | |
| }: React.ComponentProps<"span"> & | |
| VariantProps<typeof badgeVariants> & { asChild?: boolean }) { | |
| const Comp = asChild ? Slot : "span" | |
| return ( | |
| <Comp | |
| data-slot="badge" | |
| className={cn(badgeVariants({ variant }), className)} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| const Badge = React.forwardRef<HTMLSpanElement, React.ComponentProps<"span"> & | |
| VariantProps<typeof badgeVariants> & { asChild?: boolean }>(function Badge({ | |
| className, | |
| variant, | |
| asChild = false, | |
| ...props | |
| }, ref) { | |
| const Comp = asChild ? Slot : "span" | |
| return ( | |
| <Comp | |
| data-slot="badge" | |
| ref={ref} | |
| className={cn(badgeVariants({ variant }), className)} | |
| {...props} | |
| /> | |
| ) | |
| }) | |
| Badge.displayName = "Badge" | |
| export { Badge, badgeVariants } |
🤖 Prompt for AI Agents
In dashboard/src/components/ui/badge.tsx around lines 30 to 46, the Badge
component should accept and forward a ref and have a displayName for better
composition and a11y; wrap the function with React.forwardRef (typing the ref as
HTMLSpanElement), change the function signature to (props, ref) and pass the ref
through to the rendered Comp element, update the props typing to include
React.ComponentPropsWithoutRef<"span"> (or ensure ref-compatible types), and set
Badge.displayName = "Badge".
- Added a new API endpoint to fetch available namespaces, improving data retrieval for the dashboard. - Integrated a Select component for namespace selection in the Dashboard and RunsTable components, enhancing user experience. - Updated the global styles to adjust font size for better readability. - Refactored the Dashboard component to fetch both configuration and namespaces on mount, ensuring a smoother user experience. These changes collectively enhance the functionality and usability of the dashboard.
- Refactored the namespace selection process to fetch both configuration and available namespaces concurrently, enhancing performance. - Updated logic to select the appropriate namespace based on availability, ensuring a more robust user experience. - Added fallback mechanisms to handle cases where no namespaces are available, maintaining a default selection. These changes enhance the functionality and reliability of the namespace management in the dashboard.
- Deleted package.json and package-lock.json as part of project restructuring. - This change simplifies the project setup and prepares for a new dependency management approach.
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dashboard/src/app/page.tsx (1)
95-102: Avoid mutating API response objects; create a hydrated copy.Directly assigning name/namespace can have side effects and may violate types. Hydrate via spread.
- const graphTemplate = await clientApiService.getGraphTemplate(namespace, graphName); - graphTemplate.name = graphName; - graphTemplate.namespace = namespace; - setSelectedGraphTemplate(graphTemplate); + const graphTemplate = await clientApiService.getGraphTemplate(namespace, graphName); + const hydrated = { ...graphTemplate, name: graphName, namespace }; + setSelectedGraphTemplate(hydrated);
♻️ Duplicate comments (11)
dashboard/src/app/globals.css (4)
7-13: Font delivery: add woff2/woff and align paths.Ship modern formats under public/fonts and keep swap.
@font-face { font-family: "Dark Grotesque"; - src: url("/DarkerGrotesque-Regular.ttf") format("truetype"); + src: + url("/fonts/DarkerGrotesque-Regular.woff2") format("woff2"), + url("/fonts/DarkerGrotesque-Regular.woff") format("woff"); font-weight: 400; font-style: normal; font-display: swap; }Also ensure the actual files exist at /public/fonts/.
55-84: Scrollbar UX: widen on pointer devices; keep thin on touch.Current 8px is hard to grab on desktops.
::-webkit-scrollbar { - width: 8px; - height: 8px; + width: 8px; + height: 8px; } @@ * { - scrollbar-width: thin; + scrollbar-width: thin; scrollbar-color: var(--border) var(--muted); } + +@media (pointer: fine) and (hover: hover) { + ::-webkit-scrollbar { width: 12px; height: 12px; } + * { scrollbar-width: auto; } +}
86-114: React Flow node: replace hexes with tokens and fix focus styles.Use theme vars and keep visible focus indicators for a11y.
.react-flow__node{ - border-radius: 0.625rem; - border: 1px solid #87ceeb; - background-color: #2a3a6a; - color: #f8fafc; + border-radius: var(--radius); + border: 1px solid var(--ring); + background-color: var(--muted); + color: var(--foreground); font-size: 0.875rem; font-weight: 500; text-align: center; transition: all 0.3s ease; &:hover{ - background-color: #6579b4; + background-color: var(--accent-lighter); } &:active{ - background-color: #6579b4; + background-color: var(--accent-lighter); } - &:focus{ - outline: none; - } &:disabled{ - background-color: #2a3a6a; + background-color: var(--muted); cursor: not-allowed; } - &:focus-visible{ - outline: none; - } + &:focus-visible{ + outline: 2px solid var(--ring); + outline-offset: 2px; + } &:focus-within{ - outline: 2px solid #87ceeb; + outline: 2px solid var(--ring); } }
161-167: Base sizing: avoid fixed 120%; use clamp for responsiveness.120% forces ~19.2px base and may conflict with user zoom. Prefer clamp or 100%.
- html { - font-size: 120%; /* Increased from default 16px */ - } + html { font-size: clamp(16px, 1.1vw + 14px, 18px); }dashboard/src/types/state-manager.ts (2)
88-98: StateStatus is missing PRUNED and NEXT_CREATED_ERROR.Add missing statuses to match server values.
export enum StateStatus { CREATED = 'CREATED', QUEUED = 'QUEUED', EXECUTED = 'EXECUTED', NEXT_CREATED = 'NEXT_CREATED', + NEXT_CREATED_ERROR = 'NEXT_CREATED_ERROR', RETRY_CREATED = 'RETRY_CREATED', TIMEDOUT = 'TIMEDOUT', ERRORED = 'ERRORED', CANCELLED = 'CANCELLED', - SUCCESS = 'SUCCESS' + SUCCESS = 'SUCCESS', + PRUNED = 'PRUNED' }
8-14: Make RetryPolicyModel.strategy a strict type (enum/union).Mirror server values to prevent invalid strategies in the UI.
Would you confirm the allowed strategy values on the server and align them here?
#!/bin/bash # Find retry policy definitions/usage on server rg -n -C2 -S "RetryPolicy|retry_?policy|strategy" state-manager | head -n 200dashboard/src/app/page.tsx (2)
133-151: A11y: associate the “Namespace” label with the Select control.Screen readers won’t link the text to the control. Replace the span with a label and add an id to Select.
- <span className="text-sm text-muted-foreground">Namespace:</span> - <Select + <label htmlFor="namespace-select" className="text-sm text-muted-foreground">Namespace:</label> + <Select + id="namespace-select" value={namespace} onChange={(e) => setNamespace(e.target.value)} className="w-32 h-8" >
177-181: Type mismatch: Tabs onValueChange excludes 'graph' while a graph TabsContent exists.Either include 'graph' in the union or remove/hide the graph content.
- <Tabs value={activeTab} onValueChange={(value) => setActiveTab(value as 'overview' | 'runs')} className="w-full"> + <Tabs value={activeTab} onValueChange={(value) => setActiveTab(value as 'overview' | 'graph' | 'runs')} className="w-full">dashboard/src/components/RunsTable.tsx (3)
231-237: Icon-only close button lacks an accessible name.Add sr-only text (or aria-label) so screen readers announce the action.
- <Button + <Button onClick={() => setShowGraph(false)} variant="ghost" size="sm" > - <XCircle className="w-5 h-5" /> + <XCircle className="w-5 h-5" aria-hidden="true" /> + <span className="sr-only">Close</span> </Button>
255-266: Associate the “Page size” label with the select (fixes a11y lint).Adds htmlFor/id; resolves Biome a11y/noLabelWithoutControl.
- <label className="text-sm text-muted-foreground">Page size:</label> - <select - value={pageSize} + <label htmlFor="page-size-select" className="text-sm text-muted-foreground">Page size:</label> + <select + id="page-size-select" + value={pageSize} onChange={(e) => handlePageSizeChange(Number(e.target.value))} className="border border-input rounded-md px-2 py-1 text-sm bg-background" >
296-300: Make clickable rows keyboard-accessible.Add role, tabIndex, and Enter key handler.
- <tr + <tr key={run.run_id} className="hover:bg-muted/50 transition-colors cursor-pointer" - onClick={() => handleRowClick(run.run_id)} + onClick={() => handleRowClick(run.run_id)} + role="button" + tabIndex={0} + onKeyDown={(e) => { if (e.key === 'Enter') handleRowClick(run.run_id) }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
dashboard/src/app/api/namespaces/route.ts(1 hunks)dashboard/src/app/globals.css(1 hunks)dashboard/src/app/page.tsx(5 hunks)dashboard/src/components/RunsTable.tsx(5 hunks)dashboard/src/components/ui/select.tsx(1 hunks)dashboard/src/services/clientApi.ts(2 hunks)dashboard/src/types/state-manager.ts(5 hunks)state-manager/app/controller/list_namespaces.py(1 hunks)state-manager/app/main.py(2 hunks)state-manager/app/models/list_models.py(1 hunks)state-manager/app/routes.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
dashboard/src/components/ui/select.tsx (1)
dashboard/src/lib/utils.ts (1)
cn(4-6)
state-manager/app/controller/list_namespaces.py (2)
state-manager/app/models/db/registered_node.py (1)
RegisteredNode(8-44)state-manager/app/singletons/logs_manager.py (2)
LogsManager(9-66)get_logger(65-66)
state-manager/app/models/list_models.py (1)
dashboard/src/types/state-manager.ts (1)
ListNamespacesResponse(141-144)
dashboard/src/types/state-manager.ts (2)
state-manager/app/models/list_models.py (1)
ListNamespacesResponse(26-29)state-manager/app/models/node_run_details_models.py (1)
NodeRunDetailsResponse(6-19)
dashboard/src/app/page.tsx (3)
dashboard/src/services/clientApi.ts (1)
clientApiService(75-75)dashboard/src/components/NamespaceOverview.tsx (1)
NamespaceOverview(33-229)dashboard/src/components/RunsTable.tsx (1)
RunsTable(43-414)
dashboard/src/components/RunsTable.tsx (7)
state-manager/app/models/run_models.py (1)
RunStatusEnum(9-12)dashboard/src/components/ui/alert.tsx (2)
Alert(66-66)AlertDescription(66-66)dashboard/src/components/ui/button.tsx (1)
Button(59-59)dashboard/src/components/ui/select.tsx (1)
Select(27-27)dashboard/src/components/ui/card.tsx (4)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardContent(91-91)dashboard/src/components/GraphVisualization.tsx (1)
GraphVisualization(120-649)dashboard/src/components/ui/badge.tsx (1)
Badge(48-48)
state-manager/app/routes.py (5)
state-manager/app/models/list_models.py (1)
ListNamespacesResponse(26-29)state-manager/app/controller/list_namespaces.py (1)
list_namespaces(10-40)state-manager/app/models/node_run_details_models.py (1)
NodeRunDetailsResponse(6-19)state-manager/app/controller/get_node_run_details.py (1)
get_node_run_details(12-88)state-manager/app/utils/check_secret.py (1)
check_api_key(11-16)
🪛 Biome (2.1.2)
dashboard/src/components/RunsTable.tsx
[error] 255-255: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
🔇 Additional comments (9)
dashboard/src/app/globals.css (1)
2-2: Verify dependency and bundle size for tw-animate-css.Confirm this import exists in package.json (or is locally vendored), tree-shakes correctly, and doesn’t bloat CSS output. If unused, drop it.
dashboard/src/types/state-manager.ts (3)
141-144: LGTM: ListNamespacesResponse matches backend.
187-190: LGTM: GraphNode now exposes inputs/outputs.This unblocks richer node tooltips/details.
207-220: LGTM: NodeRunDetailsResponse aligns with server model.state-manager/app/main.py (2)
27-27: LGTM: Importing global_router is correct.
90-91: LGTM: Register global_router before namespace router.Order is fine; paths won’t conflict.
state-manager/app/models/list_models.py (1)
26-30: LGTM: ListNamespacesResponse is clear and minimal.state-manager/app/routes.py (2)
343-360: LGTM: Node run details route wiring is correct.Auth, request ID, and delegation look good.
362-383: LGTM: Global /v0/namespaces endpoint matches model.Returns both namespaces and count; consistent with dashboard expectations.
| import { NextRequest, NextResponse } from 'next/server'; | ||
|
|
||
| const API_BASE_URL = process.env.EXOSPHERE_STATE_MANAGER_URI || 'http://localhost:8000'; | ||
| const API_KEY = process.env.EXOSPHERE_API_KEY; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Disable ISR for this API route.
Ensure this stays dynamic and never statically cached.
import { NextRequest, NextResponse } from 'next/server';
const API_BASE_URL = process.env.EXOSPHERE_STATE_MANAGER_URI || 'http://localhost:8000';
const API_KEY = process.env.EXOSPHERE_API_KEY;
+export const revalidate = 0;📝 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.
| import { NextRequest, NextResponse } from 'next/server'; | |
| const API_BASE_URL = process.env.EXOSPHERE_STATE_MANAGER_URI || 'http://localhost:8000'; | |
| const API_KEY = process.env.EXOSPHERE_API_KEY; | |
| import { NextRequest, NextResponse } from 'next/server'; | |
| const API_BASE_URL = process.env.EXOSPHERE_STATE_MANAGER_URI || 'http://localhost:8000'; | |
| const API_KEY = process.env.EXOSPHERE_API_KEY; | |
| export const revalidate = 0; |
🤖 Prompt for AI Agents
In dashboard/src/app/api/namespaces/route.ts around lines 1 to 4, the API route
can be statically cached; to disable ISR and force dynamic rendering add an
explicit export to opt out of static caching. Add the line `export const dynamic
= 'force-dynamic';` near the top of the file (before any request handler) so the
route is always dynamic and never statically cached; also ensure you do not set
any revalidate headers or caching directives that could re-enable ISR.
| const response = await fetch(`${API_BASE_URL}/v0/namespaces`, { | ||
| headers: { | ||
| 'X-API-Key': API_KEY, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Forward request ID and set fetch timeout + no-store.
Forward x-exosphere-request-id for log correlation; add AbortController and cache: 'no-store'.
- const response = await fetch(`${API_BASE_URL}/v0/namespaces`, {
- headers: {
- 'X-API-Key': API_KEY,
- 'Content-Type': 'application/json',
- },
- });
+ const reqId = request.headers.get('x-exosphere-request-id') ?? undefined;
+ const controller = new AbortController();
+ const timeout = setTimeout(() => controller.abort(), 10_000);
+ try {
+ const response = await fetch(`${API_BASE_URL}/v0/namespaces`, {
+ headers: {
+ 'X-API-Key': API_KEY,
+ ...(reqId ? { 'X-Exosphere-Request-Id': reqId } : {}),
+ },
+ cache: 'no-store',
+ signal: controller.signal,
+ });
+ // ... unchanged handling below
+ } finally {
+ clearTimeout(timeout);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const response = await fetch(`${API_BASE_URL}/v0/namespaces`, { | |
| headers: { | |
| 'X-API-Key': API_KEY, | |
| 'Content-Type': 'application/json', | |
| }, | |
| }); | |
| const reqId = request.headers.get('x-exosphere-request-id') ?? undefined; | |
| const controller = new AbortController(); | |
| const timeout = setTimeout(() => controller.abort(), 10_000); | |
| try { | |
| const response = await fetch(`${API_BASE_URL}/v0/namespaces`, { | |
| headers: { | |
| 'X-API-Key': API_KEY, | |
| ...(reqId ? { 'X-Exosphere-Request-Id': reqId } : {}), | |
| }, | |
| cache: 'no-store', | |
| signal: controller.signal, | |
| }); | |
| // ... unchanged handling below | |
| } finally { | |
| clearTimeout(timeout); | |
| } |
🤖 Prompt for AI Agents
In dashboard/src/app/api/namespaces/route.ts around lines 12-17, the outgoing
fetch to `${API_BASE_URL}/v0/namespaces` must forward the incoming request's
x-exosphere-request-id for log correlation, use an AbortController with a short
timeout, and disable caching via cache: 'no-store'; to fix, read the request
header (request.headers.get('x-exosphere-request-id')) and add it to the fetch
headers as 'x-exosphere-request-id' if present, create an AbortController, set a
timeout (e.g. 10_000 ms) that calls controller.abort(), pass controller.signal
to fetch, include cache: 'no-store' in fetch options, and clear the timeout
after fetch completes or errors. Ensure error handling treats AbortError
appropriately.
| if (!response.ok) { | ||
| throw new Error(`State manager API error: ${response.status} ${response.statusText}`); | ||
| } | ||
|
|
||
| const data = await response.json(); | ||
| return NextResponse.json(data); | ||
| } catch (error) { | ||
| console.error('Error fetching namespaces:', error); | ||
| return NextResponse.json( | ||
| { error: 'Failed to fetch namespaces' }, | ||
| { status: 500 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Propagate upstream status, add timeout and no-store caching.
Avoid masking 401/404 from the state-manager as 500s; also add a short timeout and disable caching to prevent stale data.
- if (!response.ok) {
- throw new Error(`State manager API error: ${response.status} ${response.statusText}`);
- }
-
- const data = await response.json();
- return NextResponse.json(data);
- } catch (error) {
- console.error('Error fetching namespaces:', error);
- return NextResponse.json(
- { error: 'Failed to fetch namespaces' },
- { status: 500 }
- );
- }
+ if (!response.ok) {
+ const body = await response.text().catch(() => '');
+ return NextResponse.json(
+ { error: body || response.statusText || 'Upstream error' },
+ { status: response.status }
+ );
+ }
+
+ const data = await response.json();
+ return NextResponse.json(data, { status: 200 });
+ } catch (error: any) {
+ const isAbort = error?.name === 'AbortError';
+ console.error('Error fetching namespaces:', error);
+ return NextResponse.json(
+ { error: isAbort ? 'Upstream timeout' : 'Failed to fetch namespaces' },
+ { status: isAbort ? 504 : 500 }
+ );
+ }Committable suggestion skipped: line range outside the PR's diff.
| @import "tailwindcss"; | ||
| @import "tw-animate-css"; | ||
|
|
||
| @custom-variant dark (&:is(.dark *)); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Tailwind v4 custom variant: align with recommended selector.
Use the documented pattern to target “.dark … &” reliably.
Apply this diff:
-@custom-variant dark (&:is(.dark *));
+@custom-variant dark (&:where(.dark &));Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In dashboard/src/app/globals.css around line 4, the custom-variant definition
uses a nonstandard selector; replace it with the documented Tailwind v4 pattern
that targets the dark parent before the component (use the ".dark &" style
selector) so the variant reliably applies to elements under a .dark container.
| --radius: 0.625rem; | ||
|
|
||
| /* Updated Color Palette based on #031035 and accents */ | ||
| --background: #031035; /* Deep navy primary */ | ||
| --foreground: #f8fafc; /* Light text */ | ||
| --card: #0a1a4a; /* Slightly lighter navy for cards */ | ||
| --card-foreground: #f8fafc; | ||
| --popover: #0a1a4a; | ||
| --popover-foreground: #f8fafc; | ||
| --primary: #87ceeb; /* Sky blue accent */ | ||
| --primary-foreground: #031035; | ||
| --secondary: #1a2a5a; /* Medium navy */ | ||
| --secondary-foreground: #f8fafc; | ||
| --muted: #2a3a6a; /* Muted navy */ | ||
| --muted-foreground: #94a3b8; /* Light gray text */ | ||
| --accent-foreground: #f8fafc; | ||
| --destructive: #ff6b8a; /* Pink accent for errors */ | ||
| --border: #1a2a5a; /* Navy border */ | ||
| --input: #1a2a5a; /* Navy input background */ | ||
| --ring: #87ceeb; /* Sky blue focus ring */ | ||
| --chart-1: #87ceeb; /* Sky blue */ | ||
| --chart-2: #4ade80; /* Green accent */ | ||
| --chart-3: #fbbf24; /* Yellow accent */ | ||
| --chart-4: #ff6b8a; /* Pink accent */ | ||
| --chart-5: #a78bfa; /* Purple accent */ | ||
| --sidebar: #0a1a4a; | ||
| --sidebar-foreground: #f8fafc; | ||
| --sidebar-primary: #87ceeb; | ||
| --sidebar-primary-foreground: #031035; | ||
| --sidebar-accent: #1a2a5a; | ||
| --sidebar-accent-foreground: #f8fafc; | ||
| --sidebar-border: #1a2a5a; | ||
| --sidebar-ring: #87ceeb; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Root tokens: add color-scheme and guard radius derivations.
Improve native form/scrollbar theming and prevent negative radii if tokens change.
:root {
--accent: #031035;
@@
- --radius: 0.625rem;
+ --radius: 0.625rem;
+ color-scheme: dark;In @theme inline (Lines 155–158), prefer non-negative radii:
- --radius-sm: calc(var(--radius) - 4px);
- --radius-md: calc(var(--radius) - 2px);
+ --radius-sm: max(0px, calc(var(--radius) - 4px));
+ --radius-md: max(0px, calc(var(--radius) - 2px));Verify nothing else relies on negative radii.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In dashboard/src/app/globals.css around lines 20 to 53, the root CSS tokens are
missing a color-scheme declaration and do not guard against negative radii;
update :root to include color-scheme (light/dark) matching the palette and
replace the raw --radius with a derived non-negative token (e.g., --radius-safe:
max(0px, var(--radius))) then use --radius-safe wherever radius is referenced;
also add native form/scrollbar theming variables (scrollbar-color/width and form
element backgrounds/borders) to align with the new tokens, and finally audit the
codebase to ensure no component relies on negative radii values.
- Updated global styles to improve select dropdown appearance in dark mode. - Adjusted select and option styles for better visibility and user experience. - Ensured consistent theming across the dashboard interface. These changes enhance the overall usability and aesthetic of the select component in the dashboard.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dashboard/src/app/page.tsx (1)
17-21: Remove unused icon import (or re-enable the Graph tab trigger)
GitBranchis only referenced in commented code; remove to satisfy linters or restore the trigger.-import { - GitBranch, - BarChart3, - AlertCircle, - Filter -} from 'lucide-react'; +import { + BarChart3, + AlertCircle, + Filter +} from 'lucide-react';
♻️ Duplicate comments (2)
dashboard/src/app/page.tsx (2)
154-172: Fix: shadcn/ui Select is misused as a native ; also add an accessible label association Current usage won’t work (wrong props and children). Replace with proper shadcn Select composition and bind a Label to the trigger. - <span className="text-sm text-muted-foreground">Namespace:</span> - <Select - value={namespace} - onChange={(e) => setNamespace(e.target.value)} - className="w-32 h-8" - > - {availableNamespaces.map((ns) => ( - <option key={ns} value={ns}> - {ns} - </option> - ))} - {/* Show current namespace even if not in the list */} - {!availableNamespaces.includes(namespace) && ( - <option key={namespace} value={namespace}> - {namespace} - </option> - )} - </Select> + <Label htmlFor="namespace-select" className="text-sm text-muted-foreground">Namespace:</Label> + <Select value={namespace} onValueChange={(v) => setNamespace(v)}> + <SelectTrigger id="namespace-select" className="w-40 h-8"> + <SelectValue placeholder="Select namespace" /> + </SelectTrigger> + <SelectContent> + {availableNamespaces.map((ns) => ( + <SelectItem key={ns} value={ns}> + {ns} + </SelectItem> + ))} + {!availableNamespaces.includes(namespace) && ( + <SelectItem key={namespace} value={namespace}> + {namespace} + </SelectItem> + )} + </SelectContent> + </Select> If you intended a native element instead, switch to <select> and drop shadcn Select entirely. 198-213: Type alignment: include 'graph' in the Tabs onValueChange union or remove the Graph content Currently the handler narrows to 'overview' | 'runs' while you still render a 'graph' TabsContent. - <Tabs value={activeTab} onValueChange={(value) => setActiveTab(value as 'overview' | 'runs')} className="w-full"> + <Tabs value={activeTab} onValueChange={(value) => setActiveTab(value as 'overview' | 'graph' | 'runs')} className="w-full"> 📜 Review details Configuration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📥 Commits Reviewing files that changed from the base of the PR and between 3301313 and ab0dcfa. 📒 Files selected for processing (1) dashboard/src/app/page.tsx (5 hunks) 🧰 Additional context used 🧬 Code graph analysis (1) dashboard/src/app/page.tsx (4) dashboard/src/services/clientApi.ts (1) clientApiService (75-75) dashboard/src/components/NamespaceOverview.tsx (1) NamespaceOverview (33-229) dashboard/src/components/GraphTemplateBuilder.tsx (1) GraphTemplateBuilder (13-290) dashboard/src/components/RunsTable.tsx (1) RunsTable (43-414)
- Integrated Next.js Image component for optimized image loading in the Dashboard. - Refactored state management in Dashboard to simplify graph name handling. - Improved error handling in namespace fetching logic for better debugging. - Updated GraphTemplateBuilder to enhance node management UI and improve user experience. - Streamlined input handling and added placeholder text for clarity in node configuration. These changes collectively enhance the performance and usability of the dashboard interface.
- Updated the padding in the select component to enhance the overall layout and user experience. - This change ensures better alignment and spacing within the dropdown, contributing to a more polished interface.
|
/gemini review |
NiveditJain
changed the title
![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 significant and impressive visual overhaul of the dashboard, migrating to a dark theme using shadcn/ui and Tailwind CSS. The introduction of new UI components, API endpoints for fetching namespaces and node run details, and the integration of reactflow for graph visualization greatly enhance the user experience and functionality. The backend changes are well-supported with new controllers, models, and tests. I've identified a couple of minor areas for improvement in the frontend code related to state management and potentially unreachable UI code, which are detailed in the comments. Overall, this is a fantastic update that modernizes the dashboard's look and feel.
| graphTemplate.name = graphName; | ||
| graphTemplate.namespace = namespace; |
There was a problem hiding this comment.
The graphTemplate object returned from the API call is being mutated directly before being set into state. This is an anti-pattern in React that can lead to unpredictable behavior. Given the backend model, the name and namespace properties should already be present in the response, making these mutations redundant.
| <Tabs value={activeTab} onValueChange={(value) => setActiveTab(value as 'overview' | 'runs')} className="w-full"> | ||
| <TabsList className="grid w-full grid-cols-2 mb-8"> | ||
| <TabsTrigger value="overview" className="flex items-center space-x-2"> | ||
| <BarChart3 className="w-4 h-4" /> | ||
| <span>Overview</span> | ||
| </TabsTrigger> | ||
| {/* <TabsTrigger value="graph" className="flex items-center space-x-2"> | ||
| <GitBranch className="w-4 h-4" /> | ||
| <span>Graph Template</span> | ||
| </TabsTrigger> */} | ||
| <TabsTrigger value="runs" className="flex items-center space-x-2"> | ||
| <Filter className="w-4 h-4" /> | ||
| <span>Runs</span> | ||
| </TabsTrigger> | ||
| </TabsList> |
There was a problem hiding this comment.
The TabsTrigger for the 'graph' tab is commented out, but its corresponding TabsContent is still present, making the 'Graph Template Builder' UI unreachable. This appears to be either dead code or an incomplete feature.
If the tab is intended to be displayed, the trigger should be uncommented, and the TabsList class updated to grid-cols-3. The onValueChange handler should also be updated to include 'graph'. If not, the TabsContent for value="graph" should be removed.
| { status: 500 } | ||
| ); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
The file is missing a newline at the end. While this doesn't affect functionality, it's a common convention to have one.
| } | |
| } | |
| { status: 500 } | ||
| ); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
The file is missing a newline at the end. It's a good practice to end files with a newline character.
| } | |
| } | |
| raise HTTPException( | ||
| status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, | ||
| detail="Internal server error while retrieving node details" | ||
| ) No newline at end of file |
There was a problem hiding this comment.
The file is missing a newline at the end. It's a standard convention in Python to have a blank line at the end of a file.
| ) | |
| ) | |
|
|
||
| except Exception as e: | ||
| logger.error(f"Error listing namespaces: {str(e)}", x_exosphere_request_id=request_id) | ||
| raise No newline at end of file |
There was a problem hiding this comment.
The file is missing a newline at the end. It's a good practice to end files with a newline character.
| raise | |
| raise | |
| app.include_router(global_router) | ||
| app.include_router(router) No newline at end of file |
There was a problem hiding this comment.
The order of including routers is important. While it might not cause issues here because the prefixes are distinct, it's generally safer to include more specific routers before more general ones. In this case, router with prefix /v0/namespace/{namespace_name} is more specific than global_router with prefix /v0. Placing the more specific one first ensures its routes are matched before the more general ones in case of any future overlaps.
| app.include_router(global_router) | |
| app.include_router(router) | |
| app.include_router(router) | |
| app.include_router(global_router) |
- Enhanced the error handling logic in the node run details API to propagate upstream errors more effectively. - Added support for returning the original response body and content type in case of an error, improving debugging and user feedback. - Updated the fetch URL to properly encode parameters, ensuring correct API requests. These changes collectively enhance the robustness and reliability of the API response handling.
2 participants
No description provided.