fix(reader): the archive banner was ~100% false — gate on the hash

docs/JOURNAL.md

@@ -19,6 +19,62 @@ or files, and the "so-what" for future readers.
 
 ---
 
+## 2026-07-17 — the archive banner was ~100% false, and structurally so
+
+Tags: `bug`, `design`.
+
+Reported from daily use: *"Major bugs are experienced daily by me in the
+content comparisons... very misleading banners so frequent that I ignore
+them."* The banner was not mistuned. It could not have worked.
+
+`shouldOfferArchive` compared **raw HTML strings**. The two sides are
+different substrates and always were:
+
+- capture → `article.content`, Readability `innerHTML`, wrapped in
+  `<div id="readability-page-1">`
+- relay → `markdownToHtml(markdown)`, rebuilt from the event
+
+For any **multi-paragraph** article — every real one — these cannot
+match: `markdownToHtml` joins paragraphs with `\n\n` while Readability
+emits `</p><p>` with no separator. So neither the equality guard nor the
+containment guard could fire, and with the default `'always'` mode
+probing unconditionally, the banner fired on every published article,
+every visit, carrying no information. Once a banner is always on, it is
+off.
+
+**Worth recording precisely, because the first cut of this was stated too
+strongly:** containment is not unreachable in *principle*. A
+single-paragraph body has no `\n\n` to introduce, so it IS a clean
+substring of its Readability wrapper and the guard suppressed correctly.
+Short pieces behaved; real articles did not. That asymmetry is why the
+flood looked erratic rather than total.
+`tests/archive-banner.test.mjs` pins both halves, with the relay fixture
+taken verbatim from real `markdownToHtml` output.
+
+**Fix:** gate on the canonical 13.4 hash, which was already correct on
+both sides and simply never consulted — the published `x` tag (read back
+as `_articleHash`), the archive row's `articleHash`, and
+`state.articleHash` agree by construction. The gate only ever
+*suppresses*: equal hashes ⇒ same canonical content ⇒ nothing to offer,
+in any mode. A missing hash (older rows, pre-13.4 events) or a real
+difference falls through to the prior heuristics untouched, so `'richer'`
+stays conservative.
+
+Two things fell out of doing it: the decision moved to
+`reader/archive-banner.js` (pure, so it can be tested — `index.js` is not
+importable from tests), and `checkArchiveAvailability` now computes the
+current hash itself when `state.articleHash` has not landed. That hash is
+filled by an async IIFE at load while the probe runs on a 100ms timer —
+the probe can win, and silently degrading to the unsound body compare is
+exactly the bug. When the user has edited, the hash is legitimately stale,
+so it stays null and the body heuristics answer, as before.
+
+**Not fixed here:** the markdown→HTML→markdown escape doubling that makes
+a Load-archive round trip mint a NEW `x` tag (see 2026-07-08, "PDF stack,
+round three" — fixed for the PDF path only; articles still re-derive).
+Until that lands, a hash difference after a round trip may be an artifact
+rather than a real edit.
+
 ## 2026-07-17 — "Load archive" served the WRONG ARTICLE: `#r` is not an identity index
 
 Tags: `bug`, `design`.

src/reader/archive-banner.js

@@ -0,0 +1,82 @@
+// Archive-banner decision — pure, so it can be tested.
+//
+// The reader offers an archived body over the current capture when the
+// two differ. Deciding "differ" on the BODIES was the bug: the two
+// sides are not comparable and never were.
+//
+//   capture side  → article.content = Readability innerHTML, wrapped
+//                   in <div id="readability-page-1">
+//   relay side    → markdownToHtml(markdown), rebuilt from the event
+//
+// For any MULTI-paragraph article — i.e. every real one — these cannot
+// match: markdownToHtml joins paragraphs with "\n\n" while Readability
+// emits "</p><p>" with no separator, so neither the equality guard nor
+// the containment guard below can fire. With the default 'always' mode
+// probing unconditionally, the banner fired on every published article,
+// every visit — ~100% false, which trained the user to ignore it.
+//
+// (A single-paragraph body has no "\n\n" to introduce, so it IS a clean
+// substring of its wrapper and the containment guard did suppress. Short
+// pieces behaved; real articles did not. tests/archive-banner.test.mjs
+// pins both halves.)
+//
+// The canonical Phase-13.4 article hash is the sound test, and it was
+// already present on both sides, simply never consulted: the published
+// `x` tag (read back as `_articleHash`), the archive row's
+// `articleHash`, and the reader's `state.articleHash` agree by
+// construction.
+//
+// No chrome.*, no DOM — the caller supplies the bodies and the hashes.
+
+/**
+ * Should an archived body be surfaced over the current capture?
+ *
+ * The hash gate only ever SUPPRESSES: a match means the same canonical
+ * content, so there is nothing to offer in any mode. A missing hash
+ * (older cache rows, a pre-13.4 event) or a genuine difference falls
+ * through to the body heuristics, unchanged from before.
+ *
+ * Sensitivity modes (Options → Advanced → Archive banner):
+ *   'richer' — the archive must be ≥1.3× longer AND >1000 chars.
+ *   'always' — any non-trivial difference; skips byte-identical bodies
+ *              and skips when the archive is strictly contained in the
+ *              current body (the current is a superset, so the archive
+ *              can only lose information).
+ *
+ * @param {string} currentBody   the capture on screen
+ * @param {string} archiveBody   the candidate archived body
+ * @param {string} mode          'always' | 'richer'
+ * @param {string|null} [currentHash]  canonical hash of currentBody
+ * @param {string|null} [archiveHash]  canonical hash of archiveBody
+ * @returns {boolean}
+ */
+export function shouldOfferArchive(currentBody, archiveBody, mode, currentHash, archiveHash) {
+    if (!archiveBody) return false;
+    if (currentHash && archiveHash && currentHash === archiveHash) return false;
+    if (mode === 'richer') {
+        return archiveBody.length > currentBody.length * 1.3 && archiveBody.length > 1000;
+    }
+    if (archiveBody === currentBody) return false;
+    if (currentBody && currentBody.includes(archiveBody)) return false;
+    return true;
+}
+
+/**
+ * Human-readable reason the banner is showing. Length-based on purpose:
+ * it describes the SIZE difference the user can act on, and says only
+ * that the bodies differ when neither is meaningfully longer.
+ *
+ * @param {string} currentBody
+ * @param {string} archiveBody
+ * @returns {string}
+ */
+export function describeMetric(currentBody, archiveBody) {
+    const cur = currentBody.length;
+    const arc = archiveBody.length;
+    if (cur > 0 && arc >= cur * 1.3) {
+        return `Archive is ${(arc / Math.max(cur, 1)).toFixed(1)}× longer`;
+    }
+    if (arc > cur) return `Archive is ${arc - cur} chars longer`;
+    if (arc < cur) return `Archive is ${cur - arc} chars shorter`;
+    return 'Archive differs from current capture';
+}

src/reader/index.js

@@ -41,6 +41,7 @@ import { loadFlags, isEnabled } from '../shared/metadata/feature-flags.js';
 import { ForensicModel, ForensicBaseline } from '../shared/forensic-model.js';
 import { openFindingModal, openBaselineModal } from '../shared/forensic-modal.js';
 import { renderFindingsBar } from './findings-section.js';
+import { shouldOfferArchive, describeMetric } from './archive-banner.js';
 import { openLlmReview } from './llm-review.js';
 import { capturePdfToArticle } from './pdf-capture.js';
 import { pageOfOffset, pageFragmentSelector } from '../shared/pdf-layout.js';
@@ -561,6 +562,24 @@ async function checkArchiveAvailability() {
     const currentBody = state.article.content || '';
     const currentLen = currentBody.length;
 
+    // The canonical hash of what's on screen — the only sound way to
+    // ask "is the archive the same content?" (see shouldOfferArchive).
+    // `state.articleHash` is filled by an async IIFE at load and this
+    // probe runs on a 100ms timer, so it can win that race; compute it
+    // here rather than silently degrading to the unsound body compare.
+    // When the user has edited, the hash is legitimately stale — leave
+    // it null and let the body heuristics answer, as before.
+    let currentHash = null;
+    if (!state.hashDirty) {
+        currentHash = state.articleHash;
+        if (!currentHash) {
+            try {
+                currentHash = await canonicalArticleHash(
+                    EventBuilder.assembleArticleBody(hashableArticle(state.article)));
+            } catch (_) { currentHash = null; }
+        }
+    }
+
     // 1. Try local cache first — then through the alias map (a prior
     //    capture of the same piece may key under the alias-resolved
     //    original of this address).
@@ -574,7 +593,7 @@ async function checkArchiveAvailability() {
     }
     if (cached && cached.article && cached.article.content) {
         const cachedBody = cached.article.content;
-        if (shouldOfferArchive(currentBody, cachedBody, mode)) {
+        if (shouldOfferArchive(currentBody, cachedBody, mode, currentHash, cached.articleHash)) {
             renderArchiveBanner({
                 source:   'cache',
                 cachedAt: cached.cachedAt,
@@ -597,7 +616,8 @@ async function checkArchiveAvailability() {
             });
             if (resp && resp.ok && resp.found && resp.article) {
                 const reconstructedBody = resp.article.content || '';
-                if (shouldOfferArchive(currentBody, reconstructedBody, mode)) {
+                if (shouldOfferArchive(currentBody, reconstructedBody, mode,
+                        currentHash, resp.article._articleHash)) {
                     renderArchiveBanner({
                         source:    'relay',
                         author:    resp.authorPubkey,
@@ -630,37 +650,6 @@ async function loadPreferences() {
     });
 }
 
-/**
- * Decide whether an archived body is worth surfacing over the current
- * capture, given the user's sensitivity preference.
- *
- * 'richer' keeps the prior 1.3×/1000-char threshold.
- * 'always' shows whenever the archive is non-trivially different —
- *          skip byte-identical matches and skip when the archive is
- *          strictly contained in the current body (the current is a
- *          superset, so the archive can only lose information).
- */
-function shouldOfferArchive(currentBody, archiveBody, mode) {
-    if (!archiveBody) return false;
-    if (mode === 'richer') {
-        return archiveBody.length > currentBody.length * 1.3 && archiveBody.length > 1000;
-    }
-    if (archiveBody === currentBody) return false;
-    if (currentBody && currentBody.includes(archiveBody)) return false;
-    return true;
-}
-
-function describeMetric(currentBody, archiveBody) {
-    const cur = currentBody.length;
-    const arc = archiveBody.length;
-    if (cur > 0 && arc >= cur * 1.3) {
-        return `Archive is ${(arc / Math.max(cur, 1)).toFixed(1)}× longer`;
-    }
-    if (arc > cur) return `Archive is ${arc - cur} chars longer`;
-    if (arc < cur) return `Archive is ${cur - arc} chars shorter`;
-    return 'Archive differs from current capture';
-}
-
 /**
  * Render the archive banner above the article body. Two actions:
  *

tests/archive-banner.test.mjs

@@ -0,0 +1,137 @@
+// Archive-banner decision (reader/archive-banner.js).
+//
+// Reported from daily use: *"Major bugs are experienced daily by me in
+// the content comparisons... very misleading banners so frequent that I
+// ignore them."* The banner was ~100% false, and structurally so — not
+// a tuning problem.
+//
+// `shouldOfferArchive` compared raw HTML strings. The capture side is
+// Readability innerHTML (wrapped in <div id="readability-page-1">); the
+// relay side is markdownToHtml(markdown). For any multi-paragraph
+// article — i.e. every real one — the two cannot match: markdownToHtml
+// joins paragraphs with "\n\n" while Readability emits "</p><p>" with
+// no separator, so neither the equality guard nor the containment guard
+// can fire. With 'always' (the default) probing unconditionally, the
+// banner fired on every published article, every visit, forever.
+//
+// (Precisely: containment is not unreachable in *principle* — a
+// single-paragraph body IS a clean substring of its Readability
+// wrapper, and then the old guard did suppress. The flood came from
+// real, multi-paragraph articles. The fixture below is built from the
+// actual markdownToHtml output so the claim is the true one.)
+//
+// The canonical 13.4 hash is the sound test and was already correct on
+// both sides. These tests pin the failure mode (so nobody "fixes" the
+// banner by tuning a threshold again) and the hash gate that closes it.
+
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+
+const { shouldOfferArchive, describeMetric } =
+    await import('../src/reader/archive-banner.js');
+
+const HASH_A = 'a1b2c3d4e5f60718293a4b5c6d7e8f90a1b2c3d4e5f60718293a4b5c6d7e8f90';
+const HASH_B = 'ffffffff11111111222222223333333344444444555555556666666677777777';
+
+// Same two-paragraph article, two substrates. RELAY_HTML is verbatim
+// ContentExtractor.markdownToHtml('...\n\n...') output; CAPTURE_HTML is
+// the Readability shape of the same prose.
+const CAPTURE_HTML = '<div id="readability-page-1"><div><p>The reporting rests on unnamed sources.</p><p>A second paragraph follows.</p></div></div>';
+const RELAY_HTML   = '<p>The reporting rests on unnamed sources.</p>\n\n<p>A second paragraph follows.</p>';
+
+// --- the root cause, pinned ---------------------------------------------------
+
+test('THE BUG: same article, two substrates — the body guards cannot fire', () => {
+    // The whole disease. Both guards are false for identical content,
+    // so pre-fix the 'always' path offered a banner for an article
+    // that had not changed at all.
+    assert.notEqual(CAPTURE_HTML, RELAY_HTML, 'equality guard cannot fire');
+    assert.equal(CAPTURE_HTML.includes(RELAY_HTML), false,
+        'containment cannot fire: markdownToHtml separates paragraphs with \\n\\n, Readability does not');
+    assert.equal(shouldOfferArchive(CAPTURE_HTML, RELAY_HTML, 'always'), true,
+        'without hashes this still offers — the pre-fix behavior, preserved as a fallback');
+});
+
+test('THE FIX: equal hashes suppress the banner even though the bodies differ', () => {
+    assert.equal(shouldOfferArchive(CAPTURE_HTML, RELAY_HTML, 'always', HASH_A, HASH_A), false,
+        'same canonical content ⇒ nothing to offer');
+});
+
+test('the hash gate suppresses in every mode', () => {
+    for (const mode of ['always', 'richer']) {
+        assert.equal(shouldOfferArchive('short', 'x'.repeat(5000), mode, HASH_A, HASH_A), false,
+            `${mode}: a hash match wins over the length heuristic`);
+    }
+});
+
+// --- the gate only ever suppresses --------------------------------------------
+
+test('differing hashes do NOT force a banner — the mode still decides', () => {
+    // 'richer' must stay conservative: a real difference that is not
+    // meaningfully fuller is still not worth interrupting for.
+    assert.equal(shouldOfferArchive('x'.repeat(2000), 'y'.repeat(2100), 'richer', HASH_A, HASH_B), false,
+        'richer: not 1.3× longer ⇒ no banner, hashes notwithstanding');
+    assert.equal(shouldOfferArchive('x'.repeat(500), 'y'.repeat(5000), 'richer', HASH_A, HASH_B), true,
+        'richer: genuinely fuller ⇒ banner');
+    assert.equal(shouldOfferArchive(CAPTURE_HTML, RELAY_HTML, 'always', HASH_A, HASH_B), true,
+        'always: a real difference ⇒ banner');
+});
+
+test('a missing hash degrades to the prior behavior, never to a wrong suppression', () => {
+    // Older cache rows and pre-13.4 events carry no hash. Falling back
+    // is honest; suppressing on a half-known comparison would hide a
+    // real difference.
+    assert.equal(shouldOfferArchive(CAPTURE_HTML, RELAY_HTML, 'always', HASH_A, null), true);
+    assert.equal(shouldOfferArchive(CAPTURE_HTML, RELAY_HTML, 'always', null, HASH_A), true);
+    assert.equal(shouldOfferArchive(CAPTURE_HTML, RELAY_HTML, 'always', null, null), true);
+    assert.equal(shouldOfferArchive(CAPTURE_HTML, RELAY_HTML, 'always', '', ''), true,
+        'empty strings are not a match');
+});
+
+// --- prior behavior, unchanged ------------------------------------------------
+
+test('no archive body is never an offer', () => {
+    assert.equal(shouldOfferArchive('anything', '', 'always'), false);
+    assert.equal(shouldOfferArchive('anything', null, 'always'), false);
+    assert.equal(shouldOfferArchive('anything', '', 'always', HASH_A, HASH_B), false);
+});
+
+test('cache path: byte-identical bodies still suppress without hashes', () => {
+    // The local-cache path CAN byte-match (same substrate both sides),
+    // which is why this guard was reachable there and the flood was
+    // worst on the relay path.
+    assert.equal(shouldOfferArchive(CAPTURE_HTML, CAPTURE_HTML, 'always'), false);
+});
+
+test('an archive strictly contained in the current body suppresses', () => {
+    assert.equal(shouldOfferArchive('<p>full body here</p>', 'full body', 'always'), false,
+        'the current capture is a superset — the archive can only lose information');
+});
+
+test('the single-paragraph case: containment DOES fire, which is why the flood looked erratic', () => {
+    // A one-paragraph body has no "\n\n" to introduce, so the relay HTML
+    // is a clean substring of its Readability wrapper and the old guard
+    // suppressed correctly. Short pieces behaved; real articles did not.
+    // Pinned so the distinction is not mistaken for a regression later.
+    const oneParaCapture = '<div id="readability-page-1"><div><p>Only one paragraph.</p></div></div>';
+    const oneParaRelay   = '<p>Only one paragraph.</p>';
+    assert.equal(oneParaCapture.includes(oneParaRelay), true);
+    assert.equal(shouldOfferArchive(oneParaCapture, oneParaRelay, 'always'), false);
+});
+
+test('richer keeps the 1.3×/1000-char threshold exactly', () => {
+    assert.equal(shouldOfferArchive('x'.repeat(1000), 'y'.repeat(1300), 'richer'), false,
+        'exactly 1.3× is not > 1.3×');
+    assert.equal(shouldOfferArchive('x'.repeat(1000), 'y'.repeat(1301), 'richer'), true);
+    assert.equal(shouldOfferArchive('x'.repeat(500), 'y'.repeat(1000), 'richer'), false,
+        '2× longer but not >1000 chars');
+});
+
+// --- describeMetric -----------------------------------------------------------
+
+test('describeMetric reports the size difference the user can act on', () => {
+    assert.equal(describeMetric('x'.repeat(1000), 'y'.repeat(3000)), 'Archive is 3.0× longer');
+    assert.equal(describeMetric('x'.repeat(1000), 'y'.repeat(1100)), 'Archive is 100 chars longer');
+    assert.equal(describeMetric('x'.repeat(1000), 'y'.repeat(900)), 'Archive is 100 chars shorter');
+    assert.equal(describeMetric('x'.repeat(1000), 'y'.repeat(1000)), 'Archive differs from current capture');
+});