fix(review): collect keeps every prose blocker and one id per finding

/ship pass 2 (PR #3660): an unlabeled line carrying an inline Blocks: yes collects as
CRITICAL again; Severity/Blocks continuation needs a colon, stops at a list item, heading,
bold paragraph opener or a repeated field, and never swallows a top-level bullet or its
section label; the native voice's FIXABLE collects as CRITICAL under its role with or
without a dispatch name; the consumer scan adds --no-color.

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

docs/review-policy.md

@@ -411,12 +411,15 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
    prompt and runs `sent` on what it passed: a hash that differs from the rendered
    one is `BRIEF_MISMATCH`, and that reader's review does not count.
 3. `collect` hashes each reader's raw output and appends its findings to
-   `merged.jsonl` with the label exactly as written (a JSON line's `severity`, or a
-   prose line's Critical, High, Medium, Low, P0-P3, FIXABLE or INVESTIGATE wherever it
-   stands, headings and table rows included; a top-level list item under a label-only
-   heading takes that label). A prose voice's `Blocks: yes` collects as CRITICAL: on
-   its own line, or on the finding just before it; with no finding there it is one
-   (normalizer v2, GSTA-167). A phantom finding costs a fold; a lost one costs the gate.
+   `merged.jsonl` with the label as written (a JSON line's `severity`, or the most
+   severe of a prose line's Critical, High, Medium, Low, P0-P3, FIXABLE or INVESTIGATE
+   wherever they stand, headings and table rows included; a top-level list item under a
+   label-only heading takes the stronger of the two). A `Severity:` or `Blocks:` line
+   continues the finding above it until a list item, heading, bold paragraph opener or a
+   second line for the same field starts another: `Severity` raises its label,
+   `Blocks: yes` makes it CRITICAL and `Blocks: no` never lowers it; an inline
+   `Blocks: yes` is CRITICAL too, and with no finding above it is one. The native
+   adversarial voice's FIXABLE collects as CRITICAL (normalizer v2, GSTA-167). A phantom finding costs a fold; a lost one costs the gate.
 4. Lead judgment writes `judged.jsonl`; `judge` refuses a set that drops a merged
    finding, lowers its severity, folds it into a less severe duplicate, or adds or
    promotes one without a reason (`JUDGMENT_REJECTED`). A merged file missing any id

lib/review-collect.ts

@@ -114,7 +114,7 @@ export function scanConsumers(root: string, paths: readonly string[], revs: read
     const runs = [...revs.map(rev => ({ at: rev, args: [rev, '--'] })), ...(live ? [{ at: '', args: ['--untracked', '--'] }] : [])];
     const next = new Set<string>();
     for (const run of runs) {
-      const r = git(root, ['grep', '-I', '-F', '-o', '--null', '--no-line-number', '--no-column', ...patterns, ...run.args]);
+      const r = git(root, ['grep', '-I', '-F', '-o', '--null', '--no-line-number', '--no-column', '--no-color', ...patterns, ...run.args]);
       if (r.status !== 0 && r.status !== 1) { errors.push({ code: 'consumer_scan_failed', detail: oneLine(r.err || `git grep exited ${r.status}`), input: `git grep -F <data basenames> ${run.at || '--untracked'}`, repair: 'run the git grep by hand and fix what git reports' }); return 'all'; }
       for (const line of r.out.split('\n')) {
         const parts = line.split('\0');

lib/review-ledger.ts

@@ -8,8 +8,9 @@
  *   judgment  each lead-judgment change: an added or promoted finding, with its reason
  *   judge     the verdict on the parent's judged set (ok, or the dropped and downgraded ids)
  *
- * Findings reach merged.jsonl only through collect, with each voice's label exactly as written,
- * except that a prose voice's `Blocks: yes` collects as CRITICAL (GSTA-167 normalizer v2).
+ * Findings reach merged.jsonl only through collect, with each voice's label as written (a prose line's most severe one,
+ * raised by the Severity line below it), except that a prose voice's `Blocks: yes` and the native voice's FIXABLE collect
+ * as CRITICAL (GSTA-167 normalizer v2).
  * The parent's judged set may add or promote a finding and may fold a duplicate into one at least
  * as severe; it never drops or downgrades one. verify replays every check from the ledger alone.
  */
@@ -60,53 +61,70 @@ const ALL_LABELS = new RegExp(ANY_LABEL.source, 'g');
 /** The most severe label on a line, so an explicit `Severity: High` outranks a stray `Low-level` or `[P2]` before it. */
 const strongest = (t: string) => [...t.matchAll(ALL_LABELS)].map(m => m[1]!).reduce<string | undefined>((a, b) => (a === undefined ? b : stronger(a, b)), undefined);
 /** A line that only continues the finding above it: its `Severity:` or `Blocks:` field. */
-const META = /^(?:[-*+]\s+)?\**\s*(?:Severity|Blocks)\b/i;
+const META = /^(?:[-*+]\s+)?\**\s*(Severity|Blocks)\**\s*:/i;
 
 export interface MergedLine { id: string; reader: string; label: string; raw: string; output_sha256: string }
 
 /**
  * One reader's findings as written: a JSON object per line (severity kept as written); else each prose line carrying a severity or
  * FIXABLE/INVESTIGATE label, at the most severe label on it, and each top-level list item under a label-only heading (the stronger
- * of the two labels). A `Severity:` or `Blocks:` line continues the finding above it in the same list item: `Severity` raises its
- * label, `Blocks: yes` makes it CRITICAL (normalizer v2), `Blocks: no` never lowers it. With no finding above, such a line is its
- * own finding carrying the item's first line.
+ * of the two labels). A `Severity:` or `Blocks:` line that is not itself a top-level list item continues the finding above it,
+ * until a list item, heading, bold paragraph opener or a second line for the same field starts another: `Severity` raises its
+ * label, `Blocks: yes` makes it CRITICAL, `Blocks: no` never lowers it. With no finding above, such a line is its own finding
+ * carrying the paragraph's first line. The native voice's FIXABLE collects as CRITICAL, since a Severity line must not bury it;
+ * other labels stay as written for lead judgment.
  */
 export function parseFindings(reader: string, output: string): Array<{ label: string; raw: string }> {
-  const out: Array<{ label: string; raw: string }> = [];
+  type Finding = { label: string; raw: string; fields: Set<string> };
+  const out: Finding[] = [];
   let section: string | null = null;
-  let current: { label: string; raw: string } | null = null;
+  let current: Finding | null = null;
   let head = '';
+  const role = reader.split('/')[0];
+  const named = (l: string | undefined) => (role === 'adversarial-native' && l?.toUpperCase() === 'FIXABLE' ? 'CRITICAL' : l);
+  const push = (label: string, raw: string, fields: string[] = []): Finding => { const f = { label, raw, fields: new Set(fields) }; out.push(f); return f; };
   for (const line of output.replace(/\r\n/g, '\n').split('\n')) {
     const t = line.trim();
     if (!t) continue;
     if (/^NO FINDINGS\.?$/.test(t) || /^Recommendation:/i.test(t)) { current = null; continue; }
     const indented = line !== line.trimStart();
-    const label = strongest(t);
+    const label = named(strongest(t));
     const blocks = BLOCKS_YES.test(t);
-    if (META.test(t)) {
-      if (current) {
+    const listItem = !indented && LIST_ITEM.test(t);
+    const meta = listItem ? null : META.exec(t);
+    if (meta) {
+      const field = meta[1]!.toLowerCase();
+      if (current && !current.fields.has(field)) {
+        current.fields.add(field);
         if (blocks) current.label = 'CRITICAL';
         else if (label) current.label = stronger(current.label, label);
         current.raw += ` | ${t}`;
-      } else if (blocks || label) out.push(current = { label: blocks ? 'CRITICAL' : label!, raw: head ? `${head} | ${t}` : t });
+      } else if (blocks || label) current = push(blocks ? 'CRITICAL' : label!, head ? `${head} | ${t}` : t, [field]);
+      else current = null;
       continue;
     }
     const heading = LABEL_HEADING.exec(t);
-    if (heading) { section = heading[1]!; current = null; head = ''; continue; }
+    if (heading) { section = named(heading[1])!; current = null; head = ''; continue; }
     if (!indented && HEADING.test(t)) { section = null; current = null; }
     if (t.startsWith('{')) {
       try {
         const o = JSON.parse(t) as Record<string, unknown>;
-        if (typeof o.severity === 'string') { out.push(current = { label: o.severity, raw: t }); continue; }
+        if (typeof o.severity === 'string') { current = push(o.severity, t); continue; }
       } catch { /* prose that starts with a brace */ }
     }
-    const listItem = !indented && LIST_ITEM.test(t);
     const inherited = section && listItem ? section : undefined;
     const found = label && inherited ? stronger(inherited, label) : label ?? inherited;
-    if (found) { out.push(current = { label: blocks ? 'CRITICAL' : found, raw: t }); if (!indented) head = t; continue; }
-    if (listItem) { current = null; head = t; }
+    if (found) { current = push(blocks ? 'CRITICAL' : found, t); if (!indented) head = t; continue; }
+    if (blocks && !t.startsWith('{')) {
+      if (current && !listItem) { current.label = 'CRITICAL'; current.raw += ` | ${t}`; continue; }
+      current = push('CRITICAL', !listItem && head ? `${head} | ${t}` : t);
+      if (listItem) head = t;
+      continue;
+    }
+    if (listItem || (!indented && t.startsWith('**'))) { current = null; head = t; }
+    else if (!indented) head = t;
   }
-  return out;
+  return out.map(({ label, raw }) => ({ label, raw }));
 }
 
 /** collect: hash the raw output, append its findings to merged.jsonl and record both. */

test/prose-voice-floor.test.ts

@@ -50,9 +50,11 @@ describe('normalizer v2 as collected: Blocks: yes is CRITICAL, Blocks: no never
   // Value: protects=the rule round 5's G1 was scored under, on the path production runs (gstack-review-dispatch collect);
   //   fails_when=Blocks: yes collects below CRITICAL or Blocks: no lowers a label; seam=parseFindings
   test.each([
-    ['FIXABLE: race in a.ts:3\nSeverity: Medium\nBlocks: no', 'Medium'],
+    ['FIXABLE: race in a.ts:3\nSeverity: Medium\nBlocks: no', 'CRITICAL'],
     ['INVESTIGATE: odd lock in b.ts:9\nSeverity: Low\nBlocks: yes', 'CRITICAL'],
     ['INVESTIGATE: odd lock in b.ts:9\nBlocks: no', 'INVESTIGATE'],
+  ] as const)('native %j -> %s', (output, want) => expect(parseFindings('adversarial-native', output).map(g => g.label)).toEqual([want]));
+  test.each([
     ['1. Drops the last row (q.ts:7). Severity: Critical. Blocks: no', 'Critical'],
     ['1. Cache evicts sessions (c.ts:4)\n   Severity: High', 'High'],
     ['1. Retry storm (r.ts:2)\n   Severity: Medium\n   Blocks: yes', 'CRITICAL'],

test/review-collect.test.ts

@@ -154,10 +154,10 @@ describe('GSTA-167 part 4a: which data files code names (scanConsumers)', () =>
   // Value: protects=the scan reads git grep the same way under any user grep config, and a failed grep keeps every data file
   //   code; fails_when=grep.lineNumber/grep.column shift the NUL fields so no consumer matches, or a grep failure drops data to
   //   docs silently; seam=git grep output and exit status
-  test('grep.lineNumber and grep.column in git config do not hide a consumer', () => {
+  test('grep.lineNumber, grep.column and color.grep in git config do not change which files count as consumers', () => {
     const r = repo({ 'src/load.ts': "export const rows = () => require('./data/rows.json');\n" });
-    r.git('config', 'grep.lineNumber', 'true'); r.git('config', 'grep.column', 'true');
-    const head = r.commit({ 'data/rows.json': rows });
+    r.git('config', 'grep.lineNumber', 'true'); r.git('config', 'grep.column', 'true'); r.git('config', 'color.grep', 'always');
+    const head = r.commit({ 'data/rows.json': rows, 'docs/notes.md': 'see rows.json and orphan.json\n', 'data/orphan.json': rows });
     expect([collectCommit(r.dir, r.baseSha, head).consumed, collectLive(r.dir, 'main').consumed]).toEqual([['data/rows.json'], ['data/rows.json']]);
   });
   test('a git grep failure keeps every data file code and records consumer_scan_failed', () => {

test/review-dispatch.test.ts

@@ -246,7 +246,6 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
     ].join('\n');
     const got = parseFindings('outside-voice', out);
     expect(got.map(g => g.label)).toEqual(['CRITICAL', 'CRITICAL', 'High', 'CRITICAL']);
-    expect(parseFindings('outside-voice', '1. Low-level retry loop in queue.ts:78 drops writes. Severity: Critical. Blocks: no\n- [P2] cache.ts:40 evicts live sessions. Severity: High. Blocks: no').map(g => g.label)).toEqual(['Critical', 'High']);
     expect(got[1]!.raw).toBe('2. INVESTIGATE: lock order in pool.ts:12 | Blocks: yes');
     expect(got[2]!.raw).toBe('3. FIXABLE: stale cache key (cache.ts:4) | Severity: High | Blocks: no');
     expect(got[3]!.raw).toBe('4. Token never expires (auth.ts:9) | **Blocks:** yes');
@@ -267,6 +266,42 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
     expect(prose.map(p => [p.label, p.raw])).toEqual([['CRITICAL', '1. FIXABLE: race in a.ts:3 | Severity: Low | Blocks: yes'], ['Medium', '2. Unlabeled item about b.ts:9 | Severity: Medium']]);
   });
 
+  // Value: protects=a prose blocker whose only marker is an inline Blocks: yes still reaches merged.jsonl as CRITICAL (#3660 pass 2
+  //   Testing finding); fails_when=an unlabeled line carrying Blocks: yes collects nothing; seam=parseFindings
+  test.each([
+    ['1. Token never expires (auth.ts:9). Blocks: yes', '1. Token never expires (auth.ts:9). Blocks: yes'],
+    ['Token never expires in auth.ts:9 — Blocks: yes', 'Token never expires in auth.ts:9 — Blocks: yes'],
+    ['1. Token never expires (auth.ts:9)\nFix: check exp **Blocks:** yes', '1. Token never expires (auth.ts:9) | Fix: check exp **Blocks:** yes'],
+  ])('an inline Blocks: yes with no label is one CRITICAL finding: %j', (output, raw) => {
+    expect(parseFindings('outside-voice', output)).toEqual([{ label: 'CRITICAL', raw }]);
+  });
+
+  // Value: protects=a top-level `- Severity:` bullet is its own finding and keeps its section's floor (#3660 pass 2 outside voice);
+  //   fails_when=two such bullets merge into one id or a Critical heading's item collects as Medium; seam=parseFindings
+  test('top-level Severity bullets stay separate findings and keep the section label', () => {
+    const got = parseFindings('outside-voice', ['## Critical findings', '- Severity: Medium — missing owner check at auth.ts:14', '- Severity: Low — token logged at log.ts:3'].join('\n'));
+    expect(got.map(g => g.label)).toEqual(['Critical', 'Critical']);
+  });
+  // Value: protects=one finding per merged id however a voice lays findings out (#3660 pass 2 native adversarial); fails_when=
+  //   paragraph findings or a sentence starting with "Blocks" fold into the finding above, or a numbered or quoted Blocks line
+  //   collects nothing; seam=parseFindings
+  test('paragraph findings stay separate, Blocks needs a colon to continue a finding, and a bare Blocks line is its own finding', () => {
+    const bold = parseFindings('outside-voice', ['**Finding 1.** a.ts:3 drops rows', 'Severity: Low', 'Blocks: no', '**Finding 2.** b.ts:4 leaks', 'Severity: High', 'Blocks: yes'].join('\n'));
+    expect(bold.map(b => [b.label, b.raw])).toEqual([['Low', '**Finding 1.** a.ts:3 drops rows | Severity: Low | Blocks: no'], ['CRITICAL', '**Finding 2.** b.ts:4 leaks | Severity: High | Blocks: yes']]);
+    const plain = parseFindings('outside-voice', ['a.ts:3 drops rows', 'Severity: High', 'Blocks: yes', 'b.ts:4 leaks', 'Severity: High', 'Blocks: yes'].join('\n'));
+    expect(plain.map(p => p.raw)).toEqual(['a.ts:3 drops rows | Severity: High | Blocks: yes', 'b.ts:4 leaks | Severity: High | Blocks: yes']);
+    expect(parseFindings('outside-voice', ['1. Medium: cache key drift (c.ts:4)', '   Blocks the event loop on every miss - High'].join('\n')).map(f => f.label)).toEqual(['Medium', 'High']);
+    expect(parseFindings('outside-voice', '2. Blocks: yes').map(f => f.label)).toEqual(['CRITICAL']);
+    expect(parseFindings('outside-voice', '> Blocks: yes').map(f => f.label)).toEqual(['CRITICAL']);
+  });
+  test('the native voice\'s FIXABLE collects as CRITICAL under its role with or without a dispatch name', () => {
+    for (const reader of ['adversarial-native', 'adversarial-native/adversarial']) expect(parseFindings(reader, 'FIXABLE: race in a.ts:3\nSeverity: Medium\nBlocks: no').map(f => f.label)).toEqual(['CRITICAL']);
+    expect(parseFindings('outside-voice', 'FIXABLE: race in a.ts:3\nSeverity: Medium').map(f => f.label)).toEqual(['Medium']);
+  });
+  test('a non-prefix Blocks: yes under a labeled finding raises it to CRITICAL', () => {
+    expect(parseFindings('outside-voice', '1. Retry loses writes (queue.ts:78). Severity: Medium\n   Fix: reject the batch. Blocks: yes')).toEqual([{ label: 'CRITICAL', raw: '1. Retry loses writes (queue.ts:78). Severity: Medium | Fix: reject the batch. Blocks: yes' }]);
+  });
+
   test('verify on a ledger that kept no prompts says so and exits 3', () => {
     const d = tmp();
     const ledger = path.join(d, 'ledger.jsonl');