RomanNum3ral
/
gstack
mirror of https://github.com/garrytan/gstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(retro): language-agnostic test detection + anti-fabrication guard 

The retro generator's test-counting commands only matched JS/TS files
(grep '\.(test|spec)\.'), returning 0 for Python (test_*.py), Terraform
(*.tftest.hcl), and Bats (*.bats) suites. Combined with a repo-wide total
count, the contradiction ('N total tests, 0 added') led the model to
fabricate a non-existent 'bootstrap commit' to reconcile the numbers.

- Commands 10/12/13: language-agnostic TEST pattern; split tests ADDED
  (--diff-filter=A) from TOUCHED; command 10 uses git ls-files.
- New Step 1.5 plausibility guard: per-commit figures must come from
  'git show --shortstat <hash>'; no invented bootstrap/foundation
  narrative; reconcile count contradictions by re-checking, not narrating.
- Applied to both SKILL.md.tmpl (source) and SKILL.md (runtime).
- regression-retro-test-detection.test.ts locks in both invariants.

Note: SKILL.md line 1 'name: gstack-retro' is pre-existing install-time
prefix drift (skill_prefix:true), not part of this fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Sholto McNeilage 2026-06-18 11:16:17 +10:00
parent c7ae63201a
commit f181fd3a4b
3 changed files with 233 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
retro/SKILL.md
View File

@ -1,5 +1,5 @@
---
name: retro
name: gstack-retro
preamble-tier: 2
version: 2.0.0
description: Weekly engineering retrospective. (gstack)
@ -972,7 +972,8 @@ git log origin/<default> --since="<window>" --format="%H|%aN|%ae|%ai|%s" --short
# 2. Per-commit test vs total LOC breakdown with author
# Each commit block starts with COMMIT:<hash>|<author>, followed by numstat lines.
# Separate test files (matching test/|spec/|__tests__/) from production files.
# Separate test files from production files using the language-agnostic TEST pattern
# below (commands 10/12) — NOT just JS/TS. Python test_*.py, *.tftest.hcl, *.bats count.
git log origin/<default> --since="<window>" --format="COMMIT:%H|%aN" --numstat
# 3. Commit timestamps for session detection and hourly distribution (with author)
@ -996,19 +997,58 @@ cat ~/.gstack/greptile-history.md 2>/dev/null || true
# 9. TODOS.md backlog (if available)
cat TODOS.md 2>/dev/null || true
# 10. Test file count
find . -name '*.test.*' -o -name '*.spec.*' -o -name '*_test.*' -o -name '*_spec.*' 2>/dev/null | grep -v node_modules | wc -l
# --- Language-agnostic TEST file pattern (used by commands 10, 12, 13) ---
# Matches: Python test_*.py / *_test.py, JS/TS *.test.* / *.spec.*, Ruby *_spec.rb,
# Terraform *.tftest.hcl, Bats *.bats, and any path under tests/ __tests__/ spec/.
# Do NOT narrow this to '\.(test|spec)\.' — that misses every non-JS test suite.
# TEST_RE='(^|/)test_|_test\.|\.test\.|\.spec\.|_spec\.|\.tftest\.hcl$|\.bats$|(^|/)(tests?|__tests__|spec)/'
# VENDOR_RE='/(node_modules|\.venv|venv|dist|build|\.git)/'
# 10. Total test file count (repo-wide; git ls-files respects .gitignore)
git ls-files | grep -ivE '/(node_modules|\.venv|venv|dist|build)/' \
| grep -iE '(^|/)test_|_test\.|\.test\.|\.spec\.|_spec\.|\.tftest\.hcl$|\.bats$|(^|/)(tests?|__tests__|spec)/' \
| sort -u | wc -l
# 11. Regression test commits in window
git log origin/<default> --since="<window>" --oneline --grep="test(qa):" --grep="test(design):" --grep="test: coverage"
# 12. gstack skill usage telemetry (if available)
cat ~/.gstack/analytics/skill-usage.jsonl 2>/dev/null || true
# 12. Test files ADDED in window (this is "tests added this period")
git log origin/<default> --since="<window>" --diff-filter=A --format="" --name-only \
| grep -ivE '/(node_modules|\.venv|venv|dist|build)/' \
| grep -iE '(^|/)test_|_test\.|\.test\.|\.spec\.|_spec\.|\.tftest\.hcl$|\.bats$|(^|/)(tests?|__tests__|spec)/' \
| sort -u | wc -l
# 12. Test files changed in window
git log origin/<default> --since="<window>" --format="" --name-only | grep -E '\.(test|spec)\.' | sort -u | wc -l
# 13. Test files TOUCHED in window (added OR modified — sanity cross-check vs command 12)
git log origin/<default> --since="<window>" --format="" --name-only \
| grep -ivE '/(node_modules|\.venv|venv|dist|build)/' \
| grep -iE '(^|/)test_|_test\.|\.test\.|\.spec\.|_spec\.|\.tftest\.hcl$|\.bats$|(^|/)(tests?|__tests__|spec)/' \
| sort -u | wc -l
# 14. gstack skill usage telemetry (if available)
cat ~/.gstack/analytics/skill-usage.jsonl 2>/dev/null || true
```
### Step 1.5: Per-commit plausibility guard (anti-fabrication)
Before describing ANY single commit's size or any "big landing," obey these rules. They
exist because a contradiction between aggregate counts (e.g. "1,446 total tests exist" from
command 10) and a window count (e.g. "0 tests added" from command 12) was once reconciled by
inventing a non-existent "bootstrap commit" with fabricated file/LOC figures.
- **Per-commit figures come from that commit only.** Any per-commit file count or LOC you
state MUST come from `git show --stat <hash>` / `git show --shortstat <hash>` for that exact
hash. Never infer a commit's size from aggregate totals, a PR number, or a commit subject.
- **Verify outliers.** If you are about to describe a commit as unusually large (>500 files
or >50k insertions), first run `git show --shortstat <hash>` and quote the real numbers. If
the commit is small, do not call it large.
- **No invented "bootstrap"/"foundation"/"initial import" narrative.** Only describe such a
commit if a real commit in the window has a matching subject AND a verified stat showing it.
A mature repo (check `git log --reverse --format=%ai | head -1`) does not get re-bootstrapped.
- **Reconcile contradictions by re-checking, not narrating.** If command 10 (total) and
command 12 (added) seem to contradict each other, re-run command 12 (it is the usual culprit
— a test-pattern miss) before writing a single word about test activity. Never bridge two
numbers with an event you have not verified exists.
### Step 2: Compute Metrics
Calculate and present these metrics in a summary table:
@ -1379,9 +1419,16 @@ Narrative covering:
### Test Health
- Total test files: N (from command 10)
- Tests added this period: M (from command 12 — test files changed)
- Tests added this period: M (from command 12 — test files ADDED). Also note tests TOUCHED
(command 13). If command 12 returns 0 but command 13 > 0, tests were modified not added —
say so; do NOT report "no test activity."
- Regression test commits: list `test(qa):` and `test(design):` and `test: coverage` commits from command 11
- If prior retro exists and has `test_health`: show delta "Test count: {last} → {now} (+{delta})"
- **Sanity check before claiming "0 tests added":** new tests are frequently committed under
`fix:`/`feat:` (not a `test:` prefix), and command 11 only counts `test(...)`-prefixed
commits — never infer "0 new tests" from command 11. Trust command 12 (file-level). If
command 12 is 0 while many files changed, re-run it before concluding — a zero there is
more often a pattern miss than a real absence.
- If test ratio < 20%: flag as growth area — "100% test coverage is the goal. Tests make vibe coding safe."
### Plan Completion

63
retro/SKILL.md.tmpl
View File

@ -172,7 +172,8 @@ git log origin/<default> --since="<window>" --format="%H|%aN|%ae|%ai|%s" --short
# 2. Per-commit test vs total LOC breakdown with author
# Each commit block starts with COMMIT:<hash>|<author>, followed by numstat lines.
# Separate test files (matching test/|spec/|__tests__/) from production files.
# Separate test files from production files using the language-agnostic TEST pattern
# below (commands 10/12) — NOT just JS/TS. Python test_*.py, *.tftest.hcl, *.bats count.
git log origin/<default> --since="<window>" --format="COMMIT:%H|%aN" --numstat
# 3. Commit timestamps for session detection and hourly distribution (with author)
@ -196,19 +197,58 @@ cat ~/.gstack/greptile-history.md 2>/dev/null || true
# 9. TODOS.md backlog (if available)
cat TODOS.md 2>/dev/null || true
# 10. Test file count
find . -name '*.test.*' -o -name '*.spec.*' -o -name '*_test.*' -o -name '*_spec.*' 2>/dev/null | grep -v node_modules | wc -l
# --- Language-agnostic TEST file pattern (used by commands 10, 12, 13) ---
# Matches: Python test_*.py / *_test.py, JS/TS *.test.* / *.spec.*, Ruby *_spec.rb,
# Terraform *.tftest.hcl, Bats *.bats, and any path under tests/ __tests__/ spec/.
# Do NOT narrow this to '\.(test|spec)\.' — that misses every non-JS test suite.
# TEST_RE='(^|/)test_|_test\.|\.test\.|\.spec\.|_spec\.|\.tftest\.hcl$|\.bats$|(^|/)(tests?|__tests__|spec)/'
# VENDOR_RE='/(node_modules|\.venv|venv|dist|build|\.git)/'
# 10. Total test file count (repo-wide; git ls-files respects .gitignore)
git ls-files | grep -ivE '/(node_modules|\.venv|venv|dist|build)/' \
| grep -iE '(^|/)test_|_test\.|\.test\.|\.spec\.|_spec\.|\.tftest\.hcl$|\.bats$|(^|/)(tests?|__tests__|spec)/' \
| sort -u | wc -l
# 11. Regression test commits in window
git log origin/<default> --since="<window>" --oneline --grep="test(qa):" --grep="test(design):" --grep="test: coverage"
# 12. gstack skill usage telemetry (if available)
cat ~/.gstack/analytics/skill-usage.jsonl 2>/dev/null || true
# 12. Test files ADDED in window (this is "tests added this period")
git log origin/<default> --since="<window>" --diff-filter=A --format="" --name-only \
| grep -ivE '/(node_modules|\.venv|venv|dist|build)/' \
| grep -iE '(^|/)test_|_test\.|\.test\.|\.spec\.|_spec\.|\.tftest\.hcl$|\.bats$|(^|/)(tests?|__tests__|spec)/' \
| sort -u | wc -l
# 12. Test files changed in window
git log origin/<default> --since="<window>" --format="" --name-only | grep -E '\.(test|spec)\.' | sort -u | wc -l
# 13. Test files TOUCHED in window (added OR modified — sanity cross-check vs command 12)
git log origin/<default> --since="<window>" --format="" --name-only \
| grep -ivE '/(node_modules|\.venv|venv|dist|build)/' \
| grep -iE '(^|/)test_|_test\.|\.test\.|\.spec\.|_spec\.|\.tftest\.hcl$|\.bats$|(^|/)(tests?|__tests__|spec)/' \
| sort -u | wc -l
# 14. gstack skill usage telemetry (if available)
cat ~/.gstack/analytics/skill-usage.jsonl 2>/dev/null || true
```
### Step 1.5: Per-commit plausibility guard (anti-fabrication)
Before describing ANY single commit's size or any "big landing," obey these rules. They
exist because a contradiction between aggregate counts (e.g. "1,446 total tests exist" from
command 10) and a window count (e.g. "0 tests added" from command 12) was once reconciled by
inventing a non-existent "bootstrap commit" with fabricated file/LOC figures.
- **Per-commit figures come from that commit only.** Any per-commit file count or LOC you
state MUST come from `git show --stat <hash>` / `git show --shortstat <hash>` for that exact
hash. Never infer a commit's size from aggregate totals, a PR number, or a commit subject.
- **Verify outliers.** If you are about to describe a commit as unusually large (>500 files
or >50k insertions), first run `git show --shortstat <hash>` and quote the real numbers. If
the commit is small, do not call it large.
- **No invented "bootstrap"/"foundation"/"initial import" narrative.** Only describe such a
commit if a real commit in the window has a matching subject AND a verified stat showing it.
A mature repo (check `git log --reverse --format=%ai | head -1`) does not get re-bootstrapped.
- **Reconcile contradictions by re-checking, not narrating.** If command 10 (total) and
command 12 (added) seem to contradict each other, re-run command 12 (it is the usual culprit
— a test-pattern miss) before writing a single word about test activity. Never bridge two
numbers with an event you have not verified exists.
### Step 2: Compute Metrics
Calculate and present these metrics in a summary table:
@ -556,9 +596,16 @@ Narrative covering:
### Test Health
- Total test files: N (from command 10)
- Tests added this period: M (from command 12 — test files changed)
- Tests added this period: M (from command 12 — test files ADDED). Also note tests TOUCHED
(command 13). If command 12 returns 0 but command 13 > 0, tests were modified not added —
say so; do NOT report "no test activity."
- Regression test commits: list `test(qa):` and `test(design):` and `test: coverage` commits from command 11
- If prior retro exists and has `test_health`: show delta "Test count: {last} → {now} (+{delta})"
- **Sanity check before claiming "0 tests added":** new tests are frequently committed under
`fix:`/`feat:` (not a `test:` prefix), and command 11 only counts `test(...)`-prefixed
commits — never infer "0 new tests" from command 11. Trust command 12 (file-level). If
command 12 is 0 while many files changed, re-run it before concluding — a zero there is
more often a pattern miss than a real absence.
- If test ratio < 20%: flag as growth area — "100% test coverage is the goal. Tests make vibe coding safe."
### Plan Completion

122
test/regression-retro-test-detection.test.ts Normal file
View File

@ -0,0 +1,122 @@
/**
* Regression tests for the /retro test-detection + anti-fabrication fix.
*
* Background: a generated retro claimed "0 new tests added" and invented a
* non-existent 5,165-file "bootstrap commit" to reconcile that zero against a
* large total-test count. Two root causes in retro/SKILL.md(.tmpl):
*
* 1. The "tests changed in window" command used `grep -E '\.(test|spec)\.'`,
* which only matches JS/TS files (foo.test.ts / foo.spec.js). It returned 0
* for Python (test_*.py), Terraform (*.tftest.hcl), and Bats (*.bats) suites.
* 2. There was no guard forbidding per-commit figures that aren't read from the
* commit itself, so the model fabricated a "bootstrap" narrative.
*
* These are static invariants against the skill body — they fail the build if the
* language-agnostic pattern is narrowed again or the plausibility guard is removed.
*/
import { describe, expect, test } from "bun:test";
import * as fs from "node:fs";
import * as path from "node:path";
const ROOT = path.resolve(import.meta.dir, "..");
const RETRO_TMPL = path.join(ROOT, "retro", "SKILL.md.tmpl");
const RETRO_MD = path.join(ROOT, "retro", "SKILL.md");
function readTmpl(): string {
return fs.readFileSync(RETRO_TMPL, "utf-8");
}
function readMd(): string {
return fs.readFileSync(RETRO_MD, "utf-8");
}
// The test-detection pattern must recognise these representative paths.
const MUST_MATCH = [
"tests/apps/engine_attribution/unit/test_register_tenant_context.py", // Python prefix
"libs/foo/bar_test.py", // Python suffix
"apps/dashboard/src/x.test.ts", // JS/TS infix
"apps/dashboard/src/x.spec.tsx", // JS/TS spec
"infrastructure/terraform/modules/cloud_run/tests/cloud_run_v2.tftest.hcl", // Terraform
"tests/infrastructure/test_assert_no_cloud_run_destroy.bats", // Bats
];
// The pattern must NOT classify these production files as tests.
const MUST_NOT_MATCH = [
"apps/engine_attribution/main.py",
"infrastructure/terraform/environments/production/main.tf",
"packages/components_ui/src/Button.tsx",
];
// The literal grep -iE pattern embedded in the skill (kept in sync with both files).
const TEST_RE =
/\(\^\|\/\)test_\|_test\\\.\|\\\.test\\\.\|\\\.spec\\\.\|_spec\\\.\|\\\.tftest\\\.hcl\$\|\\\.bats\$\|\(\^\|\/\)\(tests\?\|__tests__\|spec\)\//;
describe("retro test-detection — language-agnostic pattern present in both layers", () => {
for (const [label, read] of [
["template", readTmpl],
["compiled SKILL.md", readMd],
] as const) {
test(`${label} embeds the language-agnostic TEST pattern (Python/TF/Bats, not just JS)`, () => {
const body = read();
expect(body).toMatch(TEST_RE);
// belt-and-braces: the specific non-JS tokens must be there
expect(body).toContain("tftest\\.hcl$");
expect(body).toContain("\\.bats$");
expect(body).toContain("(^|/)test_");
});
test(`${label} does NOT use the narrow JS-only grep as a counting command`, () => {
const body = read();
// The narrow pattern may appear ONLY inside the explanatory "Do NOT narrow" comment.
const narrowOnCommandLine = body
.split("\n")
.filter((l) => l.includes("(test|spec)") && !l.includes("Do NOT narrow"));
expect(narrowOnCommandLine).toEqual([]);
});
}
});
describe("retro test-detection — pattern behaviour (model the embedded regex in JS)", () => {
// Mirror of the embedded grep -iE pattern, as a JS RegExp, case-insensitive.
const re = /(^|\/)test_|_test\.|\.test\.|\.spec\.|_spec\.|\.tftest\.hcl$|\.bats$|(^|\/)(tests?|__tests__|spec)\//i;
for (const p of MUST_MATCH) {
test(`matches test file: ${p}`, () => {
expect(re.test(p)).toBe(true);
});
}
for (const p of MUST_NOT_MATCH) {
test(`does not match production file: ${p}`, () => {
expect(re.test(p)).toBe(false);
});
}
});
describe("retro anti-fabrication — Step 1.5 plausibility guard present", () => {
for (const [label, read] of [
["template", readTmpl],
["compiled SKILL.md", readMd],
] as const) {
test(`${label} has the Step 1.5 plausibility guard`, () => {
const body = read();
expect(body).toMatch(/### Step 1\.5: Per-commit plausibility guard/);
});
test(`${label} forbids inventing a bootstrap/foundation narrative`, () => {
const body = read();
expect(body).toMatch(/No invented "bootstrap"\/"foundation"\/"initial import" narrative/);
});
test(`${label} requires per-commit figures from git show --shortstat`, () => {
const body = read();
expect(body).toMatch(/git show --shortstat <hash>/);
});
test(`${label} guard sits between the command block and Step 2`, () => {
const body = read();
const guard = body.indexOf("### Step 1.5: Per-commit plausibility guard");
const step2 = body.indexOf("### Step 2: Compute Metrics");
expect(guard).toBeGreaterThan(-1);
expect(step2).toBeGreaterThan(-1);
expect(guard).toBeLessThan(step2);
});
}
});