src/hooks/persisted/useNovel/useChapterOperations.ts (2)

59-71: Remove unnecessary async keyword from synchronous functions.

transformAndSetChapters and extendChapters don't contain any await expressions. The async keyword causes them to return Promise<void> instead of void, which may confuse callers.

♻️ Remove async

   const transformAndSetChapters = useCallback(
-    async (chs: ChapterInfo[]) => {
+    (chs: ChapterInfo[]) => {
       _setChapters(transformChapters(chs));
     },
     [transformChapters, _setChapters],
   );

   const extendChapters = useCallback(
-    async (chs: ChapterInfo[]) => {
+    (chs: ChapterInfo[]) => {
       _setChapters(prev => prev.concat(transformChapters(chs)));
     },
     [transformChapters, _setChapters],
   );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/useChapterOperations.ts` around lines 59 - 71,
Both transformAndSetChapters and extendChapters are declared async but contain
no await and thus return Promise<void>; remove the unnecessary async keyword
from the function declarations for transformAndSetChapters and extendChapters so
they return void; keep their bodies unchanged and retain the same dependencies
(transformChapters and _setChapters) in the useCallback calls.

---

17-26: chapters parameter is declared but unused.

The chapters field in UseChapterOperationsParams is never referenced in the hook body. Either remove it from the interface or use it if needed.

♻️ Remove unused parameter

 export interface UseChapterOperationsParams {
   novel: NovelInfo | undefined;
-  chapters: ChapterInfo[];
   _setChapters: React.Dispatch<React.SetStateAction<ChapterInfo[]>>;
   transformChapters: (chs: ChapterInfo[]) => ChapterInfo[];
   settingsSort: ChapterOrderKey;
   settingsFilter: ChapterFilterKey[];
   currentPage: string;
   fetching: boolean;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/useChapterOperations.ts` around lines 17 - 26,
The interface UseChapterOperationsParams declares a chapters field that is never
used in the hook; remove the unused property from UseChapterOperationsParams
(delete the chapters: ChapterInfo[] entry) and update any call sites that
construct objects of this type (calls to useChapterOperations or factories
creating UseChapterOperationsParams) to stop providing chapters, or
alternatively, if the data is actually needed, wire the existing chapters usage
into the hook logic (e.g., inside useChapterOperations reference the passed
chapters instead of any other source). Ensure you only change the interface and
its callers or integrate the chapters parameter in the useChapterOperations
function to keep types consistent.

src/hooks/persisted/useNovel/types.ts (1)

7-18: Consider declaring the interface before its default value.

defaultNovelSettings references NovelSettings before the interface is declared. While TypeScript hoists interface declarations, defining the interface first improves readability.

♻️ Suggested reordering

 export const LAST_READ_PREFIX = 'LAST_READ_PREFIX';

+export interface NovelSettings {
+  sort?: ChapterOrderKey;
+  filter: ChapterFilterKey[];
+  showChapterTitles?: boolean;
+}
+
 export const defaultNovelSettings: NovelSettings = {
   showChapterTitles: true,
   filter: [],
 };

 export const defaultPageIndex = 0;
-
-export interface NovelSettings {
-  sort?: ChapterOrderKey;
-  filter: ChapterFilterKey[];
-  showChapterTitles?: boolean;
-}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/types.ts` around lines 7 - 18, Move the
NovelSettings interface declaration above the default values so the type is
declared before it’s referenced: place the NovelSettings interface first,
followed by defaultNovelSettings and defaultPageIndex; update any imports/usages
if necessary to reflect the new order, ensuring the symbols NovelSettings and
defaultNovelSettings remain unchanged.

src/utils/mmkv/zustand-adapter.ts (1)

19-25: Replace console.error with a project logger or suppress the lint rule.

ESLint flags console.error statements. If logging storage errors is intentional for debugging, either use a project-sanctioned logger utility or add an inline eslint-disable comment with justification.

♻️ Option: Inline suppression if console is intentional

     } catch (error) {
+      // eslint-disable-next-line no-console -- Storage adapter error logging for debugging
       console.error(
         `[mmkvZustandAdapter] Error getting item for key "${key}":`,
         error,
       );

Apply the same pattern to lines 36 and 50.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/mmkv/zustand-adapter.ts` around lines 19 - 25, Replace the
console.error calls inside the mmkv-zustand adapter error handlers with the
project logging utility or explicitly suppress the lint rule with a justified
inline comment: locate the catch blocks in the mmkv adapter functions (e.g.,
getItem, setItem, removeItem in zustand-adapter.ts) and either swap
console.error(...) for the shared logger (processLogger or logger) with the same
message and error object, or add an eslint-disable-next-line no-console comment
plus a brief justification above each console.error (the occurrences at the
catch blocks currently around the key-handling logic and the ones at lines
corresponding to getItem, setItem, and removeItem).

src/plugins/pluginManager.ts (1)

172-174: Consider extracting dev plugin IDs as constants.

The dev plugin IDs 'dev-novel' and 'dev-paged-novel' are duplicated here and in the registry initialization (lines 90-91). Extracting them as constants would reduce duplication and prevent typos.

♻️ Suggested refactor

+const DEV_NOVEL_ID = 'dev-novel';
+const DEV_PAGED_NOVEL_ID = 'dev-paged-novel';
+
 const plugins: Record<string, Plugin | undefined> = __DEV__
   ? {
-      'dev-novel': devNovelPlugin,
-      'dev-paged-novel': devPagedNovelPlugin,
+      [DEV_NOVEL_ID]: devNovelPlugin,
+      [DEV_PAGED_NOVEL_ID]: devPagedNovelPlugin,
     }
   : {};

Then update the check:

-  if (__DEV__ && (pluginId === 'dev-novel' || pluginId === 'dev-paged-novel')) {
+  if (__DEV__ && (pluginId === DEV_NOVEL_ID || pluginId === DEV_PAGED_NOVEL_ID)) {
     return plugins[pluginId];
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/pluginManager.ts` around lines 172 - 174, Extract the duplicate
dev plugin ID strings into shared constants (e.g., DEV_PLUGIN_NOVEL and
DEV_PLUGIN_PAGED_NOVEL) and replace the inline literals in both the registry
initialization and the conditional that checks pluginId in pluginManager.ts;
specifically update the check that currently uses (pluginId === 'dev-novel' ||
pluginId === 'dev-paged-novel') to reference the new constants and ensure the
registry initialization also uses the same constants to avoid duplication and
typos.

src/hooks/persisted/usePlugins.ts (1)

53-76: Consider extracting dev plugin PluginItem mapping to reduce duplication.

The same property mapping pattern is repeated for both dev plugins. A helper function would reduce duplication.

♻️ Suggested refactor

const toPluginItem = (plugin: typeof devNovelPlugin): PluginItem => ({
  id: plugin.id,
  name: plugin.name,
  site: plugin.site,
  lang: plugin.lang,
  version: plugin.version,
  url: plugin.url,
  iconUrl: plugin.iconUrl,
  hasSettings: !!plugin.pluginSettings,
});

const devPlugins: PluginItem[] = __DEV__
  ? [toPluginItem(devNovelPlugin), toPluginItem(devPagedNovelPlugin)]
  : [];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/usePlugins.ts` around lines 53 - 76, Extract the repeated
mapping of devNovelPlugin and devPagedNovelPlugin into a small helper (e.g.,
toPluginItem) that converts a plugin (devNovelPlugin/devPagedNovelPlugin) into a
PluginItem by copying id, name, site, lang, version, url, iconUrl and computing
hasSettings as !!plugin.pluginSettings; then replace the inline objects in the
devPlugins array with calls to that helper (devPlugins, devNovelPlugin,
devPagedNovelPlugin, PluginItem, toPluginItem).

src/screens/reader/components/ChapterDrawer/index.tsx (1)

43-58: Consider extracting useNovelDomainValue to a shared utility.

This helper function is duplicated across multiple files (NovelScreenList.tsx, NovelScreen.tsx, useChapter.ts). Extract it to a shared location (e.g., @hooks/persisted/useNovel/useNovelDomainValue.ts) to reduce duplication and ensure consistency.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/screens/reader/components/ChapterDrawer/index.tsx` around lines 43 - 58,
Extract the duplicated hook useNovelDomainValue into a shared utility and update
callers to import it instead of redefining it; create a new file (e.g.,
`@hooks/persisted/useNovel/useNovelDomainValue.ts`) that exports
useNovelDomainValue with the same signature and implementation (it should
reference useNovelContext's novelStore, useCallback for subscribe/getSnapshot,
and call novelStore.subscribe and novelStore.getState with the selector), then
replace the local definitions in ChapterDrawer (index.tsx), NovelScreenList.tsx,
NovelScreen.tsx, and useChapter.ts with an import from the new module to remove
duplication and keep behavior identical.

src/screens/reader/components/ChapterDrawer/__tests__/ChapterDrawer.test.tsx (1)

135-162: Consider adding more test cases for edge conditions.

The current test covers the happy path but doesn't test:

• When fetching is true (should not call getNextChapterBatch)
• When batch >= total (should not call getNextChapterBatch)
• When pages array is empty or chapter page is not found

💡 Additional test cases to consider

it('does not call getNextChapterBatch when fetching is true', async () => {
  const store = createStore({ fetching: true });
  // ... setup and render
  fireEvent.press(screen.getByTestId('legend-end-reached'));
  expect(store.state.getNextChapterBatch).not.toHaveBeenCalled();
});

it('does not call getNextChapterBatch when batch >= total', async () => {
  const store = createStore({ 
    batchInformation: { batch: 1, total: 1, totalChapters: 2 } 
  });
  // ... setup and render
  fireEvent.press(screen.getByTestId('legend-end-reached'));
  expect(store.state.getNextChapterBatch).not.toHaveBeenCalled();
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/screens/reader/components/ChapterDrawer/__tests__/ChapterDrawer.test.tsx`
around lines 135 - 162, Add unit tests in ChapterDrawer.test.tsx that cover the
edge conditions the reviewer suggested: create variants of createStore with
fetching: true, with batchInformation where batch >= total, and with pages empty
or missing the current chapter page; for each test use mockUseChapterContext and
mockUseNovelContext as in the existing test, render <ChapterDrawer />, trigger
the end-reached event via
fireEvent.press(screen.getByTestId('legend-end-reached')), and assert that
store.state.getNextChapterBatch is NOT called; also keep the existing
expectation that setPageIndex is invoked where appropriate to ensure
selector-backed behavior remains tested.

src/screens/novel/components/NovelScreenList.tsx (1)

108-130: Consider consolidating multiple selector subscriptions.

Each useNovelDomainValue call creates a separate subscription. With 16+ subscriptions in this component, consider using a single combined selector that returns an object with all needed values, then destructure it. This reduces subscription overhead and re-render frequency.

♻️ Example consolidated approach

const selectNovelScreenListState = (state: NovelStoreState) => ({
  chapters: state.chapters,
  deleteChapter: state.deleteChapter,
  fetching: state.fetching,
  // ... other needed values
});

// In component:
const {
  chapters,
  deleteChapter,
  fetching,
  // ...
} = useNovelDomainValue(novelStore, selectNovelScreenListState);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/screens/novel/components/NovelScreenList.tsx` around lines 108 - 130,
Multiple calls to useNovelDomainValue (e.g., useNovelDomainValue(novelStore,
selectChapters), selectDeleteChapter, selectFetching, selectFirstUnreadChapter,
selectLoading, selectNovelSettings, selectPages, selectSetNovel, selectNovel,
selectBatchInformation, selectPageIndex, selectOpenPage, selectUpdateChapter,
selectRefreshNovel, selectLastRead) create many subscriptions; replace them with
a single consolidated selector (e.g., selectNovelScreenListState) that returns
an object with all required fields, call useNovelDomainValue once with that
selector, and then destructure the returned object into chapters, deleteChapter,
fetching, firstUnreadChapter, loading, novelSettings, pages, setNovel,
fetchedNovel, batchInformation, pageIndex, openPage, updateChapter,
refreshNovel, lastRead to reduce subscription overhead and re-renders.

src/screens/novel/__tests__/NovelScreen.test.tsx (1)

43-48: Assert the store boundary here, not the DB helper.

This test is supposed to protect the novelStore cutover, but the progress-reset expectation still hangs off @database/queries/ChapterQueries.updateChapterProgressByIds. That couples the suite to an implementation detail instead of the screen’s public dependency.

Possible refactor

 const state = {
   novel: baseNovel,
   chapters: [],
   fetching: false,
   batchInformation: { batch: 0, total: 0, totalChapters: 0 },
   getNextChapterBatch: jest.fn(),
   loadUpToBatch: jest.fn(),
   setNovel: jest.fn(),
   bookmarkChapters: jest.fn(),
   markChaptersRead: jest.fn(),
   markChaptersUnread: jest.fn(),
+  updateChapterProgressByIds: jest.fn(),
   markPreviouschaptersRead: jest.fn(),
   markPreviousChaptersUnread: jest.fn(),
   refreshChapters: jest.fn(),
   deleteChapters: jest.fn(),
   ...overrides,
@@
     fireEvent.press(screen.getByTestId('select-read'));
     fireEvent.press(screen.getByTestId('action-check-outline'));
 
     expect(store.state.markChaptersUnread).toHaveBeenCalledTimes(1);
-    expect(mockUpdateChapterProgressByIds).toHaveBeenCalledWith([11], 0);
+    expect(store.state.updateChapterProgressByIds).toHaveBeenCalledWith(
+      [11],
+      0,
+    );
     expect(store.state.refreshChapters).toHaveBeenCalledTimes(1);
   });

Also applies to: 223-239, 290-305

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/screens/novel/__tests__/NovelScreen.test.tsx` around lines 43 - 48, The
test currently asserts behavior on the DB helper updateChapterProgressByIds
which ties the spec to an implementation detail; instead spy or mock the public
store API (novelStore) used by the screen and assert the store boundary was
invoked. Replace expectations that reference
`@database/queries/ChapterQueries.updateChapterProgressByIds` with assertions
against the novelStore method the screen calls (e.g., the store method that
resets/updates chapter progress), and update the mocks in the three affected
blocks (around lines 43-48, 223-239, 290-305) to stop verifying the DB helper
and verify novelStore was called with the expected ids/args; keep the existing
jest.mock of getAllUndownloadedAndUnreadChapters/getAllUndownloadedChapters but
move the progress-reset assertion to the store-level spy.

src/hooks/persisted/useNovel/novelStore.ts (2)

331-335: Address ESLint warning: prefer explicit async handling over void.

ESLint flags the void state.getChapters() expression. Consider using .catch() to explicitly handle potential errors, or refactor to make the intent clearer.

♻️ Suggested fix

       const state = get();
       if (state.novel && state.pages.length > 0) {
-        void state.getChapters();
+        state.getChapters().catch(() => {
+          // Fire-and-forget: errors handled internally
+        });
       }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/novelStore.ts` around lines 331 - 335, The
expression using void to ignore the Promise from state.getChapters() should be
replaced with explicit async/error handling: either call
state.getChapters().catch(err => { /* log or handle error */ }) to handle
rejection, or make the enclosing callback async and await state.getChapters()
inside a try/catch; update the code around get() in novelStore.ts (the section
that reads const state = get(); if (state.novel && state.pages.length > 0) { ...
}) to use one of these patterns and ensure any errors are logged or handled
instead of being swallowed.

---

340-362: Address ESLint warning and add error handling for followNovel.

The void deps.switchNovelToLibrary(...) pattern triggers the ESLint warning. Additionally, if switchNovelToLibrary fails, the UI state will remain unchanged, but users won't receive feedback. Consider adding error handling.

♻️ Suggested improvement

      followNovel: () => {
        const state = get();
        const currentNovel = state.novel;
        if (!currentNovel || !deps.switchNovelToLibrary) {
          return;
        }

-       void deps
-         .switchNovelToLibrary(state.novelPath, state.pluginId)
+       deps
+         .switchNovelToLibrary(state.novelPath, state.pluginId)
          .then(() => {
            set(inner => {
              if (!inner.novel) {
                return {};
              }
              return {
                novel: {
                  ...inner.novel,
                  inLibrary: !inner.novel.inLibrary,
                },
              };
            });
+         })
+         .catch(() => {
+           // Optionally show error toast via deps
          });
      },

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/novelStore.ts` around lines 340 - 362, The
followNovel function currently uses "void deps.switchNovelToLibrary(...)" which
triggers ESLint and lacks error handling; change followNovel to an async
function, await deps.switchNovelToLibrary(state.novelPath, state.pluginId)
inside a try/catch, and move the state toggle (set(...)) into the try block so
it only runs on success; in the catch block, surface the failure (e.g., call a
provided notifier like deps.notifyError or at minimum console.error with the
caught error) so users get feedback when switchNovelToLibrary fails and ESLint
warnings are resolved.

src/screens/novel/NovelContext.tsx (1)

45-94: Consider stabilizing route.params to prevent store recreation.

Including route.params directly in the dependency array may cause unnecessary store recreation if the params object reference changes between renders while the actual path and pluginId values remain the same. Since path and pluginId are already extracted and used individually, consider removing route.params from the deps or extracting the id check outside the memo.

♻️ Suggested improvement

+ const routeNovel = 'id' in route.params ? (route.params as NovelInfo) : undefined;
+
  const novelStore = useMemo(
    () =>
      createNovelStore({
        pluginId,
        novelPath: path,
-       novel: 'id' in route.params ? (route.params as NovelInfo) : undefined,
+       novel: routeNovel,
        defaultChapterSort,
        // ... rest unchanged
      }),
-   [defaultChapterSort, path, pluginId, route.params, switchNovelToLibrary],
+   [defaultChapterSort, path, pluginId, routeNovel, switchNovelToLibrary],
  );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/screens/novel/NovelContext.tsx` around lines 45 - 94, The memo currently
includes route.params in the dependency array which can cause unnecessary
re-creation of novelStore; remove route.params from the deps and instead derive
the NovelInfo check outside the useMemo (e.g. compute a local variable like
initialNovel = 'id' in route.params ? (route.params as NovelInfo) : undefined)
and pass that stable value (or just rely on path and pluginId) into
createNovelStore; update the useMemo dependency array to [defaultChapterSort,
path, pluginId, switchNovelToLibrary] so novelStore (created by
createNovelStore) only rebuilds when actual identifying values change.

src/plugins/dev/DevPagedNovelPlugin.ts (1)

252-363: Consider extracting shared page calculation logic.

The page calculation logic (lines 262-294 and 322-354) is duplicated between parseNovel and parsePage. Consider extracting this into a helper function for maintainability.

♻️ Suggested extraction

const calculateTotalPages = async (
  novelPath: string,
  newContent: boolean,
  newPages: boolean,
): Promise<number> => {
  const res = await dbManager
    .select({ count: novelSchema.totalChapters })
    .from(novelSchema)
    .where(
      and(
        eq(novelSchema.pluginId, 'dev-paged-novel'),
        eq(novelSchema.path, novelPath),
      ),
    )
    .get();

  const currentChapters = res?.count || 0;
  let totalPages = Math.ceil(currentChapters / CHAPTERS_PER_PAGE);

  if (totalPages < DEFAULT_PAGES) {
    totalPages = DEFAULT_PAGES;
  }

  if (newPages) {
    totalPages += 2;
  } else if (newContent) {
    const extraChapters = Math.min(
      5,
      CHAPTERS_PER_PAGE - (totalPages * CHAPTERS_PER_PAGE - currentChapters),
    );
    if (extraChapters > 0) {
      totalPages = Math.ceil(
        (currentChapters + extraChapters) / CHAPTERS_PER_PAGE,
      );
    }
  }

  return totalPages;
};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/dev/DevPagedNovelPlugin.ts` around lines 252 - 363, Both
parseNovel and parsePage duplicate the page-calculation logic; extract that
logic into a new async helper (e.g., calculateTotalPages) that accepts
novelPath, newContent, newPages and returns totalPages by performing the
dbManager select on novelSchema, applying CHAPTERS_PER_PAGE and DEFAULT_PAGES
rules and the newPages/newContent adjustments; then replace the duplicated
blocks in parseNovel and parsePage to call calculateTotalPages(novelPath,
newContent, newPages) and use its result for generating chapters and page bounds
checks (references: parseNovel, parsePage, dbManager, novelSchema,
CHAPTERS_PER_PAGE, DEFAULT_PAGES, Storage keys 'provideNewContent' and
'provideNewPages').

src/hooks/persisted/useNovelSettings.ts (1)

37-48: setChapterSort dependency array may cause stale closures.

The callback references novelSettings?.showChapterTitles and _filter, but novelSettings object itself changes on every settings update. Consider whether including the entire novelSettings object would be more robust.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovelSettings.ts` around lines 37 - 48, The
setChapterSort callback can capture stale values because it depends on
novelSettings?.showChapterTitles and _filter while novelSettings is a new object
on each update; update the dependency array to include novelSettings (the whole
object) and _filter so the hook re-creates when settings change, and keep using
novelSettings?.showChapterTitles inside the body when calling
writeNovelSettings; ensure the dependency list reads [novel, writeNovelSettings,
novelSettings, _filter] targeting the setChapterSort function.

src/screens/novel/NovelContext.tsx (1)

157-163: Consider whether layoutValue memoization is correct.

The useMemo has an empty dependency array, but the value depends on navigationBarHeightRef.current and statusBarHeightRef.current which may change. Since refs don't trigger re-renders, this memo will only compute once. If this is intentional (to capture initial layout values), this is fine. Otherwise, consider if the layout needs to update when insets change.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/screens/novel/NovelContext.tsx` around lines 157 - 163, layoutValue is
memoized with an empty dependency array but reads navigationBarHeightRef.current
and statusBarHeightRef.current, so it won't update when those refs change;
either include the values that drive layoutValue in the dependency list (e.g.
navigationBarHeightRef.current and statusBarHeightRef.current) or convert the
refs into state and update that state when insets change, then memoize using
that state; update the code around layoutValue, navigationBarHeightRef,
statusBarHeightRef and the useMemo call accordingly so layoutValue reflects
changes.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/services/updates/LibraryUpdateQueries.ts (1)

197-197: ⚠️ Potential issue | 🟠 Major

Remove debug console output from the update path.

console.log on Line 197 is still violating no-console and will keep ESLint failing. Please remove it (or swap to a dev-gated logger).

Proposed fix

-  console.log(novel.chapters);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/services/updates/LibraryUpdateQueries.ts` at line 197, Remove the debug
console output: delete the console.log(novel.chapters) call in
LibraryUpdateQueries (the update path handling where novel.chapters is being
printed) or replace it with the project logger behind a dev-only gate (e.g.,
process.env.NODE_ENV check) so ESLint no-console no longer fails; ensure any
retained logging uses the existing logging utility used elsewhere in this module
instead of console.log.

src/screens/novel/NovelScreen.tsx (1)

37-39: ⚠️ Potential issue | 🟡 Minor

Remove the render-time debug log.

This still trips no-console, and because it runs during render it will log on every store-driven re-render of the screen.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/screens/novel/NovelScreen.tsx` around lines 37 - 39, Remove the
render-time console.log by deleting the console.log('Novel render', novel?.name,
chapters.length) call in the NovelScreen component; if you need persistent
debugging, move the log into an effect such as useEffect that watches
useNovelValue('novel') and useNovelValue('chapters') or replace with a proper
logger obeying no-console rules, referencing the useNovelValue hook and the
novel/chapters variables.

src/hooks/__tests__/useNovel.test.ts (1)

37-93: ⚠️ Potential issue | 🟡 Minor

Seed the legacy keys you assert on.

This test now verifies deletion of novelPersistence.keys.settings() and novelPersistence.keys.lastRead(), but the setup still only writes the keyContract.* variants. As written, those assertions stay green even if the legacy-key cleanup path stops working.

Suggested fix

       MMKVStorage.set(
         keyContract.settings({
           pluginId: novel.pluginId,
           novelPath: novel.path,
         }),
         JSON.stringify({ filter: [], showChapterTitles: true }),
       );
+      MMKVStorage.set(
+        novelPersistence.keys.settings({
+          pluginId: novel.pluginId,
+          novelPath: novel.path,
+        }),
+        JSON.stringify({ filter: [], showChapterTitles: true }),
+      );
       MMKVStorage.set(
         keyContract.lastRead({
           pluginId: novel.pluginId,
           novelPath: novel.path,
         }),
         JSON.stringify({ id: 1 }),
       );
+      MMKVStorage.set(
+        novelPersistence.keys.lastRead({
+          pluginId: novel.pluginId,
+          novelPath: novel.path,
+        }),
+        JSON.stringify({ id: 1 }),
+      );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/__tests__/useNovel.test.ts` around lines 37 - 93, The test seeds
only the keyContract.* variants but asserts that legacy keys
(novelPersistence.keys.settings and novelPersistence.keys.lastRead) are removed;
modify the setup loop in the test so that for each novel you also write entries
using novelPersistence.keys.settings({pluginId: novel.pluginId, novelPath:
novel.path}) and novelPersistence.keys.lastRead({pluginId: novel.pluginId,
novelPath: novel.path}) (e.g. via MMKVStorage.set with the same payloads you
currently use for keyContract.pageIndex/settings/lastRead) so the assertions
actually validate legacy-key cleanup.

src/services/plugin/fetch.ts (1)

48-53: ⚠️ Potential issue | 🟡 Minor

Differentiate unsupported pagination from a novel fetch failure.

For page !== '1' without parsePage(), this still throws the generic novel error. That makes logs and caller handling much less actionable than an explicit "paged fetch unsupported" failure.

Suggested fix

-  if (plugin.parseNovel && page === '1') {
-    const res = await plugin.parseNovel(novelPath);
-    if (res.chapters) return { chapters: res.chapters };
-    throw new Error(`Could not fetch chapters for novel ${novelPath}`);
-  }
-  throw new Error(`Could not fetch chapters for novel ${novelPath}`);
+  if (page !== '1') {
+    throw new Error(
+      `Plugin ${pluginId} cannot fetch chapters for page ${page} of ${novelPath} without parsePage()`,
+    );
+  }
+
+  const res = await plugin.parseNovel(novelPath);
+  if (res.chapters) return { chapters: res.chapters };
+  throw new Error(`Could not fetch chapters for novel ${novelPath}`);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/services/plugin/fetch.ts` around lines 48 - 53, The current block always
throws the generic novel fetch error even when the caller requested a page
beyond '1' but the plugin doesn't implement pagination; update the logic in the
fetch routine handling plugin.parseNovel so that if page !== '1' and
plugin.parsePage is not provided you throw a distinct "paged fetch unsupported"
(or similar) error, otherwise preserve the existing behavior: call
plugin.parseNovel(novelPath) when page === '1', return { chapters: res.chapters
} if present, and if parseNovel returns no chapters throw the existing "Could
not fetch chapters for novel ..." error; reference plugin.parseNovel,
plugin.parsePage, novelPath and page to locate and implement this conditional
change.

src/screens/novel/components/NovelScreenList.tsx (1)

481-481: ⚠️ Potential issue | 🟡 Minor

Remove the render-time debug log.

This runs on every list render and still fails no-console.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/screens/novel/components/NovelScreenList.tsx` at line 481, Remove the
runtime debug console output in NovelScreenList.tsx: delete the
console.log('rendering chapter', fetching, chapters.length) call (or replace it
with a conditional/debug logger that respects the app's logging mechanism)
inside the component that renders the chapter list so the render path no longer
violates the no-console rule; locate the statement in the component where
chapters and fetching are available (the render function or JSX body) and remove
or guard it.

src/hooks/__tests__/mocks.ts (3)

125-127: ⚠️ Potential issue | 🟠 Major

Export the selector hooks from the shared NovelContext mock too.

src/hooks/persisted/useNovelSettings.ts now imports useNovelValue and useNovelAction, and src/screens/novel/components/NovelScreenList.tsx imports useNovelValue and useNovelActions. With only useNovelContext mocked here, tests that bootstrap this shared helper will still crash on those newer imports.

Possible fix

 jest.mock('@screens/novel/NovelContext', () => ({
   useNovelContext: () => mockUseNovelContext(),
+  useNovelValue: (key: string) =>
+    mockUseNovelContext().novelStore.getState()[key],
+  useNovelAction: (key: string) =>
+    mockUseNovelContext().novelStore.getState()[key],
+  useNovelActions: () => {
+    const state = mockUseNovelContext().novelStore.getState();
+    return {
+      deleteChapter: state.deleteChapter,
+      setNovel: state.setNovel,
+      getNextChapterBatch: state.getNextChapterBatch,
+      openPage: state.openPage,
+      updateChapter: state.updateChapter,
+      refreshNovel: state.refreshNovel,
+    };
+  },
 }));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/__tests__/mocks.ts` around lines 125 - 127, The shared NovelContext
mock only exports useNovelContext; extend that jest.mock call to also export the
selector hooks used across the codebase—specifically add exports for
useNovelValue, useNovelAction and useNovelActions (matching the actual import
names used by useNovelSettings.ts and NovelScreenList.tsx) and have them
delegate to the corresponding test helpers (e.g., mockUseNovelValue,
mockUseNovelAction, mockUseNovelActions) so tests importing those hooks don't
crash.

---

98-123: ⚠️ Potential issue | 🟠 Major

Make the mock store notify subscribers and stop reusing one singleton instance.

Selector hooks rely on the external-store contract. Right now setState() never notifies listeners, subscribe() is inert, and defaultMockNovelContext shares one mutable store across tests.

Possible fix

 export const createMockNovelStore = (
   stateOverrides: Record<string, unknown> = {},
 ) => {
   let state = createMockNovelStoreState(stateOverrides);
+  const listeners = new Set<() => void>();

   return {
     getState: jest.fn(() => state),
     setState: jest.fn(nextState => {
       const partial =
         typeof nextState === 'function' ? nextState(state) : nextState;
       state = {
         ...state,
         ...partial,
       };
+      listeners.forEach(listener => listener());
     }),
-    subscribe: jest.fn(() => () => {}),
+    subscribe: jest.fn(listener => {
+      listeners.add(listener);
+      return () => listeners.delete(listener);
+    }),
   };
 };
 
-const defaultMockNovelContext = {
-  novelStore: createMockNovelStore(),
-  navigationBarHeight: 0,
-  statusBarHeight: 0,
-};
-
-export const mockUseNovelContext = jest.fn(() => defaultMockNovelContext);
+const createDefaultMockNovelContext = () => ({
+  novelStore: createMockNovelStore(),
+  navigationBarHeight: 0,
+  statusBarHeight: 0,
+});
+
+export const mockUseNovelContext = jest.fn(() =>
+  createDefaultMockNovelContext(),
+);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/__tests__/mocks.ts` around lines 98 - 123, The mock store currently
never notifies listeners and defaultMockNovelContext reuses a single mutable
store across tests; update createMockNovelStore to maintain a subscribers
Set/array, have subscribe(fn) add the listener and return an unsubscribe
function that removes it, and call all subscribers with the new state inside
setState (handle functional and object updates as currently done); also stop
sharing one singleton by making mockUseNovelContext return a fresh context
(create a new novelStore via createMockNovelStore() each call) instead of using
defaultMockNovelContext so each test gets an isolated store.

---

56-59: ⚠️ Potential issue | 🟡 Minor

Include totalChapters in the shared batch mock.

batchInformation.totalChapters is now read by the novel screen, so leaving it out here keeps test state behind the production shape.

Possible fix

   batchInformation: {
     batch: 0,
     total: 0,
+    totalChapters: 0,
   },

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/__tests__/mocks.ts` around lines 56 - 59, The shared test mock's
batchInformation object is missing the totalChapters property used by the novel
screen; update the mock in src/hooks/__tests__/mocks.ts by adding totalChapters
(e.g., totalChapters: 0) to the batchInformation object so the test shape
matches production and prevents missing-property errors when code reads
batchInformation.totalChapters.

src/hooks/persisted/useNovel/__tests__/persistence.test.ts (1)

167-173: ⚠️ Potential issue | 🟡 Minor

Rename this test so it describes a runtime export check, not compile-time safety.

Destructuring from require() here only proves that the exports exist at runtime. It does not give the same compile-time guarantees as a typed import, so the current title overstates the protection this test provides.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/__tests__/persistence.test.ts` around lines 167
- 173, Rename the test title to reflect that it checks runtime exports rather
than compile-time safety: update the it(...) description currently asserting
typeof novelPersistence.copySettings, novelPersistence.readLastRead, and
migrateNovel to something like "verifies runtime exports exist for migrateNovel
and novelPersistence functions"; keep the same body that requires
'@services/migrate/migrateNovel' and asserts typeof for
novelPersistence.copySettings, novelPersistence.readLastRead, and migrateNovel
so the test intent matches the actual runtime-only check.

src/screens/novel/components/__tests__/NovelScreenList.test.tsx (1)

9-11: ⚠️ Potential issue | 🟠 Major

Update the NovelContext mock to export the selector hooks this component now uses.

NovelScreenList no longer reads everything from useNovelContext(). With only that one export mocked here, the test will still fail on useNovelValue / useNovelActions before it reaches any assertions.

Possible fix

 jest.mock('../../NovelContext', () => ({
   useNovelContext: () => mockUseNovelContext(),
+  useNovelValue: (key: string) =>
+    mockUseNovelContext().novelStore.getState()[key],
+  useNovelActions: () => {
+    const state = mockUseNovelContext().novelStore.getState();
+    return {
+      deleteChapter: state.deleteChapter,
+      setNovel: state.setNovel,
+      getNextChapterBatch: state.getNextChapterBatch,
+      openPage: state.openPage,
+      updateChapter: state.updateChapter,
+      refreshNovel: state.refreshNovel,
+    };
+  },
 }));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/screens/novel/components/__tests__/NovelScreenList.test.tsx` around lines
9 - 11, The test mock for NovelContext currently only exports useNovelContext
which causes missing exports for the selector hooks; update the jest.mock to
also export useNovelValue and useNovelActions (e.g., map them to
mockUseNovelValue and mockUseNovelActions or return appropriate stub
implementations) so NovelScreenList can import those hooks during the test;
ensure the exported names exactly match useNovelValue and useNovelActions and
keep useNovelContext mapped to mockUseNovelContext for compatibility.

src/hooks/persisted/useNovel/store-helper/bootstrapService.ts (4)

172-184: ⚠️ Potential issue | 🟠 Major

Don't turn loader failures into empty successful states.

The catch blocks only console.error and continue with [], so a DB/query failure is surfaced as a successful bootstrap or batch load with missing chapters. Let the error bubble or return an explicit failure result.

🛠️ Proposed fix

     let newChapters: ChapterInfo[] = [];
-    try {
-      newChapters =
-        (await getPageChaptersBatched(
-          novel.id,
-          settingsSort,
-          settingsFilter,
-          page,
-          nextBatch,
-        )) || [];
-    } catch (error) {
-      console.error('teaser', error);
-    }
+    newChapters =
+      (await getPageChaptersBatched(
+        novel.id,
+        settingsSort,
+        settingsFilter,
+        page,
+        nextBatch,
+      )) || [];

Also applies to lines 223-235.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/store-helper/bootstrapService.ts` around lines
172 - 184, The catch around getPageChaptersBatched swallows errors and returns
an empty array (newChapters), turning loader failures into successful empty
states; instead either rethrow the caught error or return an explicit failure
result from the enclosing function (do not replace with []), so update the
try/catch in bootstrapService (the block that calls getPageChaptersBatched and
the similar block at lines 223-235) to propagate the error (throw error) or
return a Result/Error object that upstream code can handle rather than assigning
[] to newChapters.

---

54-57: ⚠️ Potential issue | 🟠 Major

Scope the inflight cache to the full bootstrap request.

The promise cache is keyed only by pluginId + novelPath, but the returned chapter state also depends on pageIndex, settingsSort, and settingsFilter. Two concurrent bootstraps for the same novel with different reader settings can reuse the wrong promise. Additionally, the module-level Map leaks dedupe across separate createBootstrapService() instances.

🛠️ Proposed fix

-const inflightBootstraps = new Map<string, Promise<BootstrapResult>>();
-
-const getBootstrapKey = (pluginId: string, novelPath: string) =>
-  `${pluginId}_${novelPath}`;
-
 export const createBootstrapService = () => {
+  const inflightBootstraps = new Map<string, Promise<BootstrapResult>>();
+
+  const getBootstrapKey = ({
+    pluginId,
+    novelPath,
+    pageIndex,
+    settingsSort,
+    settingsFilter,
+  }: {
+    pluginId: string;
+    novelPath: string;
+    pageIndex: number;
+    settingsSort: ChapterOrderKey;
+    settingsFilter: ChapterFilterKey[];
+  }) =>
+    JSON.stringify([pluginId, novelPath, pageIndex, settingsSort, settingsFilter]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/store-helper/bootstrapService.ts` around lines
54 - 57, The inflight cache Map (inflightBootstraps) is too coarse and
module-scoped causing wrong promise reuse and cross-instance leakage; update
getBootstrapKey to include pageIndex, settingsSort, and settingsFilter (e.g.,
`${pluginId}_${novelPath}_${pageIndex}_${settingsSort}_${settingsFilter}`) and
move the inflightBootstraps Map into the createBootstrapService() factory so
each service instance has its own cache; adjust any callers of getBootstrapKey/
inflightBootstraps within bootstrapService.ts to use the expanded key and the
instance-scoped Map to ensure dedupe is per-full-request and per-service
instance.

---

137-141: ⚠️ Potential issue | 🟡 Minor

Fix the zero-based batch total calculation.

Batch 0 is already loaded, so Math.floor(chapterCount / 1000) overstates total for exact multiples of 1000 and allows one phantom follow-up fetch. Compute the last zero-based batch index instead.

🛠️ Proposed fix

     const batchInformation: BatchInfo = {
       batch: 0,
-      total: Math.floor(chapterCount / 1000),
+      total: Math.max(0, Math.ceil(chapterCount / 1000) - 1),
       totalChapters: chapterCount,
     };

Also applies to line 353.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/store-helper/bootstrapService.ts` around lines
137 - 141, The batch total is computed as a 1-based count but stored as a
zero-based last index; update the BatchInfo initialization (batchInformation) to
set total to the last zero-based batch index by using Math.max(0,
Math.ceil(chapterCount / 1000) - 1) (instead of Math.floor(chapterCount /
1000)), and make the same change for the second occurrence in this file (the
other total assignment around the later batch computation).

---

112-123: ⚠️ Potential issue | 🟠 Major

Replace console statements with proper logging to fix pipeline failures.

Multiple console.time, console.error, and console.timeEnd statements are causing ESLint no-console failures. Either remove debug statements or use an application logger.

🛠️ Proposed fix - remove debug timing and error swallowing

   const getChaptersForPage = async ({
     ...
   }): Promise<ChapterLoadResult> => {
-    console.time('getChaptersForPage');
     const page = pages[pageIndex] ?? '1';
     let newChapters: ChapterInfo[] = [];
     const config = [novel.id, settingsSort, settingsFilter, page] as const;

     let chapterCount = await getChapterCount(novel.id, page);
     if (chapterCount) {
-      try {
-        newChapters = (await getPageChaptersBatched(...config)) || [];
-      } catch (error) {
-        console.error('Error fetching chapters:', error);
-      }
+      newChapters = (await getPageChaptersBatched(...config)) || [];
     } else if (Number(page)) {

Apply similar changes to lines 143, 183, 234, 256, and 304.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/store-helper/bootstrapService.ts` around lines
112 - 123, The code uses console.time/console.timeEnd and console.error inside
the chapter-fetching block (around getChapterCount and getPageChaptersBatched)
which triggers ESLint no-console; replace these with the application logger
(e.g., logger or processLogger used in the project) and include the error object
in the log call instead of swallowing it; remove timing calls or convert them to
logger.debug/trace if timing is needed; update the console.* occurrences in the
getChaptersForPage block and apply the same replacement pattern to the other
occurrences referenced (lines near the other blocks) so all console.time,
console.timeEnd, and console.error calls are replaced with logger.debug/trace
and logger.error respectively and preserve the error details when logging.

src/hooks/persisted/useNovel/store/chapterActions.ts (2)

188-208: ⚠️ Potential issue | 🟠 Major

State/persistence inconsistency: clamped value persisted but raw value stored in memory.

Line 194 persists Math.min(progress, 100), but line 204 stores the raw progress in local state. If progress > 100, the DB will have 100 while the UI shows the unclamped value.

🐛 Proposed fix: Clamp both values consistently

 export const updateChapterProgressAction = (
   chapterId: number,
   progress: number,
   mutateChapters: MutateChapters,
   deps: ChapterActionsDependencies = defaultChapterActionsDependencies,
 ) => {
-  deps.updateChapterProgress(chapterId, Math.min(progress, 100));
+  const clampedProgress = Math.max(0, Math.min(progress, 100));
+  deps.updateChapterProgress(chapterId, clampedProgress);

   mutateChapters(chs =>
     chs.map(c => {
       if (c.id !== chapterId) {
         return c;
       }

       return {
         ...c,
-        progress,
+        progress: clampedProgress,
       };
     }),
   );
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/store/chapterActions.ts` around lines 188 - 208,
The updateChapterProgressAction currently persists a clamped value but writes
the raw progress into memory; change the implementation so both persistence and
in-memory state use the same clamped value by computing a local const clamped =
Math.min(progress, 100) and passing clamped to
deps.updateChapterProgress(chapterId, clamped) and using clamped when updating
the chapter in mutateChapters (inside the map that returns {...c, progress}).
Ensure you only modify updateChapterProgressAction and keep MutateChapters/deps
usage the same.

---

210-263: ⚠️ Potential issue | 🟡 Minor

Missing error handling on delete operations.

deleteChapter and deleteChapters use .then() without .catch(). If the delete fails, the promise rejection goes unhandled and the UI state may become inconsistent (state not updated, but no error feedback).

🛡️ Proposed fix: Add error handling

 export const deleteChapterAction = (
   _chapter: ChapterInfo,
   novel: NovelInfo | undefined,
   mutateChapters: MutateChapters,
   deps: ChapterActionsDependencies = defaultChapterActionsDependencies,
 ) => {
   if (novel) {
-    deps.deleteChapter(novel.pluginId, novel.id, _chapter.id).then(() => {
+    deps.deleteChapter(novel.pluginId, novel.id, _chapter.id)
+      .then(() => {
         mutateChapters(chs =>
           chs.map(chapter => {
             if (chapter.id !== _chapter.id) {
               return chapter;
             }
             return { ...chapter, isDownloaded: false };
           }),
         );
         deps.showToast(deps.getString('common.deleted', { name: _chapter.name }));
-      });
+      })
+      .catch(() => {
+        deps.showToast(deps.getString('common.error'));
+      });
   }
 };

Apply the same pattern to deleteChaptersAction.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/store/chapterActions.ts` around lines 210 - 263,
Both deleteChapterAction and deleteChaptersAction currently call
deps.deleteChapter / deps.deleteChapters with .then() but no .catch(), leaving
promise rejections unhandled; add a .catch(error => {
console.error('deleteChapter(s) failed', error);
deps.showToast(deps.getString('common.deleteFailed', { message: error?.message
|| String(error) })); }) to each promise chain (or convert to async/await with
try/catch) so errors are logged and the user gets feedback; keep the existing
mutateChapters updates inside the success path so state only changes on
successful deletion and include the unique symbols deleteChapterAction,
deleteChaptersAction, deps.deleteChapter, deps.deleteChapters, mutateChapters
and deps.showToast to locate where to add the handlers.

src/hooks/persisted/useNovel/store-helper/persistence.ts (1)

69-94: ⚠️ Potential issue | 🟡 Minor

Reject negative or fractional page indices.

Number.isFinite accepts values like -1 and 1.5, so the bridge can persist/read indices that cause pages[pageIndex] to resolve to undefined, silently falling back to page '1'. Normalize both legacy-string parsing and new writes to a non-negative integer.

🛠️ Proposed fix

   const readPageIndex = (input: NovelPersistenceInput): number => {
     const key = keys.pageIndex(input);
     const numberValue = storage.getNumber(key);

-    if (typeof numberValue === 'number' && Number.isFinite(numberValue)) {
+    if (
+      typeof numberValue === 'number' &&
+      Number.isInteger(numberValue) &&
+      numberValue >= 0
+    ) {
       return numberValue;
     }

     const stringValue = storage.getString(key);
     if (stringValue !== undefined) {
       const parsed = Number(stringValue);
-      if (Number.isFinite(parsed)) {
+      if (Number.isInteger(parsed) && parsed >= 0) {
         storage.set(key, parsed);
         return parsed;
       }
     }

     storage.set(key, defaultPageIndex);
     return defaultPageIndex;
   };

   const writePageIndex = (input: NovelPersistenceInput, value: number) => {
     const key = keys.pageIndex(input);
-    const safeValue = Number.isFinite(value) ? value : defaultPageIndex;
+    const safeValue =
+      Number.isInteger(value) && value >= 0 ? value : defaultPageIndex;
     storage.set(key, safeValue);
   };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/store-helper/persistence.ts` around lines 69 -
94, The readPageIndex and writePageIndex logic currently accepts negative and
fractional values because it only checks Number.isFinite; update both functions
(keys.pageIndex, readPageIndex, writePageIndex) to only accept non-negative
integers: replace the Number.isFinite checks with Number.isInteger(value) &&
value >= 0 for the numeric path (storage.getNumber), and when parsing
stringValue ensure parsed is an integer >= 0 before calling storage.set;
similarly compute safeValue in writePageIndex as value if
Number.isInteger(value) && value >= 0 else defaultPageIndex so only valid
non-negative integers are persisted.

src/database/queries/ChapterQueries.ts (1)

453-454: Batch size increased from 300 to 1000.

This 3x increase in batch size will reduce the number of queries needed but increases memory usage per batch. Ensure this is acceptable for your target devices' memory constraints.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/database/queries/ChapterQueries.ts` around lines 453 - 454, The batch
size was changed from 300 to 1000 in ChapterQueries.ts (see the constants limit
and offset calculation), which may spike memory usage; either revert limit back
to 300 or make the batch size configurable (e.g., a BATCH_SIZE constant/env var)
and add validation or comments documenting expected memory impact and
recommended limits for target devices; update any callers or tests that assume
the old size and ensure offset calculation (offset = limit * batch) uses the
same variable name to remain consistent.

src/utils/mmkv/zustand-adapter.ts (2)

19-25: Replace console.error with a logging utility.

The console.error statements violate the no-console ESLint rule. Consider using the project's logging utility or conditionally logging in __DEV__ mode.

💡 Suggested approach

+import { __DEV__ } from '@utils/constants'; // or similar

 getItem: (key: string): string | null => {
   try {
     const value = MMKVStorage.getString(key);
     return value ?? null;
   } catch (error) {
-    console.error(
-      `[mmkvZustandAdapter] Error getting item for key "${key}":`,
-      error,
-    );
+    if (__DEV__) {
+      // eslint-disable-next-line no-console
+      console.error(`[mmkvZustandAdapter] Error getting item for key "${key}":`, error);
+    }
     return null;
   }
 },

Apply similar changes to setItem and removeItem.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/mmkv/zustand-adapter.ts` around lines 19 - 25, Replace
console.error calls in mmkvZustandAdapter functions with the project's logging
utility (or guard logs behind __DEV__) to satisfy no-console linting: in
getItem, setItem, and removeItem replace the console.error usage with a call to
the logger (e.g., use the existing logger instance or import createLogger) or
wrap the console call in if (__DEV__) { ... } and include the key and error
details in the log message; ensure you import or reference the same logging
utility used elsewhere so all three functions (getItem, setItem, removeItem)
consistently log errors without using console directly.

---

32-41: Silent failures in storage operations may mask data loss.

While catching errors prevents crashes, silently swallowing setItem failures means the app may continue believing state is persisted when it isn't. Consider whether critical write failures should surface to the caller or trigger a user notification.

Also applies to: 46-55

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/mmkv/zustand-adapter.ts` around lines 32 - 41, The current MMKV
adapter methods (e.g., setItem) swallow errors after logging, which can hide
write failures; update setItem and the corresponding methods referenced
(removeItem, and whichever method occupies lines ~46-55) to surface failures to
callers by either rethrowing the caught error after logging or returning a clear
failure result (e.g., boolean or error value) so callers can react; ensure the
method signatures (setItem, removeItem, getItem or similar) are updated
consistently and unit tests/usage sites handle the propagated error or returned
status.

src/screens/novel/components/ChapterItem.tsx (1)

101-106: Consider moving parseTime outside the component or memoizing it.

The function is recreated on every render. While cheap, moving it outside the component (since it has no dependencies on props/state) would be cleaner.

♻️ Suggested refactor

 import dayjs from 'dayjs';

+function parseTime(time?: string | Date | null) {
+  if (!time) return undefined;
+  const parsedTime = dayjs(time);
+  return parsedTime.isValid() ? parsedTime.format('LL') : (time as string);
+}
+
 const ChapterItem: React.FC<ChapterItemProps> = ({
   ...
 }) => {
   ...
-  function parseTime(time?: string | Date | null) {
-    if (!time) return undefined;
-    const parsedTime = dayjs(time);
-    return parsedTime.isValid() ? parsedTime.format('LL') : (time as string);
-  }
   const parsedTime = parseTime(releaseTime);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/screens/novel/components/ChapterItem.tsx` around lines 101 - 106, The
parseTime helper inside ChapterItem is recreated on every render; move the
parseTime(time?: string | Date | null) function declaration out of the component
scope (so it becomes a module-level helper) or, if you prefer to keep it inside,
memoize it with useCallback/useMemo tied to releaseTime; ensure parseTime still
uses dayjs and that parsedTime = parseTime(releaseTime) continues to work
unchanged in the component.

src/screens/novel/__tests__/NovelScreen.test.tsx (1)

54-58: Consider avoiding shadowed React variable in mock factories.

ESLint flags that React is redeclared inside the mock factory. While this works, you can avoid the warning by using a different variable name or restructuring.

♻️ Suggested approach

 jest.mock('../components/NovelAppbar', () => {
-  const React = require('react');
+  const { createElement } = require('react');
   const { Text } = require('react-native');
-  return () => React.createElement(Text, { testID: 'novel-appbar' }, 'appbar');
+  return () => createElement(Text, { testID: 'novel-appbar' }, 'appbar');
 });

Apply similarly to other mock factories at lines 61, 123, 150, 158, 183, 190, 196, and 207.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/screens/novel/__tests__/NovelScreen.test.tsx` around lines 54 - 58, The
mock factory for '../components/NovelAppbar' declares a shadowed React variable
which triggers ESLint; update the mock to avoid redeclaring React by either
importing React at top-level and using that name inside the factory or renaming
the local require to a different identifier (e.g., ReactLib or RN) so you don't
shadow the outer React symbol; apply the same change to the other mock factories
mentioned (the mocks for the other components in this test file) to remove all
shadowed React declarations.

src/hooks/persisted/useNovel.ts (1)

31-42: Redundant key removal: novelPersistence.keys.* delegates to keyContract.*.

Looking at persistence.ts (context snippet 2), novelPersistence.keys.pageIndex calls keyContract.pageIndex internally. Both generate the same key, so lines 35-37 and 39-41 remove the same three keys twice.

♻️ Proposed simplification

 const clearNovelPersistence = ({
   pluginId,
   novelPath,
 }: NovelPersistenceInput) => {
-  MMKVStorage.remove(keyContract.pageIndex({ pluginId, novelPath }));
-  MMKVStorage.remove(keyContract.settings({ pluginId, novelPath }));
-  MMKVStorage.remove(keyContract.lastRead({ pluginId, novelPath }));
-
-  MMKVStorage.remove(novelPersistence.keys.pageIndex({ pluginId, novelPath }));
-  MMKVStorage.remove(novelPersistence.keys.settings({ pluginId, novelPath }));
-  MMKVStorage.remove(novelPersistence.keys.lastRead({ pluginId, novelPath }));
+  const input = { pluginId, novelPath };
+  MMKVStorage.remove(novelPersistence.keys.pageIndex(input));
+  MMKVStorage.remove(novelPersistence.keys.settings(input));
+  MMKVStorage.remove(novelPersistence.keys.lastRead(input));
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel.ts` around lines 31 - 42, The
clearNovelPersistence function is removing the same keys twice because
novelPersistence.keys.pageIndex/settings/lastRead delegate to
keyContract.pageIndex/settings/lastRead; remove the redundant calls by keeping
only one set of MMKVStorage.remove calls (either the keyContract.* calls or the
novelPersistence.keys.* calls) so each of the three keys (pageIndex, settings,
lastRead) is removed exactly once; update clearNovelPersistence to call
MMKVStorage.remove for either keyContract.pageIndex/settings/lastRead or for
novelPersistence.keys.pageIndex/settings/lastRead and delete the other trio of
remove calls.

src/plugins/dev/DevPagedNovelPlugin.ts (1)

252-363: Extract duplicated totalPages calculation logic.

Both parseNovel (lines 258-294) and parsePage (lines 318-354) contain identical logic for:

1. Fetching currentChapters from the database
2. Calculating totalPages based on settings

Since this is dev-only code the impact is lower, but extracting a shared helper would improve maintainability.

♻️ Suggested refactor

const getTotalPages = async (
  novelPath: string,
  storage: Storage,
): Promise<number> => {
  const newContent = storage.get('provideNewContent');
  const newPages = storage.get('provideNewPages');

  const res = await dbManager
    .select({ count: novelSchema.totalChapters })
    .from(novelSchema)
    .where(
      and(
        eq(novelSchema.pluginId, 'dev-paged-novel'),
        eq(novelSchema.path, novelPath),
      ),
    )
    .get();

  const currentChapters = res?.count || 0;
  let totalPages = Math.ceil(currentChapters / CHAPTERS_PER_PAGE);

  if (totalPages < DEFAULT_PAGES) {
    totalPages = DEFAULT_PAGES;
  }

  if (newPages) {
    totalPages += 2;
  } else if (newContent) {
    const extraChapters = Math.min(
      5,
      CHAPTERS_PER_PAGE - (totalPages * CHAPTERS_PER_PAGE - currentChapters),
    );
    if (extraChapters > 0) {
      totalPages = Math.ceil(
        (currentChapters + extraChapters) / CHAPTERS_PER_PAGE,
      );
    }
  }

  return totalPages;
};

Then use in both parseNovel and parsePage.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/dev/DevPagedNovelPlugin.ts` around lines 252 - 363, Duplicate
logic for computing totalPages appears in parseNovel and parsePage; extract it
into a shared async helper (e.g., getTotalPages) that accepts novelPath and a
Storage instance, moves the dbManager query, currentChapters calculation,
DEFAULT_PAGES/CHAPTERS_PER_PAGE adjustments and newPages/newContent handling
into that helper, and then call getTotalPages(...) from both parseNovel and
parsePage (replace the repeated blocks with await getTotalPages(novelPath, s)).
Ensure helper returns a number and use the exact symbols novelSchema, dbManager,
CHAPTERS_PER_PAGE, DEFAULT_PAGES, and Storage from the module.

src/hooks/persisted/useNovel/store-helper/persistence.ts (1)

109-112: Redundant delete before set.

Line 110 deletes the key, then line 111 immediately sets a new value. The set operation will overwrite regardless, making the delete unnecessary.

♻️ Proposed fix

     if (isValidNovelSettings(parsed)) {
       return parsed;
     }

-    storage.delete(key);
     storage.set(key, JSON.stringify(defaultNovelSettings));
     return defaultNovelSettings;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/persisted/useNovel/store-helper/persistence.ts` around lines 109 -
112, The storage.delete call before immediately calling storage.set is
redundant; remove the storage.delete(key) line and simply call storage.set(key,
JSON.stringify(defaultNovelSettings)) and return defaultNovelSettings (keep
references to storage.set, key, and defaultNovelSettings in the persistence
helper).