feat(media): support paused indexing with resume action#108
Conversation
Adds a media.generate.resume API call and surfaces paused/reconnecting states in MediaDatabaseCard so users can resume an interrupted index generation. ConnectionProvider now refetches media state on connect (with one delayed retry) so the card reflects an in-progress run after a reconnect, and suppresses fetch error toasts while the WS isn't live.
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors media fetch to handle cancellations and delayed retry, adds connection-state-aware toast suppression, implements pause/resume for media indexing with a new Changes
Sequence DiagramsequenceDiagram
participant UI as MediaDatabaseCard
participant CP as ConnectionProvider
participant API as CoreAPI
participant RQ as React Query
participant Store as gamesIndex
UI->>Store: Subscribe/read indexing state (currentStep, currentStepDisplay, paused)
CP->>RQ: Invalidate ["media"] cache
CP->>API: Call media()
alt media returns results
API-->>CP: Media results
CP->>Store: Update gamesIndex / playing
else media request cancelled
CP->>CP: Schedule single delayed retry (mediaRetryTimer)
end
UI->>API: (user) Click Resume -> mediaGenerateResume()
API-->>UI: Promise resolves/rejects
UI->>RQ: Invalidate ["media"]
UI->>Store: Update resumeRequested / paused state
Note over CP,UI: Connection state changes
CP->>UI: Update connectionState
alt connectionState == CONNECTED
UI->>UI: Show error toasts for media/tokens
else
UI->>UI: Suppress error toasts
end
CP->>CP: On unmount/connection change -> clear mediaRetryTimer
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/__tests__/unit/components/MediaDatabaseCard.test.tsx (2)
37-50: ⚡ Quick winKeep
connectionStatetyped as theConnectionStateunion, notstring.
connectionState: ConnectionState.CONNECTED as stringremoves compile-time validation and can allow invalid states in test setup.Suggested typing adjustment
const { ConnectionState, mockStore } = vi.hoisted(() => { + type ConnectionStateValue = + (typeof ConnectionState)[keyof typeof ConnectionState]; + const mockStore = { connected: true, - connectionState: ConnectionState.CONNECTED as string, + connectionState: ConnectionState.CONNECTED as ConnectionStateValue,As per coding guidelines:
**/*.{ts,tsx}: Use TypeScript strict mode and avoid any without justification.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/unit/components/MediaDatabaseCard.test.tsx` around lines 37 - 50, The test is casting connectionState to string which removes compile-time validation; change the mockStore typing so connectionState uses the ConnectionState union instead of string (e.g., declare mockStore with connectionState: typeof ConnectionState[keyof typeof ConnectionState] or type alias using ConnectionState's value union and assign ConnectionState.CONNECTED without "as string") so the value must be one of the defined ConnectionState members; update the mockStore declaration and any related type annotations to use that union rather than a plain string.
299-346: ⚡ Quick winCollapse spinner visibility variants into
it.eachto reduce duplication.These three tests are the same assertion shape with different inputs; a table-driven test keeps behavior coverage while reducing maintenance drift.
Refactor sketch with
it.each- it("should hide the spinner during phase steps (currentStep===0)", () => { ... }); - it("should hide the spinner during the writing phase (currentStep===totalSteps)", () => { ... }); - it("should show the spinner during per-system steps", () => { ... }); + it.each([ + ["phase steps", { currentStep: 0, totalSteps: 0, currentStepDisplay: "Initializing database" }, false], + ["writing phase", { currentStep: 11, totalSteps: 11, currentStepDisplay: "Writing database" }, false], + ["per-system steps", { currentStep: 3, totalSteps: 11, currentStepDisplay: "Nintendo Entertainment System" }, true], + ])("should handle spinner visibility for %s", (_, stepState, shouldShow) => { + mockStore.gamesIndex = { + indexing: true, + exists: true, + totalFiles: 100, + ...stepState, + }; + render(<MediaDatabaseCard />); + const spinner = screen.queryByRole("status", { name: "Loading" }); + if (shouldShow) expect(spinner).toBeInTheDocument(); + else expect(spinner).not.toBeInTheDocument(); + });As per coding guidelines:
In tests, use it.each for repetitive test patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/unit/components/MediaDatabaseCard.test.tsx` around lines 299 - 346, Collapse the three duplicate tests into a table-driven it.each that iterates over cases for mockStore.gamesIndex (rows: phase step with currentStep:0 and totalSteps:0 expecting spinner hidden; writing phase with currentStep===totalSteps expecting hidden; per-system step with currentStep between 1 and totalSteps-1 expecting spinner visible), render MediaDatabaseCard inside each iteration, set mockStore.gamesIndex accordingly, and assert presence using screen.queryByRole("status", { name: "Loading" }) and expect(...).not.toBeInTheDocument() for hidden cases or expect(screen.getByRole("status", { name: "Loading" })).toBeInTheDocument() for visible case; name the test string to include the case description for clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/__tests__/unit/components/MediaDatabaseCard.test.tsx`:
- Around line 37-50: The test is casting connectionState to string which removes
compile-time validation; change the mockStore typing so connectionState uses the
ConnectionState union instead of string (e.g., declare mockStore with
connectionState: typeof ConnectionState[keyof typeof ConnectionState] or type
alias using ConnectionState's value union and assign ConnectionState.CONNECTED
without "as string") so the value must be one of the defined ConnectionState
members; update the mockStore declaration and any related type annotations to
use that union rather than a plain string.
- Around line 299-346: Collapse the three duplicate tests into a table-driven
it.each that iterates over cases for mockStore.gamesIndex (rows: phase step with
currentStep:0 and totalSteps:0 expecting spinner hidden; writing phase with
currentStep===totalSteps expecting hidden; per-system step with currentStep
between 1 and totalSteps-1 expecting spinner visible), render MediaDatabaseCard
inside each iteration, set mockStore.gamesIndex accordingly, and assert presence
using screen.queryByRole("status", { name: "Loading" }) and
expect(...).not.toBeInTheDocument() for hidden cases or
expect(screen.getByRole("status", { name: "Loading" })).toBeInTheDocument() for
visible case; name the test string to include the case description for clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a822a98-e335-4049-92a2-a1007b2ca2b4
📒 Files selected for processing (9)
src/__tests__/unit/components/ConnectionProvider.test.tsxsrc/__tests__/unit/components/MediaDatabaseCard.test.tsxsrc/__tests__/unit/components/MediaIndexingToast.test.tsxsrc/components/ConnectionProvider.tsxsrc/components/MediaDatabaseCard.tsxsrc/components/MediaIndexingToast.tsxsrc/lib/coreApi.tssrc/lib/models.tssrc/translations/en-US.json
Adds an api-contract test for `mediaGenerateResume`, a regression test for the MediaDatabaseCard re-enabling the Resume button after a failed resume call, and two ConnectionProvider tests covering the new retry-on-cancel behaviour and its cleanup-on-unmount path.
* feat(media): support paused indexing with resume action Adds a media.generate.resume API call and surfaces paused/reconnecting states in MediaDatabaseCard so users can resume an interrupted index generation. ConnectionProvider now refetches media state on connect (with one delayed retry) so the card reflects an in-progress run after a reconnect, and suppresses fetch error toasts while the WS isn't live. * test(media): cover resume API, retry-on-cancel, and resume-failure paths Adds an api-contract test for `mediaGenerateResume`, a regression test for the MediaDatabaseCard re-enabling the Resume button after a failed resume call, and two ConnectionProvider tests covering the new retry-on-cancel behaviour and its cleanup-on-unmount path.
media.generate.resumeAPI method and surface apausedflag onIndexResponse.MediaDatabaseCardnow shows paused/reconnecting status text, swaps the Cancel button for Resume while paused, and pauses the progress animation when not actively indexing.ConnectionProviderinvalidates the["media"]query and refetches media state on (re)connect, with one delayed retry to avoid racing the transport coming back up.CONNECTED— the connection-state UI already conveys reconnect/disconnect, so flapping shouldn't spam toasts.currentStepDisplayfor every phase, including the final write.Summary by CodeRabbit
New Features
Bug Fixes
Tests