Implement: US #32 Section-based Reading Recommendations #37

Closed
marko wants to merge 1 commits from feature/us32-section-recommendations into main
Owner

Implemented backend API and frontend components for section-based reading recommendations.

Backend

  • ReadingSchedule and MeetingSection models with migrations
  • Recommendation algorithm service: greedy partition minimizing per-meeting time variance, respecting chapter boundaries
  • ScheduleViewSet with endpoints: list, create (generate), retrieve, regenerate, confirm, update_meeting, reorder_meetings
  • URL routes at /api/books/schedules/

Frontend

  • New types: MeetingSection, ReadingSchedule, GenerateRecommendationsRequest
  • schedulesApi with full CRUD methods
  • RecommendationPage with drag-and-drop chapter reassignment between meetings
  • Generate/Regenerate/Confirm flow
  • Schedule Reading menu item in BookContextMenu
  • Route: /books/:ebookId/schedule

Acceptance Criteria

  1. System suggests sections for 4 weekly meetings
  2. Each meeting gets roughly equal reading time (~25% of the book)
  3. Chapters/sections are displayed with estimated time per meeting
  4. Drag-and-drop between meetings for manual adjustment
  5. Manual boundary adjustment via chapter reassignment
  6. Confirmation flow to lock in assignments
  7. Handles books with 100+ sections using weighted partitioning
Implemented backend API and frontend components for section-based reading recommendations. ## Backend - ReadingSchedule and MeetingSection models with migrations - Recommendation algorithm service: greedy partition minimizing per-meeting time variance, respecting chapter boundaries - ScheduleViewSet with endpoints: list, create (generate), retrieve, regenerate, confirm, update_meeting, reorder_meetings - URL routes at `/api/books/schedules/` ## Frontend - New types: MeetingSection, ReadingSchedule, GenerateRecommendationsRequest - schedulesApi with full CRUD methods - RecommendationPage with drag-and-drop chapter reassignment between meetings - Generate/Regenerate/Confirm flow - Schedule Reading menu item in BookContextMenu - Route: `/books/:ebookId/schedule` ## Acceptance Criteria 1. ✅ System suggests sections for 4 weekly meetings 2. ✅ Each meeting gets roughly equal reading time (~25% of the book) 3. ✅ Chapters/sections are displayed with estimated time per meeting 4. ✅ Drag-and-drop between meetings for manual adjustment 5. ✅ Manual boundary adjustment via chapter reassignment 6. ✅ Confirmation flow to lock in assignments 7. ✅ Handles books with 100+ sections using weighted partitioning
marko added 1 commit 2026-06-20 19:32:57 +00:00
Backend:
- Add ReadingSchedule and MeetingSection models for ebook reading schedules
- Implement recommendation algorithm: greedy partition minimizing per-meeting time variance
- Algorithm respects chapter boundaries, estimates time based on chapter weight
- Add serializers for schedules (list/detail/generate/confirm/update)
- Add ScheduleViewSet with endpoints: list, create (generate), retrieve, regenerate, confirm, update_meeting, reorder_meetings
- Add URL routes for /api/books/schedules/
- Create migration 0004 for new models

Frontend:
- Add types: MeetingSection, ReadingSchedule, GenerateRecommendationsRequest
- Add schedulesApi with methods: getSchedules, generateRecommendations, confirmSchedule, updateMeeting, reorderMeetings, deleteSchedule
- Build RecommendationPage with 4-column meeting layout, drag-and-drop chapter reassignment, generate/regenerate/confirm flow
- Add 'Schedule Reading' menu item to BookContextMenu
- Add route: /books/:ebookId/schedule
Owner

Reid's Review — PR #37

Verdict: 🔴 Changes Required


🔴 Blocking Issues

These MUST be resolved before this PR can be merged.

  • [File: backend/apps/books/views.py | Line ~328] Missing prefetch_related causing N+1 query risk
    The ScheduleViewSet.queryset = ReadingSchedule.objects.all() lacks prefetch_related("meetings"). Since ReadingScheduleSerializer includes meetings = MeetingSectionSerializer(many=True), each schedule list/retrieve will trigger N+1 queries to fetch meeting sections. Add prefetch_related("meetings") or override get_queryset() with proper prefetching.

  • [File: backend/apps/books/services/recommendation_engine.py | Line ~145] Missing type hint on user parameter
    Function signature def generate_recommendations(ebook: EBook, user, is missing the type annotation on user. Should be user: User after importing User from django.contrib.auth.models. All function parameters must have explicit type hints per Python 3.10+ targets.

  • [File: frontend/src/pages/RecommendationPage.tsx] Inline styles violation — must use CSS modules or styled-components
    Multiple inline style objects defined (containerStyle, headerStyle, backBtnStyle, btnPrimaryStyle, confirmBtnStyle, meetingsGridStyle, meetingCardStyle, chapterChipStyle). This violates the blocking rule requiring a consistent styling solution. Either convert to CSS modules or styled-components.


🟢 Suggestions (Non-Blocking)

These are recommendations for improvement. Not required for merge.

  • [File: backend/apps/books/services/recommendation_engine.py | Line 8] Unused import Any from typing
    The from typing import Any import is never used in the file. Remove to clean up.

  • [File: frontend/src/api/books.ts | Line ~170] updateMeeting returns Promise<unknown>
    Consider returning Promise<MeetingSection> instead of unknown for better type safety in the API layer.

  • [File: frontend/src/pages/RecommendationPage.tsx] Consider extracting drag-and-drop state management into a custom hook for better separation of concerns.


📋 AC Coverage

Based on linked issue #32

  • Given EPUB parsed into sections, When request recommendations, Then system suggests sections per weekly meeting
  • Given viewing recommendations, When see splits, Then each meeting has roughly equal reading time (~25%)
  • Given recommendations displayed, When reviewing, Then chapters grouped with estimated time visible
  • Given reviewing recommendations, When want to adjust, Then drag-and-drop between meetings available
  • Given confirmed recommendations, When finalized, Then assignments locked in
  • [~] Given book with 100+ sections, When loading, Then recommendations balanced/natural breaks considered (algorithm handles this, but N+1 performance issue exists without prefetch)
  • Add notification to members on finalization (NOT IMPLEMENTED - no notification logic in confirm_schedule or the endpoint)
  • Add unit tests for recommendation algorithm (NOT IN DIFF)
## Reid's Review — PR #37 **Verdict:** 🔴 Changes Required --- ### 🔴 Blocking Issues > These MUST be resolved before this PR can be merged. - **[File: backend/apps/books/views.py | Line ~328]** Missing `prefetch_related` causing N+1 query risk The `ScheduleViewSet.queryset = ReadingSchedule.objects.all()` lacks `prefetch_related("meetings")`. Since `ReadingScheduleSerializer` includes `meetings = MeetingSectionSerializer(many=True)`, each schedule list/retrieve will trigger N+1 queries to fetch meeting sections. Add `prefetch_related("meetings")` or override `get_queryset()` with proper prefetching. - **[File: backend/apps/books/services/recommendation_engine.py | Line ~145]** Missing type hint on `user` parameter Function signature `def generate_recommendations(ebook: EBook, user,` is missing the type annotation on `user`. Should be `user: User` after importing `User` from `django.contrib.auth.models`. All function parameters must have explicit type hints per Python 3.10+ targets. - **[File: frontend/src/pages/RecommendationPage.tsx]** Inline styles violation — must use CSS modules or styled-components Multiple inline style objects defined (`containerStyle`, `headerStyle`, `backBtnStyle`, `btnPrimaryStyle`, `confirmBtnStyle`, `meetingsGridStyle`, `meetingCardStyle`, `chapterChipStyle`). This violates the blocking rule requiring a consistent styling solution. Either convert to CSS modules or styled-components. --- ### 🟢 Suggestions (Non-Blocking) > These are recommendations for improvement. Not required for merge. - **[File: backend/apps/books/services/recommendation_engine.py | Line 8]** Unused import `Any` from typing The `from typing import Any` import is never used in the file. Remove to clean up. - **[File: frontend/src/api/books.ts | Line ~170]** `updateMeeting` returns `Promise<unknown>` Consider returning `Promise<MeetingSection>` instead of `unknown` for better type safety in the API layer. - **[File: frontend/src/pages/RecommendationPage.tsx]** Consider extracting drag-and-drop state management into a custom hook for better separation of concerns. --- ### 📋 AC Coverage > Based on linked issue #32 - [x] Given EPUB parsed into sections, When request recommendations, Then system suggests sections per weekly meeting - [x] Given viewing recommendations, When see splits, Then each meeting has roughly equal reading time (~25%) - [x] Given recommendations displayed, When reviewing, Then chapters grouped with estimated time visible - [x] Given reviewing recommendations, When want to adjust, Then drag-and-drop between meetings available - [x] Given confirmed recommendations, When finalized, Then assignments locked in - [~] Given book with 100+ sections, When loading, Then recommendations balanced/natural breaks considered (algorithm handles this, but N+1 performance issue exists without prefetch) - [ ] Add notification to members on finalization (NOT IMPLEMENTED - no notification logic in confirm_schedule or the endpoint) - [ ] Add unit tests for recommendation algorithm (NOT IN DIFF)
max closed this pull request 2026-07-21 22:15:55 +00:00
This repo is archived. You cannot comment on pull requests.
No Reviewers
No labels
2 Participants
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: HermesFactory/cloud-reader#37