Skip to content

Release v0.6.9#420

Open
masonfox wants to merge 382 commits into
mainfrom
develop
Open

Release v0.6.9#420
masonfox wants to merge 382 commits into
mainfrom
develop

Conversation

@masonfox

Copy link
Copy Markdown
Owner

Release v0.6.9

This release includes two important improvements to the Tome application.


Features & Fixes

1. Tap & Hold to Select Shelf Items (#415)

Implements a long-press gesture to enter select mode on shelf items, eliminating the need to scroll to the top to press the "Select" button.

Key Features:

  • Long-press (500ms) on book card body enters select mode and auto-selects the item
  • Separate touch targets: drag handle (200ms) vs card body (500ms) to avoid conflicts
  • 10px movement tolerance to prevent activation during scrolling
  • Mobile-focused UX improvement with keyboard accessibility and screen reader support

Mobile UX Benefits:

Before: User must scroll to top → press "Select" button → scroll back down → select items

After: User long-presses on any item → immediately in select mode with that item selected → continue selecting

Changes:

  • Added hooks/useLongPress.ts - Reusable long-press detection hook with comprehensive test coverage (21 tests)
  • Extended hooks/useBookListView.ts with enterSelectModeWithSelection(bookId) function
  • Updated components/Books/BookListItem.tsx to support long-press activation with keyboard and ARIA support
  • Modified components/Books/DraggableBookList.tsx and app/shelves/[id]/page.tsx to wire up functionality

Resolves: #414


2. Fix: DNF Books Not Appearing in Library Filter (#418)

Fixed critical bug where books with DNF (Did Not Finish) status were not showing up when filtering by status on the library page.

Root Cause:
The book repository was incorrectly treating DNF status like active statuses (reading, to-read, read-next) by requiring is_active = 1. However, DNF is a terminal/completed state like "read", where sessions are archived with is_active = 0.

Before: DNF filter returned 0 books despite 3 DNF books existing in database
After: DNF filter correctly returns all 3 DNF books

Changes:

  • Updated lib/repositories/book.repository.ts to treat 'dnf' status like 'read' status (terminal states)
  • Added comprehensive test coverage with 9 new tests in __tests__/integration/repositories/books/book-status-filtering.test.ts

Impact:

  • Users can now filter for DNF books on the library page
  • Fixes incorrect behavior where terminal states were treated as active states
  • No breaking changes to other status filters

Testing

  • All 4,034 tests passing
  • Added 21 new tests for useLongPress hook
  • Added 9 new tests for status filtering
  • Verified via API: /api/books?status=dnf returns correct results
  • Manual testing confirms all gestures and accessibility features work correctly

Files Changed

  • .opencode/package-lock.json - Updated OpenCode dependencies
  • __tests__/hooks/useLongPress.test.ts - New: Comprehensive hook tests
  • __tests__/integration/repositories/books/book-status-filtering.test.ts - New: Status filtering tests
  • app/shelves/[id]/page.tsx - Wire up long-press functionality
  • components/Books/BookListItem.tsx - Long-press gesture support with accessibility
  • components/Books/DraggableBookList.tsx - Pass long-press handlers
  • hooks/useBookListView.ts - Add select mode entry function
  • hooks/useLongPress.ts - New: Reusable long-press hook
  • lib/repositories/book.repository.ts - Fix DNF status filtering logic
  • package.json - Version bump to 0.6.9

Total: 10 files changed, 1,289 insertions(+), 6 deletions(-)

masonfox and others added 30 commits January 3, 2026 20:07
## Summary

- Adds a "Check All" / "Uncheck All" button when filtering tags via
search query
- Button layout splits 2/3 "Select Multiple" and 1/3 "Check All" when
actively filtering
- Check All button adds remaining filtered tags to existing selections
(allows building selections across queries)
- Button text dynamically toggles between "Check All" and "Uncheck All"
based on current state
- Auto-enters checkbox mode when Check All is clicked
- Responsive design: shows "All" on small screens, full text on larger
screens

## Changes

- Updated `TagList` component to conditionally render split buttons when
search query is active
- Added `allFilteredChecked` computed value to track if all filtered
tags are currently selected
- Added `handleToggleAllFiltered` function to toggle all filtered tags
on/off
- Import `CheckCheck` icon from lucide-react for the new button
- Maintains existing functionality when not filtering (single "Select
Multiple" button)

## Testing

- Manually tested filtering + check all functionality
- Tested responsive behavior on different screen sizes
- Verified selection building across multiple queries
- Confirmed auto-enter checkbox mode behavior
## Summary

- Upgrades Next.js from 14 to 16.1.1 and React from 18 to 19
- Migrates all route handlers to use async params API (Next.js 15+
requirement)
- Converts next.config.js to TypeScript and removes deprecated options
- Adds Turbopack support and fixes CSS import order
- Updates React 19 type compatibility for refs

This major framework upgrade modernizes the application with the latest
stable versions, enabling improved performance, Turbopack bundling, and
access to new React 19 features.

Closes #192

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
## Summary

Fixes production error where Next.js fails to load the pino logger
module during the instrumentation phase with error: `EISDIR reading
"/app/.next/node_modules/pino-28069d5257187539"`

## Changes

- **lib/logger.ts**: Changed pino initialization to lazy-load at runtime
instead of at module import time
- **next.config.ts**: Added pino and pino-pretty to
serverExternalPackages and webpack externals to prevent bundling

## Technical Details

Next.js 16 with Turbopack has issues bundling certain Node.js packages
during the instrumentation phase. This fix:
1. Uses `require('pino')` inside a lazy initialization function instead
of top-level import
2. Marks pino packages as external so they're not bundled by
Next.js/Turbopack
3. Maintains full logging functionality - logger initializes on first
use

## Testing

- Build completes successfully without pino module errors
- Logging functionality preserved at runtime
…ck bundling error

- Replace pino logger with console.log in instrumentation.ts
- Prevents Turbopack from trying to bundle pino during instrumentation phase
- Resolves EISDIR error: BuildMessage: EISDIR reading pino module
- Maintains structured logging for API routes and application code
…g error

Root cause: Turbopack performs static analysis at build time and attempts to
bundle pino-pretty when it encounters the string literal 'pino-pretty', even
inside conditional blocks that evaluate to false at runtime.

Changes:
- Remove pino-pretty transport configuration from logger.ts
- Logs now output as structured JSON (pino's default format)
- Multi-stream logging (stdout + file) preserved and unaffected
- For pretty logs in dev, pipe through pino-pretty externally

This fixes the production error:
  Failed to load external module pino-28069d5257187539:
  BuildMessage: EISDIR reading "/app/.next/node_modules/pino-28069d5257187539"
…tion phase loading

Critical fix: lib/db/calibre.ts had module-level code (lines 9-12) that called
getLogger() immediately when the module was imported. This caused pino to load
during the instrumentation phase, triggering Turbopack bundling errors.

The import chain was:
  instrumentation.ts → sync-service.ts → calibre.ts → logger → pino

Changes:
- Move logger call from module-level into getCalibreDB() function
- Add hasLoggedWarning flag to log warning only once
- Warning now logs when function is called, not at module load time

This completes the fix for:
  Failed to load external module pino-28069d5257187539:
  BuildMessage: EISDIR reading "/app/.next/node_modules/pino-28069d5257187539"
…g during instrumentation

The instrumentation hook was failing in production with EISDIR error because:
- lib/db/sqlite.ts had module-level getLogger() call (line 6)
- lib/db/migrate.ts had module-level getLogger() call (line 11)
- lib/db/seeders/index.ts had module-level getLogger() call (line 22)

Import chain during instrumentation phase:
instrumentation.ts → sync-service.ts → repositories → lib/db/sqlite.ts → lib/logger.ts → pino

This caused Turbopack to attempt bundling pino during the instrumentation phase,
creating a symlink that was incorrectly read as a directory.

Solution: Replace all module-level logger initialization with lazy getLoggerSafe()
functions that only load pino when actually called, not at import time.

Files modified:
- lib/db/sqlite.ts: Replaced const logger with lazy getLoggerSafe()
- lib/db/migrate.ts: Replaced const logger with lazy getLoggerSafe()
- lib/db/seeders/index.ts: Replaced const logger with lazy getLoggerSafe()

This completes the fix chain from commits 224d4a6, 3587203, 433d498, and 118f6dd.
…tibility issue

Next.js 16 Turbopack has compatibility issues with pino logger when running
with Bun runtime, causing 'Failed to load external module pino' EISDIR errors
during instrumentation phase.

Root cause: Turbopack cannot handle pino's dynamic worker thread requires.
Upstream issue: vercel/next.js#86099 (still open)

Changes:
- package.json: Add --webpack flag to dev and build scripts
- tailwind.config.ts: Convert require() to ESM import for webpack compatibility
- docs/TURBOPACK_PINO_WORKAROUND.md: Document issue and future migration plan

This is a temporary workaround. Phase 2 will migrate to Consola logger which
has native Turbopack compatibility and is used by Vercel internally.

Testing:
✅ Development server starts without errors
✅ Production build completes successfully
✅ Production server runs without pino/instrumentation errors
discovered that, in production, these fonts are not loading and applying
and we're falling back to serif. Maintain serif, for now.
## Summary

- For libraries with 150k books, for example, we reduce database queries
from ~600,000 to ~50-100 via batch processing (99.9% reduction)
- Implement chunked processing for constant memory usage (~50MB)
regardless of library size
- Add comprehensive hierarchical logging with progress tracking, ETAs,
and visual separators
- **Result**: 150k books sync in 3-10 minutes (10-30x faster) instead of
hours, using ~50MB constant memory

## Changes

### Phase 1: Batch Processing
- **lib/db/calibre.ts**: Add `getAllBookTags()` for bulk tag fetching (1
query vs 150k queries)
- **lib/repositories/book.repository.ts**: Add `findAllByCalibreIds()`
and `bulkUpsert()` for batch operations
- **lib/repositories/session.repository.ts**: Add `bulkCreate()` for
batch session creation
- **lib/sync-service.ts**: Refactor with batch processing, add
`detectOrphans` option

### Phase 2: Chunked Processing
- **lib/db/calibre.ts**: Add pagination support (`getBooksCount()`,
`LIMIT/OFFSET` in `getAllBooks()`)
- **lib/sync-service.ts**: Implement chunked processing (default: 5000
books/chunk) with enhanced logging
- **__tests__/lib/sync-service.test.ts**: Add 8 comprehensive tests for
both phases

## Performance Impact

| Metric | Before | After | Improvement |
|--------|--------|-------|-------------|
| Queries | ~600k | ~50-100 | 99.9% reduction |
| Memory | 300-750MB | ~50MB | 85% reduction |
| Time (150k) | Hours | 3-10 min | **10-30x faster** |

## Testing

✅ All 1971 tests pass (1966 existing + 5 new)
✅ 83.77% line coverage maintained
✅ Zero breaking changes - full backward compatibility

## Configuration

```typescript
syncCalibreLibrary(calibreSource, {
  detectOrphans: true,  // Default: true
  chunkSize: 5000,      // Default: 5000
});
```

## Related

Fixes #204
## Summary

Complete migration of the test suite from Bun Test to Vitest for
improved cross-platform compatibility and broader ecosystem support.

## Key Changes

### Test Framework Migration
- ✅ Migrated all 103 test files from Bun Test to Vitest
- ✅ Replaced `bun:sqlite` with `better-sqlite3` for cross-platform
SQLite compatibility
- ✅ Updated all imports: `bun:test` → `vitest`
- ✅ Updated all mock patterns: `mock.module()` → `vi.mock()` with
`vi.hoisted()`
- ✅ Fixed timezone handling with `TZ=UTC` environment variable

### Test Results
- **103/103 test files passing** ✅
- **1,998 tests passing** ✅
- **0 failures** 🎉
- **Test duration**: ~14 seconds

### Documentation Updates
- Updated `docs/TESTING_GUIDELINES.md` to v2.0.0 with comprehensive
Vitest patterns
- Added "Vitest-Specific Patterns" section covering:
  - Mock hoisting with `vi.hoisted()`
  - better-sqlite3 API differences
  - Async test patterns
  - Environment variable handling
- Removed Bun-specific troubleshooting (93 lines)
- Updated all code examples to use Vitest syntax

### CI/CD Updates
- Updated GitHub workflows (pr.yml, nightly.yml, publish.yml)
- All workflows now use `npm run test:coverage`
- Automatic `TZ=UTC` handling via package.json scripts
- Consistent `NODE_ENV=test` across all CI environments

### Configuration
- Updated npm scripts in package.json
- All test commands now include `TZ=UTC` prefix
- Test setup migrated from `bunfig.toml` to `vitest.config.ts`

## Technical Highlights

### Calibre Test Migration
The most complex part was migrating Calibre database tests:
- Converted `lib/db/calibre.ts` to use lazy environment loading
- Added `resetCalibreDB()` for test cleanup
- Migrated 111 tests from `bun:sqlite` to `better-sqlite3`
- Fixed API differences (exec vs run, undefined vs null)

### Mock Pattern Improvements
- Implemented `vi.hoisted()` pattern for shared mock state
- Service layer mocking now properly isolated
- Better test isolation with explicit mock cleanup

### Cross-Platform Compatibility
- Tests now run in Node.js (not Bun-specific)
- Compatible with standard CI/CD platforms
- Better ecosystem support (Vitest has broader adoption)

## Breaking Changes

⚠️ **None for end users** - This is purely a test infrastructure change.

For developers:
- Test commands changed from `bun test` to `npm test`
- Must use `vi.mock()` instead of `mock.module()` for new tests
- Must use `vi.hoisted()` for module-level mock variables

## Files Modified

### Code (8 files)
- `lib/db/calibre.ts` - Lazy env loading + reset function
- `__tests__/lib/calibre.test.ts` - Environment setup
- `__tests__/lib/calibre-write.test.ts` - better-sqlite3 migration
- `__tests__/api/*.test.ts` - vi.hoisted() patterns (5 files)

### Documentation (2 files)
- `docs/TESTING_GUIDELINES.md` - Comprehensive v2.0.0 update
- `README.md` - Updated test command

### CI/CD (3 files)
- `.github/workflows/pr.yml` - Use npm scripts
- `.github/workflows/nightly.yml` - Use npm scripts
- `.github/workflows/publish.yml` - Use npm scripts

### Configuration (1 file)
- `package.json` - Updated test scripts with TZ=UTC

## Testing

Local verification:
```bash
npm test                    # ✅ 103/103 passing
npm run test:watch          # ✅ Works
npm run test:coverage       # ✅ Works
npm run test:ui             # ✅ Works
```

## Checklist

- [x] All tests passing (103/103)
- [x] Documentation updated
- [x] CI/CD workflows updated
- [x] NPM scripts configured
- [x] README updated
- [x] No breaking changes for end users
- [x] Cross-platform compatibility verified

## Migration Motivation

### Why Vitest?
1. **Cross-platform** - Runs on Node.js, not Bun-specific
2. **Ecosystem** - Better tooling and IDE support
3. **Compatibility** - Works with standard CI/CD platforms
4. **Maturity** - Stable, well-documented, widely adopted
5. **Performance** - Fast test execution (~14s for 1,998 tests)

### Why Now?
- Bun Test is still evolving with breaking changes
- Vitest has reached stability (v1.0+)
- Better long-term maintainability
- Easier onboarding for new contributors

## References

- [Vitest Documentation](https://vitest.dev)
- [better-sqlite3
Documentation](https://github.com/WiseLibs/better-sqlite3)
- `docs/TESTING_GUIDELINES.md` - Updated testing patterns

---

**Ready for review!** All tests passing, documentation complete, CI/CD
configured. 🚀

Closes #186

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
## Summary

Completes the migration to a single Node.js runtime by removing all Bun
dependencies and dual-runtime detection logic. This follows PR #229
which migrated tests from Bun Test to Vitest.

## Changes

### Infrastructure
- **Dockerfile**: Switched from `oven/bun:1.3.0` to `node:22-alpine`,
added build dependencies for better-sqlite3 native compilation
- **docker-entrypoint.sh**: Changed `bun run` to `node` commands  
- **package.json**: Updated all scripts from `bun run`/`bunx` to
`node`/`npx`

### Database Layer Simplification
- **lib/db/factory.ts**: Removed `detectRuntime()` function and
dual-runtime logic, now exclusively uses better-sqlite3
- **lib/db/migrate.ts**: Removed `getMigrator()` function and
Bun-specific migration path
- **lib/db/sqlite.ts**: Removed runtime detection, simplified to always
use `process.env.NODE_ENV` instead of `Bun.env.BUN_ENV`
- **lib/db/calibre.ts**: Removed runtime-specific logging
- **lib/db/calibre-write.ts**: Removed runtime-specific logging  
- **lib/db/preflight-checks.ts**: Simplified disk space check, removed
runtime branching

### Scripts
- **scripts/fix-orphan-migration.ts**: Converted from `bun:sqlite` to
better-sqlite3, replaced `Bun.file()` with `fs.readFileSync()`
- **scripts/seed.ts & sync-calibre.ts**: Changed shebang from
`#!/usr/bin/env bun` to `#!/usr/bin/env node`

### Configuration
- **instrumentation.ts**: Removed runtime detection message
- **next.config.ts**: Removed `bun:sqlite` from serverExternalPackages
- **__tests__/helpers/db-setup.ts**: Removed runtime detection logic

## Rationale

**Why Node.js everywhere?**
- Single runtime simplifies deployment and operations
- Everyone already has Node.js installed
- Better ecosystem support and tooling
- Eliminates dual-runtime maintenance burden

**Performance impact?**
- Negligible (~10-20ms difference for typical operations)
- For a self-hosted reading tracker, this is acceptable
- Simplicity and maintainability outweigh minor performance differences

**Docker impact?**
- Image size: +30MB (for better-sqlite3 build dependencies)
- Build time: +30s (for native module compilation)
- Both acceptable trade-offs for operational simplicity

## Testing

- ✅ All 1999 tests passing (103 test files)
- ✅ Production build succeeds
- ✅ No runtime detection code remains in codebase
- ✅ Database operations work identically to before

## Migration Notes

- **No data migration needed** - SQLite format is unchanged
- **No breaking changes** - API contracts remain the same
- **Deployment**: Simply rebuild Docker image or reinstall dependencies

## Files Changed

15 files changed: 114 insertions(+), 201 deletions(-)

Key deletions:
- Removed ~90 lines of dual-runtime detection logic
- Removed Bun-specific code paths throughout database layer
- Net reduction in code complexity
masonfox and others added 28 commits February 23, 2026 18:14
Implements a two-agent review automation system:
- /review-loop: Automated iteration between @review agent and GitHub Copilot
- /review: Single-pass review for quick feedback
- /review-status: Check PR status without triggering reviews

Features:
- Max 3 iterations to prevent infinite loops
- Auto-implements recommended changes
- Runs tests after each change
- Both agents must approve for completion
- Full documentation and usage examples

Files added:
- .opencode/command/review-loop.md (main automation)
- .opencode/command/review.md (single review)
- .opencode/command/review-status.md (status checker)
- .opencode/command/README.md (command docs)
- docs/REVIEW_LOOP.md (workflow guide)

Updated:
- AGENTS.md (added review commands and docs reference)
## Summary

Fixes critical transaction handling bug in `shelf.repository.ts`
identified during review of PR #381.

## Changes

### 🔴 Critical Fix: Transaction Handling

**Problem**: The `moveBookToBottom` method in `shelf.repository.ts` was
not using the transaction parameter correctly:

```typescript
// Before (INCORRECT):
db.transaction(() => {
  db.update(bookShelves)  // ❌ Uses outer db instance
    .set({ sortOrder: sql`...` })
    .run();
});
```

**Impact**: Updates were not atomic, defeating the purpose of the
transaction and potentially causing race conditions.

**Fix**: Now correctly uses the transaction parameter:

```typescript
// After (CORRECT):
await db.transaction((tx) => {  // ✅ Uses tx parameter
  tx.update(bookShelves)        // ✅ Uses transaction instance
    .set({ sortOrder: sql`...` })
    .run();
});
```

### Additional Improvements

- Added `await` keyword for consistency with `session.repository.ts`
pattern
- Aligns transaction pattern across both repositories

## Testing

✅ All 3,891 tests pass
- 33 shelf repository tests
- 18 move-to-bottom API tests (sessions + shelves)
- 38 session repository tests
- All other test suites

## Related

- Fixes issue identified in PR #381 review
- Follows pattern established in `session.repository.ts:699`
- Maintains consistency across repository layer

## Review Notes

This change ensures atomic database updates when moving books to the
bottom of shelves. Without this fix, concurrent operations could result
in corrupted sort orders.
…384)

## Summary

Fixes critical optimistic update logic bugs in "Move to Bottom"
functionality identified by GitHub Copilot in PR #381.

The optimistic updates were causing UI flickers because they didn't
match the server-side logic, creating inconsistent cache state.

## Problem

Both `useShelfBooks` and `useReadNextBooks` hooks had optimistic updates
that reassigned **ALL** `sortOrder`/`readNextOrder` values sequentially
(0, 1, 2...), but the server-side logic in the repositories only:
- Decrements items **above** the moved item
- Sets the moved item to `maxOrder`
- Leaves items **below** unchanged

This mismatch between optimistic state and actual server response caused
visual flickers when the cache was invalidated and real data was
fetched.

## Changes

### 1. Fixed `hooks/useShelfBooks.ts` (moveToBottom mutation)
- Updated optimistic update to mirror server-side logic from
`shelf.repository.ts`
- Keep books below moved book unchanged
- Decrement `sortOrder` only for books above moved book
- Set moved book to `maxOrder`

### 2. Fixed `hooks/useReadNextBooks.ts` (moveToBottom mutation)
- Updated optimistic update to mirror server-side logic from
`session.repository.ts`
- Keep sessions below moved session unchanged
- Decrement `readNextOrder` only for sessions above moved session
- Set moved session to `maxOrder`

## Code Changes

**Before (incorrect):**
```typescript
// ❌ Reassigns ALL sortOrder values
const otherBooks = previousShelf.books.filter((b) => b.id !== bookId);
const optimisticBooks = [
  ...otherBooks.map((book, index) => ({
    ...book,
    sortOrder: index, // All books get new sequential values
  })),
  { ...bookToMove, sortOrder: maxOrder },
];
```

**After (correct):**
```typescript
// ✅ Only updates books above moved book
const optimisticBooks = previousShelf.books.map((book) => {
  if (book.id === bookId) {
    return { ...book, sortOrder: maxOrder };
  }

  const bookOrder = book.sortOrder ?? 0;
  if (bookOrder > currentOrder) {
    // Only decrement books above the moved book
    return { ...book, sortOrder: bookOrder - 1 };
  }

  // Books below keep their sortOrder unchanged
  return book;
});
```

## Testing

✅ All **3891 tests pass** with the new logic
- No test changes required (existing tests validate the behavior)
- Logic now matches server-side implementation exactly

## References

- Addresses GitHub Copilot PR review feedback on PR #381:
  - `hooks/useShelfBooks.ts:451`
  - `hooks/useReadNextBooks.ts:261`
- Original PR: #381 (Release v0.6.6)

## Impact

- **User Experience:** Eliminates UI flickers when moving books/sessions
to bottom
- **Correctness:** Optimistic updates now correctly predict server
response
- **Performance:** No performance changes (same number of operations)
- **Breaking Changes:** None

## Verification

To verify the fix manually:
1. Open a shelf or Read Next queue with multiple books/sessions
2. Move an item to the bottom
3. Observe smooth UI update without flicker
4. Check that all items retain correct order after cache refresh

---------

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
useDropdownPosition now reads menuRef.current.offsetWidth at calculation
time, falling back to the configured menuWidth option (default 192) only
when the element is not measurable. Prevents silent positioning bugs if
menu CSS width changes.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Resolves Copilot review comments from PR #381.

### Changes
- **JSDoc**: Added documentation to `ShelfService.moveBookToBottom` to
match `moveBookToTop` pattern
- **Performance**: Replaced O(n) `findIndex`/`find` per row with
pre-computed `Map` lookup in read-next page (eliminates O(n^2) render
cost)
- **Horizontal overflow**: Clamped dropdown `left` position within
viewport bounds in `useDropdownPosition` hook
- **Test corrections**: Fixed swapped test names in
`useDropdownPosition` edge case tests ("position above" vs "position
below" were inverted)

### Notes
- Copilot comments 1 & 2 (optimistic update mismatch) were already
resolved in the current code
- All 3894 tests pass, build succeeds

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
#386)

`isAtTop`/`isAtBottom` flags in filtered/sorted views were computed from
the filtered list index rather than the book's actual position in the
underlying data, causing "Move to Top/Bottom" actions to be incorrectly
disabled or enabled when a filter or non-default sort was active.

## Changes

- **`read-next` BookTable branch**: Use `entry.index` (from
`sessionLookup`) and `sessions.length - 1` instead of the filtered array
index
- **`shelves` mobile non-draggable list**: Use `book.sortOrder === 0` /
`book.sortOrder === books.length - 1` instead of filtered index
- **`shelves` desktop `BookTable` branch**: Same `sortOrder`-based check
for `isAtBottom`

```tsx
// Before — wrong when filter is active
isAtTop={index === 0}
isAtBottom={index === listView.filteredBooks.length - 1}

// After — reflects actual queue/shelf position
isAtTop={entry.index === 0}                        // read-next
isAtBottom={entry.index === sessions.length - 1}   // read-next

isAtTop={book.sortOrder === 0}                     // shelves
isAtBottom={book.sortOrder === books.length - 1}   // shelves
```

Draggable variants are unaffected — they only render when no filter is
applied, so index and position are equivalent there.

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in
our [2 minute survey](https://gh.io/copilot-coding-agent-survey).

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
## Summary

Fixes incorrect Move to Top/Bottom button states in shelf pages when
filtering or sorting is applied.

## Problem

In PR #381, GitHub Copilot identified that the shelf page was using
**reference equality** (`book === books[0]`) to determine if a book is
at the top/bottom of a shelf. This was updated to use ID-based
comparison (`book.id === books[0]?.id`), but this still had a
fundamental issue:

**The buttons were checking position in the filtered/sorted display
view, not the canonical shelf order.**

### Example Issue

Consider a shelf with books having sortOrders [0, 1, 2, 3, 4]:
- User sorts by title (alphabetically)
- Book with sortOrder=2 appears first in the display
- "Move to Top" button becomes disabled (incorrectly thinks it's already
at top)
- But the book isn't actually at sortOrder=0 in the canonical shelf
order

## Solution

Replace array position checks with **sortOrder-based comparison** using
a lookup map pattern:

1. **Calculate canonical position** from all books' sortOrder values:
   ```tsx
const minSortOrder = Math.min(...books.map(b => b.sortOrder ??
Infinity));
const maxSortOrder = Math.max(...books.map(b => b.sortOrder ??
-Infinity));
   ```

2. **Create lookup map** for O(1) access to each book's sortOrder:
   ```tsx
const sortOrderLookup = new Map(books.map(b => [b.id, b.sortOrder ??
Infinity]));
   ```

3. **Compare against canonical position** instead of display position:
   ```tsx
   isAtTop={sortOrderLookup.get(book.id) === minSortOrder}
   isAtBottom={sortOrderLookup.get(book.id) === maxSortOrder}
   ```

### Why This Works

- Move to Top/Bottom operations always work with **canonical shelf
order** (sortOrder field)
- The buttons now check against **canonical position**, matching the
operation behavior
- Filtering/sorting only affects display, not the underlying shelf order
- Buttons remain semantically correct regardless of current view
settings

## Benefits

- ✅ Buttons reflect **actual shelf position** (sortOrder), not display
position
- ✅ Correct behavior regardless of filtering/sorting
- ✅ Type-safe (avoids runtime property access issues)
- ✅ O(1) lookups after initial map creation
- ✅ Mirrors pattern used in `/read-next` page
- ✅ Semantically consistent with Move operations

## Changes

**File:** `app/shelves/[id]/page.tsx`

- Added canonical position calculation (minSortOrder/maxSortOrder)
- Created sortOrderLookup map for type-safe access
- Updated all 4 button locations to use canonical position checks:
  1. Mobile draggable list (line ~377-378)
  2. Mobile filtered list (line ~399-400)
  3. Desktop draggable table (line ~429-430)
  4. Desktop filtered table (line ~452-453)

## Testing

- ✅ All 3,894 tests pass
- ✅ Move to Top/Bottom buttons correctly disabled at extremes
- ✅ Buttons remain accurate when filtering/sorting applied
- ✅ Buttons now reflect canonical shelf order, not display order
- ✅ No performance regression (O(1) lookups)

## Related

- Addresses feedback from PR #381 review
- Uses lookup map pattern consistent with `app/read-next/page.tsx`
- Properly implements concept identified in original PR #381 discussion
## Summary

Fixes a bug where editing an existing review in the Session Edit modal
would not persist. User edits were silently discarded and the PATCH
request payload did not include the updated review value.

### Root Cause

The initialization effect included `draftReview` in its dependency
array, causing it to re-run whenever the user edited the review. This
created a loop:
1. User edits review → auto-save updates `draftReview`
2. `draftReview` change triggers init effect to re-run
3. Init effect resets review back to `currentReview` (original value)
4. User's edits are lost

### Solution

Refactored `SessionEditModal` to use the standard two-effect pattern
with `hasRestoredDraft` ref, aligning it with the pattern already used
by:
- `CompleteBookModal`
- `FinishBookModal`
- `DNFBookModal`
- `ProgressEditModal`

This standardizes draft management across all 5 modals that use the
`useDraftField` hook.

### Changes

- Added `hasRestoredDraft` ref to track draft restoration state
- Split mega-effect into separate initialization and draft restoration
effects
- Removed `draftReview` from initialization effect dependencies
- Updated comments to explain the pattern

### Testing

✅ All 3894 tests passing
✅ No regressions detected

### Files Changed

- `components/Modals/SessionEditModal.tsx` (42 lines changed)

### Related

- Plan document: `.opencode/plans/fix-session-edit-review-bug.md`
- User reported bug: review edits not saving in Session Edit modal
## Summary

Implements dynamic HTML page titles throughout the app to improve tab
distinguishability when multiple Tome tabs are open. All titles follow
the format `Tome - {detail}` where detail is context-specific.

**BONUS**: Also implements a centralized query key factory to fix
critical React Query bugs discovered during this work.

## Changes

### Dynamic Page Titles

#### New Hook
- **`lib/hooks/usePageTitle.ts`**: Reusable client-side hook for
managing page titles
  - Automatically prefixes with "Tome - "
  - Cleans up on unmount
  - Handles undefined/null gracefully

#### Static Page Titles (12 pages)
Updated all static pages with appropriate titles:
- Dashboard: `Tome - Dashboard`
- Library: `Tome - Library`
- Series List: `Tome - Series`
- Stats: `Tome - Stats`
- Journal: `Tome - Journal`
- Goals: `Tome - Goals`
- Read Next: `Tome - Read Next`
- Shelves: `Tome - Shelves`
- Tags: `Tome - Tags`
- Streak: `Tome - Streak`
- Settings: `Tome - Settings`
- Login: `Tome - Login`

#### Dynamic Page Titles (3 pages)
Updated all detail pages with data-driven titles:
- **Book Detail**: `Tome - {bookTitle} by {authors}`
  - Example: `Tome - The Fellowship of the Ring by J.R.R. Tolkien`
- **Series Detail**: `Tome - Series / {seriesName}`
  - Example: `Tome - Series / The Lord of the Rings`
- **Shelf Detail**: `Tome - Shelf / {shelfName}`
  - Example: `Tome - Shelf / My Favorites`

#### Implementation Details
- **All pages**: Use `usePageTitle()` hook
- **Note**: Goals, Streak, and Settings were converted to client
components because Next.js metadata exports don't work during
client-side navigation (only SSR). Using `usePageTitle()` ensures titles
update correctly during both direct navigation and client-side routing.
- **Loading behavior**: Shows "Tome" during data loading, then updates
to full title
- **Cleanup**: Titles automatically reset to "Tome" when navigating away

---

### Query Key Factory (Bonus Fix)

#### Problem Discovered
While working on dynamic titles, discovered critical React Query bugs:

1. **Invalidation Bug**: `useStreak.ts` was invalidating
`['streak-analytics']` but actual key was `['streak-analytics-full', 7]`
- invalidation did nothing!
2. **Query Key Collision**: `useStreakQuery` and `StreakChartSection`
both used `'streak-analytics'` with different data structures, causing
cache confusion
3. **Maintenance Risk**: 139 hardcoded query key strings across 20+
files with no type safety

#### Solution: Centralized Query Key Factory

Created `lib/query-keys.ts` with type-safe, hierarchical query key
factory:

```typescript
import { queryKeys } from '@/lib/query-keys';

// Type-safe query keys
useQuery({ queryKey: queryKeys.book.detail(bookId), ... });
useQuery({ queryKey: queryKeys.streak.analytics(7), ... });

// Wildcard invalidation
queryClient.invalidateQueries({ queryKey: queryKeys.streak.base() });
```

#### Benefits
- ✅ **Prevents collisions**: Hierarchical keys like `['streak',
'analytics', 7]` vs `['streak', 'analytics', 'heatmap', 365]`
- ✅ **Type safety**: TypeScript catches typos at compile time
- ✅ **Easier refactoring**: Change key structure in one place
- ✅ **Consistent patterns**: Base keys enable wildcard invalidation

#### Migration Scope
- **28 files changed**: 1 new factory, 17 hooks, 8 components, 4 pages
- **139 hardcoded strings** → Centralized factory
- **2 critical bugs fixed**: Invalidation failure and key collision
- **Documentation added**: Full pattern guide in
`docs/AI_CODING_PATTERNS.md`

#### Files Migrated
**Hooks (17):**
- `useStreak.ts` (FIXED INVALIDATION BUG)
- `useStreakQuery.ts` (FIXED COLLISION)
- `useBookDetail.ts`, `useBookProgress.ts`, `useBookStatus.ts`,
`useBookRating.ts`
- `useShelfBooks.ts`, `useReadNextBooks.ts`, `useReadingGoals.ts`
- `useDashboard.ts`, `useStats.ts`, `useSessionProgress.ts`
- `useLibraryData.ts`, `useTagManagement.ts`, `useTagBooks.ts`
- `usePullToRefreshLogic.ts`, `useVersion.ts`

**Components (8):**
- `StreakChartSection.tsx`, `GoalsPagePanel.tsx`
- `CurrentlyReadingList.tsx`, `ReadingHistoryTab.tsx`
- `LogProgressModal.tsx`

**Pages (4):**
- `app/books/[id]/page.tsx`, `app/journal/page.tsx`
- `app/series/[name]/page.tsx`, `app/series/page.tsx`

## Testing

✅ All **3937 tests pass** (43 new tests added)  
✅ Build succeeds with no errors or warnings  
✅ Manual testing confirmed:
- Multiple tabs are now easily distinguishable
- Titles update correctly when data loads
- No console errors
- Proper cleanup on navigation
- Streak analytics invalidation now works correctly
- No more query key collisions

## Documentation

- Added comprehensive **Query Key Factory Pattern** section to
`docs/AI_CODING_PATTERNS.md`
- Enhanced JSDoc in `lib/query-keys.ts` with usage examples
- Updated implementation plan in
`docs/plans/query-key-factory-implementation.md`

## Closes

Closes #394
## Summary

Adds a mobile-only back button to the book detail page that appears when
users navigate from another page within the app. The button uses browser
history to navigate back, preserving scroll position and page state.

## Changes

- **Mobile-only display**: Hidden on desktop (>= 768px)
- **Smart detection**: Shows when `window.history.length > 1`
- **Browser history navigation**: Uses `router.back()` to preserve state
- **Circular background**: Arrow icon in a subtle card-bg circle
- **Visual polish**: Improved vertical alignment and bolder arrow

## Behavior

✅ Internal navigation (Library → Book): Button shows, navigates back  
✅ Direct URL (bookmark/typed): No button shown  
⚠️ Pasted from Google: Button shows, goes back to Google (acceptable
edge case)

## Technical Details

- Uses simple `window.history.length > 1` check (opted for simplicity
over complex referrer detection)
- Works universally from any page (Library, Series, Shelves, Journal,
Dashboard, etc.)
- No URL query parameters or link modifications needed
- Consistent with existing mobile navigation patterns

## Testing

- ✅ Build successful
- ✅ All 3,999 tests passing
- ✅ Manual testing on mobile viewport

Resolves #392
## Summary

Fixes #399 - Journal entry date changes now produce correct, consistent
"pages read" calculations when multiple entries exist on the same date.

### Problem

When changing a journal entry's date, the "pages read" calculation
became incorrect and non-idempotent:
- **Initial**: Mar 9 shows "Read 65 pages" (122 - 57) ✓
- **After moving to Mar 8**: Shows "Read 122 pages" 
- **After moving back to Mar 9**: Shows "Read 79 pages" ❌ (should be 65)

**Root Cause**: Sorting by `progressDate` alone (YYYY-MM-DD) doesn't
provide stable ordering when multiple entries share the same date. The
sort became unpredictable, causing `findLast()` to return arbitrary
entries.

### Solution

Implemented stable multi-column sort using:
1. **progressDate** (primary - YYYY-MM-DD calendar day)
2. **createdAt** (secondary - chronological order within same day)  
3. **id** (tertiary - database insertion order as final tiebreaker)

This ensures deterministic, repeatable results regardless of database
retrieval order.

### Changes

- ✅ **progress.service.ts**: Updated `updateProgress()` with stable sort
- ✅ **progress.repository.ts**: Added stable `orderBy` to all query
methods
- ✅ **journal.service.ts**: Included `id` in sort order
- ✅ **Integration tests**: 3 new tests for same-date entry scenarios  
- ✅ **E2E test**: Reproduces exact issue #399 scenario

### Testing

- **All tests passing**: 4003/4003 tests (100%)
- **New integration tests**: 3 tests covering same-date edge cases
- **New E2E test**: Verifies issue #399 fix with exact reproduction
- **Regression tested**: All existing progress/journal tests still pass

### Verification

Reproduction of issue #399 now produces correct, idempotent results:
- Moving entry Mar 9 → Mar 8 → Mar 9 returns to original 65 pages ✓
- Multiple entries on same date are sorted deterministically ✓
- No breaking changes to existing functionality ✓

---

**Impact**: Low-risk bug fix, no API changes, backward compatible
## Summary

Addresses 4 code quality improvements identified in [PR #401 review
feedback](#401 (review)).

## Changes

### 1. Remove unused import (hooks/usePageTitle.ts)
- **Issue**: `useRef` was imported but never used
- **Fix**: Removed unused import to prevent lint failures
- **Impact**: Clean code, no functional change

### 2. Fix shelf ID 0 handling bug (hooks/useShelfBooks.ts) 🐛
- **Issue**: `if (!shelfId)` treats `0` as falsy, incorrectly skipping
shelf ID 0
- **Fix**: Changed to `if (shelfId === null)` for strict null checking
- **Impact**: **Critical bug fix** - shelf with ID 0 now works correctly
- **Type**: `shelfId` is typed as `number | null`, so this aligns with
the type system

### 3. Add targeted query invalidations (hooks/usePullToRefreshLogic.ts)
- **Issue**: Unmapped paths like `/journal` and `/settings` fell back to
invalidating **all queries**
- **Fix**: Added specific invalidation keys for these routes:
  - `/journal` → `["journal-entries", "journal-archive"]`
  - `/settings` → `["user-preferences"]`
- **Impact**: Prevents expensive full-cache invalidation, improves
pull-to-refresh performance

### 4. Use query key factory consistently (hooks/useLibraryData.ts)
- **Issue**: Query key used hardcoded `'library-books'` string while
refresh used `queryKeys.library.books()`
- **Fix**: Changed to `[...queryKeys.library.books(), ...]` for
consistent factory usage
- **Impact**: Prevents query key drift, ensures invalidation and query
construction stay coupled

## Testing

- ✅ All tests passing: **4006/4007 tests** (99.98%)
- ✅ 1 pre-existing failure in `streaks-coverage.test.ts` (unrelated,
already failing on develop)
- ✅ No new test failures introduced
- ✅ All 4 fixes verified with existing test coverage

## Files Changed

- `lib/hooks/usePageTitle.ts` - Removed unused import
- `hooks/useShelfBooks.ts` - Fixed null check for shelf ID 0
- `hooks/usePullToRefreshLogic.ts` - Added targeted invalidations
- `hooks/useLibraryData.ts` - Use query key factory consistently

## Review Feedback Source

[PR #401 Review Comment
Thread](#401 (review))
## Summary

Fixes a cache invalidation bug where changing a book's status on
`/books/:id` doesn't invalidate the cache for `/shelves/:id`, causing
shelves to show stale book status for up to 30 seconds.

## Problem

When a user changes a book's status on the book detail page, the
`invalidateBookQueries()` helper function invalidates several React
Query caches (book, sessions, progress, dashboard, library, read-next),
but **does not invalidate shelf queries**. This means:

- Shelf pages continue showing the old status until the 30-second
`staleTime` expires
- Users see inconsistent data between the book detail page and shelf
pages
- Manual refresh or waiting 30s is required to see updated status on
shelves

## Solution

Modified `invalidateBookQueries()` in `hooks/useBookStatus.ts` to also
invalidate shelf caches:

### Cache-Based Approach (Option A)

1. **Check cache first**: Query React Query cache for the book's shelves
using `queryKeys.book.shelves(bookId)`
2. **Surgical invalidation**: If cached shelves exist, invalidate only
those specific shelf queries using `queryKeys.shelf.byId(shelfId)`
3. **Nuclear fallback**: If no cached shelves, invalidate all shelf
queries using `queryKeys.shelf.base()` to ensure correctness

This approach balances performance (surgical when possible) with
correctness (nuclear when cache unavailable).

## Changes

### Core Changes

- **`hooks/useBookStatus.ts`** (lines 42-74):
  - Enhanced `invalidateBookQueries()` to check cached book shelves
- Invalidate affected shelf queries (surgical) or all shelves (nuclear)
  - Added JSDoc comments documenting the shelf invalidation logic
  
- **`app/api/books/[id]/status/route.ts`** (line 69):
- Removed misleading comment about SessionService handling cache
invalidation

## Testing

- ✅ All 4007 existing tests pass
- ✅ No breaking changes to function signature
- ✅ Compatible with all existing uses of `invalidateBookQueries()`
- ✅ Server running and responsive during testing

## Implementation Pattern

This follows the same pattern already used in `app/books/[id]/page.tsx`
(lines 245-261) for the `updateShelves()` function, which demonstrates
proper shelf invalidation when books are added/removed from shelves.

## Related Files

- Investigation details: `docs/plans/fix-shelf-cache-invalidation.md`
- Query keys definition: `lib/query-keys.ts` (lines 162-172)
- Book detail page: `app/books/[id]/page.tsx` (lines 229-242 for shelf
caching)

---------

Co-authored-by: GitHub Copilot <copilot@github.com>
## Summary

Fixes a cache invalidation bug where changing a book's status on
`/books/:id` doesn't refresh the cache on the `/series/:name` page,
causing stale status badges and ratings to be displayed.

## Root Cause

The cache invalidation bug existed at two levels:

1. **Client-Side (React Query)**: `invalidateBookQueries()` in
`useBookStatus.ts` was not invalidating series queries
2. **Server-Side (Next.js)**: `SessionService` and `ProgressService`
were not calling `revalidatePath('/series')`

## Changes

**3 files modified, 3 lines added:**

- `hooks/useBookStatus.ts:57` - Added series query invalidation using
`queryKeys.series.all()`
- `lib/services/session.service.ts:1365` - Added
`revalidatePath('/series')`
- `lib/services/progress.service.ts:524` - Added
`revalidatePath('/series')`

## Implementation Details

The fix uses "nuclear" invalidation (invalidate all series queries) for
simplicity and reliability, consistent with the existing shelf
invalidation pattern when specific targeting is complex.

- `queryKeys.series.all()` returns `['series']`, which matches both
series list and all series detail pages
- `revalidatePath('/series')` revalidates both `/series` (list) and
`/series/:name` (detail) pages

## Testing

- ✅ All 4,013 tests pass (188 test files)
- ✅ No regressions detected
- ✅ Changes follow existing patterns

## Manual Testing

To verify the fix:
1. Open a series detail page (e.g., `/series/Harry%20Potter`)
2. Note a book's status badge
3. Change that book's status on `/books/:id`
4. Return to the series page - status should now update immediately
## Summary

Fixes critical bug where books with DNF (Did Not Finish) status were not
showing up when filtering by status on the library page.

## Root Cause

The book repository was incorrectly treating DNF status like active
statuses (reading, to-read, read-next) by requiring `is_active = 1`.
However, DNF is a terminal/completed state like "read", where sessions
are archived with `is_active = 0`.

**Before**: DNF filter returned 0 books despite 3 DNF books existing in
the database
**After**: DNF filter correctly returns all 3 DNF books

## Changes

### Repository Logic (`lib/repositories/book.repository.ts`)
- Updated `findWithFilters()` to treat 'dnf' status like 'read' status
(lines 377-380)
- Updated `findWithFiltersAndRelations()` to treat 'dnf' status like
'read' status (lines 754-757)
- Terminal states (read, dnf) now include all sessions regardless of
`is_active`
- Active states (reading, to-read, read-next) still require `is_active =
1`

### Test Coverage
(`__tests__/integration/repositories/books/book-status-filtering.test.ts`)
Added 9 comprehensive tests covering:
- ✅ DNF status filtering (terminal state)
- ✅ Read status filtering (terminal state)
- ✅ Active status filtering (reading, to-read, read-next)
- ✅ Books with multiple sessions (DNF + active)
- ✅ Both `findWithFilters()` and `findWithFiltersAndRelations()` methods

## Testing

- All 4,022 existing tests pass ✅
- 9 new tests added for status filtering ✅
- Verified via API: `/api/books?status=dnf` returns 3 books ✅
- Confirmed database has 3 DNF sessions with `is_active = 0` ✅

## Impact

- Users can now filter for DNF books on the library page
- Fixes incorrect behavior where terminal states were treated as active
states
- No breaking changes to other status filters

## Database Evidence

```sql
-- Before fix: 3 DNF sessions exist with is_active = 0
sqlite> SELECT status, is_active, COUNT(*) FROM reading_sessions 
        GROUP BY status, is_active;
dnf|0|3
read|0|66
read|1|2
read-next|1|35
reading|1|3
to-read|1|915
```

## Related Files

- `lib/repositories/book.repository.ts` (2 locations updated)
-
`__tests__/integration/repositories/books/book-status-filtering.test.ts`
(new file)
## Summary

Fixes PR #415 which was marked as MERGED in GitHub but the commits never
actually made it into the develop branch. This PR cherry-picks the three
orphaned feature commits to restore the tap & hold to select
functionality.

**Problem:** PR #415 shows as merged, but the feature code is missing
from develop (except for `useLongPress.ts` which was added separately in
commit 9f2ca50).

**Solution:** Cherry-picked the three orphaned commits and squashed them
into a single commit with proper attribution.

---

## Changes

### Modified Files (5)
1. **`hooks/useBookListView.ts`** - Added
`enterSelectModeWithSelection()` function
2. **`components/Books/BookListItem.tsx`** - Integrated useLongPress
hook + keyboard accessibility
3. **`components/Books/DraggableBookList.tsx`** - Added long-press props
and wiring
4. **`app/shelves/[id]/page.tsx`** - Wired up long-press handlers to
list views

### New Files (1)
5. **`__tests__/hooks/useLongPress.test.ts`** - Comprehensive test suite
(22 tests)

**Note:** `hooks/useLongPress.ts` already existed on develop (added by
commit 9f2ca50) and matches the final PR version, so it was kept
unchanged.

---

## Feature: Tap & Hold to Select

Implements long-press gesture to enter select mode on shelf items,
eliminating the need to scroll to the top to press the "Select" button.

**Key Features:**
- Long-press (500ms) on book card body enters select mode and
auto-selects the item
- Separate touch targets: drag handle (200ms) vs card body (500ms) to
avoid conflicts
- 10px movement tolerance to prevent activation during scrolling
- Keyboard accessibility: Space and Enter keys trigger selection
- ARIA attributes for screen readers
- Mobile-focused UX improvement

**Behavior:**
1. User long-presses on book card (NOT in select mode)
2. After 500ms: enters select mode + selects that book
3. BulkActionBar appears at bottom
4. User can tap other books to select/deselect
5. Existing "Select" button still works

---

## Testing

✅ **All 4044 tests pass** (includes 22 new useLongPress tests)
✅ **Build succeeds** with no TypeScript errors

### Test Coverage
- Mouse events (down, up, move, leave)
- Touch events (start, end, cancel, move)
- Keyboard events (Space, Enter)
- Movement tolerance (cancels when exceeding 10px)
- Configurable delay and tolerance options
- Memory leak prevention (cleanup on unmount)
- ARIA attributes for accessibility

---

## Original Commits

This PR incorporates the following orphaned commits:
- `b88e3fc` - feat: implement tap & hold to select shelf items
- `990c8aa` - fix: implement @review feedback (iteration 1)
- `f623bb4` - fix: add onTouchCancel handler and Enter key accessibility
support

---

## References

- **Original PR:** #415
- **Issue:** #414
- **Problem Commit:** 9f2ca50 (added useLongPress.ts but not integration
code)

**Resolves:** #414  
**Fixes:** #415

Co-authored-by: OpenCode <assistant@opencode.ai>
@codecov

codecov Bot commented Apr 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.48485% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
hooks/useLongPress.ts 97.91% 0 Missing and 1 partial ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #420      +/-   ##
==========================================
+ Coverage   78.67%   78.85%   +0.18%     
==========================================
  Files         167      168       +1     
  Lines        7559     7620      +61     
  Branches     1850     1861      +11     
==========================================
+ Hits         5947     6009      +62     
  Misses       1127     1127              
+ Partials      485      484       -1     
Flag Coverage Δ
unittests 78.85% <98.48%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
app/api/books/[id]/sessions/route.ts 88.23% <100.00%> (+3.61%) ⬆️
lib/repositories/book.repository.ts 64.24% <100.00%> (+0.63%) ⬆️
lib/repositories/session.repository.ts 79.27% <100.00%> (+0.38%) ⬆️
lib/services/session.service.ts 88.42% <100.00%> (+0.24%) ⬆️
package.json 100.00% <100.00%> (ø)
hooks/useLongPress.ts 97.91% <97.91%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

## Summary

Fixes #413 - Session numbers now display correctly after deleting
sessions.

**Problem:** When a session was deleted and a new one was created, the
session number would increment incorrectly (e.g., showing "Read # 2"
when it should be "Read # 1").

**Root Cause:** The `deleteSession()` method hardcoded `sessionNumber:
1` when creating a new "to-read" session after deletion, which
conflicted with the `getNextSessionNumber()` logic that increments based
on the highest existing session number.

**Solution:** Implemented array index-based display numbers that are
calculated from chronologically ordered sessions.

## Changes

### Backend
- **Quick Fix:** Updated `deleteSession()` to use
`getNextSessionNumber()` instead of hardcoded `1`
- **Repository:** Added `findAllByBookIdOrdered()` method that sorts
sessions by `COALESCE(startedDate, createdAt)` for chronological
ordering
- **Service:** Added `getSessionsWithDisplayNumbers()` method that
calculates `displayNumber` based on array position (index + 1)
- **API:** Updated `/api/books/[id]/sessions` endpoint to include
`displayNumber` in responses

### Frontend
- Updated `ReadingHistoryTab.tsx` to display `displayNumber` (with
fallback to `sessionNumber`)
- Updated modal components to use `displayNumber`:
  - `SessionEditModal.tsx`
  - `DeleteSessionModal.tsx`
  - `SessionProgressModal.tsx`

### Testing
- Added comprehensive integration tests in
`__tests__/integration/issues/issue-413-session-number-after-deletion.test.ts`
- All 4 new tests pass:
  - ✅ Quick fix prevents session number conflicts
  - ✅ Display numbers calculated chronologically
  - ✅ Display numbers renumber after deletion (no gaps)
  - ✅ Handles null startedDate with createdAt fallback
- Full test suite: **4016 passing tests** (up from 4012)

## Behavior

**Before:**
- Delete session # 1 → Creates "to-read" with sessionNumber: 1
- Change to "Read" → Creates "read" with sessionNumber: 2 ❌ (should be #
1)

**After:**
- Delete session # 1 → Creates "to-read" with sessionNumber: 1 (via
`getNextSessionNumber()`)
- Change to "Read" → Creates "read" with sessionNumber: 2
- **Display number:** Shows as "Read # 1" (calculated from array
position) ✅

**Key Features:**
- Display numbers are always sequential (1, 2, 3...) regardless of
deletions
- No gaps in numbering after deletion
- Database `sessionNumber` preserved for backward compatibility
- Display numbers represent "1st read, 2nd read, 3rd read"
chronologically

## Testing

```bash
npm test __tests__/integration/issues/issue-413-session-number-after-deletion.test.ts
# ✅ All 4 tests pass

npm test
# ✅ 4016 / 4017 tests pass (1 pre-existing failure in streaks)
```

## Files Changed
- `lib/services/session.service.ts` - Quick fix + display number service
method
- `lib/repositories/session.repository.ts` - Chronological ordering
method
- `app/api/books/[id]/sessions/route.ts` - Add displayNumber to API
response
- `components/CurrentlyReading/ReadingHistoryTab.tsx` - Display
calculated numbers
- `components/Modals/*.tsx` - Update modals to use displayNumber
-
`__tests__/integration/issues/issue-413-session-number-after-deletion.test.ts`
- Integration tests

## Manual Testing Needed

- [ ] Delete a session and verify "Read #" numbers display correctly
- [ ] Create multiple sessions, delete middle one, verify renumbering
- [ ] Verify modals show correct "Read #X" titles

---------

Co-authored-by: GitHub Copilot <copilot@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants