Add analytics dashboard with SSR, Client, and RSC versions#23
Merged
AbanoubGhadban merged 6 commits intomainfrom Feb 28, 2026
Merged
Add analytics dashboard with SSR, Client, and RSC versions#23AbanoubGhadban merged 6 commits intomainfrom
AbanoubGhadban merged 6 commits intomainfrom
Conversation
…omparison Implements a realistic e-commerce product page demo (#17) with three rendering strategies. Features product gallery, reviews (50K per product), rating distribution charts, markdown descriptions with syntax highlighting, related products, and add-to-cart interactions. RSC version uses Suspense streaming to progressively load heavy sections while keeping ~400KB of libraries (marked, highlight.js, date-fns) server-side only. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Add fetchpriority="high" and decoding="async" to main product image - Add loading="lazy" to thumbnail images to reduce bandwidth contention - Move below-the-fold content (description, features, specs) behind Suspense boundary to shrink initial RSC stream and prioritize LCP - Add product page entries to benchmark scripts (constants, collectors, runner) with per-page selector overrides Co-Authored-By: Claude Opus 4.6 <[email protected]>
…mance comparison Build a full analytics dashboard page with 6 data sections (KPI stats, revenue chart, order status, hourly distribution, recent orders, top menu items) using d3 for SVG chart rendering and date-fns for date formatting. Three rendering versions demonstrate RSC streaming advantages: - SSR: All 6 queries run sequentially, entire page blocks until complete - Client: Shell renders instantly, data fetched via 6 parallel API calls - RSC: Shell streams immediately, each section streams as its query completes Key RSC optimization: All chart components (d3-scale, d3-shape, d3-array, d3-time-format, d3-format, date-fns) are pure server components that render SVG server-side. Zero charting JS shipped to the browser. Benchmark results (median of 5 runs, 10M orders dataset): - RSC vs SSR FCP/LCP: 95.4% faster (10.37s -> 472ms) - RSC vs SSR TTMC: 45.5% faster (10.34s -> 5.63s) - RSC vs Client TTMC (1st data): 39.8% faster (5.53s -> 3.33s) - RSC JS size: 237KB vs Client 316KB (25% less) vs SSR 302KB - RSC scripts: 5 vs Client 12 (58% fewer) vs SSR 6 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add three 'use client' interactive components: - DashboardFilters: time range toggle + status filter pills - SortableOrdersTable: click-to-sort columns with direction indicators - InteractiveTopItems: category filter buttons with CSS bar charts Wire interactive components into all three page versions (SSR, Client, RSC). RSC renders charts server-side and only ships ~10KB interactive islands. Update benchmark with accurate TTI (hydration timing via data-hydrated attribute MutationObserver) and simulated INP (click-to-sort/filter). Results: RSC TTI 591ms (92.4% faster than SSR, 84.1% faster than Client) because small client islands hydrate immediately while charts stream server-side via Suspense boundaries. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
Warning Rate limit exceeded
⌛ 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 (1)
WalkthroughThis PR implements comprehensive dashboard and product page systems with three rendering strategies (SSR, Client-side, RSC streaming), including API controllers, React components, backend models, database migrations, type definitions, and configuration for both features. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant SSRServer as SSR Server
participant ClientServer as Client Rendering
participant APIServer as API / DB
participant RSCServer as RSC Server
Note over Browser,RSCServer: Dashboard SSR Flow (V1)
Browser->>SSRServer: GET /dashboard/ssr
SSRServer->>APIServer: Query KPI stats, revenue, orders, etc.
SSRServer->>APIServer: Fetch dashboard data (7 datasets)
APIServer-->>SSRServer: All data returned
SSRServer->>ClientServer: Pre-render DashboardPageSSR with data
ClientServer-->>Browser: HTML + JS (hydrated with data)
Browser->>Browser: Hydrate & interactive filters
Note over Browser,RSCServer: Dashboard Client Fetch Flow (V2)
Browser->>SSRServer: GET /dashboard/client
SSRServer->>ClientServer: Pre-render shell + header only
ClientServer-->>Browser: HTML + JS (minimal initial data)
Browser->>Browser: Mount AsyncDashboardContent
Browser->>APIServer: fetch /api/dashboard/kpi_stats, etc. (parallel)
APIServer-->>Browser: Stream data responses
Browser->>Browser: Render charts & tables as data arrives
Note over Browser,RSCServer: Dashboard RSC Streaming Flow (V3)
Browser->>RSCServer: GET /dashboard/rsc
RSCServer->>RSCServer: Render header immediately
RSCServer-->>Browser: Stream header HTML + shell
loop Sequential Streaming
RSCServer->>APIServer: Query KPI stats
APIServer-->>RSCServer: Return data
RSCServer-->>Browser: Stream KPI section (via emit)
RSCServer->>APIServer: Query revenue, orders, etc.
APIServer-->>RSCServer: Return data
RSCServer-->>Browser: Stream section (via emit)
end
Browser->>Browser: Progressive hydration as sections arrive
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Co-Authored-By: Claude Opus 4.6 <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (12)
package.json-45-45 (1)
45-45:⚠️ Potential issue | 🟡 MinorUpgrade to stable version or document RC requirement.
react-on-rails-rscis pinned to19.0.5-rc.1, but the latest stable release is19.0.4. For production use, upgrade to the stable version or document why the RC is required with a tracked follow-up to migrate off it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 45, The package.json currently pins react-on-rails-rsc to the pre-release "19.0.5-rc.1"; either update the dependency entry for react-on-rails-rsc to the latest stable "19.0.4" (and run npm/yarn install and update lockfile) or, if the RC is required, add a short rationale in the repo (e.g., in README or a DEPENDENCIES.md) and create/link a tracked issue to migrate off "19.0.5-rc.1" to a stable release—refer to the react-on-rails-rsc dependency entry when making the change.app/javascript/startup/DashboardPageClient.server.tsx-1-1 (1)
1-1:⚠️ Potential issue | 🟡 Minor
'use client'directive appears incorrect for a server-side rendering file.This file performs server-side rendering using
renderToStringandChunkExtractor. The'use client'directive is meant to mark client component boundaries in RSC, not server rendering entry points. This directive may cause unexpected behavior or bundling issues.🔧 Proposed fix
-'use client'; - import path from 'path';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/startup/DashboardPageClient.server.tsx` at line 1, Remove the top-level "'use client'" directive from this server-side rendering module so it is not treated as a client component; the file uses server-side APIs like renderToString and ChunkExtractor, so delete the "'use client'" line and keep the existing server-rendering code (renderToString, ChunkExtractor usage) intact so bundlers/RSC treat it as a server module.app/javascript/components/product/AsyncReviewStatsRSC.tsx-12-19 (1)
12-19:⚠️ Potential issue | 🟡 MinorAdd a defensive guard for
review_statspayload shape.Line 12 assumes the async payload is always complete; if the stream key is missing/malformed, passing undefined fields can break rendering downstream.
Defensive fallback example
export default async function AsyncReviewStatsRSC({ getReactOnRailsAsyncProp }: Props) { const data = await getReactOnRailsAsyncProp('review_stats'); + if (!data || !Array.isArray(data.distribution)) return null; return ( <ReviewDistributionChart distribution={data.distribution} - averageRating={data.average_rating} - totalReviews={data.total_reviews} + averageRating={Number(data.average_rating ?? 0)} + totalReviews={Number(data.total_reviews ?? 0)} /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/AsyncReviewStatsRSC.tsx` around lines 12 - 19, Add a defensive guard in AsyncReviewStatsRSC around the payload returned by getReactOnRailsAsyncProp('review_stats') so missing or malformed keys don't crash ReviewDistributionChart: validate that data exists and has the expected properties (distribution, average_rating, total_reviews), provide sensible defaults (empty array for distribution, 0 for numeric fields) or return null/placeholder if the shape is invalid, and pass the sanitized values into ReviewDistributionChart; update references to data.distribution, data.average_rating, and data.total_reviews accordingly (use optional chaining/null coalescing or an early return) so downstream rendering is safe.app/javascript/components/product/ProductPageRSC.tsx-40-42 (1)
40-42:⚠️ Potential issue | 🟡 MinorFix the “0KB to client” claim in the version banner.
Line 41 currently says “0KB to client,” but this page still ships client islands (
ProductImageGallery,AddToCartSection,INPOverlay). Please adjust the message to avoid misleading users.Suggested text update
- V3: RSC Streaming — marked + highlight.js + date-fns stay server-side, 0KB to client. Data streams progressively. + V3: RSC Streaming — heavy libs (marked + highlight.js + date-fns) stay server-side; only small interactive islands ship to the client. Data streams progressively.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/ProductPageRSC.tsx` around lines 40 - 42, Update the banner copy in ProductPageRSC.tsx to remove the inaccurate “0KB to client” claim and reflect that some client islands are still shipped; edit the <p> element text (in the version banner) to say something like “minimal client JS for client islands” or “streams server-side, with client islands (ProductImageGallery, AddToCartSection, INPOverlay) shipped as needed” so it no longer claims 0KB; locate the banner paragraph in ProductPageRSC.tsx and replace its inner text accordingly.app/javascript/components/dashboard/DashboardFilters.tsx-44-55 (1)
44-55:⚠️ Potential issue | 🟡 MinorAdd
type="button"on all non-submit buttons.These buttons can accidentally submit a parent form because no explicit type is set.
🛠️ Proposed fix
<button + type="button" key={value} onClick={() => handleRangeClick(value)} @@ <button + type="button" onClick={() => handleStatusClick(null)} @@ <button + type="button" key={status} onClick={() => handleStatusClick(status)}Also applies to: 62-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/dashboard/DashboardFilters.tsx` around lines 44 - 55, Buttons in DashboardFilters.tsx (the range buttons rendered using the handleRangeClick handler and relying on activeRange) are missing an explicit type and may unintentionally submit a parent form; update each non-submit <button> (including the time-range buttons rendered with key={value} and any other buttons in the same component between the indicated ranges) to include type="button" so they do not trigger form submission.app/javascript/components/product/AddToCartSection.tsx-34-55 (1)
34-55:⚠️ Potential issue | 🟡 MinorSet explicit button types for all action controls.
These buttons currently rely on default submit behavior.
🛠️ Proposed fix
<button + type="button" onClick={handleDecrement} @@ <button + type="button" onClick={handleIncrement} @@ <button + type="button" onClick={handleAddToCart} @@ <button + type="button" disabled={!inStock}Also applies to: 60-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/AddToCartSection.tsx` around lines 34 - 55, The increment/decrement controls in AddToCartSection.tsx (buttons wired to handleDecrement and handleIncrement) currently omit explicit types and may act as submit buttons; update each interactive button (the decrement/increment buttons and any other action buttons around lines noted) to include type="button" so they do not trigger form submission, keeping their onClick handlers unchanged and preserving existing classes/ARIA attributes.app/javascript/components/product/ReviewDistributionChart.tsx-34-37 (1)
34-37:⚠️ Potential issue | 🟡 MinorClamp percentage before computing bar width.
Width is only lower-bounded now; out-of-range percentages can overflow or render invalid bars.
🛠️ Proposed fix
function RatingBar({ stars, percentage, count }: RatingDistribution) { + const normalized = Math.min(100, Math.max(0, percentage)); + const widthPct = normalized === 0 ? 0 : Math.max(normalized, 1); const barColor = @@ <div className={`h-full rounded-full ${barColor} transition-all duration-500`} - style={{ width: `${Math.max(percentage, 1)}%` }} + style={{ width: `${widthPct}%` }} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/ReviewDistributionChart.tsx` around lines 34 - 37, The bar width uses Math.max(percentage, 1) which only lower-bounds the value and can still allow >100 or negative values; compute a clamped percentage first (e.g., const clamped = Math.min(Math.max(percentage, 0), 100)) inside the ReviewDistributionChart component and use `${Math.max(clamped, 1)}%` (or just `${clamped}%` if you want 0% allowed) in the style for the div so the width is always between 0 and 100 percent.app/javascript/components/product/AddToCartSection.tsx-23-26 (1)
23-26:⚠️ Potential issue | 🟡 MinorTrack and clear the add-to-cart reset timer.
Repeated clicks/unmount can leave stale timers that flip state unexpectedly.
🛠️ Proposed fix
-import React, { useState, useCallback } from 'react'; +import React, { useState, useCallback, useEffect, useRef } from 'react'; @@ export function AddToCartSection({ price, inStock, stockQuantity }: Props) { const [quantity, setQuantity] = useState(1); const [addedToCart, setAddedToCart] = useState(false); + const resetTimerRef = useRef<number | null>(null); + + useEffect(() => { + return () => { + if (resetTimerRef.current !== null) window.clearTimeout(resetTimerRef.current); + }; + }, []); @@ const handleAddToCart = useCallback(() => { + if (resetTimerRef.current !== null) window.clearTimeout(resetTimerRef.current); setAddedToCart(true); - setTimeout(() => setAddedToCart(false), 2000); + resetTimerRef.current = window.setTimeout(() => setAddedToCart(false), 2000); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/AddToCartSection.tsx` around lines 23 - 26, In AddToCartSection, handleAddToCart currently starts a timeout that sets added state back but never tracks or clears the timer; modify handleAddToCart to store the timeout id (e.g., in a ref like resetTimerRef) and clear any existing timer before creating a new one, and add a useEffect cleanup that clears resetTimerRef on unmount so setAddedToCart(false) cannot run after the component is unmounted or after a subsequent click.app/javascript/components/dashboard/InteractiveTopItems.tsx-23-26 (1)
23-26:⚠️ Potential issue | 🟡 MinorHydration marker may target the wrong component instance.
document.querySelectorpicks the first matching node globally. Use a local ref so each instance marks itself correctly.🔧 Proposed fix
-import React, { useState, useMemo, useEffect, useCallback } from 'react'; +import React, { useState, useMemo, useEffect, useCallback, useRef } from 'react'; @@ export default function InteractiveTopItems({ items }: InteractiveTopItemsProps) { const [activeCategory, setActiveCategory] = useState<string | null>(null); + const rootRef = useRef<HTMLDivElement | null>(null); @@ useEffect(() => { - const el = document.querySelector('[data-interactive-items]'); + const el = rootRef.current; if (el) el.setAttribute('data-hydrated', 'true'); }, []); @@ - <div data-interactive-items className="bg-white rounded-xl shadow-sm border border-gray-100 p-6"> + <div ref={rootRef} data-interactive-items className="bg-white rounded-xl shadow-sm border border-gray-100 p-6">Also applies to: 49-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/dashboard/InteractiveTopItems.tsx` around lines 23 - 26, The hydration marker currently uses document.querySelector which will mark the first global match; update the InteractiveTopItems component to use a local ref (e.g., rootRef) on the root DOM element and replace document.querySelector calls in the useEffect(s) with checks against rootRef.current, calling rootRef.current.setAttribute('data-hydrated','true') so each instance marks itself; apply the same ref-based change for the second useEffect referenced around line 49.app/javascript/components/product/ProductInfo.tsx-51-57 (1)
51-57:⚠️ Potential issue | 🟡 MinorBreadcrumb items appear clickable but are not semantically interactive.
Using
spanwithcursor-pointerhere hurts keyboard/accessibility behavior. Use real links/buttons (or remove pointer styling if static text).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/ProductInfo.tsx` around lines 51 - 57, The breadcrumb markup in ProductInfo (the <nav> containing the "Home" and product.category spans) uses non-interactive <span> elements with cursor-pointer which breaks keyboard/accessibility; replace those spans with proper interactive elements (e.g., <a> for links or <button> if actions) and remove pointer styling from static text, ensure focus styles are preserved, add accessible attributes like aria-current="page" on the current item, and keep the SVG separators intact so keyboard and screen-reader users can navigate the breadcrumb correctly.app/models/concerns/dashboard_analytics.rb-24-34 (1)
24-34:⚠️ Potential issue | 🟡 MinorAvoid overlapping comparison windows in KPI deltas.
Line 24 and Line 33 both use inclusive ranges (
..) withcutoff, so boundary records are counted in both periods and can skew% change.🔧 Proposed fix
- prev_stats = Order.where(placed_at: prev_cutoff..cutoff) + prev_stats = Order.where(placed_at: prev_cutoff...cutoff)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/models/concerns/dashboard_analytics.rb` around lines 24 - 34, The KPI delta calculation double-counts records at the cutoff because both stats and prev_stats use inclusive ranges; update the previous period query (prev_stats) to exclude the cutoff boundary by changing the range from prev_cutoff..cutoff to an exclusive-end range prev_cutoff...cutoff (or equivalent like prev_cutoff...cutoff) in the Order.where call so records at cutoff are only counted in stats and not in prev_stats.app/models/concerns/dashboard_analytics.rb-59-70 (1)
59-70:⚠️ Potential issue | 🟡 MinorNormalize day boundaries for stable 14-day chart buckets.
Line 60 uses a timestamp cutoff and then groups by date. That can create partial first-day buckets and inconsistent day counts.
🔧 Proposed fix
- cutoff = window_end - 14.days + window_end = window_end.end_of_day + cutoff = (window_end - 13.days).beginning_of_day Order.where(placed_at: cutoff..window_end)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/models/concerns/dashboard_analytics.rb` around lines 59 - 70, The current cutoff uses a timestamp that can create partial first-day buckets; change the range to explicit whole-day boundaries by deriving a start_date and end_date from dashboard_data_window (e.g., set start_date = window_start.beginning_of_day and end_date = window_end.end_of_day or compute start_date as (window_end.to_date - 13).beginning_of_day to guarantee 14 full days) and then use Order.where(placed_at: start_date..end_date) (keeping the existing date_trunc grouping and pluck). Update the variable names (replace cutoff and window_end usage) in the method in dashboard_analytics.rb so the query always covers exactly 14 full calendar days.
🧹 Nitpick comments (21)
scripts/lib/runner.mjs (1)
99-100: Include selector/path in the “button not found” log.This makes failed metric collection triage much faster in verbose runs.
Suggested tweak
- } else if (verbose) { - console.log(' Like button not found'); + } else if (verbose) { + console.log(` Like button not found (selector: "${btnSelector}", page: "${pageConfig.path}")`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lib/runner.mjs` around lines 99 - 100, The verbose branch currently logs "Like button not found" without context; update the else-if handling in scripts/lib/runner.mjs (the verbose branch where it currently calls console.log(' Like button not found')) to include the selector/path variable used to query the button (e.g., the selector or path identifier in scope where the lookup occurs) so the message reads like "Like button not found: <selector>" (include any page or URL variable if available) to speed triage.scripts/lib/constants.mjs (1)
12-14: Use a stable, page-specific button selector for benchmark actions.
selectors.likeButton: 'button'is very broad and can target the wrong element as the page evolves, which can skew INP/hydration metrics. Prefer a deterministic selector (e.g., dedicateddata-testid/class tied to the intended interaction).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lib/constants.mjs` around lines 12 - 14, The likeButton selector is too generic ('button') in the product-ssr, product-client, and product-rsc entries; replace it with a stable, page-specific selector (e.g., a dedicated data-testid or specific class) so benchmark actions always target the intended control. Update the selectors.likeButton value for the 'product-ssr', 'product-client', and 'product-rsc' objects to use the new deterministic selector (and ensure the corresponding button in the page markup has that data-testid/class).scripts/lib/collectors.mjs (1)
20-23: Escape override values before embedding them into injected script source.Raw string interpolation for selectors/text is brittle and can break collector injection when values contain quotes or escape characters.
Safer literal embedding
export function getCollectorScript(overrides = {}) { const likeSelector = overrides.likeButton || SELECTORS.likeButton; const headingSelector = SELECTORS.relatedPostsHeading; const headingText = overrides.relatedHeadingText || SELECTORS.relatedPostsText; + const likeSelectorLiteral = JSON.stringify(likeSelector); + const headingSelectorLiteral = JSON.stringify(headingSelector); + const headingTextLiteral = JSON.stringify(headingText); return ` + const LIKE_SELECTOR = ${likeSelectorLiteral}; + const HEADING_SELECTOR = ${headingSelectorLiteral}; + const HEADING_TEXT = ${headingTextLiteral}; + // Inject web-vitals IIFE — creates window.webVitals ${webVitalsSource} @@ - const headings = document.querySelectorAll('${headingSelector}'); + const headings = document.querySelectorAll(HEADING_SELECTOR); @@ - if (h.textContent && h.textContent.includes('${headingText}')) { + if (h.textContent && h.textContent.includes(HEADING_TEXT)) { @@ - const headings = document.querySelectorAll('${headingSelector}'); + const headings = document.querySelectorAll(HEADING_SELECTOR); @@ - if (h.textContent && h.textContent.includes('${headingText}')) { + if (h.textContent && h.textContent.includes(HEADING_TEXT)) { @@ - const btn = document.querySelector('${likeSelector}'); + const btn = document.querySelector(LIKE_SELECTOR);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lib/collectors.mjs` around lines 20 - 23, The getCollectorScript function is currently interpolating override values (likeSelector, headingSelector, headingText) directly into the injected script source which breaks when those values contain quotes or escapes; update getCollectorScript to sanitize/escape these overrides before embedding (e.g., serialize each override with a robust encoder such as JSON.stringify or a utility escape function) and fall back to SELECTORS.* when overrides are undefined so the injected script receives safe, quoted string literals; ensure you update the code paths that set likeSelector, headingSelector and headingText inside getCollectorScript so the final injected string is constructed from the escaped/serialized values rather than raw interpolation.benchmark-dashboard.js (2)
317-322: Consider importingmedianfrom the shared utility library.This
medianfunction duplicates the implementation inscripts/lib/stats.mjs. Consolidating to a single source would reduce maintenance burden.♻️ Suggested refactor
-function median(arr) { - if (arr.length === 0) return 0; - const sorted = [...arr].sort((a, b) => a - b); - const mid = Math.floor(sorted.length / 2); - return sorted.length % 2 !== 0 ? sorted[mid] : (sorted[mid - 1] + sorted[mid]) / 2; -}At the top of the file, add:
const { median } = require('./scripts/lib/stats.mjs');Note: If this script needs to run standalone without module resolution, the duplication may be acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmark-dashboard.js` around lines 317 - 322, The local median function duplicates the one in scripts/lib/stats.mjs; remove the local median implementation and import the shared median to avoid duplication: replace uses of the local median function with the imported median and add a require/import for median from './scripts/lib/stats.mjs' at the top of the file so all calls (references to median in benchmark-dashboard.js) use the shared implementation.
141-151: TTI measurement depends on components settingdata-hydrated="true".The TTI metric relies on interactive components setting
data-hydrated="true"after hydration. If this attribute is missing, TTI defaults to 0, which may be misleading in benchmark results. Consider adding a comment documenting this requirement for component authors, or providing a fallback heuristic (e.g., DCL + long task completion).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmark-dashboard.js` around lines 141 - 151, The TTI logic in the setInterval loop (see interval, checkAll, and window.__ttiTimestamp) assumes components will set data-hydrated="true", which can leave TTI at 0 if missing; update the code by (1) adding a clear inline comment documenting that components must set data-hydrated="true" after hydration so authors know the requirement, and (2) implement a fallback: if no hydrated element is found by a reasonable timeout (e.g., 3–5s or the existing 30s threshold), compute __ttiTimestamp from a heuristic such as DOMContentLoaded/first-contentful-paint combined with long-task completion (or use performance.timing.domContentLoadedEventEnd / performance.now() + waiting-for-long-tasks), and set window.__ttiTimestamp accordingly before clearing interval so benchmarks don’t yield 0; reference window.__ttiTimestamp, data-hydrated, and the interval/checkAll loop when making the change.app/javascript/components/product/ProductPageClient.tsx (1)
64-64: Gate INP diagnostics overlay behind an explicit debug flag.Line 64 mounts instrumentation UI for every user. Consider rendering it only in debug/perf mode.
Proposed fix
interface Props { product: Product; + showPerfOverlay?: boolean; } -export default function ProductPageClient({ product }: Props) { +export default function ProductPageClient({ product, showPerfOverlay = false }: Props) { return ( @@ - <INPOverlay /> + {showPerfOverlay && <INPOverlay />} </div> </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/ProductPageClient.tsx` at line 64, ProductPageClient currently mounts the INPOverlay for all users; change ProductPageClient to render INPOverlay only when an explicit debug/perf flag is set (e.g., a prop or env/feature flag). Locate the JSX that mounts <INPOverlay /> and wrap it in a conditional using a clearly named predicate like enableINPOverlay or showINPDebug (derived from process.env, a runtime window flag, or a passed-in prop), ensure the predicate defaults to false for production, and add any small helper/getter (e.g., isINPDebugEnabled) near the component to centralize the logic.db/migrate/20260223101156_create_product_reviews.rb (1)
7-7: Consider adding a database-level constraint for rating range.The
ratingcolumn accepts any integer. If ratings should be constrained (e.g., 1-5 stars), adding a CHECK constraint provides defense-in-depth alongside model validations.🛡️ Optional: Add CHECK constraint for rating
t.references :product, null: false, foreign_key: true - t.integer :rating, null: false + t.integer :rating, null: false, limit: 1 t.string :titleThen add after the table creation:
execute <<-SQL ALTER TABLE product_reviews ADD CONSTRAINT check_rating_range CHECK (rating >= 1 AND rating <= 5); SQL🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@db/migrate/20260223101156_create_product_reviews.rb` at line 7, Add a DB-level CHECK constraint to ensure product_reviews.rating stays within the expected range (e.g., 1..5) by updating the CreateProductReviews migration: after the create_table :product_reviews block, execute an ALTER TABLE to add a constraint (suggested name check_rating_range) that enforces rating >= 1 AND rating <= 5; reference the table name product_reviews and the column rating when adding this constraint so the DB enforces the same bounds as your model validations.app/javascript/components/dashboard/OrderStatusChart.tsx (1)
51-51: Add an accessible name to the donut chart SVG.Line 51 renders meaningful chart content; adding
roleandaria-labelwill improve screen-reader support.Suggested accessibility tweak
- <svg width={SIZE} height={SIZE} viewBox={`${-RADIUS} ${-RADIUS} ${SIZE} ${SIZE}`} className="flex-shrink-0"> + <svg + width={SIZE} + height={SIZE} + viewBox={`${-RADIUS} ${-RADIUS} ${SIZE} ${SIZE}`} + className="flex-shrink-0" + role="img" + aria-label="Order status distribution chart" + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/dashboard/OrderStatusChart.tsx` at line 51, The SVG for the donut chart in the OrderStatusChart component lacks an accessible name; update the <svg> element used in OrderStatusChart (the element using SIZE and RADIUS) to include appropriate accessibility attributes (e.g., role="img" and an aria-label or aria-labelledby referencing a visually-hidden <title> or label) so screen readers can describe the chart; ensure the label text is meaningful (e.g., "Order status donut chart showing X, Y, Z") and unique within the component.app/javascript/components/dashboard/DashboardFilters.tsx (1)
22-25: Scope hydration marking to this component instance.Using
document.querySelectorhere can target the wrong element when more than one filter instance is present. Prefer a localreffor deterministic TTI instrumentation.♻️ Proposed refactor
-import React, { useState, useEffect, useCallback } from 'react'; +import React, { useState, useEffect, useCallback, useRef } from 'react'; @@ export default function DashboardFilters({ statuses, onStatusFilter, onTimeRange }: DashboardFiltersProps) { + const rootRef = useRef<HTMLDivElement>(null); const [activeRange, setActiveRange] = useState('7d'); const [activeStatus, setActiveStatus] = useState<string | null>(null); @@ useEffect(() => { - const el = document.querySelector('[data-dashboard-filters]'); - if (el) el.setAttribute('data-hydrated', 'true'); + rootRef.current?.setAttribute('data-hydrated', 'true'); }, []); @@ - <div data-dashboard-filters className="bg-white rounded-xl shadow-sm border border-gray-100 p-4 mb-6"> + <div ref={rootRef} data-dashboard-filters className="bg-white rounded-xl shadow-sm border border-gray-100 p-4 mb-6">Also applies to: 38-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/dashboard/DashboardFilters.tsx` around lines 22 - 25, The useEffect is using document.querySelector('[data-dashboard-filters]') which can hit the wrong instance when multiple DashboardFilters are mounted; replace that document.querySelector usage with a component-local ref (create a ref via useRef, attach it to the root element that currently has data-dashboard-filters, e.g. ref={rootRef}), then in the useEffect set rootRef.current.setAttribute('data-hydrated','true') only if rootRef.current exists; apply the same ref-based replacement for the second occurrence that currently uses document.querySelector (the block around the other hydration/attribute-setting call).app/controllers/api/dashboard_controller.rb (1)
5-5: Scope CSRF bypass explicitly to current read endpoints.All six actions (
kpi_stats,revenue_data,order_status,recent_orders,top_menu_items,hourly_distribution) are read-only GET endpoints. Keepingskip_forgery_protectionat class scope could inadvertently exempt future write actions from CSRF protection.🔧 Proposed fix
- skip_forgery_protection + skip_forgery_protection only: %i[ + kpi_stats + revenue_data + order_status + recent_orders + top_menu_items + hourly_distribution + ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/api/dashboard_controller.rb` at line 5, Replace the class-level skip_forgery_protection with an explicit per-action exemption so only the read GET endpoints are bypassed: remove the top-level skip_forgery_protection in DashboardController and add skip_forgery_protection only: [:kpi_stats, :revenue_data, :order_status, :recent_orders, :top_menu_items, :hourly_distribution] (use the same symbol names for the controller actions) so future write actions remain protected.app/controllers/products_controller.rb (1)
41-47:find_productfallback may raise in empty database.
Product.first!on line 45 raisesRecordNotFoundif no products exist. This is fine for demo purposes, but consider a friendlier error message or redirect if this could occur in development/staging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/products_controller.rb` around lines 41 - 47, The find_product method currently falls back to Product.first! which raises ActiveRecord::RecordNotFound when the products table is empty; update find_product to handle empty DB more gracefully by rescuing RecordNotFound (or checking Product.exists? / Product.any?) and then either rendering a friendly error/flash and redirecting (e.g., to products_path) or returning nil and letting the caller handle it. Locate the find_product method and modify the Product.first! usage to perform a safe lookup (Product.first / Product.first!) with an existence check or a rescue for ActiveRecord::RecordNotFound, then implement the chosen friendly response (redirect/render or nil) in the same controller action flow.app/javascript/components/dashboard/SortableOrdersTable.tsx (2)
11-12:items_countis defined as sortable but the Items column header isn't clickable.
SortFieldincludes'items_count'but the "Items"<th>on line 90 doesn't have anonClickhandler like the other sortable columns. Either add sorting to this column or removeitems_countfromSortField.Also applies to: 89-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/dashboard/SortableOrdersTable.tsx` around lines 11 - 12, The SortField type includes 'items_count' but the Items column header in the SortableOrdersTable component lacks the sorting behavior; either remove 'items_count' from the SortField union or add the same onClick sorting handler used by other headers to the Items <th>. To fix, update the Items <th> in the SortableOrdersTable component to call the existing sort handler (e.g., the onSort or toggleSort function used by 'placed_at'/'total_price' headers) with field 'items_count' and properly toggle SortDir, or alternatively remove 'items_count' from the SortField type so it is not advertised as sortable.
37-46: Nested setState calls may cause unexpected behavior.Calling
setSortDirinsidesetSortField's updater function is an unusual pattern that can lead to unpredictable batching behavior. Consider using a single state object or separating the logic:♻️ Cleaner approach using separate logic
-const handleSort = useCallback((field: SortField) => { - setSortField(prev => { - if (prev === field) { - setSortDir(d => d === 'asc' ? 'desc' : 'asc'); - return prev; - } - setSortDir('desc'); - return field; - }); -}, []); +const handleSort = useCallback((field: SortField) => { + if (field === sortField) { + setSortDir(d => d === 'asc' ? 'desc' : 'asc'); + } else { + setSortField(field); + setSortDir('desc'); + } +}, [sortField]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/dashboard/SortableOrdersTable.tsx` around lines 37 - 46, handleSort currently calls setSortDir from inside the setSortField updater which can cause batching/unexpected behavior; refactor so state updates are separate or combined: compute the new field and dir synchronously inside handleSort (using the current sortField and sortDir values via closure or functional reads) and then call setSortField(newField) and setSortDir(newDir) in sequence (or replace both with a single useState of an object {field, dir}); update the function named handleSort and references to setSortField and setSortDir accordingly so no setX is invoked inside another state updater.app/controllers/dashboard_analytics_controller.rb (1)
36-46:Restaurant.sum(:review_count)is called for every request.
dashboard_header_datais called by all three actions, executing this aggregate query each time. For a demo this is fine, but in production consider caching this value or computing it less frequently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/dashboard_analytics_controller.rb` around lines 36 - 46, dashboard_header_data currently calls Restaurant.sum(:review_count) on every request causing repeated DB aggregation; change this to cache the computed value (e.g., via Rails.cache.fetch with an appropriate expiry or a memoized class-level value refreshed on a schedule) instead of hitting the DB each time. Locate the dashboard_header_data method and replace the direct call to Restaurant.sum(:review_count) with a cache fetch key (or a memoized helper) that returns the stored sum and falls back to computing Restaurant.sum(:review_count) only when the cache is missing or expired; ensure the cache expiry or invalidation strategy is appropriate for production data freshness.db/seed_scripts/product_seeder.rb (2)
4-75: Trailing commas in multiline arrays (style).RuboCop flags missing trailing commas on the last items of multiline arrays (lines 8, 15, 22, 29, 36, 45, 52, 59, 66, 73). This is a style preference that improves git diffs when adding items.
♻️ Example fix
REVIEW_TITLES_5 = [ 'Absolutely amazing!', 'Best purchase ever!', 'Exceeded expectations', ... - 'Five stars is not enough' + 'Five stars is not enough', ].freeze🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@db/seed_scripts/product_seeder.rb` around lines 4 - 75, RuboCop flagged missing trailing commas on multiline array literals; update each multiline array constant (REVIEW_TITLES_5, REVIEW_TITLES_4, REVIEW_TITLES_3, REVIEW_TITLES_2, REVIEW_TITLES_1) and each multiline array inside the REVIEW_COMMENTS hash (values for keys 5,4,3,2,1) to include a trailing comma after the final element on the last line of each array so they conform to the project's style and produce cleaner git diffs.
356-404: Consider wrapping bulk inserts in a transaction or improving idempotency.The early return on line 357 (
return if ProductReview.count > 0) prevents re-running on any existing data, but if seeding is interrupted mid-way, the database will have partial data and re-running won't fix it. Consider:
- Wrapping the entire loop in a transaction for atomicity
- Checking per-product review counts instead of global
- Using
truncatebefore seeding for clean re-runs💡 Suggested improvement for idempotency
def seed_product_reviews(reviews_per_product:) - return if ProductReview.count > 0 + if ProductReview.count > 0 + puts "Product reviews already exist. Skipping. Run ProductReview.delete_all to re-seed." + return + end product_ids = Product.pluck(:id) total = product_ids.size * reviews_per_product puts "\nSeeding #{format_number(total)} product reviews..." + ActiveRecord::Base.transaction do product_ids.each_with_index do |product_id, prod_idx| # ... existing batch logic ... end + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@db/seed_scripts/product_seeder.rb` around lines 356 - 404, The seed_product_reviews method currently returns early if ProductReview.count > 0 which prevents safe re-runs after a partial run; update seed_product_reviews to ensure idempotency by either wrapping the bulk insert loop in a single transaction (use ActiveRecord::Base.transaction around the product_ids.each loop) or by checking per-product counts (e.g., query ProductReview.where(product_id: product_id).count and only insert missing reviews for that product), or by truncating the product_reviews table before seeding when a full reseed is intended; adjust logic around ProductReview.count, product_ids, batches and the raw INSERT execution to reflect the chosen approach so interrupted runs can be retried safely.app/models/product.rb (1)
20-40: Consider cachingreview_statsfor products with many reviews.
review_statsexecutes aGROUP BYquery each time it's called. For products with 50k+ reviews (per the seeder), this could be slow if called frequently. Consider memoization or caching if this becomes a hot path.def review_stats `@review_stats` ||= begin # existing implementation end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/models/product.rb` around lines 20 - 40, The review_stats method runs a costly GROUP BY over product_reviews on every call; memoize its result to avoid repeated DB work by caching the computed hash in an instance variable (e.g. `@review_stats`) inside review_stats (and return the cached value on subsequent calls), and ensure any code that mutates reviews (or callbacks like after_create/after_destroy on ProductReview) clears/invalidate `@review_stats` so the cache stays correct; reference review_stats, product_reviews, average_rating, and review_count when implementing the memoization and invalidation.app/javascript/components/dashboard/DashboardPageRSC.tsx (1)
52-54: DashboardFilters callbacks are not wired up.The
DashboardFilterscomponent acceptsonStatusFilterandonTimeRangecallbacks (perDashboardFilters.tsxlines 16-90), but they're not passed here. The filter buttons will update local state but won't affect the displayed data.If this is intentional for demo purposes, consider adding a comment. Otherwise, wire up the callbacks to re-fetch or filter the data.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/dashboard/DashboardPageRSC.tsx` around lines 52 - 54, DashboardFilters is rendered without the onStatusFilter and onTimeRange callbacks so user interactions won't affect data; add handlers in DashboardPageRSC.tsx (e.g., implement handleStatusFilter and handleTimeRange) and pass them as onStatusFilter={handleStatusFilter} and onTimeRange={handleTimeRange} to DashboardFilters so selections either update local state (e.g., setStatusFilter / setTimeRange) and then trigger a re-fetch/filter of the displayed orders (call your existing fetch/mutate function or update the query params) — reference the DashboardFilters component and implement the handlers inside DashboardPageRSC.tsx to wire UI actions to data updates.app/controllers/api/products_controller.rb (1)
41-68: Serializer methods are duplicated inProductsController.
serialize_reviewandserialize_product_cardare nearly identical to those inapp/controllers/products_controller.rb(lines 70-97). Consider extracting these to a shared concern or presenter class to maintain consistency and reduce duplication.💡 Suggested extraction
# app/controllers/concerns/product_serialization.rb module ProductSerialization extend ActiveSupport::Concern private def serialize_review(review) { id: review.id, rating: review.rating, # ... rest of fields } end def serialize_product_card(product) # ... end endThen include in both controllers:
class ProductsController < ApplicationController include ProductSerialization # ... end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/api/products_controller.rb` around lines 41 - 68, Extract the duplicated serialize_review and serialize_product_card methods into a shared concern (e.g., module ProductSerialization) and include that concern in both ProductsController and Api::ProductsController; move the current implementations from each controller into the concern as private methods, remove the duplicates from both controllers, and ensure both controllers include ProductSerialization so calls to serialize_review and serialize_product_card resolve to the single shared implementation.app/models/concerns/dashboard_analytics.rb (2)
6-178: Consider extracting query objects from this concern.This concern now combines windowing, query building, and response shaping for multiple datasets. Splitting into small query/service classes would reduce complexity and make unit testing easier.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/models/concerns/dashboard_analytics.rb` around lines 6 - 178, The concern mixes windowing, SQL/query construction and result shaping; extract focused query/service objects for each dataset (e.g. KpiQuery, RevenueByDayQuery, TopMenuItemsQuery, RecentOrdersQuery, OrderStatusQuery, HourlyDistributionQuery) and keep dashboard_data_window and percent_change as shared helpers; move raw ActiveRecord/SQL logic from methods like dashboard_kpi_stats, dashboard_revenue_by_day, dashboard_top_menu_items, dashboard_recent_orders, dashboard_order_status and dashboard_hourly_distribution into those query classes (each class should accept the window_end/cutoff or compute it via DashboardAnalytics.dashboard_data_window), return raw relation or simple result structs, and let the concern methods call the query objects to only orchestrate windowing and response shaping so tests can target query classes independently.
94-100: Remove unused eager load and unnecessary per-row allocations in recent orders.Line 94 eager-loads
:restaurantbut this method does not use it. Lines 98-100 build fields (quantity,price) that are never returned.♻️ Proposed refactor
- .includes(:restaurant, order_lines: :menu_item) + .includes(order_lines: :menu_item) .order(placed_at: :desc) .limit(limit) .map do |order| - items = order.order_lines.map do |ol| - { name: ol.menu_item.name, quantity: ol.quantity, price: ol.price_per_unit.to_f } - end + item_names = order.order_lines.first(3).map { |ol| ol.menu_item.name } { id: order.id, order_number: order.order_number, status: order.status, total_price: order.total_price.to_f.round(2), items_count: order.order_lines.size, - item_names: items.first(3).map { |i| i[:name] }, + item_names: item_names, is_delivery: order.is_delivery, placed_at: order.placed_at.iso8601, completed_at: order.completed_at&.iso8601 } endAlso applies to: 107-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/models/concerns/dashboard_analytics.rb` around lines 94 - 100, The .includes(:restaurant, order_lines: :menu_item) call is eagerly loading :restaurant which isn’t used and the per-order mapping builds hashes with quantity and price that are never returned; update the query to remove :restaurant from the includes and simplify the order.order_lines mapping (in the block that references order.order_lines and ol.menu_item.name) to only produce the fields actually returned (e.g., only menu_item.name or use a lighter projection), and apply the same change to the second occurrence that mirrors this logic at the later block referenced in the file.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (78)
app/controllers/api/dashboard_controller.rbapp/controllers/api/products_controller.rbapp/controllers/dashboard_analytics_controller.rbapp/controllers/products_controller.rbapp/javascript/components/dashboard/AsyncDashboardContent.tsxapp/javascript/components/dashboard/AsyncHourlyChartRSC.tsxapp/javascript/components/dashboard/AsyncKpiStatsRSC.tsxapp/javascript/components/dashboard/AsyncOrderStatusRSC.tsxapp/javascript/components/dashboard/AsyncRecentOrdersRSC.tsxapp/javascript/components/dashboard/AsyncRevenueChartRSC.tsxapp/javascript/components/dashboard/AsyncTopItemsRSC.tsxapp/javascript/components/dashboard/DashboardFilters.tsxapp/javascript/components/dashboard/DashboardHeader.tsxapp/javascript/components/dashboard/DashboardPageClient.tsxapp/javascript/components/dashboard/DashboardPageRSC.tsxapp/javascript/components/dashboard/DashboardPageSSR.tsxapp/javascript/components/dashboard/DashboardSkeletons.tsxapp/javascript/components/dashboard/HourlyChart.tsxapp/javascript/components/dashboard/InteractiveTopItems.tsxapp/javascript/components/dashboard/OrderStatusChart.tsxapp/javascript/components/dashboard/RecentOrdersTable.tsxapp/javascript/components/dashboard/RevenueChart.tsxapp/javascript/components/dashboard/SortableOrdersTable.tsxapp/javascript/components/dashboard/StatCards.tsxapp/javascript/components/dashboard/TopMenuItemsChart.tsxapp/javascript/components/product/AddToCartSection.tsxapp/javascript/components/product/AsyncProductContent.tsxapp/javascript/components/product/AsyncProductDetailsRSC.tsxapp/javascript/components/product/AsyncRelatedProductsRSC.tsxapp/javascript/components/product/AsyncReviewStatsRSC.tsxapp/javascript/components/product/AsyncReviewsRSC.tsxapp/javascript/components/product/ProductDescription.tsxapp/javascript/components/product/ProductFeatures.tsxapp/javascript/components/product/ProductImageGallery.tsxapp/javascript/components/product/ProductInfo.tsxapp/javascript/components/product/ProductPageClient.tsxapp/javascript/components/product/ProductPageRSC.tsxapp/javascript/components/product/ProductPageSSR.tsxapp/javascript/components/product/ProductSkeletons.tsxapp/javascript/components/product/ProductSpecs.tsxapp/javascript/components/product/RelatedProducts.tsxapp/javascript/components/product/ReviewCard.tsxapp/javascript/components/product/ReviewDistributionChart.tsxapp/javascript/components/product/ReviewsList.tsxapp/javascript/startup/DashboardPageClient.client.tsxapp/javascript/startup/DashboardPageClient.server.tsxapp/javascript/startup/DashboardPageRSC.tsxapp/javascript/startup/DashboardPageSSR.tsxapp/javascript/startup/ProductPageClient.client.tsxapp/javascript/startup/ProductPageClient.server.tsxapp/javascript/startup/ProductPageRSC.tsxapp/javascript/startup/ProductPageSSR.tsxapp/javascript/types/dashboard.tsapp/javascript/types/product.tsapp/models/concerns/dashboard_analytics.rbapp/models/product.rbapp/models/product_review.rbapp/models/restaurant.rbapp/views/dashboard_analytics/show_client.html.erbapp/views/dashboard_analytics/show_rsc.html.erbapp/views/dashboard_analytics/show_ssr.html.erbapp/views/home/index.html.erbapp/views/products/show_client.html.erbapp/views/products/show_rsc.html.erbapp/views/products/show_ssr.html.erbbenchmark-dashboard.jsconfig/database.ymlconfig/routes.rbconfig/webpack/clientWebpackConfig.jsdb/migrate/20260223101148_create_products.rbdb/migrate/20260223101156_create_product_reviews.rbdb/schema.rbdb/seed_scripts/product_seeder.rbdb/seeds.rbpackage.jsonscripts/lib/collectors.mjsscripts/lib/constants.mjsscripts/lib/runner.mjs
Comment on lines
+48
to
+49
| const [timeRange, setTimeRange] = useState('7d'); | ||
|
|
There was a problem hiding this comment.
Time-range filter currently has no effect.
timeRange is updated but never used in request parameters or local data transformations, so the UI control is non-functional.
Also applies to: 99-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/components/dashboard/AsyncDashboardContent.tsx` around lines
48 - 49, The timeRange state (timeRange, setTimeRange) is never applied so the
filter UI is non-functional; update the component so timeRange is included in
network requests and local transforms: add timeRange to the dependency array of
the useEffect that fetches data and include it as a request parameter when
calling the backend (e.g., append ?time_range=${timeRange} or include in the
fetchDashboardData/fetchData call), and apply timeRange in any local
filtering/aggregation (update useMemo/selectors or the function that computes
dashboard metrics like getFilteredData/getDashboardMetrics) so derived data and
rendered charts respect the selected range. Ensure setTimeRange triggers a
refetch and that memoized values depend on timeRange.
Comment on lines
+50
to
+88
| const fetchData = useCallback(async () => { | ||
| const controller = new AbortController(); | ||
| const opts = { signal: controller.signal }; | ||
| const base = '/api/dashboard'; | ||
|
|
||
| // Fetch all data in parallel | ||
| const [kpiRes, revenueRes, statusRes, ordersRes, itemsRes, hourlyRes] = await Promise.all([ | ||
| fetch(`${base}/kpi_stats`, opts), | ||
| fetch(`${base}/revenue_data`, opts), | ||
| fetch(`${base}/order_status`, opts), | ||
| fetch(`${base}/recent_orders`, opts), | ||
| fetch(`${base}/top_menu_items`, opts), | ||
| fetch(`${base}/hourly_distribution`, opts), | ||
| ]); | ||
|
|
||
| const [kpi, revenue, status, orders, items, hourly] = await Promise.all([ | ||
| kpiRes.json(), | ||
| revenueRes.json(), | ||
| statusRes.json(), | ||
| ordersRes.json(), | ||
| itemsRes.json(), | ||
| hourlyRes.json(), | ||
| ]); | ||
|
|
||
| setData({ | ||
| kpiStats: kpi.stats, | ||
| revenueData: revenue.data, | ||
| orderStatus: status.data, | ||
| recentOrders: orders.orders, | ||
| topItems: items.items, | ||
| hourlyData: hourly.data, | ||
| }); | ||
|
|
||
| return () => controller.abort(); | ||
| }, [restaurant.id]); | ||
|
|
||
| useEffect(() => { | ||
| fetchData(); | ||
| }, [fetchData]); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
React useEffect async function cleanup callback pattern does cleanup function returned from async work
💡 Result:
useEffect’s callback must return either undefined or a cleanup function synchronously. If you make the effect callback async, it always returns a Promise, so React cannot treat the eventual resolved value as the cleanup function (you’ll get warnings / it’s ignored). React runs cleanup before the next effect (when deps change) and on unmount. [1]
Correct pattern (async work inside, cleanup returned synchronously)
useEffect(() => {
let ignore = false;
async function run() {
const data = await fetchBio(person);
if (!ignore) setBio(data);
}
run();
return () => {
ignore = true; // cleanup is synchronous
};
}, [person]);This is the pattern shown in the React docs for async/await in effects. [1]
If you need actual cancellation (e.g., fetch)
useEffect(() => {
const controller = new AbortController();
(async () => {
const res = await fetch(url, { signal: controller.signal });
setData(await res.json());
})().catch(err => {
if (err.name !== "AbortError") throw err;
});
return () => controller.abort(); // synchronous cleanup
}, [url]);Key point: React will only ever call the function you return immediately from the effect body—not something you return later from awaited/async code. [1][2]
Sources:
[1] React docs: useEffect reference + async fetching example and cleanup semantics https://react.dev/reference/react/useEffect
[2] Explanation that async effects return a Promise (not a cleanup function) https://stackoverflow.com/questions/65706459/returning-promise-from-useeffect
🏁 Script executed:
find . -type f -name "AsyncDashboardContent.tsx" | head -1 | xargs wc -lRepository: shakacode/react-server-components-marketplace-demo
Length of output: 161
🏁 Script executed:
find . -type f -name "AsyncDashboardContent.tsx" | head -1 | xargs cat -nRepository: shakacode/react-server-components-marketplace-demo
Length of output: 5287
Fix request lifecycle and error handling in dashboard data fetcher.
AbortController cleanup is never registered with the effect (async functions always return Promises, not cleanup functions). This means requests won't be aborted on unmount. Additionally, Promise.all lacks error handling and response validation—a single failed request will leave the UI in a perpetual loading state. timeRange state is also wired but never used in any API call.
🔧 Proposed fix
- const fetchData = useCallback(async () => {
- const controller = new AbortController();
- const opts = { signal: controller.signal };
+ const fetchData = useCallback(async (signal: AbortSignal) => {
+ const opts = { signal };
const base = '/api/dashboard';
// Fetch all data in parallel
const [kpiRes, revenueRes, statusRes, ordersRes, itemsRes, hourlyRes] = await Promise.all([
fetch(`${base}/kpi_stats`, opts),
fetch(`${base}/revenue_data`, opts),
fetch(`${base}/order_status`, opts),
fetch(`${base}/recent_orders`, opts),
fetch(`${base}/top_menu_items`, opts),
fetch(`${base}/hourly_distribution`, opts),
]);
+ if (![kpiRes, revenueRes, statusRes, ordersRes, itemsRes, hourlyRes].every(r => r.ok)) {
+ throw new Error('Failed to load dashboard data');
+ }
const [kpi, revenue, status, orders, items, hourly] = await Promise.all([
kpiRes.json(),
revenueRes.json(),
statusRes.json(),
ordersRes.json(),
itemsRes.json(),
hourlyRes.json(),
]);
setData({
kpiStats: kpi.stats,
revenueData: revenue.data,
orderStatus: status.data,
recentOrders: orders.orders,
topItems: items.items,
hourlyData: hourly.data,
});
-
- return () => controller.abort();
}, [restaurant.id]);
useEffect(() => {
- fetchData();
+ const controller = new AbortController();
+ fetchData(controller.signal).catch(() => {
+ // optional: set explicit error state here
+ });
+ return () => controller.abort();
}, [fetchData]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/components/dashboard/AsyncDashboardContent.tsx` around lines
50 - 88, The fetchData implementation misuses AbortController and lacks
error/response handling and timeRange usage; move controller creation into the
useEffect so the effect can return a cleanup that calls controller.abort(),
change fetchData to accept the controller.signal (or create the signal inside
fetchData but still called from the effect), wrap the Promise.all call in
try/catch and validate each Response with response.ok before calling .json() (or
use Promise.allSettled and handle failures), set error/loading state
appropriately on failure to avoid stuck UI, and append the timeRange (from
state) as a query parameter to each API endpoint; reference fetchData,
useEffect, AbortController, restaurant.id, timeRange, setData in your fixes.
Comment on lines
+10
to
+11
| const stats = await getReactOnRailsAsyncProp('kpi_stats'); | ||
| return <StatCards stats={stats} />; |
There was a problem hiding this comment.
Add a fallback path when KPI payload is missing/invalid.
Line 11 passes raw async data directly into StatCards; a bad payload can break this section’s render.
Proposed fix
export default async function AsyncKpiStatsRSC({ getReactOnRailsAsyncProp }: Props) {
const stats = await getReactOnRailsAsyncProp('kpi_stats');
- return <StatCards stats={stats} />;
+ if (!stats || typeof stats !== 'object') return null;
+ return <StatCards stats={stats} />;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const stats = await getReactOnRailsAsyncProp('kpi_stats'); | |
| return <StatCards stats={stats} />; | |
| export default async function AsyncKpiStatsRSC({ getReactOnRailsAsyncProp }: Props) { | |
| const stats = await getReactOnRailsAsyncProp('kpi_stats'); | |
| if (!stats || typeof stats !== 'object') return null; | |
| return <StatCards stats={stats} />; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/components/dashboard/AsyncKpiStatsRSC.tsx` around lines 10 -
11, The code passes raw async data from getReactOnRailsAsyncProp('kpi_stats')
directly into StatCards which can crash if the payload is missing or malformed;
update AsyncKpiStatsRSC.tsx to validate the returned stats (e.g., check for
null/undefined and that it’s an array or expected object shape) and provide a
safe fallback (such as an empty array or default KPI structure) before calling
StatCards, or wrap the await in a try/catch and use the fallback on error so
StatCards always receives a valid value.
Comment on lines
+12
to
+13
| const { items } = await getReactOnRailsAsyncProp('top_items'); | ||
| return <InteractiveTopItems items={items} />; |
There was a problem hiding this comment.
Avoid unsafe destructuring of async response.
Line 12 can throw when payload is null/undefined. Normalize before passing to InteractiveTopItems.
Proposed fix
export default async function AsyncTopItemsRSC({ getReactOnRailsAsyncProp }: Props) {
- const { items } = await getReactOnRailsAsyncProp('top_items');
+ const data = await getReactOnRailsAsyncProp('top_items');
+ const items = Array.isArray(data?.items) ? data.items : [];
return <InteractiveTopItems items={items} />;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { items } = await getReactOnRailsAsyncProp('top_items'); | |
| return <InteractiveTopItems items={items} />; | |
| export default async function AsyncTopItemsRSC({ getReactOnRailsAsyncProp }: Props) { | |
| const data = await getReactOnRailsAsyncProp('top_items'); | |
| const items = Array.isArray(data?.items) ? data.items : []; | |
| return <InteractiveTopItems items={items} />; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/components/dashboard/AsyncTopItemsRSC.tsx` around lines 12 -
13, The async response from getReactOnRailsAsyncProp('top_items') may be
null/undefined so destructuring const { items } can throw; change to await the
call into a temp variable (eg. resp or payload), extract items defensively using
optional chaining/default (resp?.items ?? []) and pass that normalized array to
InteractiveTopItems so the component always receives a defined items value.
Comment on lines
+55
to
+67
| const [timeRange, setTimeRange] = useState('7d'); | ||
|
|
||
| const statuses = useMemo(() => { | ||
| return [...new Set(recent_orders.map(o => o.status))].sort(); | ||
| }, [recent_orders]); | ||
|
|
||
| const handleStatusFilter = useCallback((status: string | null) => { | ||
| setStatusFilter(status); | ||
| }, []); | ||
|
|
||
| const handleTimeRange = useCallback((range: string) => { | ||
| setTimeRange(range); | ||
| }, []); |
There was a problem hiding this comment.
Wire timeRange to data rendering or remove the control.
This state updates but does not influence any chart/table inputs, so period changes are visually acknowledged yet functionally ignored.
Also applies to: 79-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/components/dashboard/DashboardPageSSR.tsx` around lines 55 -
67, The timeRange state and handlers (timeRange, setTimeRange, handleTimeRange)
are updated but not used to filter or drive rendering; either wire timeRange
into the data pipeline or remove the control — specifically, update the
components that render charts/tables (the consumer of recent_orders and any
chart components in this file) to accept a timeRange prop and filter
recent_orders by the selected range before computing statuses/useMemo, or filter
inside the existing useMemo that computes statuses (and any other derived data)
so that changing timeRange recomputes the displayed dataset; if you intend no
time-based filtering, remove timeRange, setTimeRange, and handleTimeRange to
avoid a non-functional UI control.
Comment on lines
+1
to
+11
| 'use client'; | ||
|
|
||
| import path from 'path'; | ||
| import React from 'react'; | ||
| import { renderToString } from 'react-dom/server'; | ||
| import { ChunkExtractor } from '@loadable/server'; | ||
| import ProductPageClient from '../components/product/ProductPageClient'; | ||
|
|
||
| const serverApp = (props: Record<string, unknown>, _railsContext: Record<string, unknown>) => { | ||
| const statsFile = path.resolve(__dirname, 'loadable-stats.json'); | ||
| const extractor = new ChunkExtractor({ entrypoints: ['client-bundle'], statsFile }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Server startup files and directives =="
fd -p '.*\.server\.tsx$' app/javascript/startup -x sh -c '
echo "---- $1 ----"
head -30 "$1"
' sh {}
echo
echo "== Repository module mode checks (ESM/CJS) =="
if [ -f package.json ]; then
echo "package.json type field:"
rg -n '"type"\s*:\s*"(module|commonjs)"' package.json || echo "No explicit type field found"
fi
echo "tsconfig files:"
fd -p 'tsconfig.*\.json$' -x sh -c '
echo "---- $1 ----"
rg -n '"module"\s*:' "$1" || echo "No module field found"
' sh {}Repository: shakacode/react-server-components-marketplace-demo
Length of output: 4056
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Build and transpilation config =="
if [ -f package.json ]; then
echo "---- package.json (relevant sections) ----"
cat package.json | jq '.type, .scripts, .devDependencies | {webpack, react, rails}' 2>/dev/null || cat package.json | grep -A5 -B5 '"build"\|"webpack"\|"react-on-rails"' || echo "Showing full package.json for context"
fi
echo
echo "== Webpack or build tool config =="
fd -p 'webpack.*config.*\.[jt]s' -x head -30 {}
fd -p 'babel.*\.[jt]s' -x head -20 {}
echo
echo "== Next.js or similar framework config =="
ls -la next.config.js 2>/dev/null || echo "No next.config.js"
echo
echo "== Check for ESM imports/exports in startup files =="
rg -n 'export.*from|import.*from' app/javascript/startup/ProductPageClient.server.tsx | head -20Repository: shakacode/react-server-components-marketplace-demo
Length of output: 5344
Remove 'use client' directive from server entry files.
The .server.tsx files in app/javascript/startup/ are marked with 'use client' while importing server-only APIs like renderToString, ChunkExtractor, path, and using __dirname. This directive contradicts their purpose as server-side entry points. Remove it from all four files: ProductPageClient.server.tsx, BlogPostClient.server.tsx, SearchPageClient.server.tsx, and DashboardPageClient.server.tsx.
The webpack and React on Rails RSC setup already separates server and client bundles correctly; the 'use client' directive is unnecessary and misleading.
Minimal fix
-'use client';
-
import path from 'path';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 'use client'; | |
| import path from 'path'; | |
| import React from 'react'; | |
| import { renderToString } from 'react-dom/server'; | |
| import { ChunkExtractor } from '@loadable/server'; | |
| import ProductPageClient from '../components/product/ProductPageClient'; | |
| const serverApp = (props: Record<string, unknown>, _railsContext: Record<string, unknown>) => { | |
| const statsFile = path.resolve(__dirname, 'loadable-stats.json'); | |
| const extractor = new ChunkExtractor({ entrypoints: ['client-bundle'], statsFile }); | |
| import path from 'path'; | |
| import React from 'react'; | |
| import { renderToString } from 'react-dom/server'; | |
| import { ChunkExtractor } from '@loadable/server'; | |
| import ProductPageClient from '../components/product/ProductPageClient'; | |
| const serverApp = (props: Record<string, unknown>, _railsContext: Record<string, unknown>) => { | |
| const statsFile = path.resolve(__dirname, 'loadable-stats.json'); | |
| const extractor = new ChunkExtractor({ entrypoints: ['client-bundle'], statsFile }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/startup/ProductPageClient.server.tsx` around lines 1 - 11,
Remove the erroneous 'use client' directive at the top of the server entry files
(e.g. ProductPageClient.server.tsx) so server-only APIs can be used;
specifically delete the "'use client';" line in files that define serverApp and
import path, renderToString, ChunkExtractor and reference __dirname
(ProductPageClient.server.tsx, BlogPostClient.server.tsx,
SearchPageClient.server.tsx, DashboardPageClient.server.tsx) to ensure these
modules run as server code and rely on the existing webpack/React-on-Rails RSC
separation.
Comment on lines
+16
to
+18
| database: <%= ENV.fetch('DATABASE_NAME', 'localhub_demo_production') %> | ||
| username: <%= ENV['DATABASE_USERNAME'] || ENV['LOCALHUB_DEMO_DATABASE_USERNAME'] || 'localhub_demo' %> | ||
| password: <%= ENV['DATABASE_PASSWORD'] || ENV['LOCALHUB_DEMO_DATABASE_PASSWORD'] %> |
There was a problem hiding this comment.
Fail fast on missing production DB credentials.
Line 16 through Line 18 currently allow silent fallbacks in production, which can boot against unintended DB/user settings instead of failing early on misconfiguration.
Suggested hardening patch
- database: <%= ENV.fetch('DATABASE_NAME', 'localhub_demo_production') %>
- username: <%= ENV['DATABASE_USERNAME'] || ENV['LOCALHUB_DEMO_DATABASE_USERNAME'] || 'localhub_demo' %>
- password: <%= ENV['DATABASE_PASSWORD'] || ENV['LOCALHUB_DEMO_DATABASE_PASSWORD'] %>
+ database: <%= ENV.fetch('DATABASE_NAME') %>
+ username: <%= ENV.fetch('DATABASE_USERNAME') { ENV.fetch('LOCALHUB_DEMO_DATABASE_USERNAME') } %>
+ password: <%= ENV.fetch('DATABASE_PASSWORD') { ENV.fetch('LOCALHUB_DEMO_DATABASE_PASSWORD') } %>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| database: <%= ENV.fetch('DATABASE_NAME', 'localhub_demo_production') %> | |
| username: <%= ENV['DATABASE_USERNAME'] || ENV['LOCALHUB_DEMO_DATABASE_USERNAME'] || 'localhub_demo' %> | |
| password: <%= ENV['DATABASE_PASSWORD'] || ENV['LOCALHUB_DEMO_DATABASE_PASSWORD'] %> | |
| database: <%= ENV.fetch('DATABASE_NAME') %> | |
| username: <%= ENV.fetch('DATABASE_USERNAME') { ENV.fetch('LOCALHUB_DEMO_DATABASE_USERNAME') } %> | |
| password: <%= ENV.fetch('DATABASE_PASSWORD') { ENV.fetch('LOCALHUB_DEMO_DATABASE_PASSWORD') } %> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/database.yml` around lines 16 - 18, The production DB credentials
currently fall back silently; change the ERB so that in production the app uses
strict ENV.fetch checks (no default values) for DATABASE_NAME, DATABASE_USERNAME
and DATABASE_PASSWORD (e.g. use Rails.env.production? to select ENV.fetch(...)
for database, username and password) so the process will raise and fail fast
when those env vars are missing, while keeping the non-production fallback
behavior intact.
Comment on lines
+8
to
+19
| t.decimal :price, precision: 10, scale: 2, null: false | ||
| t.decimal :original_price, precision: 10, scale: 2 | ||
| t.string :category, null: false | ||
| t.string :brand, null: false | ||
| t.string :sku, null: false | ||
| t.jsonb :images, default: [] | ||
| t.jsonb :specs, default: {} | ||
| t.jsonb :features, default: [] | ||
| t.decimal :average_rating, precision: 3, scale: 2, default: 0 | ||
| t.integer :review_count, default: 0 | ||
| t.integer :stock_quantity, default: 100 | ||
| t.boolean :in_stock, default: true |
There was a problem hiding this comment.
Add DB-level constraints for product numeric invariants.
Negative prices/counts or out-of-range ratings can still be persisted via non-model paths. This should be guarded in the database.
💡 Suggested fix
class CreateProducts < ActiveRecord::Migration[7.2]
def change
create_table :products do |t|
@@
t.boolean :in_stock, default: true
t.timestamps
end
+ add_check_constraint :products, 'price >= 0', name: 'products_price_non_negative'
+ add_check_constraint :products, '(original_price IS NULL) OR (original_price >= 0)', name: 'products_original_price_non_negative'
+ add_check_constraint :products, '(average_rating >= 0) AND (average_rating <= 5)', name: 'products_average_rating_range'
+ add_check_constraint :products, 'review_count >= 0', name: 'products_review_count_non_negative'
+ add_check_constraint :products, 'stock_quantity >= 0', name: 'products_stock_quantity_non_negative'
+
add_index :products, :category
add_index :products, :brand
add_index :products, :sku, unique: true
end
endAlso applies to: 23-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@db/migrate/20260223101148_create_products.rb` around lines 8 - 19, Add
DB-level CHECK constraints on the products table to enforce numeric invariants:
ensure price >= 0 and (original_price IS NULL OR original_price >= 0),
average_rating BETWEEN 0 AND 5, review_count >= 0, and stock_quantity >= 0.
Update the migration that defines these columns (the
t.decimal/t.integer/t.boolean lines) to add matching add_check_constraint calls
(or equivalent SQL CHECK clauses) with clear names (e.g.,
chk_products_price_nonnegative, chk_products_original_price_nonnegative,
chk_products_average_rating_range, chk_products_review_count_nonnegative,
chk_products_stock_quantity_nonnegative) so the database prevents negative or
out-of-range values even when bypassing model validations.
Comment on lines
+74
to
+88
| create_table "product_reviews", force: :cascade do |t| | ||
| t.bigint "product_id", null: false | ||
| t.integer "rating", null: false | ||
| t.string "title" | ||
| t.text "comment" | ||
| t.string "reviewer_name", null: false | ||
| t.boolean "verified_purchase", default: false | ||
| t.integer "helpful_count", default: 0 | ||
| t.datetime "created_at", null: false | ||
| t.datetime "updated_at", null: false | ||
| t.index ["product_id", "created_at"], name: "index_product_reviews_on_product_id_and_created_at" | ||
| t.index ["product_id", "helpful_count"], name: "index_product_reviews_on_product_id_and_helpful_count" | ||
| t.index ["product_id", "rating"], name: "index_product_reviews_on_product_id_and_rating" | ||
| t.index ["product_id"], name: "index_product_reviews_on_product_id" | ||
| end |
There was a problem hiding this comment.
Add DB constraints for product_reviews integrity.
product_reviews.rating and helpful_count are unconstrained at DB level. Invalid values can slip in and break rating/analytics assumptions.
🔧 Proposed migration-level fix
# new migration (do not edit schema.rb directly)
change_column_null :product_reviews, :helpful_count, false
add_check_constraint :product_reviews, "rating >= 1 AND rating <= 5", name: "product_reviews_rating_check"
add_check_constraint :product_reviews, "helpful_count >= 0", name: "product_reviews_helpful_count_check"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@db/schema.rb` around lines 74 - 88, Add DB-level constraints for the
product_reviews table: make helpful_count non-null and add CHECK constraints to
enforce rating is between 1 and 5 and helpful_count is >= 0. Implement this in a
new Rails migration (do not edit schema.rb) using change_column_null
:product_reviews, :helpful_count, false and two add_check_constraint calls with
names product_reviews_rating_check and product_reviews_helpful_count_check
targeting the rating and helpful_count columns respectively.
Comment on lines
+20
to
+21
| reviews_count = mode == 'full' ? 100_000 : 50_000 | ||
| ProductSeeder.seed!(reviews_per_product: reviews_count) |
There was a problem hiding this comment.
SEED_MODE=small now seeds a very large dataset.
50_000 reviews per product in small mode is likely to make local seeding slow/heavy and blurs the small vs full contract.
💡 Suggested fix
-# Always seed products (idempotent)
-reviews_count = mode == 'full' ? 100_000 : 50_000
+# Always seed products (idempotent)
+reviews_count =
+ if mode == 'full'
+ ENV.fetch('REVIEWS_PER_PRODUCT_FULL', '100000').to_i
+ else
+ ENV.fetch('REVIEWS_PER_PRODUCT_SMALL', '500').to_i
+ end
ProductSeeder.seed!(reviews_per_product: reviews_count)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@db/seeds.rb` around lines 20 - 21, The small seed mode currently sets
reviews_count to 50_000 which is too large; change the ternary assignment for
reviews_count (the variable set using mode and passed into ProductSeeder.seed!)
to a much smaller number for SEED_MODE='small' (e.g., 5_000 or 500) so local
seeding is lightweight and preserves the small vs full distinction; update the
line where reviews_count is computed (the conditional that checks mode) to use
the new smaller value.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
react-on-rails-rscfrom 19.0.3 to 19.0.5-rc.1What's in the demo
Routes
/analytics/ssr— Server-Side Rendered (V1): waits for ALL 6 API calls to complete before sending any HTML/analytics/client— Client-Side (V2): sends shell immediately, fetches all data client-side after hydration/analytics/rsc— React Server Components (V3): streams each section independently as its data resolvesKey architectural differences
Dashboard API endpoints
/api/dashboard/kpi_stats— Key performance indicators/api/dashboard/revenue_data— Revenue chart data/api/dashboard/order_status— Order status breakdown/api/dashboard/recent_orders— Recent orders table/api/dashboard/top_menu_items— Top menu items ranking/api/dashboard/hourly_distribution— Hourly order distributionRSC Performance Improvements
Normal conditions (dedicated benchmark with interaction testing)
Throttled conditions (4x CPU slowdown + Slow 3G)
Test plan
/analytics/ssr,/analytics/client,/analytics/rscand verify all three render correctlyBASE_URL=http://localhost:PORT node benchmark-dashboard.jsto reproduce benchmarks🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores