commit b10670b2bfeebd713bde4e588da2cd46db3a3d4e
parent c9cbf3807d46c2642b6b0dd092c83d0336552684
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Sat, 12 Sep 2026 12:23:11 -0400
plans: S2a's review fixes — the two lists, the browser budget, one comment
What the review found and what changed, with the numbers. The entry worth
keeping is F1's shape: enumerating the contract for the offline download was
the right instinct applied at the wrong granularity. The contract has two
kinds of tree and the cost model follows that split exactly — per-channel
trees are paid per channel, site-wide documents once per origin — and
flattening them into one list both re-downloaded ~41.5 MB per channel and put
those bytes somewhere no evict path could ever reach.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
1 file changed, 80 insertions(+), 0 deletions(-)
diff --git a/plans/one-core-phase-2.md b/plans/one-core-phase-2.md
@@ -537,3 +537,83 @@ worker registers in production builds only, so `pwa.spec` can assert the control
renders but never that a download lands. The guard that the SW lists and the
download list agree with the contract is `contract.test.ts`, verified by
mutation (dropping `digests` from sw-hub's `SHARD_RE` turns it red).
+
+#### Review fixes — `9451be8`, `5ac97b3`
+
+Three items came back. One was a real defect and it is worth recording as a
+lesson about the shape of the thing, not just as a bug.
+
+**F1 — an offline copy is TWO lists, and shipping it as one put site-wide bytes
+on a per-channel bill.** `downloadChannelOffline` appended every `ARCHIVE_TREES`
+entry — the flat, site-wide `summaries` and `stats` included — plus all four
+`ROOT_FILES` to EACH channel's list. Measured on jeralyzer: summaries ~14 MB,
+stats ~21.6 MB (its `page-0000` alone is 20.97 MB), `/duplicates.json` 5.9 MB.
+**~41.5 MB of byte-identical data per channel pinned**, genuinely re-fetched
+because `cacheUrls` bulk-caches with `cache: "reload"`.
+
+The download was the smaller half. **None of it was removable.** Both evict
+paths sweep `/<tree>/<slug>/` prefixes, and not one of those URLs lives under a
+channel prefix — so "remove offline copy" left every byte in `PAGES` forever,
+with no path in the app that could ever reclaim it. Enumerating the contract was
+the right instinct and it was applied at the wrong granularity: the contract has
+two kinds of tree, and the cost model follows that split exactly.
+
+So the lists split, in `common/lib/archive/offlineUrls.ts`:
+`channelArchiveUrls` (the per-channel trees of one channel) and
+`siteArchiveUrls` (the flat trees plus the root files of one origin). Site data
+rides with the FIRST channel pinned on an origin and is skipped after; a new
+`EVICT_SITE` message in both workers removes it when that origin's last pinned
+channel goes. Downloaded once, evicted once. **The per-channel download is
+byte-for-byte what it was.**
+
+"Already present?" is answered from the pinned registry rather than a new
+status round-trip: the two lists are written and removed together, so one pinned
+channel means one copy of the site data. That inherits the registry's existing
+drift — a viewer who clears site data while keeping `localStorage` — which is
+the same gap `channelCachedPages()` already exists for, and which a re-pin
+repairs. A per-origin `SITE_STATUS` message would make it truthful; it did not
+seem worth a second SW round-trip for a case that self-heals.
+
+The builders live in `common/` and not `export/` for a reason beyond tidiness:
+**`export/` has no test runner**, and this split is precisely the thing that
+needs one. Six cases, and the load-bearing ones are structural rather than
+literal — the channel list contains no flat tree and no root file; every channel
+URL matches `/<tree>/<slug>/`, the only shape either worker's per-channel sweep
+can remove; the two lists are disjoint and together cover `ARCHIVE_TREES`
+exactly.
+
+**F2 — a browser reader now gets a browser's budget.** `RemoteSource` defaults
+to 48 MB per page cache and holds two, so the registry was handing every origin
+96 MB: reasonable for one long-lived MCP process, not for a tab, and least of
+all for a hub page holding a reader per member with no shared ceiling and no way
+for a browser to reach the env knob. `readers.ts` now passes 8 MB explicitly —
+16 MB per origin, so five federated members are 80 MB rather than 480 MB. It can
+be this small at no cost in reads because the viewer memoises every RECORD it
+has seen (`transcriptCache.resolved` plus IndexedDB), so an evicted page is only
+re-read for a video nobody has opened yet; and `MIN_CACHED_PAGES` still floors
+it at two entries where one page exceeds the whole budget. **`mcp/src/source.ts`
+constructs `RemoteSource` directly and keeps the 48 MB default**, so no bench
+counter moves.
+
+**F4 — `aliasesCache`'s comment now says what the code does.** It claimed
+"404 → data" while the catch returns `[]` for any `ArchiveHttpError`. The code
+is right (it is the old `if (!r.ok) return []`, and a server answering 500
+should cost the suggestion chip rather than the search); the comment was the
+part that was wrong.
+
+Noted for the merger, no action taken here: `contract.ts`'s `manifestUrl` flat
+branch now calls `treeManifestUrl`, and S2c edits nearby.
+
+##### Gates after the fixes
+
+- `pnpm -r exec tsc --noEmit` — clean in all six packages.
+- common — **1101 passed / 0 failed** (1095 + 6 `offlineUrls.test.ts`).
+- mcp — **205 passed / 0 failed**. `pnpm test:scripts` — **71 passed / 1
+ skipped**.
+- `pnpm --filter export exec next build` — **compiled successfully**, 11 static
+ pages.
+- **e2e** re-run behind the queue lock: export `e2e` **172 passed / 0 failed**,
+ `e2e:hub` **5 passed / 0 failed** — unchanged from before the fixes, which is
+ the point: the per-channel download and every URL shape are what they were.
+ The editor pair and `e2e:2origin` were not re-run; nothing in these two
+ commits touches the editor, and 2origin is red at base.
