fix(bubble-runtime): add Promise.all instrumentation support with map callbacks#250
fix(bubble-runtime): add Promise.all instrumentation support with map callbacks#250
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds conditional token-usage logging, call-site deduplication and concise-arrow inline replacement for Promise.all elements, propagates callText through injector, records dynamic-import parsing errors, fixes placeholder restoration in script sanitization, adds two complex test fixtures, and updates tests and studio hooks related to revalidation and updatedAt handling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Comment |
Deploying bubblelab-documentation with
|
| Latest commit: |
6e5b15c
|
| Status: | ⚡️ Build in progress... |
Suggested PR title from PearlTitle: Body: SummaryEnhances the BubbleParser to fully support Promise.all patterns with array.map() callbacks, including both expression-body and block-body arrow functions. Fixes invocation index double-counting and improves logging controls. ChangesParser Enhancements
Logger Injection
Bug Fixes
Testing
Technical DetailsNew helper methods in BubbleParser:
Breaking ChangesNone Related IssuesCloses issues related to Promise.all with .map() not being detected as parallel execution |
zhubzy
changed the title
zhubzy
changed the title
Deploying bubble-studio with
|
| Latest commit: |
779e9ce
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2c485527.bubble-studio.pages.dev |
| Branch Preview URL: | https://fix-more-loggin-instrumentat.bubble-studio.pages.dev |
There was a problem hiding this comment.
Pull request overview
This PR enhances logging instrumentation for BubbleFlow execution, specifically improving support for Promise.all patterns and fixing string replacement bugs. The changes enable better tracking and debugging of parallel execution flows.
Key Changes
- Added support for parsing and instrumenting Promise.all patterns with .map() callbacks in arrow function expression bodies
- Fixed string replacement bug in sanitize-script that could cause issues with special characters like
$ - Improved error tracking by logging dynamic import failures to parsingErrors array
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/bubble-runtime/tests/fixtures/string-literal-complex.ts | New test fixture demonstrating complex Amazon product sourcing flow with nested API calls |
| packages/bubble-runtime/tests/fixtures/promises-all-map.ts | New test fixture for execution monitoring using Promise.all with .map() pattern |
| packages/bubble-runtime/tests/fixtures/promise-all-patterns.ts | New comprehensive test fixture covering various Promise.all usage patterns |
| packages/bubble-runtime/tests/fixtures/index.ts | Exports for new test fixtures |
| packages/bubble-runtime/src/utils/sanitize-script.ts | Fixed replacement string handling to avoid $ special character interpretation |
| packages/bubble-runtime/src/runtime/BubbleRunner.ts | Added parsing error tracking for dynamic import failures |
| packages/bubble-runtime/src/runtime/BubbleRunner.test.ts | Added tests for new Promise.all patterns and re-enabled previously skipped test |
| packages/bubble-runtime/src/injection/LoggerInjector.ts | Enhanced Promise.all element logging with inline replacement support for arrow functions |
| packages/bubble-runtime/src/extraction/BubbleParser.ts | Added .map() callback parsing, deduplication logic for invocation indexes, and helper methods |
| packages/bubble-runtime/src/extraction/BubbleParser.test.ts | Added comprehensive tests for Promise.all parsing patterns |
| packages/bubble-core/src/logging/BubbleLogger.ts | Removed debug console.log and gated token usage logging behind feature flag |
Comments suppressed due to low confidence (2)
packages/bubble-runtime/src/injection/LoggerInjector.ts:717
- There is code duplication in this method. The async IIFE construction code (lines 678-690) is duplicated in the wrappedLines array (lines 706-717). Consider extracting this into a helper function or reusing the asyncIIFE variable that is already constructed. The duplication makes maintenance harder and increases the risk of inconsistencies if one block is updated without the other.
const wrappedLines = [
`${indentation}(async () => {`,
`${innerIndent}const __functionCallStart_${variableId} = Date.now();`,
`${innerIndent}const ${argsVar} = ${argsArray};`,
`${innerIndent}__bubbleFlowSelf.logger?.logFunctionCallStart(${variableId}, '${methodName}', ${argsVar}, ${lineNumber});`,
`${innerIndent}const ${prevInvocationVar} = __bubbleFlowSelf?.__setInvocationCallSiteKey?.(${callSiteKeyLiteral});`,
`${innerIndent}const ${resultVar} = await this.${methodName}(${args});`,
`${innerIndent}const ${durationVar} = Date.now() - __functionCallStart_${variableId};`,
`${innerIndent}__bubbleFlowSelf.logger?.logFunctionCallComplete(${variableId}, '${methodName}', ${resultVar}, ${durationVar}, ${lineNumber});`,
`${innerIndent}__bubbleFlowSelf?.__restoreInvocationCallSiteKey?.(${prevInvocationVar});`,
`${innerIndent}return ${resultVar};`,
`${indentation}})()${trailingComma ? ',' : ''}`,
packages/bubble-runtime/src/extraction/BubbleParser.ts:4076
- The
findReturnStatementsmethod walks the entire block statement AST and collects all return statements, but it doesn't stop at nested function boundaries. This means if there's a nested arrow function or function expression with its own return statement, those will be incorrectly included. The walk should skip traversing into nested ArrowFunctionExpression and FunctionExpression nodes to only collect return statements from the current function scope.
private findReturnStatements(
block: TSESTree.BlockStatement
): TSESTree.ReturnStatement[] {
const returns: TSESTree.ReturnStatement[] = [];
const walk = (node: TSESTree.Node) => {
if (node.type === 'ReturnStatement') {
returns.push(node);
}
for (const key in node) {
const child = (node as any)[key];
if (Array.isArray(child)) child.forEach(walk);
else if (child?.type) walk(child);
}
};
walk(block);
return returns;
}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
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/injection/LoggerInjector.ts (1)
355-377: Unconditionally wrapping arrow function callbacks in async IIFE changes semantics for non-Promise.all casesThe code correctly identifies arrow functions with concise bodies in
BubbleParser.ts(line 1277) and routes them throughinjectPromiseAllElementLoggingfor inline instrumentation viacallText. However, there is a semantic risk:In
BubbleParser.ts, any arrow function with a concise body (e.g.,arr.map(item => this.transform(item))) getsinvocationContext = 'promise_all_element'regardless of whether that arrow is actually part of a Promise.all call. Only a separate check (line 1331) validates true Promise.all array elements viaisPromiseAllArrayElement.In
LoggerInjector.tslines 685 and 713,injectPromiseAllElementLoggingunconditionally injectsawait this.${methodName}(...)into the async IIFE wrapper. ThehasAwaitflag is captured inBubbleParser.tsbut never passed through to the injection function, so the original await semantics are lost.Impact: For
items.map(item => this.sync(item))wheresyncis synchronous and not awaited, after instrumentation the callback returnsPromise[]instead ofValue[], which breaks downstream code expecting the original return type.Recommendation: Either narrow arrow function detection to confirm the arrow is actually used in Promise.all context, or pass
hasAwaitand method async information toinjectPromiseAllElementLoggingand only injectawaitwhen the original call was already asynchronous, falling back to synchronous wrapping for synchronous callbacks.
🧹 Nitpick comments (5)
packages/bubble-runtime/src/runtime/BubbleRunner.test.ts (1)
15-18: Parsing error logging inexpectValidScriptLogging
runner.bubbleScript.parsingErrorsbefore asserting length is 0 is useful for debugging dynamic import / parsing failures; behavior is fine as-is. If you ever reuse this helper for negative/expected-failure tests, consider wiring this under thelogOnErrorflag to keep test output quieter, but not required now.packages/bubble-core/src/logging/BubbleLogger.ts (1)
5-5: Console token-usage logging flag is good; consider making it configurableGating the
logTokenUsageinfo log behindSHOULD_ENABLE_TOKEN_USAGE_LOGGING_IN_CONSOLEremoves noisy per-call logs while preserving aggregated usage viaaddServiceUsage, which is a nice improvement. Longer term, you may want this to be driven byLoggerConfigor an env toggle rather than a hardcoded constant so it can be enabled in specific environments without code changes.Also applies to: 376-388
packages/bubble-runtime/tests/fixtures/promises-all-map.ts (1)
1-181: ExecutionMonitorCron fixture correctly exercises Promise.all + map patternThe fixture cleanly wires together PostgreSQLBubble, AIAgentBubble, and SlackBubble with a
Promise.all(userGroups.entries().map(...))pattern, which is ideal for exercising the new parser/injector behavior. The parameterized query and error handling are reasonable for a test fixture. If you later promote this pattern to production code, you might tighten theRecord<string, unknown>typings (e.g., a typed row interface forexecution.email,flow_name, etc.), but it’s not required for test coverage here.packages/bubble-runtime/src/runtime/BubbleRunner.ts (1)
462-472: Recording dynamic import failures inparsingErrorsis useful—watch exposure of raw messagesPushing
Dynamic import failed: <message>intothis.bubbleScript.parsingErrorsmakes it much easier for tests and tooling (e.g.,expectValidScript) to surface syntax/import issues, which is a nice improvement. Just double-check whetherparsingErrorsare ever sent to clients or logs where absolute temp file paths from the import error message might be visible; if so, consider passing the message through your existinggetSafeErrorMessage-style sanitization or replacing it with a more generic description.packages/bubble-runtime/tests/fixtures/string-literal-complex.ts (1)
124-130: Consider simplifying the type assertion.The nested type assertion is fragile and could mask runtime type mismatches. A cleaner approach would extract the suppliers with optional chaining and a fallback.
🔎 Proposed simplification
- return ( - ( - result.data?.result as { - suppliers: { name: string; price: number; url: string }[]; - } - )?.suppliers || [] - ); + const data = result.data?.result as { suppliers?: { name: string; price: number; url: string }[] } | undefined; + return data?.suppliers ?? [];
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/bubble-core/src/logging/BubbleLogger.tspackages/bubble-runtime/src/extraction/BubbleParser.tspackages/bubble-runtime/src/injection/LoggerInjector.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.tspackages/bubble-runtime/src/runtime/BubbleRunner.tspackages/bubble-runtime/src/utils/sanitize-script.tspackages/bubble-runtime/tests/fixtures/index.tspackages/bubble-runtime/tests/fixtures/promises-all-map.tspackages/bubble-runtime/tests/fixtures/string-literal-complex.ts
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
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
📚 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/runtime/BubbleRunner.tspackages/bubble-runtime/tests/fixtures/index.tspackages/bubble-runtime/src/extraction/BubbleParser.tspackages/bubble-runtime/src/runtime/BubbleRunner.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/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/runtime/BubbleRunner.tspackages/bubble-runtime/tests/fixtures/index.tspackages/bubble-runtime/tests/fixtures/string-literal-complex.tspackages/bubble-runtime/src/runtime/BubbleRunner.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/scripts/bubble-bundler.ts : The bundler script at `packages/bubble-core/scripts/bubble-bundler.ts` must use absolute file paths as cache keys to prevent collisions between packages with identically named files
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.tspackages/bubble-runtime/tests/fixtures/index.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.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/runtime/BubbleRunner.tspackages/bubble-runtime/src/extraction/BubbleParser.tspackages/bubble-runtime/src/runtime/BubbleRunner.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 : 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/runtime/BubbleRunner.tspackages/bubble-runtime/tests/fixtures/index.tspackages/bubble-runtime/tests/fixtures/string-literal-complex.tspackages/bubble-runtime/src/runtime/BubbleRunner.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 apps/bubble-studio/src/components/MonacoEditor.tsx : The Monaco Editor integration in `apps/bubble-studio/src/components/MonacoEditor.tsx` must fetch the bundled types from `/bubble-types.txt`, wrap them in a module declaration for `bubblelab/bubble-core`, and add them to Monaco's TypeScript type system
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.tspackages/bubble-runtime/tests/fixtures/index.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 apps/bubble-studio/public/bubble-types.txt : The bundle file at `apps/bubble-studio/public/bubble-types.txt` must be kept synchronized with `packages/bubble-core/dist/bubble-bundle.d.ts` after each build
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.tspackages/bubble-runtime/tests/fixtures/index.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: The bundle process must run the TypeScript compiler (`tsc`) before executing the bundler script (`tsx scripts/bubble-bundler.ts`) to ensure all .d.ts files are generated
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.tspackages/bubble-runtime/src/runtime/BubbleRunner.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: When adding new types to `bubblelab/shared-schemas`, they are automatically included in the bundle without requiring manual configuration changes
Applied to files:
packages/bubble-runtime/tests/fixtures/index.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-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:
packages/bubble-runtime/tests/fixtures/promises-all-map.tspackages/bubble-runtime/tests/fixtures/string-literal-complex.tspackages/bubble-runtime/src/runtime/BubbleRunner.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 **/bubblelab-api/**/*.test.{js,ts} : Reference webhook.test as the example for how backend tests should be written
Applied to files:
packages/bubble-runtime/tests/fixtures/string-literal-complex.tspackages/bubble-runtime/src/runtime/BubbleRunner.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-runtime/tests/fixtures/string-literal-complex.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/runtime/BubbleRunner.test.ts
🧬 Code graph analysis (1)
packages/bubble-runtime/tests/fixtures/promises-all-map.ts (2)
packages/bubble-shared-schemas/src/trigger.ts (1)
CronEvent(58-62)packages/bubble-core/src/index.ts (2)
BubbleFlow(19-19)PostgreSQLBubble(34-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: Cloudflare Pages: bubblelab-documentation
🔇 Additional comments (6)
packages/bubble-runtime/src/runtime/BubbleRunner.test.ts (1)
523-560: New flow tests give good coverage for map + complex string casesRe‑enabling the
mapping-function-calltest and adding thepromises-all-mapandstring-literal-complexflow tests is a solid way to exercise the new parser/injector behavior. The pattern ofrunAll+expectValidScript+resultexistence aligns with existing integration-style tests and looks consistent.packages/bubble-runtime/src/utils/sanitize-script.ts (1)
57-64: Safe placeholder restoration avoids$replacement pitfallsSwitching placeholder restoration to a function-based replacement correctly avoids
$-pattern interpretation in replacement strings while preserving existing behavior. This is a solid, low-risk fix.packages/bubble-runtime/tests/fixtures/index.ts (1)
72-74: New fixtures wired correctly into the registryAdding
string-literal-complexandpromises-all-mapto thefixturesmap keepsFixtureNamein sync with the new.tsfiles and allows the new BubbleRunner tests to load them viagetFixturewithout any further changes. Looks good.packages/bubble-runtime/tests/fixtures/string-literal-complex.ts (3)
1-7: LGTM!Imports are appropriate for the workflow implementation. All imported symbols are used in the file.
9-44: LGTM!Well-documented interfaces with proper typing. The
@canBeFileannotations and field descriptions provide good clarity for payload configuration.
161-179: Sequential processing is acceptable for this fixture.The
for...ofloop processes products sequentially. WhilePromise.allwould be more performant, this pattern is appropriate for a test fixture that validates sequential async instrumentation paths (complementing thepromises-all-map.tsfixture that covers parallel patterns).
| /** | ||
| * Track which call expressions have been assigned an invocation index. | ||
| * Key: `methodName:startOffset` (using AST range start position) | ||
| * Value: the assigned invocation index | ||
| * This prevents double-counting when the same call site is processed multiple times | ||
| * (e.g., once in .map() callback processing, again in Promise.all resolution) | ||
| */ | ||
| private processedCallSiteIndexes: Map<string, number> = new Map(); |
There was a problem hiding this comment.
Guard against return; (no value) in extractCallbackExpression to avoid crashes
The new helpers for .map-based Promise.all detection and call-site dedupe generally look good, and using processedCallSiteIndexes keyed by methodName:startOffset is a reasonable way to reuse invocation indices across multiple passes. One edge case, though:
- In
extractCallbackExpression, when handling block-bodied callbacks you do:
const returns = this.findReturnStatements(callback.body);
return returns[0]?.argument as TSESTree.Expression | null;But ReturnStatement.argument can be null (e.g., return;). In that case callbackExpr will be null, later pushed into elements in findArrayElements, and eventually passed to detectFunctionCall / buildParallelExecutionNode, which expect a real TSESTree.Expression and will blow up on expr.type.
Suggestion: treat callbacks with only return; (or no return at all) as having no usable element expression:
const returns = this.findReturnStatements(callback.body);
const firstWithArg = returns.find(r => r.argument != null);
if (!firstWithArg || !firstWithArg.argument) {
return null;
}
return firstWithArg.argument as TSESTree.Expression;and then skip those entries in findArrayElements when callbackExpr is null. That keeps the parser robust for map callbacks that do side effects or explicit return; without a value.
Also applies to: 267-268, 3734-3737, 4198-4231, 3975-3993, 4061-4076
🤖 Prompt for AI Agents
In packages/bubble-runtime/src/extraction/BubbleParser.ts around lines 56 to 63,
guard extractCallbackExpression from returning a null/undefined
ReturnStatement.argument (e.g., `return;`) by selecting the first
ReturnStatement that has a non-null argument and returning its argument,
otherwise return null; then ensure findArrayElements skips null callbackExprs so
you don't push null into elements; apply the same pattern to the other mention
locations (lines ~267-268, 3734-3737, 3975-3993, 4061-4076, 4198-4231) to avoid
passing null expressions into detectFunctionCall/buildParallelExecutionNode.
| const supplierResearch = new ResearchAgentTool({ | ||
| task: `Find suppliers for the product "${productName}" that offer it at a lower price than ${currency}${amazonPrice}. | ||
| Check AliExpress, Alibaba, and general wholesalers. Provide the supplier name, their price, and the product URL.`, | ||
| model: 'google/gemini-3-pro-preview', | ||
| expectedResultSchema: z.object({ | ||
| suppliers: z | ||
| .array( | ||
| z.object({ | ||
| name: z.string().describe('Name of the supplier or website'), | ||
| price: z.number().describe('The price offered by this supplier'), | ||
| url: z | ||
| .string() | ||
| .describe('Direct URL to the product on the supplier website'), | ||
| }) | ||
| ) | ||
| .describe('A list of alternative suppliers found'), | ||
| }), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ResearchAgentTool implementation and model definitions
echo "=== Searching for ResearchAgentTool class definition ==="
rg -n "class ResearchAgentTool|interface ResearchAgentTool" --type=ts
echo -e "\n=== Searching for model identifiers and validation ==="
rg -n "google/gemini|model.*:" --type=ts -A 2 | head -50
echo -e "\n=== Checking current file for model usage ==="
cat -n packages/bubble-runtime/tests/fixtures/string-literal-complex.ts | sed -n '98,115p'
echo -e "\n=== Searching for other ResearchAgentTool usages ==="
rg -n "ResearchAgentTool" --type=ts -B 2 -A 2Repository: bubblelabai/BubbleLab
Length of output: 50377
🌐 Web query:
Google Gemini API model identifiers 2024 2025
💡 Result:
Here are the common Gemini API model identifiers used across 2024–2025 (major stable, preview, experimental names and notable variants):
- gemini-1.5-pro, gemini-1.5-pro-002, gemini-1.5-pro-latest. [1][2]
- gemini-1.5-flash, gemini-1.5-flash-002, gemini-1.5-flash-8b (and experimental variants). [1][2]
- gemini-2.0-flash, gemini-2.0-flash-001, gemini-2.0-flash-lite-001. [2][3]
- gemini-2.5-pro, gemini-2.5-flash, gemini-2.5-flash-lite (and preview suffixes like -preview-09-2025). [2][3][5]
- gemini-2.5-flash-image (aka “nano banana”) and gemini-2.5-flash-image-preview. [2][5]
- gemini-exp-* / gemini-exp-1114 / gemini-exp-1121 (experimental releases noted in 2024–2025). [1][2]
Notes / sources:
- Official Gemini model page and naming/version guidance. [1]
- Gemini API release notes / changelog (lists specific version IDs, preview/experimental releases, and deprecations through 2025). [2]
- Vertex AI / Firebase docs showing model IDs available via Vertex/Firebase (Gemini 2.0 / 2.5 names and lifecycle). [3][5]
If you want, I can: (a) return a machine-readable list (JSON) of these IDs, or (b) query the live models.list endpoint and return the exact, currently-available model IDs for your project/region.
Use a valid Gemini model identifier.
The model 'google/gemini-3-pro-preview' is not listed in the official Gemini API documentation. Replace it with a currently supported model such as 'google/gemini-2.5-pro' or 'google/gemini-2.5-flash'.
🤖 Prompt for AI Agents
In packages/bubble-runtime/tests/fixtures/string-literal-complex.ts around lines
98 to 115, the test uses an unsupported Gemini model identifier
'google/gemini-3-pro-preview'; replace it with a valid, supported model such as
'google/gemini-2.5-pro' or 'google/gemini-2.5-flash' in the ResearchAgentTool
instantiation so the test uses an official Gemini model name recognized by the
API.
| async handle(payload: AmazonSourcingPayload): Promise<AmazonSourcingOutput> { | ||
| const { | ||
| amazonCategoryUrl = 'https://www.amazon.com/Best-Sellers/zgbs', | ||
| maxProducts = 5, | ||
| } = payload; |
There was a problem hiding this comment.
Unused sheetName parameter.
The sheetName field is defined in AmazonSourcingPayload but is not destructured or used in the handle method. If this is intentional (e.g., placeholder for future functionality), consider adding a comment. Otherwise, remove it from the interface.
🤖 Prompt for AI Agents
In packages/bubble-runtime/tests/fixtures/string-literal-complex.ts around lines
137 to 141, the payload defines a sheetName field in AmazonSourcingPayload that
is not being destructured or used in handle; either add sheetName to the
destructuring (e.g., const { amazonCategoryUrl = ..., maxProducts = 5, sheetName
} = payload) and use it appropriately (or add a brief comment explaining it’s
intentionally unused), or remove sheetName from the AmazonSourcingPayload
interface if it’s not needed; update tests/types accordingly to keep signatures
consistent.
3 participants
Summary
Related Issues
Type of Change
Checklist
pnpm checkand all tests passScreenshots (if applicable)
Additional Context
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.