From 568cb9dca6735c6a5d1b181b3f2f7cdca3f81302 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 12:06:32 +0000 Subject: [PATCH] fix(pregate): literals reports every removed value when the name still has one, so a delete-plus-edit keeps the edited site's old value (GST-1211 review) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gst-1211-pregate-false-positives.md | 2 +- lib/pregate/checks/literals.ts | 3 ++- test/pregate-selection.test.ts | 3 +++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/changelog.d/gst-1211-pregate-false-positives.md b/changelog.d/gst-1211-pregate-false-positives.md index 0ee87b79b..6e4cee14f 100644 --- a/changelog.d/gst-1211-pregate-false-positives.md +++ b/changelog.d/gst-1211-pregate-false-positives.md @@ -2,7 +2,7 @@ ### Fixed - **`strays`:** the scratch-name check matched three prefixes the docs never listed (`temp`, `debug-` and `probe`), so real files like `probe.py` and `temporal-graph-shipped.test.ts` failed preflight. It now matches only the documented names: `zz-*`, `tmp*`, `scratch*`, plus editor leftovers (`*.orig`, `*.rej`, `*.swp`, `*.swo`, `*.bak`, `*.tmp`, `*~`, `.DS_Store`, `Thumbs.db`), which docs/pregate.md and docs/troubleshooting.md now list too. -- **`literals`:** a named literal the branch only adds, such as `limit: 300` in a new object, was read as an existing `limit: 10` elsewhere in the file changing to 300, and every test that mentioned 10 was flagged. The check now compares every value under a name, so an added literal isn't a change. Editing an existing value still fails, and so does changing one of two equal values under the same name, which the old last-value check could miss. +- **`literals`:** a named literal the branch only adds, such as `limit: 300` in a new object, was read as an existing `limit: 10` elsewhere in the file changing to 300, and every test that mentioned 10 was flagged. The check now compares every value under a name, so an added literal isn't a change. Editing an existing value still fails, and so do two cases the old last-value check could miss: changing one of two equal values under the same name, and deleting one site while editing another. ### Tests - `test/pregate-selection.test.ts`: `probe.py` and `temporal-*.test.ts` pass `strays` while `zz-notes.md`, `tmp-x.ts`, `scratch.py` and `util.js.orig` still fail; a new object literal beside `limit: 10` passes `literals` while editing that value still fails; the pure `changedLiterals` case keeps an exported constant change. diff --git a/lib/pregate/checks/literals.ts b/lib/pregate/checks/literals.ts index 46373f985..a1db40bb7 100644 --- a/lib/pregate/checks/literals.ts +++ b/lib/pregate/checks/literals.ts @@ -73,8 +73,9 @@ export function changedLiterals(file: string, oldText: string, newText: string): if (oldC.has(name)) continue; const added = [...(newCall.get(name) ?? [])]; const removed = olds.filter(v => { const i = added.indexOf(v); if (i >= 0) added.splice(i, 1); return i < 0; }); + if (!added.length) continue; const seen = new Set(); - removed.forEach((old, i) => { if (i < added.length && !seen.has(old)) { seen.add(old); out.push({ name, old, new: added[i]!, file }); } }); + removed.forEach((old, i) => { if (!seen.has(old)) { seen.add(old); out.push({ name, old, new: added[Math.min(i, added.length - 1)]!, file }); } }); } return out; } diff --git a/test/pregate-selection.test.ts b/test/pregate-selection.test.ts index 99618df1b..d0f9e7a02 100644 --- a/test/pregate-selection.test.ts +++ b/test/pregate-selection.test.ts @@ -207,6 +207,9 @@ describe('literals (pure)', () => { expect(changedLiterals('f.ts', before, `${before}const query = { selection: { order: 'asc', limit: 300 } };\n`)).toEqual([]); expect(changedLiterals('f.ts', before, 'export const MAX = 3;\nconst page = { limit: 20 };\n')).toEqual([{ name: 'limit', old: '10', new: '20', file: 'f.ts' }]); expect(changedLiterals('f.ts', before, 'export const MAX = 4;\nconst page = { limit: 10 };\nconst query = { limit: 300 };\n')).toEqual([{ name: 'MAX', old: '3', new: '4', file: 'f.ts' }]); + expect(changedLiterals('f.ts', 'const retired = { limit: 10 };\nconst page = { limit: 20 };\n', 'const page = { limit: 30 };\n')).toEqual([ + { name: 'limit', old: '10', new: '30', file: 'f.ts' }, { name: 'limit', old: '20', new: '30', file: 'f.ts' }, + ]); }); // Value: protects=a sibling file that shares the module's stem (`postgres-unit-arms.txt` beside `postgres-unit-arms.ts`) is not the // module, so a test that only names it is not a consumer, while an extensionless import, a JS/TS import extension, the module's own