fix: reading-progress un-stick, one title rule per surface, port-typed offline consumers, reader cache and sort guards #93
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/app-small-fixes"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Five small, independently droppable app fixes from the backlog, one commit each, plus review fix-ups.
What's in
core/url_utils.dart'sjoinServerUrl; the library card keeps "no link → placeholder".variantKindLabellives once incore/variant_display.dart; display surfaces (editor pane header, delete prompt) usecore/title_display.dart's kind-priorityprimaryTitle; the editor's list-pane sync mirrors the server's summary rule (original, else first title) viaserverSummaryTitle, so a list title no longer changes on mere selection (review finding — the first version of this branch had diverged from the server rule).createAppEpic,createOfflineEpics,OfflineDownloadServiceandcreateStoretake theOfflineStoreport; the manifest-relative join lives in a Flutter-free core helper used by the store and the offline page, so onlymain.dartnamesFilesystemOfflineStoreunderlib/; deadStorageKeysincore/constants.dartand the never-assignedDownloadStatus.done(+ its unreachable queue-row arm) are gone.insertEntitySorteduses code-point order, mirroring the server'sBINARYcollation, instead ofString.compareTo.Verification
scripts/verify.sh app: green (implementer run 697 tests; fixer run of the 12 touched suites 125 tests; full app suite green on the six-branch integration tree).bdfda65): un-stick guard dropped />=/!=→ red; shrink-then-grow → red without the max rule; placeholder branch → red; each former title helper restored → red; server-rule helper swapped forprimaryTitle→ red; cache guard removed → red; ordering reverted → red.MapToSummarywith the kind-priority rule is a backend change worth its own line. The offline orphan scan (#84) was left as is (not a one-liner) and stays in the backlog.🤖 Generated with Claude Code
What: - `createAppEpic`, `createOfflineEpics`, `OfflineDownloadService` and `createStore` now take the `OfflineStore` port instead of `FilesystemOfflineStore`; only `main.dart` names the implementation. Chosen over deleting the port because every consumer already used port methods only (the one concrete-only member, static `joinUnder`, is called from the offline page without a store handle), so the change is a type swap plus imports, and it restores the intended layering (presentation/middleware → domain port, not data implementation). - `StorageKeys` in `core/constants.dart` was a dead duplicate of the live class in `data/secure_storage.dart`; removed. - `DownloadStatus.done` was never assigned (the reducer's done-path moves the item into the library and drops it from the queue), so the enum value and the queue row's unreachable arm are gone; the switch stays exhaustive without a wildcard. Verified: flutter test on the offline suites (epics, reader, reducer, page, download service, store) plus detail and reader pages (125 pass; 1 new). Mutation probe: reverting `createOfflineEpics`'s parameter to `FilesystemOfflineStore` makes offline_epics_test fail to compile ("argument type 'OfflineStore' can't be assigned"); the reader test's `_buildStore` pins `createAppEpic` the same way. flutter analyze clean. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>Flutter Coverage
Total: 79.1% (7264 of 9185)
🔮 fufu~ Jibril reviewed your code!
Oh? Oh~? Five independently droppable fixes, and every single one of them carries its own directional test and its own probe history? You even caught your own first version diverging from the server's summary rule and fixed it in
bdfda65before I could pounce~ ♡ I read the whole 1,416-line diff twice, both title rules side by side with the backend'sMapToSummary, and every sibling that ever whisperedFilesystemOfflineStore. My knife came out three times and found nowhere to land. How rude~Verdict: ✅ Looks good to me~
✅ What I liked~
grewonly fires beyond the finished count because sticky keepsmax(existing.pageCount, action.pageCount)— so the offline-shrink-then-online-restore sequence (8→6→8) can't masquerade as growth. The reducer tests walk exactly that sequence, plus re-finish and independence. I mutation-probed both halves: dropping!grew→ 2 tests red; dropping themax()keep → 1 test red. Neither is a tautology~serverSummaryTitlemirrors the backend exactly. I readDoujinEndpoints.cs:307—Original ?? FirstOrDefault()— and the app helper is its faithful reflection, while display surfaces keep kind-priorityprimaryTitle. The one fixture where the rules disagree (translated-first vs romaji) is pinned from both directions intitle_display_test.dart. Swapping the reducer toprimaryTitlegoes red. Delicious~maximumSizeBytes == _raisedImageCacheMaxBytesTo— a value someone else set meanwhile is theirs. Removed the guard → exactly the new dispose test red. The comment explains the why beautifully.compareCodePoints— code-point order is UTF-8 byte order is SQLite BINARY order, and the doc comment says precisely that. The emoji-vs-fullwidth-Ａ fixture proves it diverges fromcompareTowhere it matters, andentity_reducer_testpins it at the reducer level too. Nested-reader loop is clean and prefix-correct.createAppEpic,createOfflineEpics,OfflineDownloadService,createStoreall takeOfflineStore; epics call only port members;main.dartis the onlylib/file namingFilesystemOfflineStore. The port test pins it at compile time.StorageKeysinconstants.dartwas a corpse (the live one lives insecure_storage.dart) — buried.DownloadStatus.donewas unreachable — gone, with the queue-row arm and the stale test fixture honestly updated. No half-deletes anywhere.joinServerUrlreplaces both_normalizeUrlclones;variantKindLabelexists exactly once with a test enumerating every arm; the library placeholder branch (imageUrl.isNotEmpty) has its own widget test.flutter analyze: No issues found (85s). All 13 touched suites pass locally (139 tests, run serially after the sandbox choked on parallel test workers — environment, not you). Clone left pristine atbdfda65.💡 Little ideas (non-blocking)~
FilesystemOfflineStore(null). A five-line_FakeOfflineStore implements OfflineStorewould make the pin purely about the port, no implementation involved. Truly optional — the null-base store degrades to no-op anyway.Fufu~ five commits, five probe-verified guards, zero leftovers. Merge it before I change my mind about letting you have this one~ ♡
Automated review by Jibril · 2026-09-06
CI/CD: absent for head SHA
bdfda65· Local checks: analyze 0 issues · 13 touched suites 139/139 pass · 5 mutation probes red→green