fix(translate): stop the giant-paragraph split from stranding a container's own text (#2109)

Tall containers are split into their descendant paragraphs so viewport-lazy translation still applies (#1881). That split silently drops any text the container holds directly. Harmless on a nested <article> — docs.docker.com's 203k-px <article> owns 0 chars of direct text — but inverted on <br>-delimited article bodies, where the bare text IS the article and the inline fragments are the strays: strobist.blogspot.com 6,580 chars -> 527 covered (8%) paulgraham.com 66,529 chars -> 723 covered (1.1%) The same defect produced the second symptom: a mid-sentence <i> became its own observed unit, so its translation was inserted in the middle of the sentence. The guard rests on a property of the labeling walk: any non-whitespace text node's immediate parent is itself labeled a paragraph, so a text node inside a giant can only be stranded when its parent IS the giant. One loop over childNodes is therefore complete, and structurally cannot see <script> bodies, hidden subtrees or our own wrappers. - Refuse the split when the container owns prose and has a block-level child, so translateWalkedElement's run path re-segments it instead - Keep splitting when it owns no prose, when its own text carries no letters (separators, dates), when it has no block child to re-segment on (refusing there would ship the whole container as one request), when it is <body>, and above a unit-count cap that keeps #1881's gating guarantee - Apply the same guard to the translate side's giant fallback, so both paths make one decision Verified with the built extension in Chrome: strobist 11 -> 41 wrappers with no mid-sentence insertion, paulgraham 236 wrappers all translated, and docs.docker.com still gated (80 wrappers before scrolling, 583 after, of 5,376 paragraphs). Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-09-01 15:36:33 +08:00 · 2026-08-20 13:17:59 -07:00
parent b022eff7dd
commit 59e155ed97
9 changed files with 328 additions and 11 deletions
@@ -0,0 +1,11 @@
+---
+"@read-frog/extension": patch
+---
+
+fix(translate): stop the giant-paragraph split from stranding a container's own text
+
+Tall containers are split into their descendant paragraphs so viewport-lazy translation still applies (#1881). That split silently drops any text the container holds directly, which is harmless on a nested `<article>` but catastrophic on `<br>`-delimited article bodies, where the bare text _is_ the article — a Blogger post kept 8% of its text and paulgraham.com/greatwork.html kept 1.1%, while the incidental inline `<i>` got its own translation inserted mid-sentence.
+
+- Refuse the split when the container owns prose of its own and has a block-level child, so the translate path re-segments it into per-line runs instead
+- Keep splitting when the container owns no prose (unchanged for docs.docker.com), when its own text carries no letters (separators, dates), when it has no block child to re-segment on, when it is `<body>`, or when the split already yields more units than the gating cap
+- Apply the same guard on the translate side's giant fallback, so both paths make one decision
@@ -2,6 +2,7 @@

 import { beforeEach, describe, expect, it, vi } from "vitest"
 import { DEFAULT_CONFIG } from "@/utils/constants/config"
+import { GIANT_SPLIT_STRANDED_TEXT_MAX_UNITS } from "@/utils/constants/translate"
 import {
  markExtensionDrivenNodeRemoval,
  registerBilingualTranslationState,
@@ -74,7 +75,10 @@ vi.mock("@/utils/host/dom/find", () => ({
  deepQueryTopLevelSelector: mockDeepQueryTopLevelSelector,
 }))

-vi.mock("@/utils/host/dom/traversal", () => ({
+// The labeling walk is mocked, but canSplitGiantWithoutStrandingOwnText is
+// kept real: it is the behavior under test in the giant-split cases below.
+vi.mock("@/utils/host/dom/traversal", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("@/utils/host/dom/traversal")>()),
  walkAndLabelElement: mockWalkAndLabelElement,
  walkAndLabelElementChunked: mockWalkAndLabelElementChunked,
 }))
@@ -179,6 +183,8 @@ function deepQueryTopLevelSelectorImpl(
  return result
 }

+const MOCK_BLOCK_TAGS = new Set(["P", "DIV", "BR", "UL", "LI", "SECTION", "ARTICLE", "BODY"])
+
 function isBlockedForTraversal(element: HTMLElement): boolean {
  return (
    Boolean(element.hidden) ||
@@ -212,6 +218,14 @@ function walkAndLabelVisibleParagraphs(
    element.setAttribute("data-read-frog-paragraph", "")
  }

+  // The real walker labels block-level elements too, and the stranded-text
+  // guard reads that label to tell a re-segmentable container from one that
+  // would collapse into a single request. Without this the guard could never
+  // fire under the mock.
+  if (MOCK_BLOCK_TAGS.has(element.tagName)) {
+    element.setAttribute("data-read-frog-block-node", "")
+  }
+
  return {
    forceBlock: false,
    isInlineNode: false,
@@ -1085,13 +1099,16 @@ describe("pageTranslationManager mutation re-walk", () => {
    manager.stop()
  })

-  it("splits a giant paragraph into its descendant paragraphs for observation (#1881)", async () => {
+  it("splits a pure-container giant into its descendant paragraphs (#1881)", async () => {
    // docs.docker.com regression shape: one flat container labeled as a
    // paragraph spanning the whole document, with real paragraphs nested
    // inside. Built via DOM APIs — the HTML parser refuses nested <p>.
+    // The container owns NO direct text, which is what makes the split
+    // lossless — measured on the real page, its <article>'s own text is 0
+    // chars. (An earlier version of this fixture appended a direct text node,
+    // which the real page does not have and which the split would strand.)
    const giant = document.createElement("p")
    giant.id = "giant"
-    giant.append("direct inline text of the giant")
    const inner1 = document.createElement("p")
    inner1.id = "inner1"
    inner1.textContent = "Nested paragraph one"
@@ -1126,4 +1143,109 @@ describe("pageTranslationManager mutation re-walk", () => {

    manager.stop()
  })
+
+  it("refuses to split a giant that owns prose beside block children", async () => {
+    // Blogger / paulgraham.com shape: the container's own bare text IS the
+    // article and the labeled descendants are incidental fragments. Splitting
+    // here observed only the fragments and stranded 92% of the post.
+    const flow = document.createElement("p")
+    flow.id = "flow"
+    const strayInner = document.createElement("p")
+    strayInner.id = "strayInner"
+    strayInner.textContent = "an incidental fragment"
+    flow.append("bare sentence one, which is the actual article", strayInner)
+    flow.append("bare sentence two, also the actual article")
+    document.body.append(flow)
+    flow.getBoundingClientRect = () => ({ height: 200_000 }) as DOMRect
+
+    const manager = new PageTranslationManager()
+    await manager.start()
+    await flushDomUpdates()
+
+    const observed = intersectionObservers[0]!.observe.mock.calls.map((call) => call[0])
+    // Observed whole, so the translate path re-segments it into runs and the
+    // bare sentences are translated instead of dropped.
+    expect(observed).toContain(flow)
+    expect(observed).not.toContain(strayInner)
+
+    manager.stop()
+  })
+
+  it("still splits a giant that owns prose but has no block child", async () => {
+    // Without a block-labeled child the translate path takes its single-node
+    // branch, so observing whole would ship the entire container as ONE
+    // request. Lossy-but-gated beats one doomed payload.
+    const flow = document.createElement("p")
+    flow.id = "flow"
+    const inlinePara = document.createElement("span")
+    inlinePara.id = "inlinePara"
+    inlinePara.textContent = "an inline fragment"
+    inlinePara.setAttribute("data-read-frog-paragraph", "")
+    inlinePara.setAttribute("data-read-frog-inline-node", "")
+    flow.append("bare sentence one", inlinePara, "bare sentence two")
+    document.body.append(flow)
+    flow.getBoundingClientRect = () => ({ height: 200_000 }) as DOMRect
+
+    const manager = new PageTranslationManager()
+    await manager.start()
+    await flushDomUpdates()
+
+    const observed = intersectionObservers[0]!.observe.mock.calls.map((call) => call[0])
+    expect(observed).toContain(inlinePara)
+    expect(observed).not.toContain(flow)
+
+    manager.stop()
+  })
+
+  it("still splits a giant whose split already yields many units", async () => {
+    // Safety valve: above the unit cap, keeping viewport gating beats
+    // rescuing the container's own text — refusing would enqueue the whole
+    // page at once, which is #1881 verbatim.
+    const giant = document.createElement("p")
+    giant.id = "giant"
+    giant.append("a stray sentence the split will strand")
+    const inners: HTMLElement[] = []
+    for (let i = 0; i < GIANT_SPLIT_STRANDED_TEXT_MAX_UNITS + 1; i++) {
+      const inner = document.createElement("p")
+      inner.textContent = `Nested paragraph ${i}`
+      inners.push(inner)
+      giant.append(inner)
+    }
+    document.body.append(giant)
+    giant.getBoundingClientRect = () => ({ height: 200_000 }) as DOMRect
+
+    const manager = new PageTranslationManager()
+    await manager.start()
+    await flushDomUpdates()
+
+    const observed = intersectionObservers[0]!.observe.mock.calls.map((call) => call[0])
+    expect(observed).not.toContain(giant)
+    expect(observed).toContain(inners[0])
+
+    manager.stop()
+  })
+
+  it("never refuses to split <body>", async () => {
+    // <body> is force-block and picks up a paragraph label from any stray
+    // direct text node. Refusing there would collapse the whole document into
+    // one observed unit. The mock walk only labels <p>, so label body the way
+    // the real walker would.
+    const real = document.createElement("p")
+    real.id = "real"
+    real.textContent = "Real paragraph"
+    document.body.append("Loading…", real)
+    document.body.setAttribute("data-read-frog-paragraph", "")
+    document.body.getBoundingClientRect = () => ({ height: 200_000 }) as DOMRect
+
+    const manager = new PageTranslationManager()
+    await manager.start()
+    await flushDomUpdates()
+
+    const observed = intersectionObservers[0]!.observe.mock.calls.map((call) => call[0])
+    expect(observed).not.toContain(document.body)
+    expect(observed).toContain(real)
+
+    manager.stop()
+    document.body.removeAttribute("data-read-frog-paragraph")
+  })
 })
@@ -34,7 +34,8 @@ vi.mock("@/utils/host/dom/find", () => ({
  deepQueryTopLevelSelector: mockDeepQueryTopLevelSelector,
 }))

-vi.mock("@/utils/host/dom/traversal", () => ({
+vi.mock("@/utils/host/dom/traversal", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("@/utils/host/dom/traversal")>()),
  walkAndLabelElement: mockWalkAndLabelElement,
  walkAndLabelElementChunked: vi
    .fn<(...args: any[]) => any>()
@@ -50,7 +50,8 @@ vi.mock("@/utils/host/dom/find", () => ({
  deepQueryTopLevelSelector: mockDeepQueryTopLevelSelector,
 }))

-vi.mock("@/utils/host/dom/traversal", () => ({
+vi.mock("@/utils/host/dom/traversal", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("@/utils/host/dom/traversal")>()),
  walkAndLabelElement: mockWalkAndLabelElement,
  walkAndLabelElementChunked: vi
    .fn<(...args: any[]) => any>()
@@ -20,6 +20,7 @@ import {
  GIANT_PARAGRAPH_MAX_SPLIT_DEPTH,
  GIANT_PARAGRAPH_SPLIT_MIN_VIEWPORT_PX,
  GIANT_PARAGRAPH_SPLIT_VIEWPORT_MULTIPLIER,
+  GIANT_SPLIT_STRANDED_TEXT_MAX_UNITS,
 } from "@/utils/constants/translate"
 import { getRandomUUID } from "@/utils/crypto-polyfill"
 import {
@@ -28,7 +29,11 @@ import {
  isWalkBlockedElement as isWalkBlockedElementFilter,
 } from "@/utils/host/dom/filter"
 import { deepQueryTopLevelSelector } from "@/utils/host/dom/find"
-import { walkAndLabelElement, walkAndLabelElementChunked } from "@/utils/host/dom/traversal"
+import {
+  canSplitGiantWithoutStrandingOwnText,
+  walkAndLabelElement,
+  walkAndLabelElementChunked,
+} from "@/utils/host/dom/traversal"
 import {
  findStaleBilingualLayoutSource,
  findStaleTranslationOnlyAnchor,
@@ -733,10 +738,14 @@ export class PageTranslationManager implements IPageTranslationManager {
   * paragraphs are split: their next-level descendant paragraphs are observed
   * individually instead.
   *
-   * Known tradeoff: direct inline children of a split giant (e.g. those date
-   * <em>s) are not covered by any observed unit and stay untranslated. Stray
-   * standalone inlines in a >3-viewport flat container are rare, and
-   * numeric-only text is skipped by the pipeline anyway.
+   * The split is only sound when the giant owns no prose of its own: by the
+   * labeling rule every other character inside it already sits in one of the
+   * chosen units. That holds on docs.docker.com (its <article>'s own direct
+   * text is zero chars) but inverts on <br>-delimited article bodies, where
+   * the bare text IS the article and the inline <i>/<span> fragments are the
+   * strays — splitting there stranded 92% of a Blogger post and 98.9% of
+   * paulgraham.com/greatwork.html, and gave the stray <i> its own
+   * mid-sentence translation. See canSplitGiantWithoutStrandingOwnText.
   *
   * Exception: a newline-preserving flow container is never split into
   * inline descendants — see canSplitParagraphIntoDescendants.
@@ -775,6 +784,22 @@ export class PageTranslationManager implements IPageTranslationManager {
      observer.observe(element)
      return
    }
+    if (
+      // Bounded by the very thing #1881 measures as harm: how many units one
+      // intersection enqueues. Above the cap, keeping viewport gating beats
+      // rescuing the giant's own text.
+      innerTopLevelParagraphs.length <= GIANT_SPLIT_STRANDED_TEXT_MAX_UNITS &&
+      // <body> is force-block and picks up a paragraph label from any stray
+      // direct text node, so refusing there would collapse the whole document
+      // into ONE observed unit — the outcome the traversal's document-root
+      // guard exists to prevent (gating dies, and the translated-region
+      // querySelector becomes a document-wide silent skip).
+      element !== element.ownerDocument.body &&
+      !canSplitGiantWithoutStrandingOwnText(element)
+    ) {
+      observer.observe(element)
+      return
+    }
    if (!canSplitParagraphIntoDescendants(element, innerTopLevelParagraphs, config)) {
      // A newline-preserving flow (X note tweet: pre-wrap div of inline
      // rich-text <span> paragraphs,
@@ -43,6 +43,14 @@ export const GIANT_PARAGRAPH_SPLIT_VIEWPORT_MULTIPLIER = 3
 export const GIANT_PARAGRAPH_SPLIT_MIN_VIEWPORT_PX = 800
 // Defensive bound on split recursion.
 export const GIANT_PARAGRAPH_MAX_SPLIT_DEPTH = 10
+// Safety valve on the stranded-text refusal: #1881's harm metric is
+// units-enqueued-at-once, so bound the refusal by exactly that. Measured
+// separation is two orders of magnitude — the containers the refusal exists
+// for yield 11 (a Blogger post body), 77 (paulgraham.com/greatwork.html) and
+// ~100 (a tc39 <li>) units, while the containers #1881 exists for yield 1.8k
+// (a Wikipedia <body>), 4k (docs.docker.com's <main>) and 28k (tc39's <body>).
+// Above this, keeping viewport gating beats rescuing a stray sentence.
+export const GIANT_SPLIT_STRANDED_TEXT_MAX_UNITS = 128

 export const MIN_CHARACTERS_PER_NODE = 0
 export const MAX_CHARACTERS_PER_NODE = 1000
@@ -0,0 +1,84 @@
+// @vitest-environment jsdom
+
+import { beforeEach, describe, expect, it } from "vitest"
+import { DEFAULT_CONFIG } from "@/utils/constants/config"
+import { canSplitGiantWithoutStrandingOwnText, walkAndLabelElement } from "../traversal"
+
+/**
+ * Build a fixture and run the REAL labeling walk over it, so the block labels
+ * the predicate reads are the ones production writes.
+ */
+function walked(markup: string): HTMLElement {
+  const root = document.createElement("div")
+  root.innerHTML = markup
+  document.body.append(root)
+  walkAndLabelElement(root, "walk-1", DEFAULT_CONFIG)
+  return root
+}
+
+describe("canSplitGiantWithoutStrandingOwnText", () => {
+  beforeEach(() => {
+    document.body.innerHTML = ""
+  })
+
+  it("refuses when the container owns prose beside block children (Blogger post body)", () => {
+    // The <br>s make it re-segmentable; the bare text between them is the
+    // article itself, and a descendant split would strand all of it.
+    const root = walked(
+      `While f/stop and shutter speed both control exposure<br><br>` +
+        `Since the light from your flash is instantaneous<i>as long as you are at sync speed</i>`,
+    )
+
+    expect(canSplitGiantWithoutStrandingOwnText(root)).toBe(false)
+  })
+
+  it("allows the split when the container owns no text at all (docs.docker.com)", () => {
+    const root = walked(`<p>First paragraph</p><em>2026-08-17</em><p>Second paragraph</p>`)
+
+    expect(canSplitGiantWithoutStrandingOwnText(root)).toBe(true)
+  })
+
+  it("allows the split when only whitespace sits between block children", () => {
+    const root = walked(`
+      <p>First paragraph</p>
+      <p>Second paragraph</p>
+    `)
+
+    expect(canSplitGiantWithoutStrandingOwnText(root)).toBe(true)
+  })
+
+  it("allows the split when the container owns prose but has no block child", () => {
+    // Refusing here would send the whole container as one request, because the
+    // translate path takes its single-node branch without a block child.
+    const root = walked(`bare sentence one<span>an inline fragment</span>bare sentence two`)
+
+    expect(canSplitGiantWithoutStrandingOwnText(root)).toBe(true)
+  })
+
+  it.each([
+    ["a separator", "·"],
+    ["a pipe", " | "],
+    ["an em dash", "—"],
+    ["an ISO date", "2026-08-17"],
+    ["a formatted number", "1,234.56"],
+  ])("allows the split when the only own text is %s", (_label, stray) => {
+    const root = walked(`<p>First paragraph</p>${stray}<p>Second paragraph</p>`)
+
+    expect(canSplitGiantWithoutStrandingOwnText(root)).toBe(true)
+  })
+
+  it("refuses for CJK own text, which carries no ASCII letters", () => {
+    const root = walked(`<p>First paragraph</p>这是正文，不能被拆掉<p>Second paragraph</p>`)
+
+    expect(canSplitGiantWithoutStrandingOwnText(root)).toBe(false)
+  })
+
+  it("ignores script and style bodies, which are never bare text children", () => {
+    const root = walked(
+      `<p>First paragraph</p><script>const a = "text"</script>` +
+        `<style>.a { color: red }</style><p>Second paragraph</p>`,
+    )
+
+    expect(canSplitGiantWithoutStrandingOwnText(root)).toBe(true)
+  })
+})
@@ -264,3 +264,63 @@ export async function walkAndLabelElementChunked(
  }
  return step.value
 }
+
+/**
+ * Text counts as prose only when it contains a letter. Deliberately NARROWER
+ * than the `.trim()` test walkNode itself uses above: a stray "·", "|", "—" or
+ * a bare "2026-08-17" sitting between two block children is not text a reader
+ * would miss, and treating it as prose would forfeit viewport gating on
+ * exactly the flat containers #1881 was about. `\p{L}` covers Han/Hiragana/
+ * Hangul and subsumes isNumericContent, whose /^[\d\s,.-]+$/ admits no letter.
+ */
+const OWN_PROSE_RE = /\p{L}/u
+
+/**
+ * Giant-paragraph split guard — the CONTENT counterpart to
+ * `canSplitParagraphIntoDescendants`, which guards paragraph STRUCTURE.
+ *
+ * Splitting a giant observes its descendant paragraphs INSIDE what is really
+ * one translation unit. That is free on docs.docker.com (a 203k-px <article>
+ * whose own direct text is ZERO chars) and catastrophic on a Blogger post body
+ * or paulgraham.com (a flat <br>-delimited container whose only labeled
+ * descendants are inline <i>/<span>/<a> fragments — 8% and 1.1% of the article
+ * respectively; the rest is bare text nodes that belong to no observed unit
+ * and are never enqueued, while the <i> gets its own translation inserted
+ * mid-sentence).
+ *
+ * Only DIRECT text children can be stranded. By the labeling rule above, any
+ * element with a non-whitespace direct text child is itself labeled a
+ * paragraph, so every deeper text node already lies inside one of the chosen
+ * units. One loop over childNodes is therefore complete — and structurally
+ * blind to <script>, <style>, <pre>, hidden subtrees and our own translation
+ * wrappers, none of which can be a bare Text child.
+ *
+ * The refusal additionally requires a block-labeled child, i.e. that
+ * translateWalkedElement will really take its run path and re-segment the
+ * container. Without one it takes the single-node path and the whole giant
+ * ships as ONE request — a single node is never split by length. Note this is
+ * the MIRROR-OPPOSITE precondition to the pre-wrap clause in
+ * `canSplitParagraphIntoDescendants`, which refuses only in the all-inline
+ * shape; the two refusal domains are disjoint by construction.
+ */
+export function canSplitGiantWithoutStrandingOwnText(element: HTMLElement): boolean {
+  let hasOwnProse = false
+  let hasBlockChild = false
+
+  for (const child of element.childNodes) {
+    // nodeType directly, matching walkNode above — this predicate runs inside
+    // the observation gate, where the filter module may be stubbed out.
+    if (child.nodeType === Node.TEXT_NODE) {
+      if (!hasOwnProse && OWN_PROSE_RE.test(child.textContent ?? "")) {
+        hasOwnProse = true
+      }
+      continue
+    }
+    if (!hasBlockChild && isHTMLElement(child) && child.hasAttribute(BLOCK_ATTRIBUTE)) {
+      hasBlockChild = true
+    }
+    if (hasOwnProse && hasBlockChild) return false
+  }
+
+  return !(hasOwnProse && hasBlockChild)
+}
@@ -28,7 +28,7 @@ import {
 } from "../../dom/filter"
 import { unwrapDeepestOnlyHTMLChild } from "../../dom/find"
 import { getOwnerDocument } from "../../dom/node"
-import { extractTextContent } from "../../dom/traversal"
+import { canSplitGiantWithoutStrandingOwnText, extractTextContent } from "../../dom/traversal"
 import {
  buildVirtualParagraphPlan,
  canMaterializeVirtualParagraphUnits,
@@ -780,6 +780,11 @@ async function translateNonMaterializableGiant(
  forceRetranslation: boolean,
 ): Promise<boolean> {
  if (!isGiantParagraphUnit(layoutSource)) return false
+  // Same content guard the observation gate applies: splitting into descendant
+  // paragraphs drops the container's own direct text. Falling back to the
+  // single run keeps that text rather than silently losing it here, after the
+  // observer already refused to lose it.
+  if (!canSplitGiantWithoutStrandingOwnText(layoutSource)) return false

  const paragraphs = collectTopLevelParagraphDescendants(layoutSource)
  if (paragraphs.length === 0) return false