Cover the Show changes highlight gates in source CI

docs/REMAINING_WORK.md

@@ -51,7 +51,7 @@ Work in steps 1–3 and most documentation work does not depend on Apple signing
 This means the automatic marks showing what the AI changed. The existing tools for drawing notes on the PDF are a separate feature.
 
 - [x] **PDF-01 — Fix the confirmed late-history-read bug.** Repaired 28 September 2026 on `fix/pdf-highlight-late-read`. `src/App.tsx` now keys the late before-PDF read on the applied project id and revision instead of `activeRun`, so a valid late reply completes while a project switch or newer build still discards it. The retained `0bee271` package record fails the delayed case (`pdf-highlight-Xt0uDy`); the repaired build passes both ordinary and delayed scenarios (`test-results/pdf-highlight-qJH9cp`, published in `releases/pdf-highlight-fix-verification.json`). **Done when:** the retained old package fails the delayed case and the repaired app passes it, plus ordinary comparison still works.
-- [x] **PDF-02 — Supply the missing declared test.** Completed 28 September 2026. The prepared fixture is promoted to `scripts/test-pdf-highlight-smoke.mjs`; `npm run test:pdf-highlight` builds and runs it from the checkout, and both scenarios pass with retained evidence and the portable failure/fix record above. Final packaged-workflow qualification remains with REL-02. **Done when:** the documented command runs from a clean checkout and the final packaged workflow passes.
+- [x] **PDF-02 — Supply the missing declared test.** Completed 28 September 2026. The prepared fixture is promoted to `scripts/test-pdf-highlight-smoke.mjs`; `npm run test:pdf-highlight` builds and runs it from the checkout, and both scenarios pass with retained evidence and the portable failure/fix record above. The late-read, displayed-document and page-clamp rules also have fast source tests in `tests/change-highlight-gate.test.ts` (15 October 2026), so ordinary `npm test` CI catches a regression without a Mac. Final packaged-workflow qualification remains with REL-02. **Done when:** the documented command runs from a clean checkout and the final packaged workflow passes.
 - [x] **PDF-03 — Check late and replaced results.** Completed for the locally reachable scope, 28 September 2026. `PdfPreview` now drops a comparison whose after-bytes differ from the displayed PDF, so old marks and their controls cannot appear over a replacement, manual rebuild, History restore or another project; `App.tsx` clears the comparison on project load and restore, and the identity guard discards late work for a stale project/build. The delayed smoke scenario exercises completion-after-settle ordering end to end. Cancellation/closing-while-pending remain covered only by the hook's abort-on-change/unmount path, not a dedicated native case. **Done when:** marks and navigation always belong to the intended displayed PDF, and late work is safely discarded when its project/build is no longer current.
 - [x] **PDF-04 — Check pages added and removed.** Completed 28 September 2026. `PdfPreview.navigate` clamps to the displayed document's page count, and dedicated native cases now run in the ordinary smoke scenario (`scripts/test-pdf-highlight-smoke.mjs`, evidence `test-results/pdf-highlight-71aVz0`): a real AI removal leaves the structural removed marker on the last surviving page and it outlives the timed flash; a real AI insertion shows the inserted outline and label and clears the stale removed marker. Page numbers stay valid, all supported changes are reachable, and no obsolete regions are shown on another page.
 - [x] **PDF-05 — Preserve correct rendering and export.** Completed for the locally reachable scope, 28 September 2026. The maintained smoke keeps the ordinary fixture's checks — normalized region placement, timed fade, replay, zoom stability, two-page Previous/Next navigation, saving, and highlight-free exported bytes equal to the saved version — and both scenarios pass on the repaired build. Worker cleanup relies on the comparison hook's abort/unmount path; no new leak was observed. **Done when:** actual exported bytes equal the selected saved PDF and comparison work does not leave active workers or stale UI after disposal.

src/App.tsx

@@ -51,6 +51,7 @@ import { TemplatePicker } from './components/TemplatePicker';
 import { LatexEditor, type EditorHandle } from './components/LatexEditor';
 import { PdfPreview, type PdfPreviewHandle } from './components/PdfPreview';
 import type { ChangeHighlightRequest } from './usePdfChangeHighlight';
+import { lateBeforeReadIsCurrent } from './change-highlight-gate';
 import { ActionMenu } from './components/ActionMenu';
 import { ProjectFiles } from './components/ProjectFiles';
 import { Modal } from './components/Modal';
@@ -1139,8 +1140,10 @@ export default function App() {
               // project/build identity is unchanged; a project switch or a
               // newer edit/build bumps id/revision and safely discards it.
               if (
-                current.current.id !== appliedProjectId ||
-                current.current.revision !== appliedRevision
+                !lateBeforeReadIsCurrent(
+                  { id: appliedProjectId, revision: appliedRevision },
+                  current.current,
+                )
               )
                 return;
               setChangeHighlight({ runId: id, before: before.pdf, after: afterPdf });

src/change-highlight-gate.ts

@@ -0,0 +1,32 @@
+// Pure gates for the AI "Show changes" PDF highlight. They are kept out of
+// App/PdfPreview so the late-read and displayed-document rules have fast
+// source tests; the native smoke (`npm run test:pdf-highlight`) covers the UI.
+
+export type AppliedIdentity = { id: string; revision: number };
+
+/**
+ * Whether a before-PDF read that resolves after an AI apply still belongs to
+ * the displayed project/build. Deliberately ignores the agent run: completion
+ * clears it, and a valid late reply must still show its highlight (PR #45).
+ * A project switch or any newer edit/build changes id/revision and discards it.
+ */
+export function lateBeforeReadIsCurrent(applied: AppliedIdentity, current: AppliedIdentity) {
+  return current.id === applied.id && current.revision === applied.revision;
+}
+
+/**
+ * The comparison to draw over `displayed`, or undefined when a different PDF
+ * (manual rebuild, History restore, project switch) is on screen, so old marks
+ * and their controls never appear over a document they do not describe.
+ */
+export function highlightForDisplayed<T extends { after: Uint8Array }>(
+  request: T | undefined,
+  displayed: Uint8Array | undefined,
+): T | undefined {
+  return request && displayed === request.after ? request : undefined;
+}
+
+/** Clamps a change-stop page to the displayed document's page range. */
+export function clampChangePage(page: number, pageCount: number | undefined) {
+  return pageCount ? Math.max(1, Math.min(page, pageCount)) : page;
+}

src/components/PdfPreview.tsx

@@ -33,6 +33,7 @@ import { PdfAnnotations } from './PdfAnnotations';
 import { PdfChangeOverlay } from './PdfChangeOverlay';
 import { usePdfDocument } from '../usePdfDocument';
 import { usePdfChangeHighlight, type ChangeHighlightRequest } from '../usePdfChangeHighlight';
+import { clampChangePage, highlightForDisplayed } from '../change-highlight-gate';
 import {
   PDF_LIMITS,
   pdfCanvasSize,
@@ -373,8 +374,7 @@ export const PdfPreview = forwardRef<
   // A different displayed PDF (manual rebuild, History restore, project
   // switch) drops the comparison entirely so old marks and their controls
   // can never appear over a document they do not describe.
-  const displayedHighlight =
-    changeHighlight && data === changeHighlight.after ? changeHighlight : undefined;
+  const displayedHighlight = highlightForDisplayed(changeHighlight, data);
   const highlightActive = !!displayedHighlight && visibleReady && !loading && !error;
   const {
     state: highlightState,
@@ -440,7 +440,7 @@ export const PdfPreview = forwardRef<
     // Change stops may reference a page that no longer exists (content
     // removed at the end of the document). Clamp to the displayed PDF so
     // the page counter and scroll target stay valid.
-    const clamped = document ? Math.max(1, Math.min(next, document.numPages)) : next;
+    const clamped = clampChangePage(next, document?.numPages);
     setPage(clamped);
     scroll.current
       ?.querySelector(`[data-page="${clamped}"]`)

tests/change-highlight-gate.test.ts

@@ -0,0 +1,37 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import {
+  clampChangePage,
+  highlightForDisplayed,
+  lateBeforeReadIsCurrent,
+} from '../src/change-highlight-gate';
+
+test('a late before-PDF read still applies after the agent run has cleared', () => {
+  // PR #45 regression: the old guard compared against the active run, which
+  // completion clears before a slow History read returns, so the highlight
+  // never appeared. Only project/build identity decides now.
+  assert.equal(lateBeforeReadIsCurrent({ id: 'p1', revision: 7 }, { id: 'p1', revision: 7 }), true);
+});
+
+test('a late before-PDF read is discarded after a project switch or newer edit', () => {
+  const applied = { id: 'p1', revision: 7 };
+  assert.equal(lateBeforeReadIsCurrent(applied, { id: 'p2', revision: 7 }), false);
+  assert.equal(lateBeforeReadIsCurrent(applied, { id: 'p1', revision: 8 }), false);
+});
+
+test('highlights only draw over the PDF they were computed for', () => {
+  const after = new Uint8Array([1, 2, 3]);
+  const request = { runId: 'r1', before: new Uint8Array([0]), after };
+  assert.equal(highlightForDisplayed(request, after), request);
+  // Equal bytes in a different buffer are a different displayed document.
+  assert.equal(highlightForDisplayed(request, new Uint8Array([1, 2, 3])), undefined);
+  assert.equal(highlightForDisplayed(request, undefined), undefined);
+  assert.equal(highlightForDisplayed(undefined, after), undefined);
+});
+
+test('change-stop navigation stays inside the displayed page range', () => {
+  assert.equal(clampChangePage(5, 3), 3);
+  assert.equal(clampChangePage(0, 3), 1);
+  assert.equal(clampChangePage(2, 3), 2);
+  assert.equal(clampChangePage(4, undefined), 4);
+});