fix(books): prevent next-book navigation from sticking at boundaries

Fetch requests now await existing promises for in-flight blocks instead of re-triggering and starving the load when crossing block boundaries. The navigation window entries are also preserved during identical structural updates to avoid infinite watcher loops.

Closes #325

client/src/features/book/composables/__tests__/useBookNavigation.spec.ts

@@ -1,3 +1,4 @@
+import { nextTick, watch } from 'vue'
 import { afterEach, describe, expect, it } from 'vitest'
 import type { BookCard } from '@bookorbit/types'
 import { useBookNavigation } from '../useBookNavigation'
@@ -139,4 +140,45 @@ describe('useBookNavigation', () => {
       expect(await nav.getNextId(2)).toBe(null)
     })
   })
+
+  describe('re-sync stability (boundary loop guard)', () => {
+    it('does not invalidate bookIds when the slot context is structurally unchanged', async () => {
+      const nav = useBookNavigation()
+      nav.setBookSlotContext([makeBook(1), makeBook(2), makeBook(3)], 100)
+
+      let fires = 0
+      const stop = watch(nav.bookIds, () => {
+        fires++
+      })
+
+      // A re-sync with the same ids/indexes (e.g. while a lazy block is still
+      // in flight) must not retrigger watchers that recompute next/prev id.
+      nav.setBookSlotContext([makeBook(1), makeBook(2), makeBook(3)], 100)
+      await nextTick()
+      expect(fires).toBe(0)
+
+      // A genuine change still propagates.
+      nav.setBookSlotContext([makeBook(1), makeBook(2), makeBook(3), makeBook(4)], 100)
+      await nextTick()
+      expect(fires).toBe(1)
+
+      stop()
+    })
+
+    it('does not invalidate bookIds when the fully-loaded context is unchanged', async () => {
+      const nav = useBookNavigation()
+      nav.setBookContext([1, 2, 3], 3)
+
+      let fires = 0
+      const stop = watch(nav.bookIds, () => {
+        fires++
+      })
+
+      nav.setBookContext([1, 2, 3], 3)
+      await nextTick()
+      expect(fires).toBe(0)
+
+      stop()
+    })
+  })
 })

client/src/features/book/composables/__tests__/useBookViewContext.spec.ts

@@ -72,6 +72,46 @@ describe('useBookViewContext', () => {
     expect(nextId).toBe(3)
   })
 
+  it('advances across a block boundary when load-more resolves asynchronously', async () => {
+    const nav = useBookNavigation()
+    // 100 contiguous loaded slots (indexes 0..99); book 100 is the last loaded
+    // of the first block, with 900 more still to load.
+    const slots = ref<BookSlot[]>(Array.from({ length: 100 }, (_, i) => makeBook(i + 1)))
+    const total = ref(1000)
+
+    let releaseLoad: (() => void) | undefined
+    const loadMore = vi.fn<() => Promise<void>>(
+      () =>
+        new Promise<void>((resolve) => {
+          releaseLoad = () => {
+            slots.value = [...slots.value, ...Array.from({ length: 100 }, (_, i) => makeBook(i + 101))]
+            resolve()
+          }
+        }),
+    )
+
+    const Harness = defineComponent({
+      setup() {
+        useBookViewContext(slots, total, loadMore)
+        return {}
+      },
+      template: '<div />',
+    })
+
+    const wrapper = mount(Harness)
+    // Mirror real navigation: the grid/list view unmounts when the editor opens.
+    wrapper.unmount()
+
+    const nextPromise = nav.getNextId(100)
+    await Promise.resolve()
+    // The next block is still loading, so the id is not resolved synchronously.
+    expect(releaseLoad).toBeDefined()
+    releaseLoad?.()
+
+    expect(await nextPromise).toBe(101)
+    expect(loadMore).toHaveBeenCalledTimes(1)
+  })
+
   it('reports the absolute slot index after a jump rail loads a far block', async () => {
     const nav = useBookNavigation()
     // First block (rows 0-1) plus a far block (rows 5000-5001) loaded; the

client/src/features/book/composables/__tests__/useBookViewWindow.spec.ts

@@ -0,0 +1,222 @@
+import { nextTick, ref } from 'vue'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import type { BookCard, BooksPage, GroupRule, JumpBucket } from '@bookorbit/types'
+
+const fetchMock = vi.fn<(url: string, init?: RequestInit) => Promise<unknown>>()
+
+vi.mock('@/lib/api', () => ({
+  api: (url: string, init?: RequestInit) => fetchMock(url, init),
+}))
+
+import { BOOK_WINDOW_BLOCK_SIZE } from '../useBookWindow'
+import { useBookViewWindow } from '../useBookViewWindow'
+
+function makeBook(id: number): BookCard {
+  return {
+    id,
+    status: 'active',
+    title: `Book ${id}`,
+    authors: [],
+    seriesName: null,
+    seriesIndex: null,
+    files: [],
+    publishedYear: null,
+    language: null,
+    genres: [],
+    tags: [],
+    rating: null,
+    readingProgress: null,
+    readStatus: null,
+    addedAt: '2024-01-01T00:00:00Z',
+    updatedAt: null,
+    metadataScore: null,
+    hasCover: false,
+    hasMetadataLocks: false,
+    lockedFields: [],
+    subtitle: null,
+    publisher: null,
+    pageCount: null,
+    isbn13: null,
+    narrators: [],
+  } as BookCard
+}
+
+function pageFor(block: number, total: number): BooksPage {
+  const start = block * BOOK_WINDOW_BLOCK_SIZE
+  const count = Math.max(0, Math.min(BOOK_WINDOW_BLOCK_SIZE, total - start))
+  return {
+    items: Array.from({ length: count }, (_, i) => makeBook(start + i + 1)),
+    total,
+    page: block,
+    size: BOOK_WINDOW_BLOCK_SIZE,
+  }
+}
+
+function bucket(key: string, index: number): JumpBucket {
+  return { key, label: key, index, count: 1 } as JumpBucket
+}
+
+function mockApi(total: number, buckets: JumpBucket[] = []) {
+  fetchMock.mockImplementation((url, init) => {
+    if (url.includes('jump-buckets')) {
+      return Promise.resolve({ ok: true, json: () => Promise.resolve({ buckets }) })
+    }
+    const body = JSON.parse(String(init?.body)) as { pagination: { page: number } }
+    return Promise.resolve({ ok: true, json: () => Promise.resolve(pageFor(body.pagination.page, total)) })
+  })
+}
+
+async function flush() {
+  await Promise.resolve()
+  await Promise.resolve()
+  await nextTick()
+}
+
+function setup(options: { scope?: number | null; viewMode?: string; collapse?: boolean; q?: string } = {}) {
+  const scopeId = ref<number | null>(options.scope === undefined ? 1 : options.scope)
+  const viewMode = ref(options.viewMode ?? 'table')
+  const collapseEnabled = ref(options.collapse ?? false)
+  const q = ref(options.q ?? '')
+  const win = useBookViewWindow({
+    scopeId,
+    listEndpoint: (id) => `/api/v1/libraries/${id}/books`,
+    bucketsEndpoint: (id) => `/api/v1/libraries/${id}/books/jump-buckets`,
+    viewMode,
+    collapseEnabled,
+    q,
+  })
+  return { win, scopeId, viewMode, collapseEnabled, q }
+}
+
+function listRequests(): number[] {
+  return fetchMock.mock.calls
+    .filter(([url]) => !url.includes('jump-buckets'))
+    .map(([, init]) => (JSON.parse(String(init?.body)) as { pagination: { page: number } }).pagination.page)
+}
+
+describe('useBookViewWindow', () => {
+  beforeEach(() => {
+    fetchMock.mockReset()
+  })
+
+  it('loads the first block on creation and exposes window state', async () => {
+    mockApi(250)
+    const { win } = setup()
+    await flush()
+
+    expect(win.total.value).toBe(250)
+    expect(win.contiguousPrefix.value).toHaveLength(100)
+    expect(win.hasMorePrefix.value).toBe(true)
+  })
+
+  it('does not fetch when the scope id is invalid', async () => {
+    mockApi(250)
+    const { win } = setup({ scope: null })
+    await flush()
+
+    expect(fetchMock).not.toHaveBeenCalled()
+    expect(win.total.value).toBe(0)
+  })
+
+  it('loadMorePrefix resolves only after the next contiguous chunk has loaded', async () => {
+    mockApi(500)
+    const { win } = setup()
+    await flush()
+    expect(win.contiguousPrefix.value).toHaveLength(100)
+
+    let resolveBlock: ((value: unknown) => void) | undefined
+    fetchMock.mockImplementationOnce(
+      () =>
+        new Promise((resolve) => {
+          resolveBlock = resolve
+        }),
+    )
+
+    let settled = false
+    const pending = win.loadMorePrefix()!.then(() => {
+      settled = true
+    })
+    await flush()
+    expect(settled).toBe(false)
+
+    resolveBlock?.({ ok: true, json: () => Promise.resolve(pageFor(1, 500)) })
+    await pending
+    expect(settled).toBe(true)
+    expect(win.contiguousPrefix.value).toHaveLength(200)
+    expect(win.hasMorePrefix.value).toBe(true)
+  })
+
+  it('handleRange loads far blocks and handleFirstVisibleIndex tracks the viewport', async () => {
+    mockApi(500)
+    const { win } = setup()
+    await flush()
+    fetchMock.mockClear()
+
+    win.handleRange(300, 320)
+    await flush()
+    expect(listRequests()).toEqual([3])
+
+    win.handleFirstVisibleIndex(42)
+    expect(win.firstVisibleIndex.value).toBe(42)
+  })
+
+  it('reflects filter, collapse, and search in the query', async () => {
+    mockApi(120)
+    const { win } = setup({ collapse: true, q: '  dune  ' })
+    await flush()
+
+    const filter: GroupRule = { type: 'group', join: 'AND', rules: [] }
+    win.filter.value = filter
+
+    expect(win.query.value).toMatchObject({
+      collapseSeries: true,
+      q: 'dune',
+      filter,
+    })
+  })
+
+  it('reverses the letter template for a descending sort', async () => {
+    mockApi(120)
+    const { win } = setup()
+    await flush()
+
+    expect(win.letterTemplate.value[0]).toBe('#')
+
+    win.sort.value = [{ field: 'title', dir: 'desc' }]
+    await nextTick()
+    expect(win.letterTemplate.value[0]).not.toBe('#')
+    expect(win.bucketKind.value).not.toBeNull()
+  })
+
+  it('keeps the jump rail hidden in table mode', async () => {
+    mockApi(250, [bucket('A', 0), bucket('B', 120)])
+    const { win } = setup({ viewMode: 'table' })
+    await flush()
+
+    expect(win.railVisible.value).toBe(false)
+    expect(win.railGutterReserved.value).toBe(false)
+  })
+
+  it('shows the jump rail in grid mode once buckets load and jumps scroll to the bucket index', async () => {
+    mockApi(250, [bucket('A', 0), bucket('B', 120)])
+    const { win } = setup({ viewMode: 'grid' })
+    await flush()
+
+    expect(win.railVisible.value).toBe(true)
+    expect(win.activeBucketKey.value).toBe('A')
+    expect(typeof win.railGutterReserved.value).toBe('boolean')
+
+    const scrollTo = vi.fn()
+    win.registerScroller(scrollTo)
+    fetchMock.mockClear()
+
+    win.handleJump(bucket('B', 120))
+    await flush()
+
+    expect(scrollTo).toHaveBeenCalledWith(120)
+    expect(win.firstVisibleIndex.value).toBe(120)
+    expect(listRequests()).toContain(1)
+
+    win.releaseRailGutter()
+  })
+})

client/src/features/book/composables/__tests__/useBookWindow.spec.ts

@@ -244,4 +244,203 @@ describe('useBookWindow', () => {
     expect(window.total.value).toBe(299)
     expect(isBookPlaceholder(window.slots.value[100]!)).toBe(true)
   })
+
+  it('ensureRange resolves only after the requested block has finished loading', async () => {
+    mockBlocks(500)
+    const { window } = makeWindow()
+    await flush()
+
+    let resolveBlock: ((value: unknown) => void) | undefined
+    fetchMock.mockImplementationOnce(
+      () =>
+        new Promise((resolve) => {
+          resolveBlock = resolve
+        }),
+    )
+
+    let settled = false
+    const pending = window.ensureRange(150, 199).then(() => {
+      settled = true
+    })
+    await flush()
+    expect(settled).toBe(false)
+    expect(isBookPlaceholder(window.slots.value[150]!)).toBe(true)
+
+    resolveBlock?.({ ok: true, json: () => Promise.resolve(pageFor(1, 500)) })
+    await pending
+    expect(settled).toBe(true)
+    expect(isBookPlaceholder(window.slots.value[150]!)).toBe(false)
+  })
+
+  it('ensureRange resolves immediately without fetching when the range is already loaded', async () => {
+    mockBlocks(50)
+    const { window } = makeWindow()
+    await flush()
+    fetchMock.mockClear()
+
+    await window.ensureRange(0, 49)
+    expect(fetchMock).not.toHaveBeenCalled()
+  })
+
+  it('ensureRange resolves even when the block fails to load', async () => {
+    mockBlocks(300)
+    const { window } = makeWindow()
+    await flush()
+    fetchMock.mockClear()
+    fetchMock.mockResolvedValueOnce({ ok: false, status: 500 })
+
+    let settled = false
+    await window.ensureRange(100, 150).then(() => {
+      settled = true
+    })
+
+    expect(settled).toBe(true)
+    expect(requestedBlocks()).toEqual([1])
+    expect(window.error.value).toBe('HTTP 500')
+  })
+
+  it('ensureRange awaits an already in-flight block rather than starting a second fetch', async () => {
+    mockBlocks(500)
+    const { window } = makeWindow()
+    await flush()
+    fetchMock.mockClear()
+
+    let resolveBlock: ((value: unknown) => void) | undefined
+    fetchMock.mockImplementationOnce(
+      () =>
+        new Promise((resolve) => {
+          resolveBlock = resolve
+        }),
+    )
+
+    let firstSettled = false
+    let secondSettled = false
+    const first = window.ensureRange(150, 199).then(() => {
+      firstSettled = true
+    })
+    const second = window.ensureRange(150, 199).then(() => {
+      secondSettled = true
+    })
+    await flush()
+    expect(requestedBlocks()).toEqual([1])
+    expect(firstSettled).toBe(false)
+    expect(secondSettled).toBe(false)
+
+    resolveBlock?.({ ok: true, json: () => Promise.resolve(pageFor(1, 500)) })
+    await Promise.all([first, second])
+    expect(firstSettled).toBe(true)
+    expect(secondSettled).toBe(true)
+  })
+
+  it('retry refetches previously failed blocks and clears the error', async () => {
+    fetchMock.mockResolvedValueOnce({ ok: false, status: 500 })
+    const { window } = makeWindow()
+    await flush()
+    expect(window.error.value).toBe('HTTP 500')
+
+    fetchMock.mockClear()
+    mockBlocks(120)
+    window.retry()
+    await flush()
+
+    expect(requestedBlocks()).toEqual([0])
+    expect(window.error.value).toBeNull()
+    expect(window.total.value).toBe(120)
+  })
+
+  it('retry reloads from scratch when the listing is empty', async () => {
+    mockBlocks(0)
+    const { window } = makeWindow()
+    await flush()
+    expect(window.total.value).toBe(0)
+
+    fetchMock.mockClear()
+    mockBlocks(40)
+    window.retry()
+    await flush()
+
+    expect(requestedBlocks()).toEqual([0])
+    expect(window.total.value).toBe(40)
+  })
+
+  it('retry re-ensures the first block without refetching what is already loaded', async () => {
+    mockBlocks(250)
+    const { window } = makeWindow()
+    await flush()
+    fetchMock.mockClear()
+
+    window.retry()
+    await flush()
+
+    expect(fetchMock).not.toHaveBeenCalled()
+  })
+
+  it('booksProxy reconciles assignments by id (remove, update, prepend)', async () => {
+    mockBlocks(50)
+    const { window } = makeWindow()
+    await flush()
+    expect(window.booksProxy.value).toHaveLength(50)
+
+    const current = window.booksProxy.value
+    window.booksProxy.value = [
+      makeBook(900),
+      ...current.filter((card) => card.id !== 1).map((card) => (card.id === 2 ? makeBook(2, { title: 'Renamed' }) : card)),
+    ]
+
+    const ids = window.loadedCards.value.map((card) => card.id)
+    expect(ids).not.toContain(1)
+    expect(ids).toContain(900)
+    expect((window.loadedCards.value.find((card) => card.id === 2) as BookCard).title).toBe('Renamed')
+    expect(window.total.value).toBe(50)
+  })
+
+  it('removeBooks is a no-op when none of the ids are loaded', async () => {
+    mockBlocks(20)
+    const { window } = makeWindow()
+    await flush()
+
+    window.removeBooks([9999])
+    expect(window.total.value).toBe(20)
+    expect(window.slots.value).toHaveLength(20)
+  })
+
+  it('prependBooks ignores cards whose ids are already loaded', async () => {
+    mockBlocks(20)
+    const { window } = makeWindow()
+    await flush()
+
+    window.prependBooks([makeBook(1), makeBook(2)])
+    expect(window.total.value).toBe(20)
+  })
+
+  it('booksProxy assignment with no structural change leaves the window untouched', async () => {
+    mockBlocks(20)
+    const { window } = makeWindow()
+    await flush()
+
+    window.booksProxy.value = window.booksProxy.value
+    expect(window.total.value).toBe(20)
+    expect(window.loadedCards.value).toHaveLength(20)
+  })
+
+  it('updateBook is a no-op when the id is not loaded', async () => {
+    mockBlocks(20)
+    const { window } = makeWindow()
+    await flush()
+
+    const before = window.slots.value
+    window.updateBook(makeBook(9999, { title: 'Ghost' }))
+    expect(window.slots.value).toBe(before)
+  })
+
+  it('booksProxy replaces a card in place when only its contents change', async () => {
+    mockBlocks(20)
+    const { window } = makeWindow()
+    await flush()
+
+    window.booksProxy.value = window.booksProxy.value.map((card) => (card.id === 5 ? makeBook(5, { title: 'Updated' }) : card))
+
+    expect((window.loadedCards.value.find((card) => card.id === 5) as BookCard).title).toBe('Updated')
+    expect(window.total.value).toBe(20)
+  })
 })

client/src/features/book/composables/useBookNavigation.ts

@@ -17,9 +17,22 @@ const positionById = computed(() => {
   return map
 })
 
+// Skip the reassignment when the listing is structurally identical. A re-sync
+// with the same ids/indexes (e.g. while a lazy block is still in flight) must
+// not invalidate `entries`, or watchers that recompute the next/prev id would
+// fire in a loop and starve the in-flight fetch.
+function sameEntries(a: NavEntry[], b: NavEntry[]): boolean {
+  if (a.length !== b.length) return false
+  for (let i = 0; i < a.length; i++) {
+    if (a[i]!.id !== b[i]!.id || a[i]!.index !== b[i]!.index) return false
+  }
+  return true
+}
+
 export function useBookNavigation() {
   const setBookContext = (ids: number[], totalCount: number) => {
-    entries.value = ids.map((id, index) => ({ id, index }))
+    const next = ids.map((id, index) => ({ id, index }))
+    if (!sameEntries(entries.value, next)) entries.value = next
     total.value = totalCount
   }
 
@@ -29,7 +42,7 @@ export function useBookNavigation() {
       const slot = slots[i]!
       if (!isBookPlaceholder(slot)) next.push({ id: slot.id, index: i })
     }
-    entries.value = next
+    if (!sameEntries(entries.value, next)) entries.value = next
     total.value = totalCount
   }
 

client/src/features/book/composables/useBookViewWindow.ts

@@ -90,7 +90,7 @@ export function useBookViewWindow(options: {
 
   function loadMorePrefix() {
     const start = window.contiguousPrefix.value.length
-    window.ensureRange(start, start + LIST_CHUNK - 1)
+    return window.ensureRange(start, start + LIST_CHUNK - 1)
   }
 
   return {

client/src/features/book/composables/useBookWindow.ts

@@ -30,7 +30,7 @@ export function useBookWindow(options: { endpoint: Ref<string | null>; query: Re
   const initialized = shallowRef(false)
   const error = shallowRef<string | null>(null)
 
-  const inFlight = new Set<number>()
+  const inFlight = new Map<number, Promise<void>>()
   const failedAt = new Map<number, number>()
   let generation = 0
   let controller: AbortController | null = null
@@ -58,13 +58,24 @@ export function useBookWindow(options: { endpoint: Ref<string | null>; query: Re
     return true
   }
 
-  async function fetchBlock(block: number) {
+  // Returns the block's load promise so callers (e.g. list-mode load-more) can
+  // await the fetch instead of firing it and racing the result. A block already
+  // in flight returns its existing promise rather than starting a second fetch.
+  function fetchBlock(block: number): Promise<void> {
+    const existing = inFlight.get(block)
+    if (existing) return existing
+    if (!options.endpoint.value) return Promise.resolve()
+    const promise = loadBlock(block)
+    inFlight.set(block, promise)
+    return promise
+  }
+
+  async function loadBlock(block: number) {
     const endpoint = options.endpoint.value
     if (!endpoint) return
 
     const gen = generation
     const signal = controller?.signal
-    inFlight.add(block)
     loading.value = true
 
     try {
@@ -106,9 +117,9 @@ export function useBookWindow(options: { endpoint: Ref<string | null>; query: Re
     }
   }
 
-  function ensureRange(startIndex: number, endIndex: number) {
-    if (!options.endpoint.value) return
-    if (initialized.value && total.value === 0 && failedAt.size === 0) return
+  function ensureRange(startIndex: number, endIndex: number): Promise<void> {
+    if (!options.endpoint.value) return Promise.resolve()
+    if (initialized.value && total.value === 0 && failedAt.size === 0) return Promise.resolve()
     const start = Math.max(0, startIndex)
     let end = Math.max(start, endIndex)
     if (total.value > 0) end = Math.min(end, total.value - 1)
@@ -116,13 +127,19 @@ export function useBookWindow(options: { endpoint: Ref<string | null>; query: Re
 
     const firstBlock = Math.floor(start / BOOK_WINDOW_BLOCK_SIZE)
     const lastBlock = Math.floor(end / BOOK_WINDOW_BLOCK_SIZE)
+    const pending: Promise<void>[] = []
     for (let block = firstBlock; block <= lastBlock; block++) {
-      if (inFlight.has(block)) continue
+      const existing = inFlight.get(block)
+      if (existing) {
+        pending.push(existing)
+        continue
+      }
       const failed = failedAt.get(block)
       if (failed !== undefined && Date.now() - failed < FAILED_BLOCK_RETRY_MS) continue
       if (initialized.value && blockFullyLoaded(block)) continue
-      void fetchBlock(block)
+      pending.push(fetchBlock(block))
     }
+    return pending.length > 0 ? Promise.all(pending).then(() => undefined) : Promise.resolve()
   }
 
   function invalidateInFlight() {