feat: optimize e2e test matrix generation#769
Closed
magne4000 wants to merge 9 commits into
Closed
Conversation
Extract the canonical combo flatten/dedupe out of runner.ts into e2e/combos.ts (Combo, comboKey, buildCombos) so the new diff harness and the runner share one source of truth instead of duplicating the flattening logic. Add matrix-diff.local.spec.ts: it flattens the current matrix and diffs it against a stubbed generateMatrix() (returns [] until Phase 3), reporting the missing/extra deltas and reserving the cutover gate as a test.todo. Baseline: 106 current combos. Also add MATRIX-GRAPH-PLAN.md, a temporary working tracker for this effort (removed once implementation completes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(Phase 1a) batiExtract is the read-only twin of batiCodemod: it walks a boilerplate file and collects every feature reference without evaluating it — $$ directive comments, BATI.has(...) calls, BATI.hasX getter reads, and $$.If<> type-map keys — via codegraft's grammar-accurate find/findComments. No namespace gate: $-generator files reference features through real meta.BATI.has with no $$ marker, so a $$ scan-gate would skip them. parse/extract.ts exposes extractReferences(code, filepath), caching one transformer per target. Collected strings stay raw; @batijs/graft-graph's resolve mines them for flags in a later step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New @batijs/graft-graph builds the feature-interaction graph in memory from the
boilerplates — the input the E2E matrix generator will cluster.
- resolve.ts: a raw reference ("$$.BATI.has(\"x\")", a getter, a $$.If key) →
the feature flags it names. Getter→flags is derived by probing BatiSet itself
rather than restating its logic, so a new getter is covered automatically.
- owners.ts: a boilerplate's owners are the flags its bati.config.ts if()
predicate tests, read off config.if.toString() and run through resolve — works
for any conjunction arity (e.g. D1 + Kysely) and reuses resolve.
- index.ts buildGraph(): per file, an edge clique over owners ∪ referenced — a
file's logic ties its owner(s) to every feature it branches on. perFile keeps
provenance for the matrix's "why" trace.
Conditional edges only: co-write (features merging into one file) was measured
and dropped — its 11 co-write-only edges were all spurious, because features
that genuinely interact reference each other via $$/BATI.has.
Exports toDist from @batijs/build (reused, not duplicated). Graph over the real
boilerplates: 38 flags, 318 edges, 93 interaction files.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
render.ts holds the pure renderers — toText (adjacency), toJson, toDot (nodes boxed into per-category subgraph clusters so the picture shows which axes interact), and toSvg (DOT → SVG via lazy @viz-js/viz WASM, no system graphviz). cli.ts is the only I/O shell: bati-graph --format text|json|dot|svg [--out file]. @viz-js/viz loads only when SVG is requested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
generate.ts derives the backend cluster from the interaction graph (category edges ≥ INTERACTION_WEIGHT_MIN, excluding the declared ride-alongs framework / linter / hosting → largest component = server/data/db/orm/auth), then builds a constraint-aware pairwise covering array over it: enumerate the valid space (cross product filtered by feature rules), then greedily t=2-cover only valid combos. This avoids the coverage holes a flat pairwise + post-hoc drop leaves (orm-without-db rows getting dropped). Framework rides along balanced; linters are constant; kind comes from the shared inferKind. verify.ts holds the test-semantics metadata the graph can't infer — requiresEnv (auth0 → TEST_AUTH0_CLIENT_ID), sync-guarded against the feature list. Supporting: - extract comboErrors from Suite.flatten (shared validity, no duplicate rule-map) - extract inferKind into combos.ts (shared with the runner) - matrix-diff.local.spec.ts wires the real generator + asserts no coverage holes 16 backend combos, full t=2 coverage. matrix.ts is untouched — the cutover (Phase 5) stays gated on reviewing the generated-vs-current delta. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
matrix.ts is now the single Combo[] source: the backend core (server / data / db / orm / auth on the home deploy) comes from generateMatrix(), and a `residue` of suites holds everything the generator doesn't produce — the peripheral feature suites (UI / CSS / analytics / sentry / storybook / prettier / aws / skills / linter-strip) and the deploy-specific backend variants (Cloudflare D1, dokploy, auth0 / better-auth cloud) extracted from the old data/auth suites. buildCombos is gone — its flatten+dedup folded into matrix.ts (which also breaks the combos↔matrix import cycle); runner.ts consumes the matrix directly. The migration-only matrix-diff spec becomes matrix.local.spec.ts: no duplicate combos, and the backend core covers every satisfiable interaction pair (t=2). 73 combos (was 106): the 40 pairwise-redundant home backend combos drop for the same t=2 coverage; deploy-specifics are preserved. A full test:e2e run is the remaining validation before merge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Split one-line nested for-loops, replace assign-in-expression get-or-create helpers with explicit statements, and give value-returning forEach callbacks void block bodies. No behavior change; check-types + tests green across core, graft-graph, tests-utils, and tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implementation (Phases 0–5) is complete and the final 8-principle self-review passed; the plan was a temporary tracker and is removed as it instructed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
shadcn-ui was the one enabled, CLI-visible feature absent from every matrix combo (in both the old and new matrices — a pre-existing gap). Add it to the UI libraries suite alongside compiled-css and mantine; the CLI resolves its dependsOn (tailwindcss + react) at scaffold, mirroring how compiled-css is already handled. 74 combos; build/lint/typecheck coverage for the shadcn output. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughIntroduces a new ChangesFeature interaction graph and dynamic e2e matrix
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/graft-graph/src/cli.ts`:
- Line 13: The CLI flow in cli.ts builds the graph before validating
values.format, so unsupported formats still trigger a full scan. Move the format
validation ahead of buildGraph() and render so invalid --format values fail
fast; use the existing CLI entrypoint and render/buildGraph path to gate
execution on a supported format list before calling buildGraph().
In `@packages/graft-graph/src/index.ts`:
- Around line 42-44: The per-file filtering in the `byDest` loop is using
`flags.owners.size + flags.referenced.size`, which can count the same flag twice
and let single-flag files into `perFile`. Update the `perFile` population logic
in `index.ts` to base the threshold on the number of unique flags participating
in the file, using the `owners` and `referenced` sets together rather than
summing their sizes, so only true multi-flag interactions are included.
In `@packages/graft-graph/src/owners.ts`:
- Around line 11-13: The config load in ownersOf currently imports
bati.config.ts directly, which breaks when running the bati-graph binary under
plain Node. Update ownersOf to resolve and import the boilerplate config through
a JavaScript entrypoint instead of the TypeScript source, and keep the existing
BatiConfig handling and config.if guard intact so the loader works without
requiring a TypeScript runtime.
In `@packages/graft-graph/src/resolve.ts`:
- Around line 33-36: resolveFlags currently accepts any has("...") literal and
silently ignores unknown hasX-style matches, which can create bogus or missing
graph edges. Update resolveFlags to validate BATI predicate names by checking
matches from FLAG_CALL/GETTER against the known allowed set and explicitly
rejecting or reporting unknown names instead of dropping them. Keep the fix
localized in resolveFlags and ensure buildGraph only receives validated flags.
In `@packages/tests/e2e/generate.ts`:
- Around line 28-40: backendValidCombos() should fail fast when
backendCategories(await buildGraph()) produces no categories instead of letting
crossProduct([]) yield an empty combo set that later collapses in
coverPairwise(); add an explicit empty-axes/no-combos check in
backendValidCombos and throw a descriptive error so upstream graph or threshold
regressions are surfaced immediately. Keep the fix localized to
backendValidCombos and preserve the existing axisByCategory/envAvailable
combo-building logic for non-empty graphs.
In `@packages/tests/e2e/matrix.ts`:
- Around line 115-133: The E2E matrix in buildMatrix is being generated
indirectly, so the exact backend combos are no longer explicit in matrix.ts.
Replace the generateMatrix() flow with a fully declared include-only matrix
using the existing suite() builders, and enumerate the desired combos directly
in matrix.ts via .matrix(...) and/or .case(...). Keep buildMatrix/combos
deduping only if needed for residue merging, but ensure the backend matrix
itself is visibly defined in this file rather than derived from graph
extraction.
- Around line 12-13: The matrix generation path is still using a direct
TEST_AUTH0_CLIENT_ID check, which can diverge from the env-gated rules used
elsewhere. Update the auth selection in matrix.ts to use envAvailable("auth0")
like generate.ts does, so the auth0 combo is only emitted when the full required
env set is present. Keep the change localized to the auths construction logic so
both core and residue generation follow the same availability source.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 97eebc3d-f9b2-4d1e-b97d-d91915d8ba04
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
packages/build/src/index.tspackages/core/src/codemods/bati-extract.test.tspackages/core/src/codemods/bati-extract.tspackages/core/src/codemods/index.tspackages/core/src/index.tspackages/core/src/parse/extract.tspackages/graft-graph/package.jsonpackages/graft-graph/src/cli.tspackages/graft-graph/src/graph.test.tspackages/graft-graph/src/index.tspackages/graft-graph/src/owners.tspackages/graft-graph/src/render.tspackages/graft-graph/src/resolve.test.tspackages/graft-graph/src/resolve.tspackages/graft-graph/tsconfig.jsonpackages/graft-graph/tsdown.config.tspackages/tests-utils/src/suite.tspackages/tests/e2e/combos.tspackages/tests/e2e/generate.tspackages/tests/e2e/matrix.tspackages/tests/e2e/runner.tspackages/tests/e2e/verify.tspackages/tests/matrix.local.spec.tspackages/tests/package.jsonpackages/tests/verify.local.spec.ts
Comment on lines
+1
to
+15
| import { writeFile } from "node:fs/promises"; | ||
| import { parseArgs } from "node:util"; | ||
| import { buildGraph } from "./index.js"; | ||
| import { toDot, toJson, toSvg, toText } from "./render.js"; | ||
|
|
||
| const { values } = parseArgs({ | ||
| options: { | ||
| format: { type: "string", default: "text" }, // text | json | dot | svg | ||
| out: { type: "string" }, // write to this file instead of stdout | ||
| }, | ||
| }); | ||
|
|
||
| const rendered = await render(await buildGraph(), values.format!); | ||
| if (values.out) await writeFile(values.out, rendered); | ||
| else process.stdout.write(rendered); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspecting CLI source and package/bin wiring..."
sed -n '1,80p' packages/graft-graph/src/cli.ts
echo
sed -n '1,120p' packages/graft-graph/package.json
echo
sed -n '1,160p' packages/graft-graph/tsdown.config.ts 2>/dev/null || true
echo
echo "Searching for any existing shebang/builder injection..."
rg -n '(^#!|shebang|banner)' packages/graft-graphRepository: vikejs/bati
Length of output: 2509
🌐 Web query:
tsdown shebang bin cli injection default banner shebang
💡 Result:
Tsdown is a library bundler powered by Rolldown that supports the creation of executable CLI tools primarily through its banner configuration. When building a CLI tool, developers typically use the banner option to inject a shebang into the bundled output [1][2][3]. Key aspects regarding tsdown, shebangs, and CLI tools include: 1. Shebang Injection: To make a bundled JavaScript file executable on systems like Linux, you configure the banner option in your tsdown.config.ts file [1][3]. This adds the necessary shebang line, such as #!/usr/bin/env node, to the top of the generated bundle [1][2][3]. Example configuration: banner: { js: '#!/usr/bin/env node' } [2][3] 2. Automatic bin Detection: Recent versions of tsdown (e.g., v0.22.0+) include an auto-detection feature for entry chunks containing a shebang [4][5]. If an entry chunk contains a shebang, tsdown automatically writes a corresponding bin field to package.json, simplifying the process of publishing executable CLI tools [4][5]. 3. CLI Injection: There is no documented native vulnerability or feature referred to as "CLI injection" within tsdown itself [6]. In the context of building CLI tools, the term is sometimes conflated with the process of "injecting" a banner/shebang into the build, or it may refer to standard security practices when developing CLI applications that handle user input (which is outside the scope of the bundler itself) [3][6]. 4. Comparison to tsup: Tsdown is designed as a seamless migration path from tsup [7][8]. While tsup and tsdown share similar goals, tsdown leverages Rolldown for faster performance and provides more modern, library-focused defaults [7][9][8].
Citations:
- 1: https://yamada-ui.com/docs/changelog/cli/2-0-2
- 2: https://github.com/47ng/prisma-field-encryption/blob/next/tsdown.config.mts
- 3: https://blog.gitcode.com/0e0d568f943d2955a304e9bfcf530334.html
- 4: https://github.com/rolldown/tsdown/releases
- 5: https://newreleases.io/project/github/rolldown/tsdown/release/v0.22.0
- 6: https://awesomeskill.ai/skill/antfu-skills-tsdown
- 7: https://alan.norbauer.com/articles/tsdown-bundler/
- 8: https://github.com/rolldown/tsdown/blob/main/CLAUDE.md
- 9: https://medium.com/@ramunarasinga/tsdown-the-elegant-bundler-for-libraries-powered-by-rolldown-bddee176882d
Add a Node shebang to the CLI entrypoint. packages/graft-graph/package.json exposes bati-graph from ./dist/cli.js, but packages/graft-graph/src/cli.ts has no #!/usr/bin/env node header and packages/graft-graph/tsdown.config.ts doesn’t inject one. Add the shebang in source or set banner.js so the published bin runs on Unix shells.
| }, | ||
| }); | ||
|
|
||
| const rendered = await render(await buildGraph(), values.format!); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Validate --format before building the graph.
Line 13 always runs buildGraph() first, so bati-graph --format nope still scans the full boilerplate tree before exiting. Fail fast on unsupported formats, then build/render only for valid ones.
Suggested change
+const format = values.format!;
+if (!["text", "json", "dot", "svg"].includes(format)) {
+ console.error(`unknown --format "${format}" (text | json | dot | svg)`);
+ process.exit(1);
+}
+
-const rendered = await render(await buildGraph(), values.format!);
+const rendered = await render(await buildGraph(), format);
if (values.out) await writeFile(values.out, rendered);
else process.stdout.write(rendered);
function render(graph: Awaited<ReturnType<typeof buildGraph>>, format: string): string | Promise<string> {
switch (format) {
@@
- default:
- console.error(`unknown --format "${format}" (text | json | dot | svg)`);
- return process.exit(1);
+ default:
+ throw new Error(`unreachable format: ${format}`);
}
}Also applies to: 17-30
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/graft-graph/src/cli.ts` at line 13, The CLI flow in cli.ts builds
the graph before validating values.format, so unsupported formats still trigger
a full scan. Move the format validation ahead of buildGraph() and render so
invalid --format values fail fast; use the existing CLI entrypoint and
render/buildGraph path to gate execution on a supported format list before
calling buildGraph().
Comment on lines
+42
to
+44
| for (const [dest, flags] of byDest) { | ||
| if (flags.owners.size + flags.referenced.size < 2) continue; | ||
| perFile[dest] = { owners: [...flags.owners].sort(), referenced: [...flags.referenced].sort() }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Use the unique flag count when filtering perFile.
If a file is owned by drizzle and also branches on drizzle, owners.size + referenced.size is 2 even though there is only one participating flag. That leaks single-flag files into perFile as if they explained an interaction.
Proposed fix
const perFile: InteractionGraph["perFile"] = {};
for (const [dest, flags] of byDest) {
- if (flags.owners.size + flags.referenced.size < 2) continue;
+ if (new Set([...flags.owners, ...flags.referenced]).size < 2) continue;
perFile[dest] = { owners: [...flags.owners].sort(), referenced: [...flags.referenced].sort() };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (const [dest, flags] of byDest) { | |
| if (flags.owners.size + flags.referenced.size < 2) continue; | |
| perFile[dest] = { owners: [...flags.owners].sort(), referenced: [...flags.referenced].sort() }; | |
| const perFile: InteractionGraph["perFile"] = {}; | |
| for (const [dest, flags] of byDest) { | |
| if (new Set([...flags.owners, ...flags.referenced]).size < 2) continue; | |
| perFile[dest] = { owners: [...flags.owners].sort(), referenced: [...flags.referenced].sort() }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/graft-graph/src/index.ts` around lines 42 - 44, The per-file
filtering in the `byDest` loop is using `flags.owners.size +
flags.referenced.size`, which can count the same flag twice and let single-flag
files into `perFile`. Update the `perFile` population logic in `index.ts` to
base the threshold on the number of unique flags participating in the file,
using the `owners` and `referenced` sets together rather than summing their
sizes, so only true multi-flag interactions are included.
Comment on lines
+11
to
+13
| export async function ownersOf(boilerplateDir: string): Promise<Set<string>> { | ||
| const config: BatiConfig = (await import(pathToFileURL(join(boilerplateDir, "bati.config.ts")).href)).default; | ||
| if (!config.if) return new Set(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== boilerplate config files =="
fd -a '^bati\.config\.(ts|js|mjs|cjs)$' boilerplates
echo
echo "== graft-graph entrypoints / scripts mentioning runtime loaders =="
rg -n -C2 --glob 'package.json' --glob '*.ts' --glob '*.js' 'tsx|ts-node|--loader|register' packages pnpm-lock.yaml . || true
echo
echo "== ownersOf import target =="
sed -n '1,40p' packages/graft-graph/src/owners.tsRepository: vikejs/bati
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/graft-graph/package.json =="
cat -n packages/graft-graph/package.json | sed -n '1,220p'
echo
echo "== packages/graft-graph/src/owners.ts =="
cat -n packages/graft-graph/src/owners.ts | sed -n '1,220p'
echo
echo "== root package scripts mentioning graft-graph / cli =="
rg -n -C2 '"graft-graph"|dist/cli\.js|packages/graft-graph|bati-graph|node .*cli|tsx .*cli' package.json packages/**/package.json .github -g 'package.json' || trueRepository: vikejs/bati
Length of output: 3260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/graft-graph/package.json =="
cat -n packages/graft-graph/package.json | sed -n '1,220p'
echo
echo "== packages/graft-graph/src/owners.ts =="
cat -n packages/graft-graph/src/owners.ts | sed -n '1,220p'
echo
echo "== root/package scripts mentioning graft-graph / cli =="
rg -n -C2 '"graft-graph"|dist/cli\.js|packages/graft-graph|bati-graph|node .*cli|tsx .*cli' package.json packages/**/package.json .github -g 'package.json' || trueRepository: vikejs/bati
Length of output: 3260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files in packages/graft-graph =="
git ls-files packages/graft-graph | sed -n '1,200p'
echo
echo "== usages of bati.config.ts =="
rg -n -C2 'bati\.config\.ts|pathToFileURL\(join\(|import\(.+bati\.config' packages boilerplates -g '!**/dist/**' || true
echo
echo "== tsdown config / build settings for graft-graph =="
fd -a 'tsdown.*' packages/graft-graph . || true
rg -n -C3 'tsdown|bundle|external|entry|format|platform|target|dts|splitting' packages/graft-graph -g '*.{ts,js,json}' || trueRepository: vikejs/bati
Length of output: 8040
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files in packages/graft-graph =="
git ls-files packages/graft-graph | sed -n '1,200p'
echo
echo "== usages of bati.config.ts =="
rg -n -C2 'bati\.config\.ts|pathToFileURL\(join\(|import\(.+bati\.config' packages boilerplates -g '!**/dist/**' || true
echo
echo "== tsdown config / build settings for graft-graph =="
fd -a 'tsdown.*' packages/graft-graph . || true
rg -n -C3 'tsdown|bundle|external|entry|format|platform|target|dts|splitting' packages/graft-graph -g '*.{ts,js,json}' || trueRepository: vikejs/bati
Length of output: 8040
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/graft-graph/tsdown.config.ts =="
cat -n packages/graft-graph/tsdown.config.ts | sed -n '1,220p'
echo
echo "== packages/graft-graph/src/index.ts =="
cat -n packages/graft-graph/src/index.ts | sed -n '1,220p'
echo
echo "== references to bati-graph / `@batijs/graft-graph` =="
rg -n -C2 'bati-graph|`@batijs/graft-graph`|graft-graph' . -g '!**/dist/**' -g '!**/node_modules/**' || trueRepository: vikejs/bati
Length of output: 7233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/graft-graph/tsdown.config.ts =="
cat -n packages/graft-graph/tsdown.config.ts | sed -n '1,220p'
echo
echo "== packages/graft-graph/src/index.ts =="
cat -n packages/graft-graph/src/index.ts | sed -n '1,220p'
echo
echo "== references to bati-graph / `@batijs/graft-graph` =="
rg -n -C2 'bati-graph|`@batijs/graft-graph`|graft-graph' . -g '!**/dist/**' -g '!**/node_modules/**' || trueRepository: vikejs/bati
Length of output: 7233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/graft-graph/tsdown.config.ts =="
cat -n packages/graft-graph/tsdown.config.ts | sed -n '1,220p'
echo
echo "== packages/graft-graph/src/index.ts =="
cat -n packages/graft-graph/src/index.ts | sed -n '1,220p'
echo
echo "== references to bati-graph / `@batijs/graft-graph` =="
rg -n -C2 'bati-graph|`@batijs/graft-graph`|graft-graph' . -g '!**/dist/**' -g '!**/node_modules/**' || trueRepository: vikejs/bati
Length of output: 7233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/graft-graph/tsdown.config.ts =="
cat -n packages/graft-graph/tsdown.config.ts | sed -n '1,220p'
echo
echo "== packages/graft-graph/src/index.ts =="
cat -n packages/graft-graph/src/index.ts | sed -n '1,220p'
echo
echo "== references to bati-graph / `@batijs/graft-graph` =="
rg -n -C2 'bati-graph|`@batijs/graft-graph`|graft-graph' . -g '!**/dist/**' -g '!**/node_modules/**' || trueRepository: vikejs/bati
Length of output: 7233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/compile/build.ts =="
cat -n packages/compile/build.ts | sed -n '1,220p'
echo
echo "== boilerplates directories containing bati.config.* =="
fd -a 'bati.config.*' boilerplates | sed -n '1,200p'
echo
echo "== any compiled config outputs in boilerplates dist =="
fd -a 'bati.config.*' boilerplates/**/dist 2>/dev/null | sed -n '1,200p' || trueRepository: vikejs/bati
Length of output: 9414
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/compile/build.ts =="
cat -n packages/compile/build.ts | sed -n '1,220p'
echo
echo "== boilerplates directories containing bati.config.* =="
fd -a 'bati.config.*' boilerplates | sed -n '1,200p'
echo
echo "== any compiled config outputs in boilerplates dist =="
fd -a 'bati.config.*' boilerplates/**/dist 2>/dev/null | sed -n '1,200p' || trueRepository: vikejs/bati
Length of output: 9414
Import boilerplate configs through a JS entrypoint. packages/graft-graph/src/owners.ts:12 loads boilerplates/*/bati.config.ts directly, but there are no compiled config copies under boilerplates/. The bati-graph binary will fail under plain Node unless a TypeScript loader is always present.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/graft-graph/src/owners.ts` around lines 11 - 13, The config load in
ownersOf currently imports bati.config.ts directly, which breaks when running
the bati-graph binary under plain Node. Update ownersOf to resolve and import
the boilerplate config through a JavaScript entrypoint instead of the TypeScript
source, and keep the existing BatiConfig handling and config.if guard intact so
the loader works without requiring a TypeScript runtime.
Comment on lines
+33
to
+36
| export function resolveFlags(ref: string): Set<string> { | ||
| const out = new Set<string>(); | ||
| for (const m of ref.matchAll(FLAG_CALL)) out.add(m[1]); | ||
| for (const m of ref.matchAll(GETTER)) for (const f of getterFlags.get(m[0]) ?? []) out.add(f); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate BATI names instead of silently accepting/dropping them.
Lines 35-36 currently add any has("...") literal and ignore unknown hasX... matches. A typo in a boilerplate predicate or extracted ref will therefore create bogus graph nodes or miss real edges, which then changes buildGraph() and the generated matrix.
Proposed fix
export function resolveFlags(ref: string): Set<string> {
const out = new Set<string>();
- for (const m of ref.matchAll(FLAG_CALL)) out.add(m[1]);
- for (const m of ref.matchAll(GETTER)) for (const f of getterFlags.get(m[0]) ?? []) out.add(f);
+ for (const m of ref.matchAll(FLAG_CALL)) {
+ const flag = m[1] as Flags;
+ if (!flags.includes(flag)) throw new Error(`Unknown BATI flag: ${m[1]}`);
+ out.add(flag);
+ }
+ for (const m of ref.matchAll(GETTER)) {
+ const deps = getterFlags.get(m[0]);
+ if (!deps) throw new Error(`Unknown BATI getter: ${m[0]}`);
+ for (const f of deps) out.add(f);
+ }
return out;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function resolveFlags(ref: string): Set<string> { | |
| const out = new Set<string>(); | |
| for (const m of ref.matchAll(FLAG_CALL)) out.add(m[1]); | |
| for (const m of ref.matchAll(GETTER)) for (const f of getterFlags.get(m[0]) ?? []) out.add(f); | |
| export function resolveFlags(ref: string): Set<string> { | |
| const out = new Set<string>(); | |
| for (const m of ref.matchAll(FLAG_CALL)) { | |
| const flag = m[1] as Flags; | |
| if (!flags.includes(flag)) throw new Error(`Unknown BATI flag: ${m[1]}`); | |
| out.add(flag); | |
| } | |
| for (const m of ref.matchAll(GETTER)) { | |
| const deps = getterFlags.get(m[0]); | |
| if (!deps) throw new Error(`Unknown BATI getter: ${m[0]}`); | |
| for (const f of deps) out.add(f); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/graft-graph/src/resolve.ts` around lines 33 - 36, resolveFlags
currently accepts any has("...") literal and silently ignores unknown hasX-style
matches, which can create bogus or missing graph edges. Update resolveFlags to
validate BATI predicate names by checking matches from FLAG_CALL/GETTER against
the known allowed set and explicitly rejecting or reporting unknown names
instead of dropping them. Keep the fix localized in resolveFlags and ensure
buildGraph only receives validated flags.
Comment on lines
+28
to
+40
| export async function backendValidCombos(): Promise<string[][]> { | ||
| const axes: (string | null)[][] = backendCategories(await buildGraph()).map((category) => { | ||
| const axis = axisByCategory[category as keyof typeof axisByCategory]; | ||
| if (!axis) throw new Error(`backend category has no axis: ${category}`); | ||
| // Server anchors every backend combo (all others require it); the rest are optional. Env-gated | ||
| // values (auth0) drop when their credentials are absent. | ||
| const values = axis.values.filter(envAvailable); | ||
| return axis.name === "server" ? values : [...values, null]; | ||
| }); | ||
|
|
||
| return crossProduct(axes) | ||
| .map((row) => row.filter((v): v is string => v !== null)) | ||
| .filter((flags) => comboErrors(flags).length === 0); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail fast when the graph yields no backend matrix.
If backendCategories() ever returns [], crossProduct([]) becomes [[]], that survives this filter, and coverPairwise() later collapses it to []. The generator then emits no backend combos at all instead of surfacing the upstream graph/threshold regression.
Suggested fix
export async function backendValidCombos(): Promise<string[][]> {
- const axes: (string | null)[][] = backendCategories(await buildGraph()).map((category) => {
+ const categories = backendCategories(await buildGraph());
+ if (categories.length === 0 || !categories.includes("Server")) {
+ throw new Error("backend graph did not produce a usable backend cluster");
+ }
+
+ const axes: (string | null)[][] = categories.map((category) => {
const axis = axisByCategory[category as keyof typeof axisByCategory];
if (!axis) throw new Error(`backend category has no axis: ${category}`);
// Server anchors every backend combo (all others require it); the rest are optional. Env-gated
// values (auth0) drop when their credentials are absent.
const values = axis.values.filter(envAvailable);
return axis.name === "server" ? values : [...values, null];
});
- return crossProduct(axes)
+ const combos = crossProduct(axes)
.map((row) => row.filter((v): v is string => v !== null))
.filter((flags) => comboErrors(flags).length === 0);
+
+ if (combos.length === 0) {
+ throw new Error("backend graph produced no valid backend combos");
+ }
+ return combos;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export async function backendValidCombos(): Promise<string[][]> { | |
| const axes: (string | null)[][] = backendCategories(await buildGraph()).map((category) => { | |
| const axis = axisByCategory[category as keyof typeof axisByCategory]; | |
| if (!axis) throw new Error(`backend category has no axis: ${category}`); | |
| // Server anchors every backend combo (all others require it); the rest are optional. Env-gated | |
| // values (auth0) drop when their credentials are absent. | |
| const values = axis.values.filter(envAvailable); | |
| return axis.name === "server" ? values : [...values, null]; | |
| }); | |
| return crossProduct(axes) | |
| .map((row) => row.filter((v): v is string => v !== null)) | |
| .filter((flags) => comboErrors(flags).length === 0); | |
| export async function backendValidCombos(): Promise<string[][]> { | |
| const categories = backendCategories(await buildGraph()); | |
| if (categories.length === 0 || !categories.includes("Server")) { | |
| throw new Error("backend graph did not produce a usable backend cluster"); | |
| } | |
| const axes: (string | null)[][] = categories.map((category) => { | |
| const axis = axisByCategory[category as keyof typeof axisByCategory]; | |
| if (!axis) throw new Error(`backend category has no axis: ${category}`); | |
| // Server anchors every backend combo (all others require it); the rest are optional. Env-gated | |
| // values (auth0) drop when their credentials are absent. | |
| const values = axis.values.filter(envAvailable); | |
| return axis.name === "server" ? values : [...values, null]; | |
| }); | |
| const combos = crossProduct(axes) | |
| .map((row) => row.filter((v): v is string => v !== null)) | |
| .filter((flags) => comboErrors(flags).length === 0); | |
| if (combos.length === 0) { | |
| throw new Error("backend graph produced no valid backend combos"); | |
| } | |
| return combos; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tests/e2e/generate.ts` around lines 28 - 40, backendValidCombos()
should fail fast when backendCategories(await buildGraph()) produces no
categories instead of letting crossProduct([]) yield an empty combo set that
later collapses in coverPairwise(); add an explicit empty-axes/no-combos check
in backendValidCombos and throw a descriptive error so upstream graph or
threshold regressions are surfaced immediately. Keep the fix localized to
backendValidCombos and preserve the existing axisByCategory/envAvailable
combo-building logic for non-empty graphs.
Comment on lines
12
to
13
| // auth0 only when its credentials are present (CI provides them). | ||
| const auths = ["authjs", ...(process.env.TEST_AUTH0_CLIENT_ID ? ["auth0"] : [])]; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use envAvailable("auth0") here too.
generate.ts already gates env-backed flags through envAvailable(...), but this residue path only checks TEST_AUTH0_CLIENT_ID. That can still emit auth0 combos when the full requiresEnv set is incomplete, so the generated core and residue follow different availability rules and the matrix can schedule impossible suites.
Suggested fix
+import { envAvailable } from "./verify.js";
...
-const auths = ["authjs", ...(process.env.TEST_AUTH0_CLIENT_ID ? ["auth0"] : [])];
+const auths = ["authjs", ...(envAvailable("auth0") ? ["auth0"] : [])];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // auth0 only when its credentials are present (CI provides them). | |
| const auths = ["authjs", ...(process.env.TEST_AUTH0_CLIENT_ID ? ["auth0"] : [])]; | |
| import { envAvailable } from "./verify.js"; | |
| // auth0 only when its credentials are present (CI provides them). | |
| const auths = ["authjs", ...(envAvailable("auth0") ? ["auth0"] : [])]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tests/e2e/matrix.ts` around lines 12 - 13, The matrix generation
path is still using a direct TEST_AUTH0_CLIENT_ID check, which can diverge from
the env-gated rules used elsewhere. Update the auth selection in matrix.ts to
use envAvailable("auth0") like generate.ts does, so the auth0 combo is only
emitted when the full required env set is present. Keep the change localized to
the auths construction logic so both core and residue generation follow the same
availability source.
Comment on lines
+115
to
+133
| export default buildMatrix(await generateMatrix()); | ||
|
|
||
| // Generated backend core first, then the residue suites, deduped — the generator and a hand suite can | ||
| // land on the same combo. | ||
| function buildMatrix(generated: Combo[]): Combo[] { | ||
| const balancer = new Balancer(); | ||
| const seen = new Set<string>(); | ||
| const combos: Combo[] = []; | ||
| const add = (combo: Combo) => { | ||
| const key = comboKey(combo); | ||
| if (seen.has(key)) return; | ||
| seen.add(key); | ||
| combos.push(combo); | ||
| }; | ||
|
|
||
| for (const combo of generated) add(combo); | ||
| for (const s of residue) | ||
| for (const flags of s.flatten(balancer)) add({ flags, mode: s.runMode ?? "dev", kind: s.suiteKind }); | ||
| return combos; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Keep the backend matrix explicitly declared in matrix.ts.
buildMatrix(await generateMatrix()) means the exercised E2E combos now change whenever graph extraction changes elsewhere, so reviewers can no longer inspect the exact matrix from this file. That is the opposite of the repo rule for packages/tests/e2e/matrix.ts.
As per coding guidelines, packages/tests/e2e/matrix.ts: Declare E2E flag combinations in matrix.ts using the include-only suite() builders (.matrix(...) / .case(...)) and enumerate the exact combos you want instead of using exclusion-based matrices.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tests/e2e/matrix.ts` around lines 115 - 133, The E2E matrix in
buildMatrix is being generated indirectly, so the exact backend combos are no
longer explicit in matrix.ts. Replace the generateMatrix() flow with a fully
declared include-only matrix using the existing suite() builders, and enumerate
the desired combos directly in matrix.ts via .matrix(...) and/or .case(...).
Keep buildMatrix/combos deduping only if needed for residue merging, but ensure
the backend matrix itself is visibly defined in this file rather than derived
from graph extraction.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes