feat: Add FlowBench kit — automated testing & benchmarking for Lamati…#229
feat: Add FlowBench kit — automated testing & benchmarking for Lamati…#229bharath-avl wants to merge 5 commits into
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughChangesFlowBench adds Lamatic flow execution, local embedding scoring, baseline comparison, filesystem persistence, a Next.js dashboard, a demo flow kit, sample tests, and documentation. FlowBench execution and reporting
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
:robot_face: AgentKit Structural ValidationNew Contributions Detected
Check Results
🎉 All checks passed! This contribution follows the AgentKit structure. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kits/flowbench/apps/app/globals.css (1)
1-6: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftThe TODO admits it: no CSS variables have entered the building.
Kit apps are required to use CSS variables for styling, but this file defines none — and the TODO on Line 5 confirms it's deferred. Downstream,
layout.tsx(bg-slate-950,text-slate-100) andRunForm.tsx(bg-cyan-600,border-red-500/20, etc.) hardcode Tailwind palette classes directly rather than referencing variables defined here, so this is the root cause to close out before those files can be brought into compliance.🤖 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 `@kits/flowbench/apps/app/globals.css` around lines 1 - 6, Replace the placeholder TODO in globals.css with the FlowBench styling CSS custom properties required by the kit-app convention, including the variables needed by layout.tsx and RunForm.tsx. Define the variables in the appropriate global scope and preserve the Tailwind directives; do not leave the styling dependent solely on hardcoded palette classes.Source: Coding guidelines
🤖 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 `@kits/flowbench/apps/__tests__/compareBaseline.test.ts`:
- Around line 221-236: Extend the compareBaseline test suite with coverage for
both untested result paths: add a case present in the baseline but absent from
the current run and assert it appears in removedCases, and add a matching case
whose latency or similarity improves and assert it appears in improvements.
Reuse makeRun and makeCase, and verify the relevant case identifier and
improvement data while preserving existing regression tests.
In `@kits/flowbench/apps/actions/orchestrate.ts`:
- Around line 28-56: Wrap the per-case scoreOutput invocation in the test-case
loop with error handling so an exception only marks that case as failed and
records the error, while allowing subsequent cases and the existing
saveRun/saveBaseline flow to continue. Update the case result’s error field with
the scoring failure and preserve normal similarity and pass handling when
scoring succeeds.
- Around line 35-38: Update the TestCaseInput definition used by runFlow.ts to
declare the expected_contains and min_similarity fields with their correct
types, then access them directly in orchestrate.ts instead of casting spec to
any. Preserve the existing default of 0.7 for min_similarity and the current
scoring behavior.
- Around line 1-19: Update executeBenchmark to resolve the benchmark flow ID
from the kit’s step definition in ../../lamatic.config rather than accepting an
arbitrary flowId parameter. Remove the caller-supplied flowId dependency and
pass the configured flow identifier through the existing runFlow/orchestration
path, preserving the current benchmark behavior for testCases.
In `@kits/flowbench/apps/actions/runFlow.ts`:
- Around line 51-52: Update the concurrency limiter in the flow execution path
around pLimit, including the logic at lines 167-180, to use the existing
CONCURRENCY_LIMIT constant instead of the hardcoded value 1. Preserve the
current request scheduling behavior while allowing up to three concurrent
Lamatic API requests.
- Around line 132-149: Update the result-processing path in the per-test-case
handler around coerceToString so serialization failures cannot escape the
documented “never throws” boundary. Catch errors from coerceToString (including
JSON.stringify failures) and return the same case-shaped result with its id,
latencyMs, null output, and a descriptive error instead of rejecting
Promise.all; preserve existing handling for null results and successful
coercion.
In `@kits/flowbench/apps/actions/scoreOutput.ts`:
- Around line 1-4: Update the scoreOutput module’s offline description to state
that execution is offline only after the initial Xenova/all-MiniLM-L6-v2 model
download, or configure Transformers.js to use local models by setting
env.allowRemoteModels to false and env.localModelPath for air-gapped operation.
- Around line 20-35: Update getEmbeddingPipeline so model loading cannot fetch
Xenova/all-MiniLM-L6-v2 remotely: configure the transformers pipeline to use a
bundled/local model path or disable remote model access, and ensure the required
model is preloaded locally while preserving the existing pipeline cache
behavior.
In `@kits/flowbench/apps/actions/storage.ts`:
- Around line 17-19: Sanitize and validate the external flowId/runId values
before constructing filesystem paths in getBaselinePath, getRunPath,
loadBaseline, saveBaseline, loadRun, and saveRun. Reject traversal or
absolute-path input and ensure each resolved target remains within BASELINES_DIR
or RUNS_DIR; leave listAllRuns unchanged because its filenames come from the
trusted directory listing.
In `@kits/flowbench/apps/app/globals.css`:
- Around line 1-3: Update the shared/root Stylelint configuration responsible
for globals.css files: replace the SCSS-specific rule reference
scss/at-rule-no-unknown with the syntax-appropriate at-rule-no-unknown
configuration, or configure the rule to ignore Tailwind at-rules such as
`@tailwind`. Ensure the change applies across all kits without modifying
globals.css.
In `@kits/flowbench/apps/app/page.tsx`:
- Around line 35-41: Guard the pass-rate calculation in the run summary, using
the existing totalCount and passCount values so a zero-case run renders a valid
0% result instead of NaN%. Update the percentage expression around the pass-rate
rendering near the avgLatency calculation; leave the latency averaging behavior
unchanged.
In `@kits/flowbench/apps/app/report/`[runId]/page.tsx:
- Around line 19-26: Update the report page’s pass-rate calculation to guard
against zero cases and return the established empty-run value instead of
producing NaN. In the P50 calculation near sortedLatencies, filter out failed
cases and their design-time zero latency before sorting, while preserving the
existing zero fallback when no valid latencies remain.
In `@kits/flowbench/apps/components/RunForm.tsx`:
- Around line 9-138: Refactor RunForm to use react-hook-form with a zod schema
for flowId and JSONL file validation, moving submission and validation out of
the raw useState/manual JSON.parse flow in handleRun. Replace the hand-styled
form controls with the project’s shadcn/ui components, while preserving
benchmark execution, loading/error handling, and report navigation behavior.
- Around line 85-108: Update the file upload dropzone around fileInputRef and
the file input in RunForm so it is keyboard accessible: use a semantic
interactive control or add the appropriate role, tabIndex, and keyboard handling
to activate the hidden file input with Enter and Space, while preserving the
existing click behavior and file selection flow.
In `@kits/flowbench/apps/lib/lamatic-client.ts`:
- Around line 158-165: Update the generated ExecuteWorkflow GraphQL selection
set in the query construction to request requestId alongside status and result,
so the response mapped to workflowResult and the declared
ExecuteWorkflowApiResult/ExecuteFlowResult fields contain the actual request ID.
- Around line 142-168: Update buildQuery to validate every payload key as a
legal GraphQL identifier before interpolating it into varDefs or payloadFields,
rejecting invalid keys such as those containing spaces, hyphens, braces, colons,
or selection syntax. Preserve valid keys and existing variable construction, and
fail before sending a query when validation fails.
- Around line 197-239: Document that executeFlow uses the native fetch-based
client and must remain compatible with the AbortSignal cancellation passed
through options?.signal from runFlow.ts. Add a brief note near executeFlow or in
the README explicitly preserving this fetch path instead of replacing it with
the SDK.
In `@kits/flowbench/apps/lib/metrics.ts`:
- Around line 55-73: Unify regression classification by updating
compareBaseline.ts to use the exported isLatencyRegression and
isSimilarityRegression helpers from metrics.ts instead of its inline 60%/0.2
thresholds. Preserve the helpers as the single source of truth for latency and
similarity regression behavior, and remove any duplicated threshold logic.
In `@kits/flowbench/apps/package.json`:
- Around line 17-27: Update the form implementation centered on RunForm.tsx to
use the required shadcn/ui components and react-hook-form with zod validation,
adding react-hook-form and zod to the app dependencies. Do not add shadcn/ui as
a direct package dependency; use the existing clsx and tailwind-merge setup with
generated shadcn/ui components or configuration.
- Line 36: Upgrade the Tailwind CSS dependency in package.json to v4+, add the
corresponding `@tailwindcss/postcss` package, update postcss.config.mjs to use the
v4 PostCSS plugin, and replace the v3 `@tailwind` directives in globals.css with
the v4 `@import` "tailwindcss" entry.
In `@kits/flowbench/constitutions/default.md`:
- Around line 3-17: Update the template or source that generates the
constitutions under kits/*/constitutions/default.md to add blank lines around
the Identity, Safety, Data Handling, and Tone headings, rather than editing this
checked-in generated file directly; ensure future generated kits inherit the
MD022-compliant formatting.
---
Outside diff comments:
In `@kits/flowbench/apps/app/globals.css`:
- Around line 1-6: Replace the placeholder TODO in globals.css with the
FlowBench styling CSS custom properties required by the kit-app convention,
including the variables needed by layout.tsx and RunForm.tsx. Define the
variables in the appropriate global scope and preserve the Tailwind directives;
do not leave the styling dependent solely on hardcoded palette classes.
🪄 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 (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4d30425d-efac-4860-af74-8eae13bda5f1
⛔ Files ignored due to path filters (2)
kits/flowbench/apps/package-lock.jsonis excluded by!**/package-lock.jsonkits/flowbench/assets/dashboard.pngis excluded by!**/*.png
📒 Files selected for processing (30)
kits/flowbench/.gitignorekits/flowbench/README.mdkits/flowbench/agent.mdkits/flowbench/apps/.env.examplekits/flowbench/apps/.gitignorekits/flowbench/apps/__tests__/compareBaseline.test.tskits/flowbench/apps/actions/compareBaseline.tskits/flowbench/apps/actions/orchestrate.tskits/flowbench/apps/actions/runFlow.tskits/flowbench/apps/actions/scoreOutput.tskits/flowbench/apps/actions/storage.tskits/flowbench/apps/app/globals.csskits/flowbench/apps/app/layout.tsxkits/flowbench/apps/app/page.tsxkits/flowbench/apps/app/report/[runId]/page.tsxkits/flowbench/apps/components/RunForm.tsxkits/flowbench/apps/lib/lamatic-client.tskits/flowbench/apps/lib/metrics.tskits/flowbench/apps/next.config.mjskits/flowbench/apps/package.jsonkits/flowbench/apps/postcss.config.mjskits/flowbench/apps/tailwind.config.tskits/flowbench/apps/tsconfig.jsonkits/flowbench/constitutions/default.mdkits/flowbench/flows/flowbench-demo-flow.tskits/flowbench/lamatic.config.tskits/flowbench/model-configs/flowbench-demo-flow_llm-node_generative-model-name.tskits/flowbench/prompts/flowbench-demo-flow_llm-node_system_0.mdkits/flowbench/prompts/flowbench-demo-flow_llm-node_user_1.mdkits/flowbench/sample-tests/example.jsonl
| "use server"; | ||
|
|
||
| import { runFlow, type TestCaseInput } from "./runFlow"; | ||
| import { scoreOutput } from "./scoreOutput"; | ||
| import { compareBaseline, type RunResult, type CompareResult, type CaseResult } from "./compareBaseline"; | ||
| import { generateRunId } from "../lib/metrics"; | ||
| import { loadBaseline, saveBaseline, saveRun } from "./storage"; | ||
|
|
||
| export interface OrchestrationResult { | ||
| run: RunResult; | ||
| diff: CompareResult; | ||
| baselineUpdated: boolean; | ||
| error?: string; | ||
| } | ||
|
|
||
| export async function executeBenchmark( | ||
| flowId: string, | ||
| testCases: TestCaseInput[] | ||
| ): Promise<OrchestrationResult> { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'kits/flowbench/apps/actions/*' 'kits/flowbench/apps/*' 'kits/*/apps/actions/orchestrate.ts' 'kits/*/apps/lamatic.config.*' 'kits/*/apps/**/lamatic.config.*' | sed -n '1,200p'
printf '\n--- orchestrate.ts outline ---\n'
ast-grep outline kits/flowbench/apps/actions/orchestrate.ts --view expanded
printf '\n--- runFlow.ts outline ---\n'
ast-grep outline kits/flowbench/apps/actions/runFlow.ts --view expanded
printf '\n--- search for lamatic.config usage ---\n'
rg -n --hidden --glob '!*node_modules*' --glob '!*.map' 'lamatic\.config|flowId|executeBenchmark|runFlow' kits/flowbench/apps kits -SRepository: Lamatic/AgentKit
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- flowbench orchestrate.ts (numbered) ---\n'
sed -n '1,220p' kits/flowbench/apps/actions/orchestrate.ts | cat -n
printf '\n--- flowbench lamatic-client.ts (numbered, relevant slice) ---\n'
sed -n '1,260p' kits/flowbench/apps/lib/lamatic-client.ts | cat -n
printf '\n--- flowbench kit config presence ---\n'
git ls-files 'kits/flowbench/lamatic.config.*' 'kits/flowbench/**/lamatic.config.*' 'kits/flowbench/lamatic.config.*' | cat -n
printf '\n--- analogous benchmark/orchestrator patterns ---\n'
sed -n '1,220p' kits/llm-eval-harness/apps/actions/orchestrate.ts | cat -nRepository: Lamatic/AgentKit
Length of output: 18803
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- flowbench lamatic.config.ts ---\n'
sed -n '1,220p' kits/flowbench/lamatic.config.ts | cat -n
printf '\n--- flowbench app page / UI hint ---\n'
sed -n '1,220p' kits/flowbench/apps/app/page.tsx | cat -n
printf '\n--- flowbench package.json scripts/name ---\n'
sed -n '1,220p' kits/flowbench/apps/package.json | cat -nRepository: Lamatic/AgentKit
Length of output: 6996
Resolve the benchmark flow from ../../lamatic.config kits/flowbench/apps/actions/orchestrate.ts should derive the flow ID from the kit’s step definition instead of accepting an arbitrary caller-supplied flowId; otherwise this benchmark can run against a flow outside the kit’s configured deployment target.
🤖 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 `@kits/flowbench/apps/actions/orchestrate.ts` around lines 1 - 19, Update
executeBenchmark to resolve the benchmark flow ID from the kit’s step definition
in ../../lamatic.config rather than accepting an arbitrary flowId parameter.
Remove the caller-supplied flowId dependency and pass the configured flow
identifier through the existing runFlow/orchestration path, preserving the
current benchmark behavior for testCases.
Source: Coding guidelines
There was a problem hiding this comment.
This is intentional, not an oversight — FlowBench's whole purpose is testing arbitrary flow IDs a user provides at runtime through the dashboard, not just wrapping one fixed flow the way most kits do. The kit still ships a real demo flow (flowbench-demo-flow.ts) and honors the standard envKey pattern as a fallback when no runtime flowId is given — see the comment block at the top of executeFlow in lamatic-client.ts. Happy to make this distinction more prominent in the README if it's unclear from the code alone.
There was a problem hiding this comment.
@bharath-avl Mission understood, and accepted. Your explanation checks out under review — the dynamic flowId isn't a security gap, it's the operative objective. FlowBench's entire mandate is to let operators test arbitrary flow IDs supplied at runtime via the dashboard, unlike kits that are wired to a single fixed flow. The presence of flowbench-demo-flow.ts as a real demo target, plus the envKey fallback pattern in executeFlow (lamatic-client.ts) for when no runtime flowId is supplied, confirms this is a deliberate design choice rather than an oversight.
This message will self-destruct... no wait, it won't — but the concern is withdrawn. A short callout in the README distinguishing FlowBench's "test any flow at runtime" model from the typical "one kit, one flow" pattern would be a good addition for future reviewers, if you're inclined to add it.
| // Type-cast because TestCaseInput doesn't have min_similarity and expected_contains by default | ||
| // but we will pass them in from the UI parse step | ||
| const expectedContains = (spec as any).expected_contains; | ||
| const minSimilarity = (spec as any).min_similarity ?? 0.7; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Retire the as any escape hatch for test-case fields.
expected_contains/min_similarity are load-bearing (they gate scoring and pass/fail), but TestCaseInput (runFlow.ts) doesn't declare them — the cast silently defeats type-checking for typos/shape drift from the JSONL parse step.
♻️ Proposed fix
export interface TestCaseInput {
id: string;
input: Record<string, unknown>;
resultField: string;
+ expected_contains?: string;
+ min_similarity?: number;
}- const expectedContains = (spec as any).expected_contains;
- const minSimilarity = (spec as any).min_similarity ?? 0.7;
+ const expectedContains = spec.expected_contains;
+ const minSimilarity = spec.min_similarity ?? 0.7;🤖 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 `@kits/flowbench/apps/actions/orchestrate.ts` around lines 35 - 38, Update the
TestCaseInput definition used by runFlow.ts to declare the expected_contains and
min_similarity fields with their correct types, then access them directly in
orchestrate.ts instead of casting spec to any. Preserve the existing default of
0.7 for min_similarity and the current scoring behavior.
| export async function executeFlow( | ||
| flowId: string | undefined, | ||
| payload: Record<string, unknown>, | ||
| options?: ExecuteFlowOptions | ||
| ): Promise<ExecuteFlowResult> { | ||
| // ── 0. Resolve the Flow ID ── | ||
| // FlowBench is designed to test *any* arbitrary flow ID at runtime (e.g. passed from the UI | ||
| // or CLI arguments). However, to conform to the AgentKit "kit" structure, it ships with a | ||
| // demo flow defined in lamatic.config.ts, whose ID is provided via an environment variable | ||
| // specified by config.steps[0].envKey. We use this demo flow ID as a fallback if the user | ||
| // doesn't explicitly provide one. | ||
| const envKey = config.steps[0]?.envKey; | ||
| const demoFlowId = envKey ? process.env[envKey] : undefined; | ||
|
|
||
| const resolvedFlowId = flowId || demoFlowId; | ||
| if (!resolvedFlowId) { | ||
| throw new Error( | ||
| "No flow ID provided. You must either specify a flow ID at runtime " + | ||
| `or set the ${envKey || 'demo flow'} environment variable.` | ||
| ); | ||
| } | ||
|
|
||
| // ── 1. Validate env vars before making any network request ── | ||
| const apiUrl = requireEnv("LAMATIC_API_URL"); | ||
| const apiKey = requireEnv("LAMATIC_API_KEY"); | ||
| const projectId = requireEnv("LAMATIC_PROJECT_ID"); | ||
|
|
||
| // ── 2. Build the dynamic GraphQL query ── | ||
| const { query, variables } = buildQuery(resolvedFlowId, payload); | ||
|
|
||
| // ── 3. Execute with timing ── | ||
| const startTime = performance.now(); | ||
|
|
||
| const response = await fetch(apiUrl, { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| Authorization: `Bearer ${apiKey}`, | ||
| "x-project-id": projectId, | ||
| }, | ||
| body: JSON.stringify({ query, variables }), | ||
| signal: options?.signal, | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare against how other kits integrate the lamatic SDK in lamatic-client.ts
fd lamatic-client.ts kits
rg -n "from \"lamatic\"|require\(\"lamatic\"\)|Lamatic\.executeFlow" kits -g '*.ts'
cat kits/flowbench/apps/package.json 2>/dev/null | jq '.dependencies.lamatic // "not declared"'Repository: Lamatic/AgentKit
Length of output: 3351
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and inspect the relevant implementation details.
ast-grep outline kits/flowbench/apps/lib/lamatic-client.ts --view expanded || true
wc -l kits/flowbench/apps/lib/lamatic-client.ts
sed -n '1,320p' kits/flowbench/apps/lib/lamatic-client.tsRepository: Lamatic/AgentKit
Length of output: 10534
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the kit config and sibling client patterns for comparison.
cat -n kits/flowbench/apps/package.json 2>/dev/null | sed -n '1,220p'
printf '\n--- sibling lamatic client sample ---\n'
sed -n '1,220p' kits/generation/apps/lib/lamatic-client.tsRepository: Lamatic/AgentKit
Length of output: 2051
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant implementation sections.
sed -n '1,260p' kits/flowbench/apps/lib/lamatic-client.tsRepository: Lamatic/AgentKit
Length of output: 9072
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for existing docs/comments explaining the custom client or abort behavior.
rg -n "AbortSignal|abort|timeout|custom fetch|lamatic SDK|Lamatic SDK|executeFlow" kits/flowbench -g '*.md' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'Repository: Lamatic/AgentKit
Length of output: 1742
Document the fetch-based client FlowBench relies on AbortSignal timeout cancellation in runFlow.ts; add a short note here or in the README so this path doesn’t get swapped back to the SDK.
🤖 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 `@kits/flowbench/apps/lib/lamatic-client.ts` around lines 197 - 239, Document
that executeFlow uses the native fetch-based client and must remain compatible
with the AbortSignal cancellation passed through options?.signal from
runFlow.ts. Add a brief note near executeFlow or in the README explicitly
preserving this fetch path instead of replacing it with the SDK.
Source: Coding guidelines
| export function isLatencyRegression( | ||
| current: number, | ||
| baseline: number, | ||
| threshold: number = 1.2 | ||
| ): boolean { | ||
| return current > baseline * threshold; | ||
| } | ||
|
|
||
| /** | ||
| * Check if current similarity is a regression vs baseline. | ||
| * Default threshold: 0.1 (drop of >0.1 = regression). | ||
| */ | ||
| export function isSimilarityRegression( | ||
| current: number, | ||
| baseline: number, | ||
| threshold: number = 0.1 | ||
| ): boolean { | ||
| return (baseline - current) > threshold; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Two regression trip-wires, two different thresholds — pick a side.
isLatencyRegression/isSimilarityRegression here default to 20%/0.1, but compareBaseline.ts's own regression classification (per the snippet in the test file's context) hardcodes 60%/0.2 inline instead of calling these exports. That means either these two functions are unused dead code, or there are now two independent sources of truth for "what counts as a regression" that can silently drift out of sync.
#!/bin/bash
rg -n "isLatencyRegression|isSimilarityRegression|LATENCY_REGRESSION_THRESHOLD_PCT|SIMILARITY_REGRESSION_THRESHOLD" kits/flowbench/apps -g '*.ts'🤖 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 `@kits/flowbench/apps/lib/metrics.ts` around lines 55 - 73, Unify regression
classification by updating compareBaseline.ts to use the exported
isLatencyRegression and isSimilarityRegression helpers from metrics.ts instead
of its inline 60%/0.2 thresholds. Preserve the helpers as the single source of
truth for latency and similarity regression behavior, and remove any duplicated
threshold logic.
| "dependencies": { | ||
| "@xenova/transformers": "^2.17.2", | ||
| "clsx": "^2.1.0", | ||
| "dotenv": "^17.4.2", | ||
| "lucide-react": "^0.460.0", | ||
| "next": "^14.2.0", | ||
| "p-limit": "^3.1.0", | ||
| "react": "^18.3.0", | ||
| "react-dom": "^18.3.0", | ||
| "tailwind-merge": "^2.6.0" | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Missing shadcn/ui, react-hook-form, and zod prescribed by kit guidelines.
The guidelines require shadcn/ui components and react-hook-form + zod for forms. The stack includes a RunForm.tsx component, but none of these packages are listed. clsx and tailwind-merge are present (typically used with shadcn/ui's cn utility), suggesting partial setup — but the actual component library and form validation libraries are missing.
As per coding guidelines: kits/*/apps/**/*.{ts,tsx,js,jsx} — "use ... shadcn/ui components, react-hook-form + zod for forms ...".
♻️ Proposed additions
"dependencies": {
"`@xenova/transformers`": "^2.17.2",
+ "`@hookform/resolvers`": "^3.9.0",
"clsx": "^2.1.0",
"dotenv": "^17.4.2",
"lucide-react": "^0.460.0",
"next": "^14.2.0",
"p-limit": "^3.1.0",
"react": "^18.3.0",
"react-dom": "^18.3.0",
+ "react-hook-form": "^7.53.0",
"tailwind-merge": "^2.6.0",
+ "zod": "^3.23.0"
}shadcn/ui components are typically installed via the CLI (npx shadcn-ui@latest init) rather than added as a direct dependency.
📝 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.
| "dependencies": { | |
| "@xenova/transformers": "^2.17.2", | |
| "clsx": "^2.1.0", | |
| "dotenv": "^17.4.2", | |
| "lucide-react": "^0.460.0", | |
| "next": "^14.2.0", | |
| "p-limit": "^3.1.0", | |
| "react": "^18.3.0", | |
| "react-dom": "^18.3.0", | |
| "tailwind-merge": "^2.6.0" | |
| }, | |
| "dependencies": { | |
| "`@xenova/transformers`": "^2.17.2", | |
| "`@hookform/resolvers`": "^3.9.0", | |
| "clsx": "^2.1.0", | |
| "dotenv": "^17.4.2", | |
| "lucide-react": "^0.460.0", | |
| "next": "^14.2.0", | |
| "p-limit": "^3.1.0", | |
| "react": "^18.3.0", | |
| "react-dom": "^18.3.0", | |
| "react-hook-form": "^7.53.0", | |
| "tailwind-merge": "^2.6.0", | |
| "zod": "^3.23.0" | |
| }, |
🤖 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 `@kits/flowbench/apps/package.json` around lines 17 - 27, Update the form
implementation centered on RunForm.tsx to use the required shadcn/ui components
and react-hook-form with zod validation, adding react-hook-form and zod to the
app dependencies. Do not add shadcn/ui as a direct package dependency; use the
existing clsx and tailwind-merge setup with generated shadcn/ui components or
configuration.
Source: Coding guidelines
| "eslint": "^8.0.0", | ||
| "eslint-config-next": "^14.2.0", | ||
| "postcss": "^8.4.0", | ||
| "tailwindcss": "^3.4.0", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Tailwind CSS v3 violates the kit guideline requiring v4+.
The coding guidelines for kits/*/apps/**/*.{ts,tsx,js,jsx} specify "Tailwind CSS v4+", but package.json declares tailwindcss: ^3.4.0. This also impacts postcss.config.mjs (which uses the v3 plugin style) and globals.css (which uses v3 @tailwind directives instead of v4's @import "tailwindcss"). Migrating to v4 requires coordinated changes across all three files.
As per coding guidelines: kits/*/apps/**/*.{ts,tsx,js,jsx} — "use Next.js 14-15, React 18, TypeScript, Tailwind CSS v4+, shadcn/ui components, react-hook-form + zod for forms, lucide-react for icons, and Vercel for deployment".
♻️ Proposed migration to Tailwind CSS v4
- "tailwindcss": "^3.4.0",
+ "tailwindcss": "^4.0.0",
+ "`@tailwindcss/postcss`": "^4.0.0",Additionally, postcss.config.mjs would need to change from tailwindcss: {} to @tailwindcss/postcss: {}, and globals.css would replace @tailwind base; @tailwindcomponents;@tailwind utilities; with @import "tailwindcss";.
📝 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.
| "tailwindcss": "^3.4.0", | |
| "tailwindcss": "^4.0.0", | |
| "`@tailwindcss/postcss`": "^4.0.0", |
🤖 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 `@kits/flowbench/apps/package.json` at line 36, Upgrade the Tailwind CSS
dependency in package.json to v4+, add the corresponding `@tailwindcss/postcss`
package, update postcss.config.mjs to use the v4 PostCSS plugin, and replace the
v3 `@tailwind` directives in globals.css with the v4 `@import` "tailwindcss" entry.
Source: Coding guidelines
| ## Identity | ||
| You are FlowBench, an automated testing and benchmarking utility for Lamatic flows. | ||
|
|
||
| ## Safety | ||
| - Never execute Lamatic flows other than the ones explicitly provided by the user in the UI or configuration. | ||
| - Do not autonomously run flows against production environments unless configured by the user. | ||
| - If uncertain about the input test cases, halt execution and report the error. | ||
|
|
||
| ## Data Handling | ||
| - Never log, store, or transmit PII from test cases or flow outputs to unauthorized external services. | ||
| - Local baselines are stored in `.flowbench/baselines/` and should be treated as potentially sensitive data depending on user test cases. | ||
|
|
||
| ## Tone | ||
| - Neutral, technical, and objective. | ||
| - Reports should focus strictly on data: latency metrics, similarity scores, and regression pass/fails. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
MD022 warnings are expected in templated constitution files.
Markdownlint flags missing blank lines around headings (lines 3, 6, 11, 15). Per prior guidance, default.md is a templated/auto-generated file — these formatting fixes should be applied at the template/source level so future kits inherit the correction, rather than patching this checked-in copy.
Based on learnings: In kits/*/constitutions/default.md, treat this default.md as a templated/auto-generated file. If you see Markdownlint MD022 (blanks-around-headings) warnings in these files, don't propose fixing only the checked-in default.md; instead, request that the correction be made at the template/source level so future kits inherit the fixed formatting.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 15-15: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 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 `@kits/flowbench/constitutions/default.md` around lines 3 - 17, Update the
template or source that generates the constitutions under
kits/*/constitutions/default.md to add blank lines around the Identity, Safety,
Data Handling, and Tone headings, rather than editing this checked-in generated
file directly; ensure future generated kits inherit the MD022-compliant
formatting.
Sources: Learnings, Linters/SAST tools
…11y, concurrency, error isolation
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kits/flowbench/apps/actions/orchestrate.ts (1)
90-97: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winThe
null as anyescape hatch leaks a broken type contract.
OrchestrationResultdeclaresrun: RunResultanddiff: CompareResultas non-optional, but the error path returnsnull as any— silently violating the interface. The current consumer (RunForm.tsx) checksresult.errorbefore accessingresult.run, so it's safe today, but any future consumer that trusts the type will hit a runtime null-dereference.🔧 Proposed fix: make the interface honest
export interface OrchestrationResult { - run: RunResult; - diff: CompareResult; + run: RunResult | null; + diff: CompareResult | null; baselineUpdated: boolean; error?: string; }Then remove the
as anycasts:return { - run: null as any, - diff: null as any, + run: null, + diff: null, baselineUpdated: false, error: err instanceof Error ? err.message : "An unknown error occurred", };🤖 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 `@kits/flowbench/apps/actions/orchestrate.ts` around lines 90 - 97, Update the OrchestrationResult type and its error-path return in the orchestration function so run and diff are nullable or optional when error is present, matching the existing null values. Remove both null as any casts and ensure successful results retain the required RunResult and CompareResult values.
♻️ Duplicate comments (3)
kits/flowbench/apps/actions/orchestrate.ts (1)
1-15: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftKit orchestrate.ts still operates without
../../lamatic.configin scope.Per coding guidelines,
kits/*/apps/actions/orchestrate.tsmust import and use../../lamatic.configto read step definitions from the parent kit.executeBenchmarkaccepts an arbitrary caller-suppliedflowIdinstead of resolving from the kit config. Config-based fallback happens downstream inexecuteFlow(lamatic-client.ts), but the guideline requires the import in this file.As per coding guidelines,
kits/*/apps/actions/orchestrate.tsmust import and use../../lamatic.configto read step definitions from the parent kit.🤖 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 `@kits/flowbench/apps/actions/orchestrate.ts` around lines 1 - 15, Update orchestrate.ts to import ../../lamatic.config and use its step definitions when resolving the flow for executeBenchmark, rather than relying solely on the caller-supplied flowId or downstream executeFlow fallback. Preserve the existing orchestration behavior while ensuring the parent kit configuration is read in this file.Source: Coding guidelines
kits/flowbench/apps/actions/runFlow.ts (1)
86-154: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winStill missing extraction:
coerceToStringserialization failure remains uncontained.
coerceToStringat line 137 sits outside the try/catch that wrapsexecuteFlow(lines 98–120). IfJSON.stringifythrows on a circular structure or BigInt, the exception escapesrunSingleCase's "never throws" contract (line 88), causingPromise.allat line 183 to reject the entire batch over one bad output shape.🛡️ Proposed fix
const rawOutput = flowResult.result[testCase.resultField]; - const output = coerceToString(rawOutput); + let output: string | null; + try { + output = coerceToString(rawOutput); + } catch (err: unknown) { + return { + id: testCase.id, + output: null, + latencyMs: flowResult.latencyMs, + error: `Failed to serialize resultField "${testCase.resultField}": ${ + err instanceof Error ? err.message : String(err) + }`, + }; + }🤖 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 `@kits/flowbench/apps/actions/runFlow.ts` around lines 86 - 154, Contain serialization failures from coerceToString within runSingleCase so its “never throws” contract is preserved when JSON.stringify encounters circular data or BigInt. Update the output-coercion path around coerceToString to catch conversion errors and return the standard failed TestCaseRunResult for the current testCase, preserving the flow latency and avoiding rejection of the surrounding batch.kits/flowbench/apps/components/RunForm.tsx (1)
9-139: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftForm still runs on raw
useStateinstead of the kit-mandatedreact-hook-form+zodstack.The file upload accessibility fix is solid, but the broader form pattern still uses manual
useState/JSON.parsevalidation and hand-styled elements instead ofreact-hook-form+zodandshadcn/uicomponents required by the coding guidelines. Parsed JSONL test cases areany[]with no schema validation before being passed toexecuteBenchmarkasTestCaseInput[].As per coding guidelines,
kits/*/apps/**/*.{ts,tsx,js,jsx}must use react-hook-form + zod for forms and shadcn/ui components.🤖 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 `@kits/flowbench/apps/components/RunForm.tsx` around lines 9 - 139, Replace the manual form state and JSON.parse-only validation in RunForm with react-hook-form and a zod schema, including schema validation that produces typed TestCaseInput[] before executeBenchmark. Use the kit’s shadcn/ui form, input, button, and related components while preserving file-upload accessibility, loading behavior, error display, and navigation.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@kits/flowbench/apps/actions/orchestrate.ts`:
- Around line 90-97: Update the OrchestrationResult type and its error-path
return in the orchestration function so run and diff are nullable or optional
when error is present, matching the existing null values. Remove both null as
any casts and ensure successful results retain the required RunResult and
CompareResult values.
---
Duplicate comments:
In `@kits/flowbench/apps/actions/orchestrate.ts`:
- Around line 1-15: Update orchestrate.ts to import ../../lamatic.config and use
its step definitions when resolving the flow for executeBenchmark, rather than
relying solely on the caller-supplied flowId or downstream executeFlow fallback.
Preserve the existing orchestration behavior while ensuring the parent kit
configuration is read in this file.
In `@kits/flowbench/apps/actions/runFlow.ts`:
- Around line 86-154: Contain serialization failures from coerceToString within
runSingleCase so its “never throws” contract is preserved when JSON.stringify
encounters circular data or BigInt. Update the output-coercion path around
coerceToString to catch conversion errors and return the standard failed
TestCaseRunResult for the current testCase, preserving the flow latency and
avoiding rejection of the surrounding batch.
In `@kits/flowbench/apps/components/RunForm.tsx`:
- Around line 9-139: Replace the manual form state and JSON.parse-only
validation in RunForm with react-hook-form and a zod schema, including schema
validation that produces typed TestCaseInput[] before executeBenchmark. Use the
kit’s shadcn/ui form, input, button, and related components while preserving
file-upload accessibility, loading behavior, error display, and navigation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96b9ec34-5323-4439-b263-37d4a465900d
📒 Files selected for processing (7)
kits/flowbench/apps/__tests__/compareBaseline.test.tskits/flowbench/apps/actions/orchestrate.tskits/flowbench/apps/actions/runFlow.tskits/flowbench/apps/actions/storage.tskits/flowbench/apps/components/RunForm.tsxkits/flowbench/apps/lib/lamatic-client.tskits/flowbench/apps/lib/metrics.ts
💤 Files with no reviewable changes (1)
- kits/flowbench/apps/lib/metrics.ts
|
A few items from this review I'm deliberately deferring rather than fixing in this PR, with reasoning: react-hook-form + zod + shadcn/ui in RunForm.tsx — agreed this is the documented convention, and I'm not doing it here because retrofitting a working, tested form this late in the review cycle risks breaking something that currently works correctly (upload, validation, loading states, keyboard accessibility) for a stylistic/convention gain rather than a correctness one. Happy to follow up with this as a separate PR if that's preferred over holding this one open longer. Tailwind CSS v3 → v4 migration — same reasoning. Real guideline gap (package.json, postcss.config.mjs, and globals.css would all need coordinated changes), but a full migration this late carries real risk of breaking the UI for a convention-compliance gain, not a functional one. Everything else flagged in this review — path traversal, GraphQL injection, keyboard accessibility, concurrency, per-case error isolation, threshold duplication, missing test coverage, the coerceToString serialization boundary, and the OrchestrationResult type honesty fix — has been addressed and pushed. Let me know if the two deferred items above should block merge; happy to tackle them if so. |
…matic WorkflowResponse type
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
/validate |
|
📡 Running Studio validation — results will appear here shortly. |
Studio Runtime Validation (Phase 2)✅ Studio validation passed. The kit loaded successfully in Lamatic Studio. This PR is ready for final review and merge. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Pushed fixes for everything else flagged since — path traversal guard in storage.ts, GraphQL identifier validation in lamatic-client.ts, per-case error isolation in orchestrator.ts + runFlow.ts, keyboard-accessible dropzone in RunForm.tsx, unified regression thresholds between metrics.ts and compareBaseline.ts, and the two missing test cases for removedCases/improvements. Still holding on the two deferred items above (react-hook-form/zod/shadcn refactor, Tailwind v4 migration) — let me know if either should block merge. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
What is FlowBench?
FlowBench runs your Lamatic flow against a set of test cases before you merge a change, not after — closer to CI for a normal codebase than a monitoring tool. Give it a flow ID and a JSONL file of test cases, and it executes each one, scores the output using local sentence-embedding similarity (no external API calls), times each request, and compares the run against a saved baseline so you can see exactly what got better or worse.
Why this isn't a duplicate of similar kits here
llm-silent-failure-detector watches a flow in production after it's shipped. FlowBench runs before you ship, against a fixed set of test cases you control.
live-api-debugger is for stepping through one request interactively. FlowBench runs a whole test suite at once, no manual steps.
Testing this locally
A couple of things worth knowing
Similarity scoring measures semantic closeness, not just correctness — a correct but differently-phrased answer can score lower than expected. More on this in the README's Known Limitations section.
No database, no auth — baselines are flat JSON files in .flowbench/, kept intentionally simple.
One more thing: the PR template that auto-populated this box referenced an older config.json/bundles/templates structure that doesn't match how this repo actually works now (confirmed against the current CONTRIBUTING.md and the reference kits). I replaced it with this instead of trying to force-fit the old format.
FlowBench kit documentation, governance, and config
kits/flowbench/README.mdkits/flowbench/agent.mdkits/flowbench/constitutions/default.mdkits/flowbench/.env.examplekits/flowbench/.gitignore(ignores.DS_Storeand local.flowbench/)kits/flowbench/lamatic.config.ts(kitflowbench, runsflowbench-demo-flowviaFLOWBENCH_DEMO_FLOW_ID)kits/flowbench/flows/flowbench-demo-flow.ts(demo flow definition)kits/flowbench/sample-tests/example.jsonl(example JSONL test cases withexpected_containsandmin_similarity)kits/flowbench/model-configs/flowbench-demo-flow_llm-node_generative-model-name.tskits/flowbench/prompts/flowbench-demo-flow_llm-node_system_0.mdkits/flowbench/prompts/flowbench-demo-flow_llm-node_user_1.mdFlowBench dashboard + benchmark runtime (Next.js)
kits/flowbench/apps/.env.examplekits/flowbench/apps/.gitignorekits/flowbench/apps/package.jsonkits/flowbench/apps/package-lock.jsonkits/flowbench/apps/next.config.mjs(allows server-component external packages for@xenova/transformers/onnxruntime-node)kits/flowbench/apps/tsconfig.jsonkits/flowbench/apps/postcss.config.mjskits/flowbench/apps/tailwind.config.tskits/flowbench/apps/app/layout.tsxkits/flowbench/apps/app/globals.csskits/flowbench/apps/app/page.tsx(recent runs list)kits/flowbench/apps/app/report/[runId]/page.tsx(run report + baseline status + test-case table)kits/flowbench/apps/components/RunForm.tsx(uploads JSONL + starts a run)kits/flowbench/apps/lib/lamatic-client.ts(typed GraphQL execution client)kits/flowbench/apps/lib/metrics.ts(cosine similarity, percentile, run-id generation)kits/flowbench/apps/actions/runFlow.ts(per-case execution with 30s timeout, bounded concurrency, safe output coercion)kits/flowbench/apps/actions/scoreOutput.ts(local embedding similarity via@xenova/transformers+Xenova/all-MiniLM-L6-v2)kits/flowbench/apps/actions/orchestrate.ts(end-to-end orchestration: run → diff vs baseline → baseline update rules → history/details)kits/flowbench/apps/actions/compareBaseline.ts(diff logic: regressions, blocking regressions, improvements, new/removed cases, first-run handling)kits/flowbench/apps/actions/storage.ts(flat JSON persistence under.flowbench/baselines/runs)kits/flowbench/apps/__tests__/compareBaseline.test.ts(baseline comparison scenarios)Assets
kits/flowbench/assets/dashboard.pngFlow node types introduced (from
kits/flowbench/flows/flowbench-demo-flow.ts) and how the demo flow workstrigger(triggerNode): GraphQL/API trigger acceptinguserPromptllmNode(dynamicNode): generates text using referenced system/user prompt markdown + referenced generative model configresponseNode(responseNode): maps{{llmNode.output.generatedResponse}}to output fieldresponsetrigger -> llmNode(defaultEdge)llmNode -> responseNode(defaultEdge)trigger -> responseNode(responseEdge)triggerusinguserPrompt, run the prompt-driven LLM generation, then return the generated text asresponse.FlowBench how it works (high level)
flowId+ JSONL test cases, executes cases with bounded concurrency and per-case timeouts, scores output quality using local embedding-based cosine similarity (no external scoring API calls), records latency, diffs results against saved baselines, and updates baselines only when all cases pass and there are no blocking regressions.