mirror of https://github.com/garrytan/gstack.git
Merge f181fd3a4b into a3259400a3
This commit is contained in:
Sholto McNeilage
GitHub
commit
63777b850e
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
name: retro
|
||||
name: gstack-retro
|
||||
preamble-tier: 2
|
||||
version: 2.0.0
|
||||
description: Weekly engineering retrospective. (gstack)
|
||||
|
|
@ -1001,7 +1001,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)
|
||||
|
|
@ -1025,19 +1026,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:
|
||||
|
|
@ -1408,9 +1448,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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
}
|
||||
});
|
||||
Loading…
Reference in New Issue