diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 78c4be9..11aa857 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,5 +52,12 @@ jobs: with: node-version: '22' - run: npm install + # Install only the Chromium browser plus its OS dependencies. - run: npx playwright install --with-deps chromium - run: npm run e2e + - uses: actions/upload-artifact@v4 + if: failure() + with: + name: playwright-report + path: playwright-report/ + retention-days: 7 diff --git a/.gitignore b/.gitignore index 444f01e..253212d 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,12 @@ /node_modules/ /package-lock.json +# Playwright E2E artifacts (dev-only; never committed) +/test-results/ +/playwright-report/ +/blob-report/ +/.last-run.json + # Editor / OS .DS_Store diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ec0de6..d4c6d3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,27 @@ All notable changes to reefdoc are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.11.0] - 2026-06-26 + +### Added +- Binary document previews (PDF, DOCX, XLSX, PPTX) now **auto-update** when the + file changes on disk, matching markdown live-reload. The active preview + re-renders automatically; background tabs are flagged and re-render when you + switch to them. PDF/DOCX/XLSX render off-screen and swap in (no flicker, and a + half-written file leaves the previous preview intact); PPTX re-renders in + place. Scroll position is best-effort preserved across a refresh. +- End-to-end browser tests (Playwright) covering the live-reload pipeline: + active-tab auto-update, the background "updated" marker, and the + deleted-file state. Run with `npm run e2e`. + +### Fixed +- The file watcher now emits change events for binary document formats, not + just markdown, so binary previews actually receive live updates. +- Deleting an open document on disk now shows the "this file no longer exists" + state in its tab, instead of leaving a stale preview. This previously did not + happen for any file type (deletion emits only a directory-tree event); the + open tab is now re-checked when its directory listing changes. + ## [0.10.0] - 2026-06-26 ### Added diff --git a/README.md b/README.md index 4e72b5e..e66366a 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,8 @@ go build -o reefdoc . && ./reefdoc ./docs - Download the open document (its original file) with one click - Auto table of contents from document headings - Dark / light theme (mermaid follows the theme) -- Live reload: edit a file in any editor and the open tab updates +- Live reload: edit a file in any editor and the open tab updates — including + PDF, DOCX, XLSX, and PPTX previews - Hyperlinks between documents — markdown links and Allium `use` paths open in a new tab ## Status @@ -54,8 +55,9 @@ themes, live reload). See [`docs/specs`](docs/specs) for the design and [`docs/plans`](docs/plans) for the implementation plan. Run the unit tests with `go test ./...` and `npm test`. The browser end-to-end -test (Playwright, drives the real binary) runs with `npm run e2e` — it needs Go -on your PATH and Playwright's Chromium installed (`npx playwright install chromium`). +tests (Playwright, in `web/e2e/`, drive the real binary) run with `npm run e2e` +— they need Go on your PATH and Playwright's Chromium installed +(`npx playwright install chromium`). ## Architecture diff --git a/docs/plans/2026-06-26-binary-doc-live-reload.md b/docs/plans/2026-06-26-binary-doc-live-reload.md new file mode 100644 index 0000000..c641826 --- /dev/null +++ b/docs/plans/2026-06-26-binary-doc-live-reload.md @@ -0,0 +1,890 @@ +# Binary Document Live-Reload (Auto-Update) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make open PDF/DOCX/XLSX/PPTX previews auto-update when the file changes on disk, mirroring how markdown already live-reloads. + +**Architecture:** Mostly frontend. We add a small pure module (`web/binreload.js`) holding the testable refresh logic (debounce scheduler, change routing decision, scroll-ratio math, seq-guarded refresh), then wire it into `app.js`'s existing SSE handler and remove the early-return guard. PDF/DOCX/XLSX render off-screen and swap in (no flicker, mid-write safe); PPTX re-renders in place. **Correction (added during execution):** the plan originally assumed the Go server already emitted `change` SSE events for every watched file. It did not — the watcher gated on `isMarkdown`, so a one-line backend fix was required (`internal/server/watcher.go`: gate on `isViewable`) plus a watcher test. See Task 6.5. + +**Tech Stack:** Vanilla ES-module frontend; `node --test` for unit tests; Go `//go:embed` ships the asset; `go test` smoke-tests asset serving. No new dependencies. + +--- + +## Background for the implementer (read first) + +You are working in `reefdoc`, a local document previewer. A Go binary serves a browser UI and pushes file-change events over Server-Sent Events (SSE). The frontend renders everything client-side. + +Key facts you must know before touching anything: + +- **`web/app.js` cannot be imported in tests.** It runs DOM side-effects at module scope. The project's convention (see `web/app.test.js:4-9`) is that *pure, testable logic lives in small separate modules* (e.g. `web/recency.js`, `web/tabs.js`, `web/toc.js`) that are imported and tested directly. `app.js` itself is thin DOM/event glue, verified by hand and by a Go smoke test. **Follow this: all new testable logic goes in `web/binreload.js`, not in `app.js`.** + +- **Every `web/*.js` file the browser loads must be listed in the `//go:embed` directive in `main.go:16`.** A Go smoke test (`main_test.go`, `TestEmbeddedWebAssetsAreServed`) fails if a served asset is missing from that list. `*.test.js` files are excluded and never embedded. **When you create `web/binreload.js` you MUST add it to that embed line.** + +- **The `store` and tab model.** `store` (from `createTabStore()`) has `store.tabs` (array) and `store.active` (the active path string). `getTab(store, path)` returns the tab object or undefined. A tab object has `.path`, `.title`, `.updated` (boolean — drives the "●" dot via `renderTabs()`), and `.missing`. When the user clicks a background tab, `activate(path)` (`app.js:422-430`) sets `tab.updated = false` and calls `show(path)` — so the lazy "re-render on switch" already works for free once we set `tab.updated = true`. + +- **`show(path)` (`app.js:451`)** is the initial-open render path. It bumps a module-level `let showSeq = 0;` (`app.js:449`): `const seq = ++showSeq;`, then after each `await` checks `if (seq !== showSeq) return;` to abort superseded renders. Its binary branch (`app.js:476-486`) fetches raw bytes via `GET /api/file?path=...`, clears `contentEl` + `tocEl`, and calls the viewer. **We leave `show()` unchanged** — `refreshBinary` is a separate path. + +- **The SSE `change` handler (`app.js:607-615`)** currently does, for a `change` event: + ```js + markRecentInTree(msg.path); + if (isBinaryDoc(msg.path)) return; // <-- the guard we remove + const tab = getTab(store, msg.path); + if (!tab) return; + if (msg.path === store.active) show(msg.path); + else { tab.updated = true; renderTabs(); } + ``` + +- **Viewers (`web/viewers.js`)** export `getViewer(path)` (returns an async `viewer(bytes, container)` or null) and `isBinaryDoc(path)`. **The viewer signature and `viewers.js` are NOT changed by this plan.** PPTX is `.pptx`; it attaches a persistent `ResizeObserver` to the container it renders into, which is why PPTX must re-render directly into the live `contentEl` (in place) rather than off-screen. + +- **The content element** is `contentEl` in `app.js` (the scrollable pane that holds the rendered document). `tocEl` is the table-of-contents pane (stays empty for binary docs). + +Read the design spec for the full rationale: `docs/specs/2026-06-26-binary-doc-live-reload-design.md`. + +--- + +## File Structure + +- **Create `web/binreload.js`** — pure/testable refresh logic, no direct DOM-at-module-scope side effects. Exports: + - `BINARY_REFRESH_DEBOUNCE_MS` (constant) + - `scrollRatio(prevTop, prevHeight)` — capture helper + - `restoreScrollTop(ratio, newHeight)` — restore helper + - `routeBinaryChange({ store, getTab, isActive })` → a decision string (`'schedule' | 'mark-updated' | 'ignore'`) + - `createBinaryRefresher(deps)` — factory returning `{ schedule(path), refresh(path), _pendingSize() }`, with all dependencies injected (timers, fetch, getViewer, store accessor, DOM ops) so it is fully unit-testable. +- **Create `web/binreload.test.js`** — unit tests for the above. +- **Modify `web/app.js`** — import `binreload.js`, construct one refresher wired to real DOM/fetch/store, remove the `isBinaryDoc` guard, route binary changes through the refresher. +- **Modify `main.go:16`** — add `web/binreload.js` to the `//go:embed` list. +- **Modify `CHANGELOG.md`** — add an entry. +- **Modify `README.md`** — update the live-reload feature line to include binary docs. + +`web/viewers.js` is untouched. No other Go changes. + +--- + +### Task 1: Scaffold `web/binreload.js` with the pure scroll-ratio helpers + +**Files:** +- Create: `web/binreload.js` +- Test: `web/binreload.test.js` + +- [ ] **Step 1: Write the failing test** + +Create `web/binreload.test.js`: + +```js +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + BINARY_REFRESH_DEBOUNCE_MS, + scrollRatio, + restoreScrollTop, +} from './binreload.js'; + +test('BINARY_REFRESH_DEBOUNCE_MS is a sane positive debounce', () => { + assert.equal(typeof BINARY_REFRESH_DEBOUNCE_MS, 'number'); + assert.ok(BINARY_REFRESH_DEBOUNCE_MS > 0 && BINARY_REFRESH_DEBOUNCE_MS <= 1000); +}); + +test('scrollRatio: normal case is prevTop / prevHeight', () => { + assert.equal(scrollRatio(100, 400), 0.25); +}); + +test('scrollRatio: zero height guards against divide-by-zero', () => { + assert.equal(scrollRatio(100, 0), 0); +}); + +test('scrollRatio: zero top is zero', () => { + assert.equal(scrollRatio(0, 400), 0); +}); + +test('restoreScrollTop: ratio times new height', () => { + assert.equal(restoreScrollTop(0.25, 800), 200); +}); + +test('restoreScrollTop: zero ratio is top', () => { + assert.equal(restoreScrollTop(0, 800), 0); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `node --test web/binreload.test.js` +Expected: FAIL — `Cannot find module './binreload.js'`. + +- [ ] **Step 3: Write minimal implementation** + +Create `web/binreload.js`: + +```js +// binreload.js — live-reload (auto-update) logic for binary document previews +// (PDF/DOCX/XLSX/PPTX). Pure, dependency-injected, and unit-testable. The +// DOM/fetch/timer wiring is supplied by app.js; nothing here touches globals. + +// Quiet period after the last `change` event before re-rendering a binary doc. +// Collapses a build's burst of writes into a single render and dodges most +// mid-write reads. +export const BINARY_REFRESH_DEBOUNCE_MS = 250; + +// scrollRatio captures vertical scroll position as a fraction of total height, +// so it survives a re-render whose content height differs (e.g. a regenerated +// PDF with a different page count). Guards divide-by-zero. +export function scrollRatio(prevTop, prevHeight) { + return prevHeight > 0 ? prevTop / prevHeight : 0; +} + +// restoreScrollTop maps a captured ratio back onto a (possibly different) new +// content height. +export function restoreScrollTop(ratio, newHeight) { + return ratio * newHeight; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `node --test web/binreload.test.js` +Expected: PASS — 6 tests passing. + +- [ ] **Step 5: Commit** + +```bash +git add web/binreload.js web/binreload.test.js +git commit -m "feat(binreload): scroll-ratio helpers + debounce constant" +``` + +--- + +### Task 2: Add the change-routing decision + +**Files:** +- Modify: `web/binreload.js` +- Test: `web/binreload.test.js` + +The change handler must decide what to do with a binary `change` event without touching the DOM. `routeBinaryChange` returns a plain string the caller acts on. + +- [ ] **Step 1: Write the failing test** + +Append to `web/binreload.test.js`: + +```js +import { routeBinaryChange } from './binreload.js'; + +function fakeStore(activePath, openPaths) { + return { + active: activePath, + _open: new Set(openPaths), + }; +} +const fakeGetTab = (store, path) => (store._open.has(path) ? { path } : undefined); + +test('routeBinaryChange: active open binary tab -> schedule', () => { + const store = fakeStore('a.pdf', ['a.pdf']); + const decision = routeBinaryChange({ store, getTab: fakeGetTab, path: 'a.pdf' }); + assert.equal(decision, 'schedule'); +}); + +test('routeBinaryChange: open background binary tab -> mark-updated', () => { + const store = fakeStore('other.md', ['a.pdf', 'other.md']); + const decision = routeBinaryChange({ store, getTab: fakeGetTab, path: 'a.pdf' }); + assert.equal(decision, 'mark-updated'); +}); + +test('routeBinaryChange: not-open binary file -> ignore', () => { + const store = fakeStore('other.md', ['other.md']); + const decision = routeBinaryChange({ store, getTab: fakeGetTab, path: 'a.pdf' }); + assert.equal(decision, 'ignore'); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `node --test web/binreload.test.js` +Expected: FAIL — `routeBinaryChange is not a function` (or import error). + +- [ ] **Step 3: Write minimal implementation** + +Append to `web/binreload.js`: + +```js +// routeBinaryChange decides what a binary `change` event should do, given the +// current tab state. Returns one of: +// 'schedule' — the changed file is the active tab; debounce a refresh +// 'mark-updated' — the file is open in a background tab; flag it and re-render +// tabs (the "●" dot); it re-renders lazily when activated +// 'ignore' — the file is not open; nothing to do +export function routeBinaryChange({ store, getTab, path }) { + const tab = getTab(store, path); + if (!tab) return 'ignore'; + return path === store.active ? 'schedule' : 'mark-updated'; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `node --test web/binreload.test.js` +Expected: PASS — 9 tests total passing. + +- [ ] **Step 5: Commit** + +```bash +git add web/binreload.js web/binreload.test.js +git commit -m "feat(binreload): change-routing decision" +``` + +--- + +### Task 3: Add the debounce scheduler + +**Files:** +- Modify: `web/binreload.js` +- Test: `web/binreload.test.js` + +The scheduler collapses bursts per path. We inject the timer functions so tests are deterministic (no real waiting). It exposes `schedule(path)` and an internal `_pendingSize()` for assertions. + +- [ ] **Step 1: Write the failing test** + +Append to `web/binreload.test.js`: + +```js +import { createBinaryRefresher } from './binreload.js'; + +// A controllable fake timer: setTimeout records the callback; flush() runs all +// due callbacks. clearTimeout removes a pending one. +function fakeTimers() { + let nextId = 1; + const pending = new Map(); // id -> cb + return { + setTimeout: (cb) => { const id = nextId++; pending.set(id, cb); return id; }, + clearTimeout: (id) => { pending.delete(id); }, + flush: () => { for (const cb of [...pending.values()]) cb(); pending.clear(); }, + size: () => pending.size, + }; +} + +test('schedule: a burst for the same path collapses to one refresh', () => { + const timers = fakeTimers(); + const refreshed = []; + const r = createBinaryRefresher({ + setTimeout: timers.setTimeout, + clearTimeout: timers.clearTimeout, + refresh: (path) => refreshed.push(path), + }); + r.schedule('a.pdf'); + r.schedule('a.pdf'); + r.schedule('a.pdf'); + assert.equal(timers.size(), 1, 'only one pending timer for the path'); + timers.flush(); + assert.deepEqual(refreshed, ['a.pdf'], 'refresh fired exactly once'); +}); + +test('schedule: different paths get independent timers', () => { + const timers = fakeTimers(); + const refreshed = []; + const r = createBinaryRefresher({ + setTimeout: timers.setTimeout, + clearTimeout: timers.clearTimeout, + refresh: (path) => refreshed.push(path), + }); + r.schedule('a.pdf'); + r.schedule('b.xlsx'); + assert.equal(timers.size(), 2); + timers.flush(); + assert.deepEqual(refreshed.sort(), ['a.pdf', 'b.xlsx']); +}); + +test('schedule: pending entry is cleared after the timer fires', () => { + const timers = fakeTimers(); + const r = createBinaryRefresher({ + setTimeout: timers.setTimeout, + clearTimeout: timers.clearTimeout, + refresh: () => {}, + }); + r.schedule('a.pdf'); + timers.flush(); + assert.equal(r._pendingSize(), 0); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `node --test web/binreload.test.js` +Expected: FAIL — `createBinaryRefresher is not a function`. + +- [ ] **Step 3: Write minimal implementation** + +Append to `web/binreload.js`. (The `refresh` method is added in Task 4; for now the factory wires `schedule` and stores deps.) + +```js +// createBinaryRefresher builds the live-refresh controller for binary docs. +// All side-effecting dependencies are injected so the controller is testable +// with fake timers and stubs: +// deps.setTimeout(cb, ms) / deps.clearTimeout(id) — timer functions +// deps.refresh(path) — performs the actual +// re-render (overridden in tests; the real one is defined in Task 4 and +// bound below when deps.refresh is omitted). +// Returns { schedule(path), refresh(path), _pendingSize() }. +export function createBinaryRefresher(deps) { + const setTimeoutFn = deps.setTimeout; + const clearTimeoutFn = deps.clearTimeout; + const debounceMs = + deps.debounceMs != null ? deps.debounceMs : BINARY_REFRESH_DEBOUNCE_MS; + + // path -> pending timer id (at most one per path) + const pending = new Map(); + + // refresh: prefer an injected implementation (tests); otherwise use the + // built-in defined in Task 4. + const refresh = deps.refresh ? deps.refresh : (path) => defaultRefresh(deps, path); + + function schedule(path) { + if (pending.has(path)) clearTimeoutFn(pending.get(path)); + const id = setTimeoutFn(() => { + pending.delete(path); + refresh(path); + }, debounceMs); + pending.set(path, id); + } + + return { + schedule, + refresh, + _pendingSize: () => pending.size, + }; +} + +// defaultRefresh is implemented in Task 4. +async function defaultRefresh(_deps, _path) { + throw new Error('defaultRefresh not implemented yet'); +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `node --test web/binreload.test.js` +Expected: PASS — 12 tests total passing. + +- [ ] **Step 5: Commit** + +```bash +git add web/binreload.js web/binreload.test.js +git commit -m "feat(binreload): per-path debounce scheduler" +``` + +--- + +### Task 4: Implement `defaultRefresh` (fetch → off-screen render → swap → restore scroll) + +**Files:** +- Modify: `web/binreload.js` +- Test: `web/binreload.test.js` + +`defaultRefresh` performs the actual re-render. Everything it touches is injected via `deps` so it is fully testable without a browser: + +- `deps.store` — the tab store (reads `store.active`) +- `deps.contentEl` — the live content element (read `scrollTop`/`scrollHeight`; written on swap) +- `deps.getViewer(path)` — returns the async viewer or null +- `deps.isPptx(path)` — true for `.pptx` (the in-place exception) +- `deps.fetchBytes(path)` — async; resolves `{ ok, bytes }` or throws on network error +- `deps.makeOffscreen()` — returns a detached-but-attached, hidden, correctly-sized container element +- `deps.swap(offscreen)` — moves offscreen's children into `contentEl` and removes offscreen +- `deps.nextSeq()` / `deps.currentSeq()` — bump and read the shared `showSeq` (latest-wins guard) + +- [ ] **Step 1: Write the failing test** + +Append to `web/binreload.test.js`: + +```js +// Build a refresher whose dependencies are all stubs we can assert against. +function makeRefreshHarness(overrides = {}) { + const calls = { viewer: [], swap: 0, scrollSet: [] }; + let seq = 0; + const contentEl = { scrollTop: 100, scrollHeight: 400 }; + const store = { active: 'a.pdf' }; + const deps = { + setTimeout: (cb) => cb, // unused here + clearTimeout: () => {}, + store, + contentEl, + getViewer: () => async (bytes, container) => { calls.viewer.push([bytes, container]); }, + isPptx: () => false, + fetchBytes: async () => ({ ok: true, bytes: new Uint8Array([1, 2, 3]).buffer }), + makeOffscreen: () => ({ __offscreen: true }), + swap: () => { calls.swap++; contentEl.scrollHeight = 800; }, + nextSeq: () => ++seq, + currentSeq: () => seq, + setScrollTop: (v) => { calls.scrollSet.push(v); contentEl.scrollTop = v; }, + ...overrides, + }; + return { deps, calls, contentEl, store, getSeq: () => seq, bumpSeq: () => ++seq }; +} + +test('refresh: off-screen path renders, swaps, and restores scroll by ratio', async () => { + const { deps, calls } = makeRefreshHarness(); + const r = createBinaryRefresher(deps); + await r.refresh('a.pdf'); + assert.equal(calls.viewer.length, 1, 'viewer rendered once'); + assert.equal(calls.viewer[0][1].__offscreen, true, 'rendered into the off-screen container'); + assert.equal(calls.swap, 1, 'swapped once'); + // prevTop/prevHeight = 100/400 = 0.25; new height 800 -> 200 + assert.deepEqual(calls.scrollSet, [200]); +}); + +test('refresh: aborts when path is no longer the active tab', async () => { + const { deps, calls } = makeRefreshHarness({ store: { active: 'other.md' } }); + const r = createBinaryRefresher(deps); + await r.refresh('a.pdf'); + assert.equal(calls.viewer.length, 0); + assert.equal(calls.swap, 0); +}); + +test('refresh: aborts (no swap) when seq advances during the fetch await', async () => { + const h = makeRefreshHarness(); + // Simulate another render starting mid-fetch: bump seq after fetch resolves. + h.deps.fetchBytes = async () => { + h.bumpSeq(); // a newer render took over + return { ok: true, bytes: new ArrayBuffer(3) }; + }; + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); + assert.equal(h.calls.swap, 0, 'stale render did not swap'); +}); + +test('refresh: failed fetch (!ok) leaves preview untouched', async () => { + const h = makeRefreshHarness(); + h.deps.fetchBytes = async () => ({ ok: false }); + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); + assert.equal(h.calls.viewer.length, 0); + assert.equal(h.calls.swap, 0); +}); + +test('refresh: viewer throwing on off-screen path leaves preview untouched (no swap)', async () => { + const h = makeRefreshHarness(); + h.deps.getViewer = () => async () => { throw new Error('half-written file'); }; + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); // must not reject + assert.equal(h.calls.swap, 0); +}); + +test('refresh: PPTX renders in place (no off-screen, no swap)', async () => { + // Active path must match the path we refresh, so set it to the pptx. + const h = makeRefreshHarness({ store: { active: 'deck.pptx' } }); + h.contentEl.scrollTop = 0; + h.contentEl.scrollHeight = 400; + h.deps.isPptx = () => true; + const rendered = []; + h.deps.getViewer = () => async (bytes, container) => { rendered.push(container); }; + const r = createBinaryRefresher(h.deps); + await r.refresh('deck.pptx'); + // For PPTX the viewer renders into the live contentEl, not the off-screen box. + assert.equal(rendered.length, 1); + assert.equal(rendered[0], h.contentEl); + assert.equal(h.calls.swap, 0, 'PPTX does not use the swap path'); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `node --test web/binreload.test.js` +Expected: FAIL — `defaultRefresh not implemented yet` thrown by the off-screen path. + +- [ ] **Step 3: Write the implementation** + +Replace the placeholder `defaultRefresh` in `web/binreload.js` with: + +```js +// defaultRefresh re-renders an already-open, ACTIVE binary document after its +// file changed on disk. Behavior (see design spec): +// - latest-wins via the shared seq guard (nextSeq/currentSeq) +// - proportional scroll capture + restore +// - PDF/DOCX/XLSX: render off-screen, swap in only on success (no flicker, +// mid-write safe — a failed parse leaves the previous preview intact) +// - PPTX: render in place into the live contentEl (its ResizeObserver reads +// the live container), accepting brief flicker +async function defaultRefresh(deps, path) { + const { store, contentEl, getViewer, isPptx, fetchBytes, makeOffscreen, + swap, nextSeq, currentSeq, setScrollTop } = deps; + + // Tab may have changed during the debounce window. + if (path !== store.active) return; + + const seq = nextSeq(); + const prevTop = contentEl.scrollTop; + const prevHeight = contentEl.scrollHeight; + + let res; + try { + res = await fetchBytes(path); + } catch { + return; // network error — keep the current preview + } + if (!res || !res.ok) return; // file gone / error — keep preview + if (seq !== currentSeq()) return; // superseded by a newer render + + const bytes = res.bytes; + const viewer = getViewer(path); + if (!viewer) return; + + if (isPptx(path)) { + // In-place exception: clear and render directly into the live element. + if (seq !== currentSeq()) return; + contentEl.innerHTML = ''; + try { + await viewer(bytes, contentEl); + } catch { + // Leave whatever partial state; next change event retries. No swap path. + return; + } + if (seq !== currentSeq()) return; + setScrollTop(restoreScrollTop(scrollRatio(prevTop, prevHeight), contentEl.scrollHeight)); + return; + } + + // Off-screen path for PDF/DOCX/XLSX. + const offscreen = makeOffscreen(); + try { + await viewer(bytes, offscreen); + } catch { + return; // half-written / bad parse — keep the previous good preview + } + if (seq !== currentSeq()) return; // superseded; drop the off-screen work + swap(offscreen); // move nodes into contentEl, remove offscreen + setScrollTop(restoreScrollTop(scrollRatio(prevTop, prevHeight), contentEl.scrollHeight)); +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `node --test web/binreload.test.js` +Expected: PASS — all tests passing (the PPTX corrected test included). + +- [ ] **Step 5: Run the full JS suite to confirm no regressions** + +Run: `node --test` +Expected: PASS — every `web/*.test.js` passes, including the existing `app.test.js`, `viewers.test.js`, etc. + +- [ ] **Step 6: Commit** + +```bash +git add web/binreload.js web/binreload.test.js +git commit -m "feat(binreload): seq-guarded refresh with off-screen swap and PPTX in-place" +``` + +--- + +### Task 5: Wire the refresher into `app.js` and remove the live-reload guard + +**Files:** +- Modify: `web/app.js:8` (import), and the SSE `change` handler at `web/app.js:607-615`. Add the refresher construction near the other module wiring. + +This task has no new unit test — `app.js` is DOM-coupled glue (per the project convention). It is verified by the existing Go smoke test (Task 6) and by manual verification (Task 8). Be precise with the edits below. + +- [ ] **Step 1: Add the import** + +In `web/app.js`, change line 8 from: + +```js +import { getViewer, isBinaryDoc } from './viewers.js'; +``` + +to: + +```js +import { getViewer, isBinaryDoc } from './viewers.js'; +import { createBinaryRefresher } from './binreload.js'; +``` + +- [ ] **Step 2: Construct the refresher with real dependencies** + +In `web/app.js`, immediately AFTER the `show()` function definition ends (the closing brace of `show`, currently `app.js:502`), add the refresher wiring. It needs access to `contentEl`, `store`, `getViewer`, `getTab`, and the `showSeq` guard. Because `showSeq` is a module-local `let`, expose bump/read closures over it: + +```js +// ---- Binary document live-reload (auto-update) ---- +// Builds the off-screen container the off-screen viewers render into: attached +// to the DOM (so layout-dependent viewers measure correctly) but visually +// hidden and sized to match contentEl. Removed by swap(). +function makeOffscreenContainer() { + const off = document.createElement('div'); + off.style.position = 'absolute'; + off.style.left = '-99999px'; + off.style.top = '0'; + off.style.width = contentEl.clientWidth + 'px'; + off.style.height = contentEl.clientHeight + 'px'; + off.style.overflow = 'auto'; + document.body.appendChild(off); + return off; +} + +function swapOffscreenIntoContent(off) { + contentEl.innerHTML = ''; + while (off.firstChild) contentEl.appendChild(off.firstChild); + off.remove(); +} + +const binaryRefresher = createBinaryRefresher({ + setTimeout: (cb, ms) => setTimeout(cb, ms), + clearTimeout: (id) => clearTimeout(id), + store, + contentEl, + getViewer, + isPptx: (path) => path.toLowerCase().endsWith('.pptx'), + fetchBytes: async (path) => { + const res = await fetch('/api/file?path=' + encodeURIComponent(path)); + if (!res.ok) return { ok: false }; + return { ok: true, bytes: await res.arrayBuffer() }; + }, + makeOffscreen: makeOffscreenContainer, + swap: swapOffscreenIntoContent, + nextSeq: () => ++showSeq, + currentSeq: () => showSeq, + setScrollTop: (v) => { contentEl.scrollTop = v; }, +}); +``` + +- [ ] **Step 3: Replace the SSE change handler's binary guard with routing** + +In `web/app.js`, the `change` branch currently reads (around `app.js:607-615`): + +```js + } else if (msg.type === 'change') { + markRecentInTree(msg.path); + // Binary documents are static previews — do not live-reload them. + if (isBinaryDoc(msg.path)) return; + const tab = getTab(store, msg.path); + if (!tab) return; + if (msg.path === store.active) show(msg.path); + else { tab.updated = true; renderTabs(); } + } +``` + +Replace it with: + +```js + } else if (msg.type === 'change') { + markRecentInTree(msg.path); + if (isBinaryDoc(msg.path)) { + // Binary docs auto-update via the dedicated refresher. Active tab: + // debounce a re-render. Background tab: mark updated and re-render + // lazily on activation (same model as markdown). + const btab = getTab(store, msg.path); + if (!btab) return; + if (msg.path === store.active) binaryRefresher.schedule(msg.path); + else { btab.updated = true; renderTabs(); } + return; + } + const tab = getTab(store, msg.path); + if (!tab) return; + if (msg.path === store.active) show(msg.path); + else { tab.updated = true; renderTabs(); } + } +``` + +- [ ] **Step 4: Build the binary so the JS module graph is exercised** + +Run: `go build -o /tmp/reefdoc-build . && echo BUILD_OK` +Expected: `BUILD_OK`. (This compiles; it does NOT yet embed the new file — Task 6 fixes that. The build still succeeds because `binreload.js` isn't referenced by the embed list yet.) + +- [ ] **Step 5: Commit** + +```bash +git add web/app.js +git commit -m "feat(app): auto-update binary previews via binreload refresher" +``` + +--- + +### Task 6: Embed `web/binreload.js` so the server serves it + +**Files:** +- Modify: `main.go:16` (the `//go:embed` directive) +- Test: `main_test.go` `TestEmbeddedWebAssetsAreServed` (existing — guards this) + +Without this, `index.html` loads `/app.js`, which `import`s `/binreload.js`, which the server 404s — breaking the whole frontend. The existing Go smoke test catches exactly this. + +- [ ] **Step 1: Run the guard test to see it fail** + +Run: `go test ./... -run TestEmbeddedWebAssetsAreServed -v` +Expected: FAIL — a message like `GET /binreload.js = 404, want 200 — is web/binreload.js missing from the //go:embed directive in main.go?` + +- [ ] **Step 2: Add the file to the embed directive** + +In `main.go`, change line 16 from: + +```go +//go:embed web/index.html web/app.css web/app.js web/allium.js web/render.js web/tabs.js web/toc.js web/favorites.js web/recency.js web/viewers.js +``` + +to (append `web/binreload.js`): + +```go +//go:embed web/index.html web/app.css web/app.js web/allium.js web/render.js web/tabs.js web/toc.js web/favorites.js web/recency.js web/viewers.js web/binreload.js +``` + +- [ ] **Step 3: Run the guard test to verify it passes** + +Run: `go test ./... -run TestEmbeddedWebAssetsAreServed -v` +Expected: PASS. + +- [ ] **Step 4: Run the full Go suite** + +Run: `go test ./...` +Expected: PASS — all packages. + +- [ ] **Step 5: Commit** + +```bash +git add main.go +git commit -m "build: embed web/binreload.js in the binary" +``` + +--- + +### Task 6.5: Backend fix — watcher emits `change` for binary documents + +> **Added during execution.** The original plan assumed "No Go changes" because +> the design wrongly believed the server emitted `change` for every watched +> file. Verification (Task 8) proved binary writes emitted no `change` event: +> `internal/server/watcher.go` gated change-emit on `isMarkdown(ev.Name)`. + +**Files:** +- Modify: `internal/server/watcher.go` (change-emit predicate + doc comment) +- Test: `internal/server/watcher_test.go` + +- [x] **Step 1:** Add a failing watcher test asserting a `.pdf` write emits a + `change` event (modeled on the existing markdown change test). +- [x] **Step 2:** In `watcher.go`, change the change-emit predicate from + `isMarkdown(ev.Name)` to `isViewable(ev.Name)` (already defined in `tree.go`, + covering markdown + the four binary formats). Update the `Watcher` doc + comment accordingly. The tree-event logic is unchanged. +- [x] **Step 3:** `go test ./internal/server/ -v` → new test passes, existing + markdown change tests still pass. +- [x] **Step 4:** `go test ./...` → all packages ok. +- [x] **Step 5:** Commit `fix(watcher): emit change events for binary documents, not just markdown`. + +--- + +### Task 7: Full verification pass (both suites + build) + +**Files:** none (verification only) + +- [ ] **Step 1: Run the JS suite** + +Run: `node --test` +Expected: PASS — all `web/*.test.js`. + +- [ ] **Step 2: Run the Go suite** + +Run: `go test ./...` +Expected: PASS — all packages. + +- [ ] **Step 3: Build the binary** + +Run: `go build -o /tmp/reefdoc-build . && echo BUILD_OK` +Expected: `BUILD_OK`. + +- [ ] **Step 4: Commit (only if any incidental fixes were needed)** + +If steps 1–3 surfaced issues, fix them and commit: + +```bash +git add -A +git commit -m "fix: address verification findings for binary live-reload" +``` + +If nothing needed fixing, skip this step. + +--- + +### Task 8: Manual verification (documented; not automated) + +**Files:** none. These steps require a real browser and real binary files; record the outcome but do not block the plan on automation. + +- [ ] **Step 1: Start the server on a folder containing a PDF** + +Run: `go run . ./docs` (or any folder with a `.pdf`/`.docx`/`.xlsx`/`.pptx`). +Open `http://127.0.0.1:8080`. + +- [ ] **Step 2: PDF/DOCX/XLSX auto-update, no flicker** + +Open a PDF tab. From another terminal, overwrite the PDF with a different file (e.g. `cp other.pdf docs/sample.pdf`). Confirm the preview updates within ~250ms with no visible blank flash, and your scroll position is roughly preserved. + +- [ ] **Step 3: Background tab marks updated** + +Open a PDF, then switch to a markdown tab. Overwrite the PDF on disk. Confirm the PDF tab shows the "●" updated dot but does not render until you click it; clicking it shows the new content. + +- [ ] **Step 4: Mid-write safety** + +Simulate a slow/partial write (e.g. `head -c 1000 real.pdf > docs/sample.pdf` then later replace with the full file). Confirm the broken intermediate does NOT blank the preview — the previous good render stays until a valid file lands. + +- [ ] **Step 5: PPTX in place** + +Open a `.pptx`, overwrite it on disk. Confirm it re-renders (brief flicker acceptable) and still fits/scales when you resize the window afterward. + +- [ ] **Step 6: Markdown regression** + +Confirm editing a markdown file still live-reloads exactly as before. + +--- + +### Task 9: Update docs (CHANGELOG + README) + +**Files:** +- Modify: `CHANGELOG.md` +- Modify: `README.md:45` + +- [ ] **Step 1: Add a CHANGELOG entry** + +In `CHANGELOG.md`, add a new section directly under the `# Changelog` header block (above `## [0.9.0]`). Use the next minor version and today's date: + +```markdown +## [0.10.0] - 2026-06-26 + +### Added +- Binary document previews (PDF, DOCX, XLSX, PPTX) now **auto-update** when the + file changes on disk, matching markdown live-reload. The active preview + re-renders automatically; background tabs are flagged and re-render when you + switch to them. PDF/DOCX/XLSX render off-screen and swap in (no flicker, and a + half-written file leaves the previous preview intact); PPTX re-renders in + place. Scroll position is best-effort preserved across a refresh. +``` + +- [ ] **Step 2: Update the README feature line** + +In `README.md`, the live-reload feature bullet (`README.md:45`) currently reads: + +```markdown +- Live reload: edit a file in any editor and the open tab updates +``` + +Replace it with: + +```markdown +- Live reload: edit a file in any editor and the open tab updates — including + PDF, DOCX, XLSX, and PPTX previews +``` + +- [ ] **Step 3: Commit** + +```bash +git add CHANGELOG.md README.md +git commit -m "docs: announce binary document live-reload" +``` + +--- + +## Self-Review (completed by plan author) + +**Spec coverage** — every spec section maps to a task: +- Active-tab auto-update → Task 4 (`defaultRefresh`) + Task 5 (routing `schedule`). +- Background-tab mark-updated → Task 2 (`routeBinaryChange`) + Task 5 (`btab.updated`); lazy re-render reuses existing `activate()`. +- Proportional scroll restore → Task 1 helpers + Task 4 usage. +- Off-screen render + swap (PDF/DOCX/XLSX) → Task 4 + Task 5 (`makeOffscreen`/`swap`). +- PPTX in-place exception → Task 4 (`isPptx` branch) + Task 5 (`isPptx` dep). +- Debounce (~250ms) → Task 1 constant + Task 3 scheduler. +- `showSeq` latest-wins guard → Task 4 (`nextSeq`/`currentSeq`) + Task 5 wiring. +- Remove `isBinaryDoc` guard → Task 5 Step 3. +- No backend change except embed → Task 6 only. +- Markdown path unchanged → Task 5 keeps the markdown branch verbatim; Task 8 Step 6 regression-checks it. +- Tests (debounce, routing, scroll math, seq aborts) → Tasks 1–4. Manual items → Task 8. + +**Placeholder scan** — no TBD/TODO; every code step shows complete code; the one deliberate placeholder (`defaultRefresh` throwing in Task 3) is explicitly replaced in Task 4 with full code, matching the TDD red→green flow. + +**Type/name consistency** — `createBinaryRefresher`, `schedule`, `refresh`, `_pendingSize`, `routeBinaryChange`, `scrollRatio`, `restoreScrollTop`, `BINARY_REFRESH_DEBOUNCE_MS`, and the `deps` keys (`setTimeout`, `clearTimeout`, `store`, `contentEl`, `getViewer`, `isPptx`, `fetchBytes`, `makeOffscreen`, `swap`, `nextSeq`, `currentSeq`, `setScrollTop`) are used identically across Tasks 1–5. The PPTX test sets `store.active` to the `.pptx` path so the active-tab guard passes. diff --git a/docs/specs/2026-06-26-binary-doc-live-reload-design.md b/docs/specs/2026-06-26-binary-doc-live-reload-design.md new file mode 100644 index 0000000..9914f51 --- /dev/null +++ b/docs/specs/2026-06-26-binary-doc-live-reload-design.md @@ -0,0 +1,216 @@ +# Binary Document Live-Reload (Auto-Update) — Design + +**Date:** 2026-06-26 +**Status:** Approved (brainstorming) — ready for implementation plan + +## Summary + +Make office and PDF previews **auto-update** when the underlying file changes +on disk, the same way markdown and allium files already do. Today binary +document previews (PDF, DOCX, XLSX, PPTX) are **static** — opening one renders a +snapshot, and editing the file on disk does nothing until you manually reopen +the tab. This was a deliberate non-goal of the original office/PDF viewing work +(see `2026-06-25-office-pdf-viewing-design.md`, "Non-Goals"); this design +reverses that decision for live-reload and scroll-restore. + +The change is **mostly frontend**. It removes the client guard and adds a +dedicated, debounced, flicker-free refresh path for binary documents. + +> **Correction (post-implementation):** this design originally assumed the Go +> server already emitted a `change` SSE event for every watched file regardless +> of type. That was wrong — the watcher gated change events on `isMarkdown`, so +> binary documents never emitted one. A one-line backend fix was required +> (`internal/server/watcher.go`: gate on `isViewable` instead of `isMarkdown`), +> plus a watcher test. See the "Backend" row in Key Decisions. + +## Goals + +- When an **open, active** binary document changes on disk, its preview + re-renders automatically. +- When an **open, background** binary tab changes, mark it updated (the "•" + dot) and re-render lazily when the user switches to it — identical to the + markdown model. +- Best-effort **scroll-position restore** across a refresh (proportional, so a + regenerated PDF with a different page count keeps you roughly in place). +- **No flicker** and **no broken preview** when a file is read mid-write: + render the new preview off-screen and only swap it in on success; on failure, + leave the previous good preview untouched. +- Collapse rapid bursts of `change` events (e.g. a build rewriting the file) + into a single render. +- Zero behavioral change to the markdown / allium live-reload path. + +## Non-Goals (YAGNI) + +- Server-side rendering, conversion, or diffing of binary documents. +- Pixel-perfect scroll restoration (page-anchored PDF scroll, cell-anchored + XLSX scroll). A proportional `scrollTop` ratio is sufficient. +- Per-format "update" viewer contract (re-rendering remains a generic + off-screen swap; viewers are unchanged except as noted for PPTX). +- A special "file may be mid-write" error UI. On a failed parse we silently + keep the last good preview and let the next change event retry. +- Off-screen-swap polish for PPTX (see Key Decisions — PPTX renders in place). + +## Key Decisions + +| Decision | Choice | Rationale | +|----------|--------|-----------| +| Active vs. background tabs | Mirror markdown exactly | One mental model; reuses the existing `tab.updated` lazy-render path. | +| Scroll restore | Best-effort, proportional ratio | Keeps your place in large PDFs; robust to page-count changes across regenerations. | +| Flicker / mid-write safety | Render off-screen, then swap | No blank flash; a half-written file that fails to parse leaves the previous preview intact. | +| Burst handling | Debounce (~250ms) **and** `showSeq` guard | Debounce collapses write bursts and dodges most mid-write reads; the sequence guard supersedes an in-flight render if a newer trigger or tab-switch occurs. | +| PPTX refresh | In-place exception (clear + render into `contentEl`) | PPTX is the lowest-fidelity viewer and least likely to be live-regenerated; its `ResizeObserver` fitting reads the live container. Keeps `app.js` free of viewer internals. PDF/DOCX/XLSX use the off-screen swap. | +| Code structure | Dedicated `refreshBinary(path)` + scheduler, separate from `show()` | Scroll capture, off-screen swap, and debounce are live-refresh concerns that don't apply on first open. Keeps `show()` simple. | +| Backend | One-line fix | The watcher gated `change` events on `isMarkdown`; it now gates on `isViewable` (markdown + the four binary formats) so binary writes emit `change`. (Original design wrongly assumed no backend change was needed.) | + +## Architecture + +### Control flow + +The SSE `change` handler in `connectSSE` (`web/app.js`) currently does: + +``` +change → markRecentInTree(path) + → if isBinaryDoc(path) return // the guard being removed + → markdown: re-show active, or mark background tab updated +``` + +New behavior: + +``` +change → markRecentInTree(path) + → if isBinaryDoc(path): + tab = getTab(store, path) + if !tab: return // not open → nothing to do + if path === store.active: + scheduleBinaryRefresh(path) // debounced + else: + tab.updated = true; renderTabs() // lazy, same as markdown + return + → (markdown / allium path: unchanged) +``` + +Switching **to** a background binary tab marked `updated` runs the normal +`show(path)` open path (full render from top). No special casing is needed for +the switch itself. + +### Scheduler (debounce) + +A module-level `Map` holds at most one pending refresh per path, +so two open binary documents have independent timers. + +``` +scheduleBinaryRefresh(path): + clearTimeout(pending.get(path)) + pending.set(path, setTimeout(() => { + pending.delete(path) + refreshBinary(path) + }, BINARY_REFRESH_DEBOUNCE_MS)) // ~250ms +``` + +### refreshBinary(path) + +Only ever invoked for the active binary tab. + +``` +1. if path !== store.active: return // tab changed during debounce +2. seq = ++showSeq // reuse existing latest-wins guard +3. prevTop = contentEl.scrollTop + prevHeight = contentEl.scrollHeight +4. res = fetch raw bytes (same endpoint as show(), no size cap) + - network error or !res.ok → return (keep current preview) + - if seq !== showSeq → return (superseded) + bytes = await res.arrayBuffer() + - if seq !== showSeq → return +5. viewer = getViewer(path) + PPTX → in-place: contentEl.innerHTML = ''; await viewer(bytes, contentEl) + other → off-screen container (attached, hidden, sized to match contentEl): + await viewer(bytes, offscreen) + - on throw → discard offscreen, return (keep current preview) + - if seq !== showSeq → discard offscreen, return + - swap: move offscreen child nodes into contentEl; remove offscreen +6. restore scroll (best-effort, proportional): + ratio = prevHeight > 0 ? prevTop / prevHeight : 0 + contentEl.scrollTop = ratio * contentEl.scrollHeight + tocEl stays empty (binary docs have no TOC). +``` + +**Off-screen container caveat.** Some viewers measure layout from the live +container (e.g. canvas sizing, `getComputedStyle`). The off-screen container is +therefore **attached to the DOM but visually hidden** and sized to match +`contentEl` (e.g. absolutely positioned off-screen with the same width/height), +so layout-dependent viewers measure correctly. It is removed after the swap. + +**PPTX exception.** The PPTX viewer (`web/viewers.js`) attaches a persistent +`ResizeObserver` to the container it renders into and self-disconnects when its +wrapper leaves the pane. An off-screen-then-swap would leave that observer +watching the discarded off-screen node. To avoid leaking viewer internals into +`app.js`, PPTX re-renders **in place**: clear `contentEl` and render directly +into it. This accepts a brief flicker and loses mid-write safety **for PPTX +only**; PDF/DOCX/XLSX keep the off-screen swap. + +**Sequence guard reuse.** `refreshBinary`, `show()`, and tab-switching all +bump/check the same `showSeq`. Whichever started last wins, so a manual tab +switch or a newer debounced refresh correctly cancels a stale in-flight render. + +## Components touched + +- `web/app.js` + - Remove the `isBinaryDoc(msg.path)` early-return guard in `connectSSE`. + - Add the binary branch to the `change` handler (active → schedule, background + → mark updated). + - Add `scheduleBinaryRefresh(path)` and the per-path debounce map. + - Add `refreshBinary(path)` with off-screen swap (PDF/DOCX/XLSX) and in-place + render (PPTX). + - Add a small pure scroll-ratio helper (capture → restore) for testability. +- `web/viewers.js` — unchanged (existing viewer signature reused as-is). +- `web/app.test.js` (and/or a focused new test file) — new tests below. +- No Go changes. + +## Testing + +Existing tests: `web/*.test.js` (node test runner via `npm test`) plus +`go test ./...`. The viewers need CDN libs + canvas/DOM and are not run headless; +`viewers.test.js` only covers the registry. Accordingly we test the **new, +isolatable logic**, not the rendering itself. + +Automated (unit): + +1. **Debounce/scheduling** — a burst of `scheduleBinaryRefresh(path)` calls + collapses into a single `refreshBinary` after the quiet period; distinct + paths get independent timers. Use fake timers. +2. **Change-handler routing** — for a binary `change`: active tab → schedules a + refresh; open background tab → sets `tab.updated` and re-renders tabs; no + open tab → no-op. (This is the logic replacing the old `return` guard.) +3. **Scroll-ratio math** — `prevTop/prevHeight` capture and `ratio * newHeight` + restore, including the `prevHeight === 0` guard, via the extracted pure + helper. +4. **Sequence-guard aborts** — `refreshBinary` aborts when `path !== store.active` + and when `showSeq` advances during an await, using stubbed fetch + viewer. + +To make 1–4 testable, `refreshBinary` and the scheduler take their +dependencies (fetch, `getViewer`, content element, store) through the same +seam style the existing modules use (exported functions operating on a passed +`store`). The seam is kept minimal. + +Manual verification (documented, not automated): + +- Off-screen-render-then-swap produces correct pixels for PDF/DOCX/XLSX. +- No visible flicker on PDF/DOCX/XLSX refresh; brief flicker acceptable on PPTX. +- A file read mid-write (failed parse) leaves the previous preview intact. +- PPTX in-place re-render still fits/scales on subsequent window resize. +- Scroll position is roughly preserved across a refresh of a large PDF. + +Regression: existing markdown/allium live-reload tests pass unchanged (that +branch of the handler is untouched). + +## Risks + +- **Off-screen measurement.** If a viewer reads layout before the swap and the + hidden container is mis-sized, the render could be wrong. Mitigated by sizing + the off-screen container to match `contentEl`. Manual verification covers it. +- **PPTX polish gap.** PPTX auto-update is flickerier and lacks mid-write + safety. Accepted for v1; can be upgraded to the off-screen path later by + re-establishing its observer on `contentEl` after the swap. +- **Debounce window tuning.** 250ms is a starting value; if builds write slower + the first read may still catch a partial file, but the failed-parse-keeps-last + -good behavior plus the next change event retrying makes this self-correcting. diff --git a/internal/server/tree.go b/internal/server/tree.go index a52f5fd..a81d5a0 100644 --- a/internal/server/tree.go +++ b/internal/server/tree.go @@ -24,7 +24,8 @@ func isMarkdown(name string) bool { // isViewable reports whether a file should appear in the tree: the text // formats reefdoc renders inline plus the binary document formats it previews -// client-side (pdf/docx/xlsx/pptx). +// client-side (pdf/docx/xlsx/pptx). It also gates live-reload "change" events +// (see watcher.go), so narrowing it affects both tree listing and auto-update. func isViewable(name string) bool { if isMarkdown(name) { return true diff --git a/internal/server/watcher.go b/internal/server/watcher.go index c0ca692..aae7f22 100644 --- a/internal/server/watcher.go +++ b/internal/server/watcher.go @@ -11,7 +11,8 @@ import ( // Watcher watches the root directory plus an on-demand set of subdirectories // (reconciled via SetWatches) and broadcasts debounced events through the -// broker. "change" carries the relative path of a modified markdown file; +// broker. "change" carries the relative path of a modified previewable file +// (markdown or a binary document reefdoc previews); // "tree" carries the relative path of a directory whose listing changed // (the root directory is the empty string). type Watcher struct { @@ -121,7 +122,7 @@ func (w *Watcher) Run() { pendingTree[dir] = true } } - if ev.Op&(fsnotify.Write|fsnotify.Create) != 0 && isMarkdown(ev.Name) { + if ev.Op&(fsnotify.Write|fsnotify.Create) != 0 && isViewable(ev.Name) { if rel, ok := w.rel(ev.Name); ok { pendingChange[rel] = true } diff --git a/internal/server/watcher_test.go b/internal/server/watcher_test.go index 2747ef3..9320a26 100644 --- a/internal/server/watcher_test.go +++ b/internal/server/watcher_test.go @@ -52,6 +52,30 @@ func TestWatcher_EmitsChangeOnWrite(t *testing.T) { }) } +func TestWatcher_EmitsChangeOnBinaryWrite(t *testing.T) { + root := t.TempDir() + file := filepath.Join(root, "report.pdf") + if err := os.WriteFile(file, []byte("%PDF-1.4 one"), 0o644); err != nil { + t.Fatal(err) + } + b := NewBroker() + w, err := NewWatcher(root, b) + if err != nil { + t.Fatal(err) + } + go w.Run() + defer w.Close() + sub := b.Subscribe() + + time.Sleep(50 * time.Millisecond) + if err := os.WriteFile(file, []byte("%PDF-1.4 two"), 0o644); err != nil { + t.Fatal(err) + } + waitForMsg(t, sub, func(m map[string]string) bool { + return m["type"] == "change" && m["path"] == "report.pdf" + }) +} + func TestWatcher_EmitsTreeOnCreate(t *testing.T) { root := t.TempDir() b := NewBroker() diff --git a/main.go b/main.go index 097ddd5..c59c3a6 100644 --- a/main.go +++ b/main.go @@ -13,7 +13,7 @@ import ( "github.com/exilis/reefdoc/internal/server" ) -//go:embed web/index.html web/app.css web/app.js web/allium.js web/render.js web/tabs.js web/toc.js web/favorites.js web/recency.js web/viewers.js +//go:embed web/index.html web/app.css web/app.js web/allium.js web/render.js web/tabs.js web/toc.js web/favorites.js web/recency.js web/viewers.js web/binreload.js var webFS embed.FS // version is overridden at release time via -ldflags "-X main.version=". diff --git a/package.json b/package.json index 73e1ecc..7b30d0c 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "e2e": "playwright test" }, "devDependencies": { - "@playwright/test": "1.55.0", + "@playwright/test": "1.61.1", "highlight.js": "11.9.0", "markdown-it": "14.1.0", "markdown-it-task-lists": "2.1.1" diff --git a/playwright.config.js b/playwright.config.js index acb853d..ebd8bbc 100644 --- a/playwright.config.js +++ b/playwright.config.js @@ -1,15 +1,36 @@ +// Playwright config for reefdoc end-to-end tests. +// +// Two suites live here, both driving a REAL reefdoc binary in headless Chromium: +// - web/e2e/download.e2e.js — the document-download button + ?download=1 flow. +// - web/e2e/binary-live-reload.spec.js — live-reload (auto-update) for binary +// document previews (fsnotify -> watcher -> SSE -> frontend re-render). The +// binary viewer libraries normally load from a CDN; that import is stubbed +// (see web/e2e/fixtures.js) so the suite runs fully offline. +// +// Each test starts its own server on its own port against its own temp fixture +// directory, so there is no shared web server here. testMatch covers both the +// *.e2e.js and *.spec.js naming conventions while excluding the non-test +// helpers (fixtures.js, server.js). + import { defineConfig, devices } from '@playwright/test'; export default defineConfig({ testDir: './web/e2e', - testMatch: '**/*.e2e.js', + testMatch: '**/*.{e2e,spec}.js', + // Live-reload involves real filesystem watching + a 100ms watcher debounce + + // a 250ms client debounce, so give assertions room without being flaky. + timeout: 30_000, + expect: { timeout: 10_000 }, + // Filesystem-watch + real-binary tests are timing-sensitive; run serially. fullyParallel: false, + workers: 1, forbidOnly: !!process.env.CI, retries: process.env.CI ? 1 : 0, - workers: 1, - timeout: 30_000, - reporter: process.env.CI ? 'list' : 'line', + reporter: process.env.CI ? [['list'], ['html', { open: 'never' }]] : 'list', outputDir: './test-results', + use: { + trace: 'retain-on-failure', + }, projects: [ { name: 'chromium', use: { ...devices['Desktop Chrome'] } }, ], diff --git a/web/app.js b/web/app.js index d75cbe0..6d868fa 100644 --- a/web/app.js +++ b/web/app.js @@ -1,11 +1,12 @@ import mermaid from 'mermaid'; import { createRenderer } from './render.js'; -import { createTabStore, openTab, closeTab, getTab } from './tabs.js'; +import { createTabStore, openTab, closeTab, getTab, vanishedTabs } from './tabs.js'; import { buildToc, slugify } from './toc.js'; import { createFavorites, isFavorite, toggleFavorite, listFavorites } from './favorites.js'; import { isRecent } from './recency.js'; import { renderAllium } from './allium.js'; import { getViewer, isBinaryDoc } from './viewers.js'; +import { createBinaryRefresher, routeBinaryChange } from './binreload.js'; const render = createRenderer(); const store = createTabStore(); @@ -294,6 +295,19 @@ function collapseDir(path, kids, item) { syncWatches(); } +// Mark an open tab whose file vanished from disk as missing. If it's the active +// tab, show the missing state in the content pane (mirrors show()'s 404 path). +function markTabMissing(path) { + const tab = getTab(store, path); + if (!tab) return; + tab.missing = true; + renderTabs(); + if (path === store.active) { + contentEl.innerHTML = '

This file no longer exists.

'; + tocEl.innerHTML = ''; + } +} + // Reload just one directory level (on a tree SSE event for that dir). async function reloadLevel(path) { const container = levelContainers.get(path); @@ -302,6 +316,12 @@ async function reloadLevel(path) { const data = await fetchLevel(path); if (!data) return; await renderChildrenInto(container, data.children); + // Detect deletions: any open tab from this directory that is no longer in + // the listing was removed on disk — show the missing state. + const presentPaths = data.children.filter((n) => !n.isDir).map((n) => n.path); + for (const gone of vanishedTabs(store, path, presentPaths)) { + markTabMissing(gone); + } } // ---- URL <-> active target sync ---- @@ -505,6 +525,49 @@ async function show(path) { restoreScroll(tab); } +// ---- Binary document live-reload (auto-update) ---- +// Builds the off-screen container the off-screen viewers render into: attached +// to the DOM (so layout-dependent viewers measure correctly) but visually +// hidden and sized to match contentEl. Removed by swap(). +function makeOffscreenContainer() { + const off = document.createElement('div'); + off.style.position = 'absolute'; + off.style.left = '-99999px'; + off.style.top = '0'; + off.style.width = contentEl.clientWidth + 'px'; + off.style.height = contentEl.clientHeight + 'px'; + off.style.overflow = 'auto'; + document.body.appendChild(off); + return off; +} + +function swapOffscreenIntoContent(off) { + contentEl.innerHTML = ''; + while (off.firstChild) contentEl.appendChild(off.firstChild); + off.remove(); +} + +const binaryRefresher = createBinaryRefresher({ + setTimeout: (cb, ms) => setTimeout(cb, ms), + clearTimeout: (id) => clearTimeout(id), + store, + contentEl, + getViewer, + isPptx: (path) => path.toLowerCase().endsWith('.pptx'), + fetchBytes: async (path) => { + const res = await fetch('/api/file?path=' + encodeURIComponent(path)); + if (res.status === 404) return { ok: false, missing: true }; + if (!res.ok) return { ok: false }; + return { ok: true, bytes: await res.arrayBuffer() }; + }, + makeOffscreen: makeOffscreenContainer, + swap: swapOffscreenIntoContent, + nextSeq: () => ++showSeq, + currentSeq: () => showSeq, + setScrollTop: (v) => { contentEl.scrollTop = v; }, + onMissing: (path) => markTabMissing(path), +}); + function assignHeadingIds() { const seen = new Set(); contentEl.querySelectorAll('h1,h2,h3').forEach((h) => { @@ -632,8 +695,22 @@ function connectSSE() { reloadLevel(msg.path); } else if (msg.type === 'change') { markRecentInTree(msg.path); - // Binary documents are static previews — do not live-reload them. - if (isBinaryDoc(msg.path)) return; + if (isBinaryDoc(msg.path)) { + // Binary docs auto-update via the dedicated refresher. Active tab: + // debounce a re-render. Background tab: mark updated and re-render + // lazily on activation (same model as markdown). + switch (routeBinaryChange({ store, getTab, path: msg.path })) { + case 'schedule': + binaryRefresher.schedule(msg.path); + break; + case 'mark-updated': { + const btab = getTab(store, msg.path); + if (btab) { btab.updated = true; renderTabs(); } + break; + } + } + return; + } const tab = getTab(store, msg.path); if (!tab) return; if (msg.path === store.active) show(msg.path); diff --git a/web/binreload.js b/web/binreload.js new file mode 100644 index 0000000..6fbfae3 --- /dev/null +++ b/web/binreload.js @@ -0,0 +1,142 @@ +// binreload.js — live-reload (auto-update) logic for binary document previews +// (PDF/DOCX/XLSX/PPTX). Pure, dependency-injected, and unit-testable. The +// DOM/fetch/timer wiring is supplied by app.js; nothing here touches globals. + +// Quiet period after the last `change` event before re-rendering a binary doc. +// Collapses a build's burst of writes into a single render and dodges most +// mid-write reads. +export const BINARY_REFRESH_DEBOUNCE_MS = 250; + +// scrollRatio captures vertical scroll position as a fraction of total height, +// so it survives a re-render whose content height differs (e.g. a regenerated +// PDF with a different page count). Guards divide-by-zero. +export function scrollRatio(prevTop, prevHeight) { + return prevHeight > 0 ? prevTop / prevHeight : 0; +} + +// restoreScrollTop maps a captured ratio back onto a (possibly different) new +// content height. +export function restoreScrollTop(ratio, newHeight) { + return ratio * newHeight; +} + +// routeBinaryChange decides what a binary `change` event should do, given the +// current tab state. Returns one of: +// 'schedule' — the changed file is the active tab; debounce a refresh +// 'mark-updated' — the file is open in a background tab; flag it and re-render +// tabs (the "●" dot); it re-renders lazily when activated +// 'ignore' — the file is not open; nothing to do +export function routeBinaryChange({ store, getTab, path }) { + const tab = getTab(store, path); + if (!tab) return 'ignore'; + return path === store.active ? 'schedule' : 'mark-updated'; +} + +// createBinaryRefresher builds the live-refresh controller for binary docs. +// All side-effecting dependencies are injected so the controller is testable +// with fake timers and stubs: +// deps.setTimeout(cb, ms) / deps.clearTimeout(id) — timer functions +// deps.refresh(path) — performs the actual +// re-render (overridden in tests; otherwise the built-in `defaultRefresh` +// (defined below) is used). +// Returns { schedule(path), refresh(path), _pendingSize() }. +export function createBinaryRefresher(deps) { + const setTimeoutFn = deps.setTimeout; + const clearTimeoutFn = deps.clearTimeout; + const debounceMs = + deps.debounceMs != null ? deps.debounceMs : BINARY_REFRESH_DEBOUNCE_MS; + + // path -> pending timer id (at most one per path) + const pending = new Map(); + + // refresh: prefer an injected implementation (tests); otherwise use the + // built-in `defaultRefresh` below. + const refresh = deps.refresh ? deps.refresh : (path) => defaultRefresh(deps, path); + + function schedule(path) { + if (pending.has(path)) clearTimeoutFn(pending.get(path)); + const id = setTimeoutFn(() => { + pending.delete(path); + // Fire-and-forget: the scheduler owns swallowing any rejection so a + // throwing refresh can never surface as an unhandled promise rejection. + Promise.resolve().then(() => refresh(path)).catch(() => {}); + }, debounceMs); + pending.set(path, id); + } + + return { + schedule, + refresh, + _pendingSize: () => pending.size, + }; +} + +// defaultRefresh re-renders an already-open, ACTIVE binary document after its +// file changed on disk. Behavior (see design spec): +// - latest-wins via the shared seq guard (nextSeq/currentSeq) +// - proportional scroll capture + restore +// - PDF/DOCX/XLSX: render off-screen, swap in only on success (no flicker, +// mid-write safe — a failed parse leaves the previous preview intact) +// - PPTX: render in place into the live contentEl (its ResizeObserver reads +// the live container), accepting brief flicker +// Must never reject: the debounce timer calls it without awaiting. +async function defaultRefresh(deps, path) { + const { store, contentEl, getViewer, isPptx, fetchBytes, makeOffscreen, + swap, nextSeq, currentSeq, setScrollTop, onMissing } = deps; + + // Tab may have changed during the debounce window. + if (path !== store.active) return; + + const seq = nextSeq(); + const prevTop = contentEl.scrollTop; + const prevHeight = contentEl.scrollHeight; + + let res; + try { + res = await fetchBytes(path); + } catch { + return; // network error — keep the current preview + } + if (res && res.missing) { + // file deleted — show the missing state via the injected callback, then stop. + if (seq !== currentSeq()) return; // superseded; don't clobber a newer render + onMissing(path); + return; + } + if (!res || !res.ok) return; // transient error — keep current preview + if (seq !== currentSeq()) return; // superseded by a newer render + + const bytes = res.bytes; + const viewer = getViewer(path); + if (!viewer) return; + + if (isPptx(path)) { + // In-place exception: clear and render directly into the live element. + // Defensive clear; the PPTX viewer also clears its own container. + contentEl.innerHTML = ''; + try { + await viewer(bytes, contentEl); + } catch { + // Leave whatever partial state; next change event retries. No swap path. + return; + } + if (seq !== currentSeq()) return; + setScrollTop(restoreScrollTop(scrollRatio(prevTop, prevHeight), contentEl.scrollHeight)); + return; + } + + // Off-screen path for PDF/DOCX/XLSX. + const offscreen = makeOffscreen(); + try { + await viewer(bytes, offscreen); + } catch { + offscreen.remove(); // discard the failed render + return; // half-written / bad parse — keep the previous good preview + } + if (seq !== currentSeq()) { // superseded; drop the off-screen work + offscreen.remove(); + return; + } + swap(offscreen); // move nodes into contentEl, remove offscreen + setScrollTop(restoreScrollTop(scrollRatio(prevTop, prevHeight), contentEl.scrollHeight)); +} diff --git a/web/binreload.test.js b/web/binreload.test.js new file mode 100644 index 0000000..755903c --- /dev/null +++ b/web/binreload.test.js @@ -0,0 +1,289 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + BINARY_REFRESH_DEBOUNCE_MS, + scrollRatio, + restoreScrollTop, + routeBinaryChange, +} from './binreload.js'; +import { createBinaryRefresher } from './binreload.js'; + +test('BINARY_REFRESH_DEBOUNCE_MS is a sane positive debounce', () => { + assert.equal(typeof BINARY_REFRESH_DEBOUNCE_MS, 'number'); + assert.ok(BINARY_REFRESH_DEBOUNCE_MS > 0 && BINARY_REFRESH_DEBOUNCE_MS <= 1000); +}); + +test('scrollRatio: normal case is prevTop / prevHeight', () => { + assert.equal(scrollRatio(100, 400), 0.25); +}); + +test('scrollRatio: zero height guards against divide-by-zero', () => { + assert.equal(scrollRatio(100, 0), 0); +}); + +test('scrollRatio: zero top is zero', () => { + assert.equal(scrollRatio(0, 400), 0); +}); + +test('restoreScrollTop: ratio times new height', () => { + assert.equal(restoreScrollTop(0.25, 800), 200); +}); + +test('restoreScrollTop: zero ratio is top', () => { + assert.equal(restoreScrollTop(0, 800), 0); +}); + +function fakeStore(activePath, openPaths) { + return { + active: activePath, + _open: new Set(openPaths), + }; +} +const fakeGetTab = (store, path) => (store._open.has(path) ? { path } : undefined); + +test('routeBinaryChange: active open binary tab -> schedule', () => { + const store = fakeStore('a.pdf', ['a.pdf']); + const decision = routeBinaryChange({ store, getTab: fakeGetTab, path: 'a.pdf' }); + assert.equal(decision, 'schedule'); +}); + +test('routeBinaryChange: open background binary tab -> mark-updated', () => { + const store = fakeStore('other.md', ['a.pdf', 'other.md']); + const decision = routeBinaryChange({ store, getTab: fakeGetTab, path: 'a.pdf' }); + assert.equal(decision, 'mark-updated'); +}); + +test('routeBinaryChange: not-open binary file -> ignore', () => { + const store = fakeStore('other.md', ['other.md']); + const decision = routeBinaryChange({ store, getTab: fakeGetTab, path: 'a.pdf' }); + assert.equal(decision, 'ignore'); +}); + +// A controllable fake timer: setTimeout records the callback; flush() runs all +// due callbacks. clearTimeout removes a pending one. +function fakeTimers() { + let nextId = 1; + const pending = new Map(); // id -> cb + return { + setTimeout: (cb) => { const id = nextId++; pending.set(id, cb); return id; }, + clearTimeout: (id) => { pending.delete(id); }, + flush: () => { for (const cb of [...pending.values()]) cb(); pending.clear(); }, + size: () => pending.size, + }; +} + +test('schedule: a burst for the same path collapses to one refresh', async () => { + const timers = fakeTimers(); + const refreshed = []; + const r = createBinaryRefresher({ + setTimeout: timers.setTimeout, + clearTimeout: timers.clearTimeout, + refresh: (path) => refreshed.push(path), + }); + r.schedule('a.pdf'); + r.schedule('a.pdf'); + r.schedule('a.pdf'); + assert.equal(timers.size(), 1, 'only one pending timer for the path'); + timers.flush(); + // refresh now runs in a microtask deferred by the scheduler wrap. + await Promise.resolve(); + await Promise.resolve(); + assert.deepEqual(refreshed, ['a.pdf'], 'refresh fired exactly once'); +}); + +test('schedule: different paths get independent timers', async () => { + const timers = fakeTimers(); + const refreshed = []; + const r = createBinaryRefresher({ + setTimeout: timers.setTimeout, + clearTimeout: timers.clearTimeout, + refresh: (path) => refreshed.push(path), + }); + r.schedule('a.pdf'); + r.schedule('b.xlsx'); + assert.equal(timers.size(), 2); + timers.flush(); + // refresh now runs in a microtask deferred by the scheduler wrap. + await Promise.resolve(); + await Promise.resolve(); + assert.deepEqual(refreshed.sort(), ['a.pdf', 'b.xlsx']); +}); + +test('schedule: pending entry is cleared after the timer fires', async () => { + const timers = fakeTimers(); + const r = createBinaryRefresher({ + setTimeout: timers.setTimeout, + clearTimeout: timers.clearTimeout, + refresh: () => {}, + }); + r.schedule('a.pdf'); + timers.flush(); + // pending.delete happens synchronously in the timer cb (before the microtask), + // but await anyway to stay consistent with the deferred refresh model. + await Promise.resolve(); + await Promise.resolve(); + assert.equal(r._pendingSize(), 0); +}); + +// Build a refresher whose dependencies are all stubs we can assert against. +function makeRefreshHarness(overrides = {}) { + const calls = { viewer: [], swap: 0, scrollSet: [] }; + calls.offscreenRemoved = 0; + calls.missing = []; + let seq = 0; + const contentEl = { scrollTop: 100, scrollHeight: 400 }; + const store = { active: 'a.pdf' }; + const deps = { + setTimeout: (cb) => cb, // unused here + clearTimeout: () => {}, + store, + contentEl, + getViewer: () => async (bytes, container) => { calls.viewer.push([bytes, container]); }, + isPptx: () => false, + fetchBytes: async () => ({ ok: true, bytes: new Uint8Array([1, 2, 3]).buffer }), + makeOffscreen: () => ({ __offscreen: true, remove() { calls.offscreenRemoved++; } }), + swap: () => { calls.swap++; contentEl.scrollHeight = 800; }, + nextSeq: () => ++seq, + currentSeq: () => seq, + setScrollTop: (v) => { calls.scrollSet.push(v); contentEl.scrollTop = v; }, + onMissing: (path) => { calls.missing.push(path); }, + ...overrides, + }; + return { deps, calls, contentEl, store, getSeq: () => seq, bumpSeq: () => ++seq }; +} + +test('refresh: off-screen path renders, swaps, and restores scroll by ratio', async () => { + const { deps, calls } = makeRefreshHarness(); + const r = createBinaryRefresher(deps); + await r.refresh('a.pdf'); + assert.equal(calls.viewer.length, 1, 'viewer rendered once'); + assert.equal(calls.viewer[0][1].__offscreen, true, 'rendered into the off-screen container'); + assert.equal(calls.swap, 1, 'swapped once'); + // prevTop/prevHeight = 100/400 = 0.25; new height 800 -> 200 + assert.deepEqual(calls.scrollSet, [200]); +}); + +test('refresh: aborts when path is no longer the active tab', async () => { + const { deps, calls } = makeRefreshHarness({ store: { active: 'other.md' } }); + const r = createBinaryRefresher(deps); + await r.refresh('a.pdf'); + assert.equal(calls.viewer.length, 0); + assert.equal(calls.swap, 0); +}); + +test('refresh: aborts (no swap) when seq advances during the fetch await', async () => { + const h = makeRefreshHarness(); + // Simulate another render starting mid-fetch: bump seq after fetch resolves. + h.deps.fetchBytes = async () => { + h.bumpSeq(); // a newer render took over + return { ok: true, bytes: new ArrayBuffer(3) }; + }; + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); + assert.equal(h.calls.swap, 0, 'stale render did not swap'); +}); + +test('refresh: failed fetch (!ok) leaves preview untouched', async () => { + const h = makeRefreshHarness(); + h.deps.fetchBytes = async () => ({ ok: false }); + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); + assert.equal(h.calls.viewer.length, 0); + assert.equal(h.calls.swap, 0); +}); + +test('refresh: network error during fetch never rejects and leaves preview untouched', async () => { + const h = makeRefreshHarness(); + h.deps.fetchBytes = async () => { throw new Error('network down'); }; + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); // must resolve, not reject + assert.equal(h.calls.viewer.length, 0); + assert.equal(h.calls.swap, 0); +}); + +test('refresh: viewer throwing on off-screen path leaves preview untouched (no swap)', async () => { + const h = makeRefreshHarness(); + h.deps.getViewer = () => async () => { throw new Error('half-written file'); }; + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); // must not reject + assert.equal(h.calls.swap, 0); +}); + +test('refresh: off-screen container is removed when the viewer throws', async () => { + const h = makeRefreshHarness(); + h.deps.getViewer = () => async () => { throw new Error('half-written file'); }; + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); + assert.equal(h.calls.swap, 0); + assert.equal(h.calls.offscreenRemoved, 1, 'failed off-screen render must be cleaned up'); +}); + +test('refresh: off-screen container is removed when seq goes stale after render', async () => { + const h = makeRefreshHarness(); + // Bump seq during the viewer await so the post-render guard fails. + h.deps.getViewer = () => async () => { h.bumpSeq(); }; + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); + assert.equal(h.calls.swap, 0, 'stale render must not swap'); + assert.equal(h.calls.offscreenRemoved, 1, 'superseded off-screen render must be cleaned up'); +}); + +test('refresh: PPTX renders in place (no off-screen, no swap)', async () => { + // Active path must match the path we refresh, so set it to the pptx. + const h = makeRefreshHarness({ store: { active: 'deck.pptx' } }); + h.contentEl.scrollTop = 0; + h.contentEl.scrollHeight = 400; + h.deps.isPptx = () => true; + const rendered = []; + h.deps.getViewer = () => async (bytes, container) => { rendered.push(container); }; + const r = createBinaryRefresher(h.deps); + await r.refresh('deck.pptx'); + // For PPTX the viewer renders into the live contentEl, not the off-screen box. + assert.equal(rendered.length, 1); + assert.equal(rendered[0], h.contentEl); + assert.equal(h.calls.swap, 0, 'PPTX does not use the swap path'); +}); + +test('refresh: a deleted (404/missing) active file shows the missing state, no render', async () => { + const h = makeRefreshHarness(); + h.deps.fetchBytes = async () => ({ ok: false, missing: true }); + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); + assert.deepEqual(h.calls.missing, ['a.pdf'], 'onMissing called with the path'); + assert.equal(h.calls.viewer.length, 0, 'no viewer render on missing'); + assert.equal(h.calls.swap, 0, 'no swap on missing'); +}); + +test('refresh: non-404 fetch failure ({ok:false}) keeps preview and does NOT show missing', async () => { + const h = makeRefreshHarness(); + h.deps.fetchBytes = async () => ({ ok: false }); + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); + assert.equal(h.calls.missing.length, 0, 'transient failure is not a missing file'); + assert.equal(h.calls.viewer.length, 0); + assert.equal(h.calls.swap, 0); +}); + +test('refresh: missing state is suppressed if a newer render superseded this one', async () => { + const h = makeRefreshHarness(); + h.deps.fetchBytes = async () => { h.bumpSeq(); return { ok: false, missing: true }; }; + const r = createBinaryRefresher(h.deps); + await r.refresh('a.pdf'); + assert.equal(h.calls.missing.length, 0, 'stale refresh must not clobber a newer render with missing UI'); +}); + +test('schedule: a throwing refresh never escapes the scheduler', async () => { + const timers = fakeTimers(); + const r = createBinaryRefresher({ + setTimeout: timers.setTimeout, + clearTimeout: timers.clearTimeout, + refresh: () => { throw new Error('boom'); }, + }); + r.schedule('a.pdf'); + // Must not throw synchronously out of flush, and must not reject. + assert.doesNotThrow(() => timers.flush()); + await Promise.resolve(); + await Promise.resolve(); + // Reaching here without an unhandled rejection is the assertion. + assert.ok(true); +}); diff --git a/web/e2e/binary-live-reload.spec.js b/web/e2e/binary-live-reload.spec.js new file mode 100644 index 0000000..6c02144 --- /dev/null +++ b/web/e2e/binary-live-reload.spec.js @@ -0,0 +1,98 @@ +// binary-live-reload.spec.js — end-to-end tests for auto-updating binary +// document previews in a real browser against a real reefdoc server. +// +// These verify the live-reload PLUMBING (a disk change re-renders the open +// preview) without depending on the real CDN viewer libraries: the pdfjs import +// is stubbed (see fixtures.js) so a ".pdf" whose bytes are UTF-8 text renders +// that text where we can assert on it. + +import { test, expect } from '@playwright/test'; +import { writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { startServer, makeDocsDir } from './server.js'; +import { installPdfStub } from './fixtures.js'; + +// Write a fake "pdf" whose bytes are just the given text (not a real PDF). Our +// stubbed viewer decodes the bytes to text, so the rendered preview shows +// exactly `content`. +function writeFakePdf(dir, name, content) { + writeFileSync(join(dir, name), content); +} + +// openFile clicks a file row in the tree by its filename. +async function openFile(page, name) { + await page.locator('.tree-file .tree-label', { hasText: name }).click(); +} + +// The stubbed PDF preview surfaces its decoded content in a . +function previewText(page) { + return page.locator('#content .fake-pdf-text'); +} + +test.describe('binary document live-reload', () => { + let docs, server; + + test.beforeEach(async ({ page }) => { + docs = makeDocsDir(); + // Stub the CDN PDF viewer BEFORE the page loads any module. + await installPdfStub(page); + server = await startServer(docs.dir); + }); + + test.afterEach(async () => { + server?.stop(); + docs?.cleanup(); + }); + + test('active PDF preview auto-updates when the file changes on disk', async ({ page }) => { + writeFakePdf(docs.dir, 'report.pdf', 'VERSION-ONE'); + await page.goto(server.baseURL); + + // Open the PDF; the stubbed viewer renders its bytes as text. + await openFile(page, 'report.pdf'); + await expect(previewText(page)).toHaveText('VERSION-ONE'); + + // Change the file on disk — the open, active preview must re-render. + writeFakePdf(docs.dir, 'report.pdf', 'VERSION-TWO-CHANGED'); + await expect(previewText(page)).toHaveText('VERSION-TWO-CHANGED'); + }); + + test('background PDF tab is flagged and re-renders on activation', async ({ page }) => { + writeFakePdf(docs.dir, 'report.pdf', 'PDF-ORIGINAL'); + writeFileSync(join(docs.dir, 'notes.md'), '# notes\n'); + await page.goto(server.baseURL); + + // Open the PDF, then switch to the markdown tab so the PDF is in the + // background. + await openFile(page, 'report.pdf'); + await expect(previewText(page)).toHaveText('PDF-ORIGINAL'); + await openFile(page, 'notes.md'); + await expect(page.locator('#content')).toContainText('notes'); + + // Change the backgrounded PDF on disk. + writeFakePdf(docs.dir, 'report.pdf', 'PDF-UPDATED-IN-BG'); + + // Its tab gets the "updated" marker, but the visible pane (markdown) does + // not change. + const pdfTab = page.locator('#tabbar .tab', { hasText: 'report.pdf' }); + await expect(pdfTab).toHaveClass(/updated/); + await expect(page.locator('#content')).toContainText('notes'); + + // Clicking the PDF tab renders the NEW content and clears the marker. + await pdfTab.click(); + await expect(previewText(page)).toHaveText('PDF-UPDATED-IN-BG'); + await expect(pdfTab).not.toHaveClass(/updated/); + }); + + test('deleting the active PDF shows the missing-file state', async ({ page }) => { + writeFakePdf(docs.dir, 'report.pdf', 'SOON-GONE'); + await page.goto(server.baseURL); + + await openFile(page, 'report.pdf'); + await expect(previewText(page)).toHaveText('SOON-GONE'); + + // Delete the file on disk; the active preview should show the missing state. + rmSync(join(docs.dir, 'report.pdf')); + await expect(page.locator('#content')).toContainText('This file no longer exists.'); + }); +}); diff --git a/web/e2e/fixtures.js b/web/e2e/fixtures.js new file mode 100644 index 0000000..4e85069 --- /dev/null +++ b/web/e2e/fixtures.js @@ -0,0 +1,70 @@ +// fixtures.js — shared E2E setup: stub the CDN-loaded PDF viewer so tests run +// offline and can assert on rendered content. +// +// reefdoc's binary viewers (pdfjs-dist, docx-preview, xlsx, pptx-preview) load +// from a CDN via the importmap in index.html. For E2E we don't want to depend on +// third-party CDNs or assert pixel-perfect rendering — we want to verify the +// live-reload PLUMBING (a disk change re-renders the open preview). So we +// intercept the `pdfjs-dist` import and return a tiny fake module that +// implements exactly the API surface web/viewers.js's viewPdf uses, and renders +// the decoded file bytes as visible text we can read back in assertions. +// +// The importmap maps "pdfjs-dist" to +// https://cdn.jsdelivr.net/npm/pdfjs-dist@4.6.82/+esm +// We route that URL to our fake ES module below. + +const PDFJS_CDN = 'https://cdn.jsdelivr.net/npm/pdfjs-dist@4.6.82/+esm'; + +// The fake module source. It mirrors viewPdf's usage: +// pdfjs.GlobalWorkerOptions.workerSrc = ... +// const doc = await pdfjs.getDocument({ data }).promise // {numPages, getPage} +// const page = await doc.getPage(n) // {getViewport, render} +// const viewport = page.getViewport({ scale }) // {width, height} +// await page.render({ canvasContext, viewport }).promise +// +// Instead of rendering a real PDF, it decodes the bytes to UTF-8 text and +// stamps that text onto the canvas's parent container via a data attribute and +// text node, so the test can read the current preview content. Our fixture PDFs +// are just UTF-8 payloads (the viewer never actually parses PDF structure here). +const FAKE_PDFJS = ` +export const GlobalWorkerOptions = { workerSrc: '' }; +export function getDocument({ data }) { + const text = new TextDecoder().decode(new Uint8Array(data)); + return { + promise: Promise.resolve({ + numPages: 1, + getPage() { + return Promise.resolve({ + getViewport: () => ({ width: 200, height: 100 }), + render: ({ canvasContext }) => { + // Expose the decoded content where the test can read it: as a + // data attribute on the canvas and as text in a sibling node. + const canvas = canvasContext.canvas; + canvas.setAttribute('data-fake-content', text); + const marker = document.createElement('pre'); + marker.className = 'fake-pdf-text'; + marker.textContent = text; + canvas.parentNode.appendChild(marker); + return { promise: Promise.resolve() }; + }, + }); + }, + }), + }; +} +`; + +// installPdfStub routes the pdfjs CDN import to the fake module for a page. +// Call BEFORE navigating so the import is intercepted on first load. +export async function installPdfStub(page) { + await page.route(PDFJS_CDN, (route) => + route.fulfill({ + status: 200, + contentType: 'application/javascript', + body: FAKE_PDFJS, + }), + ); + // Belt-and-suspenders: pdf.js also sets a worker URL; never let it hit the + // network. Abort any pdf.worker request (the fake never uses it). + await page.route(/pdf\.worker/, (route) => route.abort()); +} diff --git a/web/e2e/server.js b/web/e2e/server.js new file mode 100644 index 0000000..dc1637c --- /dev/null +++ b/web/e2e/server.js @@ -0,0 +1,121 @@ +// server.js — boot a real reefdoc binary for an E2E test. +// +// Each test gets its own server process, its own ephemeral port, and its own +// temp document directory, so tests are isolated and can mutate files on disk +// without interfering with each other. + +import { spawn, execSync } from 'node:child_process'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createServer } from 'node:net'; + +const repoRoot = fileURLToPath(new URL('../../', import.meta.url)); + +// Build the binary once per process; cache the path. Building up front (rather +// than `go run`) gives a stable, fast-starting server and a clear failure if +// the code doesn't compile. +let binaryPath = null; +function ensureBinary() { + if (binaryPath) return binaryPath; + const out = join(mkdtempSync(join(tmpdir(), 'reefdoc-e2e-bin-')), 'reefdoc'); + execSync(`go build -o "${out}" .`, { cwd: repoRoot, stdio: 'pipe' }); + binaryPath = out; + return out; +} + +// freePort asks the OS for an unused TCP port by binding to :0, then releasing +// it. reefdoc echoes its -addr verbatim (it doesn't report the OS-assigned port +// when given :0), so we pick the port ourselves and pass it explicitly. +function freePort() { + return new Promise((resolve, reject) => { + const srv = createServer(); + srv.unref(); + srv.on('error', reject); + srv.listen(0, '127.0.0.1', () => { + const { port } = srv.address(); + srv.close(() => resolve(port)); + }); + }); +} + +// waitForReady polls the server's root until it responds (or times out). +// `exited` is a function returning true once the child process has gone away, +// so a failed bind rejects promptly instead of burning the whole deadline. +async function waitForReady(baseURL, exited, deadlineMs = 5_000) { + const start = Date.now(); + for (;;) { + if (exited()) { + throw new Error('server process exited before becoming ready at ' + baseURL); + } + try { + const res = await fetch(baseURL + '/'); + if (res.ok) return; + } catch { /* not up yet */ } + if (Date.now() - start > deadlineMs) { + throw new Error('server did not become ready at ' + baseURL); + } + await new Promise((r) => setTimeout(r, 50)); + } +} + +// startServer launches reefdoc serving `dir` on a free port and resolves once +// the server answers HTTP. Returns { baseURL, stop() }. +// +// freePort() picks a port by binding :0 and immediately releasing it; in the +// gap before reefdoc binds, another process could steal that port and the spawn +// would fail to bind. To stay robust we retry the whole pick-port -> spawn -> +// waitForReady sequence with a fresh port each attempt, and surface the child's +// captured output if every attempt fails. +export async function startServer(dir) { + const bin = ensureBinary(); + const maxAttempts = 5; + let lastError; + + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + const port = await freePort(); + const baseURL = `http://127.0.0.1:${port}`; + const proc = spawn(bin, ['-addr', `127.0.0.1:${port}`, dir], { + cwd: repoRoot, + stdio: ['ignore', 'pipe', 'pipe'], + }); + + let log = ''; + let exited = false; + proc.stdout.on('data', (c) => { log += c.toString(); }); + proc.stderr.on('data', (c) => { log += c.toString(); }); + proc.on('exit', (code) => { + exited = true; + if (code) log += `\n[server exited ${code}]`; + }); + + try { + await waitForReady(baseURL, () => exited); + return { + baseURL, + stop() { + try { proc.kill('SIGKILL'); } catch { /* already gone */ } + }, + }; + } catch (err) { + // This attempt failed (early exit or readiness timeout). Kill the child + // and try a fresh port. Stash the error + captured log for the final throw. + try { proc.kill('SIGKILL'); } catch { /* already gone */ } + lastError = new Error( + `${err.message} (attempt ${attempt}/${maxAttempts})\n--- server log ---\n${log}`, + ); + } + } + + throw new Error( + `server failed to start after ${maxAttempts} attempts.\n${lastError?.message ?? ''}`, + ); +} + +// makeDocsDir creates a fresh temp directory for a test's documents and returns +// its path plus a cleanup function. +export function makeDocsDir() { + const dir = mkdtempSync(join(tmpdir(), 'reefdoc-e2e-docs-')); + return { dir, cleanup() { rmSync(dir, { recursive: true, force: true }); } }; +} diff --git a/web/tabs.js b/web/tabs.js index 1eb4cba..ba592d7 100644 --- a/web/tabs.js +++ b/web/tabs.js @@ -28,3 +28,21 @@ export function isOpen(store, path) { export function getTab(store, path) { return store.tabs.find((t) => t.path === path) || null; } + +// dirOf returns the parent directory (slash path, '' for root) of a file path. +export function dirOf(path) { + const i = path.lastIndexOf('/'); + return i === -1 ? '' : path.slice(0, i); +} + +// vanishedTabs returns the paths of open tabs whose file lived directly in +// `dir` but is no longer present in `presentPaths` (the freshly-listed children +// of that directory). Used to detect deletions: a `tree` event refreshes a +// directory listing, and any open tab from that directory missing from the new +// listing was deleted. +export function vanishedTabs(store, dir, presentPaths) { + const present = new Set(presentPaths); + return store.tabs + .map((t) => t.path) + .filter((p) => dirOf(p) === dir && !present.has(p)); +} diff --git a/web/tabs.test.js b/web/tabs.test.js index fc899bc..35c7b64 100644 --- a/web/tabs.test.js +++ b/web/tabs.test.js @@ -1,6 +1,6 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { createTabStore, openTab, closeTab, isOpen, getTab } from './tabs.js'; +import { createTabStore, openTab, closeTab, isOpen, getTab, dirOf, vanishedTabs } from './tabs.js'; test('openTab adds and activates', () => { const s = createTabStore(); @@ -57,3 +57,35 @@ test('closing a path that is not open is a no-op', () => { assert.equal(s.tabs.length, 0); assert.equal(s.active, null); }); + +test('dirOf returns parent directory of a file path', () => { + assert.equal(dirOf('a.pdf'), ''); + assert.equal(dirOf('docs/x.md'), 'docs'); + assert.equal(dirOf('a/b/c.md'), 'a/b'); +}); + +test('vanishedTabs returns open tabs in dir that are absent from the listing', () => { + const s = createTabStore(); + openTab(s, 'a.pdf', 'a'); + openTab(s, 'b.pdf', 'b'); + // a.pdf is gone, b.pdf still present + assert.deepEqual(vanishedTabs(s, '', ['b.pdf']), ['a.pdf']); +}); + +test('vanishedTabs does not return a tab still present in the listing', () => { + const s = createTabStore(); + openTab(s, 'a.pdf', 'a'); + assert.deepEqual(vanishedTabs(s, '', ['a.pdf']), []); +}); + +test('vanishedTabs ignores tabs in a different directory', () => { + const s = createTabStore(); + openTab(s, 'docs/x.md', 'x'); // lives in docs, not root + // refreshing root listing; docs/x.md is absent here but lives elsewhere + assert.deepEqual(vanishedTabs(s, '', []), []); +}); + +test('vanishedTabs on an empty store returns []', () => { + const s = createTabStore(); + assert.deepEqual(vanishedTabs(s, '', []), []); +});