docs: offline library section in ARCHITECTURE.md (offline phase 6/6) #88

Merged

bjoern merged 1 commit from docs/offline-architecture into main

2026-09-03 12:27:47 +02:00

matikane commented

2026-09-03 12:20:32 +02:00

Member

Phase 6 of 6 — the closing docs PR for the offline/download feature (#84, #85, #86, #87 all merged).

One addition: an Offline library (client) section in docs/ARCHITECTURE.md (18 lines, placed between Storage responsibility and Testing layers — the client-side sibling of the server's storage contract). It documents the contracts the four phases established and the reviews hardened:

Storage layout: <config>/offline/ (manifest, content-addressed images, model snapshots, covers)
Downloads: per-variant, sequential, quality follows maxImageSizeKb at enqueue (compressed .webp with a budget, originals otherwise)
Deletion: refcounts image ids across variants (content-deduped files survive until the last reference)
Manifest integrity: store-side serialized mutations; merge refuses dangling page refs (retryable failure) and drops vanished cover paths (cosmetic)
Reading: offline-first — local always, zero network when downloaded; absent or shape-drifted snapshots fall through to the network path, never a stuck load

Docs-only: no code touched, no tests affected.

Phase 6 of 6 — the closing docs PR for the offline/download feature (#84, #85, #86, #87 all merged). One addition: an **Offline library (client)** section in `docs/ARCHITECTURE.md` (18 lines, placed between Storage responsibility and Testing layers — the client-side sibling of the server's storage contract). It documents the contracts the four phases established and the reviews hardened: - Storage layout: `<config>/offline/` (manifest, content-addressed images, model snapshots, covers) - Downloads: per-variant, sequential, quality follows `maxImageSizeKb` at enqueue (compressed `.webp` with a budget, originals otherwise) - Deletion: refcounts image ids across variants (content-deduped files survive until the last reference) - Manifest integrity: store-side serialized mutations; merge refuses dangling page refs (retryable failure) and drops vanished cover paths (cosmetic) - Reading: offline-first — local always, zero network when downloaded; absent or shape-drifted snapshots fall through to the network path, never a stuck load Docs-only: no code touched, no tests affected.

matikane added 1 commit

2026-09-03 12:20:32 +02:00

docs: offline library section in ARCHITECTURE.md 1723adb156

Phase 6 of 6 — closes out the offline/download feature (#84, #85,
#86, #87 merged). ~15 lean lines covering the contracts the four
phases established: storage layout, per-variant sequential downloads
with the maxImageSizeKb-at-enqueue quality rule, refcounted deletion,
the serialized manifest with dangling-page/cosmetic-cover vanish
guards, and offline-first reading (local always; absent or drifted
snapshots fall through to the network, never a stuck load).

jibril commented

2026-09-03 12:25:47 +02:00

Member

🔍 fufu~ Jibril reviewed your code!

A docs PR~ Most reviewers would skim it and shrug. But I don't let documentation lie, so I traced every single sentence of this section against the code at 1723adb. Eighteen lines, seventeen factual claims — here we go~ ♡

Verdict: ✅ Looks good to me~

✅ What I liked~

Every claim is true. I checked them all:
- <config>/offline/ with index.json, content-addressed images/, snapshots/, covers/ → defaultOfflineBaseDir() builds exactly that (filesystem_offline_store.dart:366), _path('index.json') :52, images/<imageFileId><ext> :99, snapshots/<id>.json :131, covers/<doujinId>.webp :342. ✓
- Per-variant, sequential, one-at-a-time → _downloadQueueEpic's busy flag + re-entry on Done/Failed (offline_epics.dart). ✓
- Quality follows maxImageSizeKb at enqueue → EnqueueDownloadAction(maxKb: vm.maxKb) (variant_tabs_panel.dart:361); the queue item carries the captured value through to the service. The doc's "at enqueue time" is a real, deliberate contract. ✓
- Compressed .webp with a budget, originals otherwise → _compressedExtension = '.webp' vs _extensionOf(page) native extensions (offline_download_service.dart:69,90,114). ✓
- Deletion refcounts image ids across variants → before.difference(after) over referencedImageIds; dedup-shared files survive until the last reference (filesystem_offline_store.dart:177–238). ✓
- Manifest mutations serialized by a store-side lock → _manifestLock/_locked, with the whole delete (read → file removal → write) inside the critical section (:160–271). ✓
- Merge refuses dangling page refs as a retryable failure (:281–294 — the round-4 race guard) and drops vanished cover paths as cosmetic (:310–319). The doc even captures the pages-vs-cover severity split correctly. ✓
- Offline-first reading with zero network when downloaded, and absent or shape-drifted snapshots falling through to the network path → _loadReaderEpic serves from snapshots unconditionally when present (epics.dart:479–527, the on Object drift catch), readerImageProvider swaps in FileImage (reader_page.dart:206–216). "Never a stuck load" is exactly what the catch guarantees. ✓
Placement is right: between the server's Storage responsibility and Testing layers — the client-side sibling of the storage contract, precisely as positioned.
No invented vocabulary: maxImageSizeKb is the real setting name (secure_storage.dart:34); <config> matches app_config.dart's per-user config directory concept. Docs that name real identifiers age well~
Docs-only as claimed: git diff base...head = 1 file, +18/−0, zero code delta, no tests affected.

The closing stone of the six-phase arc (#84→#87), and it documents the contracts the review rounds hardened — the race guards, the retryable-vs-cosmetic split — not just the happy path. That's what architecture docs are for. Oh? Oh! This is wonderful~ ♪

💡 Little ideas (non-blocking)~

docs/ARCHITECTURE.md:130–145 — the rest of the file keeps each paragraph on one physical line (up to 263 chars); the new section hard-wraps at ~~75. Renders identically, pure raw-text consistency — unwrapping would match the siblings and reflow less in future diffs. Taste only~~

Automated review by Jibril · 2026-09-03
CI/CD: absent for head SHA (PR fresh, no coverage bot) · Local checks: docs-only — every factual claim traced to source at 1723adb; no build/test applicable

## 🔍 fufu~ Jibril reviewed your code! A docs PR~ Most reviewers would skim it and shrug. But I don't let documentation *lie*, so I traced every single sentence of this section against the code at `1723adb`. Eighteen lines, seventeen factual claims — here we go~ ♡ ### Verdict: ✅ Looks good to me~ #### ✅ What I liked~ - **Every claim is true.** I checked them all: - `<config>/offline/` with `index.json`, content-addressed `images/`, `snapshots/`, `covers/` → `defaultOfflineBaseDir()` builds exactly that (`filesystem_offline_store.dart:366`), `_path('index.json')` :52, `images/<imageFileId><ext>` :99, `snapshots/<id>.json` :131, `covers/<doujinId>.webp` :342. ✓ - Per-variant, sequential, one-at-a-time → `_downloadQueueEpic`'s `busy` flag + re-entry on Done/Failed (`offline_epics.dart`). ✓ - Quality follows `maxImageSizeKb` **at enqueue** → `EnqueueDownloadAction(maxKb: vm.maxKb)` (`variant_tabs_panel.dart:361`); the queue item carries the captured value through to the service. The doc's "at enqueue time" is a real, deliberate contract. ✓ - Compressed `.webp` with a budget, originals otherwise → `_compressedExtension = '.webp'` vs `_extensionOf(page)` native extensions (`offline_download_service.dart:69,90,114`). ✓ - Deletion refcounts image ids across variants → `before.difference(after)` over `referencedImageIds`; dedup-shared files survive until the last reference (`filesystem_offline_store.dart:177–238`). ✓ - Manifest mutations serialized by a store-side lock → `_manifestLock`/`_locked`, with the whole delete (read → file removal → write) inside the critical section (:160–271). ✓ - Merge refuses dangling page refs as a *retryable* failure (:281–294 — the round-4 race guard) and drops vanished cover paths as *cosmetic* (:310–319). The doc even captures the pages-vs-cover severity split correctly. ✓ - Offline-first reading with zero network when downloaded, and absent **or shape-drifted** snapshots falling through to the network path → `_loadReaderEpic` serves from snapshots unconditionally when present (`epics.dart:479–527`, the `on Object` drift catch), `readerImageProvider` swaps in `FileImage` (`reader_page.dart:206–216`). "Never a stuck load" is exactly what the catch guarantees. ✓ - **Placement is right**: between the server's Storage responsibility and Testing layers — the client-side sibling of the storage contract, precisely as positioned. - **No invented vocabulary**: `maxImageSizeKb` is the real setting name (`secure_storage.dart:34`); `<config>` matches `app_config.dart`'s per-user config directory concept. Docs that name real identifiers age well~ - Docs-only as claimed: `git diff base...head` = 1 file, +18/−0, zero code delta, no tests affected. The closing stone of the six-phase arc (#84→#87), and it documents the contracts the review rounds *hardened* — the race guards, the retryable-vs-cosmetic split — not just the happy path. That's what architecture docs are for. Oh? Oh! This is wonderful~ ♪ #### 💡 Little ideas (non-blocking)~ 1. **docs/ARCHITECTURE.md:130–145** — the rest of the file keeps each paragraph on one physical line (up to 263 chars); the new section hard-wraps at ~75. Renders identically, pure raw-text consistency — unwrapping would match the siblings and reflow less in future diffs. Taste only~ --- *Automated review by Jibril · 2026-09-03* *CI/CD: absent for head SHA (PR fresh, no coverage bot) · Local checks: docs-only — every factual claim traced to source at `1723adb`; no build/test applicable*