From de838e3cc161911f31bb976571ddba362b0bd38f Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 05:22:21 +0000 Subject: [PATCH 1/4] fix(pregate): literals skips a version-history row when the branch extended its table (GST-344) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gsta-145-literals-history.md | 8 +++++ docs/pregate.md | 8 ++++- lib/pregate/checks/literals.ts | 30 ++++++++++++++--- test/pregate-selection.test.ts | 42 ++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 changelog.d/gsta-145-literals-history.md diff --git a/changelog.d/gsta-145-literals-history.md b/changelog.d/gsta-145-literals-history.md new file mode 100644 index 000000000..3775be5f0 --- /dev/null +++ b/changelog.d/gsta-145-literals-history.md @@ -0,0 +1,8 @@ +**pregate `literals` no longer flags a version-history table that the branch extended.** A test that keeps one row per past version of a constant (`[45, ""]`, then `[46, ""]`) used to fail `literals` on every bump, because the old row still holds the old value. Truther turned the check down to a warning over it, which also hid real stale tests. + +### Fixed +- `lib/pregate/checks/literals.ts`: a row that starts with the old value is skipped when the same table holds a new-value row the branch added (the file at the merge base lacks it). The scan continues to the next line holding the old value, so `expect(PROOF_CHECKS).toBe(45)` below an extended table still fails. A table the branch did not extend, or one that already held the new row, still fails. +- docs/pregate.md describes the history-row rule and the table shapes it recognizes. + +### Tests +- `test/pregate-selection.test.ts`: an extended table, a newest-first table and a new file pass; an unextended table, a stale assertion below an extended table, an unextended table beside an extended one, and a table that already held the new row fail at the right line; a string-valued history passes; and an end-to-end `gstack-pregate --only literals` run fails before the table is extended and passes after. diff --git a/docs/pregate.md b/docs/pregate.md index 03de0f36a..5056e6064 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -169,7 +169,13 @@ Two stages with separate clocks, both printed: - `literals`: for every changed exported constant or named timeout/retry literal, the whole test tree (nightly- and Heavy-only files included) is searched for the old value; a consumer the branch did not update fails with - `file:line`. + `file:line`. A version-history row, a line that starts a bracketed row with + the old value (`[45, ""]`), is skipped when the same table (a run of + consecutive lines starting with `[`) also holds a row for the new value that + the branch added. A table the branch did not extend, a table that already + held the new row at the merge base, and every other line holding the old + value still fail. Rows in object form (`{ v: 45 }`) or spread over several + lines are not recognized as history rows. - `timing`: added or changed test lines that compare an elapsed wall-clock or CPU time against a bound, or sleep and then assert, print diff --git a/lib/pregate/checks/literals.ts b/lib/pregate/checks/literals.ts index 034de66db..2b77175bc 100644 --- a/lib/pregate/checks/literals.ts +++ b/lib/pregate/checks/literals.ts @@ -66,10 +66,20 @@ function unquote(v: string): string { return /^['"`]/.test(v) ? v.slice(1, -1) : * English word in half the test tree, and a one-character old value (`2`) * sits in every test, so that value needs the constant's name (a wrapper * version 2 → 3 flagged 575 consumers through the stem alone). + * + * A version-history row (`[45, ""]`, the old value first in a + * bracketed row) is skipped when the same table, a run of consecutive + * `[…` lines, holds a new-value row that `baseText` (the file at the merge + * base, null for a new file) lacks: the branch extended the table. A table it + * did not extend, or one that already held the new row, still counts, and the + * scan goes on past skipped rows to the first real consumer. */ -export function consumersOf(repoRoot: string, testFiles: readonly string[], change: ChangedLiteral): string[] { +export function consumersOf(repoRoot: string, testFiles: readonly string[], change: ChangedLiteral, baseText?: (rel: string) => string | null): string[] { const value = unquote(change.old); - const valueRe = /^-?\d/.test(value) ? new RegExp(`(? /^-?\d/.test(v) ? escape(v) : `['"\`]${escape(v)}['"\`]`; + const valueRe = /^-?\d/.test(value) ? new RegExp(`(? { + if (!oldRowRe.test(lines[i]!)) return false; + let a = i; while (a > 0 && /^\s*\[/.test(lines[a - 1]!)) a--; + let b = i; while (b < lines.length - 1 && /^\s*\[/.test(lines[b + 1]!)) b++; + if (!lines.slice(a, b + 1).some(l => newRowRe.test(l))) return false; + extended ??= baseText !== undefined && !newRowRe.test(baseText(rel) ?? ''); + return extended; + }; + for (let i = 0; i < lines.length; i++) if (valueRe.test(lines[i]!) && !skip(i)) { hits.push(`${rel}:${i + 1}`); break; } } return hits; } @@ -101,7 +120,10 @@ function run(ctx: PregateContext): CheckResult { const testFiles = listRepoFiles(ctx.repoRoot).filter(f => scansLiterals(f) && !f.startsWith('bin/')); const lines: string[] = []; for (const c of changes) { - const hits = consumersOf(ctx.repoRoot, testFiles, c); + const hits = consumersOf(ctx.repoRoot, testFiles, c, rel => { + const r = spawnSync('git', ['show', `${ctx.mergeBase}:${rel}`], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 64 * 1024 * 1024, env: ctx.env }); + return r.status === 0 ? r.stdout : null; + }); ctx.explain.push(`literals: ${c.file} ${c.name} ${c.old} → ${c.new}: ${hits.length} test file(s) still carry ${c.old}`); for (const h of hits) lines.push(`${h}: ${c.name} still ${c.old} (now ${c.new} in ${c.file})`); } diff --git a/test/pregate-selection.test.ts b/test/pregate-selection.test.ts index 489bb1982..05511ac70 100644 --- a/test/pregate-selection.test.ts +++ b/test/pregate-selection.test.ts @@ -223,6 +223,34 @@ describe('literals (pure)', () => { expect(consumersOf(root, all, { name: 'TIMEOUT_MS', old: '5000', new: '9000', file: 'lib/retry.js' })).toEqual(['test/retry.test.ts:1', 'test/retry-path.test.ts:2', 'test/retry-bare.test.ts:2']); fs.rmSync(root, { recursive: true, force: true }); }); + test('consumersOf: a history row is skipped only when this branch added the new-value row to the same table; other old-value lines still count (GST-344)', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-literals-history-')); + const nl = String.fromCharCode(10); + const head = "import { PROOF_CHECKS } from '../src/proof-checks.ts';"; + const rows = (...ns: number[]) => ns.map(n => ` [${n}, "digest-${n}"],`); + const files: Record = { + 'test/history.test.ts': [[head, 'const versions = [', ...rows(44, 45, 46), '];', 'expect(PROOF_CHECKS).toBe(46);'], [head, 'const versions = [', ...rows(44, 45), '];', 'expect(PROOF_CHECKS).toBe(45);']], + 'test/newest-first.test.ts': [[head, 'const versions = [', ...rows(46, 45, 44), '];'], [head, 'const versions = [', ...rows(45, 44), '];']], + 'test/unextended.test.ts': [[head, 'const versions = [', ...rows(44, 45), '];'], [head, 'const versions = [', ...rows(44, 45), '];']], + 'test/stale-later.test.ts': [[head, 'const versions = [', ...rows(44, 45, 46), '];', 'expect(PROOF_CHECKS).toBe(45);'], [head, 'const versions = [', ...rows(44, 45), '];', 'expect(PROOF_CHECKS).toBe(45);']], + 'test/two-tables.test.ts': [[head, 'const a = [', ...rows(44, 45), '];', 'const b = [', ...rows(45, 46), '];'], [head, 'const a = [', ...rows(44, 45), '];', 'const b = [', ...rows(45), '];']], + 'test/boundary.test.ts': [[head, 'test.each([', ...rows(45, 46), '])("%d", () => {});'], [head, 'test.each([', ...rows(45, 46), '])("%d", () => {});']], + 'test/new-file.test.ts': [[head, 'const versions = [', ...rows(45, 46), '];'], null], + }; + for (const [rel, [now]] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); fs.writeFileSync(path.join(root, rel), now.join(nl) + nl); } + const baseText = (rel: string) => files[rel]?.[1]?.join(nl) ?? null; + const lineOf = (rel: string, text: string) => `${rel}:${files[rel]![0].indexOf(text) + 1}`; + expect(consumersOf(root, Object.keys(files), { name: 'PROOF_CHECKS', old: '45', new: '46', file: 'src/proof-checks.ts' }, baseText).sort()).toEqual([ + lineOf('test/boundary.test.ts', ' [45, "digest-45"],'), + lineOf('test/stale-later.test.ts', 'expect(PROOF_CHECKS).toBe(45);'), + lineOf('test/two-tables.test.ts', ' [45, "digest-45"],'), + lineOf('test/unextended.test.ts', ' [45, "digest-45"],'), + ].sort()); + fs.writeFileSync(path.join(root, 'test/versions.test.ts'), ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", ' ["v4", \'b\'],', '];', ''].join(nl)); + const strings = (rel: string) => rel === 'test/versions.test.ts' ? ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", '];'].join(nl) : null; + expect(consumersOf(root, ['test/versions.test.ts'], { name: 'SCHEMA', old: "'v3'", new: "'v4'", file: 'src/schema.ts' }, strings)).toEqual([]); + fs.rmSync(root, { recursive: true, force: true }); + }); test('runner summaries: bun, node --test, vitest, jest, pytest, zero-run', () => { expect(parseSummary(' 12 pass\n 1 fail\nRan 13 tests across 2 files.')).toEqual({ ran: 13, failed: 1 }); expect(parseSummary('# tests 4\n# pass 4\n# fail 0')).toEqual({ ran: 4, failed: 0 }); @@ -369,6 +397,20 @@ describe('gstack-pregate (bin)', () => { expect(pregate(repo, ['--stage', 'preflight', '--only', 'literals']).status).toBe(EXIT.ok); }); + test('GST-344: a history table the branch extends passes literals through the merge-base read; one it does not extend fails', () => { + const history = (...ns: number[]) => ["const { MAX_RETRIES } = require('../lib/retry.js');", 'const versions = [', ...ns.map(n => ` [${n}, 'digest-${n}'],`), '];', ''].join('\n'); + const repo = fixture({ baseFiles: { 'test/history.test.js': history(2, 3) } }); + const bump = { 'lib/retry.js': "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n", 'test/retry.test.js': "const test = require('node:test');\nconst assert = require('node:assert');\nconst { MAX_RETRIES } = require('../lib/retry.js');\ntest('retries four times', () => assert.strictEqual(MAX_RETRIES, 4));\n" }; + commit(repo, bump); + const stale = pregate(repo, ['--stage', 'preflight', '--only', 'literals']); + expect(stale.status).toBe(EXIT.fail); + expect(stale.out).toContain('literals: test/history.test.js:4: MAX_RETRIES still 3 (now 4 in lib/retry.js)'); + commit(repo, { 'test/history.test.js': history(2, 3, 4) }); + const r = pregate(repo, ['--stage', 'preflight', '--only', 'literals']); + expect(r.out).toMatch(/^preflight {2}literals {2}pass /m); + expect(r.status).toBe(EXIT.ok); + }); + test('secrets: a credential in an added line fails and cannot be downgraded; a committed-then-removed line is clean', () => { const repo = fixture({ pregate: { ...BASE_PREGATE, warn: ['secrets'] } }); const bad = pregate(repo, ['--stage', 'preflight', '--only', 'secrets']); From 623cdd7f7e3e096907f31c965731ab0d370f0027 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 05:33:18 +0000 Subject: [PATCH 2/4] fix(pregate): literals reads the merge base through renames, treats an unreadable base as not extended, and recognizes quoted digit-leading history keys Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gsta-145-literals-history.md | 4 ++-- docs/pregate.md | 7 +++--- lib/pregate/checks/literals.ts | 28 ++++++++++++++---------- test/pregate-selection.test.ts | 9 ++++++++ 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/changelog.d/gsta-145-literals-history.md b/changelog.d/gsta-145-literals-history.md index 3775be5f0..372f06332 100644 --- a/changelog.d/gsta-145-literals-history.md +++ b/changelog.d/gsta-145-literals-history.md @@ -1,8 +1,8 @@ **pregate `literals` no longer flags a version-history table that the branch extended.** A test that keeps one row per past version of a constant (`[45, ""]`, then `[46, ""]`) used to fail `literals` on every bump, because the old row still holds the old value. Truther turned the check down to a warning over it, which also hid real stale tests. ### Fixed -- `lib/pregate/checks/literals.ts`: a row that starts with the old value is skipped when the same table holds a new-value row the branch added (the file at the merge base lacks it). The scan continues to the next line holding the old value, so `expect(PROOF_CHECKS).toBe(45)` below an extended table still fails. A table the branch did not extend, or one that already held the new row, still fails. +- `lib/pregate/checks/literals.ts`: a row that starts with the old value is skipped when the same table holds a new-value row the branch added (the file at the merge base lacks it). The scan continues to the next line holding the old value, so `expect(PROOF_CHECKS).toBe(45)` below an extended table still fails. A table the branch did not extend, a file that already held a new-value row at the merge base (read through a rename), and a merge base that cannot be read still fail. - docs/pregate.md describes the history-row rule and the table shapes it recognizes. ### Tests -- `test/pregate-selection.test.ts`: an extended table, a newest-first table and a new file pass; an unextended table, a stale assertion below an extended table, an unextended table beside an extended one, and a table that already held the new row fail at the right line; a string-valued history passes; and an end-to-end `gstack-pregate --only literals` run fails before the table is extended and passes after. +- `test/pregate-selection.test.ts`: an extended table, a newest-first table and a new file pass; an unextended table, a stale assertion below an extended table, an unextended table beside an extended one, and a table that already held the new row fail at the right line; string-valued and quoted semver histories pass; and an end-to-end `gstack-pregate --only literals` run fails before the table is extended, passes after, and still fails for a renamed file whose table already held the new row. diff --git a/docs/pregate.md b/docs/pregate.md index 5056e6064..f37455562 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -172,9 +172,10 @@ Two stages with separate clocks, both printed: `file:line`. A version-history row, a line that starts a bracketed row with the old value (`[45, ""]`), is skipped when the same table (a run of consecutive lines starting with `[`) also holds a row for the new value that - the branch added. A table the branch did not extend, a table that already - held the new row at the merge base, and every other line holding the old - value still fail. Rows in object form (`{ v: 45 }`) or spread over several + the branch added (the file at the merge base, read through a rename, has no + new-value row). A table the branch did not extend, a file that already held a + new-value row at the merge base, a merge base that cannot be read, and every + other line holding the old value still fail. Rows in object form (`{ v: 45 }`) or spread over several lines are not recognized as history rows. - `timing`: added or changed test lines that compare an elapsed wall-clock or diff --git a/lib/pregate/checks/literals.ts b/lib/pregate/checks/literals.ts index 2b77175bc..5c428c6ed 100644 --- a/lib/pregate/checks/literals.ts +++ b/lib/pregate/checks/literals.ts @@ -69,17 +69,18 @@ function unquote(v: string): string { return /^['"`]/.test(v) ? v.slice(1, -1) : * * A version-history row (`[45, ""]`, the old value first in a * bracketed row) is skipped when the same table, a run of consecutive - * `[…` lines, holds a new-value row that `baseText` (the file at the merge - * base, null for a new file) lacks: the branch extended the table. A table it - * did not extend, or one that already held the new row, still counts, and the - * scan goes on past skipped rows to the first real consumer. + * `[…` lines, holds a new-value row and `baseText` (the file at the merge + * base; null for a file the base lacks, undefined when it cannot be read) + * has no new-value row: the branch extended the table. A table it did not + * extend, a file that already held a new-value row, and an unreadable base + * still count, and the scan goes on past skipped rows to the first real consumer. */ -export function consumersOf(repoRoot: string, testFiles: readonly string[], change: ChangedLiteral, baseText?: (rel: string) => string | null): string[] { +export function consumersOf(repoRoot: string, testFiles: readonly string[], change: ChangedLiteral, baseText?: (rel: string) => string | null | undefined): string[] { const value = unquote(change.old); - const literal = (v: string) => /^-?\d/.test(v) ? escape(v) : `['"\`]${escape(v)}['"\`]`; - const valueRe = /^-?\d/.test(value) ? new RegExp(`(? new RegExp(`^\\s*\\[\\s*${/^['"`]/.test(raw) ? `['"\`]${escape(unquote(raw))}['"\`]` : escape(raw)}\\s*,`, 'm'); + const oldRowRe = row(change.old); + const newRowRe = row(change.new); const base = path.basename(change.file); const stem = base.replace(/\.[^.]+$/, ''); const moduleRe = new RegExp(`(?:(?:^|[/\\\\])${escape(stem)}|(? 0 && /^\s*\[/.test(lines[a - 1]!)) a--; let b = i; while (b < lines.length - 1 && /^\s*\[/.test(lines[b + 1]!)) b++; if (!lines.slice(a, b + 1).some(l => newRowRe.test(l))) return false; - extended ??= baseText !== undefined && !newRowRe.test(baseText(rel) ?? ''); + if (extended === undefined) { const b = baseText?.(rel); extended = b === null || (b !== undefined && !newRowRe.test(b)); } return extended; }; for (let i = 0; i < lines.length; i++) if (valueRe.test(lines[i]!) && !skip(i)) { hits.push(`${rel}:${i + 1}`); break; } @@ -121,8 +122,11 @@ function run(ctx: PregateContext): CheckResult { const lines: string[] = []; for (const c of changes) { const hits = consumersOf(ctx.repoRoot, testFiles, c, rel => { - const r = spawnSync('git', ['show', `${ctx.mergeBase}:${rel}`], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 64 * 1024 * 1024, env: ctx.env }); - return r.status === 0 ? r.stdout : null; + const from = ctx.touched.find(t => t.path === rel)?.renamedFrom ?? rel; + const r = spawnSync('git', ['show', `${ctx.mergeBase}:${from}`], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 64 * 1024 * 1024, env: ctx.env }); + if (r.status === 0) return r.stdout; + const ls = spawnSync('git', ['ls-tree', ctx.mergeBase, '--', from], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: 30_000, env: ctx.env }); + return ls.status === 0 && !ls.stdout.trim() ? null : undefined; }); ctx.explain.push(`literals: ${c.file} ${c.name} ${c.old} → ${c.new}: ${hits.length} test file(s) still carry ${c.old}`); for (const h of hits) lines.push(`${h}: ${c.name} still ${c.old} (now ${c.new} in ${c.file})`); diff --git a/test/pregate-selection.test.ts b/test/pregate-selection.test.ts index 05511ac70..027f587c4 100644 --- a/test/pregate-selection.test.ts +++ b/test/pregate-selection.test.ts @@ -249,6 +249,10 @@ describe('literals (pure)', () => { fs.writeFileSync(path.join(root, 'test/versions.test.ts'), ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", ' ["v4", \'b\'],', '];', ''].join(nl)); const strings = (rel: string) => rel === 'test/versions.test.ts' ? ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", '];'].join(nl) : null; expect(consumersOf(root, ['test/versions.test.ts'], { name: 'SCHEMA', old: "'v3'", new: "'v4'", file: 'src/schema.ts' }, strings)).toEqual([]); + expect(consumersOf(root, ['test/versions.test.ts'], { name: 'SCHEMA', old: "'v3'", new: "'v4'", file: 'src/schema.ts' }, () => undefined)).toEqual(['test/versions.test.ts:3']); + fs.writeFileSync(path.join(root, 'test/semver.test.ts'), ["import { RELEASE } from '../src/release.ts';", 'const history = [', " ['1.2.3', 'a'],", " ['1.2.4', 'b'],", '];', ''].join(nl)); + expect(consumersOf(root, ['test/semver.test.ts'], { name: 'RELEASE', old: "'1.2.3'", new: "'1.2.4'", file: 'src/release.ts' }, () => null)).toEqual([]); + expect(consumersOf(root, ['test/semver.test.ts'], { name: 'RELEASE', old: "'1.2.3'", new: "'1.2.4'", file: 'src/release.ts' }, () => " ['1.2.4', 'b'],")).toEqual(['test/semver.test.ts:3']); fs.rmSync(root, { recursive: true, force: true }); }); test('runner summaries: bun, node --test, vitest, jest, pytest, zero-run', () => { @@ -409,6 +413,11 @@ describe('gstack-pregate (bin)', () => { const r = pregate(repo, ['--stage', 'preflight', '--only', 'literals']); expect(r.out).toMatch(/^preflight {2}literals {2}pass /m); expect(r.status).toBe(EXIT.ok); + const renamed = fixture({ baseFiles: { 'test/history.test.js': history(2, 3, 4) } }); + commit(renamed, { ...bump, 'test/history.test.js': null, 'test/versions.test.js': history(2, 3, 4) }); + const held = pregate(renamed, ['--stage', 'preflight', '--only', 'literals']); + expect(held.status).toBe(EXIT.fail); + expect(held.out).toContain('literals: test/versions.test.js:4: MAX_RETRIES still 3 (now 4 in lib/retry.js)'); }); test('secrets: a credential in an added line fails and cannot be downgraded; a committed-then-removed line is clean', () => { From bcb1d7e05ae2fc4ca9a260bc65e979f51f5ebf5b Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 05:41:36 +0000 Subject: [PATCH 3/4] fix(pregate): literals exempts only a history row's first field, finds tables in one pass, and matches rows within one line Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gsta-145-literals-history.md | 4 ++-- docs/pregate.md | 4 ++-- lib/pregate/checks/literals.ts | 18 ++++++++++++------ test/pregate-selection.test.ts | 14 +++++++++----- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/changelog.d/gsta-145-literals-history.md b/changelog.d/gsta-145-literals-history.md index 372f06332..cf835926b 100644 --- a/changelog.d/gsta-145-literals-history.md +++ b/changelog.d/gsta-145-literals-history.md @@ -1,8 +1,8 @@ **pregate `literals` no longer flags a version-history table that the branch extended.** A test that keeps one row per past version of a constant (`[45, ""]`, then `[46, ""]`) used to fail `literals` on every bump, because the old row still holds the old value. Truther turned the check down to a warning over it, which also hid real stale tests. ### Fixed -- `lib/pregate/checks/literals.ts`: a row that starts with the old value is skipped when the same table holds a new-value row the branch added (the file at the merge base lacks it). The scan continues to the next line holding the old value, so `expect(PROOF_CHECKS).toBe(45)` below an extended table still fails. A table the branch did not extend, a file that already held a new-value row at the merge base (read through a rename), and a merge base that cannot be read still fail. +- `lib/pregate/checks/literals.ts`: a row that starts with the old value is skipped when the same table holds a new-value row the branch added (the file at the merge base lacks it). Only the row's first field is exempt, and the scan continues to the next line holding the old value, so `expect(PROOF_CHECKS).toBe(45)` below an extended table still fails. A table the branch did not extend, a file that already held a new-value row at the merge base (read through a rename git detects), and a merge base that cannot be read still fail. - docs/pregate.md describes the history-row rule and the table shapes it recognizes. ### Tests -- `test/pregate-selection.test.ts`: an extended table, a newest-first table and a new file pass; an unextended table, a stale assertion below an extended table, an unextended table beside an extended one, and a table that already held the new row fail at the right line; string-valued and quoted semver histories pass; and an end-to-end `gstack-pregate --only literals` run fails before the table is extended, passes after, and still fails for a renamed file whose table already held the new row. +- `test/pregate-selection.test.ts`: an extended table, a newest-first table and a new file pass; an unextended table, a stale assertion below an extended table, an unextended table beside an extended one, and a table that already held the new row fail at the right line; string-valued and quoted semver histories pass; a stale assertion on the same line as a history row fails; a 20,000-row table is checked in linear time; and an end-to-end `gstack-pregate --only literals` run fails before the table is extended, passes after, and still fails for a renamed file whose table already held the new row. diff --git a/docs/pregate.md b/docs/pregate.md index f37455562..5db84b139 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -172,8 +172,8 @@ Two stages with separate clocks, both printed: `file:line`. A version-history row, a line that starts a bracketed row with the old value (`[45, ""]`), is skipped when the same table (a run of consecutive lines starting with `[`) also holds a row for the new value that - the branch added (the file at the merge base, read through a rename, has no - new-value row). A table the branch did not extend, a file that already held a + the branch added (the file at the merge base, read through a rename git + detects, has no new-value row). A table the branch did not extend, a file that already held a new-value row at the merge base, a merge base that cannot be read, and every other line holding the old value still fail. Rows in object form (`{ v: 45 }`) or spread over several lines are not recognized as history rows. diff --git a/lib/pregate/checks/literals.ts b/lib/pregate/checks/literals.ts index 5c428c6ed..e6ce81021 100644 --- a/lib/pregate/checks/literals.ts +++ b/lib/pregate/checks/literals.ts @@ -78,7 +78,7 @@ function unquote(v: string): string { return /^['"`]/.test(v) ? v.slice(1, -1) : export function consumersOf(repoRoot: string, testFiles: readonly string[], change: ChangedLiteral, baseText?: (rel: string) => string | null | undefined): string[] { const value = unquote(change.old); const valueRe = /^-?\d/.test(value) ? new RegExp(`(? new RegExp(`^\\s*\\[\\s*${/^['"`]/.test(raw) ? `['"\`]${escape(unquote(raw))}['"\`]` : escape(raw)}\\s*,`, 'm'); + const row = (raw: string) => new RegExp(`^[ \\t]*\\[[ \\t]*${/^['"`]/.test(raw) ? `['"\`]${escape(unquote(raw))}['"\`]` : escape(raw)}[ \\t]*,`, 'm'); const oldRowRe = row(change.old); const newRowRe = row(change.new); const base = path.basename(change.file); @@ -92,13 +92,19 @@ export function consumersOf(repoRoot: string, testFiles: readonly string[], chan try { text = fs.readFileSync(path.join(repoRoot, rel), 'utf8'); } catch { continue; } if (!(nameRe.test(text) || (!nameRequired && moduleRe.test(text)))) continue; const lines = text.split('\n'); + const table: number[] = []; + const tableHasNew: boolean[] = []; + for (let i = 0; i < lines.length; i++) { + if (!/^[ \t]*\[/.test(lines[i]!)) { table.push(-1); continue; } + if (table[i - 1] === undefined || table[i - 1] === -1) tableHasNew.push(false); + table.push(tableHasNew.length - 1); + if (newRowRe.test(lines[i]!)) tableHasNew[tableHasNew.length - 1] = true; + } let extended: boolean | undefined; const skip = (i: number) => { - if (!oldRowRe.test(lines[i]!)) return false; - let a = i; while (a > 0 && /^\s*\[/.test(lines[a - 1]!)) a--; - let b = i; while (b < lines.length - 1 && /^\s*\[/.test(lines[b + 1]!)) b++; - if (!lines.slice(a, b + 1).some(l => newRowRe.test(l))) return false; - if (extended === undefined) { const b = baseText?.(rel); extended = b === null || (b !== undefined && !newRowRe.test(b)); } + const line = lines[i]!; + if (!oldRowRe.test(line) || !tableHasNew[table[i]!] || valueRe.test(line.replace(oldRowRe, '['))) return false; + if (extended === undefined) { const base = baseText?.(rel); extended = base === null || (base !== undefined && !newRowRe.test(base)); } return extended; }; for (let i = 0; i < lines.length; i++) if (valueRe.test(lines[i]!) && !skip(i)) { hits.push(`${rel}:${i + 1}`); break; } diff --git a/test/pregate-selection.test.ts b/test/pregate-selection.test.ts index 027f587c4..2a4eda364 100644 --- a/test/pregate-selection.test.ts +++ b/test/pregate-selection.test.ts @@ -227,7 +227,7 @@ describe('literals (pure)', () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-literals-history-')); const nl = String.fromCharCode(10); const head = "import { PROOF_CHECKS } from '../src/proof-checks.ts';"; - const rows = (...ns: number[]) => ns.map(n => ` [${n}, "digest-${n}"],`); + const rows = (...ns: number[]) => ns.map(n => ` [${n}, "sha${n}"],`); const files: Record = { 'test/history.test.ts': [[head, 'const versions = [', ...rows(44, 45, 46), '];', 'expect(PROOF_CHECKS).toBe(46);'], [head, 'const versions = [', ...rows(44, 45), '];', 'expect(PROOF_CHECKS).toBe(45);']], 'test/newest-first.test.ts': [[head, 'const versions = [', ...rows(46, 45, 44), '];'], [head, 'const versions = [', ...rows(45, 44), '];']], @@ -236,15 +236,17 @@ describe('literals (pure)', () => { 'test/two-tables.test.ts': [[head, 'const a = [', ...rows(44, 45), '];', 'const b = [', ...rows(45, 46), '];'], [head, 'const a = [', ...rows(44, 45), '];', 'const b = [', ...rows(45), '];']], 'test/boundary.test.ts': [[head, 'test.each([', ...rows(45, 46), '])("%d", () => {});'], [head, 'test.each([', ...rows(45, 46), '])("%d", () => {});']], 'test/new-file.test.ts': [[head, 'const versions = [', ...rows(45, 46), '];'], null], + 'test/same-line.test.ts': [[head, 'const versions = [', ' [45, "sha45"], expect(PROOF_CHECKS).toBe(45),', ' [46, "sha46"],', '];'], [head, 'const versions = [', ' [45, "sha45"], expect(PROOF_CHECKS).toBe(45),', '];']], }; for (const [rel, [now]] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); fs.writeFileSync(path.join(root, rel), now.join(nl) + nl); } const baseText = (rel: string) => files[rel]?.[1]?.join(nl) ?? null; const lineOf = (rel: string, text: string) => `${rel}:${files[rel]![0].indexOf(text) + 1}`; expect(consumersOf(root, Object.keys(files), { name: 'PROOF_CHECKS', old: '45', new: '46', file: 'src/proof-checks.ts' }, baseText).sort()).toEqual([ - lineOf('test/boundary.test.ts', ' [45, "digest-45"],'), + lineOf('test/boundary.test.ts', ' [45, "sha45"],'), + lineOf('test/same-line.test.ts', ' [45, "sha45"], expect(PROOF_CHECKS).toBe(45),'), lineOf('test/stale-later.test.ts', 'expect(PROOF_CHECKS).toBe(45);'), - lineOf('test/two-tables.test.ts', ' [45, "digest-45"],'), - lineOf('test/unextended.test.ts', ' [45, "digest-45"],'), + lineOf('test/two-tables.test.ts', ' [45, "sha45"],'), + lineOf('test/unextended.test.ts', ' [45, "sha45"],'), ].sort()); fs.writeFileSync(path.join(root, 'test/versions.test.ts'), ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", ' ["v4", \'b\'],', '];', ''].join(nl)); const strings = (rel: string) => rel === 'test/versions.test.ts' ? ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", '];'].join(nl) : null; @@ -253,6 +255,8 @@ describe('literals (pure)', () => { fs.writeFileSync(path.join(root, 'test/semver.test.ts'), ["import { RELEASE } from '../src/release.ts';", 'const history = [', " ['1.2.3', 'a'],", " ['1.2.4', 'b'],", '];', ''].join(nl)); expect(consumersOf(root, ['test/semver.test.ts'], { name: 'RELEASE', old: "'1.2.3'", new: "'1.2.4'", file: 'src/release.ts' }, () => null)).toEqual([]); expect(consumersOf(root, ['test/semver.test.ts'], { name: 'RELEASE', old: "'1.2.3'", new: "'1.2.4'", file: 'src/release.ts' }, () => " ['1.2.4', 'b'],")).toEqual(['test/semver.test.ts:3']); + fs.writeFileSync(path.join(root, 'test/big.test.ts'), [head, 'const versions = [', ...Array.from({ length: 20_000 }, () => ' [45, "x"],'), ' [46, "y"],', '];', ...Array.from({ length: 20_000 }, () => ''), ''].join(nl)); + expect(consumersOf(root, ['test/big.test.ts'], { name: 'PROOF_CHECKS', old: '45', new: '46', file: 'src/proof-checks.ts' }, () => ['', ...Array.from({ length: 20_000 }, () => '')].join(nl))).toEqual([]); fs.rmSync(root, { recursive: true, force: true }); }); test('runner summaries: bun, node --test, vitest, jest, pytest, zero-run', () => { @@ -402,7 +406,7 @@ describe('gstack-pregate (bin)', () => { }); test('GST-344: a history table the branch extends passes literals through the merge-base read; one it does not extend fails', () => { - const history = (...ns: number[]) => ["const { MAX_RETRIES } = require('../lib/retry.js');", 'const versions = [', ...ns.map(n => ` [${n}, 'digest-${n}'],`), '];', ''].join('\n'); + const history = (...ns: number[]) => ["const { MAX_RETRIES } = require('../lib/retry.js');", 'const versions = [', ...ns.map(n => ` [${n}, 'sha${n}'],`), '];', ''].join('\n'); const repo = fixture({ baseFiles: { 'test/history.test.js': history(2, 3) } }); const bump = { 'lib/retry.js': "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n", 'test/retry.test.js': "const test = require('node:test');\nconst assert = require('node:assert');\nconst { MAX_RETRIES } = require('../lib/retry.js');\ntest('retries four times', () => assert.strictEqual(MAX_RETRIES, 4));\n" }; commit(repo, bump); From a0edd745a77f356a260fe33ce3cd6c8d071f4703 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 05:46:18 +0000 Subject: [PATCH 4/4] fix(pregate): literals checks only the text after a history row's closing bracket; restore digest fixtures Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gsta-145-literals-history.md | 2 +- docs/pregate.md | 3 ++- lib/pregate/checks/literals.ts | 2 +- test/pregate-selection.test.ts | 14 +++++++------- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/changelog.d/gsta-145-literals-history.md b/changelog.d/gsta-145-literals-history.md index cf835926b..4b28ac91b 100644 --- a/changelog.d/gsta-145-literals-history.md +++ b/changelog.d/gsta-145-literals-history.md @@ -1,7 +1,7 @@ **pregate `literals` no longer flags a version-history table that the branch extended.** A test that keeps one row per past version of a constant (`[45, ""]`, then `[46, ""]`) used to fail `literals` on every bump, because the old row still holds the old value. Truther turned the check down to a warning over it, which also hid real stale tests. ### Fixed -- `lib/pregate/checks/literals.ts`: a row that starts with the old value is skipped when the same table holds a new-value row the branch added (the file at the merge base lacks it). Only the row's first field is exempt, and the scan continues to the next line holding the old value, so `expect(PROOF_CHECKS).toBe(45)` below an extended table still fails. A table the branch did not extend, a file that already held a new-value row at the merge base (read through a rename git detects), and a merge base that cannot be read still fail. +- `lib/pregate/checks/literals.ts`: a row that starts with the old value is skipped when the same table holds a new-value row the branch added (the file at the merge base lacks it). Only the row itself is exempt (the old value after its closing bracket on the same line still fails), and the scan continues to the next line holding the old value, so `expect(PROOF_CHECKS).toBe(45)` below an extended table still fails. A table the branch did not extend, a file that already held a new-value row at the merge base (read through a rename git detects), and a merge base that cannot be read still fail. - docs/pregate.md describes the history-row rule and the table shapes it recognizes. ### Tests diff --git a/docs/pregate.md b/docs/pregate.md index 5db84b139..944c3c921 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -175,7 +175,8 @@ Two stages with separate clocks, both printed: the branch added (the file at the merge base, read through a rename git detects, has no new-value row). A table the branch did not extend, a file that already held a new-value row at the merge base, a merge base that cannot be read, and every - other line holding the old value still fail. Rows in object form (`{ v: 45 }`) or spread over several + other line holding the old value still fail. Only the row itself is exempt: + the old value after its closing bracket on the same line still fails. Rows in object form (`{ v: 45 }`) or spread over several lines are not recognized as history rows. - `timing`: added or changed test lines that compare an elapsed wall-clock or diff --git a/lib/pregate/checks/literals.ts b/lib/pregate/checks/literals.ts index e6ce81021..a769626e4 100644 --- a/lib/pregate/checks/literals.ts +++ b/lib/pregate/checks/literals.ts @@ -103,7 +103,7 @@ export function consumersOf(repoRoot: string, testFiles: readonly string[], chan let extended: boolean | undefined; const skip = (i: number) => { const line = lines[i]!; - if (!oldRowRe.test(line) || !tableHasNew[table[i]!] || valueRe.test(line.replace(oldRowRe, '['))) return false; + if (!oldRowRe.test(line) || !tableHasNew[table[i]!] || valueRe.test(line.slice(line.indexOf(']') + 1))) return false; if (extended === undefined) { const base = baseText?.(rel); extended = base === null || (base !== undefined && !newRowRe.test(base)); } return extended; }; diff --git a/test/pregate-selection.test.ts b/test/pregate-selection.test.ts index 2a4eda364..74f47756e 100644 --- a/test/pregate-selection.test.ts +++ b/test/pregate-selection.test.ts @@ -227,7 +227,7 @@ describe('literals (pure)', () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-literals-history-')); const nl = String.fromCharCode(10); const head = "import { PROOF_CHECKS } from '../src/proof-checks.ts';"; - const rows = (...ns: number[]) => ns.map(n => ` [${n}, "sha${n}"],`); + const rows = (...ns: number[]) => ns.map(n => ` [${n}, "digest-${n}"],`); const files: Record = { 'test/history.test.ts': [[head, 'const versions = [', ...rows(44, 45, 46), '];', 'expect(PROOF_CHECKS).toBe(46);'], [head, 'const versions = [', ...rows(44, 45), '];', 'expect(PROOF_CHECKS).toBe(45);']], 'test/newest-first.test.ts': [[head, 'const versions = [', ...rows(46, 45, 44), '];'], [head, 'const versions = [', ...rows(45, 44), '];']], @@ -236,17 +236,17 @@ describe('literals (pure)', () => { 'test/two-tables.test.ts': [[head, 'const a = [', ...rows(44, 45), '];', 'const b = [', ...rows(45, 46), '];'], [head, 'const a = [', ...rows(44, 45), '];', 'const b = [', ...rows(45), '];']], 'test/boundary.test.ts': [[head, 'test.each([', ...rows(45, 46), '])("%d", () => {});'], [head, 'test.each([', ...rows(45, 46), '])("%d", () => {});']], 'test/new-file.test.ts': [[head, 'const versions = [', ...rows(45, 46), '];'], null], - 'test/same-line.test.ts': [[head, 'const versions = [', ' [45, "sha45"], expect(PROOF_CHECKS).toBe(45),', ' [46, "sha46"],', '];'], [head, 'const versions = [', ' [45, "sha45"], expect(PROOF_CHECKS).toBe(45),', '];']], + 'test/same-line.test.ts': [[head, 'const versions = [', ' [45, "digest-45"], expect(PROOF_CHECKS).toBe(45),', ' [46, "digest-46"],', '];'], [head, 'const versions = [', ' [45, "digest-45"], expect(PROOF_CHECKS).toBe(45),', '];']], }; for (const [rel, [now]] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); fs.writeFileSync(path.join(root, rel), now.join(nl) + nl); } const baseText = (rel: string) => files[rel]?.[1]?.join(nl) ?? null; const lineOf = (rel: string, text: string) => `${rel}:${files[rel]![0].indexOf(text) + 1}`; expect(consumersOf(root, Object.keys(files), { name: 'PROOF_CHECKS', old: '45', new: '46', file: 'src/proof-checks.ts' }, baseText).sort()).toEqual([ - lineOf('test/boundary.test.ts', ' [45, "sha45"],'), - lineOf('test/same-line.test.ts', ' [45, "sha45"], expect(PROOF_CHECKS).toBe(45),'), + lineOf('test/boundary.test.ts', ' [45, "digest-45"],'), + lineOf('test/same-line.test.ts', ' [45, "digest-45"], expect(PROOF_CHECKS).toBe(45),'), lineOf('test/stale-later.test.ts', 'expect(PROOF_CHECKS).toBe(45);'), - lineOf('test/two-tables.test.ts', ' [45, "sha45"],'), - lineOf('test/unextended.test.ts', ' [45, "sha45"],'), + lineOf('test/two-tables.test.ts', ' [45, "digest-45"],'), + lineOf('test/unextended.test.ts', ' [45, "digest-45"],'), ].sort()); fs.writeFileSync(path.join(root, 'test/versions.test.ts'), ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", ' ["v4", \'b\'],', '];', ''].join(nl)); const strings = (rel: string) => rel === 'test/versions.test.ts' ? ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", '];'].join(nl) : null; @@ -406,7 +406,7 @@ describe('gstack-pregate (bin)', () => { }); test('GST-344: a history table the branch extends passes literals through the merge-base read; one it does not extend fails', () => { - const history = (...ns: number[]) => ["const { MAX_RETRIES } = require('../lib/retry.js');", 'const versions = [', ...ns.map(n => ` [${n}, 'sha${n}'],`), '];', ''].join('\n'); + const history = (...ns: number[]) => ["const { MAX_RETRIES } = require('../lib/retry.js');", 'const versions = [', ...ns.map(n => ` [${n}, 'digest-${n}'],`), '];', ''].join('\n'); const repo = fixture({ baseFiles: { 'test/history.test.js': history(2, 3) } }); const bump = { 'lib/retry.js': "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n", 'test/retry.test.js': "const test = require('node:test');\nconst assert = require('node:assert');\nconst { MAX_RETRIES } = require('../lib/retry.js');\ntest('retries four times', () => assert.strictEqual(MAX_RETRIES, 4));\n" }; commit(repo, bump);