mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
fix(receipt): --check-prose skips the required Review-loop exception line (GST-977)
write --review-loop-exception requires the PR body to carry the exception line word for word, and read --check-prose read the owner reason in it as counter claims ("outside 1 and 2 skip detector" as 2 skip against 70, gstack-evals #139). The prose check now skips the line that matches the receipt exception exactly, as it skips the rendered Approvals block.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
69c14b17d5
commit
8a7d853dfd
5 files changed
+55
-4
No files matched your search
@@ -0,0 +1,7 @@
|
||||
**`gstack-ship-receipt read --check-prose` no longer reads the required Review-loop exception line as counter claims (GST-977).** `write --review-loop-exception` requires the PR body to carry the whole `Review-loop exception: …` line word for word, but the prose check read that line as prose. On gstack-evals #139 the owner's reason "outside 1 and 2 skip detector" was read as a `2 skip` claim against the receipt's 70 skips, so no body could pass both checks.
|
||||
|
||||
### Fixed
|
||||
- **`lib/ship-receipt-prose.ts`:** the line that matches the receipt's own exception exactly (the same test `write` applies) is skipped, like the rendered `## Approvals` block. The same words anywhere else, or a line for a different exception, are still checked.
|
||||
|
||||
### Tests
|
||||
- `test/ship-receipt-prose.test.ts`: the #139 exception text passes `--check-prose` beside a matching `70 skip` claim, and a wrong number elsewhere still fails. On main the same body failed with `gate.unit.skip body=2 receipt=70`.
|
||||
+5
-1
@@ -1222,7 +1222,11 @@ block `gstack-approvals` renders (its heading, the Date | Asked | Exact words
|
||||
table and the `Source: gstack-approvals record` line) quotes the owner's
|
||||
words, not this PR's numbers, so it is skipped: "one Step 11 pass" in that table
|
||||
is not a `pass` claim. Text you write under the heading after that block, and
|
||||
a `## Approvals` example inside a code fence, are still checked. Data PRs (evals,
|
||||
a `## Approvals` example inside a code fence, are still checked. The
|
||||
`Review-loop exception: …` line the receipt's own exception requires is
|
||||
skipped the same way (GST-977): it quotes the owner's reason, so "outside 1 and
|
||||
2 skip detector" is not a `2 skip` claim. Only a line that matches the rendered
|
||||
line exactly is skipped; the same words anywhere else are still checked. Data PRs (evals,
|
||||
measurements, sweeps) run it before READY so the body's numbers are the run's.
|
||||
|
||||
<a id="review-scope"></a>
|
||||
|
||||
@@ -5766,7 +5766,8 @@ body that names a counter the receipt records (`passes=3` against
|
||||
against `gate.passed: 41`) with a different value. The receipt is written from
|
||||
the run; the prose was typed, so a reader of the body would learn a number the
|
||||
run did not produce. The `PROSE:` line names each field with both values. The
|
||||
`## Approvals` table `gstack-approvals` renders is never read as a claim.
|
||||
`## Approvals` table `gstack-approvals` renders, and the `Review-loop exception: …`
|
||||
line the receipt's exception requires, are never read as claims.
|
||||
|
||||
**Fix.** Correct the body to the receipt's number. When the receipt is the
|
||||
stale one (the run was repeated), rerun `/ship` so the receipt is rewritten
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
*/
|
||||
import { APPROVALS_FOOTER_END, APPROVALS_TABLE_HEADER } from './approval-record';
|
||||
import { RECEIPT_FENCE } from './ship-receipt';
|
||||
import { parseReviewLoopException, reviewLoopExceptionLine } from './ship-receipt-depth';
|
||||
|
||||
export interface ProseClaim { field: string; prose: number; receipt: number }
|
||||
export interface ProseCheck { checked: number; mismatches: ProseClaim[] }
|
||||
@@ -65,9 +66,23 @@ function withoutApprovalsBlocks(body: string): string {
|
||||
return out.join('');
|
||||
}
|
||||
|
||||
/**
|
||||
* The body without the `Review-loop exception: …` line the receipt's own exception requires (GST-977): write and the
|
||||
* receipt check want it word for word, and it quotes the owner's reason (`outside 1 and 2 skip detector`), not this
|
||||
* PR's counters. Only a line whose trimmed text is that rendered line, the test write applies, is left out; the same
|
||||
* words anywhere else stay prose. A marker line takes its place so the text on either side never joins into a claim.
|
||||
*/
|
||||
function withoutExceptionLine(body: string, exception: unknown): string {
|
||||
if (exception === undefined) return body;
|
||||
const parsed = parseReviewLoopException(exception);
|
||||
if (!parsed.ok) return body;
|
||||
const line = reviewLoopExceptionLine(parsed.exception);
|
||||
return body.split(/(?<=\n)/).map(l => (l.trim() === line ? '(review-loop exception line)\n' : l)).join('');
|
||||
}
|
||||
|
||||
/**
|
||||
* Every `<name>=<n>`, `<name>: <n>` and `<n> <name>` in the prose (the body
|
||||
* without its receipt block and its rendered Approvals block), where `<name>` is a counter's leaf key with `_`
|
||||
* without its receipt block, its rendered Approvals block and its review-loop exception line), where `<name>` is a counter's leaf key with `_`
|
||||
* matched as `_`, space or `-` and not part of a longer word (`3 pass-through`
|
||||
* is not a `pass` claim), compared with the receipt. A leaf key shared by
|
||||
* several counters (`pass` under every gate lane, `passed` under gate and ci)
|
||||
@@ -77,7 +92,7 @@ function withoutApprovalsBlocks(body: string): string {
|
||||
* `<n> <name>` claim about the next key (#3513).
|
||||
*/
|
||||
export function checkReceiptProse(receipt: Record<string, unknown>, body: string): ProseCheck {
|
||||
const prose = withoutApprovalsBlocks(body.replace(BLOCK_RE, ''));
|
||||
const prose = withoutExceptionLine(withoutApprovalsBlocks(body.replace(BLOCK_RE, '')), receipt.review_loop_exception);
|
||||
const byLeaf = new Map<string, Array<{ field: string; value: number }>>();
|
||||
for (const [field, value] of receiptCounters(receipt)) {
|
||||
const leaf = field.split('.').at(-1)!;
|
||||
|
||||
@@ -118,6 +118,30 @@ describe('the rendered Approvals block is not prose', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('the Review-loop exception line (GST-977, gstack-evals #139)', () => {
|
||||
const excepted = {
|
||||
...receipt,
|
||||
review_loop: { ...receipt.review_loop, fix_checks: 1 },
|
||||
gate: { status: 'green', run: '139', unit: { pass: 1181, skip: 70 } },
|
||||
review_loop_exception: { accepted_by: 'Garry', date: '2026-11-01', via: 'GEVA-2', reason: 'outside 1 and 2 skip detector', fix_check: { head: 'b'.repeat(40), findings: 0 } },
|
||||
};
|
||||
const line = 'Review-loop exception: accepted by Garry 2026-11-01 via GEVA-2: outside 1 and 2 skip detector (fix-check at bbbbbbbbbbbb, 0 findings)';
|
||||
|
||||
// Value: protects=a body that carries the exception line write requires can also pass --check-prose; fails_when=the
|
||||
// owner's reason is read as counter claims ("2 skip" against 70); seam=lib/ship-receipt-prose.ts checkReceiptProse
|
||||
test('the exact line the receipt\'s exception requires is not prose; the rest of the body still is', () => {
|
||||
expect(checkReceiptProse(excepted, `${line}\n\nGate: 70 skip.\n`)).toEqual({ checked: 1, mismatches: [] });
|
||||
expect(checkReceiptProse(excepted, ` ${line}\r\nGate: 69 skip.`).mismatches).toEqual([{ field: 'gate.unit.skip', prose: 69, receipt: 70 }]);
|
||||
});
|
||||
|
||||
test('the same words anywhere else, or a line for a different exception, stay prose', () => {
|
||||
const twoSkip = [{ field: 'gate.unit.skip', prose: 2, receipt: 70 }];
|
||||
expect(checkReceiptProse(excepted, `Note: ${line}`).mismatches).toEqual(twoSkip);
|
||||
expect(checkReceiptProse(excepted, line.replace('GEVA-2', 'GEVA-3')).mismatches).toEqual(twoSkip);
|
||||
expect(checkReceiptProse({ ...excepted, review_loop_exception: undefined }, line).mismatches).toEqual(twoSkip);
|
||||
});
|
||||
});
|
||||
|
||||
describe('gstack-ship-receipt read --check-prose', () => {
|
||||
const run = (body: string) => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'receipt-prose-'));
|
||||
|
||||
Reference in new issue
Block a user