Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements bulk creation and publishing of benchmark workflows from catalog templates. The changes add functionality to seed benchmark workflows from JSON files stored in a catalog directory, creating and publishing multiple flows atomically in a single database transaction.
Changes:
- Added new
benchmark-flow-bulk-create.tsmodule with bulk flow creation and publishing functionality - Enhanced
createBenchmarkto seed workflows from catalog usingbenchmarkConfigurationparameter - Updated
seedBenchmarkWorkflowsFromCatalogto read workflow templates from files and bulk create flows - Added comprehensive test coverage for the new seeding functionality
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
packages/server/api/src/app/benchmark/benchmark-flow-bulk-create.ts |
New file implementing bulk flow creation and publishing with database transaction support |
packages/server/api/src/app/benchmark/create-benchmark.service.ts |
Added seedBenchmarkWorkflowsFromCatalog function and updated createBenchmark to use benchmark configuration for workflow seeding |
packages/server/api/test/unit/benchmark/create-benchmark.service.test.ts |
Added test coverage for the new seeding functionality including empty paths scenario |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/server/api/src/app/benchmark/create-benchmark.service.ts
Outdated
Show resolved
Hide resolved
| const content = await fs.readFile(filePath, 'utf-8'); | ||
| const parsed = JSON.parse(content) as { | ||
| template: WorkflowTemplate['template']; | ||
| }; |
There was a problem hiding this comment.
The fs.readFile and JSON.parse operations in the loop lack error handling. If a catalog file is corrupted, missing, or contains invalid JSON, the entire benchmark creation will fail without a clear error message.
Consider wrapping these operations in try-catch blocks to provide more specific error messages about which workflow file failed to load or parse. This will make debugging easier and provide better user feedback.
| const content = await fs.readFile(filePath, 'utf-8'); | |
| const parsed = JSON.parse(content) as { | |
| template: WorkflowTemplate['template']; | |
| }; | |
| let content: string; | |
| try { | |
| content = await fs.readFile(filePath, 'utf-8'); | |
| } catch (err) { | |
| const message = | |
| err instanceof Error ? err.message : String(err); | |
| throwValidationError( | |
| `Failed to read benchmark catalog file at "${filePath}": ${message}`, | |
| ); | |
| } | |
| let parsed: { template: WorkflowTemplate['template'] }; | |
| try { | |
| parsed = JSON.parse(content) as { | |
| template: WorkflowTemplate['template']; | |
| }; | |
| } catch (err) { | |
| const message = | |
| err instanceof Error ? err.message : String(err); | |
| throwValidationError( | |
| `Failed to parse JSON in benchmark catalog file at "${filePath}": ${message}`, | |
| ); | |
| } |
packages/server/api/test/unit/benchmark/create-benchmark.service.test.ts
Show resolved
Hide resolved
packages/server/api/src/app/benchmark/benchmark-flow-bulk-create.ts
Outdated
Show resolved
Hide resolved
| return results.map((r, index) => ({ | ||
| flowId: r.id, | ||
| displayName: r.version.displayName, | ||
| isOrchestrator: index === 0, |
There was a problem hiding this comment.
The isOrchestrator flag is determined solely by the index position (index === 0) in the results array. This assumes that the first workflow in the paths array will always be the orchestrator.
This logic depends on the order returned by resolveWorkflowPathsForSeed. Looking at catalog-resolver.ts line 63, the order is: [orchestratorId, cleanupId, ...subWorkflowIds]. This means:
- index 0 = orchestrator (correct ✓)
- index 1 = cleanup (marked as isOrchestrator: false, which is correct)
- index 2+ = sub-workflows (marked as isOrchestrator: false, which is correct)
However, this implicit dependency on array ordering is fragile. Consider adding a comment explaining this assumption, or better yet, explicitly mark which workflow is the orchestrator based on the workflow ID rather than position.
| return results.map((r, index) => ({ | |
| flowId: r.id, | |
| displayName: r.version.displayName, | |
| isOrchestrator: index === 0, | |
| // NOTE: The isOrchestrator flag relies on the ordering contract of | |
| // resolveWorkflowPathsForSeed, which returns paths in the form: | |
| // [orchestratorId, cleanupId, ...subWorkflowIds] | |
| // This means: | |
| // - index 0 = orchestrator | |
| // - index 1 = cleanup | |
| // - index 2+ = sub-workflows | |
| // If the ordering in resolveWorkflowPathsForSeed changes, this constant | |
| // (and the associated logic) must be updated accordingly. | |
| const orchestratorIndex = 0; | |
| return results.map((r, index) => ({ | |
| flowId: r.id, | |
| displayName: r.version.displayName, | |
| isOrchestrator: index === orchestratorIndex, |
packages/server/api/src/app/benchmark/benchmark-flow-bulk-create.ts
Outdated
Show resolved
Hide resolved
packages/server/api/src/app/benchmark/benchmark-flow-bulk-create.ts
Outdated
Show resolved
Hide resolved
|
| import { fetchConnectionsWithSupportedBlocks } from './connections-with-supported-blocks'; | ||
| import { throwValidationError } from './errors'; | ||
|
|
||
| function validateBenchmarkConfiguration(config: BenchmarkConfiguration): void { |
There was a problem hiding this comment.
We might move this validation later to the controller; keeping it here for now.
| provider, | ||
| workflows: [], | ||
| workflows, | ||
| webhookPayload: { |
There was a problem hiding this comment.
Will build the payload in later PRs
| const parsedTemplates = paths.map(async ({ filePath }) => { | ||
| const content = await fs.readFile(filePath, 'utf-8'); | ||
| const parsed = JSON.parse(content) as { | ||
| template: WorkflowTemplate['template']; | ||
| }; | ||
| return { template: parsed.template }; | ||
| }); | ||
| const templates = await Promise.all(parsedTemplates); |
There was a problem hiding this comment.
Extract to a method:
async function loadWorkflowTemplates(provider: string, workflowIds: string[]): Promise<WorkflowTemplate[]> {
const paths = resolveWorkflowPathsForSeed(provider, workflowIds);
if (paths.length === 0) {
return [];
}
const parsedTemplates = paths.map(async ({ filePath }) => {
const content = await fs.readFile(filePath, 'utf-8');
const parsed = JSON.parse(content) as {
template: WorkflowTemplate['template'];
};
return { template: parsed.template };
});
return await Promise.all(parsedTemplates);
}
| userId, | ||
| }); | ||
|
|
||
| const paths = resolveWorkflowPathsForSeed(provider, workflowIds); |
There was a problem hiding this comment.
| const paths = resolveWorkflowPathsForSeed(provider, workflowIds); | |
| const templates = await loadWorkflowTemplates(provider, workflowIds); |
|
|
||
| const paths = resolveWorkflowPathsForSeed(provider, workflowIds); | ||
| const workflows = await seedBenchmarkWorkflowsFromCatalog({ | ||
| paths, |
There was a problem hiding this comment.
| paths, | |
| templates, |
| ); | ||
| } | ||
|
|
||
| export async function seedBenchmarkWorkflowsFromCatalog(params: { |
There was a problem hiding this comment.
| export async function seedBenchmarkWorkflowsFromCatalog(params: { | |
| export async function createBenchmarkWorkflows(params: { |
| } | ||
|
|
||
| export async function seedBenchmarkWorkflowsFromCatalog(params: { | ||
| paths: ResolvedWorkflowPath[]; |
There was a problem hiding this comment.
| paths: ResolvedWorkflowPath[]; | |
| templates: WorkflowTemplate[]; |
| import { appConnectionService } from '../app-connection/app-connection-service/app-connection-service'; | ||
| import { getProviderMetadataForAllBlocks } from '../app-connection/connection-providers-resolver'; | ||
|
|
||
| export async function fetchConnectionsWithSupportedBlocks( |
There was a problem hiding this comment.
| export async function fetchConnectionsWithSupportedBlocks( | |
| export async function getConnectionsWithBlockSupport( |
| }); | ||
| const templates = await Promise.all(parsedTemplates); | ||
|
|
||
| const connections = await fetchConnectionsWithSupportedBlocks(projectId, [ |
There was a problem hiding this comment.
| const connections = await fetchConnectionsWithSupportedBlocks(projectId, [ | |
| const connections = await getConnectionsWithBlockSupport(projectId, [ |



Fixes OPS-3715.
Additional Notes
There is another ticket to add the seeded flows to benchmark tables.