From 4066370a14df84d526961364f3d0f81030c6823d Mon Sep 17 00:00:00 2001 From: Ammar Date: Tue, 9 Dec 2025 18:18:17 -0600 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20fix:=20make=20bulk=20review=20st?= =?UTF-8?q?ory=20ordering=20deterministic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The BulkReviewActions story was unstable because createReview() used Math.random() for createdAt timestamps, causing reviews to appear in different orders on each render. Fix: Add optional createdAt parameter to createReview() and pass deterministic timestamps in the story. _Generated with mux_ --- docs/AGENTS.md | 1 + src/browser/stories/App.reviews.stories.tsx | 20 ++++++++++++++------ src/browser/stories/storyHelpers.ts | 5 +++-- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 7bd91d7672..a2e05e0784 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -72,6 +72,7 @@ Avoid mock-heavy tests that verify implementation details rather than behavior. - Prefer full-app stories (`App.stories.tsx`) to isolated component stories. This tests components in their real context with proper providers, state management, and styling. - Use play functions with `@storybook/test` utilities (`within`, `userEvent`, `waitFor`) to interact with the UI and set up the desired visual state. Do not add props to production components solely for storybook convenience. +- Keep story data deterministic: avoid `Math.random()`, `Date.now()`, or other non-deterministic values in story setup. Pass explicit values when ordering or timing matters for visual stability. ### TDD Expectations diff --git a/src/browser/stories/App.reviews.stories.tsx b/src/browser/stories/App.reviews.stories.tsx index 77f820ac74..e37c608e45 100644 --- a/src/browser/stories/App.reviews.stories.tsx +++ b/src/browser/stories/App.reviews.stories.tsx @@ -168,6 +168,8 @@ export const BulkReviewActions: AppStory = { setup={() => { const workspaceId = "ws-bulk-reviews"; + // Use deterministic timestamps so reviews render in stable order + const baseTime = 1700000000000; setReviews(workspaceId, [ // Attached reviews - shown in ChatInput with "Clear all" button createReview( @@ -175,21 +177,24 @@ export const BulkReviewActions: AppStory = { "src/api/auth.ts", "42-48", "Consider using a constant for the token expiry", - "attached" + "attached", + baseTime + 1 ), createReview( "review-attached-2", "src/utils/helpers.ts", "15-20", "This function could be simplified using reduce", - "attached" + "attached", + baseTime + 2 ), createReview( "review-attached-3", "src/hooks/useAuth.ts", "30-35", "Missing error handling for network failures", - "attached" + "attached", + baseTime + 3 ), // Pending reviews - shown in banner with "Attach all to chat" button createReview( @@ -197,21 +202,24 @@ export const BulkReviewActions: AppStory = { "src/components/LoginForm.tsx", "55-60", "Add loading state while authenticating", - "pending" + "pending", + baseTime + 4 ), createReview( "review-pending-2", "src/services/api.ts", "12-18", "Consider adding retry logic for failed requests", - "pending" + "pending", + baseTime + 5 ), createReview( "review-pending-3", "src/types/user.ts", "5-10", "Make email field optional for guest users", - "pending" + "pending", + baseTime + 6 ), ]); diff --git a/src/browser/stories/storyHelpers.ts b/src/browser/stories/storyHelpers.ts index 09cb81d50c..02826ec95e 100644 --- a/src/browser/stories/storyHelpers.ts +++ b/src/browser/stories/storyHelpers.ts @@ -76,7 +76,8 @@ export function createReview( filePath: string, lineRange: string, note: string, - status: "pending" | "attached" | "checked" = "pending" + status: "pending" | "attached" | "checked" = "pending", + createdAt?: number ): Review { return { id, @@ -87,7 +88,7 @@ export function createReview( userNote: note, }, status, - createdAt: Date.now() - Math.random() * 3600000, // Random time in last hour + createdAt: createdAt ?? Date.now(), statusChangedAt: status === "checked" ? Date.now() : undefined, }; }