Skip to content

Commit ec86a08

Browse files
authored
fix(scripts): preserve manual NOTES block in incremental report generator (#1119)
* fix(scripts): preserve manual NOTES block in incremental report generator scripts/update-incremental-report.ts regenerates the report from scratch on every release. PR #1008 added a manual <!-- NOTES_START --> ... <!-- NOTES_END --> block explaining why 3.9.5 had null build metrics, but the 3.9.6 release silently dropped it because the script had no awareness of the sentinel pair. The version-history table now jumps 3.9.4 → 3.9.6 with no explanation. Extract any NOTES block from the existing file (mirroring how INCREMENTAL_BENCHMARK_DATA is preserved) and re-emit it just before the data comment in the regenerated markdown. Restore the lost 3.9.5 note and add a regression test gated on the env-overridable report path. Closes #1042 * fix(scripts): preserve all NOTES blocks via matchAll (#1119) If multiple NOTES_START/NOTES_END blocks are added to annotate different anomalous releases, .match() silently dropped all but the first — the same data-loss class this PR was created to prevent. Switch to .matchAll() and join with blank lines so every block survives regeneration. Add a test covering two distinct NOTES blocks.
1 parent 0138915 commit ec86a08

3 files changed

Lines changed: 154 additions & 2 deletions

File tree

generated/benchmarks/INCREMENTAL-BENCHMARKS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ Import resolution: native batch vs JS fallback throughput.
8888
| Per-import (JS) | 0ms |
8989
| Speedup ratio | 1.2x |
9090

91+
<!-- NOTES_START -->
92+
**Note (3.9.5):** No build/rebuild metrics for this release (both engines null) — only import resolution data was collected. Both the WASM and native workers reached the 1-file rebuild phase and then hung past the benchmark's 10-minute per-engine timeout (see `scripts/lib/fork-engine.ts`), so each was killed (`SIGKILL`) before returning results. Import resolution is unaffected because it runs in the parent process and doesn't depend on the full build. 3.9.5 is consequently absent from the top-level version-history comparison table since there are no build-time figures to compare against prior releases. The workflow run is [here](https://github.com/optave/ops-codegraph-tool/actions/runs/24863501577); the root cause will be investigated and the numbers backfilled in a follow-up if possible.
93+
<!-- NOTES_END -->
94+
9195
<!-- INCREMENTAL_BENCHMARK_DATA
9296
[
9397
{

scripts/update-incremental-report.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,13 @@ if (arg) {
2727
const entry = JSON.parse(jsonText);
2828

2929
// ── Paths ────────────────────────────────────────────────────────────────
30-
const reportPath = path.join(root, 'generated', 'benchmarks', 'INCREMENTAL-BENCHMARKS.md');
30+
const reportPath =
31+
process.env.CODEGRAPH_INCREMENTAL_REPORT_PATH ??
32+
path.join(root, 'generated', 'benchmarks', 'INCREMENTAL-BENCHMARKS.md');
3133

32-
// ── Load existing history ────────────────────────────────────────────────
34+
// ── Load existing history + manual NOTES block ───────────────────────────
3335
let history = [];
36+
let notesBlock = '';
3437
if (fs.existsSync(reportPath)) {
3538
const content = fs.readFileSync(reportPath, 'utf8');
3639
const match = content.match(/<!--\s*INCREMENTAL_BENCHMARK_DATA\s*([\s\S]*?)\s*-->/);
@@ -41,6 +44,13 @@ if (fs.existsSync(reportPath)) {
4144
/* start fresh if corrupt */
4245
}
4346
}
47+
// Use matchAll so multiple NOTES blocks (annotating different anomalous releases)
48+
// are all preserved. The exact data-loss bug this fix targets stemmed from silently
49+
// dropping a NOTES block; we must not reintroduce that failure mode for additional blocks.
50+
const notesMatches = content.matchAll(
51+
/<!--\s*NOTES_START\s*-->[\s\S]*?<!--\s*NOTES_END\s*-->/g,
52+
);
53+
notesBlock = Array.from(notesMatches, (m) => m[0]).join('\n\n');
4454
}
4555

4656
// Add new entry — dev entries are rolling, releases replace dev
@@ -155,6 +165,8 @@ if (r.nativeBatchMs != null && r.jsFallbackMs > 0) {
155165
}
156166
md += '\n';
157167

168+
if (notesBlock) md += `${notesBlock}\n\n`;
169+
158170
md += `<!-- INCREMENTAL_BENCHMARK_DATA\n${JSON.stringify(history, null, 2)}\n-->\n`;
159171

160172
fs.mkdirSync(path.dirname(reportPath), { recursive: true });
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import { execFileSync } from 'node:child_process';
2+
import fs from 'node:fs';
3+
import os from 'node:os';
4+
import path from 'node:path';
5+
import { fileURLToPath } from 'node:url';
6+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
7+
8+
const __dirname = path.dirname(fileURLToPath(import.meta.url));
9+
const repoRoot = path.resolve(__dirname, '..', '..');
10+
const scriptPath = path.join(repoRoot, 'scripts', 'update-incremental-report.ts');
11+
12+
// --experimental-strip-types works in Node 22.6+ through current 24.x; the
13+
// renamed --strip-types was added then removed again across 24.x minor
14+
// versions, so prefer the experimental name for compatibility.
15+
const stripFlag = '--experimental-strip-types';
16+
17+
const SAMPLE_ENTRY = {
18+
version: '9.9.9',
19+
date: '2026-05-14',
20+
files: 100,
21+
wasm: { fullBuildMs: 1000, noopRebuildMs: 10, oneFileRebuildMs: 50 },
22+
native: { fullBuildMs: 500, noopRebuildMs: 5, oneFileRebuildMs: 25 },
23+
resolve: {
24+
imports: 200,
25+
nativeBatchMs: 2,
26+
jsFallbackMs: 4,
27+
perImportNativeMs: 0,
28+
perImportJsMs: 0,
29+
},
30+
};
31+
32+
let tmpDir: string;
33+
let reportPath: string;
34+
let entryPath: string;
35+
36+
beforeEach(() => {
37+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-incr-report-'));
38+
reportPath = path.join(tmpDir, 'INCREMENTAL-BENCHMARKS.md');
39+
entryPath = path.join(tmpDir, 'entry.json');
40+
fs.writeFileSync(entryPath, JSON.stringify(SAMPLE_ENTRY));
41+
});
42+
43+
afterEach(() => {
44+
if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true });
45+
});
46+
47+
function runScript() {
48+
execFileSync('node', [stripFlag, scriptPath, entryPath], {
49+
env: { ...process.env, CODEGRAPH_INCREMENTAL_REPORT_PATH: reportPath },
50+
stdio: 'pipe',
51+
});
52+
}
53+
54+
describe('update-incremental-report script', () => {
55+
it('preserves a manual NOTES_START/NOTES_END block across regeneration', () => {
56+
const NOTES = `<!-- NOTES_START -->
57+
**Note (9.9.8):** Workers hung past the 10-minute timeout and were SIGKILL'd.
58+
<!-- NOTES_END -->`;
59+
const initial = `# Codegraph Incremental Build Benchmarks
60+
61+
${NOTES}
62+
63+
<!-- INCREMENTAL_BENCHMARK_DATA
64+
[
65+
{
66+
"version": "9.9.8",
67+
"date": "2026-05-01",
68+
"files": 99,
69+
"wasm": null,
70+
"native": null,
71+
"resolve": { "imports": 100, "nativeBatchMs": 1, "jsFallbackMs": 2, "perImportNativeMs": 0, "perImportJsMs": 0 }
72+
}
73+
]
74+
-->
75+
`;
76+
fs.writeFileSync(reportPath, initial);
77+
78+
runScript();
79+
80+
const out = fs.readFileSync(reportPath, 'utf8');
81+
expect(out).toContain('<!-- NOTES_START -->');
82+
expect(out).toContain('<!-- NOTES_END -->');
83+
expect(out).toContain("Workers hung past the 10-minute timeout and were SIGKILL'd");
84+
// Notes should appear before the data comment, after the latest summary
85+
expect(out.indexOf('<!-- NOTES_START -->')).toBeLessThan(
86+
out.indexOf('<!-- INCREMENTAL_BENCHMARK_DATA'),
87+
);
88+
});
89+
90+
it('preserves multiple NOTES_START/NOTES_END blocks across regeneration', () => {
91+
const NOTES_A = `<!-- NOTES_START -->
92+
**Note (9.9.8):** Workers hung past the 10-minute timeout and were SIGKILL'd.
93+
<!-- NOTES_END -->`;
94+
const NOTES_B = `<!-- NOTES_START -->
95+
**Note (9.9.7):** Build artifact corrupted during upload, metrics re-run manually.
96+
<!-- NOTES_END -->`;
97+
const initial = `# Codegraph Incremental Build Benchmarks
98+
99+
${NOTES_A}
100+
101+
${NOTES_B}
102+
103+
<!-- INCREMENTAL_BENCHMARK_DATA
104+
[]
105+
-->
106+
`;
107+
fs.writeFileSync(reportPath, initial);
108+
109+
runScript();
110+
111+
const out = fs.readFileSync(reportPath, 'utf8');
112+
// Both notes must survive regeneration — single-block preservation would
113+
// silently drop the second block (the same data-loss class this PR fixes).
114+
expect(out).toContain("Workers hung past the 10-minute timeout and were SIGKILL'd");
115+
expect(out).toContain('Build artifact corrupted during upload, metrics re-run manually');
116+
// Each block keeps its own pair of delimiters.
117+
expect(out.match(/<!--\s*NOTES_START\s*-->/g)?.length).toBe(2);
118+
expect(out.match(/<!--\s*NOTES_END\s*-->/g)?.length).toBe(2);
119+
});
120+
121+
it('does not invent a NOTES block when none was present', () => {
122+
const initial = `# Codegraph Incremental Build Benchmarks
123+
124+
<!-- INCREMENTAL_BENCHMARK_DATA
125+
[]
126+
-->
127+
`;
128+
fs.writeFileSync(reportPath, initial);
129+
130+
runScript();
131+
132+
const out = fs.readFileSync(reportPath, 'utf8');
133+
expect(out).not.toContain('NOTES_START');
134+
expect(out).not.toContain('NOTES_END');
135+
});
136+
});

0 commit comments

Comments
 (0)