Keep a numbered review finding's closing paragraph on that finding after a nested list (GST-2174) (#4237)

* Keep a numbered review finding's closing paragraph on that finding after a nested list (GST-2174)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* Keep the nested-list join from lowering a separate finding or a CRITICAL item

The join that keeps a numbered native finding's closing paragraph now ends after that first paragraph, skips bold-led margin paragraphs and verdict lines, applies only to the native voice's items, and never lowers an item that names CRITICAL or P0. Each of these let a separate finding's Severity and Blocks: no lower a FIXABLE blocker to Low (GST-2174).

Review-Finding: review/specialist-batched#1 reader=testing
Review-Finding: review/red-team#2 reader=red-team
Review-Finding: review/red-team#3 reader=red-team
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* Describe the numbered-finding nested-list rule and add its changelog entry

docs/review-policy.md now says how the collector keeps a native numbered finding's first closing paragraph after a nested list (GST-2174).

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

changelog.d/capy-keep-a-numbered-review.md

@@ -0,0 +1 @@
+- **/ship's review gate no longer blocks on a Low finding just because the native reviewer wrote it as a numbered item with a nested list.** When the native reviewer wrote `1. **FIXABLE. …**`, then a sub-list, then a closing paragraph with `Severity: Low. Blocks: no.`, the collector split that paragraph into a second record. The numbered item was left with no fields, so its FIXABLE label counted as CRITICAL and stopped /ship until an owner ruled on it (6 findings collected as 9 in one alphaclaw run). The collector now keeps that first closing paragraph on its finding. The join stays narrow so it can't hide a real blocker. It applies only to the native reviewer's items, ends after the first closing paragraph, never takes in a bold-led paragraph, a verdict line, a list item, a heading or a file:line, and never lowers an item that names CRITICAL or P0.

docs/review-policy.md

@@ -913,7 +913,11 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
    never split off as a second record (GST-1031); the line right under the heading that carries those fields, no other
    label and no file:line joins it at the stronger label (not under a heading that names CRITICAL or P0 itself), the way an unlocated paragraph after a blank line already does. An unnumbered, unlabeled bullet with no file:line
    under a paragraph finding whose Severity or Blocks line has not come yet is that finding's detail, so the field line after it
-   lands on the finding; a numbered item is its own finding.
+   lands on the finding; a numbered item is its own finding. A native adversarial numbered list item (`1. **FIXABLE. …**`)
+   with no fields of its own keeps, after an unlabeled nested list, its indented lines and the first paragraph after a blank
+   line until its fields come, so that paragraph's `Severity` and `Blocks` bind to the item instead of forming a second record;
+   an item naming CRITICAL or P0 is never lowered, and a list item, heading, lead label, file:line, bold paragraph opener,
+   verdict line or any later paragraph starts the next finding (ALPH-93).
    A reader's own no-defect closing sentence is no finding at the label it denies (GST-435): a prose line with no
    list, heading, bold or lead label, no `Severity` or `Blocks` field, no FIXABLE or INVESTIGATE, no file:line or path
    and no exception clause, with no line above it in its paragraph that runs on (no closing period) or names an

lib/review-ledger.ts

@@ -179,7 +179,11 @@ export interface MergedLine { id: string; reader: string; label: string; stated?
  * under it, located or listed or bold, until its fields come (GST-1031), so its Severity and Blocks never split off as a second record;
  * the line right under it that carries those fields, no other label and no file:line joins it at the stronger label, as a paragraph after
  * a blank line does (a located line there is another finding), unless the heading itself names CRITICAL or P0, whose blocker another
- * subject's Blocks: no must not lift. A prose line that only names a severity in passing is a summary (summaryLine).
+ * subject's Blocks: no must not lift. The native voice's numbered list item ("1. **FIXABLE. …**") with no fields of its own owns, after an
+ * unlabelled nested list, its indented lines and the first paragraph after a blank line (ALPH-93), until its fields come; their Severity
+ * and Blocks bind to it as a field line's do, except that an item naming CRITICAL or P0 is never lowered. A list item, heading, lead label,
+ * file:line, bold margin lead, verdict line or any later margin paragraph there starts the next finding.
+ * A prose line that only names a severity in passing is a summary (summaryLine).
  * A native FIXABLE finding that states `Blocks: no` and one Severity field collects at that field on every class (GST-1031 ruling A);
  * one with no Blocks: no, no Severity field or only a FIXABLE field stays CRITICAL. fixableRaises false (a code or docs review with no
  * risk, error, history or floor reason, GST-987) also drops FIXABLE's raise from label headings and leads.
@@ -188,7 +192,7 @@ export interface MergedLine { id: string; reader: string; label: string; stated?
  * nothing; elsewhere it collects as above. A line led by a bold, unlabelled lead ("**1. Coverage:** …") is read for a closing summary after it.
  */
 function parseLabelled(reader: string, output: string, fixableRaises: boolean): Array<{ label: string; raw: string; shaped: boolean }> {
-  type Finding = { label: string; raw: string; fields: Set<string>; list: boolean; lead?: string; fixable?: boolean; heading?: boolean; section?: boolean; context?: string };
+  type Finding = { label: string; raw: string; fields: Set<string>; list: boolean; lead?: string; fixable?: boolean; heading?: boolean; item?: boolean; nested?: boolean; closed?: boolean; section?: boolean; context?: string };
   const out: Finding[] = [];
   let section: string | null = null;
   let current: Finding | null = null;
@@ -269,6 +273,21 @@ function parseLabelled(reader: string, output: string, fixableRaises: boolean):
       else if (label) current.label = stronger(current.label, label);
       continue;
     }
+    if (current?.item && !current.fields.size && !HEADING.test(t) && !NUMBERED_HEAD.test(t) && !t.startsWith('{')) {
+      if (indented && LIST_ITEM.test(t) && !label && !blocks) { current.nested = true; continue; }
+      if (current.nested && (indented || (afterGap && !t.startsWith('**'))) && !(afterGap && current.closed) && !LIST_ITEM.test(t) && !LEAD_LABEL.test(t)
+        && !LOCUS.test(t) && !quietSummary(t)) {
+        const pinned = [...current.raw.split(' | ')[0]!.matchAll(ALL_LABELS)].some(m => rank(m[1]) === 4);
+        current.raw += ` | ${t}`;
+        if (afterGap) current.closed = true;
+        if (blocks) current.label = 'CRITICAL';
+        else if (field !== undefined && !critical(current.raw) && !pinned) current.label = strongestOf([current.lead, label])!;
+        else if (label) current.label = stronger(current.label, label);
+        if (field !== undefined) current.fields.add('severity');
+        if (t.match(BLOCKS_FIELD)) current.fields.add('blocks');
+        continue;
+      }
+    }
     const fresh = (listItem && !detail) || (!indented && !detail && (t.startsWith('**') || afterGap)) || (!!current?.fields.size && locus);
     if (fresh) { current = null; head = t; }
     const inherited = section && listItem ? section : undefined;
@@ -279,6 +298,7 @@ function parseLabelled(reader: string, output: string, fixableRaises: boolean):
       current.section = !!inherited;
       if (continued) current.context = before;
       current.heading = !indented && NUMBERED_HEAD.test(t);
+      current.item = role === 'adversarial-native' && !indented && /^\d+[.)]\s/.test(t) && field === undefined && !t.match(BLOCKS_FIELD);
       if (!indented) head = t;
       continue;
     }

test/review-native-nested-list.test.ts

@@ -0,0 +1,83 @@
+import { describe, expect, test } from 'bun:test';
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import { collect, ledgerBlockers, parseFindings, statesNoBlocks } from '../lib/review-ledger';
+
+const native = (output: string, raises = true) => parseFindings('adversarial-native', output, raises);
+/** A numbered finding with a bold lead, a body line, an indented sub-list, a blank line and a closing paragraph holding its fields. */
+const nested = (n: number, tag: string, close: string, indent = '   ') => [
+  `${n}. **${tag}. The retry queue drops a job on shutdown** (\`queue.ts\` \`drainJobs\`). The drain loop exits before the last flush:`,
+  '   - The in-flight job is never written back.',
+  '   - The next boot sees no record of it.',
+  '',
+  `${indent}${close}`,
+].join('\n');
+const LOW = 'So a job queued in the last second is lost, which is rare and already retried upstream. Severity: Low. Blocks: no.';
+const reply = (findings: string[]) => ['**Findings**', '', ...findings.flatMap(f => [f, '']), 'Recommendation: Ship after one fix round on the drain loop.'].join('\n');
+const blockers = (output: string) => {
+  const d = fs.mkdtempSync(path.join(os.tmpdir(), 'nested-list-'));
+  const [ledger, merged, out] = ['l.jsonl', 'm.jsonl', 'out.txt'].map(f => path.join(d, f)) as [string, string, string];
+  fs.writeFileSync(out, output);
+  const c = collect(ledger, 'adversarial-native', output, merged, out);
+  const result = { labels: c.lines.map(l => l.label), blockers: ledgerBlockers(ledger).length };
+  fs.rmSync(d, { recursive: true, force: true });
+  return result;
+};
+
+describe('a numbered native finding keeps its closing paragraph after a nested list', () => {
+  // Value: protects=a native finding written as "N. **FIXABLE. ...**", a body line, an indented sub-list, a blank line, then a paragraph
+  //   carrying "Severity: Low. Blocks: no." stays one record, so its Severity binds to it and GST-1031 ruling A collects it at Low;
+  //   fails_when=the paragraph splits off as a second record and the heading's FIXABLE collects CRITICAL as a blocker; why_new=ALPH-93
+  //   (alphaclaw#387): 6 native findings collected as 9 and a Low FIXABLE blocked /ship until an owner ruling; seam=parseFindings,
+  //   collect and ledgerBlockers; no_claim=other list-continuation spellings
+  test('the closing paragraph joins its finding, indented or at the margin', () => {
+    for (const indent of ['   ', ''])
+      for (const raises of [true, false]) {
+        const found = native(reply([nested(1, 'FIXABLE', LOW, indent), nested(2, 'INVESTIGATE', 'The fix is a final flush before exit. Severity: Medium. Blocks: no.', indent)]), raises);
+        expect([indent, raises, found.map(f => f.label)]).toEqual([indent, raises, ['Low', 'Medium']]);
+        expect(found[0]!.raw.startsWith('1. **FIXABLE.')).toBe(true);
+        expect(found[0]!.raw.endsWith(`| ${LOW}`)).toBe(true);
+        expect(found.map(f => statesNoBlocks(f.raw))).toEqual([true, true]);
+      }
+    expect(blockers(reply([nested(1, 'FIXABLE', LOW)]))).toEqual({ labels: ['Low'], blockers: 0 });
+  });
+
+  // Value: protects=the fail-closed holds around that join: a FIXABLE whose paragraph has no Severity field, or says Blocks: yes, still
+  //   collects CRITICAL and counts as a blocker; a later numbered item, a lead-labelled paragraph or a located paragraph stays its own
+  //   record; a heading naming CRITICAL is never lowered by the paragraph; fails_when=any of these merges, lowers or stops blocking;
+  //   why_new=ALPH-93; seam=parseFindings, collect and ledgerBlockers; no_claim=that each control is a real finding
+  test('the join never weakens a blocker or swallows a separate finding', () => {
+    expect(native(reply([nested(1, 'FIXABLE', 'So a job queued in the last second is lost. Blocks: no.')])).map(f => f.label)).toEqual(['CRITICAL']);
+    expect(native(reply([nested(1, 'FIXABLE', 'So a job queued in the last second is lost.')])).map(f => f.label)).toEqual(['CRITICAL']);
+    for (const close of ['So jobs are lost on every deploy. Severity: Low. Blocks: yes.', 'So jobs are lost. Blocks: yes. Severity: Low.'])
+      expect(blockers(reply([nested(1, 'FIXABLE', close)]))).toEqual({ labels: ['CRITICAL'], blockers: 1 });
+    expect(native(reply([nested(1, 'CRITICAL', LOW)]), false).map(f => f.label)).toEqual(['CRITICAL']);
+    expect(native(reply([nested(1, 'FIXABLE', LOW), nested(2, 'FIXABLE', 'So restarts leak a lock file. Severity: Medium. Blocks: yes.')])).map(f => f.label))
+      .toEqual(['Low', 'CRITICAL']);
+    for (const later of ['**High:** the lock file in lock.ts leaks on restart. Blocks: no.', 'Separately, lock.ts:12 leaks the lock file on restart. Severity: High. Blocks: no.']) {
+      const found = native([nested(1, 'FIXABLE', LOW), '', later].join('\n'));
+      expect([later, found.map(f => f.label)]).toEqual([later, ['Low', 'High']]);
+    }
+    const located = native([nested(1, 'FIXABLE', 'Also lock.ts:12 leaks on restart. Severity: Low. Blocks: no.', ''), ''].join('\n'));
+    expect(located.map(f => f.label)).toEqual(['CRITICAL', 'Low']);
+  });
+
+  // Value: protects=the join's bounds: a bold-led margin paragraph or a second paragraph after the closing one is its own finding, so its
+  //   Severity and Blocks: no never lower the FIXABLE item; an item naming CRITICAL is never lowered, and other readers' numbered items
+  //   never join; fails_when=any of these collects as one Low record and the blocker leaves the ledger; why_new=the GST-2174 /ship review
+  //   (specialist and Red Team probes); seam=parseFindings and ledgerBlockers; no_claim=that each control is a real finding
+  test('the join ends at the first closing paragraph and never lowers a CRITICAL item', () => {
+    for (const [close, later] of [['So jobs are lost.', '**The lock file leaks on restart.** A second process cannot start. Severity: Low. Blocks: no.'],
+      ['So a queued job is lost.', 'The lock file leaks on restart. Severity: Low. Blocks: no.']]) {
+      const output = [nested(1, 'FIXABLE', close!), '', later].join('\n');
+      expect([later, native(output).map(f => f.label)]).toEqual([later, ['CRITICAL', 'Low']]);
+      expect(blockers(output).blockers).toBe(1);
+    }
+    const critical = ['1. Webhook routes skip signature validation, a CRITICAL gap.', '   - first detail', '   - second detail', '', 'The log line omits the locale. Severity: Low.'].join('\n');
+    for (const reader of ['outside-voice', 'red-team', 'specialist/testing'])
+      expect([reader, parseFindings(reader, critical).map(f => f.label)]).toEqual([reader, ['CRITICAL', 'Low']]);
+    expect(native(critical).map(f => f.label)[0]).toBe('CRITICAL');
+    expect(blockers(critical).blockers).toBe(1);
+  });
+});