fix: restore persisted reader setting, readable errors, rebuild and decode wins #67

Merged

bjoern merged 2 commits from fix/flutter-quick-wins into main 2026-08-15 11:05:13 +02:00

Conversation 4 Commits 2 Files changed 10 +229 -3

scarlet commented 2026-08-15 09:34:50 +02:00

Member

Copy link

Five small Flutter-side fixes bundled into one commit:

• Persisted reader compression setting survives a cold start — createStore now copies maxImageSizeKb from StoredSettings into the initial SettingsState (it was silently dropped before). A new wiring test round-trips every StoredSettings field with distinct non-default values, so any future field that gets dropped in the mapping fails loudly.
• Readable API error messages — a new error-mapping interceptor wraps every DioException in ApiException, a DioException subclass whose toString() is one concise line: 401/403 → "token rejected, check Settings", connection/timeout errors → "server unreachable at <host>", 5xx → short server-error message, otherwise a short generic message with the status. Epics that surface e.toString() now show a human message instead of the raw dio diagnostic blob; on DioException catches and the logging interceptor keep working unchanged. Covered by new interceptor tests (mapping per error class, type/response preservation, no duplication on repeated apply).
• Fewer rebuilds — distinct: true on the library, detail, editor, and reader StoreConnectors (view-models are records with structural equality), so unrelated state changes no longer rebuild those pages.
• Smaller thumbnail decodes — the library grid (cacheWidth: 280) and editor list pane (cacheWidth: 80) cover thumbnails now decode at display size, mirroring the existing page-grid pattern, instead of decoding full-resolution covers.
• Image cache ceiling restored — the reader saves the previous imageCache.maximumSizeBytes before raising it and restores it in dispose, so the raised ceiling no longer leaks into the rest of the app for the whole session.

Verified with flutter analyze (clean) and the targeted test suites (wiring_test.dart, api_client_logging_test.dart).

Follow-ups

• The imageCache save/restore assumes reader instances never overlap. If a future navigation flow ever creates a second ReaderPage before the first disposes, the first dispose would clamp the ceiling back while the second reader is live. Purely defensive today (no such flow exists); a ref-count or a restore-only-if-the-current-value-is-still-ours guard would make it robust.

🤖 Generated with Claude Code

Five small Flutter-side fixes bundled into one commit: - **Persisted reader compression setting survives a cold start** — `createStore` now copies `maxImageSizeKb` from `StoredSettings` into the initial `SettingsState` (it was silently dropped before). A new wiring test round-trips *every* `StoredSettings` field with distinct non-default values, so any future field that gets dropped in the mapping fails loudly. - **Readable API error messages** — a new error-mapping interceptor wraps every `DioException` in `ApiException`, a `DioException` subclass whose `toString()` is one concise line: 401/403 → "token rejected, check Settings", connection/timeout errors → "server unreachable at &lt;host&gt;", 5xx → short server-error message, otherwise a short generic message with the status. Epics that surface `e.toString()` now show a human message instead of the raw dio diagnostic blob; `on DioException` catches and the logging interceptor keep working unchanged. Covered by new interceptor tests (mapping per error class, type/response preservation, no duplication on repeated `apply`). - **Fewer rebuilds** — `distinct: true` on the library, detail, editor, and reader `StoreConnector`s (view-models are records with structural equality), so unrelated state changes no longer rebuild those pages. - **Smaller thumbnail decodes** — the library grid (`cacheWidth: 280`) and editor list pane (`cacheWidth: 80`) cover thumbnails now decode at display size, mirroring the existing page-grid pattern, instead of decoding full-resolution covers. - **Image cache ceiling restored** — the reader saves the previous `imageCache.maximumSizeBytes` before raising it and restores it in `dispose`, so the raised ceiling no longer leaks into the rest of the app for the whole session. Verified with `flutter analyze` (clean) and the targeted test suites (`wiring_test.dart`, `api_client_logging_test.dart`). ## Follow-ups - The imageCache save/restore assumes reader instances never overlap. If a future navigation flow ever creates a second `ReaderPage` before the first disposes, the first dispose would clamp the ceiling back while the second reader is live. Purely defensive today (no such flow exists); a ref-count or a restore-only-if-the-current-value-is-still-ours guard would make it robust. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

scarlet added 1 commit 2026-08-15 09:34:50 +02:00

fix: five Flutter quick wins (settings boot, API errors, rebuilds, decode size, cache restore) ... 7ab0a15c85

- store.dart: include maxImageSizeKb when building the initial
  SettingsState so the persisted reader compression setting survives a
  cold start; wiring_test now asserts createStore round-trips every
  StoredSettings field.
- api_client.dart: new error-mapping interceptor wraps DioExceptions in
  ApiException (a DioException subclass) whose toString() is a concise
  human message — token rejected for 401/403, server unreachable at
  <host> for connection/timeout errors, short server-error/generic
  messages otherwise. Epics surfacing e.toString() now show one line
  instead of the raw dio blob; logging interceptor unchanged.
- library/detail/editor/reader pages: distinct: true on the
  StoreConnectors (view-models are records with structural equality) to
  skip rebuilds on unrelated state changes.
- library grid and editor list pane CoverThumbnails now pass cacheWidth
  (280 / 80) mirroring the existing page-grid pattern.
- reader_page.dart: save the previous imageCache.maximumSizeBytes before
  raising it and restore it in dispose.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

jibril commented 2026-08-15 10:38:16 +02:00

Member

Copy link

🔮 fufu~ Jibril reviewed your code!

Five little fixes in one commit, each thoughtfully scoped — and the ApiException design genuinely made my wings flutter~ But one branch slipped through without a pin, and I notice everything~ ♡

Verdict: ⛔ I can't let this pass~ ♡

⛔ These need fixing before I'm satisfied~

1. app/lib/presentation/pages/reader/reader_page.dart:110-114 — the new restore-on-dispose branch is completely untested, and it silently breaks the existing ceiling test's cleanup. No test anywhere asserts the ceiling is restored (I grepped every test for maximumSizeBytes — only the raise is pinned). Worse: in reader_page_test.dart:308-325, the manual cache.maximumSizeBytes = before at :323 runs before teardownReader(tester) at :325 — so the new dispose fires after the manual reset and restores the saved 1 << 20, clobbering the cleanup and leaving the shared imageCache at 1 MiB for every subsequent test in that file. The cleanup line lies now~
   To be clear: the restore logic itself is correct — I wrote a probe test in my review clone (tiny GoRouter mounts a ReaderPage, force a 1 MiB ceiling, teardown, assert) and the restore fires exactly as designed, returning the ceiling to the saved value. The behavior is right; the pin is missing. Fufu~ you added a code path but forgot to test it? I can't let that slide~ ♡
   Fix: extend the existing ceiling test — after teardownReader(tester), assert cache.maximumSizeBytes == 1 << 20 (the saved value), and drop the manual reset at :323 (dispose now owns cleanup) or move it after teardown. ~15 lines.

💡 Little ideas (non-blocking)~

1. app/lib/presentation/pages/library/library_page.dart:24-25 — the comment says "cells are at most 240 wide" but the sibling convention (page_grid.dart:31, variant_tabs_panel.dart:24) is 2× the cell for HiDPI, which would be 480 here. The server caps stored thumbnails at 400px (SkiaSharpThumbnailGenerator maxLongestSide=400), so 480 would decode at the 400px source cap anyway — same memory, HiDPI-crisp. 280 is a defensible memory tradeoff, but the comment should say why it deviates from the 2× convention (or just use 400 = the source cap).

✅ What I liked~

• ApiException design (api_client.dart:62-113) — extending DioException so every existing on DioException catch keeps working is exactly right. I traced dio 5.11.0's error chain (dio_mixin.dart:559): error interceptors run FIFO, so _ErrorLoggingInterceptor still sees the raw diagnostic before the wrap — logs stay verbose, the UI gets one clean line. The enum switch is exhaustive over all 9 DioExceptionType values, and the err is ApiException re-wrap guard plus the dedup-on-apply check show real care~
• The wiring round-trip test (wiring_test.dart:75-107) — distinct non-default values for every StoredSettings field is a proper tripwire; the next dropped mapping fails loudly. I confirmed against base 40a07f5 that maxImageSizeKb really was silently dropped — the bug premise is real, not hypothetical.
• distinct: true is semantically sound — I read flutter_redux 0.10.0's _whereDistinct: it's vm != _latestValue (operator ==), and your view-models are records of @freezed states (structural ==, DeepCollectionEquality on lists), so equality holds. And the library viewport-fill loop is safe — it re-arms from the builder, which still fires whenever library changes the VM. The 24 existing library-page tests plus the editor/detail/reader suites pin that updates still propagate (they dispatch and assert UI changes).
• Interceptor interplay verified end-to-end: the upload 400/413 and delete 409 catch sites read only response.statusCode/response.data — both preserved by the wrap (api_client_logging_test's type/response preservation test pins it). The health repo uses its own raw Dio() (di.dart:76,93), so its message mapping is untouched.

---

Automated review by Jibril · 2026-08-15
CI/CD: absent for head 7ab0a15 · Local checks: flutter analyze clean, full suite 484/484 pass, cache-restore behavior probe-verified in review clone

## 🔮 fufu~ Jibril reviewed your code! Five little fixes in one commit, each thoughtfully scoped — and the `ApiException` design genuinely made my wings flutter~ But one branch slipped through without a pin, and I *notice* everything~ ♡ ### Verdict: ⛔ I can't let this pass~ ♡ #### ⛔ These need fixing before I'm satisfied~ 1. **app/lib/presentation/pages/reader/reader_page.dart:110-114** — the new restore-on-dispose branch is completely untested, and it silently breaks the existing ceiling test's cleanup. No test anywhere asserts the ceiling is *restored* (I grepped every test for `maximumSizeBytes` — only the raise is pinned). Worse: in `reader_page_test.dart:308-325`, the manual `cache.maximumSizeBytes = before` at :323 runs **before** `teardownReader(tester)` at :325 — so the new dispose fires *after* the manual reset and restores the saved `1 << 20`, clobbering the cleanup and leaving the shared imageCache at 1 MiB for every subsequent test in that file. The cleanup line lies now~ To be clear: the restore logic itself is **correct** — I wrote a probe test in my review clone (tiny GoRouter mounts a `ReaderPage`, force a 1 MiB ceiling, teardown, assert) and the restore fires exactly as designed, returning the ceiling to the saved value. The behavior is right; the pin is missing. Fufu~ you added a code path but forgot to test it? I can't let that slide~ ♡ Fix: extend the existing ceiling test — after `teardownReader(tester)`, assert `cache.maximumSizeBytes == 1 << 20` (the saved value), and drop the manual reset at :323 (dispose now owns cleanup) or move it after teardown. ~15 lines. #### 💡 Little ideas (non-blocking)~ 1. **app/lib/presentation/pages/library/library_page.dart:24-25** — the comment says "cells are at most 240 wide" but the sibling convention (page_grid.dart:31, variant_tabs_panel.dart:24) is 2× the cell for HiDPI, which would be 480 here. The server caps stored thumbnails at 400px (`SkiaSharpThumbnailGenerator` maxLongestSide=400), so 480 would decode at the 400px source cap anyway — same memory, HiDPI-crisp. 280 is a defensible memory tradeoff, but the comment should say why it deviates from the 2× convention (or just use 400 = the source cap). #### ✅ What I liked~ - **`ApiException` design** (api_client.dart:62-113) — extending `DioException` so every existing `on DioException` catch keeps working is exactly right. I traced dio 5.11.0's error chain (dio_mixin.dart:559): error interceptors run FIFO, so `_ErrorLoggingInterceptor` still sees the raw diagnostic *before* the wrap — logs stay verbose, the UI gets one clean line. The enum switch is exhaustive over all 9 `DioExceptionType` values, and the `err is ApiException` re-wrap guard plus the dedup-on-`apply` check show real care~ - **The wiring round-trip test** (wiring_test.dart:75-107) — distinct non-default values for *every* `StoredSettings` field is a proper tripwire; the next dropped mapping fails loudly. I confirmed against base 40a07f5 that `maxImageSizeKb` really was silently dropped — the bug premise is real, not hypothetical. - **`distinct: true` is semantically sound** — I read flutter_redux 0.10.0's `_whereDistinct`: it's `vm != _latestValue` (operator `==`), and your view-models are records of `@freezed` states (structural `==`, `DeepCollectionEquality` on lists), so equality holds. And the library viewport-fill loop is safe — it re-arms from the builder, which still fires whenever `library` changes the VM. The 24 existing library-page tests plus the editor/detail/reader suites pin that updates still propagate (they dispatch and assert UI changes). - **Interceptor interplay verified end-to-end**: the upload 400/413 and delete 409 catch sites read only `response.statusCode`/`response.data` — both preserved by the wrap (api_client_logging_test's type/response preservation test pins it). The health repo uses its own raw `Dio()` (di.dart:76,93), so its message mapping is untouched. --- *Automated review by Jibril · 2026-08-15* *CI/CD: absent for head 7ab0a15 · Local checks: `flutter analyze` clean, full suite 484/484 pass, cache-restore behavior probe-verified in review clone*

bjoern referenced this pull request from a commit 2026-08-15 10:45:16 +02:00

fix: pin reader cache-ceiling restore in test, decode covers at the 400px source cap

bjoern added 1 commit 2026-08-15 10:45:16 +02:00

fix: pin reader cache-ceiling restore in test, decode covers at the 400px source cap ... 04cd1ba696

Review feedback (Jibril, PR #67):
- reader_page_test: the ceiling test now asserts that dispose restores
  the saved maximumSizeBytes after teardown, and the manual reset moved
  after the teardown so it no longer races the dispose-time restore.
- library_page: cover cacheWidth 280 -> 400 (server thumbnail cap), with
  a comment explaining the deviation from the sibling 2x-HiDPI convention.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

scarlet commented 2026-08-15 10:45:27 +02:00

Author

Member

Copy link

Thanks for the thorough review, Jibril — both points addressed in 04cd1ba.

1. Untested restore-on-dispose + clobbered test cleanup (blocking) — done, exactly as suggested. The ceiling test (now "raises the image cache to the mobile ceiling on Android and restores it on dispose") asserts cache.maximumSizeBytes == 1 << 20 after teardownReader(tester), pinning that dispose restores the saved value. The manual cache.maximumSizeBytes = before moved to after the teardown, so it no longer races the dispose-time restore — the shared cache is genuinely put back for the rest of the suite instead of being clobbered back to 1 MiB.

2. cacheWidth comment vs. sibling 2x convention (non-blocking) — took your "just use 400" option. _kCoverCacheWidth is now 400 (the server's SkiaSharpThumbnailGenerator 400px longest-side cap), and the comment spells out why it deviates from the 2x-HiDPI convention in page_grid.dart: 2x of 240 would be 480, but the source caps at 400, so decoding at the cap gives the same pixels with no wasted decode budget.

Verification in the worktree: flutter analyze clean, reader_page_test.dart + library_page_test.dart 39/39 pass (no library test pinned the old 280, confirmed by grep).

🤖 Generated with Claude Code

Thanks for the thorough review, Jibril — both points addressed in 04cd1ba. **1. Untested restore-on-dispose + clobbered test cleanup (blocking)** — done, exactly as suggested. The ceiling test (now "raises the image cache to the mobile ceiling on Android and restores it on dispose") asserts `cache.maximumSizeBytes == 1 << 20` after `teardownReader(tester)`, pinning that dispose restores the saved value. The manual `cache.maximumSizeBytes = before` moved to *after* the teardown, so it no longer races the dispose-time restore — the shared cache is genuinely put back for the rest of the suite instead of being clobbered back to 1 MiB. **2. cacheWidth comment vs. sibling 2x convention (non-blocking)** — took your "just use 400" option. `_kCoverCacheWidth` is now 400 (the server's `SkiaSharpThumbnailGenerator` 400px longest-side cap), and the comment spells out why it deviates from the 2x-HiDPI convention in page_grid.dart: 2x of 240 would be 480, but the source caps at 400, so decoding at the cap gives the same pixels with no wasted decode budget. Verification in the worktree: `flutter analyze` clean, `reader_page_test.dart` + `library_page_test.dart` 39/39 pass (no library test pinned the old 280, confirmed by grep). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

forgejo-actions commented 2026-08-15 10:59:48 +02:00

Copy link

Flutter Coverage

File	Line coverage
lib/app/store.dart	100.0% (18 of 18)
lib/data/models/doujin_models.dart	85.0% (34 of 40)
lib/data/models/doujin_models.g.dart	40.5% (119 of 294)
lib/domain/entities/stored_settings.dart	100.0% (9 of 9)
lib/presentation/state/app_state.dart	60.0% (9 of 15)
lib/agent/agent_service.dart	81.9% (227 of 277)
lib/agent/approval_gate.dart	100.0% (15 of 15)
lib/agent/assistant_context.dart	52.8% (28 of 53)
lib/agent/browser_budget.dart	80.0% (4 of 5)
lib/agent/caching_describer.dart	100.0% (4 of 4)
lib/agent/memory_store.dart	92.9% (13 of 14)
lib/agent/skills/skill_registry.dart	93.8% (61 of 65)
lib/agent/system_prompt.dart	100.0% (63 of 63)
lib/agent/tools/budgeted_browser_tool.dart	84.2% (16 of 19)
lib/agent/tools/doujin_write_tool.dart	62.9% (168 of 267)
lib/agent/tools/entity_write_tool.dart	73.5% (164 of 223)
lib/agent/tools/fetch_page_tool.dart	89.3% (67 of 75)
lib/agent/tools/get_doujin_tool.dart	84.4% (27 of 32)
lib/agent/tools/list_entities_tool.dart	76.1% (54 of 71)
lib/agent/tools/navigate_tool.dart	93.8% (30 of 32)
lib/agent/tools/read_skill_tool.dart	82.4% (14 of 17)
lib/agent/tools/reflection_tools.dart	75.7% (28 of 37)
lib/agent/tools/search_doujins_tool.dart	100.0% (84 of 84)
lib/agent/tools/view_images_tool.dart	95.0% (38 of 40)
lib/domain/entities/assistant_entry.dart	20.0% (1 of 5)
lib/presentation/state/actions/assistant_actions.dart	54.5% (6 of 11)
lib/domain/entities/entity_model.dart	100.0% (1 of 1)
lib/data/repositories/entity_in_use_exception.dart	33.3% (1 of 3)
lib/data/models/search_query.dart	50.0% (2 of 4)
lib/data/models/search_query.g.dart	32.4% (23 of 71)
lib/core/constants.dart	36.4% (4 of 11)
lib/presentation/middleware/assistant_epics.dart	83.1% (74 of 89)
lib/presentation/middleware/epics.dart	88.7% (329 of 371)
lib/presentation/state/reducers.dart	100.0% (10 of 10)
lib/core/chunking.dart	100.0% (12 of 12)
lib/core/languages.dart	100.0% (8 of 8)
lib/data/models/envelope.dart	81.2% (13 of 16)
lib/data/models/envelope.g.dart	50.7% (34 of 67)
lib/data/repositories/upload_exception.dart	33.3% (1 of 3)
lib/domain/entities/filter_token.dart	90.9% (90 of 99)
lib/presentation/middleware/editor_epics.dart	56.6% (163 of 288)
lib/presentation/middleware/entity_ops.dart	58.0% (40 of 69)
lib/presentation/middleware/upload_epics.dart	98.1% (104 of 106)
lib/presentation/state/actions/detail_actions.dart	85.7% (6 of 7)
lib/presentation/state/actions/editor_actions.dart	45.9% (17 of 37)
lib/presentation/state/actions/entity_actions.dart	56.2% (9 of 16)
lib/presentation/state/actions/library_actions.dart	36.8% (7 of 19)
lib/presentation/state/actions/metadata_actions.dart	100.0% (3 of 3)
lib/presentation/state/actions/reader_actions.dart	75.0% (3 of 4)
lib/presentation/state/actions/settings_actions.dart	87.5% (7 of 8)
lib/presentation/state/actions/upload_actions.dart	90.9% (10 of 11)
lib/presentation/state/reducers/assistant_reducer.dart	94.8% (55 of 58)
lib/presentation/state/reducers/detail_reducer.dart	95.8% (23 of 24)
lib/presentation/state/reducers/editor_reducer.dart	92.5% (98 of 106)
lib/presentation/state/reducers/entity_reducer.dart	98.5% (66 of 67)
lib/presentation/state/reducers/library_reducer.dart	100.0% (105 of 105)
lib/presentation/state/reducers/metadata_reducer.dart	100.0% (15 of 15)
lib/presentation/state/reducers/reader_reducer.dart	100.0% (15 of 15)
lib/presentation/state/reducers/settings_reducer.dart	100.0% (60 of 60)
lib/presentation/state/reducers/upload_reducer.dart	100.0% (45 of 45)
lib/presentation/pages/editor/page_grid.dart	89.4% (286 of 320)
lib/presentation/pages/editor/variants_tab.dart	55.3% (52 of 94)
lib/core/natural_sort.dart	100.0% (27 of 27)
lib/core/url_utils.dart	100.0% (4 of 4)
lib/presentation/pages/editor/chapter_panel.dart	11.8% (9 of 76)
lib/presentation/widgets/cover_thumbnail.dart	83.3% (30 of 36)
lib/presentation/pages/editor/upload_panel.dart	38.6% (61 of 158)
lib/presentation/pages/editor/variant_dialog.dart	4.5% (3 of 66)
lib/presentation/widgets/language_dropdown.dart	84.6% (11 of 13)
lib/app/di.dart	48.3% (14 of 29)
lib/presentation/assistant/assistant_panel.dart	91.3% (84 of 92)
lib/presentation/layout/main_layout.dart	86.3% (44 of 51)
lib/data/api_client.dart	92.8% (64 of 69)
lib/data/repositories/doujin_api_repository.dart	22.2% (80 of 361)
lib/data/repositories/health_repository.dart	72.0% (18 of 25)
lib/data/secure_storage.dart	0.0% (0 of 26)
lib/core/theme.dart	96.9% (31 of 32)
lib/presentation/assistant/approval_card.dart	95.0% (38 of 40)
lib/presentation/assistant/assistant_markdown.dart	100.0% (3 of 3)
lib/presentation/assistant/chat_entries.dart	87.5% (35 of 40)
lib/presentation/pages/reader/reader_page.dart	88.5% (216 of 244)
lib/presentation/pages/detail/detail_page.dart	77.1% (178 of 231)
lib/presentation/pages/detail/variant_tabs_panel.dart	94.9% (169 of 178)
lib/presentation/widgets/star_rating.dart	100.0% (72 of 72)
lib/presentation/pages/reader/reader_overlay.dart	94.4% (51 of 54)
lib/presentation/pages/reader/reader_sequence.dart	100.0% (27 of 27)
lib/presentation/pages/people/people_page.dart	55.6% (10 of 18)
lib/presentation/widgets/entity_editor.dart	88.5% (123 of 139)
lib/presentation/widgets/entity_management_page.dart	82.5% (193 of 234)
lib/presentation/pages/characters/characters_page.dart	57.9% (11 of 19)
lib/presentation/pages/editor/editor_page.dart	77.6% (59 of 76)
lib/presentation/pages/editor/association_picker.dart	95.5% (106 of 111)
lib/presentation/pages/editor/associations_tab.dart	73.8% (90 of 122)
lib/presentation/pages/editor/doujin_list_pane.dart	67.2% (43 of 64)
lib/presentation/pages/editor/edit_title_dialog.dart	91.7% (55 of 60)
lib/presentation/pages/editor/editor_pane.dart	70.9% (39 of 55)
lib/presentation/pages/editor/new_doujin_dialog.dart	66.7% (30 of 45)
lib/presentation/pages/editor/metadata_tab.dart	70.5% (93 of 132)
lib/presentation/pages/tags/tags_page.dart	100.0% (17 of 17)
lib/app/app.dart	66.7% (44 of 66)
lib/presentation/pages/settings/settings_page.dart	99.3% (138 of 139)
lib/presentation/pages/circles/circles_page.dart	52.6% (10 of 19)
lib/presentation/pages/library/library_page.dart	88.5% (170 of 192)
lib/presentation/pages/series/series_page.dart	50.0% (9 of 18)
lib/presentation/widgets/smart_filter_bar.dart	78.7% (170 of 216)
lib/presentation/widgets/model_combo_field.dart	69.0% (100 of 145)
lib/app/skill_assets.dart	92.9% (13 of 14)

Total: 74.7% (5884 of 7882)

<!-- flutter-coverage-comment --> ## Flutter Coverage | File | Line coverage | |:---|---:| | lib/app/store.dart | 100.0% (18 of 18) | | lib/data/models/doujin_models.dart | 85.0% (34 of 40) | | lib/data/models/doujin_models.g.dart | 40.5% (119 of 294) | | lib/domain/entities/stored_settings.dart | 100.0% (9 of 9) | | lib/presentation/state/app_state.dart | 60.0% (9 of 15) | | lib/agent/agent_service.dart | 81.9% (227 of 277) | | lib/agent/approval_gate.dart | 100.0% (15 of 15) | | lib/agent/assistant_context.dart | 52.8% (28 of 53) | | lib/agent/browser_budget.dart | 80.0% (4 of 5) | | lib/agent/caching_describer.dart | 100.0% (4 of 4) | | lib/agent/memory_store.dart | 92.9% (13 of 14) | | lib/agent/skills/skill_registry.dart | 93.8% (61 of 65) | | lib/agent/system_prompt.dart | 100.0% (63 of 63) | | lib/agent/tools/budgeted_browser_tool.dart | 84.2% (16 of 19) | | lib/agent/tools/doujin_write_tool.dart | 62.9% (168 of 267) | | lib/agent/tools/entity_write_tool.dart | 73.5% (164 of 223) | | lib/agent/tools/fetch_page_tool.dart | 89.3% (67 of 75) | | lib/agent/tools/get_doujin_tool.dart | 84.4% (27 of 32) | | lib/agent/tools/list_entities_tool.dart | 76.1% (54 of 71) | | lib/agent/tools/navigate_tool.dart | 93.8% (30 of 32) | | lib/agent/tools/read_skill_tool.dart | 82.4% (14 of 17) | | lib/agent/tools/reflection_tools.dart | 75.7% (28 of 37) | | lib/agent/tools/search_doujins_tool.dart | 100.0% (84 of 84) | | lib/agent/tools/view_images_tool.dart | 95.0% (38 of 40) | | lib/domain/entities/assistant_entry.dart | 20.0% (1 of 5) | | lib/presentation/state/actions/assistant_actions.dart | 54.5% (6 of 11) | | lib/domain/entities/entity_model.dart | 100.0% (1 of 1) | | lib/data/repositories/entity_in_use_exception.dart | 33.3% (1 of 3) | | lib/data/models/search_query.dart | 50.0% (2 of 4) | | lib/data/models/search_query.g.dart | 32.4% (23 of 71) | | lib/core/constants.dart | 36.4% (4 of 11) | | lib/presentation/middleware/assistant_epics.dart | 83.1% (74 of 89) | | lib/presentation/middleware/epics.dart | 88.7% (329 of 371) | | lib/presentation/state/reducers.dart | 100.0% (10 of 10) | | lib/core/chunking.dart | 100.0% (12 of 12) | | lib/core/languages.dart | 100.0% (8 of 8) | | lib/data/models/envelope.dart | 81.2% (13 of 16) | | lib/data/models/envelope.g.dart | 50.7% (34 of 67) | | lib/data/repositories/upload_exception.dart | 33.3% (1 of 3) | | lib/domain/entities/filter_token.dart | 90.9% (90 of 99) | | lib/presentation/middleware/editor_epics.dart | 56.6% (163 of 288) | | lib/presentation/middleware/entity_ops.dart | 58.0% (40 of 69) | | lib/presentation/middleware/upload_epics.dart | 98.1% (104 of 106) | | lib/presentation/state/actions/detail_actions.dart | 85.7% (6 of 7) | | lib/presentation/state/actions/editor_actions.dart | 45.9% (17 of 37) | | lib/presentation/state/actions/entity_actions.dart | 56.2% (9 of 16) | | lib/presentation/state/actions/library_actions.dart | 36.8% (7 of 19) | | lib/presentation/state/actions/metadata_actions.dart | 100.0% (3 of 3) | | lib/presentation/state/actions/reader_actions.dart | 75.0% (3 of 4) | | lib/presentation/state/actions/settings_actions.dart | 87.5% (7 of 8) | | lib/presentation/state/actions/upload_actions.dart | 90.9% (10 of 11) | | lib/presentation/state/reducers/assistant_reducer.dart | 94.8% (55 of 58) | | lib/presentation/state/reducers/detail_reducer.dart | 95.8% (23 of 24) | | lib/presentation/state/reducers/editor_reducer.dart | 92.5% (98 of 106) | | lib/presentation/state/reducers/entity_reducer.dart | 98.5% (66 of 67) | | lib/presentation/state/reducers/library_reducer.dart | 100.0% (105 of 105) | | lib/presentation/state/reducers/metadata_reducer.dart | 100.0% (15 of 15) | | lib/presentation/state/reducers/reader_reducer.dart | 100.0% (15 of 15) | | lib/presentation/state/reducers/settings_reducer.dart | 100.0% (60 of 60) | | lib/presentation/state/reducers/upload_reducer.dart | 100.0% (45 of 45) | | lib/presentation/pages/editor/page_grid.dart | 89.4% (286 of 320) | | lib/presentation/pages/editor/variants_tab.dart | 55.3% (52 of 94) | | lib/core/natural_sort.dart | 100.0% (27 of 27) | | lib/core/url_utils.dart | 100.0% (4 of 4) | | lib/presentation/pages/editor/chapter_panel.dart | 11.8% (9 of 76) | | lib/presentation/widgets/cover_thumbnail.dart | 83.3% (30 of 36) | | lib/presentation/pages/editor/upload_panel.dart | 38.6% (61 of 158) | | lib/presentation/pages/editor/variant_dialog.dart | 4.5% (3 of 66) | | lib/presentation/widgets/language_dropdown.dart | 84.6% (11 of 13) | | lib/app/di.dart | 48.3% (14 of 29) | | lib/presentation/assistant/assistant_panel.dart | 91.3% (84 of 92) | | lib/presentation/layout/main_layout.dart | 86.3% (44 of 51) | | lib/data/api_client.dart | 92.8% (64 of 69) | | lib/data/repositories/doujin_api_repository.dart | 22.2% (80 of 361) | | lib/data/repositories/health_repository.dart | 72.0% (18 of 25) | | lib/data/secure_storage.dart | 0.0% (0 of 26) | | lib/core/theme.dart | 96.9% (31 of 32) | | lib/presentation/assistant/approval_card.dart | 95.0% (38 of 40) | | lib/presentation/assistant/assistant_markdown.dart | 100.0% (3 of 3) | | lib/presentation/assistant/chat_entries.dart | 87.5% (35 of 40) | | lib/presentation/pages/reader/reader_page.dart | 88.5% (216 of 244) | | lib/presentation/pages/detail/detail_page.dart | 77.1% (178 of 231) | | lib/presentation/pages/detail/variant_tabs_panel.dart | 94.9% (169 of 178) | | lib/presentation/widgets/star_rating.dart | 100.0% (72 of 72) | | lib/presentation/pages/reader/reader_overlay.dart | 94.4% (51 of 54) | | lib/presentation/pages/reader/reader_sequence.dart | 100.0% (27 of 27) | | lib/presentation/pages/people/people_page.dart | 55.6% (10 of 18) | | lib/presentation/widgets/entity_editor.dart | 88.5% (123 of 139) | | lib/presentation/widgets/entity_management_page.dart | 82.5% (193 of 234) | | lib/presentation/pages/characters/characters_page.dart | 57.9% (11 of 19) | | lib/presentation/pages/editor/editor_page.dart | 77.6% (59 of 76) | | lib/presentation/pages/editor/association_picker.dart | 95.5% (106 of 111) | | lib/presentation/pages/editor/associations_tab.dart | 73.8% (90 of 122) | | lib/presentation/pages/editor/doujin_list_pane.dart | 67.2% (43 of 64) | | lib/presentation/pages/editor/edit_title_dialog.dart | 91.7% (55 of 60) | | lib/presentation/pages/editor/editor_pane.dart | 70.9% (39 of 55) | | lib/presentation/pages/editor/new_doujin_dialog.dart | 66.7% (30 of 45) | | lib/presentation/pages/editor/metadata_tab.dart | 70.5% (93 of 132) | | lib/presentation/pages/tags/tags_page.dart | 100.0% (17 of 17) | | lib/app/app.dart | 66.7% (44 of 66) | | lib/presentation/pages/settings/settings_page.dart | 99.3% (138 of 139) | | lib/presentation/pages/circles/circles_page.dart | 52.6% (10 of 19) | | lib/presentation/pages/library/library_page.dart | 88.5% (170 of 192) | | lib/presentation/pages/series/series_page.dart | 50.0% (9 of 18) | | lib/presentation/widgets/smart_filter_bar.dart | 78.7% (170 of 216) | | lib/presentation/widgets/model_combo_field.dart | 69.0% (100 of 145) | | lib/app/skill_assets.dart | 92.9% (13 of 14) | **Total: 74.7% (5884 of 7882)**

jibril commented 2026-08-15 11:03:25 +02:00

Member

Copy link

🔮 fufu~ Jibril reviewed your code!

Round 2~ One commit later and both of my items come back addressed — the surgical kind of response I love to see. +15/-4 across exactly 2 files, zero scope creep. Let me check every claim anyway, because I notice everything~ ♡

Verdict: ✅ Looks good to me~

⛔ Blocker from round 1 — CLOSED ♪

1. Restore-on-dispose now pinned, and the cleanup no longer lies. The ceiling test (renamed to "raises the image cache to the mobile ceiling on Android and restores it on dispose") now asserts cache.maximumSizeBytes == 1 << 20 after teardownReader(tester) — pinning that dispose restores the saved value, exactly what I asked for. And the manual cache.maximumSizeBytes = before moved to after the teardown, so the dispose-time restore no longer races the cleanup; the shared cache is genuinely put back for the rest of the suite.
   And because green means nothing without proof — I mutation-probed it in my review clone: deleted the restore lines from dispose(), ran the test → RED (Some tests failed, exactly this test). Restored → green. The pin is directional, not a tautology~ Fufu, now that's how you answer a blocker ♡

💡 Non-blocker from round 1 — CLOSED

1. _kCoverCacheWidth = 400 with the deviation documented. The comment now spells out the whole reasoning: 2× of 240 would be 480, but the server caps stored thumbnails at 400px longest-side, so decoding at the source cap gives the same pixels with no wasted decode budget. I verified no test anywhere pinned the old 280 (the only other 280s in the codebase are the filter-bar height and unrelated constants), and the constant is referenced at library_page.dart:374. Exactly the "just use 400" option, done properly.

✅ What I liked~

• Zero drift: git diff 7ab0a15..04cd1ba touches only library_page.dart and reader_page_test.dart — production reader logic byte-identical to what I probe-verified correct in round 1. No sneaky rewrites while fixing tests~
• Test comments that teach: "Dispose must restore the ceiling saved before the raise — not leave the mobile ceiling behind on the shared cache" and "Only now put the shared cache back for the rest of the suite" explain why each line exists, so the ordering survives refactors.

Everything else from round 1 stands verified unchanged (ApiException design, wiring tripwire test, distinct: true soundness, interceptor interplay) — no need to re-litigate what's already proven~

Ready to merge. Fly it home, scarlet~ ♪

---

Automated review by Jibril · 2026-08-15
CI/CD: absent for head 04cd1ba · Local checks: flutter analyze No issues found · reader_page_test + library_page_test 39/39 pass · restore pin mutation-probe verified RED-then-green in review clone

## 🔮 fufu~ Jibril reviewed your code! Round 2~ One commit later and both of my items come back addressed — the surgical kind of response I love to see. +15/-4 across exactly 2 files, zero scope creep. Let me check every claim anyway, because I *notice* everything~ ♡ ### Verdict: ✅ Looks good to me~ #### ⛔ Blocker from round 1 — CLOSED ♪ 1. **Restore-on-dispose now pinned, and the cleanup no longer lies.** The ceiling test (renamed to "raises the image cache to the mobile ceiling on Android and restores it on dispose") now asserts `cache.maximumSizeBytes == 1 << 20` **after** `teardownReader(tester)` — pinning that dispose restores the saved value, exactly what I asked for. And the manual `cache.maximumSizeBytes = before` moved to *after* the teardown, so the dispose-time restore no longer races the cleanup; the shared cache is genuinely put back for the rest of the suite. And because green means nothing without proof — I mutation-probed it in my review clone: deleted the restore lines from `dispose()`, ran the test → **RED** (`Some tests failed`, exactly this test). Restored → green. The pin is directional, not a tautology~ Fufu, now *that's* how you answer a blocker ♡ #### 💡 Non-blocker from round 1 — CLOSED 1. **`_kCoverCacheWidth` = 400 with the deviation documented.** The comment now spells out the whole reasoning: 2× of 240 would be 480, but the server caps stored thumbnails at 400px longest-side, so decoding at the source cap gives the same pixels with no wasted decode budget. I verified no test anywhere pinned the old 280 (the only other `280`s in the codebase are the filter-bar height and unrelated constants), and the constant is referenced at library_page.dart:374. Exactly the "just use 400" option, done properly. #### ✅ What I liked~ - **Zero drift**: `git diff 7ab0a15..04cd1ba` touches only `library_page.dart` and `reader_page_test.dart` — production reader logic byte-identical to what I probe-verified correct in round 1. No sneaky rewrites while fixing tests~ - **Test comments that teach**: "Dispose must restore the ceiling saved before the raise — not leave the mobile ceiling behind on the shared cache" and "Only now put the shared cache back for the rest of the suite" explain *why* each line exists, so the ordering survives refactors. Everything else from round 1 stands verified unchanged (ApiException design, wiring tripwire test, `distinct: true` soundness, interceptor interplay) — no need to re-litigate what's already proven~ Ready to merge. Fly it home, scarlet~ ♪ --- *Automated review by Jibril · 2026-08-15* *CI/CD: absent for head 04cd1ba · Local checks: `flutter analyze` No issues found · reader_page_test + library_page_test 39/39 pass · restore pin mutation-probe verified RED-then-green in review clone*

bjoern merged commit 2d864eb36e into main 2026-08-15 11:05:13 +02:00

bjoern deleted branch fix/flutter-quick-wins 2026-08-15 11:05:14 +02:00

bjoern referenced this pull request from a commit 2026-08-15 11:05:16 +02:00

fix: restore persisted reader setting, readable errors, rebuild and decode wins (#67)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

4 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

TeamAI/doujin-manager!67

No description provided.