Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a Changes
Sequence DiagramsequenceDiagram
participant User
participant Dashboard as LookaheadDashboard
participant Hooks as Lookahead Hooks
participant API as Server
participant Empty as EmptyForecastState
participant History as SnapshotHistoryPanel
User->>Dashboard: mount / interact
Dashboard->>Hooks: fetch snapshot & diagnostics
Hooks->>API: GET /snapshot, GET /diagnostics
API-->>Hooks: response (data or error)
Hooks-->>Dashboard: data or snapshotError / alertsError
alt snapshotError blocking
Dashboard->>Empty: render reason="load-error" with title/message and Retry CTA
User->>Empty: clicks Retry
Empty->>Dashboard: onAction -> refreshLookaheadWorkspace()
Dashboard->>Hooks: retry fetch / mutate
else snapshot ok, alertsError present
Dashboard->>Dashboard: append synthetic diagnostics alert with Retry CTA -> mutateAlerts()
end
Dashboard->>History: pass currentSnapshot
History-->>User: render history rows with prioritized row_count badge
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/lookahead/LookaheadDashboard.tsx`:
- Around line 700-711: The workspace blocking logic should check for a blocking
snapshot error (snapshotError && !heatmap) instead of snapshotError alone;
create a derived boolean (e.g., hasBlockingSnapshotError = Boolean(snapshotError
&& !heatmap)) and use it in the diagnostics suppression branches that push the
diagnostics_unavailable alert (where alertsError and diagnosticsLoadMessage are
checked), the hero copy logic, and the load-error empty state; update the
condition that currently reads if (alertsError && !snapshotError &&
diagnosticsLoadMessage) to if (alertsError && !hasBlockingSnapshotError &&
diagnosticsLoadMessage) and replace other usages of snapshotError in the same
contexts (see uses around nextAlerts push, the hero copy branch, and the
load-error state) to ensure cached snapshot heatmap data is not incorrectly
blocked, keeping mutateAlerts and diagnosticsLoadMessage behavior unchanged.
In `@src/hooks/lookahead/api.ts`:
- Around line 224-230: The rowCount calculation currently uses rows.length first
and the || operator which causes authoritative numeric counts (including 0) in
entry.row_count (or other count fields) to be ignored or collapsed to null;
update the logic in the rowCount assignment to prefer numeric counts returned by
asNumber(entry.row_count), asNumber(entry.rows_count),
asNumber(entry.snapshot_row_count), asNumber(entry.count) using nullish
coalescing (or explicit undefined checks) so 0 is preserved, and only fall back
to rows?.length (via rows.length or asNumber on rows.length) as the last option,
referencing the rowCount variable and the asNumber helper to locate and change
the expression.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58e0ad71-4fa1-4206-81cf-7ea6528659c6
📒 Files selected for processing (6)
src/components/lookahead/EmptyForecastState.tsxsrc/components/lookahead/LookaheadDashboard.tsxsrc/components/lookahead/SnapshotHistoryPanel.tsxsrc/hooks/lookahead/api.test.tssrc/hooks/lookahead/api.tssrc/types/index.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/lookahead/LookaheadDashboard.tsx (1)
154-207: Well-structured error message helpers with HTTP-aware logic.The functions correctly handle various error scenarios (5xx, 404, offset-naive timezone issues). A few observations:
- Line 201: The hardcoded
"Something went wrong"string comparison could be brittle ifgetApiErrorMessagechanges its default fallback.- Line 188: The conditional
rawMessage === fallback ? fallback : ...appears redundant since both branches produce similar output.♻️ Consider extracting the magic string
+const GENERIC_ERROR_FALLBACK = "Something went wrong"; + function getDiagnosticsLoadMessage(error: unknown): string { const fallback = "Planning diagnostics could not be refreshed right now. You can still use the heatmap, but alert badges may be stale until the next retry."; const rawMessage = getApiErrorMessage(error, fallback).trim(); if ( /offset-naive|offset-aware/i.test(rawMessage) || (isAxiosError(error) && (error.response?.status ?? 0) >= 500) || /status code 5\d\d/i.test(rawMessage) || - rawMessage === "Something went wrong" + rawMessage === GENERIC_ERROR_FALLBACK ) { return fallback; } return rawMessage; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/lookahead/LookaheadDashboard.tsx` around lines 154 - 207, Both helpers should avoid the brittle hardcoded "Something went wrong" and simplify the fallback logic: extract a shared constant (e.g., DEFAULT_API_FALLBACK or DEFAULT_ERROR_MSG) and use that constant as the fallback argument to getApiErrorMessage and for comparisons in getDiagnosticsLoadMessage (replace rawMessage === "Something went wrong" with rawMessage === DEFAULT_ERROR_MSG or startsWith check). In getForecastLoadMessage simplify the final return by consistently building the message from the fallback and rawMessage but prevent duplication by checking rawMessage === fallback (or rawMessage.startsWith(fallback)) before concatenating; refer to the functions getForecastLoadMessage and getDiagnosticsLoadMessage and the local variables fallback and rawMessage to locate the changes.src/components/lookahead/UploadReviewDialog.tsx (1)
427-451: Consider extracting duplicated pagination controlsTop and bottom pagination blocks duplicate behavior/UI. Extracting a small
PaginationControlssubcomponent would reduce drift risk when logic changes.Also applies to: 690-723
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/lookahead/UploadReviewDialog.tsx` around lines 427 - 451, Extract the duplicated pagination UI into a new presentational component (e.g., PaginationControls) that accepts props {page, totalPages, setPage} and renders the two Buttons and the page/totalPages span using the existing handlers (onClick setPage((p)=>Math.max(1,p-1)) and setPage((p)=>Math.min(totalPages,p+1))) and icons (ChevronLeft, ChevronRight); replace both occurrences in UploadReviewDialog with this new component to centralize behavior and prevent drift while preserving the existing disabled logic (disabled={page <= 1} and disabled={page >= totalPages}).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/lookahead/UploadReviewDialog.tsx`:
- Around line 185-187: busyId/memoryBusyId currently store a single inflight id
which lets overlapping actions overwrite each other and re-enable rows
prematurely; change these to per-row inflight tracking (e.g.,
useState<Record<string, boolean>> or a Ref holding a Set) and update usages in
the UploadReviewDialog handlers (replace setBusyId/setMemoryBusyId calls with
keyed updates and check the per-row map/set before allowing an action), and
update actionErrors to be maintained per-row consistently so each row's
busy/error state is isolated (apply the same pattern where busyId/memoryBusyId
are read/checked and cleared).
- Around line 428-450: The icon-only pagination Buttons around ChevronLeft and
ChevronRight lack accessible names; add descriptive accessible labels (e.g.,
aria-label="Previous page" and aria-label="Next page" or aria-labelledby
pointing to a visually-hidden label) on the Button components that invoke
setPage so screen readers announce their purpose; update the two Button elements
that call setPage((p) => Math.max(1, p - 1)) and setPage((p) =>
Math.min(totalPages, p + 1)) (the ones rendering ChevronLeft and ChevronRight)
to include those aria attributes and ensure any visually-hidden text IDs
referenced are unique and present.
---
Nitpick comments:
In `@src/components/lookahead/LookaheadDashboard.tsx`:
- Around line 154-207: Both helpers should avoid the brittle hardcoded
"Something went wrong" and simplify the fallback logic: extract a shared
constant (e.g., DEFAULT_API_FALLBACK or DEFAULT_ERROR_MSG) and use that constant
as the fallback argument to getApiErrorMessage and for comparisons in
getDiagnosticsLoadMessage (replace rawMessage === "Something went wrong" with
rawMessage === DEFAULT_ERROR_MSG or startsWith check). In getForecastLoadMessage
simplify the final return by consistently building the message from the fallback
and rawMessage but prevent duplication by checking rawMessage === fallback (or
rawMessage.startsWith(fallback)) before concatenating; refer to the functions
getForecastLoadMessage and getDiagnosticsLoadMessage and the local variables
fallback and rawMessage to locate the changes.
In `@src/components/lookahead/UploadReviewDialog.tsx`:
- Around line 427-451: Extract the duplicated pagination UI into a new
presentational component (e.g., PaginationControls) that accepts props {page,
totalPages, setPage} and renders the two Buttons and the page/totalPages span
using the existing handlers (onClick setPage((p)=>Math.max(1,p-1)) and
setPage((p)=>Math.min(totalPages,p+1))) and icons (ChevronLeft, ChevronRight);
replace both occurrences in UploadReviewDialog with this new component to
centralize behavior and prevent drift while preserving the existing disabled
logic (disabled={page <= 1} and disabled={page >= totalPages}).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35ebbfa0-95c5-4dbe-9f5b-dd57eefabf00
📒 Files selected for processing (3)
src/components/lookahead/LookaheadDashboard.tsxsrc/components/lookahead/UploadReviewDialog.tsxsrc/hooks/lookahead/api.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/lookahead/api.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/lookahead/UploadReviewDialog.tsx`:
- Around line 450-455: The Input components (e.g., the search box using
value={searchTerm} onChange={setSearchTerm} and the custom asset-type Input
fields referenced elsewhere) currently only use placeholders and need
programmatic labels for accessibility; add either an explicit aria-label prop or
associate an HTML label with an id/htmlFor for each Input (give the search Input
a descriptive label like "Search activities, locations, or types" and similarly
label each custom asset-type field) so assistive technologies can identify them.
- Around line 285-301: The page value can be stale during render and causes
empty/incorrect UI; clamp the derived page before using it for slicing and range
calculations by computing a local clampedPage = Math.max(1, Math.min(page,
totalPages)) (use the existing totalPages logic) and replace uses of page inside
the visibleRows useMemo (start calculation and slice), and the
rangeStart/rangeEnd calculations with clampedPage; leave the existing effect
that updates state but ensure all render-time computations reference clampedPage
(also apply same change to the other occurrences around
visibleRows/rangeStart/rangeEnd).
- Around line 264-279: The search filter in filteredRows currently compares raw
snake_case values (e.g., asset_type) against the query, so queries like "scissor
lift" won't match "scissor_lift"; update the filter inside the useMemo to
normalize asset_type to the same displayed label before matching (e.g., replace
underscores with spaces and toLowerCase(), or map asset_type to its display
string) and use that normalized string in the array passed to .some; keep the
rest of the fields (activity_name, source_value, etc.) unchanged and ensure the
normalization is applied to suggested_classification/current_classification if
they also render with spaces.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5cd99d49-957e-46a8-943a-6e591840dd40
📒 Files selected for processing (2)
src/components/lookahead/LookaheadDashboard.tsxsrc/components/lookahead/UploadReviewDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/lookahead/LookaheadDashboard.tsx
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary by CodeRabbit
New Features
Bug Fixes
Tests