added audio book services#16
Open
parthdude07 wants to merge 2 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request implements a complete audiobook learning feature. It introduces database schema for playlists and episodes with user progress tracking, a backend API for browsing audiobooks and saving playback progress, frontend React components for rendering chapter roadmaps and playing audio, and pages for listing audiobooks and viewing learning paths. ChangesAudiobook Learning Roadmap
Sequence DiagramsequenceDiagram
participant User
participant Audiobooks as Audiobooks Page
participant LearningPath as LearningPath Page
participant AudioPlayer
participant FrontendAPI as Frontend API
participant BackendAPI as Backend API
User->>Audiobooks: Visit /audiobooks
Audiobooks->>FrontendAPI: usePlaylists({ playlist_type: "series" })
FrontendAPI->>BackendAPI: GET /api/v1/audiobooks/playlists
BackendAPI-->>FrontendAPI: playlists[]
FrontendAPI-->>Audiobooks: cached playlists
Audiobooks-->>User: display playlist cards
User->>LearningPath: Click playlist → /learning-path/:slug
LearningPath->>FrontendAPI: usePlaylistBySlug(slug)
FrontendAPI->>BackendAPI: GET /api/v1/audiobooks/playlists/:slug?x-user-id=...
BackendAPI-->>FrontendAPI: playlist + episodes + user_progress
FrontendAPI-->>LearningPath: cached playlist
LearningPath-->>User: display roadmap with chapters
User->>AudioPlayer: Click chapter → mount player
AudioPlayer->>AudioPlayer: resume from chapter.progress.current_seconds
AudioPlayer->>FrontendAPI: saveProgress(chapterId, currentSeconds, false)
FrontendAPI->>BackendAPI: POST /api/v1/audiobooks/progress
BackendAPI-->>FrontendAPI: success
User->>AudioPlayer: Reach ~95% completion
AudioPlayer->>FrontendAPI: saveProgress(chapterId, 0, true)
FrontendAPI->>BackendAPI: POST /api/v1/audiobooks/progress
BackendAPI-->>FrontendAPI: success
FrontendAPI->>LearningPath: invalidate + update cache
LearningPath-->>User: chapter marked completed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an end-to-end "Audiobook Learning Roadmap" feature, including new database schema and seed data, backend API endpoints, frontend pages/components, and a persistent audio player with progress tracking.
Changes:
- New backend:
audiobooksschema, service, controller, and routes for listing audiobooks, fetching a roadmap (with progress-based unlock logic), and saving progress. - New frontend:
Audiobookslisting page,LearningPathroadmap page, and supporting components (AudiobookHeader,RoadmapTrack,RoadmapNode,AudioPlayer), wired via React Query hooks and a service layer with anonymous user IDs. - Schema migrations and seed data for two audiobooks ("Bitcoin Fundamentals", "Bitcoin Beginners Guide"), plus nav/route wiring.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| types.ts | Adds Audiobook, AudiobookChapter, ChapterProgress, RoadmapChapter, and AudiobookRoadmap types. |
| src/pages/LearningPath.tsx | New roadmap page that loads a roadmap, hosts the audio player, and saves progress. |
| src/pages/Audiobooks.tsx | New listing page for all audiobook series with loading/empty states. |
| src/hooks/useAudiobooks.ts | React Query hooks for list, roadmap, and save-progress mutation. |
| src/components/audiobook/constants.ts | Shared difficulty config and formatDuration helper. |
| src/components/audiobook/RoadmapTrack.tsx | Vertical alternating-track layout for chapter nodes. |
| src/components/audiobook/RoadmapNode.tsx | Single chapter node with status visuals and resume %. |
| src/components/audiobook/AudiobookHeader.tsx | Header card with thumbnail, stats, and progress bar. |
| src/components/audiobook/AudioPlayer.tsx | Fixed bottom audio player with scrub, speed, expand, autosave. |
| src/components/Layout.tsx | Adds Audiobooks nav item. |
| src/App.tsx | Registers /audiobooks and /learning-path/:id routes. |
| services/config.ts | Adds audiobook endpoints config. |
| services/audiobookService.ts | Frontend API client with anonymous user ID via localStorage. |
| backend/supabase/migrations/002_audiobooks_schema.sql | Creates audiobooks schema, series/chapters/user_progress tables. |
| backend/supabase/migrations/003_seed_audiobooks_fundamentals.sql | Seed data for "Bitcoin Fundamentals". |
| backend/supabase/migrations/004_seed_audiobooks_beginners.sql | Seed data for "Bitcoin Beginners Guide" (18 chapters). |
| backend/src/services/audiobookService.js | Postgres pool + queries for audiobooks, chapters, progress. |
| backend/src/routes/audiobookRoutes.js | Express routes for audiobook endpoints. |
| backend/src/routes/index.js | Mounts audiobook routes and documents them. |
| backend/src/controllers/audiobookController.js | Controllers implementing list, roadmap (with unlock logic), and progress save. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (4)
src/hooks/useAudiobooks.ts (1)
44-65: 💤 Low valueDocument that
audiobookIdis required for automatic UI updates.The
useSaveProgressmutation only invalidates the roadmap query whenaudiobookIdis provided. If callers omit this parameter, progress will save but the UI won't automatically reflect the updated chapter status, leading to stale state.Consider either making
audiobookIdrequired or documenting this behavior clearly in a comment.📝 Proposed documentation enhancement
/** * Mutation to save user progress on a chapter. * Automatically invalidates the roadmap query to update node states. + * `@param` audiobookId - Required for automatic UI invalidation. If omitted, + * progress will save but UI won't auto-refresh. */ export const useSaveProgress = (audiobookId?: string) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useAudiobooks.ts` around lines 44 - 65, The useSaveProgress hook currently treats audiobookId as optional which prevents automatic roadmap cache invalidation when omitted; either make audiobookId required on the hook signature (change useSaveProgress(audiobookId?: string) to useSaveProgress(audiobookId: string)) so callers must pass it and the onSuccess always calls queryClient.invalidateQueries({ queryKey: AUDIOBOOK_QUERY_KEYS.roadmap(audiobookId) }), or if you want to keep it optional add a clear JSDoc comment above useSaveProgress explaining that passing audiobookId is required for automatic UI updates (invalidate of AUDIOBOOK_QUERY_KEYS.roadmap), and consider adding a runtime warning (console.warn) when onSuccess runs without audiobookId to help callers notice missing parameter.services/audiobookService.ts (1)
32-62: ⚡ Quick winInconsistent error handling across API methods.
The
getAudiobooks()function swallows all errors and returns an empty array, whilegetAudiobookRoadmap()rethrows non-404 errors. This inconsistency makes it harder for consumers to handle failures uniformly.Consider standardizing the error handling pattern: either let React Query handle all errors (recommended), or consistently return null/empty for all methods.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/audiobookService.ts` around lines 32 - 62, The two API functions have inconsistent error handling: getAudiobooks currently swallows all errors and returns an empty array while getAudiobookRoadmap only swallows 404s and rethrows others; standardize by letting errors bubble up so React Query can handle them—remove or simplify the try/catch in getAudiobooks (or rethrow after logging) and keep the existing 404-to-null behavior in getAudiobookRoadmap (only catch APIError with statusCode 404 and return null, otherwise rethrow); ensure you reference APIError, getAudiobooks, and getAudiobookRoadmap when making the change so logging remains informative but errors are not silently swallowed.src/components/audiobook/AudioPlayer.tsx (2)
76-81: ⚡ Quick winAuto-save every 5s forces a roadmap refetch each time.
onProgressSaveflows intouseSaveProgress, whoseonSuccessinvalidatesAUDIOBOOK_QUERY_KEYS.roadmap(audiobookId). WithSAVE_INTERVAL_MS = 5000, that triggers a roadmap network refetch roughly every 5 seconds during continuous playback. Also notelastSaveRef.currentstarts at0, so the firsttimeupdatealways saves immediately. Consider skipping invalidation for incremental progress saves (only invalidate on completion), or debouncing/optimistically updating the cache instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/audiobook/AudioPlayer.tsx` around lines 76 - 81, Auto-saves every SAVE_INTERVAL_MS cause useSaveProgress's onSuccess to always invalidate AUDIOBOOK_QUERY_KEYS.roadmap, triggering frequent refetches; to fix, change the auto-save call site (where onProgressSaveRef.current is invoked) to pass an "incremental" or "skipInvalidate" flag and initialize lastSaveRef.current = Date.now() so the first timeupdate doesn't immediately save, and update useSaveProgress to respect that flag: only run queryClient.invalidateQueries(AUDIOBOOK_QUERY_KEYS.roadmap(audiobookId)) on final/completion saves (or when flag is false). Also consider making SAVE_INTERVAL_MS configurable or increasing it, but the main fix is to add and honor the skip-invalidation flag in onProgressSaveRef/useSaveProgress to avoid roadmap refetches during continuous playback.
169-175: 💤 Low valueRedundant double progress save on close.
handleClosesaves progress, then callsonClose()which unmounts the component, firing the unmount effect (Lines 113-119) that saves again for the same chapter. Harmless but doubles the mutation/invalidation. Could rely solely on the unmount effect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/audiobook/AudioPlayer.tsx` around lines 169 - 175, The handleClose function currently calls onProgressSave and then calls onClose which triggers the component unmount effect that also saves progress, causing a duplicate save; remove the onProgressSave call from handleClose so it only pauses playback (using audioRef.current.pause()) and calls onClose, and rely on the existing unmount effect cleanup that invokes onProgressSave for the chapter to perform the single save. Ensure audioRef and chapter.id are still available to the unmount effect and do not delete that effect.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/services/audiobookService.js`:
- Around line 45-54: The interpolation for setting statement_timeout in the
query function should coerce timeoutMs to a number to satisfy static analysis
and harden against non-numeric callers; update the SET statement_timeout
interpolation in the query function (the client.query call that currently uses
`SET statement_timeout = ${timeoutMs}`) to use a numeric coercion like
Number(timeoutMs) (or +timeoutMs) before embedding it, ensuring the rest of the
function (getPool(), client.query(text, params), client.release()) remains
unchanged.
- Around line 24-30: The Pool configuration currently disables TLS verification
via ssl: { rejectUnauthorized: false } which is insecure; update the Pool
instantiation (Pool) so it does not set rejectUnauthorized: false, instead
enable certificate verification and provide the Supabase/Postgres CA/root
certificate via the ssl options (e.g., ssl: { ca: process.env.PG_SSL_ROOT_CERT,
rejectUnauthorized: true } or equivalent) and ensure the connectionString
(config.database.url) and environment provide the CA (sslrootcert) so
node-postgres uses verify-full semantics; remove the insecure flag and wire in
the proper CA source while keeping verification enabled.
In `@backend/supabase/migrations/004_seed_audiobooks_beginners.sql`:
- Around line 121-201: Chapters 11–18 in the migration are seeded with
audio_url=NULL and duration_seconds=0 which prevents frontend AudioPlayer and
RoadmapNode from allowing completion and thus blocks progression; update the
INSERT tuples for UUIDs 'cb000000-0000-0000-0000-000000000011' through
'd2000000-0000-0000-0000-000000000018' to include real audio_url values and
realistic duration_seconds, then re-run the migration (or add a corrective
migration) so audiobookController.js's availability/completion gating can
function normally without changing controller logic.
In `@services/audiobookService.ts`:
- Around line 32-43: The getAudiobooks function currently swallows failures by
returning an empty array; update getAudiobooks (and its catch block) to rethrow
the original error (preserving APIError details) instead of returning [] so
callers (e.g., useAudiobooks/React Query) can distinguish network/API failures
from a legitimate empty result; ensure you still log the error (using
error.message and error.code for APIError) and throw the same error object after
logging so upstream handlers can present errors to the user.
- Around line 18-25: The getUserId function should not call crypto.randomUUID()
or access localStorage unguarded; wrap localStorage.getItem and
localStorage.setItem calls in try/catch to make persistence best-effort, and
check for crypto?.randomUUID before calling it (fall back to a safe UUID
generator such as a lightweight Math.random-based or timestamp+random string) so
the function never throws; update getUserId to attempt reading USER_ID_KEY from
localStorage inside try, if missing generate id using crypto.randomUUID() when
available else use fallback, then attempt to persist it in localStorage inside
try/catch and always return the id string.
In `@src/components/audiobook/AudioPlayer.tsx`:
- Around line 134-146: The handleSkip callback must guard against audio.duration
being NaN by using the component's known duration state (e.g., duration or
audioDuration) as a fallback; compute a safeDuration =
Number.isFinite(audio.duration) ? audio.duration : durationState, then compute
targetTime = direction === "forward" ? Math.min(audio.currentTime + skip,
safeDuration) : Math.max(audio.currentTime - skip, 0), assign audio.currentTime
= targetTime and call setCurrentTime(targetTime); update the logic in handleSkip
(referencing audioRef, handleSkip, setCurrentTime and your duration state
variable) to use safeDuration and set the UI time from targetTime rather than
relying on audio.currentTime after a failing assignment.
- Around line 195-203: The AnimatePresence is inside AudioPlayer so it unmounts
with the component and prevents the exit animation from running; move/keep a
parent AnimatePresence mounted while closing and make the sliding motion.div the
direct child keyed by the active chapter ID so exit can run. Concretely: ensure
the parent component (e.g., LearningPath or a persistent wrapper around
AudioPlayer) renders AnimatePresence persistently, and inside AudioPlayer render
only the motion.div as the direct child of that AnimatePresence with
key={activeChapter?.id} (or similar) and exit={{ y: "100%" }} so the slide-out
animation can play before the component is removed. Ensure the motion.div
remains the immediate child controlled by AnimatePresence and is keyed by
activeChapter.id.
In `@src/components/audiobook/constants.ts`:
- Line 31: The duration formatting is inconsistent: when seconds exist it
returns the short form `${mins}m ${secs}s` but when seconds are zero it returns
the long form `${mins} min`; update the formatter to use a single, consistent
style (pick one). For example, change the return logic in the formatter (the
line using mins and secs) so it always uses the short form (`${mins}m` when secs
=== 0, otherwise `${mins}m ${secs}s`) or always uses the long form (`${mins}
min` when secs === 0, otherwise `${mins} min ${secs} sec`) — apply the chosen
style consistently where mins and secs are used.
In `@src/components/audiobook/RoadmapNode.tsx`:
- Around line 31-36: The computed resumePct can exceed 100 when
chapter.progress.current_seconds > chapter.duration_seconds; update the
calculation in RoadmapNode (the resumePct assignment) to clamp the result
between 0 and 100 — compute the percentage as currently done using
chapter.progress.current_seconds and chapter.duration_seconds, then apply
Math.min(100, Math.max(0, computedPct)) (or equivalent) so resumePct never goes
above 100 or below 0 while preserving the existing isAvailable and
duration_seconds checks.
In `@src/hooks/useAudiobooks.ts`:
- Around line 32-38: The query currently uses an empty-string fallback for id
which registers keys like ['audiobook','roadmap',''] and calls
getAudiobookRoadmap(''); update useAudiobookRoadmap so the queryKey omits or
marks the id when undefined (e.g. use id ? AUDIOBOOK_QUERY_KEYS.roadmap(id) :
['audiobook','roadmap','disabled'] or only include the id element when present)
and make queryFn guarded so it only calls getAudiobookRoadmap(id) when id is
defined (return null otherwise); adjust useAudiobookRoadmap,
AUDIOBOOK_QUERY_KEYS.roadmap and the queryFn references to avoid registering
keys with an empty string.
---
Nitpick comments:
In `@services/audiobookService.ts`:
- Around line 32-62: The two API functions have inconsistent error handling:
getAudiobooks currently swallows all errors and returns an empty array while
getAudiobookRoadmap only swallows 404s and rethrows others; standardize by
letting errors bubble up so React Query can handle them—remove or simplify the
try/catch in getAudiobooks (or rethrow after logging) and keep the existing
404-to-null behavior in getAudiobookRoadmap (only catch APIError with statusCode
404 and return null, otherwise rethrow); ensure you reference APIError,
getAudiobooks, and getAudiobookRoadmap when making the change so logging remains
informative but errors are not silently swallowed.
In `@src/components/audiobook/AudioPlayer.tsx`:
- Around line 76-81: Auto-saves every SAVE_INTERVAL_MS cause useSaveProgress's
onSuccess to always invalidate AUDIOBOOK_QUERY_KEYS.roadmap, triggering frequent
refetches; to fix, change the auto-save call site (where
onProgressSaveRef.current is invoked) to pass an "incremental" or
"skipInvalidate" flag and initialize lastSaveRef.current = Date.now() so the
first timeupdate doesn't immediately save, and update useSaveProgress to respect
that flag: only run
queryClient.invalidateQueries(AUDIOBOOK_QUERY_KEYS.roadmap(audiobookId)) on
final/completion saves (or when flag is false). Also consider making
SAVE_INTERVAL_MS configurable or increasing it, but the main fix is to add and
honor the skip-invalidation flag in onProgressSaveRef/useSaveProgress to avoid
roadmap refetches during continuous playback.
- Around line 169-175: The handleClose function currently calls onProgressSave
and then calls onClose which triggers the component unmount effect that also
saves progress, causing a duplicate save; remove the onProgressSave call from
handleClose so it only pauses playback (using audioRef.current.pause()) and
calls onClose, and rely on the existing unmount effect cleanup that invokes
onProgressSave for the chapter to perform the single save. Ensure audioRef and
chapter.id are still available to the unmount effect and do not delete that
effect.
In `@src/hooks/useAudiobooks.ts`:
- Around line 44-65: The useSaveProgress hook currently treats audiobookId as
optional which prevents automatic roadmap cache invalidation when omitted;
either make audiobookId required on the hook signature (change
useSaveProgress(audiobookId?: string) to useSaveProgress(audiobookId: string))
so callers must pass it and the onSuccess always calls
queryClient.invalidateQueries({ queryKey:
AUDIOBOOK_QUERY_KEYS.roadmap(audiobookId) }), or if you want to keep it optional
add a clear JSDoc comment above useSaveProgress explaining that passing
audiobookId is required for automatic UI updates (invalidate of
AUDIOBOOK_QUERY_KEYS.roadmap), and consider adding a runtime warning
(console.warn) when onSuccess runs without audiobookId to help callers notice
missing parameter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 46d5a159-5db2-4d44-a8e0-1eca6fe1a08f
📒 Files selected for processing (20)
backend/src/controllers/audiobookController.jsbackend/src/routes/audiobookRoutes.jsbackend/src/routes/index.jsbackend/src/services/audiobookService.jsbackend/supabase/migrations/002_audiobooks_schema.sqlbackend/supabase/migrations/003_seed_audiobooks_fundamentals.sqlbackend/supabase/migrations/004_seed_audiobooks_beginners.sqlservices/audiobookService.tsservices/config.tssrc/App.tsxsrc/components/Layout.tsxsrc/components/audiobook/AudioPlayer.tsxsrc/components/audiobook/AudiobookHeader.tsxsrc/components/audiobook/RoadmapNode.tsxsrc/components/audiobook/RoadmapTrack.tsxsrc/components/audiobook/constants.tssrc/hooks/useAudiobooks.tssrc/pages/Audiobooks.tsxsrc/pages/LearningPath.tsxtypes.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
15 issues found across 20 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/pages/Audiobooks.tsx">
<violation number="1" location="src/pages/Audiobooks.tsx:15">
P2: Missing error-state handling for `useAudiobooks()` masks fetch failures as an empty catalog.</violation>
</file>
<file name="src/components/audiobook/AudioPlayer.tsx">
<violation number="1" location="src/components/audiobook/AudioPlayer.tsx:131">
P2: `isPlaying` state can desync from actual audio playback when `audio.play()` fails</violation>
<violation number="2" location="src/components/audiobook/AudioPlayer.tsx:169">
P2: Duplicate progress save when closing the player: `handleClose` explicitly saves progress and then calls `onClose()`, but the unmount cleanup `useEffect` saves the same progress again when the component unmounts. This produces duplicate API requests and potential race conditions.</violation>
</file>
<file name="services/audiobookService.ts">
<violation number="1" location="services/audiobookService.ts:41">
P2: Returning `[]` on error swallows failures silently — React Query cannot distinguish "no audiobooks exist" from "the API is down", so users see an empty state instead of an error message. Let the error propagate and handle it at the query layer.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Member
|
Please include demo/visuals of the changes that you made @parthdude07 |
Contributor
Author
Screencast.from.2026-06-03.14-01-36.webm |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
backend/src/services/audiobookService.js (2)
79-80:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid logging raw client user IDs at info level.
Line 80 and Line 101 emit the stable anonymous UUID directly. That's still a persistent user identifier, so this creates an unnecessary tracking trail for every roadmap fetch/save. Log counts or a redacted/hash form instead.
Also applies to: 100-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/services/audiobookService.js` around lines 79 - 80, The logs in fetchUserProgress (and the related saveUserProgress calls) output raw stable user IDs; change those logger.info usages to avoid emitting the raw UUID by either logging a one-way hash (e.g., SHA-256/HMAC of userId) or a redacted form (e.g., keep only first/last chars) or only logging counts; update the logger call in fetchUserProgress and the analogous log call around saveUserProgress to compute the hash/redacted value before logging and include that in the message instead of the plain userId to prevent persistent identifier leakage.
103-110:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake progress updates monotonic.
Autosave and completion writes can arrive out of order. This UPSERT lets a later
completed=falseor smallercurrent_secondsoverwrite an earlier completion, which can relock later chapters on the next roadmap fetch. Preserve the max playback position and keepcompletedsticky once it becomes true.Proposed fix
ON CONFLICT (user_id, chapter_id) DO UPDATE SET - current_seconds = EXCLUDED.current_seconds, - completed = EXCLUDED.completed, + current_seconds = GREATEST(audiobooks.user_progress.current_seconds, EXCLUDED.current_seconds), + completed = audiobooks.user_progress.completed OR EXCLUDED.completed, updated_at = NOW()`,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/services/audiobookService.js` around lines 103 - 110, The UPSERT in audiobooks.user_progress currently allows out-of-order writes to lower current_seconds or revert completed to false; change the ON CONFLICT DO UPDATE to make progress monotonic by setting current_seconds = GREATEST(user_progress.current_seconds, EXCLUDED.current_seconds) and completed = user_progress.completed OR EXCLUDED.completed, and keep updated_at as needed; update the SQL in the INSERT query (the audiobooks.user_progress UPSERT block that mentions current_seconds and completed) to use GREATEST for position and OR to make completed sticky.src/components/audiobook/AudioPlayer.tsx (2)
82-92:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't let later progress saves undo completion.
After
onCompletefires, this component still callsonProgressSavefrom autosave, pause, and close.src/pages/LearningPath.tsx:39-49maps that callback to{ completed: false }, so a completed chapter can be written back as incomplete during the last 5% of playback or immediately when the player is closed.Suggested minimal fix
const now = Date.now(); - if (now - lastSaveRef.current >= SAVE_INTERVAL_MS) { + if (!isCompletedRef.current && now - lastSaveRef.current >= SAVE_INTERVAL_MS) { lastSaveRef.current = now; onProgressSaveRef.current(chapter.id, time); } @@ if (isPlaying) { audio.pause(); - onProgressSave(chapter.id, audio.currentTime); + if (!isCompletedRef.current) { + onProgressSave(chapter.id, audio.currentTime); + } } else { audio.play().catch(console.error); } @@ const handleClose = () => { if (audioRef.current) { audioRef.current.pause(); - onProgressSave(chapter.id, audioRef.current.currentTime); + if (!isCompletedRef.current) { + onProgressSave(chapter.id, audioRef.current.currentTime); + } } onClose(); };Also applies to: 151-154, 195-199
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/audiobook/AudioPlayer.tsx` around lines 82 - 92, The autosave/pause/close logic calls onProgressSaveRef.current even after onCompleteRef.current has been fired, which can overwrite a completed state; update every place that calls onProgressSaveRef.current(chapter.id, time) (including the autosave block using lastSaveRef, the pause handler, and the close/unmount save) to first check isCompletedRef.current and skip calling onProgressSaveRef.current if true, ensuring setIsCompleted(true) / onCompleteRef.current(chapter.id) are not undone.
45-60:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon’t reset on
chapter.idchange inAudioPlayer: it’s already keyed; fix 0-progress and status syncing
src/pages/LearningPath.tsxrenders<AudioPlayer key={activeChapter.id} ... />, soAudioPlayerremounts whenchapter.idchanges—previous chapter state won’t carry over, and the proposed “local reset” forchapter.idisn’t necessary.Remaining issues in
src/components/audiobook/AudioPlayer.tsx:
handleLoadedonly resumes whenchapter.progress?.current_secondsis truthy, so a saved/resumed value of0won’t set/clearcurrentTime(lines ~69-73).isCompleted/isCompletedRefare initialized from the initialchapter.statusbut are never synced whenchapter.statuschanges while the samechapter.idis mounted (lines ~45-60); this can desync UI/auto-complete behavior from updated server state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/audiobook/AudioPlayer.tsx` around lines 45 - 60, The component incorrectly treats a saved progress of 0 as "no progress" and never updates completion state when chapter.status changes; update handleLoaded to treat chapter.progress?.current_seconds !== undefined (or use null check) instead of truthiness so a 0 value still calls setCurrentTime(0) and updates playback/resume logic, and add a useEffect that watches chapter.status to call setIsCompleted(chapter.status === "completed") and update isCompletedRef.current accordingly (so isCompleted and isCompletedRef stay in sync when the same AudioPlayer instance receives a new chapter.status); do not add any manual reset on chapter.id since the parent keys the component.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/controllers/audiobookController.js`:
- Around line 122-137: saveProgress currently accepts completed=true without
verifying the chapter is playable or prerequisites are met; before calling
audiobookService.upsertProgress, fetch the chapter state and validate its
playability/prerequisites for this user (e.g., via
audiobookService.getChapter/getChapterById and a new or existing
audiobookService.arePrerequisitesMet(user_id, chapter_id) or
isChapterPlayable(user_id, chapter_id)); if the chapter is locked or coming_soon
or prerequisites are not satisfied, reject the request (respond 400/validation
error) or coerce completed to false and only allow upsert of progress without
marking complete; ensure these checks happen inside saveProgress and reference
audiobookService.upsertProgress, saveProgress, isUUID, MAX_SECONDS when locating
where to add the logic.
In `@backend/src/services/supabaseService.js`:
- Around line 24-38: The remaining transcript queries in searchTranscripts() and
fetchTranscriptMeta() still select legacy columns from the transcripts table;
update those queries to pull normalized fields from the joined content_items
alias (c) instead—replace occurrences of transcripts.title,
transcripts.summary/description, transcripts.conference,
transcripts.channel_name, transcripts.url, etc. with c.title, c.description AS
summary, c.source_metadata->>'conference' AS conference,
c.source_metadata->>'channel_name' AS channel_name, c.url AS loc (and ensure
ARRAY[]::text[] placeholders for speakers/tags/topics/categories remain if
needed) and keep the JOIN on content_item_id consistent with the other
normalized query; verify the selected column aliases match the list/detail
responses so searchTranscripts() and fetchTranscriptMeta() return the same
schema as the main transcripts query.
In `@src/components/audiobook/AudioPlayer.tsx`:
- Around line 118-145: The cleanup currently reads audioRef.current during
unmount which can be nullified before the effect cleanup runs; fix by tracking
the last playback time in a stable variable updated from the audio element
(e.g., add a 'timeupdate' listener that writes currentTime into a local
lastKnownTime variable or ref) and then, in the useEffect cleanup, use that
lastKnownTime and isCompletedRef.current (not audioRef.current) to build the
payload and call navigator.sendBeacon / fetch as before; attach and remove the
event listener inside the same useEffect so the logic lives with the cleanup for
audioRef/audio element changes.
---
Outside diff comments:
In `@backend/src/services/audiobookService.js`:
- Around line 79-80: The logs in fetchUserProgress (and the related
saveUserProgress calls) output raw stable user IDs; change those logger.info
usages to avoid emitting the raw UUID by either logging a one-way hash (e.g.,
SHA-256/HMAC of userId) or a redacted form (e.g., keep only first/last chars) or
only logging counts; update the logger call in fetchUserProgress and the
analogous log call around saveUserProgress to compute the hash/redacted value
before logging and include that in the message instead of the plain userId to
prevent persistent identifier leakage.
- Around line 103-110: The UPSERT in audiobooks.user_progress currently allows
out-of-order writes to lower current_seconds or revert completed to false;
change the ON CONFLICT DO UPDATE to make progress monotonic by setting
current_seconds = GREATEST(user_progress.current_seconds,
EXCLUDED.current_seconds) and completed = user_progress.completed OR
EXCLUDED.completed, and keep updated_at as needed; update the SQL in the INSERT
query (the audiobooks.user_progress UPSERT block that mentions current_seconds
and completed) to use GREATEST for position and OR to make completed sticky.
In `@src/components/audiobook/AudioPlayer.tsx`:
- Around line 82-92: The autosave/pause/close logic calls
onProgressSaveRef.current even after onCompleteRef.current has been fired, which
can overwrite a completed state; update every place that calls
onProgressSaveRef.current(chapter.id, time) (including the autosave block using
lastSaveRef, the pause handler, and the close/unmount save) to first check
isCompletedRef.current and skip calling onProgressSaveRef.current if true,
ensuring setIsCompleted(true) / onCompleteRef.current(chapter.id) are not
undone.
- Around line 45-60: The component incorrectly treats a saved progress of 0 as
"no progress" and never updates completion state when chapter.status changes;
update handleLoaded to treat chapter.progress?.current_seconds !== undefined (or
use null check) instead of truthiness so a 0 value still calls setCurrentTime(0)
and updates playback/resume logic, and add a useEffect that watches
chapter.status to call setIsCompleted(chapter.status === "completed") and update
isCompletedRef.current accordingly (so isCompleted and isCompletedRef stay in
sync when the same AudioPlayer instance receives a new chapter.status); do not
add any manual reset on chapter.id since the parent keys the component.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f4ae5aa5-ff1c-4a34-859d-abeaa6c18444
📒 Files selected for processing (12)
backend/src/config/index.jsbackend/src/controllers/audiobookController.jsbackend/src/services/audiobookService.jsbackend/src/services/dbPool.jsbackend/src/services/supabaseService.jsbackend/supabase/migrations/004_seed_audiobooks_beginners.sqlservices/audiobookService.tssrc/components/audiobook/AudioPlayer.tsxsrc/components/audiobook/RoadmapNode.tsxsrc/components/audiobook/constants.tssrc/hooks/useAudiobooks.tssrc/pages/LearningPath.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/audiobook/constants.ts
- services/audiobookService.ts
- src/pages/LearningPath.tsx
- src/hooks/useAudiobooks.ts
- src/components/audiobook/RoadmapNode.tsx
There was a problem hiding this comment.
3 issues found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/pages/Audiobooks.tsx">
<violation number="1" location="src/pages/Audiobooks.tsx:15">
P2: Missing error-state handling for `useAudiobooks()` masks fetch failures as an empty catalog.</violation>
</file>
<file name="src/components/audiobook/AudioPlayer.tsx">
<violation number="1" location="src/components/audiobook/AudioPlayer.tsx:169">
P2: Duplicate progress save when closing the player: `handleClose` explicitly saves progress and then calls `onClose()`, but the unmount cleanup `useEffect` saves the same progress again when the component unmounts. This produces duplicate API requests and potential race conditions.</violation>
</file>
<file name="services/audiobookService.ts">
<violation number="1" location="services/audiobookService.ts:41">
P2: Returning `[]` on error swallows failures silently — React Query cannot distinguish "no audiobooks exist" from "the API is down", so users see an empty state instead of an error message. Let the error propagate and handle it at the query layer.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/pages/Audiobooks.tsx">
<violation number="1" location="src/pages/Audiobooks.tsx:15">
P2: Missing error-state handling for `useAudiobooks()` masks fetch failures as an empty catalog.</violation>
</file>
<file name="src/components/audiobook/AudioPlayer.tsx">
<violation number="1" location="src/components/audiobook/AudioPlayer.tsx:169">
P2: Duplicate progress save when closing the player: `handleClose` explicitly saves progress and then calls `onClose()`, but the unmount cleanup `useEffect` saves the same progress again when the component unmounts. This produces duplicate API requests and potential race conditions.</violation>
</file>
<file name="services/audiobookService.ts">
<violation number="1" location="services/audiobookService.ts:41">
P2: Returning `[]` on error swallows failures silently — React Query cannot distinguish "no audiobooks exist" from "the API is down", so users see an empty state instead of an error message. Let the error propagate and handle it at the query layer.</violation>
</file>
<file name="backend/src/services/dbPool.js">
<violation number="1" location="backend/src/services/dbPool.js:33">
P1: Non-production environments disable SSL entirely instead of using SSL without certificate verification, which may break connections to SSL-required databases and reduce transport security in shared networks.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
Author
|
@staru09 please review the PR . few links are null for now which will show "comming soon". i will update those as i generate the audiobooks . |
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
services/audiobookService.ts (2)
84-87:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t convert API failures into success-like fallback values.
These catch blocks currently hide request failures (
saveProgressresolves even on write failure; playlist reads return empty/null on network/5xx). That prevents proper error states/retries and can produce misleading UI.🔧 Proposed fix
export const saveProgress = async ( chapterId: string, currentSeconds: number, completed: boolean ): Promise<void> => { const userId = getUserId(); try { await api.post(config.endpoints.audiobookProgress, { user_id: userId, chapter_id: chapterId, current_seconds: currentSeconds, completed, }); } catch (error) { - // Don't throw — progress saves should not break the user experience console.error('Error saving progress:', error); + throw error; } }; @@ export const getPlaylists = async (params?: { @@ } catch (error) { console.error('Error fetching playlists:', error); - return { data: [], total: 0 }; + throw error; } }; @@ export const getPlaylistBySlug = async (slug: string): Promise<AudioPlaylist | null> => { @@ } catch (error) { - console.error(`Error fetching playlist ${slug}:`, error); - return null; + if (error instanceof APIError && error.statusCode === 404) return null; + console.error(`Error fetching playlist ${slug}:`, error); + throw error; } }; @@ export const getEpisodeById = async (episodeId: string): Promise<AudioEpisode | null> => { @@ } catch (error) { - console.error(`Error fetching episode ${episodeId}:`, error); - return null; + if (error instanceof APIError && error.statusCode === 404) return null; + console.error(`Error fetching episode ${episodeId}:`, error); + throw error; } };Also applies to: 109-112, 121-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/audiobookService.ts` around lines 84 - 87, The catch blocks in the audiobookService functions (e.g., saveProgress and the playlist read functions) are swallowing errors and returning success-like fallbacks; change them to surface failures instead: remove silent console-only catches and either rethrow the caught error or return a rejected Promise with the error (or return an explicit failure result object) so callers can handle retries/UI error state; keep any non-throwing cleanup logic but ensure the error is propagated from saveProgress and the playlist read functions rather than resolving with an empty/null/success value.
18-24:⚠️ Potential issue | 🟠 MajorValidate the persisted
bitscribe_user_idbefore reusing it.A malformed/stale value in
localStoragewill be sent asuser_idinPOST /api/v1/audiobooks/progress, where the controller rejects non-UUID values (400 VALIDATION_ERROR). ForGET /api/v1/audiobooks/:id/roadmap, an invalidx-user-idis treated asnulland progress is just omitted, not rejected.🔧 Proposed fix
const USER_ID_KEY = 'bitscribe_user_id'; +const UUID_REGEX = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; export const getUserId = (): string => { let userId = localStorage.getItem(USER_ID_KEY); - if (!userId) { + if (!userId || !UUID_REGEX.test(userId)) { userId = crypto.randomUUID(); localStorage.setItem(USER_ID_KEY, userId); } return userId; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/audiobookService.ts` around lines 18 - 24, getUserId currently reuses any value found under USER_ID_KEY without validating it, which can send malformed IDs to the backend; update getUserId to validate the stored value as a UUID (e.g., with a UUID v4 regex or a small isValidUUID helper) and if validation fails generate a new id via crypto.randomUUID(), persist it with localStorage.setItem(USER_ID_KEY, id), and return the new id; keep the same behavior when no value exists. Ensure the changes are made in the getUserId function and that USER_ID_KEY usage remains unchanged.
♻️ Duplicate comments (1)
backend/src/controllers/audiobookController.js (1)
121-143:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftLocked-chapter progression rules are still missing.
buildRoadmapChapters()can never returnlocked, so later playable chapters are always exposed asavailable.saveProgress()also only verifies that the chapter exists before acceptingcompleted=true, which still allows out-of-order completion for later chapters. Restore the prerequisite walk here and reuse the same rule when persisting completion. This still diverges from the PR’s stated locked/available/completed roadmap behavior and server-side completion checks.Also applies to: 220-228
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/audiobookController.js` around lines 121 - 143, buildRoadmapChapters currently never returns "locked"; update buildRoadmapChapters to enforce sequential prerequisites by walking chapters in order and tracking whether all previous chapters are completed (or are coming_soon/unavailable) and set status="locked" for any chapter that comes after the first not-yet-completed prerequisite, while preserving existing "coming_soon" and "completed" logic; likewise update saveProgress to re-check the same prerequisite rule before accepting completed=true (reject or return an error if an attempt is made to mark a chapter completed while an earlier prerequisite chapter is incomplete) so both buildRoadmapChapters and saveProgress use the same sequence-based lock rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/services/audiobookService.js`:
- Around line 123-128: The progress map is being keyed by a non-existent
episode_id, so resume state never attaches; update the mapping returned by
fetchEpisodeProgress to use the correct key by changing progressMap construction
to Object.fromEntries(progressRows.map(p => [p.chapter_id, p])) (or
alternatively change the SQL in fetchEpisodeProgress to alias chapter_id AS
episode_id), ensuring the code that sets ep.user_progress (in the playlist
hydration loop) looks up progressMap[ep.id] after this fix.
- Around line 341-349: The INSERT ... ON CONFLICT DO UPDATE currently overwrites
the stored completed flag with whatever the latest request sends; change the DO
UPDATE so it preserves any prior completion (i.e. make completion monotonic). In
the query call that inserts/updates progress (the query(...) invocation updating
audiobooks.user_progress), modify the DO UPDATE clause to set completed =
audiobooks.user_progress.completed OR EXCLUDED.completed (so an earlier true
cannot be cleared by a later false), leaving current_seconds and updated_at
behavior unchanged.
- Around line 34-79: listPlaylists (and the other public lookup helpers in this
module such as getPlaylistBySlug, getPlaylistByUUID, listEpisodes,
getEpisodeBySlug) must default to only returning published rows when no explicit
status is passed; change the filter-building logic in these functions so that if
status is undefined/null you push 'published' into values and add a status = $N
condition (rather than allowing all statuses). Also update getEpisodeById to
ensure the parent playlist is published before returning an episode (either by
joining audio_playlists and adding playlist.status = 'published' to the query or
by an extra query that verifies the parent playlist’s status and returns
null/not-found for non-published parents). Ensure parameter indexing uses
values.length consistently when adding the defaulted status value.
In `@backend/supabase/migrations/001_audiobook_schema.sql`:
- Around line 183-187: The RLS policy "Allow public read access on all episodes"
currently uses USING (true) and must be changed to restrict anon reads to
episodes whose parent playlist is published; update the policy on
public.audio_episodes to reference public.audio_playlists (e.g., JOIN on
audio_playlists.id = audio_episodes.playlist_id) and replace USING (true) with a
predicate that verifies the joined playlist is published (for example USING
(EXISTS (SELECT 1 FROM public.audio_playlists p WHERE p.id =
public.audio_episodes.playlist_id AND p.status = 'published')) or an equivalent
boolean check against the playlist's published flag). Ensure the DROP/CREATE
policy block keeps the same name "Allow public read access on all episodes" but
uses this joined predicate so unpublished/draft/archived playlists' episodes are
not readable via the anon key.
- Around line 140-157: sync_playlist_counters currently only recomputes counters
for v_playlist_id derived from NEW on UPDATE, so when an episode is moved the
OLD playlist is stale; modify the trigger logic in sync_playlist_counters (the
block that sets v_playlist_id and runs the UPDATE on public.audio_playlists) to
detect UPDATE operations where OLD.playlist_id <> NEW.playlist_id and refresh
both playlists — e.g. compute both ids (v_old_playlist_id := OLD.playlist_id,
v_new_playlist_id := NEW.playlist_id) and run the UPDATE statement for both ids
(either run the same UPDATE twice with v_playlist_id = v_old_playlist_id and
v_new_playlist_id, or run a single UPDATE with WHERE id IN (v_old_playlist_id,
v_new_playlist_id)), keeping the same SELECT COUNT/SUM expressions and
updated_at behavior.
- Around line 239-253: The INSERT is writing a JSONB literal into the tags
column but public.audio_playlists.tags is TEXT[], so replace the JSONB literal
with a text array literal: use ARRAY[]::text[] (or ARRAY['tag1','tag2']::text[]
when non-empty) in the SELECT that fills tags (the INSERT/SELECT that references
s.id, s.title, public.slugify(s.title), ...), and make the same change in the
other migration that uses the same JSONB shape so all backfills consistently
write ARRAY[]::text[] to public.audio_playlists.tags.
In `@services/audiobookService.ts`:
- Around line 104-105: The pagination check wrongly uses truthy checks so
explicit zero values (e.g., offset: 0) are skipped; update the conditions around
the query.append calls for limit and offset to check for undefined/null instead
of truthiness (e.g., use params?.limit !== undefined / params?.offset !==
undefined or nullish checks) so zero values are preserved, then call toString()
as before when appending to query.
In `@src/components/audiobook/AudiobookHeader.tsx`:
- Around line 15-17: The progress percent calculation treats "coming_soon"
chapters as completable; change the denominator to count only playable chapters
by replacing totalCount with a playableCount that excludes chapters with status
=== "coming_soon" (e.g. const playableCount = chapters.filter(c => c.status !==
"coming_soon").length) and compute progressPct as playableCount > 0 ?
Math.round((completedCount / playableCount) * 100) : 0 so users can reach 100%
when all playable chapters are completed (leave completedCount as-is, since it
already checks status === "completed").
In `@src/pages/Audiobooks.tsx`:
- Around line 16-17: Call usePlaylists with explicit filters so only audiobook
playlists are returned: update the call to usePlaylists to pass { playlist_type:
"audiobook", status: "published" } (or whatever canonical published/active
status the service uses) and remove or tighten any downstream filtering; adjust
the audiobooks assignment (const audiobooks = response?.data || []) if needed to
rely on the filtered response from usePlaylists().
In `@src/pages/LearningPath.tsx`:
- Around line 22-39: The mapper in the useMemo for roadmap is inferring chapter
availability from audio_url which allows locked chapters to appear playable;
instead read and use the server-provided roadmap status field (e.g.,
ep.roadmap_status or ep.status — whichever the API provides) to set
RoadmapChapter['status'] (falling back to 'coming_soon' if the server value is
missing), and update any related logic (notably handleSelectChapter) to check
that server status (locked/available/completed) rather than audio_url; also
apply this same replacement at the other mapping site mentioned (lines 76-79) so
both mapper and selection logic rely on the server roadmap status.
In `@types.ts`:
- Line 139: The RoadmapChapter.status union in types.ts is missing the 'locked'
state; update the RoadmapChapter.status type definition (the status line) to
include 'locked' so it becomes 'available' | 'completed' | 'coming_soon' |
'locked' to restore frontend/backend contract compatibility for roadmap
rendering logic.
---
Outside diff comments:
In `@services/audiobookService.ts`:
- Around line 84-87: The catch blocks in the audiobookService functions (e.g.,
saveProgress and the playlist read functions) are swallowing errors and
returning success-like fallbacks; change them to surface failures instead:
remove silent console-only catches and either rethrow the caught error or return
a rejected Promise with the error (or return an explicit failure result object)
so callers can handle retries/UI error state; keep any non-throwing cleanup
logic but ensure the error is propagated from saveProgress and the playlist read
functions rather than resolving with an empty/null/success value.
- Around line 18-24: getUserId currently reuses any value found under
USER_ID_KEY without validating it, which can send malformed IDs to the backend;
update getUserId to validate the stored value as a UUID (e.g., with a UUID v4
regex or a small isValidUUID helper) and if validation fails generate a new id
via crypto.randomUUID(), persist it with localStorage.setItem(USER_ID_KEY, id),
and return the new id; keep the same behavior when no value exists. Ensure the
changes are made in the getUserId function and that USER_ID_KEY usage remains
unchanged.
---
Duplicate comments:
In `@backend/src/controllers/audiobookController.js`:
- Around line 121-143: buildRoadmapChapters currently never returns "locked";
update buildRoadmapChapters to enforce sequential prerequisites by walking
chapters in order and tracking whether all previous chapters are completed (or
are coming_soon/unavailable) and set status="locked" for any chapter that comes
after the first not-yet-completed prerequisite, while preserving existing
"coming_soon" and "completed" logic; likewise update saveProgress to re-check
the same prerequisite rule before accepting completed=true (reject or return an
error if an attempt is made to mark a chapter completed while an earlier
prerequisite chapter is incomplete) so both buildRoadmapChapters and
saveProgress use the same sequence-based lock rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 82c5b96f-244e-4825-b4ec-114f8b76e05f
📒 Files selected for processing (20)
backend/src/app.jsbackend/src/controllers/audiobookController.jsbackend/src/routes/audiobookRoutes.jsbackend/src/routes/index.jsbackend/src/services/audiobookService.jsbackend/src/services/dbPool.jsbackend/src/services/supabaseService.jsbackend/supabase/migrations/001_audiobook_schema.sqlbackend/supabase/migrations/002_audiobook_seed.sqlservices/audiobookService.tsservices/config.tssrc/App.tsxsrc/components/audiobook/AudioPlayer.tsxsrc/components/audiobook/AudiobookHeader.tsxsrc/components/audiobook/RoadmapNode.tsxsrc/components/audiobook/topicUtils.tssrc/hooks/useAudiobooks.tssrc/pages/Audiobooks.tsxsrc/pages/LearningPath.tsxtypes.ts
✅ Files skipped from review due to trivial changes (1)
- backend/src/app.js
🚧 Files skipped from review as they are similar to previous changes (5)
- backend/src/routes/index.js
- services/config.ts
- src/App.tsx
- backend/src/services/dbPool.js
- src/components/audiobook/AudioPlayer.tsx
There was a problem hiding this comment.
12 issues found across 21 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/pages/Audiobooks.tsx">
<violation number="1" location="src/pages/Audiobooks.tsx:15">
P2: Missing error-state handling for `useAudiobooks()` masks fetch failures as an empty catalog.</violation>
</file>
<file name="src/components/audiobook/AudioPlayer.tsx">
<violation number="1" location="src/components/audiobook/AudioPlayer.tsx:169">
P2: Duplicate progress save when closing the player: `handleClose` explicitly saves progress and then calls `onClose()`, but the unmount cleanup `useEffect` saves the same progress again when the component unmounts. This produces duplicate API requests and potential race conditions.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/audiobook/topicUtils.ts (1)
240-274:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffDocumented priority doesn't match implementation logic.
The comment (lines 234–238) states priority 1 is "Explicit difficulty_level from DB (if not null/default)," but the implementation only trusts non-
'beginner'values from the DB (line 242). When the DB explicitly stores'beginner', haystack keyword signals can override it (lines 253–265), and the DB value is only respected as a fallback if no signals match (line 268).If
'beginner'is intentionally treated as the "default" and thus deprioritized, clarify this in the comment. Otherwise, enforce true priority-1 behavior by returning early for any explicit DB value.📝 Suggested comment clarification
/** * Infer difficulty level from playlist metadata. * * Priority: - * 1. Explicit difficulty_level from DB (if not null/default) + * 1. Explicit difficulty_level from DB (if 'intermediate' or 'advanced') * 2. Tag-based detection (tags often contain 'beginner', 'advanced', etc.) * 3. Title + description keyword matching - * 4. Fallback: 'beginner' + * 4. DB 'beginner' value (if set and no signals found) + * 5. Fallback: 'beginner' */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/audiobook/topicUtils.ts` around lines 240 - 274, The inferDifficulty function's documented priority contradicts its implementation. The comments state that explicit difficulty_level from the DB is priority 1, but the code at line 242 only checks for non-'beginner' values, allowing haystack keyword signals to override an explicitly stored 'beginner' value from the database. Either clarify the comment to explain that 'beginner' is intentionally treated as a default value and thus deprioritized below keyword signals, OR fix the logic to respect any explicit DB value (regardless of whether it is 'beginner' or another level) by removing the 'beginner' exclusion condition and returning the DB value early before checking keyword signals. Choose the approach that matches the intended behavior and update accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/audiobook/topicUtils.ts`:
- Around line 240-274: The inferDifficulty function's documented priority
contradicts its implementation. The comments state that explicit
difficulty_level from the DB is priority 1, but the code at line 242 only checks
for non-'beginner' values, allowing haystack keyword signals to override an
explicitly stored 'beginner' value from the database. Either clarify the comment
to explain that 'beginner' is intentionally treated as a default value and thus
deprioritized below keyword signals, OR fix the logic to respect any explicit DB
value (regardless of whether it is 'beginner' or another level) by removing the
'beginner' exclusion condition and returning the DB value early before checking
keyword signals. Choose the approach that matches the intended behavior and
update accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 93055431-127a-4a55-a18e-812db8b929da
📒 Files selected for processing (10)
backend/src/controllers/audiobookController.jsbackend/src/services/audiobookService.jsbackend/supabase/migrations/001_audiobook_schema.sqlbackend/supabase/migrations/002_audiobook_seed.sqlservices/audiobookService.tssrc/components/audiobook/AudioPlayer.tsxsrc/components/audiobook/AudiobookHeader.tsxsrc/components/audiobook/topicUtils.tssrc/pages/Audiobooks.tsxtypes.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/components/audiobook/AudiobookHeader.tsx
- types.ts
- src/pages/Audiobooks.tsx
- backend/supabase/migrations/002_audiobook_seed.sql
- services/audiobookService.ts
- backend/supabase/migrations/001_audiobook_schema.sql
- src/components/audiobook/AudioPlayer.tsx
There was a problem hiding this comment.
2 issues found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/pages/Audiobooks.tsx">
<violation number="1" location="src/pages/Audiobooks.tsx:15">
P2: Missing error-state handling for `useAudiobooks()` masks fetch failures as an empty catalog.</violation>
</file>
<file name="src/components/audiobook/AudioPlayer.tsx">
<violation number="1" location="src/components/audiobook/AudioPlayer.tsx:169">
P2: Duplicate progress save when closing the player: `handleClose` explicitly saves progress and then calls `onClose()`, but the unmount cleanup `useEffect` saves the same progress again when the component unmounts. This produces duplicate API requests and potential race conditions.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
backend/src/controllers/audiobookController.js (1)
220-227:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftEnforce progression rules before accepting
completed=true.The existing validation remains insufficient:
saveProgressaccepts any validchapter_idandcompletedpair without verifying that the chapter is unlocked or that prerequisites are met. A client can directly mark locked or "coming soon" chapters as complete, bypassing the sequential progression model enforced by the roadmap UI.Validate server-side that the chapter is playable and prerequisites are satisfied before accepting completion.
🔒 Verification script to check for prerequisite validation logic
#!/bin/bash # Description: Search for prerequisite or playability checks in audiobookService # Check if upsertProgress or related functions validate chapter state rg -nC5 'upsertProgress|arePrerequisitesMet|isChapterPlayable' backend/src/services/audiobookService.js # Check for any validation of chapter locks or prerequisites rg -nC3 'locked|prerequisite|coming.?soon' backend/src/services/audiobookService.js🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/controllers/audiobookController.js` around lines 220 - 227, The upsertProgress method call currently lacks validation to ensure the chapter is playable and prerequisites are met before accepting completion. Add server-side validation logic before the audiobookService.upsertProgress call that checks whether the chapter is playable (not locked or marked as coming soon) and verifies that all prerequisite chapters have been completed. Only allow the upsertProgress call to proceed if isCompleted is false or if the chapter is unlocked and all prerequisites are satisfied; otherwise, throw an appropriate APIError indicating that the chapter is not yet available or prerequisites are not met.src/components/audiobook/AudiobookHeader.tsx (1)
17-17:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftProgress calculation still includes "coming soon" chapters in denominator.
This issue was flagged in a previous review and remains unresolved. When
totalCountincludes chapters withstatus === "coming_soon"or missingaudio_url, users cannot reach 100% completion even after finishing all playable content.Downstream impact: Lines 86 and 94 display
{totalCount} Chaptersand{completedCount}/{totalCount} Completed, which would need updating for consistency if the denominator is adjusted to count only playable chapters.Suggested fix
const completedCount = chapters.filter((c) => c.status === "completed").length; - const totalCount = chapters.length; - const progressPct = totalCount > 0 ? Math.round((completedCount / totalCount) * 100) : 0; + const totalCount = chapters.length; + const playableCount = chapters.filter((c) => c.status !== "coming_soon" && c.audio_url).length; + const progressPct = playableCount > 0 ? Math.round((completedCount / playableCount) * 100) : 0;Then update the stats display for consistency:
<CheckCircle className="w-3.5 h-3.5" /> - {completedCount}/{totalCount} Completed + {completedCount}/{playableCount} Completed🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/audiobook/AudiobookHeader.tsx` at line 17, The progressPct calculation on line 17 uses totalCount which includes chapters with "coming_soon" status or missing audio_url, preventing users from reaching 100% completion. Filter totalCount to only include playable chapters by excluding entries where status equals "coming_soon" or audio_url is missing, then use this filtered count in the progressPct calculation. Additionally, update the chapter count displays on lines 86 and 94 to use the same filtered playable chapter count for consistency so users see accurate completion statistics based only on the content they can actually listen to.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/config/index.js`:
- Line 65: The rejectUnauthorized configuration in the database connection
defaults to false (insecure), which disables certificate validation by default
and enables MITM attacks in production. Invert the logic so that certificate
rejection defaults to true and is only disabled when DB_REJECT_UNAUTHORIZED is
explicitly set to the string 'false' for development/testing scenarios, ensuring
secure defaults for production environments.
---
Duplicate comments:
In `@backend/src/controllers/audiobookController.js`:
- Around line 220-227: The upsertProgress method call currently lacks validation
to ensure the chapter is playable and prerequisites are met before accepting
completion. Add server-side validation logic before the
audiobookService.upsertProgress call that checks whether the chapter is playable
(not locked or marked as coming soon) and verifies that all prerequisite
chapters have been completed. Only allow the upsertProgress call to proceed if
isCompleted is false or if the chapter is unlocked and all prerequisites are
satisfied; otherwise, throw an appropriate APIError indicating that the chapter
is not yet available or prerequisites are not met.
In `@src/components/audiobook/AudiobookHeader.tsx`:
- Line 17: The progressPct calculation on line 17 uses totalCount which includes
chapters with "coming_soon" status or missing audio_url, preventing users from
reaching 100% completion. Filter totalCount to only include playable chapters by
excluding entries where status equals "coming_soon" or audio_url is missing,
then use this filtered count in the progressPct calculation. Additionally,
update the chapter count displays on lines 86 and 94 to use the same filtered
playable chapter count for consistency so users see accurate completion
statistics based only on the content they can actually listen to.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 680f5e49-8cc2-4f6b-bc6a-918f27571a22
📒 Files selected for processing (11)
backend/src/config/index.jsbackend/src/controllers/audiobookController.jsbackend/src/services/audiobookService.jsbackend/src/services/dbPool.jsbackend/supabase/migrations/001_audiobook_schema.sqlsrc/components/audiobook/AudioPlayer.tsxsrc/components/audiobook/AudiobookHeader.tsxsrc/components/audiobook/topicUtils.tssrc/hooks/useAudiobooks.tssrc/pages/LearningPath.tsxtypes.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- backend/src/services/dbPool.js
- src/components/audiobook/topicUtils.ts
- types.ts
- backend/supabase/migrations/001_audiobook_schema.sql
- backend/src/services/audiobookService.js
- src/pages/LearningPath.tsx
- src/hooks/useAudiobooks.ts
- src/components/audiobook/AudioPlayer.tsx
There was a problem hiding this comment.
3 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/pages/Audiobooks.tsx">
<violation number="1" location="src/pages/Audiobooks.tsx:15">
P2: Missing error-state handling for `useAudiobooks()` masks fetch failures as an empty catalog.</violation>
</file>
<file name="src/components/audiobook/AudioPlayer.tsx">
<violation number="1" location="src/components/audiobook/AudioPlayer.tsx:169">
P2: Duplicate progress save when closing the player: `handleClose` explicitly saves progress and then calls `onClose()`, but the unmount cleanup `useEffect` saves the same progress again when the component unmounts. This produces duplicate API requests and potential race conditions.</violation>
</file>
<file name="backend/src/services/audiobookService.js">
<violation number="1" location="backend/src/services/audiobookService.js:7">
P2: Cross-file schema documentation inconsistency: controller still documents `public.audio_user_progress` while service uses `audiobooks.user_progress`</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/pages/Audiobooks.tsx">
<violation number="1" location="src/pages/Audiobooks.tsx:15">
P2: Missing error-state handling for `useAudiobooks()` masks fetch failures as an empty catalog.</violation>
</file>
<file name="src/components/audiobook/AudioPlayer.tsx">
<violation number="1" location="src/components/audiobook/AudioPlayer.tsx:169">
P2: Duplicate progress save when closing the player: `handleClose` explicitly saves progress and then calls `onClose()`, but the unmount cleanup `useEffect` saves the same progress again when the component unmounts. This produces duplicate API requests and potential race conditions.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
Author
|
@Sansh2356 this is also ready to be merge |
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
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.
resolves #17
Summary by cubic
Adds audiobook learning paths with a unified playlists/episodes schema, REST APIs, and a UI for browsing, listening, and progress tracking. Anonymous progress persists via a stable ID in
x-user-id; autosaves reliably; chapters unlock in order, and missing-audio chapters show “coming soon” and can’t be completed. Fulfills #17.New Features
/api/v1/audiobooks/playlists, GET/api/v1/audiobooks/playlists/:slug, GET/api/v1/audiobooks/episodes/:episode_id, GET/api/v1/audiobooks, GET/api/v1/audiobooks/:id/roadmap(merges progress viax-user-id), POST/api/v1/audiobooks/progress. Playlists supportstatus,playlist_type,source, and pagination.current_seconds, max 24h per episode, UUID/slug checks, block completion for locked or missing-audio./audiobooks(browse with topic-aware images) and/learning-path/:slug(roadmap + player); player supports speed control, skip, resume, and transcript summary; dynamic difficulty from tags; top nav link added.public.audio_playlists,public.audio_episodes, andaudiobooks.user_progress; shared PostgreSQL pool (dbPool.js) used across services;supabaseServicenow uses the pool.localStoragesent viax-user-idon GET anduser_idin POST; CORS allowsx-user-id.Migration
DATABASE_URLand run Supabase migrations001–002.DB_REJECT_UNAUTHORIZEDas needed for Postgres connections.Written for commit fa23c16. Summary will update on new commits.
Summary by CodeRabbit