Fix: Promise.all visualization for dynamic maps & Pearl JSON response parsing#252
Fix: Promise.all visualization for dynamic maps & Pearl JSON response parsing#252Devil1716 wants to merge 7 commits intobubblelabai:mainfrom
Conversation
📝 WalkthroughWalkthroughDetects dynamic parallelism from map-based Promise.all, surfaces isDynamic/sourceArray through parser and schema, updates studio step labeling for mapped parallel children, adds a runtime test, and makes Pearl agent response parsing more robust to JSON-in-markdown formats. Changes
Sequence Diagram(s)sequenceDiagram
participant DevCode as Dev source (AST)
participant Parser as BubbleParser
participant Schema as Shared Schema
participant Studio as Studio transformer
participant UI as Visualization (StepData)
DevCode->>Parser: parse code with Promise.all over .map(...)
Parser->>Parser: findArrayElements -> { elements, isDynamic, sourceArray }
Parser-->>Schema: emit `parallel_execution` node with isDynamic & sourceArray
Schema-->>Studio: workflow JSON includes parallel_execution metadata
Studio->>Studio: iterate children (index-based), adjust functionName/description per isDynamic/sourceArray
Studio-->>UI: create StepData with updated labels/descriptions
note right of UI: Dynamic items labelled "Mapped" or "Parallel item from <sourceArray>"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
Comment |
Suggested PR title from PearlTitle: Body: ChangesDynamic Parallel Execution DetectionAdds capability to detect and properly label parallel execution patterns that originate from array map operations (e.g., Key improvements:
Pearl AI Error RecoveryImproves JSON parsing resilience in the Pearl AI service:
Code Quality
Modified Files
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bubble-runtime/src/extraction/BubbleParser.ts (1)
4111-4215: Fix critical temporal dead zone error: movevariableDeclarationbefore branches inbuildParallelExecutionNodeAt line 4151, the dynamic branch early-returns using
variableDeclarationbefore it is declared at line 4189. SincevariableDeclarationis aletbinding, accessing it before initialization throws a ReferenceError at runtime: "Cannot access 'variableDeclaration' before initialization."Move the
variableDeclarationdeclaration and initialization block (currently lines 4188–4215) to the start of the function—immediately afterconst children: WorkflowNode[] = [];(line 4107)—so both the dynamic and non-dynamic returns reference the same initialized variable.Proposed fix
const code = this.bubbleScript.substring(stmt.range![0], stmt.range![1]); const children: WorkflowNode[] = []; + // Extract variable declaration info if this is part of a variable declaration + let variableDeclaration: + | { + variableNames: string[]; + variableType: 'const' | 'let' | 'var'; + } + | undefined; + + if ( + stmt.type === 'VariableDeclaration' && + stmt.declarations.length > 0 && + stmt.declarations[0].id.type === 'ArrayPattern' + ) { + const arrayPattern = stmt.declarations[0].id; + const variableNames: string[] = []; + for (const element of arrayPattern.elements) { + if (element && element.type === 'Identifier') { + variableNames.push(element.name); + } + } + variableDeclaration = { + variableNames, + variableType: stmt.kind as 'const' | 'let' | 'var', + }; + } + // Handle variable reference (e.g., Promise.all(exampleScrapers)) if (promiseAllInfo.arrayExpr.type === 'Identifier' && this.cachedAST) { // ...existing code using variableDeclaration in early return... } else if (promiseAllInfo.arrayExpr.type === 'ArrayExpression') { // ...existing code... } - - // Extract variable declaration info if this is part of a variable declaration - let variableDeclaration: - | { - variableNames: string[]; - variableType: 'const' | 'let' | 'var'; - } - | undefined; - // ...rest of block removed...
🧹 Nitpick comments (3)
apps/bubblelab-api/src/services/ai/pearl.ts (1)
572-605: Fallback on failed agent runs looks correct, with minor simplification possibleThe recovery path for
!result.successthat:
- first tries
parseJsonWithFallbacks,- then validates via
PearlAgentOutputSchema,- and finally falls back to an
answer-type on schema failure or parsing failurematches the issue goal of treating legitimate JSON blocks (and non‑Pearl JSON) as normal answers rather than hard errors.
You might simplify slightly by assigning
result.data.response = recoveryParse.response(or cachingrecoveryParse.parsed) and reusing it in the main parse block to avoid re-runningparseJsonWithFallbacksandPearlAgentOutputSchema.parsetwice on the same string, but functionally this is already sound.apps/bubble-studio/src/utils/workflowToSteps.ts (1)
338-385: Dynamic parallel labeling logic is consistent with the new metadataThe new handling for
parallel_execution:
- Reads
isDynamic/sourceArrayfrom the node.- For a single dynamic child, annotates the function name with
" (Mapped)"and description with"Parallel execution over <sourceArray>".- For multiple dynamic children, appends
"Parallel item from <sourceArray>"to each description.This matches the intended UX difference between “one step representing a map over an array” and “individual parallel items” and doesn’t affect non‑dynamic flows.
packages/bubble-runtime/src/extraction/BubbleParser.ts (1)
3884-3897: NewfindArrayElementsreturn shape and dynamic flags are coherentReturning
{ elements, isDynamic, sourceArray? }fromfindArrayElements:
- Keeps
.push()patterns compatible by fillingelementsand leavingisDynamicfalse.- Detects
.map()-based array construction by settingisDynamic = trueandsourceArrayNamewhen the source is an identifier.- Uses
getSourceArrayElementsto approximate multiplicity (replicating the callback expression per static element and using a single element when the length is unknown).This gives
buildParallelExecutionNodeenough information to distinguish dynamic Promise.all flows from static ones without regressing existing patterns.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/bubble-studio/src/utils/workflowToSteps.tsapps/bubblelab-api/src/services/ai/pearl.tspackages/bubble-runtime/src/extraction/BubbleParser.test.tspackages/bubble-runtime/src/extraction/BubbleParser.tspackages/bubble-shared-schemas/src/bubble-definition-schema.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/packages/bubble-shared-schemas/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api.mdc)
**/packages/bubble-shared-schemas/**/*.{ts,tsx}: Write shared schemas between frontend and backend in/packages/bubble-shared-schemasdirectory
Runpnpm build:coreafter modifying shared schemas since it is a separate package and types need to be regenerated
Files:
packages/bubble-shared-schemas/src/bubble-definition-schema.ts
🧠 Learnings (8)
📚 Learning: 2025-12-22T09:55:47.873Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.873Z
Learning: Applies to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts : Refer to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts for documentation on how bubble flow is supposed to be generated
Applied to files:
apps/bubblelab-api/src/services/ai/pearl.tspackages/bubble-runtime/src/extraction/BubbleParser.test.tspackages/bubble-shared-schemas/src/bubble-definition-schema.ts
📚 Learning: 2025-12-22T09:55:47.873Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.873Z
Learning: Applies to packages/bubble-runtime/src/extraction/README.md : Refer to packages/bubble-runtime/src/extraction/README.md for information about bubble parsing, dependency graphs, and per-invocation cloning
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.test.tsapps/bubble-studio/src/utils/workflowToSteps.tspackages/bubble-runtime/src/extraction/BubbleParser.ts
📚 Learning: 2025-12-19T03:17:06.825Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.825Z
Learning: Applies to packages/bubble-core/dist/**/*.d.ts : The type bundling system for Monaco Editor must process TypeScript declaration files (.d.ts) and inline all dependencies into a single self-contained bundle in `packages/bubble-core/dist/bubble-bundle.d.ts`
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.test.tspackages/bubble-runtime/src/extraction/BubbleParser.ts
📚 Learning: 2025-12-19T03:17:06.825Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.825Z
Learning: Applies to packages/bubble-core/src/index.ts : All new types and classes added to bubble-core must be exported from `packages/bubble-core/src/index.ts` to ensure they are included in the generated bundle
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.test.tspackages/bubble-runtime/src/extraction/BubbleParser.ts
📚 Learning: 2025-12-19T03:16:48.801Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-12-19T03:16:48.801Z
Learning: Applies to **/bubblelab-api/**/*.test.{js,ts} : Use `pnpm bun test` command to run backend tests to ensure setup files are properly loaded
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.test.ts
📚 Learning: 2025-12-19T03:17:06.825Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.825Z
Learning: Applies to packages/bubble-core/dist/**/*.d.ts : All external imports (except relative imports) must be removed from the bundled output to prevent circular processing and external dependency issues
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.test.tspackages/bubble-runtime/src/extraction/BubbleParser.ts
📚 Learning: 2025-12-19T03:16:48.801Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-12-19T03:16:48.801Z
Learning: Applies to **/bubblelab-api/**/*.test.{js,ts} : Reference webhook.test as the example for how backend tests should be written
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.test.ts
📚 Learning: 2025-12-19T03:16:48.801Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-12-19T03:16:48.801Z
Learning: Applies to **/packages/bubble-shared-schemas/**/*.{ts,tsx} : Write shared schemas between frontend and backend in `/packages/bubble-shared-schemas` directory
Applied to files:
packages/bubble-shared-schemas/src/bubble-definition-schema.ts
🧬 Code graph analysis (1)
apps/bubblelab-api/src/services/ai/pearl.ts (3)
packages/bubble-core/src/index.ts (1)
parseJsonWithFallbacks(139-139)packages/bubble-core/src/utils/json-parsing.ts (1)
parseJsonWithFallbacks(653-778)packages/bubble-shared-schemas/src/pearl.ts (1)
PearlAgentOutputSchema(123-127)
🪛 Biome (2.1.2)
packages/bubble-runtime/src/extraction/BubbleParser.ts
[error] 4151-4152: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🔇 Additional comments (15)
apps/bubblelab-api/src/services/ai/pearl.ts (1)
614-621: Switch toparseJsonWithFallbacksin main path is a solid robustness winReplacing direct
JSON.parsewithparseJsonWithFallbacksforresponseTextparsing correctly handles markdown code fences, embedded JSON, and minor formatting issues before validating withPearlAgentOutputSchema. No behavioural regressions are apparent in the surrounding control flow.packages/bubble-shared-schemas/src/bubble-definition-schema.ts (2)
361-378: Dynamic metadata onParallelExecutionWorkflowNodeis wired correctlyAdding optional
isDynamicandsourceArrayto both the TypeScript interface andParallelExecutionWorkflowNodeSchemakeeps the public shape consistent and is backwards compatible for existing clients.Per the guidelines for shared schemas, remember to run
pnpm build:coreso the regenerated types are up to date across packages.
547-562: Zod schema matches new interface fieldsThe additions of
isDynamic: z.boolean().optional()andsourceArray: z.string().optional()correctly mirror the interface and will allow BubbleParser / studio to consume the new metadata without extra casting.packages/bubble-runtime/src/extraction/BubbleParser.test.ts (1)
668-707: Dynamic parallel execution test covers the key behaviourThe new test cleanly exercises the
.map(...)+Promise.all(tasks)pattern and validates:
- presence of a
parallel_executionnode,isDynamic === true,sourceArray === 'items',- and non‑empty children.
This is exactly the regression surface described in the issue and should protect the new parser logic well.
apps/bubble-studio/src/utils/workflowToSteps.ts (4)
341-359: Index-based loop over parallel children looks safeSwitching to an index-based
foroverparallelNode.children(instead offor...of) is appropriate here, since you needifor dynamic labeling. Bubble id extraction and control‑flow collection remain identical in behaviour.
168-171: Formatting-only change aroundextractBubbleIdsByLineRangeThe minor formatting tweak around the
extractBubbleIdsByLineRangecall doesn’t alter behaviour; usage is still correct for derivingbubbleIdsfrom method definition locations.
485-487: LegacyextractStepsFromWorkflowremains consistent with new helperThe reflowed
extractBubbleIdsByLineRangecall here still mirrors the logic used inextractStepGraph, so legacy consumers ofextractStepsFromWorkflowwon’t see behavioural changes.
527-529: Parallel steps in legacy extractor unchanged semanticallyIn the legacy
extractStepsFromWorkflowpath, parallel children still derive theirbubbleIdsby either child bubbles or method ranges, with no logic changes beyond formatting. This keeps old views aligned with the new graph-based extractor.packages/bubble-runtime/src/extraction/BubbleParser.ts (7)
320-348: Extended dependency metadata handling forbubbleDependenciesDetailedlooks correctThe widened
metadatatype and subsequent handling ofbubbleDependenciesDetailed(including nestedtoolsfor AI agents) infindDependenciesForBubblecorrectly prefers detailed specs when available and falls back tobubbleDependenciesotherwise. This keeps the dependency graph richer without breaking existing metadata shapes.
1924-1988: Parameterlocationenrichment inextractFromNewExpressionis consistentAdding
locationwith{startLine,startCol,endLine,endCol}for:
- object-literal properties,
- spread arguments,
- and first-argument variables
aligns BubbleParameter with
BubbleParameterSchemaand will help downstream tooling (e.g., editors, hover, diagnostics) localize parameter usage precisely. The guards onlocbeing present before constructing the object are safe.
2199-2224: Stricter string checks for custom toolname/descriptionare safeThe added
typeof ... === 'string'checks when extractingtoolNameanddescriptionfrom custom tool definitions avoid accidental non‑string values while preserving existing behaviour for well-formed tool configs.
3022-3052: Refactored else-branch construction inbuildIfNodepreserves semanticsThe small IIFE used to build
elseBranchnow:
- handles
else { ... },else if (...), and single-statementelsecases explicitly,- always returns an array (or
[]) when an alternate exists.This keeps the shape of
elseBranchconsistent and doesn’t change howbranchTerminatesoperates on the resulting tree.
3094-3099:buildForNodecondition string construction remains correctThe expanded multi-line
substringcall for theupdatepart of aForStatementis equivalent to the previous implementation and still yields a human-readableinit; test; updatestring for the loop condition.
4369-4373: Invocation-specific dependency graph cloning continues to look soundThe adjusted multi-line call to
cloneDependencyGraphNodeForInvocationis purely formatting; the logic still clones dependency graph nodes with invocation-scopeduniqueId/variableId, consistent with the per-invocation bubbling behaviour described in the comments.
543-571:getPayloadJsonSchemanow handles all supported handle entrypoint shapesUpdating the param extraction to treat
FunctionExpressionandArrowFunctionExpressionidentically toFunctionDeclarationwhen building the payload schema ensures:
- top-level
handlefunctions,- exported
handlevariables with arrow/func expressions,- and class
handlemethodsall feed their first parameter's type into the JSON schema. This matches the shapes returned by
findHandleFunctionNodeand closes a previous gap for non-declaration forms.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bubble-runtime/src/extraction/BubbleParser.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-22T09:55:47.873Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.873Z
Learning: Applies to packages/bubble-runtime/src/extraction/README.md : Refer to packages/bubble-runtime/src/extraction/README.md for information about bubble parsing, dependency graphs, and per-invocation cloning
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.ts
📚 Learning: 2025-12-19T03:17:06.825Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.825Z
Learning: Applies to packages/bubble-core/dist/**/*.d.ts : All external imports (except relative imports) must be removed from the bundled output to prevent circular processing and external dependency issues
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.ts
🔇 Additional comments (9)
packages/bubble-runtime/src/extraction/BubbleParser.ts (9)
322-326: LGTM: Type definition formatting improved.The explicit type definition for
bubbleDependenciesDetailedimproves code readability and makes the expected structure clearer.
3946-3982: LGTM: Dynamic array detection is well-structured with intentional scope limitation.The logic correctly identifies dynamic parallel execution from
.map()operations and propagates the source array information. The conservative approach of only detectingIdentifiersources (line 3952) is appropriate, as complex expressions likeitems.filter(...).map(...)would be harder to track reliably.The loop at lines 3965-3967 creates multiple references to the same
callbackExprfor visualization purposes, which correctly represents that each parallel execution runs the same code over different elements.
3022-3052: LGTM: Cleaner control flow structure.The refactored else branch handling improves readability with explicit variable declarations and clearer if-else-if chain, while maintaining identical behavior.
3223-3238: LGTM: Consistent refactoring pattern applied.The catch block handling follows the same improved structure as the else branch refactoring, providing consistency across the codebase.
4139-4181: LGTM: Dynamic parallel detection correctly integrated.The logic properly distinguishes between dynamic (variable-sourced) and static (literal) parallel execution:
- Variable references can be marked
isDynamicwhen sourced from.map()operations- Literal array expressions remain static (as expected)
- The
variableDeclarationfor array destructuring is correctly threaded through both paths
3889-3893: LGTM: Signature change properly implemented and all callers updated.The structured return type makes the method's contract explicit and enables clean propagation of dynamic array metadata. All callers (line 4139 onwards) correctly consume the new structure.
928-932: LGTM: Consistent formatting improvements.Multiple location extraction and formatting improvements throughout the file enhance readability and maintain consistent code style without any functional changes.
Also applies to: 949-953, 974-978, 3095-3097, 3394-3396, 3615-3618, 4369-4371
2207-2209: LGTM: Defensive type checking.The literal value extraction properly validates both the AST node type and the runtime type of the value, ensuring type safety when extracting custom tool metadata.
Also applies to: 2222-2224
3884-3983: Overall assessment: Well-structured implementation of dynamic parallel detection.The changes to
findArrayElementsand related methods successfully implement detection of dynamic parallel execution from.map()operations. The code is type-safe, handles the main use cases correctly, and includes appropriate defensive checks.Key strengths:
- Clear separation of dynamic (variable-sourced) vs static (literal) parallel execution
- Conservative approach for detectability (Identifier sources only)
- Proper propagation of metadata through structured return types
- Type-safe implementation that will catch schema mismatches at compile time
The only minor limitation is the incomplete handling of rest elements in array destructuring (already noted in separate comment).
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bubble-runtime/src/extraction/BubbleParser.ts (1)
3889-3982: Schema is correct, but test coverage for non-dynamic cases should explicitly verifyisDynamic = false, and RestElement handling needs explicit tests.The
ParallelExecutionWorkflowNodetype inpackages/bubble-shared-schemas/src/bubble-definition-schema.tscorrectly declares bothisDynamic?: booleanandsourceArray?: stringas optional properties.The test "should identify dynamic parallel execution from mapped array" properly verifies dynamic detection with
expect(parallelNode.isDynamic).toBe(true)andexpect(parallelNode.sourceArray).toBe('items'). However, the existing non-dynamic test cases (direct array literal, .push() pattern, .map() with literal array, etc.) do not explicitly assert thatisDynamicremains false or undefined for non-dynamic scenarios. Additionally, RestElement handling in array destructuring is not explicitly tested to ensure the new dynamic properties behave correctly in this context.Recommend adding assertions to non-dynamic test cases to verify
isDynamicis false/undefined, and a test case for RestElement patterns in the context of dynamic parallel execution detection.
♻️ Duplicate comments (1)
packages/bubble-runtime/src/extraction/BubbleParser.ts (1)
4109-4138: Excellent! RestElement handling implemented as suggested.The code now properly captures both regular identifiers and rest elements from array destructuring patterns in Promise.all declarations:
const [a, b] = await Promise.all([...])→ captures['a', 'b']const [a, ...rest] = await Promise.all([...])→ captures['a', 'rest']This directly addresses the previous review comment and implements the suggested enhancement. The implementation correctly checks for
RestElementtype and extracts theargument.namewhen the argument is an Identifier.Based on learnings, this resolves the concern raised in the previous review about missing rest element handling.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bubble-runtime/src/extraction/BubbleParser.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-22T09:55:47.873Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.873Z
Learning: Applies to packages/bubble-runtime/src/extraction/README.md : Refer to packages/bubble-runtime/src/extraction/README.md for information about bubble parsing, dependency graphs, and per-invocation cloning
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.ts
📚 Learning: 2025-12-19T03:17:06.825Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.825Z
Learning: Applies to packages/bubble-core/dist/**/*.d.ts : All external imports (except relative imports) must be removed from the bundled output to prevent circular processing and external dependency issues
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.ts
📚 Learning: 2025-12-19T03:17:06.825Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.825Z
Learning: Applies to packages/bubble-core/dist/**/*.d.ts : The type bundling system for Monaco Editor must process TypeScript declaration files (.d.ts) and inline all dependencies into a single self-contained bundle in `packages/bubble-core/dist/bubble-bundle.d.ts`
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.ts
🧬 Code graph analysis (1)
packages/bubble-runtime/src/extraction/BubbleParser.ts (2)
packages/bubble-shared-schemas/src/types.ts (1)
BubbleName(51-100)packages/bubble-shared-schemas/src/bubble-definition-schema.ts (1)
WorkflowNode(410-419)
🔇 Additional comments (6)
packages/bubble-runtime/src/extraction/BubbleParser.ts (6)
322-327: LGTM! Type extension for detailed bubble dependencies.The addition of the optional
bubbleDependenciesDetailedproperty appropriately extends the metadata structure to support hierarchical dependency information with tools.
544-545: Also applies to: 1928-1932, 1949-1953, 1974-1978, 3095-3097, 3394-3396, 3615-3618, 4375-4377
2207-2207: LGTM! Proper type guards for Literal node values.The type checking ensures
nameProp.value.valueanddescProp.value.valueare strings before assignment, preventing runtime type errors.Also applies to: 2222-2222
3022-3052: LGTM! IIFE pattern improves branch handling readability.The use of immediately-invoked function expressions for
elseBranchandcatchBlockconstruction maintains clean conditional logic while properly handling BlockStatement, nested IfStatement, and single-statement cases.Also applies to: 3223-3238
4145-4150: LGTM! Dynamic metadata properly integrated into parallel execution nodes.The implementation correctly:
- Captures the structured result from
findArrayElements(line 4145)- Iterates over
arrayInfo.elements(line 4152)- Returns early with
isDynamic: trueandsourceArraymetadata when detected (lines 4177-4187)- Falls through to standard parallel_execution for non-dynamic cases
The early return pattern ensures dynamic arrays get the additional metadata while static arrays remain unchanged. This aligns with the schema updates mentioned in the PR objectives.
Also applies to: 4152-4152, 4177-4187
3889-3893: Dynamic array detection correctly implements Identifier-based.map()pattern detection.The implementation successfully detects Promise.all arrays sourced from simple
.map()operations on identifiers (e.g.,items.map(...)) and captures source array metadata. The return type structure{ elements, isDynamic, sourceArray }properly propagates dynamic detection downstream.The scope limitation to
Identifiersource types (excludingMemberExpressionlikethis.items.map()orCallExpressionlikegetItems().map()) aligns with actual usage patterns in the codebase, where all Promise.all examples use direct identifier references. This limitation is acceptable for the current requirements.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bubble-runtime/src/extraction/BubbleParser.ts (2)
3950-3955: Consider supporting MemberExpression sources for dynamic parallel detection.Currently, only
Identifiersources are marked as dynamic (e.g.,items.map(...)). Sources likethis.items.map(...)orobj.array.map(...)(which would havesourceArray.type === 'MemberExpression') won't be detected as dynamic, even though they represent the same pattern.If this limitation is intentional for the MVP, consider documenting it. Otherwise, you may want to extend support:
🔎 Potential enhancement for MemberExpression sources
const sourceArray = decl.init.callee.object; if (sourceArray.type === 'Identifier') { sourceArrayName = sourceArray.name; isDynamic = true; +} else if (sourceArray.type === 'MemberExpression') { + // Extract the full member expression as source (e.g., "this.items") + sourceArrayName = this.bubbleScript.substring( + sourceArray.range![0], + sourceArray.range![1] + ); + isDynamic = true; }
928-932: LGTM: Formatting improvements for location extraction.The conversion of inline location object literals to multi-line format improves readability without changing functionality. These are pure refactoring changes.
Also applies to: 949-952, 974-977, 1928-1932, 1949-1953, 1974-1978, 3394-3396
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bubble-runtime/src/extraction/BubbleParser.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-22T09:55:47.873Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.873Z
Learning: Applies to packages/bubble-runtime/src/extraction/README.md : Refer to packages/bubble-runtime/src/extraction/README.md for information about bubble parsing, dependency graphs, and per-invocation cloning
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.ts
📚 Learning: 2025-12-19T03:17:06.825Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.825Z
Learning: Applies to packages/bubble-core/dist/**/*.d.ts : The type bundling system for Monaco Editor must process TypeScript declaration files (.d.ts) and inline all dependencies into a single self-contained bundle in `packages/bubble-core/dist/bubble-bundle.d.ts`
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.ts
📚 Learning: 2025-12-19T03:17:06.825Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.825Z
Learning: Applies to packages/bubble-core/dist/**/*.d.ts : All external imports (except relative imports) must be removed from the bundled output to prevent circular processing and external dependency issues
Applied to files:
packages/bubble-runtime/src/extraction/BubbleParser.ts
🧬 Code graph analysis (1)
packages/bubble-runtime/src/extraction/BubbleParser.ts (2)
packages/bubble-shared-schemas/src/types.ts (1)
BubbleName(51-100)packages/bubble-shared-schemas/src/bubble-definition-schema.ts (1)
WorkflowNode(410-419)
🔇 Additional comments (2)
packages/bubble-runtime/src/extraction/BubbleParser.ts (2)
4124-4146: Well done addressing the rest element handling!The array destructuring logic now correctly handles both simple identifiers and rest elements (like
const [a, ...rest] = await Promise.all([...])), addressing the concern raised in the previous review.
4185-4195: Schema alignment verified for dynamic parallel execution fields.The schema in
packages/bubble-shared-schemas/src/bubble-definition-schema.tscorrectly declaresisDynamic?: booleanandsourceArray?: stringas optional fields onParallelExecutionWorkflowNode. The code at lines 4185-4195 properly returns these fields, and test coverage inBubbleParser.test.tsconfirms downstream consumers correctly handle them.
2 participants
Summary
This PR implements two critical fixes for the BubbleLab workflow visualization and Pearl AI agent integration:
1. Promise.all Visualization for Dynamic Maps (Issue #241)
workflowToSteps.tsto properly handle and visualize dynamic parallel execution from.map()operationsisDynamicflag for parallel nodes created from mapped arrays2. Pearl JSON Response Parsing (Issue #238)
parseJsonWithFallbacks()to handle:pearl.tsto extract valid JSON even from failed agent executionsImplementation Details
Files Modified:
apps/bubble-studio/src/utils/workflowToSteps.ts- Dynamic parallel step visualizationapps/bubblelab-api/src/services/ai/pearl.ts- JSON parsing with fallbackspackages/bubble-runtime/src/extraction/BubbleParser.ts- Dynamic array detection in Promise.allpackages/bubble-shared-schemas/src/bubble-definition-schema.ts- Schema updates for isDynamic and sourceArraypackages/bubble-runtime/src/extraction/BubbleParser.test.ts- Test cases for dynamic map detectionRelated Issues
Fixes #238, Fixes #241
Type of Change
Checklist
pnpm checkand all tests passSummary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.