fix(review): revert GST-766's negation drop; collect keeps every label as before 8cef24ce, stated stays (GSTA-167 S1)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-11-01 14:25:19 +00:00
1 parent 43cdd9ad6c
commit fe177bc149
3 files changed
+16 -27

No files matched your search

@@ -0,0 +1,10 @@
**/ship's review collector keeps every severity label again, so a blocker named after "no CRITICAL issues except" is no longer lost.** v1.92.29.0 (GST-766) skipped any severity word that followed "no", "nothing", "none" and similar words, to stop a reviewer's closing line like "found nothing CRITICAL" from counting as a finding. That also threw away real blockers named in an exception clause: "No CRITICAL issues except the token leak in log.ts:3", "Nothing CRITICAL apart from the auth bypass (auth.ts:9)" and four more wordings collected nothing at all, so the gate never saw them. The collector now reads labels exactly as it did before v1.92.29.0. The reviewer's own Severity and Blocks fields are still recorded beside the label (`stated`), as in v1.92.29.0.
### Known
- **A reviewer's closing summary collects at its label again.** Lines like "found nothing CRITICAL" or "Neither finding is CRITICAL." collect as CRITICAL, the same as before v1.92.29.0. They are known phantoms for lead judgment, and a lost blocker costs more than an extra one.
### Tests
- `test/review-dispatch.test.ts`: the six exception-clause wordings collect CRITICAL for every reader. GST-766's test that a negated label collects nothing now checks that each of those lines keeps its label. GST-435's label-keeping probes (table rows, FIXABLE, negated subjects, closing summaries kept as known phantoms) are added with their HARN-15 and GEVA-77 fixtures under `test/fixtures/review-ledger/`.
### Docs
- `docs/review-policy.md` (collect) says a negated severity word still collects at its label.
+5 -6
View File
@@ -469,7 +469,7 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
3. `collect` hashes each reader's raw output and appends its findings to
`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 outside a negation (below), headings and table rows included; a top-level list item under a
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,
@@ -478,11 +478,10 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
adversarial voice's FIXABLE collects as CRITICAL (normalizer v2, GSTA-167). 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.
A label a negation word governs ("found nothing CRITICAL", "no new **High** or Critical issues", "No P0/P1 issues":
no, nothing, not, none, zero, without or never, optionally followed by new, remaining, other, further, additional, more
or at, through Markdown, and carried to labels joined by a comma, slash, or, nor or and) is no finding unless it opens a
field (`High:`, GST-766). A line whose labels are all negated closes the open finding and heads the next, so a finding
worded around a negated label still arrives through its Severity or Blocks field. Each prose finding's line also
A negated severity word still collects at its label ("found nothing CRITICAL", "No P0/P1 issues"), so a reader's
closing summary stays a known phantom that lead judgment can fold. GST-766 skipped negated labels and lost
real blockers named in an exception clause ("No CRITICAL issues except the token leak in log.ts:3"); GSTA-167
reverted that. Each prose finding's line also
records the Severity and Blocks its own text states as `stated`, beside the collected label and never in its place;
a classified finding that carries `stated` shows it in its deferred-review issue next to the label ("CRITICAL (the
reviewer wrote Severity: Low, Blocks: no)"). /ship's Step 9.4 does not copy `stated` into its findings file yet: that
+1 -21
View File
@@ -59,14 +59,6 @@ const stronger = (a: string, b: string) => ((rank(a) ?? 0) >= (rank(b) ?? 0) ? a
/** A prose voice's `Blocks: yes` (GSTA-167 normalizer v2): the finding is CRITICAL whatever its label; `Blocks: no` never lowers one. */
const BLOCKS_YES = /(?<![A-Za-z0-9])Blocks:?\**:?\s*\**\s*yes(?![A-Za-z0-9])/i;
const ALL_LABELS = new RegExp(ANY_LABEL.source, 'g');
/**
* A label a negation word governs ("found nothing CRITICAL", "no new **High** or Critical issues", "No P0/P1 issues") is no finding
* (GST-766), unless it opens a field ("High:"). Such a line still closes the open finding and heads the next, so a prose finding
* phrased around a negated label reaches merged.jsonl through its required Severity or Blocks field.
*/
const NEGATED = /(?<![A-Za-z0-9-])(?:no|nothing|not|none|(?<!\bby\s)zero|without|never)\s+(?:(?:new|remaining|other|further|additional|more|at)\s+)?$/i;
const NEGATION_LIST = /^\s*(?:,|\/|or|nor|and|,\s*(?:or|nor|and))\s*$/i;
const MARKUP = /[*_`[\]]/g;
/** A file:line locus ("auth.ts:9"): prose naming one after a finding's fields starts the next finding. */
const LOCUS = /[\w./-]+\.\w+:\d+/;
/** A line that is only a finding's `Severity` or `Blocks` field ("Severity: High", "- Blocks: yes", "Severity High"). */
@@ -105,18 +97,7 @@ export function parseFindings(reader: string, output: string): Array<{ label: st
let treeSeen = false;
const role = reader.split('/')[0];
const named = (l: string) => (role === 'adversarial-native' && l.toUpperCase() === 'FIXABLE' ? 'CRITICAL' : l);
const asserted = (t: string) => {
const all = [...t.matchAll(ALL_LABELS)];
let end = -1, negated = false;
return all.filter(m => {
const at = m.index!;
negated = !/^\s*:/.test(t.slice(at + m[0].length, at + m[0].length + 8).replace(MARKUP, ''))
&& (NEGATED.test(t.slice(Math.max(0, at - 60), at).replace(MARKUP, '')) || (negated && NEGATION_LIST.test(t.slice(end, at).replace(MARKUP, ''))));
end = at + m[0].length;
return !negated;
});
};
const strongest = (t: string) => asserted(t).map(m => named(m[1]!)).reduce<string | undefined>((a, b) => (a === undefined ? b : stronger(a, b)), undefined);
const strongest = (t: string) => [...t.matchAll(ALL_LABELS)].map(m => named(m[1]!)).reduce<string | undefined>((a, b) => (a === undefined ? b : stronger(a, b)), undefined);
const push = (label: string, raw: string, fields: string[] = []): Finding => { const f = { label, raw, fields: new Set(fields), list: LIST_ITEM.test(raw) }; out.push(f); return f; };
for (const line of output.replace(/\r\n/g, '\n').split('\n')) {
const t = line.trim();
@@ -151,7 +132,6 @@ export function parseFindings(reader: string, output: string): Array<{ label: st
if (typeof o.severity === 'string') { current = push(o.severity, t); continue; }
} catch { /* prose that starts with a brace */ }
}
if (!label && !blocks && !(section && listItem) && ANY_LABEL.test(t)) { current = null; head = t; continue; }
const detail = listItem && /^[-*+]\s/.test(t) && !locus && !label && !section && !!current && !current.list && !current.fields.size;
if (detail && !blocks) { current!.raw += ` | ${t}`; continue; }
const fresh = (listItem && !detail) || (!indented && !detail && (t.startsWith('**') || afterGap)) || (!!current?.fields.size && locus);