From 053355d0c6c4205c3d6f823d5bacd16c082e125a Mon Sep 17 00:00:00 2001 From: Sulthan Zaki Date: Sat, 25 Jul 2026 12:01:10 +0700 Subject: [PATCH] docs: add implementation plan for bookmark reorder/latest-chapter/favorites Concrete backend + userscript plan against the approved design doc, including the Store.Upsert return-value fix needed to keep ordering correct once updated_at becomes conditional, and background-refresh triggering on both init() and SPA navigation per user preference. Co-Authored-By: Claude Sonnet 5 --- ...2026-07-25-bookmark-implementation-plan.md | 286 ++++++++++++++++++ 1 file changed, 286 insertions(+) create mode 100644 plans/2026-07-25-bookmark-implementation-plan.md diff --git a/plans/2026-07-25-bookmark-implementation-plan.md b/plans/2026-07-25-bookmark-implementation-plan.md new file mode 100644 index 0000000..69f160b --- /dev/null +++ b/plans/2026-07-25-bookmark-implementation-plan.md @@ -0,0 +1,286 @@ +# Implementation plan: bookmark reorder, latest-chapter tracking, favorites + +## Context + +Design already approved and committed at +`plans/2026-07-25-bookmark-list-favorites-design.md` on branch +`feat/bookmark-list-favorites-latest`. It covers three requested userscript +features plus one incidental bug found while verifying feasibility live via +Playwright: + +1. Bookmark list reordered so the most-recently-progressed manga is first. +2. Show the latest *available* chapter for a manga, not just the last one + read — including a background same-origin refresh mechanism to get closer + to "live" without server-side polling (Cloudflare blocks that; confirmed + live, and confirmed no JSON API / RSS exists on either site to poll + instead). +3. A favorites mechanism (star toggle + tabs) that doesn't remove a manga + from the normal list. +4. `asuracomic.net` deep links now 301-redirect straight to the + `asurascans.com` homepage (path discarded) — a Cloudflare-edge redirect + confirmed live, with no client-side fix possible. Doc-only correction. + +This plan turns that design into concrete code changes against the actual +current backend (Go/SQLite) and userscript, informed by full reads of +`backend/store.go`, `backend/handlers.go`, `backend/store_test.go`, +`backend/main.go`, and the full 782-line +`userscript/manga-bookmark.user.js`. + +## Key design decision surfaced during planning + +Today `handlers.go`'s `put()` echoes back the client's decoded request +struct as the API response, not what was actually persisted. Once +`updated_at` is sometimes *not* bumped (this whole feature's core mechanic), +echoing the request struct back would return a **wrong** `updated_at` to the +caller on every no-bump write — silently breaking the ordering guarantee the +entire feature depends on, since the userscript's `syncUpsert`/mutation +helpers adopt whatever the server echoes back (`upsertLocal(saved)`) as the +new source of truth. **`Store.Upsert` must therefore return the row as +actually written (read back inside the same transaction), and `handlers.go` +must respond with that**, not the client's payload. This is a correctness +fix required by the design, not a new decision to re-litigate. + +Confirmed via the user: background latest-chapter refresh should fire on +Asura's SPA in-app navigation too (`onNavigate()`), not only true browser +page loads (`init()`) — more refresh opportunities on a client-routed site +that rarely does full reloads, still bounded by the same throttle/batch +limits. + +## Phase 1 — Backend (`backend/`), TDD + +### 1.1 Tests first — `backend/store_test.go` + +Add four tests (all go through the existing `newTestServer(t)` / +`httptest` pattern already used in this file, since there are no direct +`Store`-level unit tests in the current style): + +- **`TestUpsertConditionalUpdatedAt`** — table-driven: new bookmark (bumps), + unchanged progress (no bump), changed progress (bumps), favorite-only + change (no bump), latest-chapter-only change (no bump). Assert on the + `updated_at` returned by each PUT response. +- **`TestFavoriteRoundTrip`** — PUT `favorite: true`, GET list, assert it + round-trips. +- **`TestLatestChapterNullable`** — PUT without `latest_chapter_num`, assert + JSON response has `"latest_chapter_num":null`; PUT again with a value, + assert it round-trips. +- **`TestOpenStoreMigratesLegacySchema`** — hand-create the *old* (10-column) + schema in a temp DB file, seed one row, then call `OpenStore` on it and + assert the row survives with the new columns defaulting cleanly + (`favorite=false`, `latest_chapter=""`, `latest_chapter_num=nil`). This is + the safety net for the already-deployed production DB. + +Run `cd backend && go test ./...` — expect compile failures (red state is +correct/expected before 1.2). + +### 1.2 `backend/store.go` + +- **`Bookmark` struct**: add `Favorite bool `json:"favorite"``, + `LatestChapter string `json:"latest_chapter"``, + `LatestChapterNum *float64 `json:"latest_chapter_num"`` (nullable — only + this one needs to be a pointer, per the design doc's data-model table). +- **`schema`**: extend `CREATE TABLE IF NOT EXISTS` with + `favorite INTEGER NOT NULL DEFAULT 0`, + `latest_chapter TEXT NOT NULL DEFAULT ''`, `latest_chapter_num REAL` + (covers fresh installs only). +- **Idempotent migration for the already-deployed DB**: add a + `migrateColumns(db)` helper using `PRAGMA table_info(bookmarks)` to check + each new column's existence before running its `ALTER TABLE ... ADD + COLUMN` (SQLite has no `ADD COLUMN IF NOT EXISTS`). Call it in + `OpenStore` right after the existing `schema` exec succeeds, same + error-wrapping style as today. +- **Shared `scanBookmark` helper**: centralizes converting the `favorite` + `INTEGER` (0/1) to `bool` and the nullable `latest_chapter_num` `REAL` to + `*float64` via `sql.NullFloat64`, used by both `List()` and `Upsert()`'s + read-back. +- **`List()`**: extend the `SELECT` to the new columns, scan via + `scanBookmark`. +- **`Upsert(b Bookmark) (Bookmark, error)`** — signature changes to return + the stored row. Implementation: wrap in `db.Begin()`/`tx.Commit()` + (explicit "read exactly what I just wrote" guarantee rather than relying + on `SetMaxOpenConns(1)` staying 1 forever). The `INSERT ... ON CONFLICT + DO UPDATE SET` gets a `CASE` expression for `updated_at`: + ```sql + updated_at = CASE + WHEN bookmarks.last_chapter_num IS NOT excluded.last_chapter_num + THEN excluded.updated_at + ELSE bookmarks.updated_at + END + ``` + This is valid SQLite upsert syntax (bare column = pre-update row value, + `excluded.col` = proposed new row) and naturally handles "new row" for + free — `ON CONFLICT DO UPDATE` only fires on the update path, so a + genuinely new row goes through the plain `INSERT ... VALUES` and always + gets the fresh `updated_at`. After the exec, `SELECT` the row back inside + the same transaction and return it via `scanBookmark`. + +### 1.3 `backend/handlers.go` + +In `put()`: keep `b.UpdatedAt = time.Now().UnixMilli()` as a *candidate* +value (update its comment — it's no longer unconditionally authoritative), +then: +```go +stored, err := h.store.Upsert(b) +... +writeJSON(w, http.StatusOK, stored) +``` +No other changes — `Favorite`/`LatestChapter`/`LatestChapterNum` already +flow through untouched from the decoded body, which is correct (they're +fully client-set synced fields). `list()`/`delete()` unchanged. + +### 1.4 Green + build + +```bash +cd backend && go test ./... # all pass, including pre-existing TestBookmarkRoundTrip unmodified +cd backend && CGO_ENABLED=0 go build # static binary still builds +``` + +## Phase 2 — Userscript (`userscript/manga-bookmark.user.js`) + +No JS test harness in this repo — verification is manual (Phase 4). + +1. **Config constants** (near `CACHE_KEY`, ~line 24): `LASTCHECKED_KEY = + "mangabm:lastchecked"`, `LATEST_CHECK_THROTTLE_MS = 4 * 60 * 60 * 1000` + (4h), `LATEST_CHECK_BATCH = 1`. + +2. **Shared anchor extraction** so the exact same per-site chapter-matching + rule runs against both the live DOM and raw fetched HTML text (no HTML + parser available for the fetch path): `anchorsFromDocument(doc)` (via + `querySelectorAll("a[href]")`) and `anchorsFromHTML(html)` (regex-based + `...` extraction). Add near `meta()` (~line 34). + +3. **Per-adapter `latestChapterFromAnchors(anchors)`** added to both `asura` + and `demonic` adapter objects, implementing the regex rules from the + design doc (Asura: href matches `/chapter/([\d.]+)$/` AND text matches + `/Chapter\s+[\d.]+/i`, excluding the "First Chapter" quick-jump button; + Demonic: all `chaptered.php?manga=\d+&chapter=([\d.]+)` matches, take + max — no order assumption). Plus a `computeLatestChapter(site, anchors)` + dispatcher near `keyOf()`. + +4. **`mangabm:lastchecked` local helpers**: `loadLastChecked()` / + `saveLastChecked(map)`, parallel to existing `loadCache`/`saveCache` + (~line 154), storing `{ [bookmarkKey]: timestampMs }`. Client-local only, + never synced. + +5. **`applyLatestChapterIfChanged(existing, latest)`** (~near `syncUpsert`, + line 295): if `latest.num` differs from the bookmark's stored + `latest_chapter_num`, optimistically update local cache + render, then + `apiPut` with `updated_at: Date.now()` as a *candidate* — the backend + (Phase 1) decides whether to actually apply it, and the client adopts + whatever comes back via `upsertLocal(saved)`, same pattern the rest of + the file already uses. No client-side "don't reorder" logic needed + beyond that — the server is the single source of truth for it. Silent + on failure (no toast), per the design doc. + +6. **Live-page capture**: `maybeCaptureLatestOnSeriesPage()` — on a + `type: "series"` page for an already-bookmarked series, scan the live + DOM via `anchorsFromDocument` + `computeLatestChapter`, then + `applyLatestChapterIfChanged`. Hooked into `onNavigate()` (~line 633), + after the existing `maybeAutoUpdate()` call. + +7. **Background opportunistic refresh**: `backgroundRefreshLatest()` — get + `currentSite()` (which adapter matches `window.location`), pick + same-site bookmarks not checked within `LATEST_CHECK_THROTTLE_MS` + (oldest-checked-first), fetch+parse at most `LATEST_CHECK_BATCH` of them + via `fetch(bm.series_url).then(r => r.text())` → `anchorsFromHTML` → + `computeLatestChapter` → `applyLatestChapterIfChanged`. Mark each + attempted bookmark's `lastchecked` timestamp regardless of success/failure + (advances the throttle window either way, avoiding hammering a + consistently-failing fetch). Silent on failure. + **Hook into both `init()` and `onNavigate()`** (per user's confirmed + preference — more refresh opportunities on Asura's SPA navigation, same + throttle/batch caps prevent request bursts either way). + +8. **`toggleFavorite(key)`** (~near `setChapterManual`, line 293): flips + `favorite`, optimistic update, `apiPut` with `Date.now()` candidate + timestamp (again, backend decides), toast on success/failure (consistent + with other explicit user-initiated actions like bookmark/remove). + +9. **Tabs**: new module state `let activeTab = "all";` (~near `panelOpen`, + line 374; not persisted, defaults to "all"). Wire click handlers in + `buildUI()` for new `#tabAll`/`#tabFav` elements. In `render()` (~line + 568), toggle each tab's `.active` class and filter which array feeds the + list-building loop (`activeTab === "favorites" ? state.list.filter(b => + b.favorite) : state.list`) — `state.list` itself is never mutated/filtered, + so a favorited manga always still appears in "All". + +10. **`renderItem(b)`** (~line 579): add a star toggle button (☆/★, + `onclick: () => toggleFavorite(b.key)`) alongside the existing + Continue/Edit/Remove buttons, and change the subtitle line to + `"Read: " + last_chapter + " · Latest: " + latest_chapter` when + `latest_chapter_num` is known and strictly greater than + `last_chapter_num` (avoids showing "Latest: Chapter 12" next to "Read: + Chapter 12" when they're numerically equal); otherwise keep today's + `" · "` text. + +11. **`TEMPLATE`** (~line 679): insert a tabs bar + (`
+
`) between + `#context` and `#list`. + +12. **`CSS`** (~near `.ctx-sub`/`.btn.danger`): add `.tab`/`.tab.active` and + `.btn.star`/`.btn.star.active` rules following the existing dark-theme + `.btn` modifier convention (`.btn.primary`, `.btn.small`, `.btn.danger`). + +13. **Fix the misleading redirect comment** (line 41, in the `asura` + adapter object): replace "asuracomic.net currently 301s to + asurascans.com; match both." with an accurate note that the redirect + now discards the path (goes straight to the asurascans.com root), + happens at the Cloudflare edge before any JS runs, so no client-side + fix is possible, and the user should navigate via asurascans.com links + directly. No change to the `matches()` regex itself. + +14. **`@version`**: bump `1.1.0` → `1.2.0`. + +## Phase 3 — Documentation + +- **`CLAUDE.md`**: update the `PUT /bookmarks/{key}` endpoint description + (currently "upsert, server sets updated_at") to describe the new + conditional rule, referencing + `plans/2026-07-25-bookmark-list-favorites-design.md` §4. +- **`README.md`**: update the matching endpoint-table row, and correct the + "Adapter reference" section's Asura row to note `asuracomic.net` deep + links currently 301 to the asurascans.com root (broken/path discarded) — + use `asurascans.com` links directly. While touching this, also fix + `CLAUDE.md`'s intro line ("asuracomic.net (formerly asurascans.com)"), + which has the relationship backwards and is inconsistent with README's + own phrasing ("asurascans.com (a.k.a. asuracomic.net)") — bundle this + small adjacent correction in since it's directly related to the same + finding. + +## Verification + +**Backend (automated):** +```bash +cd backend && go test ./... +cd backend && CGO_ENABLED=0 go build +``` + +**Backend (manual smoke test, extends the existing curl convention in +CLAUDE.md/README.md):** PUT a new bookmark, then PUT again changing only +`favorite`, then only `latest_chapter*`, then a real `last_chapter_num` +advance — confirm via `jq .updated_at` that only the first and last calls +change `updated_at`. + +**Userscript (manual — no JS test harness exists in this repo, matching +existing project convention):** +- List reorders only on a genuine progress advance, not on plain re-visit, + favorite toggle, or latest-chapter capture. +- Latest-chapter capture fires when visiting a bookmarked series page on + both sites and displays the "Read: X · Latest: Y" subtitle correctly. +- Background refresh: check `localStorage['mangabm:lastchecked']` in + devtools to confirm throttling behavior; confirm via the Network tab that + it never fires a cross-site request (only same-origin as the currently + loaded site). +- Favorite toggle persists across a panel close/reopen and a `refresh()` + round-trip through the backend; favorited manga still shows in "All". +- Fastest iteration path: desktop Tampermonkey/Violentmonkey first (script + stays `GM_*`-free), then confirm on Bromite per existing project + convention. + +## Critical files +- `backend/store.go` +- `backend/handlers.go` +- `backend/store_test.go` +- `userscript/manga-bookmark.user.js` +- `CLAUDE.md` +- `README.md`