Skip to content

fix: show both engines side-by-side in README benchmark table #826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carlos-alm merged 6 commits into from
Apr 5, 2026
+71 −33
Merged

fix: show both engines side-by-side in README benchmark table #826

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0a43d2b
fix: show only preferred engine in README benchmark table
carlos-alm Apr 4, 2026
ca6a828
fix: guard wasm null access and add prefLabel to query rows
carlos-alm Apr 4, 2026
97b2cf4
fix: show both engines in all README benchmark rows
carlos-alm Apr 4, 2026
8a83fe5
fix: guard against null wasm data in README benchmark table
carlos-alm Apr 5, 2026
f2a4335
Merge main into fix/benchmark-readme-table
carlos-alm Apr 5, 2026
7914b33
Merge branch 'main' into fix/benchmark-readme-table
carlos-alm Apr 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 71 additions & 33 deletions scripts/update-benchmark-report.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,11 @@ md += '| Metric | Native (Rust) | WASM |\n';
md += '|--------|---:|---:|\n';

const estNative = latest.native?.perFile;
const estWasm = latest.wasm.perFile;
md += `| Build time | ${estNative ? formatMs(estNative.buildTimeMs * ESTIMATE_FILES) : 'n/a'} | ${formatMs(estWasm.buildTimeMs * ESTIMATE_FILES)} |\n`;
md += `| DB size | ${estNative ? formatBytes(estNative.dbSizeBytes * ESTIMATE_FILES) : 'n/a'} | ${formatBytes(estWasm.dbSizeBytes * ESTIMATE_FILES)} |\n`;
md += `| Nodes | ${estNative ? Math.round(estNative.nodes * ESTIMATE_FILES).toLocaleString() : 'n/a'} | ${Math.round(estWasm.nodes * ESTIMATE_FILES).toLocaleString()} |\n`;
md += `| Edges | ${estNative ? Math.round(estNative.edges * ESTIMATE_FILES).toLocaleString() : 'n/a'} | ${Math.round(estWasm.edges * ESTIMATE_FILES).toLocaleString()} |\n\n`;
const estWasm = latest.wasm?.perFile;
md += `| Build time | ${estNative ? formatMs(estNative.buildTimeMs * ESTIMATE_FILES) : 'n/a'} | ${estWasm ? formatMs(estWasm.buildTimeMs * ESTIMATE_FILES) : 'n/a'} |\n`;
md += `| DB size | ${estNative ? formatBytes(estNative.dbSizeBytes * ESTIMATE_FILES) : 'n/a'} | ${estWasm ? formatBytes(estWasm.dbSizeBytes * ESTIMATE_FILES) : 'n/a'} |\n`;
md += `| Nodes | ${estNative ? Math.round(estNative.nodes * ESTIMATE_FILES).toLocaleString() : 'n/a'} | ${estWasm ? Math.round(estWasm.nodes * ESTIMATE_FILES).toLocaleString() : 'n/a'} |\n`;
md += `| Edges | ${estNative ? Math.round(estNative.edges * ESTIMATE_FILES).toLocaleString() : 'n/a'} | ${estWasm ? Math.round(estWasm.edges * ESTIMATE_FILES).toLocaleString() : 'n/a'} |\n\n`;

// ── Incremental Rebuilds section ──────────────────────────────────────────
const hasIncremental = history.some(
Expand Down Expand Up @@ -324,41 +324,71 @@ if (prev) {
if (fs.existsSync(readmePath)) {
let readme = fs.readFileSync(readmePath, 'utf8');

// Build the table rows — show both engines when native is available
// Pick the preferred engine: native when available, WASM as fallback
const pref = latest.native || latest.wasm;
const prefLabel = latest.native ? ' (native)' : '';
// Show both engines side-by-side when both are available;
// fall back to native-only or WASM-only single-column layout otherwise.
const hasNative = latest.native != null;
const hasBoth = hasNative && latest.wasm != null;

let rows = '';
if (latest.native) {
rows += `| Build speed (native) | **${latest.native.perFile.buildTimeMs} ms/file** |\n`;
rows += `| Build speed (WASM) | **${latest.wasm.perFile.buildTimeMs} ms/file** |\n`;
rows += `| Query time (native) | **${formatMs(latest.native.queryTimeMs)}** |\n`;
rows += `| Query time (WASM) | **${formatMs(latest.wasm.queryTimeMs)}** |\n`;
if (hasBoth) {
rows += `| Build speed | **${latest.native.perFile.buildTimeMs} ms/file** | **${latest.wasm.perFile.buildTimeMs} ms/file** |\n`;
rows += `| Query time | **${formatMs(latest.native.queryTimeMs)}** | **${formatMs(latest.wasm.queryTimeMs)}** |\n`;
} else if (hasNative) {
rows += `| Build speed | **${latest.native.perFile.buildTimeMs} ms/file** |\n`;
rows += `| Query time | **${formatMs(latest.native.queryTimeMs)}** |\n`;
} else {
rows += `| Build speed | **${latest.wasm.perFile.buildTimeMs} ms/file** |\n`;
rows += `| Query time | **${formatMs(latest.wasm.queryTimeMs)}** |\n`;
}

// Incremental rebuild rows (prefer native, fallback to WASM)
if (pref.noopRebuildMs != null) {
rows += `| No-op rebuild${prefLabel} | **${formatMs(pref.noopRebuildMs)}** |\n`;
}
if (pref.oneFileRebuildMs != null) {
rows += `| 1-file rebuild${prefLabel} | **${formatMs(pref.oneFileRebuildMs)}** |\n`;
// Incremental rebuild rows
if (hasBoth) {
const nativeNoop = latest.native.noopRebuildMs != null ? `**${formatMs(latest.native.noopRebuildMs)}**` : 'n/a';
const wasmNoop = latest.wasm.noopRebuildMs != null ? `**${formatMs(latest.wasm.noopRebuildMs)}**` : 'n/a';
if (latest.native.noopRebuildMs != null || latest.wasm.noopRebuildMs != null) {
rows += `| No-op rebuild | ${nativeNoop} | ${wasmNoop} |\n`;
}
const nativeOneFile = latest.native.oneFileRebuildMs != null ? `**${formatMs(latest.native.oneFileRebuildMs)}**` : 'n/a';
const wasmOneFile = latest.wasm.oneFileRebuildMs != null ? `**${formatMs(latest.wasm.oneFileRebuildMs)}**` : 'n/a';
if (latest.native.oneFileRebuildMs != null || latest.wasm.oneFileRebuildMs != null) {
rows += `| 1-file rebuild | ${nativeOneFile} | ${wasmOneFile} |\n`;
}
} else {
const pref = latest.native || latest.wasm;
if (pref.noopRebuildMs != null) {
rows += `| No-op rebuild | **${formatMs(pref.noopRebuildMs)}** |\n`;
}
if (pref.oneFileRebuildMs != null) {
rows += `| 1-file rebuild | **${formatMs(pref.oneFileRebuildMs)}** |\n`;
}
}

// Query latency rows (pick two representative queries, skip if null)
if (pref.queries) {
if (pref.queries.fnDepsMs != null) rows += `| Query: fn-deps | **${pref.queries.fnDepsMs}ms** |\n`;
if (pref.queries.pathMs != null) rows += `| Query: path | **${pref.queries.pathMs}ms** |\n`;
// Query latency rows (pick two representative queries)
if (hasBoth) {
const nq = latest.native.queries;
const wq = latest.wasm.queries;
if (nq?.fnDepsMs != null || wq?.fnDepsMs != null) {
rows += `| Query: fn-deps | **${nq?.fnDepsMs ?? 'n/a'}ms** | **${wq?.fnDepsMs ?? 'n/a'}ms** |\n`;
}
if (nq?.pathMs != null || wq?.pathMs != null) {
rows += `| Query: path | **${nq?.pathMs ?? 'n/a'}ms** | **${wq?.pathMs ?? 'n/a'}ms** |\n`;
}
} else {
const pref = latest.native || latest.wasm;
if (pref.queries?.fnDepsMs != null) rows += `| Query: fn-deps | **${pref.queries.fnDepsMs}ms** |\n`;
if (pref.queries?.pathMs != null) rows += `| Query: path | **${pref.queries.pathMs}ms** |\n`;
}

// 50k-file estimate
const estBuild = latest.native
? formatMs(latest.native.perFile.buildTimeMs * ESTIMATE_FILES)
: formatMs(latest.wasm.perFile.buildTimeMs * ESTIMATE_FILES);
rows += `| ~${(ESTIMATE_FILES).toLocaleString()} files (est.) | **~${estBuild} build** |\n`;
if (hasBoth) {
const estNativeBuild = formatMs(latest.native.perFile.buildTimeMs * ESTIMATE_FILES);
const estWasmBuild = formatMs(latest.wasm.perFile.buildTimeMs * ESTIMATE_FILES);
rows += `| ~${(ESTIMATE_FILES).toLocaleString()} files (est.) | **~${estNativeBuild} build** | **~${estWasmBuild} build** |\n`;
} else {
const pref = latest.native || latest.wasm;
const estBuild = formatMs(pref.perFile.buildTimeMs * ESTIMATE_FILES);
rows += `| ~${(ESTIMATE_FILES).toLocaleString()} files (est.) | **~${estBuild} build** |\n`;
}
Comment on lines 332 to +391
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Null access on latest.wasm when native is present but WASM is absent

hasNative is only latest.native != null, but every code path inside the if (hasNative) block directly dereferences latest.wasm (lines 332, 333, 342, 346, 362, 378–379) without guarding against latest.wasm being null. If a benchmark run produces native data but WASM data is absent (e.g. the WASM grammar failed to build), this throws TypeError: Cannot read properties of null. The immediately preceding commit (ca6a828) was literally titled "fix: guard wasm null access", so this scenario is known to happen in practice.

Use a hasBoth flag instead:

const hasNative = latest.native != null;
const hasBoth   = hasNative && latest.wasm != null;

let rows = '';
if (hasBoth) {
    rows += `| Build speed | **${latest.native.perFile.buildTimeMs} ms/file** | **${latest.wasm.perFile.buildTimeMs} ms/file** |\n`;
    rows += `| Query time | **${formatMs(latest.native.queryTimeMs)}** | **${formatMs(latest.wasm.queryTimeMs)}** |\n`;
} else if (hasNative) {
    rows += `| Build speed | **${latest.native.perFile.buildTimeMs} ms/file** |\n`;
    rows += `| Query time | **${formatMs(latest.native.queryTimeMs)}** |\n`;
} else {
    rows += `| Build speed | **${latest.wasm.perFile.buildTimeMs} ms/file** |\n`;
    rows += `| Query time | **${formatMs(latest.wasm.queryTimeMs)}** |\n`;
}

And replace every downstream if (hasNative) guard (incremental rebuild rows, query latency rows, 50k-file estimate, resolution rows, and tableHeader) with if (hasBoth) so that WASM data is only shown when it is actually present.

Copy link
Copy Markdown
Contributor Author

@carlos-alm carlos-alm Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8a83fe5. Introduced a hasBoth flag (hasNative && latest.wasm != null) and replaced every if (hasNative) guard in the README-patch block with if (hasBoth). The two-column layout now only activates when both engines have data. When only native is present (no WASM), falls back to single-column layout using a pref = latest.native || latest.wasm pattern — same approach used for the existing WASM-only fallback. The hasNative variable is retained only for the intermediate native-only branch in the build speed / query time rows.


// Preserve existing benchmark link line from README rather than hardcoding.
// Fall back to a default if we can't find it.
Expand All @@ -369,28 +399,36 @@ if (fs.existsSync(readmePath)) {
}

// Resolution precision/recall — from resolution-benchmark.ts JSON merged into entry
// Resolution is engine-independent, so show single value (span both columns when needed)
if (latest.resolution) {
// Compute aggregate precision/recall across all languages
const langs = Object.values(latest.resolution);
if (langs.length > 0) {
const totalResolved = langs.reduce((s, l) => s + l.totalResolved, 0);
const totalExpected = langs.reduce((s, l) => s + l.totalExpected, 0);
const totalTP = langs.reduce((s, l) => s + l.truePositives, 0);
const aggPrecision = totalResolved > 0 ? `${((totalTP / totalResolved) * 100).toFixed(1)}%` : 'n/a';
const aggRecall = totalExpected > 0 ? `${((totalTP / totalExpected) * 100).toFixed(1)}%` : 'n/a';
rows += `| Resolution precision | **${aggPrecision}** |\n`;
rows += `| Resolution recall | **${aggRecall}** |\n`;
if (hasBoth) {
rows += `| Resolution precision | **${aggPrecision}** | — |\n`;
rows += `| Resolution recall | **${aggRecall}** | — |\n`;
} else {
rows += `| Resolution precision | **${aggPrecision}** |\n`;
rows += `| Resolution recall | **${aggRecall}** |\n`;
}
}
}

const tableHeader = hasBoth
? `| Metric | Native | WASM |\n|---|---|---|`
: `| Metric | Latest |\n|---|---|`;

const perfSection = `## 📊 Performance

Self-measured on every release via CI (${benchmarkLinks}):

*Last updated: v${latest.version} (${latest.date})*

| Metric | Latest |
|---|---|
${tableHeader}
${rows}
Metrics are normalized per file for cross-version comparability. Times above are for a full initial build — incremental rebuilds only re-parse changed files.
`;
Expand Down
Loading