test(review): pin the fold invariant the per-line escape rests on

packages/cli/src/commands/review/compose-review.test.ts

@@ -7342,6 +7342,18 @@ describe('composeReview — fixedFindings', () => {
       // An inline link's DESTINATION is raw characters, so a backtick in it
       // is not a delimiter either.
       ['see [x](a`b) <details>` here', 'see [x](a`b) &lt;details>` here'],
+      // A reference link's LABEL is not: MEASURED against cmark-gfm, a
+      // backtick inside `[a`b][c]`, `[x][a`b]`, `[a`b]` or a link
+      // reference definition's own label is an ordinary code-span
+      // delimiter, and the tag it pairs over is code. Ordinary text is
+      // what this pass already reads them as, so nothing special is
+      // needed — the rows are here because two audits reasoned the other
+      // way and neither could build a counterexample (#9940 review,
+      // rounds 11 and 12).
+      ['see [a`b][c] and <details>` here', 'see [a`b][c] and <details>` here'],
+      ['see [x][a`b] <details>` here', 'see [x][a`b] <details>` here'],
+      ['see [a`b] <details>` here', 'see [a`b] <details>` here'],
+      ['[a`b]: /u <details>` here', '[a`b]: /u <details>` here'],
       // …and the `_` rule reaches the LAST label too, not only the one
       // before it: `http://a.b_c` does not link, so the backticks pair and
       // the tag between them is live.
@@ -7413,6 +7425,183 @@ describe('composeReview — fixedFindings', () => {
     );
   });
 
+  it('every model-written channel reaches the escape as ONE line — the invariant the per-line escape rests on (#9940 review, round 12 reverse audit)', () => {
+    // `escapeTagOpeners` models CommonMark INLINE structure only. That is
+    // correct exactly while every channel folds its text to ONE LINE before
+    // posting it — and the fold is not one primitive. `collapseEntry` and
+    // `collapseToLine` carry the entry, disclosure and duplicate-drop
+    // legs; the downgrade reasons have their own `\s+` pass; the deferral
+    // leg is held TWICE, by a `collapse` pass and by `mdField`'s
+    // line-ending strip (lib/md-field.ts), and either alone holds it —
+    // MEASURED: break only one and the row is still folded; the bare CR
+    // reaches the POSTED LINE only with both broken. Two more legs are
+    // held twice: cannot-tell by `collapseEntry` AND `collapseToLine`, and
+    // the RELOCATION exit THREE times — `collapseToLine` at
+    // `toDeferredEntries` and again inside `boundDeferredLine`, then
+    // `mdField`. Breaking any two of the three still leaves it folded.
+    //
+    // So THREE rows cannot redden on a SINGLE-site regression — a property
+    // of the code, not a gap here — and the other five do redden on theirs
+    // (all eight measured). Do not read a double-held row as a sentinel
+    // for its own folds; it is here for the channel, not for them.
+    //
+    // The two rows asserted inert as a CODE SPAN get there differently.
+    // The deferral LIST line is `- ${mdField(entry)}` and never reaches
+    // `escapeTagOpeners` at all; the relocation line does reach it, and
+    // comes back unchanged because the escape reads `mdField`'s backtick
+    // span correctly. The fold is what all eight share, and it is what
+    // this case is about.
+    //
+    // Three producers push into `bodyCriticals` AFTER `ingestEntryList`
+    // has run, and that is how a channel gets missed: `scriptLintGate`'s
+    // push, which for a while did not fold; a Critical deferral's
+    // RELOCATION exit, which a `Suggestion` row never reaches (armed
+    // below); and the non-convergence Critical, which is this module's own
+    // fixed prose and carries no line break to fold. The gate's push is
+    // the one this case does not arm — it needs a report fixture beside
+    // the plan — and it is pinned by `folds its own entry`. A producer
+    // outside these still owes a case.
+    const multiline = [
+      'the retry helper\n<div class="w"> and `x\nnever clears it',
+      'the retry helper\r\n<div class="w"> and `x\r\nnever clears it',
+      'the retry helper\r<div class="w"> and `x\rnever clears it',
+    ];
+    const carries = (body: string, where: string, escapes = true) => {
+      // Split on `\n` ONLY, so a surviving bare `\r` stays INSIDE the
+      // matched line where the next assertion can see it. Splitting on the
+      // folds' own set here would consume the very break under test, and
+      // counting lines proves nothing on its own: the filter keeps the
+      // entry's first physical line whatever happened to the rest.
+      const lines = body
+        .split('\n')
+        .filter(
+          (l) =>
+            l.includes('retry helper') && !l.includes('qwen-review-ledger'),
+        );
+      expect([where, lines.length]).toEqual([where, 1]);
+      // The head and the TAIL of the entry on the SAME line, and no break
+      // left inside it. That is the fold, stated as the reader sees it: an
+      // LF that survives puts the tail on another line, a bare CR that
+      // survives stays in this one — and the per-line escape then decides
+      // each half on its own and can post a tag the renderer holds live
+      // (compose-review.ts records that happening). `&amp;lt;` was asserted
+      // here before and could not fail: the escape only ever writes
+      // `&lt;`, so the only body that trips it is one a model wrote with
+      // those characters already in it.
+      expect([where, lines[0]!.includes('never clears it')]).toEqual([
+        where,
+        true,
+      ]);
+      expect([where, /[\r\n]/.test(lines[0]!)]).toEqual([where, false]);
+      // Inert by ESCAPE on the prose channels; the deferral line is a code
+      // span, where the renderer already holds the tag and an escape would
+      // show the reader a literal `&lt;`.
+      expect([
+        where,
+        escapes
+          ? lines[0]!.includes('&lt;div class="w">')
+          : /^[^`]*`[^`]*<div class="w">[^`]*`[^`]*$/.test(lines[0]!),
+      ]).toEqual([where, true]);
+    };
+    for (const text of multiline) {
+      carries(
+        composeReview(base({ bodyCriticals: [text, 'blocker two'] })).body,
+        'bodyCriticals',
+      );
+      carries(
+        composeReview(base({ cannotTellCriticals: [text] })).body,
+        'cannotTellCriticals',
+      );
+      carries(
+        composeReview(
+          base({
+            suggestionsInline: 1,
+            suggestionsDroppedAsDuplicates: [text],
+          }),
+        ).body,
+        'suggestionsDroppedAsDuplicates',
+      );
+      carries(
+        composeReview(base({ uncoverableChunks: [text] })).body,
+        'uncoverableChunks',
+      );
+      carries(
+        composeReview(base({ unreviewedDimensions: [text] })).body,
+        'unreviewedDimensions',
+      );
+      carries(
+        composeReview(
+          base({
+            criticalsInline: 1,
+            presubmit: {
+              downgradeRequestChanges: true,
+              downgradeReasons: [text],
+            },
+          }),
+        ).body,
+        'downgradeReasons',
+      );
+      carries(
+        composeReview(
+          base({
+            suggestionsInline: 1,
+            deferredSuggestions: [
+              {
+                file: 'a.ts',
+                line: 1,
+                source: 'review',
+                severity: 'Suggestion',
+                title: text,
+              },
+            ],
+          }),
+        ).body,
+        'deferredSuggestions',
+        false,
+      );
+      // The deferral channel has a SECOND exit: a Critical relocates into
+      // `bodyCriticals` — another producer that joins after
+      // `ingestEntryList` has run, and the one this case would otherwise
+      // miss, since a `Suggestion` never reaches it.
+      carries(
+        composeReview(
+          base({
+            suggestionsInline: 1,
+            deferredSuggestions: [
+              {
+                file: 'a.ts',
+                line: 1,
+                source: 'review',
+                severity: 'Critical',
+                title: text,
+              },
+            ],
+          }),
+        ).body,
+        'deferredSuggestions (relocated)',
+        false,
+      );
+      // A ruling note's `by` does not fold — it REFUSES, because the note
+      // is one posted line and a folded `by` would misquote the author.
+      expect(() =>
+        composeReview(base({ fixedFindings: [{ id: 'R1-2', by: text }] })),
+      ).toThrow(/must be ONE non-empty line/);
+    }
+    // A quoted FENCE is refused rather than folded on the two Critical
+    // channels: folding it would join the delimiters onto one line and
+    // silently change what the author quoted.
+    const fenced =
+      'the retry helper\n```\n<div class="w">\n```\nnever clears it';
+    for (const input of [
+      { bodyCriticals: [fenced, 'blocker two'] },
+      { cannotTellCriticals: [fenced] },
+    ]) {
+      expect(() => composeReview(base(input))).toThrow(
+        /quotes a code fence its one-line render cannot carry/,
+      );
+    }
+  });
+
   it('a raw `<![CDATA[ … > … <details> … ]]>` does not fold the rest of the body away (#9940 review, round 10 reverse audit)', () => {
     // The unit rows pin the escape; this pins what the escape is FOR. The
     // HTML parser closes the bogus comment at the first `>`, so under the

packages/cli/src/commands/review/compose-review.ts

@@ -3673,11 +3673,20 @@ export function escapeTagOpeners(text: string): string {
   // each closing one hole and opening the next; a line is the unit the
   // rule below can decide exactly (#9940 review, round 31 reverse audit).
   //
-  // The channels are folded at four sites, not one: `ingestEntryList` for
-  // the entry channels, the `\s+` normalisation for downgrade reasons,
-  // `collapseEntry` for the `Not reviewed:` disclosures, and
-  // `scriptLintGate`'s own push — that last one joins `bodyCriticals`
-  // after `ingestEntryList` has run, so the shared fold never sees it.
+  // The channels are folded at several sites, not one: `collapseEntry` for
+  // the entry channels and the `Not reviewed:` disclosures (the `\r\n?`
+  // normalisation `ingestEntryList` does first folds nothing on its own),
+  // the `\s+` pass for downgrade reasons, `collapseToLine` for the
+  // duplicate-drop leg and — at `toDeferredEntries` and again through
+  // `boundDeferredLine` — for a Critical deferral's relocation exit, and
+  // `scriptLintGate`'s own push. The
+  // relocation exit and the gate push join `bodyCriticals` AFTER
+  // `ingestEntryList` has run, so the shared fold never sees them. (The
+  // deferral LIST line is folded by `mdField` and posts without coming
+  // here at all.) `every model-written channel reaches the escape as ONE
+  // line` in the tests drives every one of these EXCEPT the gate push,
+  // which needs a report fixture and is pinned by `folds its own entry`;
+  // it also says which legs are held more than once.
   // Per-line is NOT a conservative fallback — it is the model that the
   // fold makes correct. Handed a multi-line string anyway it differs from
   // the renderer in BOTH directions: it pairs backticks the renderer keeps

packages/cli/src/commands/review/lib/review-footer.test.ts

@@ -1078,9 +1078,26 @@ describe('the review footer and the regex that strips it', () => {
         'q <!--the auth check at line 40 is missing',
         'q <![CDATA[the auth check at line 40 is missing',
         'q <?the auth check at line 40 is missing',
+        // A `<?` whose span leaves prose behind it: what it hides ends at
+        // the first `>`, so the sentence after that still counts.
+        'q <?x?> the auth check at line 40 is missing',
+        '<?x> the auth check at line 40 is missing',
       ]) {
         expect([body, rendersAsNothing(body)]).toEqual([body, false]);
       }
+      // These arms are POSITION-BLIND on purpose, and `&nbsp;<?x>` is the
+      // shape where that shows: mid-line a `<?` with no `?>` forms nothing
+      // in the renderer, so its characters are VISIBLE and calling the
+      // body empty is wrong. The split that would fix it was measured and
+      // REVERTED — being right about it needs the block structure this
+      // module does not model, and the half-measure cost more bodies than
+      // it saved. Pinned so the boundary is a decision on the record
+      // rather than something the next audit rediscovers; the numbers are
+      // on `rendersAsNothing` (#9940 review, round 12 reverse audit).
+      for (const body of ['&nbsp;<?x>', '<?x>', '<?php echo 1 ?>']) {
+        expect([body, rendersAsNothing(body)]).toEqual([body, true]);
+      }
+      expect(rendersAsNothing('q <?x>')).toBe(false);
     });
 
     it('still counts real content wearing the same shapes', () => {

packages/cli/src/commands/review/lib/review-footer.ts

@@ -902,6 +902,48 @@ function linkRefDefLines(lines: string[], i: number): number {
  * toward the verdict, and re-promotes as an unanswerable blocker. This is a
  * judgment projection, not a sanitizer, so it is deliberately fence-blind: a
  * quotation of scaffolding is still not a finding.
+ *
+ * Two further blind spots are DECISIONS, not gaps, and both were measured
+ * against cmark-gfm and an HTML parser before being left alone (#9940
+ * review, round 12 reverse audit):
+ *
+ * FOLDS. A body whose prose sits inside a `<details>` renders as a
+ * collapsed triangle — one click from readable, not invisible — and an arm
+ * that deleted the fold's content would delete a quoted `` `<details>` ``
+ * too, which is the shape a review OF THIS FILE writes. That trade loses a
+ * real finding to catch a readable one, so a fold that hides an inline
+ * comment's whole text posts, and counts.
+ *
+ * BLOCK CONTEXT. The `<!` and `<?` arms hide the same span wherever the
+ * opener stands, though the renderer does not: mid-line a `<?` without its
+ * `?>` forms nothing and its characters are VISIBLE, while at a line's
+ * content start it is an HTML block. Splitting the two BY HAND was tried
+ * and REVERTED. Measured on a prose-weighted corpus of 30 000 bodies: the
+ * split alone traded 121 → 116 bodies wrongly called empty for 1426 → 1730
+ * wrongly called visible, and teaching it the container prefixes (so
+ * `><?…` reads as a block) then needs the container's EXTENT as well —
+ * without that, bodies wrongly called EMPTY went 121 → 3594. (A one-off
+ * corpus, not kept: the numbers are the shape of the trade, not a
+ * benchmark to re-run.)
+ *
+ * The parse is not what stops it, and neither is the parser. `BLOCK_PARSER`
+ * ABOVE hands every BLOCK token its line span, and this path already pays
+ * for a run of it: `stripReviewFooter` opens `canProjectFooterMarker` on a
+ * `<`, which every body these arms judge carries by construction, and
+ * reaches `scanLines`. So the parse is already in the bill — on the body
+ * shapes these arms fire on, adding one costs about what this leg already
+ * spends in `stripReviewFooter`, and the two move together across shapes
+ * (a directional measurement, not a benchmark: the absolute figures swing
+ * more than twentyfold with the body's structure).
+ *
+ * What stops it is the PROJECTION. `scanLines` flattens `token.map` into a
+ * per-line kind and returns `{line, kind, depth, content}` — the extent is
+ * discarded, and `depth` counts block-quote markers only, so a list item
+ * reads 0. Extent is the exact fact the numbers above say the split needs:
+ * a container prefix tells you an opener is at a line's content start, and
+ * only the container's END tells you how far the block it opens reaches.
+ * So the arms stay position-blind until something here surfaces that, and
+ * the price is the `&nbsp;<?x>` miscall pinned in the tests.
  */
 /**
  * The `<!` family and `<?`, taken as ONE left-to-right decision.