Feat/google picker integration #234
Conversation
📝 WalkthroughWalkthroughAdds Google Drive Picker support to bubble-studio: environment config and script load, a shared picker loader hook, token caching utilities, a googlePicker service for OAuth + Picker, a GoogleFilePicker React component, and integration into input field rendering plus schema fields for Drive folder IDs. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as GoogleFilePicker UI
participant Hook as useGooglePicker
participant TokenSvc as requestPickerAccessToken
participant GoogleOAuth as Google OAuth/GSI
participant PickerAPI as Google Picker API
participant Form as InputFieldsRenderer
User->>UI: Click "Pick from Google Drive"
UI->>Hook: ensure API loaded
alt API not loaded
Hook->>Hook: load scripts (gsi, gapi)
end
UI->>UI: check session cache
alt cached token valid
UI->>PickerAPI: showPicker(cachedToken)
else
UI->>TokenSvc: requestPickerAccessToken()
TokenSvc->>GoogleOAuth: init token client & request token
GoogleOAuth->>User: OAuth consent (if required)
GoogleOAuth->>TokenSvc: return access token
TokenSvc->>UI: return token
UI->>PickerAPI: showPicker(token)
end
User->>PickerAPI: select file
PickerAPI->>UI: onSelect(fileId,fileName)
UI->>Form: handleGoogleFileSelect(fieldName,fileId,fileName)
Form->>Form: populate field with ID
Form->>User: show selected file info
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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 |
Suggested PR title from PearlTitle: Body: ChangesNew Components
Enhanced Features
ConfigurationRequires two new environment variables:
See Implementation Details
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/bubble-studio/index.html (1)
10-14: Consider addingasyncto the api.js script to avoid render-blocking.The
api.jsscript loads synchronously, which may delay initial page render. Additionally,useGooglePicker.tsalready dynamically loads these scripts as a fallback. Consider:
- Adding
asyncto avoid render-blocking:- <script src="https://apis.google.com/js/api.js"></script> + <script src="https://apis.google.com/js/api.js" async></script>
- Alternatively, rely solely on the dynamic loading in
useGooglePicker.tsand remove these script tags to reduce initial page load time.apps/bubble-studio/src/utils/googlePickerCache.ts (1)
22-48: Consider adding validation after JSON.parse to handle corrupted storage.If
sessionStoragecontains malformed data (e.g., from a previous version or manual tampering),JSON.parsecould return an object missing expected properties, causing subtle issues.🔎 Proposed fix
const token: CachedPickerToken = JSON.parse(cached); + + // Validate token structure + if (!token.accessToken || typeof token.expiresAt !== 'number') { + sessionStorage.removeItem(key); + continue; + } // Check if expired (with buffer)apps/bubble-studio/src/components/GoogleFilePicker.tsx (1)
39-54: Consider refetching credentials after modal closes or when credentials change elsewhere.The credentials are only fetched on component mount. If the user creates credentials via the modal and then closes and reopens the picker without remounting, the
googleCredentialsstate may become stale. WhilehandleCredentialCreatedupdates the local state, credentials created elsewhere (e.g., on the Credentials page) won't be reflected.This is a minor UX consideration and may be acceptable for the initial implementation.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/bubble-studio/.env.exampleapps/bubble-studio/index.htmlapps/bubble-studio/src/components/GoogleFilePicker.tsxapps/bubble-studio/src/components/InputFieldsRenderer.tsxapps/bubble-studio/src/env.tsapps/bubble-studio/src/hooks/useGooglePicker.tsapps/bubble-studio/src/services/googlePickerService.tsapps/bubble-studio/src/utils/googlePickerCache.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
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:
apps/bubble-studio/src/services/googlePickerService.ts
📚 Learning: 2025-12-19T03:16:54.994Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-19T03:16:54.994Z
Learning: Ensure that bubbles.json in apps/bubble-studio/public/ is a condensed definition of all bubbles and requires building/bundling to be updated
Applied to files:
apps/bubble-studio/index.html
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
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:
apps/bubble-studio/src/env.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
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:
apps/bubble-studio/src/env.ts
🧬 Code graph analysis (2)
apps/bubble-studio/src/components/InputFieldsRenderer.tsx (1)
apps/bubble-studio/src/components/GoogleFilePicker.tsx (1)
GoogleFilePicker(25-197)
apps/bubble-studio/src/components/GoogleFilePicker.tsx (7)
apps/bubble-studio/src/services/googlePickerService.ts (2)
GooglePickerFileType(35-39)showPicker(136-204)packages/bubble-shared-schemas/src/credential-schema.ts (1)
CredentialResponse(530-530)apps/bubble-studio/src/hooks/useGooglePicker.ts (1)
useGooglePicker(12-55)apps/bubble-studio/src/services/credentialsApi.ts (1)
credentialsApi(8-54)apps/bubble-studio/src/env.ts (2)
GOOGLE_API_KEY(34-35)GOOGLE_OAUTH_CLIENT_ID(36-37)apps/bubble-studio/src/utils/googlePickerCache.ts (1)
getCachedPickerToken(22-48)apps/bubble-studio/src/pages/CredentialsPage.tsx (1)
CreateCredentialModal(323-757)
🪛 dotenv-linter (4.0.0)
apps/bubble-studio/.env.example
[warning] 22-22: [UnorderedKey] The VITE_ANALYTICS_ENABLED key should go before the VITE_POSTHOG_API_KEY key
(UnorderedKey)
🔇 Additional comments (8)
apps/bubble-studio/.env.example (1)
24-42: LGTM! Well-documented Google Picker configuration.The environment variables are properly documented with clear setup instructions. The note about the API key being public and safe to expose in frontend is helpful context for developers.
apps/bubble-studio/src/env.ts (1)
33-37: LGTM!The new environment variable exports follow the established pattern in this file and correctly type them as
string | undefinedto handle cases where they may not be configured.apps/bubble-studio/src/components/InputFieldsRenderer.tsx (2)
275-301: LGTM! Clean helper functions for Google file field detection.The regex pattern and type mapping are well-structured. The field detection covers the expected Google file ID field naming conventions.
One minor suggestion: The
console.loginhandleGoogleFileSelectcould be removed in production or replaced with user-visible feedback (e.g., a toast notification), but this is fine for now during development.
1102-1113: LGTM! Well-integrated Google File Picker button.The conditional rendering based on
isGoogleFileFieldensures the picker only appears for relevant fields, and thedisabledprop correctly respects the execution state.apps/bubble-studio/src/utils/googlePickerCache.ts (1)
55-91: LGTM! Well-structured caching utilities.The caching implementation is clean with proper error handling. Using
sessionStorageis the right choice since tokens should not persist across browser sessions.apps/bubble-studio/src/components/GoogleFilePicker.tsx (2)
159-196: LGTM! Clean component structure with good UX considerations.The button properly handles multiple disabled states with informative title attributes. The modal integration follows the established pattern in the codebase.
190-190: The type assertion is valid.'GOOGLE_DRIVE_CRED'is a confirmed member of theCredentialTypeenum, defined inpackages/bubble-shared-schemas/src/types.ts. The assertion safely and appropriately casts the string literal to the enum type without compromising type safety.apps/bubble-studio/src/services/googlePickerService.ts (1)
136-204: LGTM on the overall picker flow structure.The picker creation and callback handling logic is well-structured. The callback properly handles both
PICKEDandCANCELactions, and errors are appropriately logged and re-thrown.
There was a problem hiding this comment.
Pull request overview
This PR implements a native Google Drive file picker integration for Bubble Studio, enabling users to browse and select Drive files directly from input fields. The implementation includes OAuth token caching, credential management integration, and automatic file type detection based on field names.
Key Changes:
- Added Google Picker API integration with token caching (1-hour expiration with 5-minute buffer)
- Implemented icon-only button for Google file ID fields with automatic file type detection
- Integrated with existing CreateCredentialModal for seamless credential management
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
apps/bubble-studio/src/components/GoogleFilePicker.tsx |
Main picker component handling OAuth flow, credential checks, and picker initialization |
apps/bubble-studio/src/hooks/useGooglePicker.ts |
Custom hook for loading Google Picker API with global state management |
apps/bubble-studio/src/services/googlePickerService.ts |
Core service for OAuth token requests and Picker UI display |
apps/bubble-studio/src/utils/googlePickerCache.ts |
Token caching utility using sessionStorage with expiration management |
apps/bubble-studio/src/components/InputFieldsRenderer.tsx |
Integration point adding Google Drive picker button to eligible input fields |
apps/bubble-studio/src/env.ts |
Configuration exports for Google API key and OAuth client ID |
apps/bubble-studio/.env.example |
Documentation for required Google Cloud setup |
apps/bubble-studio/index.html |
Google Picker API script includes |
Issues Identified:
- Critical: Array access without bounds checking could cause runtime errors
- Critical: Incomplete loading state management in error paths
- Moderate: Security concerns with token storage in sessionStorage
- Moderate: Invalid Tailwind CSS class and potential false-positive field detection
- Moderate: Script loading performance impact
- Multiple: Console logging statements should be removed from production code
| continue; | ||
| } | ||
|
|
||
| console.log(`✅ Using cached token for ${token.accountEmail}`); |
There was a problem hiding this comment.
The console.log statement in production code should be removed or converted to a proper logging mechanism. Console logs in production can expose sensitive information and clutter the browser console. Consider using a logging service or removing this entirely.
| console.log('✅ Google Picker API already loaded'); | ||
| return true; | ||
| } | ||
|
|
||
| // Load Google API client if not already loaded | ||
| if (!window.gapi) { | ||
| await loadScript('https://apis.google.com/js/api.js'); | ||
| console.log('✅ Loaded gapi script'); | ||
| } | ||
|
|
||
| // Load Google Sign-In if not already loaded | ||
| if (!window.google) { | ||
| await loadScript('https://accounts.google.com/gsi/client'); | ||
| console.log('✅ Loaded Google Sign-In script'); | ||
| } | ||
|
|
||
| // Wait for gapi to be ready | ||
| await new Promise<void>((resolve) => { | ||
| if (window.gapi) { | ||
| resolve(); | ||
| } else { | ||
| // Retry a few times | ||
| let attempts = 0; | ||
| const interval = setInterval(() => { | ||
| if (window.gapi || attempts++ > 20) { | ||
| clearInterval(interval); | ||
| resolve(); | ||
| } | ||
| }, 100); | ||
| } | ||
| }); | ||
|
|
||
| // Load the Picker library | ||
| await new Promise<void>((resolve, reject) => { | ||
| const gapi = window.gapi as { | ||
| load: ( | ||
| api: string, | ||
| options: { | ||
| callback: () => void; | ||
| onerror: () => void; | ||
| timeout: number; | ||
| ontimeout: () => void; | ||
| } | ||
| ) => void; | ||
| }; | ||
| gapi.load('picker', { | ||
| callback: () => { | ||
| console.log('✅ Loaded Google Picker library'); | ||
| resolve(); | ||
| }, | ||
| onerror: () => { | ||
| console.error('❌ Failed to load Google Picker library'); | ||
| reject(new Error('Failed to load Picker library')); | ||
| }, | ||
| timeout: 10000, // 10 second timeout | ||
| ontimeout: () => { | ||
| console.error('❌ Timeout loading Google Picker library'); | ||
| reject(new Error('Timeout loading Picker library')); | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| // Verify picker is available | ||
| if (!window.google?.picker) { | ||
| throw new Error( | ||
| 'Picker library loaded but window.google.picker is undefined' | ||
| ); | ||
| } | ||
|
|
||
| console.log('✅ Google Picker API fully loaded and ready'); |
There was a problem hiding this comment.
Console.log statements should be removed from production code. These debug logs can clutter the browser console and may expose implementation details. Consider using a proper logging service or removing them entirely.
apps/bubble-studio/index.html
Outdated
| <script src="https://apis.google.com/js/api.js"></script> | ||
| <script src="https://accounts.google.com/gsi/client" async defer></script> |
There was a problem hiding this comment.
The Google Picker API scripts are loaded synchronously in the HTML head, which can block the initial page rendering. Consider moving these script tags to the bottom of the body tag or using async/defer attributes more effectively. Note that one script already has async/defer, but the first one (api.js) does not, which will block parsing.
| export const GOOGLE_API_KEY: string | undefined = import.meta.env | ||
| .VITE_GOOGLE_API_KEY; | ||
| export const GOOGLE_OAUTH_CLIENT_ID: string | undefined = import.meta.env | ||
| .VITE_GOOGLE_OAUTH_CLIENT_ID; | ||
|
|
There was a problem hiding this comment.
The API key and OAuth client ID environment variables lack validation. If these values are undefined, the application will attempt to use them, leading to runtime errors or confusing behavior. Consider adding runtime validation at application startup to ensure these required configuration values are present, or provide clear error messages when they're missing during initialization rather than at usage time.
| export const GOOGLE_API_KEY: string | undefined = import.meta.env | |
| .VITE_GOOGLE_API_KEY; | |
| export const GOOGLE_OAUTH_CLIENT_ID: string | undefined = import.meta.env | |
| .VITE_GOOGLE_OAUTH_CLIENT_ID; | |
| const requireEnv = (value: string | undefined, key: string): string => { | |
| if (typeof value !== 'string' || value.trim() === '') { | |
| throw new Error( | |
| `Environment variable ${key} is required but was not provided. ` + | |
| 'Please set it in your Vite environment configuration.', | |
| ); | |
| } | |
| return value; | |
| }; | |
| const rawGoogleApiKey = import.meta.env.VITE_GOOGLE_API_KEY; | |
| const rawGoogleOAuthClientId = import.meta.env.VITE_GOOGLE_OAUTH_CLIENT_ID; | |
| export const GOOGLE_API_KEY: string = requireEnv( | |
| rawGoogleApiKey, | |
| 'VITE_GOOGLE_API_KEY', | |
| ); | |
| export const GOOGLE_OAUTH_CLIENT_ID: string = requireEnv( | |
| rawGoogleOAuthClientId, | |
| 'VITE_GOOGLE_OAUTH_CLIENT_ID', | |
| ); |
| accountEmail, | ||
| }; | ||
|
|
||
| sessionStorage.setItem(key, JSON.stringify(token)); |
There was a problem hiding this comment.
Storing access tokens in sessionStorage could pose a security risk if the application is vulnerable to XSS attacks. While sessionStorage is more secure than localStorage (cleared on tab close), tokens could still be accessed by malicious scripts. Consider whether the token caching is necessary given the 5-minute expiry buffer, or implement additional security measures like token encryption.
| console.error('❌ Failed to load Google Picker library'); | ||
| reject(new Error('Failed to load Picker library')); | ||
| }, | ||
| timeout: 10000, // 10 second timeout |
There was a problem hiding this comment.
The magic number '10000' for the timeout lacks explanation. Consider extracting this to a named constant at the top of the file like 'PICKER_LOAD_TIMEOUT_MS' to make the code more maintainable and the timeout value easier to adjust.
| if (data.action === googlePicker.Action.PICKED) { | ||
| const file = data.docs![0]; |
There was a problem hiding this comment.
Accessing data.docs[0] without checking if the array exists or has elements could cause a runtime error. While data.docs is checked with optional chaining on line 180, accessing data.docs![0] directly assumes the array has at least one element. Add validation to ensure the array is not empty before accessing the first element.
| if (data.action === googlePicker.Action.PICKED) { | |
| const file = data.docs![0]; | |
| if ( | |
| data.action === googlePicker.Action.PICKED && | |
| data.docs && | |
| data.docs.length > 0 | |
| ) { | |
| const file = data.docs[0]; |
| // Global state - shared across all component instances | ||
| let isLoading = false; | ||
| let isLoaded = false; | ||
| let loadPromise: Promise<boolean> | null = null; | ||
|
|
||
| /** | ||
| * Custom hook to load and manage Google Picker API | ||
| * Loads the API once globally and shares state across all components | ||
| */ | ||
| export const useGooglePicker = () => { | ||
| const [apiLoaded, setApiLoaded] = useState(isLoaded); | ||
| const [error, setError] = useState<string | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| // If already loaded, just update state | ||
| if (isLoaded) { | ||
| setApiLoaded(true); | ||
| return; | ||
| } | ||
|
|
||
| // If currently loading, wait for the existing promise | ||
| if (isLoading && loadPromise) { | ||
| loadPromise.then((loaded) => { | ||
| setApiLoaded(loaded); | ||
| if (!loaded) { | ||
| setError('Failed to load Google Picker API'); | ||
| } | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // Start loading (only happens once globally) | ||
| isLoading = true; | ||
| loadPromise = loadGooglePickerApi(); | ||
|
|
||
| loadPromise | ||
| .then((loaded) => { | ||
| isLoaded = loaded; | ||
| isLoading = false; | ||
| setApiLoaded(loaded); | ||
| if (!loaded) { | ||
| setError('Failed to load Google Picker API'); | ||
| } | ||
| }) | ||
| .catch((err) => { | ||
| isLoading = false; |
There was a problem hiding this comment.
Using module-level mutable state variables in a React application can lead to issues with hot module reloading during development and potential state inconsistencies. Consider using a singleton pattern with proper initialization or moving this state management to a React context or a state management library.
| // Global state - shared across all component instances | |
| let isLoading = false; | |
| let isLoaded = false; | |
| let loadPromise: Promise<boolean> | null = null; | |
| /** | |
| * Custom hook to load and manage Google Picker API | |
| * Loads the API once globally and shares state across all components | |
| */ | |
| export const useGooglePicker = () => { | |
| const [apiLoaded, setApiLoaded] = useState(isLoaded); | |
| const [error, setError] = useState<string | null>(null); | |
| useEffect(() => { | |
| // If already loaded, just update state | |
| if (isLoaded) { | |
| setApiLoaded(true); | |
| return; | |
| } | |
| // If currently loading, wait for the existing promise | |
| if (isLoading && loadPromise) { | |
| loadPromise.then((loaded) => { | |
| setApiLoaded(loaded); | |
| if (!loaded) { | |
| setError('Failed to load Google Picker API'); | |
| } | |
| }); | |
| return; | |
| } | |
| // Start loading (only happens once globally) | |
| isLoading = true; | |
| loadPromise = loadGooglePickerApi(); | |
| loadPromise | |
| .then((loaded) => { | |
| isLoaded = loaded; | |
| isLoading = false; | |
| setApiLoaded(loaded); | |
| if (!loaded) { | |
| setError('Failed to load Google Picker API'); | |
| } | |
| }) | |
| .catch((err) => { | |
| isLoading = false; | |
| class GooglePickerLoader { | |
| private isLoading = false; | |
| private isLoaded = false; | |
| private loadPromise: Promise<boolean> | null = null; | |
| getStatus() { | |
| return { | |
| isLoading: this.isLoading, | |
| isLoaded: this.isLoaded, | |
| loadPromise: this.loadPromise, | |
| }; | |
| } | |
| load(): Promise<boolean> { | |
| // If already loaded, return resolved promise | |
| if (this.isLoaded) { | |
| return Promise.resolve(true); | |
| } | |
| // If currently loading, return existing promise | |
| if (this.isLoading && this.loadPromise) { | |
| return this.loadPromise; | |
| } | |
| // Start loading (only happens once per singleton instance) | |
| this.isLoading = true; | |
| this.loadPromise = loadGooglePickerApi() | |
| .then((loaded) => { | |
| this.isLoaded = loaded; | |
| this.isLoading = false; | |
| return loaded; | |
| }) | |
| .catch((err) => { | |
| this.isLoading = false; | |
| this.isLoaded = false; | |
| throw err; | |
| }); | |
| return this.loadPromise; | |
| } | |
| } | |
| // Singleton loader instance - shared across all component instances | |
| const googlePickerLoader = new GooglePickerLoader(); | |
| /** | |
| * Custom hook to load and manage Google Picker API | |
| * Loads the API once globally and shares state across all components | |
| */ | |
| export const useGooglePicker = () => { | |
| const { isLoaded } = googlePickerLoader.getStatus(); | |
| const [apiLoaded, setApiLoaded] = useState(isLoaded); | |
| const [error, setError] = useState<string | null>(null); | |
| useEffect(() => { | |
| const { isLoaded: loaded, isLoading, loadPromise } = googlePickerLoader.getStatus(); | |
| // If already loaded, just update state | |
| if (loaded) { | |
| setApiLoaded(true); | |
| return; | |
| } | |
| // If currently loading, wait for the existing promise | |
| if (isLoading && loadPromise) { | |
| loadPromise.then((result) => { | |
| setApiLoaded(result); | |
| if (!result) { | |
| setError('Failed to load Google Picker API'); | |
| } | |
| }).catch((err) => { | |
| setError(err.message); | |
| setApiLoaded(false); | |
| }); | |
| return; | |
| } | |
| // Start loading via singleton loader | |
| const promise = googlePickerLoader.load(); | |
| promise | |
| .then((result) => { | |
| setApiLoaded(result); | |
| if (!result) { | |
| setError('Failed to load Google Picker API'); | |
| } | |
| }) | |
| .catch((err) => { |
| clearInterval(interval); | ||
| resolve(); | ||
| } | ||
| }, 100); |
There was a problem hiding this comment.
The magic number '100' for the retry interval lacks explanation. Consider extracting this to a named constant like 'GAPI_LOAD_RETRY_INTERVAL_MS' to make the code more self-documenting and easier to adjust if needed.
| // Show the picker | ||
| builtPicker.setVisible(true); | ||
| } catch (error) { | ||
| console.error('Error showing picker:', error); |
There was a problem hiding this comment.
The error handling uses console.error for logging errors but these should also be removed or replaced with a proper logging mechanism in production code, similar to the console.log statements. Error logs can expose stack traces and implementation details.
|
@AkaashThawani thank you for this, a bit busy the next few days but wil try to get it reviewed by next end of next week |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
apps/bubble-studio/src/hooks/useGooglePicker.ts (1)
121-121: Debug logging should be removed for production.This
console.logstatement (and similar ones at lines 143, 146) should be removed or replaced with a conditional debug flag, consistent with the commented-out logs elsewhere in this file.apps/bubble-studio/src/components/InputFieldsRenderer.tsx (1)
294-303: Remove debug logging from production code.The
console.logat line 302 should be removed or replaced with a proper logging mechanism.🔎 Proposed fix
const handleGoogleFileSelect = ( fieldName: string, fileId: string, fileName: string ) => { onInputChange(fieldName, fileId); - // Optionally show the file name as feedback - console.log(`Selected ${fileName} with ID: ${fileId}`); };apps/bubble-studio/src/services/googlePickerService.ts (1)
204-204: Remove debug logging from production code.The
console.logstatement should be removed for production, consistent with past review feedback.🔎 Proposed fix
if ( data.action === googlePicker.Action.PICKED && data.docs && data.docs.length > 0 ) { const file = data.docs[0]; const fileId = file.id; const fileName = file.name; - console.log('File selected:', { fileId, fileName }); onSelect(fileId, fileName);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/bubble-studio/index.htmlapps/bubble-studio/src/components/GoogleFilePicker.tsxapps/bubble-studio/src/components/InputFieldsRenderer.tsxapps/bubble-studio/src/hooks/useGooglePicker.tsapps/bubble-studio/src/services/googlePickerService.tsapps/bubble-studio/src/utils/googlePickerCache.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/bubble-studio/src/utils/googlePickerCache.ts
- apps/bubble-studio/src/components/GoogleFilePicker.tsx
- apps/bubble-studio/index.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
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:
apps/bubble-studio/src/services/googlePickerService.tsapps/bubble-studio/src/components/InputFieldsRenderer.tsx
🧬 Code graph analysis (2)
apps/bubble-studio/src/services/googlePickerService.ts (2)
apps/bubble-studio/src/env.ts (2)
GOOGLE_OAUTH_CLIENT_ID(36-37)GOOGLE_API_KEY(34-35)apps/bubble-studio/src/utils/googlePickerCache.ts (1)
cachePickerToken(67-84)
apps/bubble-studio/src/components/InputFieldsRenderer.tsx (1)
apps/bubble-studio/src/components/GoogleFilePicker.tsx (1)
GoogleFilePicker(25-199)
🔇 Additional comments (5)
apps/bubble-studio/src/hooks/useGooglePicker.ts (1)
66-149: LGTM with minor observations.The
loadGooglePickerApifunction is well-structured with proper error handling, timeouts, and verification. The retry loop correctly rejects on timeout (addressing previous concerns), and the Picker library loading includes proper timeout and error callbacks.apps/bubble-studio/src/components/InputFieldsRenderer.tsx (2)
275-282: LGTM - Improved regex pattern.The regex now uses word boundaries (
\b) which correctly prevents false positives like "driver_id" while still matching legitimate Google file ID fields.
1104-1115: LGTM - Clean integration of GoogleFilePicker.The Google file picker is conditionally rendered only for relevant fields, properly receives the derived file type, and correctly wires the selection callback. The layout maintains the existing input alongside the picker button.
apps/bubble-studio/src/services/googlePickerService.ts (2)
85-89: LGTM - Proper guard added for Google Sign-In API.The runtime check for
window.google?.accounts?.oauth2before accessing the API properly handles the case where the Google Sign-In script hasn't fully loaded, addressing the previous concern.
195-212: LGTM - Proper callback handling with bounds checking.The callback correctly checks both
data.docsexistence anddata.docs.length > 0before accessing the first element, and properly handles both PICKED and CANCEL actions with appropriate loading state management.
|
Hey @zhubzy , any update on this ? |
|
hey @AkaashThawani just took look, can you make sure the google drive picker allows folder selection when type is folder? Here is code for you to reproduce: |
zhubzy
left a comment
zhubzy
left a comment
There was a problem hiding this comment.
- Folder should be selectable when google drive picker is in folder mode
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/bubble-studio/src/components/InputFieldsRenderer.tsx (1)
293-303: Critical:fieldis not defined in this scope; usefieldNameinstead.The variable
fieldis not accessible withinhandleArrayEntryChange. This will cause a runtime error. The function parameter isfieldName, notfield.🔎 Proposed fix
// Clear file name if user types manually - if (value && arrayEntryFileNames[field.name]?.[index]) { + if (value && arrayEntryFileNames[fieldName]?.[index]) { setArrayEntryFileNames((prev) => {
🤖 Fix all issues with AI agents
In @apps/bubble-studio/src/components/InputFieldsRenderer.tsx:
- Around line 315-323: getGoogleFileType currently only checks for
/spreadsheet/i so fields like "sheet_id" or "sheets" are classified as 'any';
update the function (getGoogleFileType) to also detect "sheet" and "sheets"
(e.g., add /sheet(s)?/i or /sheet/i before the spreadsheet check) so that field
names like "sheet_id" return 'spreadsheet' and will correctly filter in the
Google file picker.
- Around line 806-811: The increment button incorrectly renders the Minus icon;
locate the button whose aria-label is `Increase ${nestedPropName}` in
InputFieldsRenderer (the increment control) and replace the `Minus` component
with the `Plus` component so the icon matches the action; ensure the Plus import
is available or add it if missing.
- Around line 926-932: The increment button currently has the wrong aria-label
and icon: update the button within InputFieldsRenderer where the increment
control uses aria-label={`Decrease ${propName}`} and renders <Minus />; change
the aria-label to `Increase ${propName}` and replace the <Minus /> with <Plus />
(keep existing props like disabled={isExecuting}, className and event handlers
unchanged).
- Around line 1030-1036: The increment button in InputFieldsRenderer is using
the wrong icon and aria-label: it currently renders the Minus icon component and
aria-label="Decrease {field.name}"; update the increment button to use the Plus
icon component and aria-label={`Increase ${field.name}`} instead, locating the
button that currently references the Minus component and the aria-label string
and replacing them with Plus and the "Increase" label while preserving existing
props like disabled, className, and onClick handler.
In @apps/bubble-studio/src/services/googlePickerService.ts:
- Around line 118-124: The success branch currently assumes
response.access_token exists; add an explicit guard in the handler that checks
response.access_token before caching or resolving. If access_token is missing,
call reject or resolve with a safe fallback and log or handle the malformed
response (e.g., process an error path), otherwise call
cachePickerToken(response.access_token, accountEmail) and
resolve(response.access_token). Update the code around cachePickerToken and
resolve usage to only run when response.access_token is truthy.
- Around line 91-92: The code uses a non-null assertion on
GOOGLE_OAUTH_CLIENT_ID when calling
window.google.accounts.oauth2.initTokenClient (tokenClient) which can crash if
the env var is missing; before using GOOGLE_OAUTH_CLIENT_ID, add a guard like
the existing API check: validate that GOOGLE_OAUTH_CLIENT_ID is defined and
handle the missing case by logging an error and returning/throwing early (or
disabling picker initialization) so initTokenClient is never called with an
undefined client_id.
🧹 Nitpick comments (2)
apps/bubble-studio/src/services/googlePickerService.ts (1)
233-237: Remove debugconsole.logstatement.This debug log should be removed before production to avoid cluttering the browser console.
🔎 Proposed fix
- console.log('File selected:', { - fileId, - fileName, - mimeType: file.mimeType, - }); onSelect(fileId, fileName);apps/bubble-studio/src/components/InputFieldsRenderer.tsx (1)
325-334: Remove debugconsole.logstatement.Debug logs should be removed for production code.
🔎 Proposed fix
const handleGoogleFileSelect = ( fieldName: string, fileId: string, fileName: string ) => { onInputChange(fieldName, fileId); - // Optionally show the file name as feedback - console.log(`Selected ${fileName} with ID: ${fileId}`); };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
apps/bubble-studio/src/components/InputFieldsRenderer.tsxapps/bubble-studio/src/routeTree.gen.tsapps/bubble-studio/src/services/googlePickerService.ts
✅ Files skipped from review due to trivial changes (1)
- apps/bubble-studio/src/routeTree.gen.ts
🧰 Additional context used
🧠 Learnings (2)
📚 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:
apps/bubble-studio/src/services/googlePickerService.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:
apps/bubble-studio/src/services/googlePickerService.ts
🧬 Code graph analysis (2)
apps/bubble-studio/src/components/InputFieldsRenderer.tsx (2)
apps/bubble-studio/src/utils/fileUtils.ts (2)
ALLOWED_FILE_TYPES_DISPLAY(90-94)FILE_INPUT_ACCEPT(72-78)apps/bubble-studio/src/components/GoogleFilePicker.tsx (1)
GoogleFilePicker(25-199)
apps/bubble-studio/src/services/googlePickerService.ts (2)
apps/bubble-studio/src/env.ts (2)
GOOGLE_OAUTH_CLIENT_ID(36-37)GOOGLE_API_KEY(34-35)apps/bubble-studio/src/utils/googlePickerCache.ts (1)
cachePickerToken(67-84)
🔇 Additional comments (4)
apps/bubble-studio/src/services/googlePickerService.ts (3)
10-33: LGTM on global type declarations.The Window interface extension properly declares the Google API types needed for the picker integration.
47-72: LGTM!The function correctly maps file types to Google Picker ViewIds with proper null safety for the Google API.
187-196: Add validation forGOOGLE_API_KEYandprojectNumberbefore building picker.The non-null assertions could cause runtime errors if environment variables are missing or malformed. The
projectNumberextraction fromGOOGLE_OAUTH_CLIENT_IDassumes a specific format.🔎 Proposed fix
+ if (!GOOGLE_API_KEY) { + throw new Error('Google API key not configured'); + } + // Extract project number from OAuth Client ID for setAppId() // Client ID format: {project_number}.apps.googleusercontent.com const projectNumber = GOOGLE_OAUTH_CLIENT_ID?.split('.')[0]; + if (!projectNumber) { + throw new Error('Invalid Google OAuth Client ID format'); + } // Create the picker const pickerBuilder = new googlePicker.PickerBuilder(); const builtPicker = pickerBuilder .setOAuthToken(accessToken) - .setDeveloperKey(GOOGLE_API_KEY!) - .setAppId(projectNumber!) + .setDeveloperKey(GOOGLE_API_KEY) + .setAppId(projectNumber)Likely an incorrect or invalid review comment.
apps/bubble-studio/src/components/InputFieldsRenderer.tsx (1)
1136-1146: LGTM on Google File Picker integration.The integration correctly detects Google file fields, determines the appropriate file type, and renders the picker button adjacent to the input field.
| aria-label={`Increase ${nestedPropName}`} | ||
| className="p-1 bg-neutral-700 hover:bg-neutral-600 disabled:bg-neutral-800 disabled:text-neutral-600 disabled:cursor-not-allowed rounded-r border border-neutral-500 transition-colors" | ||
| > | ||
| <Plus className="w-2.5 h-2.5" /> | ||
| <Minus className="w-2.5 h-2.5" /> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
Wrong icon: Minus used for increment button.
The increment button at line 809 uses a Minus icon, but it should use Plus since clicking it increases the value.
🔎 Proposed fix
className="p-1 bg-neutral-700 hover:bg-neutral-600 disabled:bg-neutral-800 disabled:text-neutral-600 disabled:cursor-not-allowed rounded-r border border-neutral-500 transition-colors"
>
- <Minus className="w-2.5 h-2.5" />
+ <Plus className="w-2.5 h-2.5" />
</button>📝 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.
| aria-label={`Increase ${nestedPropName}`} | |
| className="p-1 bg-neutral-700 hover:bg-neutral-600 disabled:bg-neutral-800 disabled:text-neutral-600 disabled:cursor-not-allowed rounded-r border border-neutral-500 transition-colors" | |
| > | |
| <Plus className="w-2.5 h-2.5" /> | |
| <Minus className="w-2.5 h-2.5" /> | |
| </button> | |
| </div> | |
| aria-label={`Increase ${nestedPropName}`} | |
| className="p-1 bg-neutral-700 hover:bg-neutral-600 disabled:bg-neutral-800 disabled:text-neutral-600 disabled:cursor-not-allowed rounded-r border border-neutral-500 transition-colors" | |
| > | |
| <Plus className="w-2.5 h-2.5" /> | |
| </button> | |
| </div> |
🤖 Prompt for AI Agents
In @apps/bubble-studio/src/components/InputFieldsRenderer.tsx around lines 806 -
811, The increment button incorrectly renders the Minus icon; locate the button
whose aria-label is `Increase ${nestedPropName}` in InputFieldsRenderer (the
increment control) and replace the `Minus` component with the `Plus` component
so the icon matches the action; ensure the Plus import is available or add it if
missing.
| }} | ||
| disabled={isExecuting} | ||
| aria-label={`Increase ${propName}`} | ||
| aria-label={`Decrease ${propName}`} | ||
| className="p-1.5 bg-neutral-800 hover:bg-neutral-700 disabled:bg-neutral-900 disabled:text-neutral-600 disabled:cursor-not-allowed rounded-r border border-neutral-600 transition-colors" | ||
| > | ||
| <Plus className="w-3 h-3" /> | ||
| <Minus className="w-3 h-3" /> | ||
| </button> |
There was a problem hiding this comment.
Wrong icon and aria-label for increment button.
The button increments the value but has:
aria-label="Decrease {propName}"(should be "Increase")<Minus>icon (should be<Plus>)
🔎 Proposed fix
disabled={isExecuting}
- aria-label={`Decrease ${propName}`}
+ aria-label={`Increase ${propName}`}
className="p-1.5 bg-neutral-800 hover:bg-neutral-700 disabled:bg-neutral-900 disabled:text-neutral-600 disabled:cursor-not-allowed rounded-r border border-neutral-600 transition-colors"
>
- <Minus className="w-3 h-3" />
+ <Plus className="w-3 h-3" />
</button>📝 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.
| }} | |
| disabled={isExecuting} | |
| aria-label={`Increase ${propName}`} | |
| aria-label={`Decrease ${propName}`} | |
| className="p-1.5 bg-neutral-800 hover:bg-neutral-700 disabled:bg-neutral-900 disabled:text-neutral-600 disabled:cursor-not-allowed rounded-r border border-neutral-600 transition-colors" | |
| > | |
| <Plus className="w-3 h-3" /> | |
| <Minus className="w-3 h-3" /> | |
| </button> | |
| }} | |
| disabled={isExecuting} | |
| aria-label={`Increase ${propName}`} | |
| className="p-1.5 bg-neutral-800 hover:bg-neutral-700 disabled:bg-neutral-900 disabled:text-neutral-600 disabled:cursor-not-allowed rounded-r border border-neutral-600 transition-colors" | |
| > | |
| <Plus className="w-3 h-3" /> | |
| </button> |
🤖 Prompt for AI Agents
In @apps/bubble-studio/src/components/InputFieldsRenderer.tsx around lines 926 -
932, The increment button currently has the wrong aria-label and icon: update
the button within InputFieldsRenderer where the increment control uses
aria-label={`Decrease ${propName}`} and renders <Minus />; change the aria-label
to `Increase ${propName}` and replace the <Minus /> with <Plus /> (keep existing
props like disabled={isExecuting}, className and event handlers unchanged).
| }} | ||
| disabled={isExecuting} | ||
| aria-label={`Increase ${field.name}`} | ||
| aria-label={`Decrease ${field.name}`} | ||
| className="p-1.5 bg-neutral-700 hover:bg-neutral-600 disabled:bg-neutral-800 disabled:text-neutral-600 disabled:cursor-not-allowed rounded-r border border-neutral-600 transition-colors" | ||
| > | ||
| <Plus className="w-3 h-3" /> | ||
| <Minus className="w-3 h-3" /> | ||
| </button> |
There was a problem hiding this comment.
Wrong icon and aria-label for increment button.
Same issue as above: the top-level number field's increment button has incorrect aria-label ("Decrease") and uses Minus icon instead of Plus.
🔎 Proposed fix
disabled={isExecuting}
- aria-label={`Decrease ${field.name}`}
+ aria-label={`Increase ${field.name}`}
className="p-1.5 bg-neutral-700 hover:bg-neutral-600 disabled:bg-neutral-800 disabled:text-neutral-600 disabled:cursor-not-allowed rounded-r border border-neutral-600 transition-colors"
>
- <Minus className="w-3 h-3" />
+ <Plus className="w-3 h-3" />
</button>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @apps/bubble-studio/src/components/InputFieldsRenderer.tsx around lines 1030
- 1036, The increment button in InputFieldsRenderer is using the wrong icon and
aria-label: it currently renders the Minus icon component and
aria-label="Decrease {field.name}"; update the increment button to use the Plus
icon component and aria-label={`Increase ${field.name}`} instead, locating the
button that currently references the Minus component and the aria-label string
and replacing them with Plus and the "Increase" label while preserving existing
props like disabled, className, and onClick handler.
AkaashThawani
force-pushed
the
feat/google-picker-integration
branch
from
66a636e to
cfc2165
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/bubble-studio/src/services/googlePickerService.ts:
- Line 210: Remove the debug console.log in googlePickerService.ts: locate the
statement console.log('File selected:', { fileId, fileName }); (inside the
Google Picker handler in googlePickerService) and delete it or replace it with a
production-appropriate logging call (e.g., use the service's logger.debug or
remove entirely) so no debug output is emitted to the console in production.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/bubble-studio/.env.exampleapps/bubble-studio/index.htmlapps/bubble-studio/src/components/GoogleFilePicker.tsxapps/bubble-studio/src/components/InputFieldsRenderer.tsxapps/bubble-studio/src/env.tsapps/bubble-studio/src/hooks/useGooglePicker.tsapps/bubble-studio/src/routeTree.gen.tsapps/bubble-studio/src/services/googlePickerService.tsapps/bubble-studio/src/utils/googlePickerCache.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/bubble-studio/src/hooks/useGooglePicker.ts
- apps/bubble-studio/src/components/GoogleFilePicker.tsx
- apps/bubble-studio/src/utils/googlePickerCache.ts
- apps/bubble-studio/index.html
- apps/bubble-studio/src/routeTree.gen.ts
🧰 Additional context used
🧠 Learnings (3)
📚 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:
apps/bubble-studio/src/env.tsapps/bubble-studio/src/services/googlePickerService.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:
apps/bubble-studio/src/env.tsapps/bubble-studio/src/components/InputFieldsRenderer.tsxapps/bubble-studio/src/services/googlePickerService.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:
apps/bubble-studio/src/env.ts
🧬 Code graph analysis (2)
apps/bubble-studio/src/components/InputFieldsRenderer.tsx (1)
apps/bubble-studio/src/components/GoogleFilePicker.tsx (1)
GoogleFilePicker(25-199)
apps/bubble-studio/src/services/googlePickerService.ts (2)
apps/bubble-studio/src/env.ts (2)
GOOGLE_OAUTH_CLIENT_ID(36-37)GOOGLE_API_KEY(34-35)apps/bubble-studio/src/utils/googlePickerCache.ts (1)
cachePickerToken(67-84)
🪛 dotenv-linter (4.0.0)
apps/bubble-studio/.env.example
[warning] 22-22: [UnorderedKey] The VITE_ANALYTICS_ENABLED key should go before the VITE_POSTHOG_API_KEY key
(UnorderedKey)
🔇 Additional comments (8)
apps/bubble-studio/.env.example (1)
24-42: LGTM! Clear setup documentation.The Google Picker API configuration is well-documented with step-by-step setup instructions and appropriate warnings about public API key usage.
apps/bubble-studio/src/components/InputFieldsRenderer.tsx (4)
306-313: Improved field detection pattern addresses past concerns.The regex now uses word boundaries and an optional
315-323: LGTM! Field type detection now handles "sheet" correctly.The function now correctly identifies fields like "sheet_id" as spreadsheet type, addressing the previous feedback.
332-333: Remove debug console.log statement.This debug log was previously flagged and should be removed from production code to avoid console clutter.
Likely an incorrect or invalid review comment.
1135-1146: Folder selection support is properly implemented.The
getGoogleFileTypefunction correctly identifies folder types,getPickerViewIdmaps the 'folder' type topicker.ViewId.FOLDERS, and theGoogleFilePickercomponent properly passes the fileType through to the service. No issues found.apps/bubble-studio/src/services/googlePickerService.ts (2)
91-92: Add guard forGOOGLE_OAUTH_CLIENT_IDbefore use.The non-null assertion on
GOOGLE_OAUTH_CLIENT_IDat line 92 could cause a runtime error if the environment variable is not configured. This issue was previously identified and should be addressed with validation similar to the API check above.Likely an incorrect or invalid review comment.
185-194: Validate environment variables before use.The non-null assertions on
GOOGLE_API_KEY(line 193) andprojectNumber(line 194) could cause runtime errors if environment variables are missing or malformed. This issue was previously identified and should be addressed with proper validation at the start of the function.Likely an incorrect or invalid review comment.
apps/bubble-studio/src/env.ts (1)
33-38: Validation for required Google API configuration is still missing.The environment variables
GOOGLE_API_KEYandGOOGLE_OAUTH_CLIENT_IDare exported without validation. This issue was previously identified and remains unresolved. Without runtime checks, the application may fail when these values are missing or malformed.Likely an incorrect or invalid review comment.
AkaashThawani
force-pushed
the
feat/google-picker-integration
branch
from
fe7c621 to
4127e08
Compare
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (15)
packages/bubble-runtime/src/utils/bubble-helper.ts (1)
21-29: Unsafe type cast violates type safety.The cast
bubbleName as BubbleNameon line 26 is unsafe. Whenctor.bubbleNameis undefined (line 22),bubbleNamefalls back toclassName, which is derived from the constructor name and can be any string. Casting an arbitrary string toBubbleName(a union of specific string literals) without runtime validation creates a false type guarantee.This defeats the purpose of using the specific
BubbleNametype and could cause issues in downstream code that relies on this type being accurate.🔎 Proposed fix with runtime validation
+import { BubbleClassWithMetadata, BubbleFactory } from '@bubblelab/bubble-core'; +import { BubbleName, BubbleNodeType } from '@bubblelab/shared-schemas'; + +// Helper to validate BubbleName +function isValidBubbleName(name: string): name is BubbleName { + const validNames: readonly BubbleName[] = [ + 'hello-world', 'ai-agent', 'postgresql', 'slack', 'resend', 'http', + 'slack-formatter-agent', 'database-analyzer', 'slack-notifier', + 'get-bubble-details-tool', 'list-bubbles-tool', 'sql-query-tool', + 'chart-js-tool', 'web-search-tool', 'web-scrape-tool', 'web-crawl-tool', + 'web-extract-tool', 'research-agent-tool', 'reddit-scrape-tool', + 'slack-data-assistant', 'bubbleflow-code-generator', 'bubbleflow-generator', + 'pdf-form-operations', 'pdf-ocr-workflow', 'generate-document-workflow', + 'parse-document-workflow', 'bubbleflow-validation-tool', 'code-edit-tool', + 'storage', 'google-drive', 'gmail', 'google-sheets', 'google-calendar', + 'apify', 'instagram-tool', 'linkedin-tool', 'tiktok-tool', 'twitter-tool', + 'google-maps-tool', 'youtube-tool', 'github', 'eleven-labs', 'followupboss', + 'agi-inc', 'telegram', 'airtable', 'notion', 'firecrawl', 'insforge-db' + ]; + return validNames.includes(name as BubbleName); +} + export function buildClassNameLookup( factory: BubbleFactory ): Map< string, { bubbleName: BubbleName; className: string; nodeType: BubbleNodeType } > { const lookup = new Map< string, { bubbleName: BubbleName; className: string; nodeType: BubbleNodeType } >(); const all = factory.getAll() as BubbleClassWithMetadata[]; for (const ctor of all) { const className = (ctor as unknown as { name: string }).name; const bubbleName = (ctor as unknown as { bubbleName?: string }).bubbleName ?? className; + + // Validate before casting + if (!isValidBubbleName(bubbleName)) { + console.warn(`Invalid bubble name: ${bubbleName} (className: ${className})`); + continue; // or throw an error depending on desired behavior + } + const nodeType = (ctor as unknown as { type?: BubbleNodeType }).type ?? 'unknown'; lookup.set(className, { - bubbleName: bubbleName as BubbleName, + bubbleName, className, nodeType, }); } return lookup; }Alternative approach: If all classNames are guaranteed to be valid BubbleNames, consider adding verification to ensure this invariant holds across the codebase.
packages/bubble-core/src/bubbles/tool-bubble/research-agent-tool.ts (1)
98-128: Update the longDescription to reflect the new iteration limits.Line 108 states "Up to 100 iterations for thorough research and data gathering", but the maximum has been increased to 4000 with a default of 400. This user-facing documentation needs to be updated to accurately describe the feature capabilities.
🔎 Proposed fix
static readonly longDescription = ` A sophisticated research agent that strategically combines web search and selective web scraping to gather and structure information from the internet. Features: - Intelligent web search using Firecrawl's search API to find relevant sources - Strategic web scraping - for detailed content from specific high-value pages - Multi-page web crawling - for comprehensive coverage across entire websites - AI-powered analysis to synthesize information into the requested JSON structure - - Up to 100 iterations for thorough research and data gathering + - Up to 4000 iterations for thorough research and data gathering (default: 400) - Structured result formatting based on provided JSON schema - Comprehensive source tracking and research summaryapps/bubble-studio/src/utils/workflowToSteps.ts (1)
1-723: Clarify the inclusion of this file in the Google Picker PR.This file appears to be unrelated to the Google Picker integration feature described in the PR objectives. The changes add
variableIdtracking and custom tool filtering to workflow step processing, which don't appear to be prerequisites for the picker functionality.Bundling unrelated changes into a feature PR can complicate review, testing, and future debugging. If these changes are indeed independent, consider moving them to a separate PR focused on workflow enhancements.
packages/bubble-core/src/bubbles/tool-bubble/instagram-tool.ts (1)
1-633: Critical: Unrelated changes in Google Picker integration PR.This Instagram tool modification has no connection to the stated PR objective (Google Picker integration for Drive file selection). Including unrelated changes in a feature PR:
- Makes code review more difficult and error-prone
- Obscures the actual scope of changes
- Complicates rollback if issues arise
- Violates the principle of focused, atomic PRs
Please move these Instagram tool changes to a separate PR with appropriate context and justification.
packages/bubble-core/src/bubbles/tool-bubble/google-maps-tool.ts (1)
174-190: Document the dual purposes of thelimitparameter.The
limitvalue is intentionally used for two different purposes:
- Line 177:
maxCrawledPlacesPerSearchin the actor input controls how many places the Compass Google Places actor scrapes per individual search query.- Line 189:
limitpassed to ApifyBubble constructor sets themaxItemsquery parameter on the Apify API, which caps the dataset results returned and serves as a cost control mechanism.These serve distinct purposes (per-query scraping depth vs. global result cap), but using the same variable name for both creates ambiguity. Consider either:
- Renaming one variable to clarify the distinction (e.g.,
datasetLimitfor the ApifyBubble parameter), or- Adding inline comments at both locations to document the semantic difference.
apps/bubblelab-api/src/config/pricing.ts (1)
388-403: Critical: Inconsistent pricing keys for Claude Opus model.The pricing keys for Claude Opus input and output tokens don't match:
- Line 390:
'anthropic/claude-opus-4-5'(dash)- Line 398:
'anthropic/claude-opus-4.5'(dot)This inconsistency will cause pricing lookup failures for one of these token types. Both keys must use the same format.
🔎 Proposed fix to standardize on dash format
[getPricingKey( CredentialType.ANTHROPIC_CRED, - 'anthropic/claude-opus-4.5', + 'anthropic/claude-opus-4-5', 'output_tokens' )]: { unit: 'output_tokens', unitCost: (15.0 * 1.05) / 1_000_000, // $15.00 per 1M tokens * 1.05 markup = $15.75 per 1M tokens },apps/bubblelab-api/src/services/bubble-flow-parser.ts (1)
831-950: Fix className values to match actual bubble class names.The bubble names (
'slack','slack-formatter-agent','database-analyzer','ai-agent') are correctly registered. However, theclassNamevalues do not match the actual TypeScript class names per theParsedBubbleschema requirement:className corrections needed
"Slack Event Listener"→"SlackBubble""Message Parser"→"SlackFormatterAgentBubble""Database Schema Analysis"→"DatabaseAnalyzerWorkflowBubble""AI Agent"→"AIAgentBubble""Slack Message Sender"→"SlackBubble"This is placeholder code marked TODO, but the schema definition requires
classNameto be the actual TypeScript class name, not a display label. Update these values before deploying or marking this function as complete.apps/bubblelab-api/src/services/subscription-validation.ts (2)
160-160: Use consistent AppType in error fallbacks.The error handling uses inconsistent AppType references for unlimited fallback limits:
- Lines 160 and 213:
AppType.NODEX- Line 249:
AppType.BUBBLE_LABThis inconsistency could lead to different fallback behaviors depending on which function fails. Since this is the
bubblelab-apiservice andextractSubscriptionInfoFromPayloadhardcodesAppType.BUBBLE_LAB(line 102), all fallbacks should use the same AppType for consistency.🔎 Standardize to AppType.BUBBLE_LAB
// Return unlimited access as fallback const unlimitedLimit = - APP_FEATURES_TO_MONTHLY_LIMITS[AppType.NODEX].unlimited_usage; + APP_FEATURES_TO_MONTHLY_LIMITS[AppType.BUBBLE_LAB].unlimited_usage;Apply the same change to lines 160 and 213.
Also applies to: 213-213, 249-249
1-293: This file appears unrelated to the PR objectives; verify it was intentionally included.The PR describes implementing Google Drive file picker integration for the frontend, but this file contains backend subscription validation logic with no cross-references to picker-related code.
Additionally, this PR introduces three substantive issues:
Redundant subscription fetches:
getMonthlyLimitForPlancallsgetEffectiveUserSubscriptiononce directly (line 122), then calls it again indirectly throughgetCurrentWebhookUsage(line 179) andgetCurrentExecutionUsage(line 228). Consider refactoring to fetch the subscription once and pass it to the helper functions to avoid redundant operations.Inconsistent AppType in error fallbacks: Error handlers use
AppType.NODEXingetMonthlyLimitForPlan(line 160) andgetCurrentWebhookUsage(line 213), butAppType.BUBBLE_LABingetCurrentExecutionUsage(line 249). Ensure consistent fallback behavior across all functions.Hardcoded AppType:
extractSubscriptionInfoFromPayloadhardcodesAppType.BUBBLE_LAB(line 102), which may not be correct for multi-app contexts. This should be derived from the payload or passed as a parameter.packages/bubble-core/src/bubbles/tool-bubble/linkedin-tool.ts (3)
397-403: Avoid mutating params; enforce minimum via schema or validation.This logic mutates
this.params.limitin place whenscrapeJobsis requested with a limit below 100. Mutation is a code smell that can cause unexpected behavior and makes debugging harder.Additionally, if the user omits
limitentirely, the schema default (50) applies, but this mutation won't trigger because line 399 checks for the presence oflimit. The fallbackparams.limit || 100at lines 905 and 909 then overrides the schema default, creating further inconsistency.Refactor to validate the minimum at schema-level or reject invalid input explicitly without side effects.
🔎 Proposed fix: validate minimum without mutation
Option 1: Return an error instead of mutating:
if ( operation === 'scrapeJobs' && this.params?.limit && this.params.limit < 100 ) { - this.params!.limit = 100; + return this.createErrorResult( + 'scrapeJobs requires a minimum limit of 100. Please increase your limit or omit it to use the default.' + ); }Option 2: Use a separate schema for
scrapeJobsthat enforces.min(100)and document the difference.
900-915: Inconsistent fallback for scrapeJobs: 100 vs. schema default 50.Lines 905 and 909 both fall back to
params.limit || 100, which conflicts with the schema's.default(50). Additionally, the mutation logic at lines 398-403 attempts to enforce a minimum of 100 forscrapeJobs, but it only triggers when the user explicitly provides a value less than 100, not whenlimitis omitted and defaults to 50.This creates a confusing control flow:
- Schema injects default 50 if user omits
limit.- Mutation logic doesn't catch it (checks for explicit presence).
- Fallback here overrides schema default with 100.
Refactor to clearly enforce the minimum limit for
scrapeJobswithout relying on redundant fallbacks or side-effect mutations.
471-491: Remove inconsistent fallback values or align them with schema default (50).The schema declares
limitwith.default(50)(line 202-207), butscrapePostsusesparams.limit || 100(lines 481, 485) whilesearchPostscorrectly usesparams.limit || 50(lines 687, 690). ThescrapeJobsoperation also incorrectly uses 100 (lines 905, 909).Either:
- Change all fallbacks to 50 to match the schema default and ensure consistent behavior across operations
- Update the schema to define operation-specific defaults if 100 is intentional for certain operations
The defensive fallback pattern is reasonable given TypeScript's typing, but the inconsistency should be resolved.
packages/bubble-core/src/bubbles/service-bubble/apify/apify.ts (2)
52-60: Fix timeout default in documentation.The description comment on line 59 states "default: 120000", but the actual default value on line 57 is
300000. This documentation should match the code.🔎 Proposed fix
timeout: z .number() .min(1000) .max(500000) .optional() .default(300000) .describe( - 'Maximum time to wait for actor completion in milliseconds (default: 120000)' + 'Maximum time to wait for actor completion in milliseconds (default: 300000)' ),
332-337: Align timeout fallback with schema default.The fallback value
120000on line 336 doesn't match the schema default of300000(line 57). Since the schema provides a default, this fallback should ideally never be used, but for consistency and defensive coding, they should match.🔎 Proposed fix
// Wait for actor to finish const finalStatus = await this.waitForActorCompletion( apiToken, runId, - timeout || 120000 + timeout || 300000 );packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts (1)
115-408: These changes are unrelated to the PR objectives and should be moved to a separate PR.This PR is titled "Feat/google picker integration" and aims to add Google Drive file picker functionality. However, this file contains AI agent configuration changes that have no connection to the Google Picker feature:
- Removing schema-level default for
model(line 115) and moving it to constructor defaults (line 408)- Changing
temperature,maxTokens, andmaxIterationsdefaults- Forcing streaming behavior for Anthropic models
- Multiple schema and behavioral modifications
Impact:
- Violates single-responsibility principle for pull requests
- Makes code review difficult by mixing unrelated changes
- Obscures Git history and makes future debugging harder
- May introduce unintended side effects that are overlooked because reviewers focus on Google Picker changes
Recommended action:
- Revert all AI agent configuration changes from this PR
- Create a separate PR specifically for AI agent default tuning with:
- Clear justification for each parameter change
- Performance/quality metrics supporting the changes
- Test results showing impact on agent behavior
🤖 Fix all issues with AI agents
In @apps/bubble-studio/src/components/CronToggle.tsx:
- Around line 85-87: The code calls editor.getCode() without ensuring editor
exists; update the CronToggle logic that computes editorCode/codeToValidate to
guard against a null/undefined editor by checking the editor reference (or using
optional chaining) before invoking getCode(), and defaulting editorCode to an
empty string or currentFlow.code when editor is missing so codeToValidate never
triggers a runtime null reference; adjust any related type annotations for
useEditor return if needed.
In @apps/bubblelab-api/src/config/env.ts:
- Line 91: The HACKATHON_COUPON_CODES addition is unrelated to the Google Picker
work and uses a plural name but a single string type; either remove/revert this
line from the current PR and move it into a dedicated PR, or change it to a
properly parsed array and update its type and docs: parse
process.env.HACKATHON_COUPON_CODES (e.g., comma-separated) into an array of
trimmed strings, update any exported/env type references that use
HACKATHON_COUPON_CODES, and add a short comment or README note describing the
expected format; apply the same treatment to the other occurrences flagged
(lines 134–136).
In @apps/bubblelab-api/src/config/pricing.ts:
- Around line 333-347: The GPT-5.2 pricing entries using
getPricingKey(CredentialType.OPENAI_CRED, 'openai/gpt-5.2', ...) have incorrect
unitCost values; update the input_tokens unitCost to (1.75 * 1.05) / 1_000_000
and the output_tokens unitCost to (14.0 * 1.05) / 1_000_000 to match OpenAI's
official rates, and consider adding separate entries for the gpt-5.2-pro variant
with input 21.0 and output 168.0 (e.g., (21.0 * 1.05) / 1_000_000 and (168.0 *
1.05) / 1_000_000) using getPricingKey and CredentialType.OPENAI_CRED for clear
identification.
In @apps/bubblelab-api/src/middleware/rate-limit.ts:
- Around line 40-42: The current keyGenerator fallback uses the literal
'anonymous', causing all unauthenticated users to share one rate-limit bucket;
update keyGenerator to derive a more granular fallback such as c.get('userId')
|| c.req.header('x-forwarded-for') || c.ip || c.req.socket?.remoteAddress ||
`${c.req.method}:${c.req.url}` (or require auth) so unauthenticated requests are
bucketed by real client IP or connection address instead of a single 'anonymous'
string, and add a brief comment noting the fallback behavior; reference the
keyGenerator function and uses of c.get('userId'),
c.req.header('x-forwarded-for'), c.ip, and c.req.socket to locate the change.
In @apps/bubblelab-api/src/services/subscription-validation.ts:
- Line 7: Calls to getEffectiveUserSubscription (and indirectly
fetchAndResolveSubscription) are missing the appType argument, which can cause
incorrect multi-app resolution; update each call site (the three occurrences
noted plus in getCurrentWebhookUsage and getCurrentExecutionUsage) to pass the
appropriate appType: either hardcode AppType.BUBBLE_LAB if these utilities
should always target Bubble Lab, or add an appType parameter to the surrounding
functions (e.g., the functions that call getEffectiveUserSubscription) and
forward that parameter through to getEffectiveUserSubscription and any calls to
fetchAndResolveSubscription so the resolver has the correct app context.
In @packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts:
- Line 130: ModelConfigSchema currently sets .default(64000) while
AIAgentParamsSchema.model.maxTokens uses 65536; make these consistent by
updating ModelConfigSchema's default to 65536 (or alternatively change
AIAgentParamsSchema.model.maxTokens to 64000 if that is the intended value) so
both ModelConfigSchema and AIAgentParamsSchema.model.maxTokens share the same
single canonical default.
- Line 707: The Anthropic client is hardcoded to streaming:true and ignores the
computed enableStreaming (derived from streamingCallback); update the
ChatAnthropic instantiation in ai-agent.ts to use streaming: enableStreaming
(where enableStreaming is set as const enableStreaming =
!!this.streamingCallback) so Anthropic respects the same conditional streaming
behavior as other providers (or alternatively add a clear comment explaining a
required always-on stream), targeting the ChatAnthropic creation site and the
enableStreaming/streamingCallback symbols.
- Around line 285-286: The temperature is set to an unusually high default
(temperature: 1) in packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts;
change it to align with other agents (e.g., temperature: 0.7) or make it
configurable via the same config mechanism used by other agents, and add a short
comment/docstring and a unit/integration test asserting the default value so the
choice is explicit and covered; locate the temperature field in the agent
options object (the temperature property) and update the default and
accompanying docs/tests accordingly.
In @packages/bubble-core/src/bubbles/tool-bubble/linkedin-tool.ts:
- Around line 202-207: The schema for parameter limit now defaults to 50, but
scrapePosts and scrapeJobs still fall back to 100; update those functions
(scrapePosts and scrapeJobs) to either remove the manual fallback or change
their fallback to 50 so behavior matches the zod schema default, and ensure any
special minimum logic in scrapeJobs (the minimum-enforcement block around lines
~398-403) still applies if a different effective minimum is required.
In @packages/bubble-core/src/bubbles/tool-bubble/research-agent-tool.ts:
- Around line 43-50: The zod schema property maxIterations declares a default of
400 but its describe() text still says "default: 100"; update the description
string in the maxIterations schema (the
z.number().min(1).max(4000).default(400).describe(...) call) to reflect
"default: 400" so the human-readable description matches the actual default.
In @packages/bubble-runtime/src/utils/parameter-formatter.ts:
- Around line 430-442: The condenseToSingleLine function currently replaces all
newlines, which corrupts multi-line template literals; update
condenseToSingleLine to preserve newlines that occur inside backtick-delimited
template literals by parsing the input and skipping newline/compression
replacements while inside a backtick region (handle backtick escapes and ${...}
expressions correctly) or alternatively extend the detection logic (e.g.,
containsFunctionLiteral) to detect multi-line template literals and return the
original input unchanged when they are present; ensure you still apply the
existing whitespace normalization outside template literals and keep references
to condenseToSingleLine and any helper like containsFunctionLiteral to locate
and modify the code.
🟡 Minor comments (17)
packages/bubble-core/CREATE_BUBBLE_README.md-731-731 (1)
731-731: Replace emphasis with proper heading level for section organization.Line 731 uses bold emphasis instead of a heading, which breaks markdown hierarchy and linting. Use
###heading syntax to maintain consistent structure.🔎 Proposed fix
-**Pattern B: Multi-Operation Bubble (like GoogleSheets, Slack)** +### Pattern B: Multi-Operation Bubble (like GoogleSheets, Slack)packages/bubble-core/CREATE_BUBBLE_README.md-98-105 (1)
98-105: Add language specification to fenced code block.Fenced code blocks should declare a language for proper syntax highlighting and linter compliance.
🔎 Proposed fix
-``` +``` google-sheets/ ├── google-sheets.schema.ts # Schemas with z.transform() for type-safe transformations ├── google-sheets.utils.ts # normalizeValues(), sanitizeValues() ├── google-sheets.ts # Main bubble class ├── index.ts # Exports └─ google-sheets.test.ts # Edge case tests -``` +```Committable suggestion skipped: line range outside the PR's diff.
packages/bubble-core/CREATE_BUBBLE_README.md-122-131 (1)
122-131: Add language specification to fenced code block.Fenced code blocks should declare a language for proper syntax highlighting and linter compliance.
🔎 Proposed fix
-``` +``` src/bubbles/ ├── service-bubble/ │ ├── {service-name}/ # Folder for organized structure │ │ ├── {service-name}.schema.ts # All Zod schemas (with preprocessing if needed) │ │ ├── {service-name}.utils.ts # Utility functions (optional, only if needed) │ │ ├── {service-name}.ts # Main bubble class │ │ ├── index.ts # Exports │ │ └─ {service-name}.test.ts # Unit tests -``` +```Committable suggestion skipped: line range outside the PR's diff.
packages/bubble-core/CREATE_BUBBLE_README.md-19-26 (1)
19-26: Add language specification to fenced code block.Fenced code blocks should declare a language for proper syntax highlighting and linter compliance.
🔎 Proposed fix
-``` +``` {service-name}/ ├─ {service-name}.schema.ts # All Zod schemas (with preprocessing if needed) ├─ {service-name}.utils.ts # Utility functions (optional, if needed) ├─ {service-name}.ts # Main bubble class ├─ index.ts # Exports └─ {service-name}.test.ts # Unit tests -``` +```Committable suggestion skipped: line range outside the PR's diff.
packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts-306-307 (1)
306-307: Clarify the relationship to PR objectives and improve instruction specificity.These additions to
BUBBLE_SPECIFIC_INSTRUCTIONSdon't appear related to the Google Drive Picker integration described in the PR objectives. Consider whether these changes should be in a separate PR or if there's missing context.Additionally, there are clarity and grammar issues to address:
Line 306 (ai-agent model parameter):
- The instruction requires "always include the model parameter object" but doesn't clarify what happens if omitted (is there a default?) or whether this conflicts with existing behavior.
- Consider specifying whether this is a hard requirement or a best practice.
Line 307 (apify actor discovery):
- Grammar: "could be rented actor" → "could be a rented actor"
- The reasoning is unclear: why exactly can't rented actors be used? Is this a technical limitation or an availability concern?
- Consider rephrasing for clarity: "DO NOT use actors that have not been discovered first, as they may be rented actors unavailable for execution."
🔎 Suggested improvements
-3. When using the ai-agent bubble, always include the model parameter object with the model name and relevant configurations. Set temperature, maxTokens, and other parameters that users might want to adjust for their specific workflow needs. -4. When using custom apify bubble, always discover the actor and its schema first using the apify bubble, DO NOT use any actor that are not discovered as it could be rented actor and not available to run. +3. When using the ai-agent bubble, explicitly include the model parameter object with the model name and relevant configurations (e.g., temperature, maxTokens) to ensure users can adjust these settings for their specific workflow needs. +4. When using custom apify bubbles, always discover the actor and its schema first using the apify bubble. DO NOT use any actor that has not been discovered, as it could be a rented actor and unavailable to run.apps/bubble-studio/src/components/MonthlyUsageBar.tsx-203-224 (1)
203-224: Fix inconsistent handling of hackathon offers without expiration.The logic renders
nullwhen a hackathon offer is active but has no expiration date (line 221). This creates inconsistent UX compared to special offers, which explicitly show "No expiration" (line 214). Users won't see any banner for active hackathon offers without expiration dates.🔎 Proposed fix
- ) : offerExpiresAt ? ( + ) : ( <> - Promotional offer active · Unlimited access until{' '} - {formatOfferExpiration(offerExpiresAt)} + {offerExpiresAt ? ( + <> + Promotional offer active · Unlimited access until{' '} + {formatOfferExpiration(offerExpiresAt)} + </> + ) : ( + <>Promotional offer active · No expiration</> + )} </> - ) : null} + )}apps/bubble-studio/src/components/execution_logs/AllEventsView.tsx-416-444 (1)
416-444: Nested<button>elements are invalid HTML.The expand/collapse button (line 430) is nested inside the parent bubble selection button (line 416). This violates HTML semantics and can cause unexpected behavior with event propagation, accessibility issues, and inconsistent browser handling.
Consider restructuring so the expand toggle is outside the clickable button, or use a
<div role="button">wrapper with separate clickable regions.🔎 Suggested refactor
- <button - type="button" - onClick={() => setSelectedTab(tab.type)} - className={`relative w-full flex items-center gap-2 pl-4 pr-3 py-1.5 transition-all group ${ - isSelected - ? 'bg-[#1f6feb]/10' - : 'hover:bg-[#161b22]' - }`} - > - {/* Connecting dot */} - <div className="absolute left-[-3px] top-1/2 -translate-y-1/2 w-1.5 h-1.5 rounded-full bg-[#30363d]" /> - - {/* Expand/collapse button for bubbles with sub-bubbles */} - {hasSubBubbles && ( - <button - type="button" - onClick={(e) => { - e.stopPropagation(); - toggleBubbleExpansion(bubbleId); - }} - className="flex-shrink-0 w-4 h-4 flex items-center justify-center text-gray-500 hover:text-gray-300 transition-colors" - > - {isExpanded ? ( - <ChevronDown className="w-3 h-3" /> - ) : ( - <ChevronRight className="w-3 h-3" /> - )} - </button> - )} + <div + className={`relative w-full flex items-center gap-2 pl-4 pr-3 py-1.5 transition-all group ${ + isSelected + ? 'bg-[#1f6feb]/10' + : 'hover:bg-[#161b22]' + }`} + > + {/* Connecting dot */} + <div className="absolute left-[-3px] top-1/2 -translate-y-1/2 w-1.5 h-1.5 rounded-full bg-[#30363d]" /> + + {/* Expand/collapse button for bubbles with sub-bubbles */} + {hasSubBubbles && ( + <button + type="button" + onClick={(e) => { + e.stopPropagation(); + toggleBubbleExpansion(bubbleId); + }} + className="flex-shrink-0 w-4 h-4 flex items-center justify-center text-gray-500 hover:text-gray-300 transition-colors" + > + {isExpanded ? ( + <ChevronDown className="w-3 h-3" /> + ) : ( + <ChevronRight className="w-3 h-3" /> + )} + </button> + )} + + {/* Clickable area for selection */} + <button + type="button" + onClick={() => setSelectedTab(tab.type)} + className="flex-1 flex items-center gap-2 min-w-0" + >(Continue restructuring the remaining child elements under the new inner button)
Committable suggestion skipped: line range outside the PR's diff.
packages/bubble-core/src/bubbles/tool-bubble/youtube-tool.ts-285-285 (1)
285-285: Fix falsy value handling formaxResultsacross all usages.The
limitparameter is a valid and widely-used ApifyBubble parameter (confirmed in instagram-tool, google-maps-tool, tiktok-tool, and twitter-tool), and the value of 1 for transcript scraping (line 325) is correct.However, there is a critical bug in falsy value handling: the schema allows
maxResults: 0(line 70:min(0)), but the code usesthis.params.maxResults || 20at lines 278, 279, 285, 379, and 384. The||operator treats0as falsy and returns20instead, violating the schema contract and user intent.Use nullish coalescing (
??) instead of logical OR (||) to fix this:Recommended fix
Replace
||with??at:
- Line 278:
maxResults: this.params.maxResults ?? 20,- Line 279:
maxResultsShorts: this.params.includeShorts ? this.params.maxResults ?? 20 : 0,- Line 285:
limit: this.params.maxResults ?? 20,- Line 379:
maxResults: this.params.maxResults ?? 20,- Line 384:
limit: this.params.maxResults ?? 20,packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts-308-311 (1)
308-311: Clarify maxIterations default and description inconsistency.The
default(40)appears inconsistent with similar agent implementations in the codebase (e.g.,slack-formatter-agent.tsdefaults to10), and explicit workflow usages typically set it to6–10. More importantly, the description "5 iterations per turn of conversation" is unclear and doesn't obviously relate to a default of 40 iterations.Required:
- Clarify the description to explain the relationship between total iterations and conversation turns
- Justify why this default is
40when other similar agents use10or less- Confirm the
.min(4)constraint doesn't break existing workflows that may have expected lower valuespackages/bubble-runtime/src/injection/BubbleInjector.ts-178-183 (1)
178-183: Dynamic model path returns 4 provider credentials (excluding FIRECRAWL_API_KEY) when model cannot be extracted.When a static model string cannot be extracted,
extractModelCredentialTypereturns [GOOGLE_GEMINI_CRED, OPENAI_CRED, ANTHROPIC_CRED, OPENROUTER_CRED]. Note that FIRECRAWL_API_KEY—which is included in the ai-agent bubble's full credential options—is excluded from this dynamic path. Verify that downstream code expects this credential set and does not rely on FIRECRAWL_API_KEY for dynamic models.packages/bubble-runtime/src/extraction/BubbleParser.test.ts-424-668 (1)
424-668: Promise.all test suite unrelated to Google Picker PR.This comprehensive test suite for Promise.all parsing is well-structured but unrelated to the Google Picker integration stated in the PR objectives. Bundling unrelated test improvements in a feature PR makes review and git history less clear.
apps/bubble-studio/src/services/googlePickerService.ts-82-92 (1)
82-92: Guard Google env vars instead of using non‑null assertions.
GOOGLE_OAUTH_CLIENT_IDandGOOGLE_API_KEYare optional inenv.tsbut are asserted non‑null here. If they’re missing or malformed, you’ll only see a low‑signal failure from the Google APIs rather than a clear error, and TypeScript’s!buys nothing at runtime.Recommend validating these upfront and failing fast with a descriptive error (and resetting loading state) rather than relying on
!.Proposed defensive checks
export const requestPickerAccessToken = async ( onLoadingChange: (isLoading: boolean) => void ): Promise<string> => { onLoadingChange(true); return new Promise((resolve, reject) => { try { // Check if Google Sign-In API is loaded if (!window.google?.accounts?.oauth2) { onLoadingChange(false); reject(new Error('Google Sign-In API not loaded')); return; } + + if (!GOOGLE_OAUTH_CLIENT_ID) { + onLoadingChange(false); + reject(new Error('Google OAuth Client ID not configured')); + return; + } - const tokenClient = window.google.accounts.oauth2.initTokenClient({ - client_id: GOOGLE_OAUTH_CLIENT_ID!, + const tokenClient = window.google.accounts.oauth2.initTokenClient({ + client_id: GOOGLE_OAUTH_CLIENT_ID, scope: 'https://www.googleapis.com/auth/drive.readonly',// Show Google Picker with the provided access token const googlePicker = window.google.picker as { @@ - const projectNumber = GOOGLE_OAUTH_CLIENT_ID?.split('.')[0]; + if (!GOOGLE_API_KEY) { + throw new Error('Google API key not configured'); + } + const projectNumber = GOOGLE_OAUTH_CLIENT_ID?.split('.')[0]; + if (!projectNumber) { + throw new Error('Invalid Google OAuth Client ID format'); + } @@ - const builtPicker = pickerBuilder + const builtPicker = pickerBuilder .setOAuthToken(accessToken) - .setDeveloperKey(GOOGLE_API_KEY!) - .setAppId(projectNumber!) + .setDeveloperKey(GOOGLE_API_KEY) + .setAppId(projectNumber)Also applies to: 185-195
apps/bubble-studio/src/components/flow_visualizer/flowvisualizer.integration.test.ts-881-893 (1)
881-893: Add explicit assertion for AI agent bubble existence.If
aiAgentBubbleIDis undefined, the test passes silently without verifyingfunctionCallChildren. The test should fail explicitly if the AI agent bubble is missing.🔎 Proposed fix
const aiAgentBubbleID = Object.keys( validationResult.workflow?.bubbles || {} ).find( (key) => validationResult.workflow?.bubbles?.[key]?.bubbleName === 'ai-agent' ); - if (aiAgentBubbleID) { - expect( - validationResult.workflow?.bubbles[aiAgentBubbleID]?.dependencyGraph - ?.functionCallChildren - ).toBeDefined(); - } + expect(aiAgentBubbleID).toBeDefined(); + expect( + validationResult.workflow?.bubbles[aiAgentBubbleID!]?.dependencyGraph + ?.functionCallChildren + ).toBeDefined(); await validateAllNodesAreUsed(validationResult);apps/bubble-studio/src/routes/settings.tsx-9-17 (1)
9-17: Side effect during render: move navigation touseEffect.Calling
navigate()during render is a side effect that can cause React warnings and inconsistent behavior. The redirect should be handled in auseEffecthook.🔎 Proposed fix
+import { useEffect } from 'react'; import { createFileRoute, useNavigate } from '@tanstack/react-router'; import { SettingsPage } from '@/pages/SettingsPage'; import { useAuth } from '@/hooks/useAuth'; export const Route = createFileRoute('/settings')({ component: SettingsRoute, }); function SettingsRoute() { const navigate = useNavigate(); const { isSignedIn } = useAuth(); - // Redirect if not signed in - if (!isSignedIn) { - navigate({ to: '/home', search: { showSignIn: true }, replace: true }); - return null; - } + // Redirect if not signed in + useEffect(() => { + if (!isSignedIn) { + navigate({ to: '/home', search: { showSignIn: true }, replace: true }); + } + }, [isSignedIn, navigate]); + + if (!isSignedIn) { + return null; + } return (packages/bubble-runtime/src/utils/sanitize-script.ts-56-65 (1)
56-65: Comment mismatch: "Step 3" should be "Step 4".The comment on line 56 says "Step 3" but this is actually Step 4 (restoration). The actual Step 3 is the pattern replacement on lines 42-54.
- // Step 3: Restore the protected strings and comments + // Step 4: Restore the protected strings and commentsapps/bubblelab-api/src/middleware/rate-limit.ts-98-106 (1)
98-106: Comment-code mismatch: documentation says "5 attempts" but code allows 10.The comment states "5 attempts per hour" but
maxAttemptsis set to 10.🔎 Align comment with code
/** * Pre-configured rate limiter for coupon redemption - * 5 attempts per hour per user + * 10 attempts per hour per user */ export const couponRedemptionRateLimit = rateLimit({ windowMs: 60 * 60 * 1000, // 1 hour maxAttempts: 10, message: 'Too many redemption attempts.', });apps/bubblelab-api/src/middleware/rate-limit.ts-19-29 (1)
19-29: Potential timer leak from module-level setInterval.The cleanup interval is created at module load time but never cleared. In test environments, hot-reload scenarios, or when the module is reloaded, this can cause timer leaks.
💡 Consider exposing a cleanup function
+// Store the interval ID for cleanup +let cleanupIntervalId: NodeJS.Timeout | null = null; + // Clean up expired entries periodically (every 5 minutes) -setInterval( +cleanupIntervalId = setInterval( () => { const now = Date.now(); for (const [key, entry] of rateLimitStore.entries()) { if (now >= entry.resetAt) { rateLimitStore.delete(key); } } }, 5 * 60 * 1000 ); + +/** + * Stop the cleanup interval and clear the store + * Useful for testing and graceful shutdown + */ +export function cleanup() { + if (cleanupIntervalId) { + clearInterval(cleanupIntervalId); + cleanupIntervalId = null; + } + rateLimitStore.clear(); +}Committable suggestion skipped: line range outside the PR's diff.
| // Use editor code if available (for unsaved changes), otherwise use saved flow code | ||
| const editorCode = editor.getCode(); | ||
| const codeToValidate = editorCode || currentFlow.code; |
There was a problem hiding this comment.
Potential null reference error when accessing editor.getCode().
Line 86 calls editor.getCode() without first checking if editor is defined. The useEditor hook may return an undefined or null editor instance in certain cases.
🔎 Add null-safety check
// Use editor code if available (for unsaved changes), otherwise use saved flow code
- const editorCode = editor.getCode();
+ const editorCode = editor?.getCode();
const codeToValidate = editorCode || currentFlow.code;📝 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.
| // Use editor code if available (for unsaved changes), otherwise use saved flow code | |
| const editorCode = editor.getCode(); | |
| const codeToValidate = editorCode || currentFlow.code; | |
| // Use editor code if available (for unsaved changes), otherwise use saved flow code | |
| const editorCode = editor?.getCode(); | |
| const codeToValidate = editorCode || currentFlow.code; |
🤖 Prompt for AI Agents
In @apps/bubble-studio/src/components/CronToggle.tsx around lines 85 - 87, The
code calls editor.getCode() without ensuring editor exists; update the
CronToggle logic that computes editorCode/codeToValidate to guard against a
null/undefined editor by checking the editor reference (or using optional
chaining) before invoking getCode(), and defaulting editorCode to an empty
string or currentFlow.code when editor is missing so codeToValidate never
triggers a runtime null reference; adjust any related type annotations for
useEditor return if needed.
| // ============= Bubble Focus State ============= | ||
| const [bubbleToFocus, setBubbleToFocus] = useState<string | null>(null); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate the FlowIDEView.tsx file and check its contents around the mentioned lines
find . -name "FlowIDEView.tsx" -type fRepository: bubblelabai/BubbleLab
Length of output: 116
🏁 Script executed:
#!/bin/bash
# Check what props are being passed and used in FlowIDEView.tsx
# Get the file and examine lines 60-100 to see the context
fd "FlowIDEView.tsx" -x wc -l {}Repository: bubblelabai/BubbleLab
Length of output: 120
🏁 Script executed:
#!/bin/bash
# Search for FlowVisualizer interface definition to see what props it accepts
ast-grep --pattern $'interface $_ {
$$$
bubbleToFocus$_
$$$
}'
ast-grep --pattern $'interface $_ {
$$$
onFocusComplete$_
$$$
}'
ast-grep --pattern $'interface $_ {
$$$
onFocusBubble$_
$$$
}'Repository: bubblelabai/BubbleLab
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check useRunExecution hook for onFocusBubble usage
rg "onFocusBubble" --type ts --type tsx -B 3 -A 3Repository: bubblelabai/BubbleLab
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Find FlowVisualizer component to understand its interface
fd "FlowVisualizer" --type fRepository: bubblelabai/BubbleLab
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Read the specific lines from FlowIDEView.tsx
cat -n ./apps/bubble-studio/src/components/FlowIDEView.tsx | sed -n '60,85p'Repository: bubblelabai/BubbleLab
Length of output: 1132
🏁 Script executed:
#!/bin/bash
# Read lines around 672-676
cat -n ./apps/bubble-studio/src/components/FlowIDEView.tsx | sed -n '665,685p'Repository: bubblelabai/BubbleLab
Length of output: 1489
🏁 Script executed:
#!/bin/bash
# Read the FlowVisualizer interface
cat -n ./apps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsx | head -100Repository: bubblelabai/BubbleLab
Length of output: 4085
🏁 Script executed:
#!/bin/bash
# Search for useRunExecution hook
fd "useRunExecution" --type fRepository: bubblelabai/BubbleLab
Length of output: 112
🏁 Script executed:
#!/bin/bash
# Search for onFocusBubble references in tsx/ts files
rg "onFocusBubble" -A 2 -B 2Repository: bubblelabai/BubbleLab
Length of output: 7188
Consider moving bubble focus management to a separate PR.
The bubble focus state (bubbleToFocus, onFocusBubble, onFocusComplete) is not mentioned in the PR objectives, which focus exclusively on Google Picker integration. While the implementation is technically sound—the props are properly defined in FlowVisualizerProps, accepted by useRunExecution, and integrated through CronScheduleNode and InputSchemaNode—mixing unrelated UI enhancements with feature additions creates scope creep, makes the PR harder to review, and creates unclear git history. Split this focus management feature into a separate PR to keep changes focused and history clear.
| POSTHOG_HOST: process.env.POSTHOG_HOST || 'https://us.i.posthog.com', | ||
| POSTHOG_ENABLED: process.env.POSTHOG_ENABLED === 'true', | ||
| WISPR_API_KEY: process.env.WISPR_API_KEY, | ||
| HACKATHON_COUPON_CODES: process.env.HACKATHON_COUPON_CODES || '', |
There was a problem hiding this comment.
Unrelated change bundled in Google Picker PR.
The addition of HACKATHON_COUPON_CODES is not mentioned in the PR objectives, which focus exclusively on Google Picker integration. Additionally, the variable name is plural ("CODES") but the type is a single string rather than an array, which may cause confusion.
Consider:
- Moving unrelated changes to a separate PR for cleaner review and git history.
- If multiple coupon codes need to be supported, parse the string into an array (e.g., comma-separated) or document the expected format.
Also applies to: 134-136
🤖 Prompt for AI Agents
In @apps/bubblelab-api/src/config/env.ts at line 91, The HACKATHON_COUPON_CODES
addition is unrelated to the Google Picker work and uses a plural name but a
single string type; either remove/revert this line from the current PR and move
it into a dedicated PR, or change it to a properly parsed array and update its
type and docs: parse process.env.HACKATHON_COUPON_CODES (e.g., comma-separated)
into an array of trimmed strings, update any exported/env type references that
use HACKATHON_COUPON_CODES, and add a short comment or README note describing
the expected format; apply the same treatment to the other occurrences flagged
(lines 134–136).
| // GPT-5.2: Official pricing from OpenAI | ||
| // Note: Cached input pricing is $0.125 / 1M tokens (not tracked separately) | ||
| [getPricingKey(CredentialType.OPENAI_CRED, 'openai/gpt-5.2', 'input_tokens')]: | ||
| { | ||
| unit: 'input_tokens', | ||
| unitCost: (1.25 * 1.05) / 1_000_000, // $1.25 per 1M tokens * 1.05 markup = $1.3125 per 1M tokens | ||
| }, | ||
| [getPricingKey( | ||
| CredentialType.OPENAI_CRED, | ||
| 'openai/gpt-5.2', | ||
| 'output_tokens' | ||
| )]: { | ||
| unit: 'output_tokens', | ||
| unitCost: (10.0 * 1.05) / 1_000_000, // $10.00 per 1M tokens * 1.05 markup = $10.50 per 1M tokens | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OpenAI GPT-5.2 official pricing
💡 Result:
Official API prices for GPT-5.2 (per 1M tokens) — as listed by OpenAI:
- GPT-5.2 / gpt-5.2-chat-latest: Input $1.75; Cached input $0.175; Output $14.00. [1]
- gpt-5.2-pro: Input $21.00; Output $168.00. [1][2]
Sources: OpenAI announcement (Introducing GPT‑5.2) and OpenAI API pricing pages. [1][2]
Correct the GPT-5.2 pricing to match official OpenAI rates.
The implementation uses incorrect pricing. According to OpenAI's official pricing, GPT-5.2 should be:
- Input: $1.75 per 1M tokens (not $1.3125)
- Output: $14.00 per 1M tokens (not $10.50)
Update the unitCost calculations to (1.75 * 1.05) / 1_000_000 and (14.0 * 1.05) / 1_000_000 respectively. Note that OpenAI also offers a gpt-5.2-pro variant with different rates ($21.00 input, $168.00 output) that may need to be added separately.
🤖 Prompt for AI Agents
In @apps/bubblelab-api/src/config/pricing.ts around lines 333 - 347, The GPT-5.2
pricing entries using getPricingKey(CredentialType.OPENAI_CRED,
'openai/gpt-5.2', ...) have incorrect unitCost values; update the input_tokens
unitCost to (1.75 * 1.05) / 1_000_000 and the output_tokens unitCost to (14.0 *
1.05) / 1_000_000 to match OpenAI's official rates, and consider adding separate
entries for the gpt-5.2-pro variant with input 21.0 and output 168.0 (e.g.,
(21.0 * 1.05) / 1_000_000 and (168.0 * 1.05) / 1_000_000) using getPricingKey
and CredentialType.OPENAI_CRED for clear identification.
| keyGenerator = (c) => | ||
| c.get('userId') || c.req.header('x-forwarded-for') || 'anonymous', | ||
| message = 'Too many requests. Please try again later.', |
There was a problem hiding this comment.
Default 'anonymous' key creates a shared rate limit bucket for all unauthenticated users.
When c.get('userId') and x-forwarded-for are both unavailable, all unauthenticated requests share the same 'anonymous' key. This means a single malicious unauthenticated user can exhaust the rate limit for all other unauthenticated users.
Consider using a more granular fallback (e.g., IP-based even if forwarded headers are missing, or requiring authentication) or documenting this behavior clearly.
🤖 Prompt for AI Agents
In @apps/bubblelab-api/src/middleware/rate-limit.ts around lines 40 - 42, The
current keyGenerator fallback uses the literal 'anonymous', causing all
unauthenticated users to share one rate-limit bucket; update keyGenerator to
derive a more granular fallback such as c.get('userId') ||
c.req.header('x-forwarded-for') || c.ip || c.req.socket?.remoteAddress ||
`${c.req.method}:${c.req.url}` (or require auth) so unauthenticated requests are
bucketed by real client IP or connection address instead of a single 'anonymous'
string, and add a brief comment noting the fallback behavior; reference the
keyGenerator function and uses of c.get('userId'),
c.req.header('x-forwarded-for'), c.ip, and c.req.socket to locate the change.
| anthropicApiKey: apiKey, | ||
| maxTokens, | ||
| streaming: enableStreaming, | ||
| streaming: true, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -n 750 packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts | tail -n +600 | cat -nRepository: bubblelabai/BubbleLab
Length of output: 5888
🏁 Script executed:
rg -A 5 -B 5 "Anthropic" packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts | grep -A 5 -B 5 "streaming"Repository: bubblelabai/BubbleLab
Length of output: 281
🏁 Script executed:
rg "streaming" packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts -C 3Repository: bubblelabai/BubbleLab
Length of output: 6740
Anthropic forces streaming, ignoring user preference.
Line 708 sets streaming: true for Anthropic, overriding the enableStreaming flag computed from streamingCallback (line 623):
const enableStreaming = !!this.streamingCallback; // Line 623
// ...
case 'anthropic':
return new ChatAnthropic({
// ...
streaming: true, // Line 708 - ignores enableStreamingThis creates inconsistent behavior across providers:
- OpenAI (line ~641): respects
enableStreaming - Google (line ~663): hardcoded
falsewith documented reason ("3.0 pro preview breaks with streaming") - Anthropic (line 708): hardcoded
truewith no explanation - OpenRouter (line ~722): respects
enableStreaming
Users cannot disable streaming for Anthropic, even when no streamingCallback is provided. This conflicts with the conditional streaming logic used elsewhere (if (this.streamingCallback)).
Fix: Either use streaming: enableStreaming to match other providers, or document the technical requirement forcing streaming for Anthropic.
🤖 Prompt for AI Agents
In @packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts at line 707, The
Anthropic client is hardcoded to streaming:true and ignores the computed
enableStreaming (derived from streamingCallback); update the ChatAnthropic
instantiation in ai-agent.ts to use streaming: enableStreaming (where
enableStreaming is set as const enableStreaming = !!this.streamingCallback) so
Anthropic respects the same conditional streaming behavior as other providers
(or alternatively add a clear comment explaining a required always-on stream),
targeting the ChatAnthropic creation site and the
enableStreaming/streamingCallback symbols.
| // Basic validation: should contain at least one cell reference | ||
| // Pattern: [SheetName!]A1[:B10] or [SheetName!]A[:G] | ||
| // Handles both quoted ('Sheet Name'!) and unquoted (Sheet1!) sheet names | ||
| const rangePattern = /^(('[^']*'|[^!]*?)!)?[A-Z]+\d*(:[A-Z]+\d*)?$/; | ||
| if (!normalized.match(rangePattern)) { | ||
| throw new Error( | ||
| `Invalid range format: "${range}". Expected format: "SheetName!A1:B10" or "'Sheet Name'!A:G"` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Google Sheets A1 notation lowercase column letters support
💡 Result:
Short answer: Yes — A1 notation is case‑insensitive, so lowercase column letters work (Sheets/formulas and most APIs accept "a1" the same as "A1"). However, Google Sheets' UI and some APIs/clients typically return/display A1 ranges in uppercase (e.g. "A1:B2"). [1][2][3]
Examples / notes:
- You can use =SUM(a1:a10) or =SUM(A1:A10) interchangeably in Sheets. [2]
- Many client libraries parse column letters case‑insensitively (gspread's column_letter_to_index accepts "a" or "A"). [1]
- Functions that produce A1 strings (getA1Notation / Sheets API outputs) usually normalize to uppercase. [3]
Sources:
- gspread docs (case‑insensitive column letters). [1]
- Google Sheets A1 notation examples / documentation. [2]
- Sheets API / client docs showing A1Notation field and typical uppercase output. [3]
🌐 Web query:
Google Sheets A1 notation absolute references $ dollar sign
💡 Result:
In Google Sheets A1 notation the dollar sign ($) locks (makes absolute) the column, the row, or both in a cell reference so it doesn't change when you copy or fill the formula.
Rules and examples
- $A$1 — absolute column and row: always refers to cell A1. (copy anywhere → $A$1). [1]
- $A1 — absolute column only: column A is fixed, row changes when copied vertically (→ $A2, $A3). [2]
- A$1 — absolute row only: row 1 is fixed, column changes when copied horizontally (→ B$1, C$1). [2]
- A1 — relative (no $): both column and row change when copied (→ B2 if moved one column right and one row down). [1]
Quick tip: while editing a formula place the cursor on a reference and press F4 (or Cmd+T on Mac in some browsers) to cycle through A1 → $A$1 → A$1 → $A1. [1]
Sources: [1] Google Sheets guide/ABLEBITS overview; [2] CustomGuide / basic Sheets docs.
🌐 Web query:
Google Sheets valid range format A1 notation specification
💡 Result:
Summary — Google Sheets A1-notation valid range format
-
General form: [sheetName!]range where sheetName (if needed) is followed by !. Use single quotes around sheet names that contain spaces or special characters (e.g. 'My Sheet'!A1:B2). [1]
-
Range forms:
- Single cell: A1 or Sheet1!A1.
- Rectangular range: A1:B2 or Sheet1!A1:B2.
- Whole column(s): A:A or Sheet1!A:C.
- Whole row(s): 1:1 or Sheet1!1:3.
- Open-ended ranges: A5:A (column A from row 5 downward).
- Entire sheet: Sheet1 or 'My Sheet'.
- Named ranges are supported and may mask sheet names. [1]
-
Notes for API use:
- A1 notation is the string used in the Sheets API "range" / "a1Range" fields. The API also exposes GridRange (zero‑based, half‑open indexes) that corresponds to A1 ranges for programmatic access. [2][3]
Sources:
- Google Sheets API concepts: A1/R1C1 notation and examples. [1]
- Sheets API reference: a1Range, range semantics and GridRange (zero‑based, half‑open). [2][3]
Regex pattern is too restrictive and rejects valid Google Sheets A1 notation.
The pattern only accepts uppercase column letters and no absolute reference syntax. This blocks valid input:
- Lowercase letters (
a1:b10) are case-insensitive in Google Sheets—this syntax works in formulas and APIs - Absolute references (
$A$1,A$1,$A1) are standard Google Sheets notation for locked cell references
Update the regex to: /^(('[^']*'|[^!]*?)!)?(\$?[A-Za-z]+\$?\d*|\$?[A-Za-z]+:\$?[A-Za-z]+)(:\$?[A-Za-z]+\$?\d*|\$?[A-Za-z]+:\$?[A-Za-z]+)?$/ or similar to accept these valid forms, or document these as deliberate constraints if the service intentionally normalizes input.
| limit: z | ||
| .number() | ||
| .min(100) | ||
| .max(1000) | ||
| .default(100) | ||
| .default(50) | ||
| .optional() | ||
| .describe('Maximum number of items to fetch (default: 50)'), |
There was a problem hiding this comment.
Inconsistent default: schema declares 50, but operations use 100.
The schema now defaults limit to 50, but scrapePosts (line 485) and scrapeJobs (lines 905, 909) fall back to 100 when params.limit is undefined. This creates confusion: if a user omits limit, the schema will inject 50, yet the fallback logic suggests 100 was intended for those operations.
Consider either:
- Aligning all operation fallbacks to 50, or
- Documenting why certain operations need 100 and handling it explicitly in the schema or validation logic.
🔎 Proposed fix: align all fallbacks to schema default
limit: z
.number()
.max(1000)
.default(50)
.optional()
- .describe('Maximum number of items to fetch (default: 50)'),
+ .describe('Maximum number of items to fetch (default: 50). Note: scrapeJobs enforces a minimum of 100 at runtime.'),Then update the operation fallbacks to consistently use 50 (or remove them since schema provides the default), and let the scrapeJobs minimum enforcement at lines 398-403 handle the exception.
🤖 Prompt for AI Agents
In @packages/bubble-core/src/bubbles/tool-bubble/linkedin-tool.ts around lines
202 - 207, The schema for parameter limit now defaults to 50, but scrapePosts
and scrapeJobs still fall back to 100; update those functions (scrapePosts and
scrapeJobs) to either remove the manual fallback or change their fallback to 50
so behavior matches the zod schema default, and ensure any special minimum logic
in scrapeJobs (the minimum-enforcement block around lines ~398-403) still
applies if a different effective minimum is required.
| maxIterations: z | ||
| .number() | ||
| .min(1) | ||
| .max(100) | ||
| .default(100) | ||
| .max(4000) | ||
| .default(400) | ||
| .describe( | ||
| 'Maximum number of iterations for the research agent (default: 100)' | ||
| ), |
There was a problem hiding this comment.
Update the description to match the new default value.
The description at line 49 still references "default: 100", but the actual default has been changed to 400. This inconsistency will confuse users of the API.
🔎 Proposed fix
maxIterations: z
.number()
.min(1)
.max(4000)
.default(400)
.describe(
- 'Maximum number of iterations for the research agent (default: 100)'
+ 'Maximum number of iterations for the research agent (default: 400)'
),📝 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.
| maxIterations: z | |
| .number() | |
| .min(1) | |
| .max(100) | |
| .default(100) | |
| .max(4000) | |
| .default(400) | |
| .describe( | |
| 'Maximum number of iterations for the research agent (default: 100)' | |
| ), | |
| maxIterations: z | |
| .number() | |
| .min(1) | |
| .max(4000) | |
| .default(400) | |
| .describe( | |
| 'Maximum number of iterations for the research agent (default: 400)' | |
| ), |
🤖 Prompt for AI Agents
In @packages/bubble-core/src/bubbles/tool-bubble/research-agent-tool.ts around
lines 43 - 50, The zod schema property maxIterations declares a default of 400
but its describe() text still says "default: 100"; update the description string
in the maxIterations schema (the
z.number().min(1).max(4000).default(400).describe(...) call) to reflect
"default: 400" so the human-readable description matches the actual default.
| /** | ||
| * Condense a parameters string to a single line. | ||
| * Used when parameters don't contain function literals. | ||
| */ | ||
| export function condenseToSingleLine(input: string): string { | ||
| return input | ||
| .replace(/\s*\n\s*/g, ' ') | ||
| .replace(/\s{2,}/g, ' ') | ||
| .replace(/\{\s+/g, '{ ') | ||
| .replace(/\s+\}/g, ' }') | ||
| .replace(/\s*,\s*/g, ', ') | ||
| .trim(); | ||
| } |
There was a problem hiding this comment.
Critical: Template literals with newlines are broken by condensation.
The function replaces all newlines with spaces, including newlines inside template literals. This changes the runtime value of template strings.
Example:
{ message: `Line1
Line2` }Becomes:
{ message: `Line1 Line2` }The newline is lost, breaking the intended string value. Template literals without embedded functions won't trigger function-literal detection, so they will be incorrectly condensed.
🔎 Proposed fix: preserve newlines inside string literals
export function condenseToSingleLine(input: string): string {
- return input
- .replace(/\s*\n\s*/g, ' ')
- .replace(/\s{2,}/g, ' ')
- .replace(/\{\s+/g, '{ ')
- .replace(/\s+\}/g, ' }')
- .replace(/\s*,\s*/g, ', ')
- .trim();
+ // Preserve newlines inside template literals, strings
+ let result = '';
+ let inSingle = false;
+ let inDouble = false;
+ let inTemplate = false;
+ let escapeNext = false;
+
+ for (let i = 0; i < input.length; i++) {
+ const ch = input[i];
+
+ if (inSingle || inDouble || inTemplate) {
+ if (escapeNext) {
+ result += ch;
+ escapeNext = false;
+ continue;
+ }
+ if (ch === '\\') {
+ result += ch;
+ escapeNext = true;
+ continue;
+ }
+ result += ch;
+ if (ch === "'" && inSingle) inSingle = false;
+ if (ch === '"' && inDouble) inDouble = false;
+ if (ch === '`' && inTemplate) inTemplate = false;
+ continue;
+ }
+
+ // Outside strings
+ if (ch === "'") { inSingle = true; result += ch; continue; }
+ if (ch === '"') { inDouble = true; result += ch; continue; }
+ if (ch === '`') { inTemplate = true; result += ch; continue; }
+
+ result += ch;
+ }
+
+ // Now condense whitespace outside strings
+ return result
+ .replace(/\s*\n\s*/g, ' ')
+ .replace(/\s{2,}/g, ' ')
+ .replace(/\{\s+/g, '{ ')
+ .replace(/\s+\}/g, ' }')
+ .replace(/\s*,\s*/g, ', ')
+ .trim();
}Alternatively, extend containsFunctionLiteral to also detect multi-line template literals and preserve them:
const FUNCTION_LITERAL_PATTERNS = [
'func:', // Object property with function value
'=>', // Arrow function
'function(', // Function expression
'function (', // Function expression with space
'async(', // Async arrow function
'async (', // Async function with space
] as const;
+/**
+ * Check if a string contains multi-line template literals.
+ */
+function containsMultiLineTemplate(value: string): boolean {
+ const templateRegex = /`[^`]*\n[^`]*`/;
+ return templateRegex.test(value);
+}
export function containsFunctionLiteral(value: string): boolean {
- return FUNCTION_LITERAL_PATTERNS.some((pattern) => value.includes(pattern));
+ return FUNCTION_LITERAL_PATTERNS.some((pattern) => value.includes(pattern)) ||
+ containsMultiLineTemplate(value);
}🤖 Prompt for AI Agents
In @packages/bubble-runtime/src/utils/parameter-formatter.ts around lines 430 -
442, The condenseToSingleLine function currently replaces all newlines, which
corrupts multi-line template literals; update condenseToSingleLine to preserve
newlines that occur inside backtick-delimited template literals by parsing the
input and skipping newline/compression replacements while inside a backtick
region (handle backtick escapes and ${...} expressions correctly) or
alternatively extend the detection logic (e.g., containsFunctionLiteral) to
detect multi-line template literals and return the original input unchanged when
they are present; ensure you still apply the existing whitespace normalization
outside template literals and keep references to condenseToSingleLine and any
helper like containsFunctionLiteral to locate and modify the code.
Adds native Google Picker UI for Drive file selection with token caching, CreateCredentialModal integration, and smart field detection. Closes bubblelabai#204
- Add GoogleFilePicker component with CreateCredentialModal integration - Implement token caching (1 hour) to minimize OAuth popups - Add smart field detection for Google file ID fields - Support all Drive file types (Sheets, Docs, Folders, etc.) - Add comprehensive setup documentation in .env.example Closes bubblelabai#204
AkaashThawani
force-pushed
the
feat/google-picker-integration
branch
from
4127e08 to
a32ef8b
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In @apps/bubble-studio/src/components/GoogleFilePicker.tsx:
- Line 163: The Tailwind class `h-9/10` in the JSX for the GoogleFilePicker
component is invalid and has no effect; update the div that currently uses
`className={`w-full h-9/10 ${className}`}` to use a valid height utility (e.g.,
`h-full`) or a custom CSS class (e.g., `custom-height`) and ensure any custom
class is defined in the component stylesheet or global CSS. Locate the div in
the GoogleFilePicker component and replace `h-9/10` with the chosen valid
utility or add the custom class, then verify layout and adjust props-safe
merging with the existing `${className}`.
- Line 132: Remove the debug console output in the GoogleFilePicker component:
delete the console.log('✅ Google credential created!', created); statement (or
replace it with a proper structured logger call if the app uses a logging
utility) inside the GoogleFilePicker component where the variable created is
available so production consoles are not polluted.
In @apps/bubble-studio/src/components/InputFieldsRenderer.tsx:
- Around line 326-334: The handleGoogleFileSelect function currently logs
selected file info to the console; remove the console.log statement from
handleGoogleFileSelect (the function that calls onInputChange with fieldName and
fileId) so production code no longer emits debug output, and if user feedback is
required replace it with a UI update or use the app's logging utility instead of
console.log.
In @apps/bubble-studio/src/hooks/useGooglePicker.ts:
- Line 121: Remove the stray console.log in the useGooglePicker hook: delete the
line console.log('✅ Loaded Google Picker library') inside the useGooglePicker
function and, if you need to retain instrumentation, replace it with the
project's logger or a debug-level call (e.g., use the existing logging utility
or a debug flag) so no console.log remains in production code.
- Line 143: Remove the debug console.log in useGooglePicker.ts (the line logging
"✅ Google Picker API fully loaded and ready") from production code; either
delete that statement or replace it with a conditional debug/logging call (e.g.,
use an existing logger.debug or guard it with a NODE_ENV/dev-only check) so no
plain console.log executes in production.
In @apps/bubble-studio/src/services/googlePickerService.ts:
- Around line 179-188: The code uses GOOGLE_OAUTH_CLIENT_ID and GOOGLE_API_KEY
without validating them: parse GOOGLE_OAUTH_CLIENT_ID into projectNumber safely
(e.g., check it exists and matches the expected pattern like
/^\d+\.apps\.googleusercontent\.com$/ or split and ensure the first segment is
numeric) and ensure GOOGLE_API_KEY is present before calling
googlePicker.PickerBuilder().setOAuthToken(...).setDeveloperKey(...).setAppId(...).
If either env var is missing or malformed, throw or return a controlled error
(or log and abort) instead of using non-null assertions; update the usage around
projectNumber, GOOGLE_API_KEY, and the googlePicker.PickerBuilder/setAppId call
to only proceed when validated.
- Line 204: Remove the debug console.log call that prints selected file details;
specifically delete the console.log('File selected:', { fileId, fileName })
statement in googlePickerService.ts (inside the file selection handler in the
Google Picker flow), or replace it with an existing structured logger at a debug
level if you need retained telemetry (e.g., use project logger.method rather
than console.log).
- Around line 118-124: The success branch assumes response.access_token exists;
explicitly check that response.access_token is defined before calling
cachePickerToken and resolve. If it's missing, call reject (or throw) with a
clear error message (e.g., "No access_token in Google response") instead of
using non-null assertions; otherwise store the token in accountEmail via the
existing accountEmail logic and then call
cachePickerToken(response.access_token, accountEmail) followed by
resolve(response.access_token). Ensure you update the code paths around
cachePickerToken and resolve to use the validated token variable rather than
response.access_token!.
- Around line 91-92: The code uses a non-null assertion on
GOOGLE_OAUTH_CLIENT_ID when calling
window.google.accounts.oauth2.initTokenClient which will throw at runtime if the
env var is undefined; add a guard before initializing tokenClient (similar to
the existing API check) to verify GOOGLE_OAUTH_CLIENT_ID is defined and handle
the missing value by logging/erroring or early returning. Specifically, check
the variable before calling initTokenClient, and only call
window.google.accounts.oauth2.initTokenClient({ client_id:
GOOGLE_OAUTH_CLIENT_ID, ... }) when it is present; otherwise surface a clear
error via the same processLogger or throw to avoid passing undefined into
initTokenClient.
🧹 Nitpick comments (1)
apps/bubble-studio/src/hooks/useGooglePicker.ts (1)
92-102: Extract magic numbers to named constants for better maintainability.The retry logic uses magic numbers (20 attempts, 100ms interval) that would benefit from being extracted as named constants.
🔎 Proposed refactor
At the top of the file:
+const MAX_GAPI_LOAD_ATTEMPTS = 20; +const GAPI_LOAD_RETRY_INTERVAL_MS = 100; + /** * Custom hook to load and manage Google Picker APIThen update the retry logic:
let attempts = 0; const interval = setInterval(() => { attempts++; if (window.gapi) { clearInterval(interval); resolve(); - } else if (attempts >= 20) { + } else if (attempts >= MAX_GAPI_LOAD_ATTEMPTS) { clearInterval(interval); reject(new Error('Timeout waiting for Google API to initialize')); } - }, 100); + }, GAPI_LOAD_RETRY_INTERVAL_MS);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/bubble-studio/.env.exampleapps/bubble-studio/index.htmlapps/bubble-studio/src/components/GoogleFilePicker.tsxapps/bubble-studio/src/components/InputFieldsRenderer.tsxapps/bubble-studio/src/env.tsapps/bubble-studio/src/hooks/useGooglePicker.tsapps/bubble-studio/src/services/googlePickerService.tsapps/bubble-studio/src/utils/googlePickerCache.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/bubble-studio/src/env.ts
- apps/bubble-studio/src/utils/googlePickerCache.ts
🧰 Additional context used
🧠 Learnings (3)
📚 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:
apps/bubble-studio/src/components/InputFieldsRenderer.tsxapps/bubble-studio/src/services/googlePickerService.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:
apps/bubble-studio/src/services/googlePickerService.tsapps/bubble-studio/index.html
📚 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 apps/bubble-studio/public/bubbles.json : Refer to apps/bubble-studio/public/bubbles.json for a condensed definition of all bubbles (building/bundling is required)
Applied to files:
apps/bubble-studio/index.htmlapps/bubble-studio/.env.example
🧬 Code graph analysis (2)
apps/bubble-studio/src/components/InputFieldsRenderer.tsx (2)
apps/bubble-studio/src/utils/fileUtils.ts (2)
ALLOWED_FILE_TYPES_DISPLAY(90-94)FILE_INPUT_ACCEPT(72-78)apps/bubble-studio/src/components/GoogleFilePicker.tsx (1)
GoogleFilePicker(25-199)
apps/bubble-studio/src/services/googlePickerService.ts (2)
apps/bubble-studio/src/env.ts (2)
GOOGLE_OAUTH_CLIENT_ID(36-37)GOOGLE_API_KEY(34-35)apps/bubble-studio/src/utils/googlePickerCache.ts (1)
cachePickerToken(67-84)
🪛 dotenv-linter (4.0.0)
apps/bubble-studio/.env.example
[warning] 22-22: [UnorderedKey] The VITE_ANALYTICS_ENABLED key should go before the VITE_POSTHOG_API_KEY key
(UnorderedKey)
🔇 Additional comments (7)
apps/bubble-studio/.env.example (1)
24-42: LGTM! Clear documentation for Google Picker configuration.The new environment variables are well-documented with setup instructions. The comments correctly note that
VITE_GOOGLE_API_KEYis a public API key safe for frontend exposure, and the instruction to copyVITE_GOOGLE_OAUTH_CLIENT_IDfrom the backend aligns with OAuth best practices.apps/bubble-studio/src/components/InputFieldsRenderer.tsx (2)
306-323: LGTM! Clean Google file field detection logic.The helper functions provide clear separation of concerns:
isGoogleFileFielduses word boundaries to avoid false positives like "driver_id"getGoogleFileTypemaps field names to appropriate Google Drive file typesThe implementation properly supports all four file types mentioned in the PR objectives: spreadsheet, document, folder, and general files.
1040-1146: LGTM! Clean integration of Google File Picker.The flex layout cleanly accommodates the Google Picker button alongside the existing file upload control without disrupting the existing UI. The conditional rendering based on
isGoogleFileFieldensures the picker only appears for relevant fields, and the disabled state properly propagates to prevent interactions during execution.apps/bubble-studio/src/components/GoogleFilePicker.tsx (2)
56-129: LGTM! Well-structured picker initialization flow.The component properly handles the complete lifecycle:
- Validates API availability and configuration
- Leverages cached tokens to minimize OAuth prompts
- Falls back to credential creation when needed
- Provides clear error messages for configuration issues
The token caching strategy aligns with the PR objective of minimizing OAuth popups.
92-96: Folder selection support is properly configured.The
googlePickerService.showPickerfunction correctly enables folder selection whenfileTypeis'folder'. ThegetPickerViewIdfunction maps'folder'topicker.ViewId.FOLDERS, and the picker is properly configured with this view ID.apps/bubble-studio/src/services/googlePickerService.ts (2)
35-45: LGTM! Folder support included.The type definitions correctly include
'folder'as a file type, which addresses the requirement from Issue #204 for folder selection support.
50-72: LGTM! Folder selection properly mapped.The function correctly maps
'folder'file type topicker.ViewId.FOLDERS(line 68), which ensures the Google Picker will filter for folders when that field type is selected. This addresses the reviewer concern raised by zhubzy.
| }; | ||
|
|
||
| const handleCredentialCreated = async (created: CredentialResponse) => { | ||
| console.log('✅ Google credential created!', created); |
There was a problem hiding this comment.
Remove console.log from production code.
This debug statement should be removed to keep the production console clean.
🔎 Proposed fix
const handleCredentialCreated = async (created: CredentialResponse) => {
- console.log('✅ Google credential created!', created);
-
// Update local credentials list📝 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.
| console.log('✅ Google credential created!', created); | |
| const handleCredentialCreated = async (created: CredentialResponse) => { | |
| // Update local credentials list |
🤖 Prompt for AI Agents
In @apps/bubble-studio/src/components/GoogleFilePicker.tsx at line 132, Remove
the debug console output in the GoogleFilePicker component: delete the
console.log('✅ Google credential created!', created); statement (or replace it
with a proper structured logger call if the app uses a logging utility) inside
the GoogleFilePicker component where the variable created is available so
production consoles are not polluted.
|
|
||
| return ( | ||
| <> | ||
| <div className={`w-full h-9/10 ${className}`}> |
There was a problem hiding this comment.
Invalid Tailwind CSS class h-9/10.
Tailwind doesn't provide fractional height utilities like h-9/10. This won't apply any styling. Consider using h-full or a custom height value.
🔎 Proposed fix
- <div className={`w-full h-9/10 ${className}`}>
+ <div className={`w-full h-full ${className}`}>
<button📝 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.
| <div className={`w-full h-9/10 ${className}`}> | |
| <div className={`w-full h-full ${className}`}> | |
| <button |
🤖 Prompt for AI Agents
In @apps/bubble-studio/src/components/GoogleFilePicker.tsx at line 163, The
Tailwind class `h-9/10` in the JSX for the GoogleFilePicker component is invalid
and has no effect; update the div that currently uses `className={`w-full h-9/10
${className}`}` to use a valid height utility (e.g., `h-full`) or a custom CSS
class (e.g., `custom-height`) and ensure any custom class is defined in the
component stylesheet or global CSS. Locate the div in the GoogleFilePicker
component and replace `h-9/10` with the chosen valid utility or add the custom
class, then verify layout and adjust props-safe merging with the existing
`${className}`.
| const handleGoogleFileSelect = ( | ||
| fieldName: string, | ||
| fileId: string, | ||
| fileName: string | ||
| ) => { | ||
| onInputChange(fieldName, fileId); | ||
| // Optionally show the file name as feedback | ||
| console.log(`Selected ${fileName} with ID: ${fileId}`); | ||
| }; |
There was a problem hiding this comment.
Remove console.log from production code.
This debug statement should be removed to keep the production console clean.
🔎 Proposed fix
const handleGoogleFileSelect = (
fieldName: string,
fileId: string,
fileName: string
) => {
onInputChange(fieldName, fileId);
- // Optionally show the file name as feedback
- console.log(`Selected ${fileName} with ID: ${fileId}`);
};🤖 Prompt for AI Agents
In @apps/bubble-studio/src/components/InputFieldsRenderer.tsx around lines 326 -
334, The handleGoogleFileSelect function currently logs selected file info to
the console; remove the console.log statement from handleGoogleFileSelect (the
function that calls onInputChange with fieldName and fileId) so production code
no longer emits debug output, and if user feedback is required replace it with a
UI update or use the app's logging utility instead of console.log.
| }; | ||
| gapi.load('picker', { | ||
| callback: () => { | ||
| console.log('✅ Loaded Google Picker library'); |
There was a problem hiding this comment.
Remove console.log from production code.
This debug log statement should be removed to avoid cluttering the browser console in production.
🔎 Proposed fix
gapi.load('picker', {
callback: () => {
- console.log('✅ Loaded Google Picker library');
resolve();
},📝 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.
| console.log('✅ Loaded Google Picker library'); | |
| gapi.load('picker', { | |
| callback: () => { | |
| resolve(); | |
| }, |
🤖 Prompt for AI Agents
In @apps/bubble-studio/src/hooks/useGooglePicker.ts at line 121, Remove the
stray console.log in the useGooglePicker hook: delete the line console.log('✅
Loaded Google Picker library') inside the useGooglePicker function and, if you
need to retain instrumentation, replace it with the project's logger or a
debug-level call (e.g., use the existing logging utility or a debug flag) so no
console.log remains in production code.
| ); | ||
| } | ||
|
|
||
| console.log('✅ Google Picker API fully loaded and ready'); |
There was a problem hiding this comment.
Remove console.log from production code.
This debug log statement should be removed to avoid cluttering the browser console in production.
🔎 Proposed fix
}
- console.log('✅ Google Picker API fully loaded and ready');
return true;
} catch (error) {📝 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.
| console.log('✅ Google Picker API fully loaded and ready'); | |
| } | |
| return true; | |
| } catch (error) { |
🤖 Prompt for AI Agents
In @apps/bubble-studio/src/hooks/useGooglePicker.ts at line 143, Remove the
debug console.log in useGooglePicker.ts (the line logging "✅ Google Picker API
fully loaded and ready") from production code; either delete that statement or
replace it with a conditional debug/logging call (e.g., use an existing
logger.debug or guard it with a NODE_ENV/dev-only check) so no plain console.log
executes in production.
| const tokenClient = window.google.accounts.oauth2.initTokenClient({ | ||
| client_id: GOOGLE_OAUTH_CLIENT_ID!, |
There was a problem hiding this comment.
Validate GOOGLE_OAUTH_CLIENT_ID before use.
The non-null assertion on line 92 will cause a runtime error if GOOGLE_OAUTH_CLIENT_ID is undefined. Add a validation guard similar to the API check above.
🔎 Proposed fix
if (!window.google?.accounts?.oauth2) {
onLoadingChange(false);
reject(new Error('Google Sign-In API not loaded'));
return;
}
+ if (!GOOGLE_OAUTH_CLIENT_ID) {
+ onLoadingChange(false);
+ reject(new Error('Google OAuth Client ID not configured'));
+ return;
+ }
+
const tokenClient = window.google.accounts.oauth2.initTokenClient({
- client_id: GOOGLE_OAUTH_CLIENT_ID!,
+ client_id: GOOGLE_OAUTH_CLIENT_ID,📝 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.
| const tokenClient = window.google.accounts.oauth2.initTokenClient({ | |
| client_id: GOOGLE_OAUTH_CLIENT_ID!, | |
| if (!window.google?.accounts?.oauth2) { | |
| onLoadingChange(false); | |
| reject(new Error('Google Sign-In API not loaded')); | |
| return; | |
| } | |
| if (!GOOGLE_OAUTH_CLIENT_ID) { | |
| onLoadingChange(false); | |
| reject(new Error('Google OAuth Client ID not configured')); | |
| return; | |
| } | |
| const tokenClient = window.google.accounts.oauth2.initTokenClient({ | |
| client_id: GOOGLE_OAUTH_CLIENT_ID, |
🤖 Prompt for AI Agents
In @apps/bubble-studio/src/services/googlePickerService.ts around lines 91 - 92,
The code uses a non-null assertion on GOOGLE_OAUTH_CLIENT_ID when calling
window.google.accounts.oauth2.initTokenClient which will throw at runtime if the
env var is undefined; add a guard before initializing tokenClient (similar to
the existing API check) to verify GOOGLE_OAUTH_CLIENT_ID is defined and handle
the missing value by logging/erroring or early returning. Specifically, check
the variable before calling initTokenClient, and only call
window.google.accounts.oauth2.initTokenClient({ client_id:
GOOGLE_OAUTH_CLIENT_ID, ... }) when it is present; otherwise surface a clear
error via the same processLogger or throw to avoid passing undefined into
initTokenClient.
| // Success case - loading state will be managed by the picker | ||
|
|
||
| // Got the token! Cache it for future use | ||
| const accountEmail = response.hd || response.email || 'default'; | ||
| cachePickerToken(response.access_token!, accountEmail); | ||
|
|
||
| resolve(response.access_token!); |
There was a problem hiding this comment.
Validate access_token exists in response.
The non-null assertions on lines 122 and 124 assume access_token is present in success responses, but Google's OAuth flow can return responses without a token in certain edge cases. Add explicit validation to prevent runtime errors.
🔎 Proposed fix
// Success case - loading state will be managed by the picker
+ if (!response.access_token) {
+ onLoadingChange(false);
+ reject(new Error('No access token received from Google'));
+ return;
+ }
+
// Got the token! Cache it for future use
const accountEmail = response.hd || response.email || 'default';
- cachePickerToken(response.access_token!, accountEmail);
+ cachePickerToken(response.access_token, accountEmail);
- resolve(response.access_token!);
+ resolve(response.access_token);🤖 Prompt for AI Agents
In @apps/bubble-studio/src/services/googlePickerService.ts around lines 118 -
124, The success branch assumes response.access_token exists; explicitly check
that response.access_token is defined before calling cachePickerToken and
resolve. If it's missing, call reject (or throw) with a clear error message
(e.g., "No access_token in Google response") instead of using non-null
assertions; otherwise store the token in accountEmail via the existing
accountEmail logic and then call cachePickerToken(response.access_token,
accountEmail) followed by resolve(response.access_token). Ensure you update the
code paths around cachePickerToken and resolve to use the validated token
variable rather than response.access_token!.
| // Extract project number from OAuth Client ID for setAppId() | ||
| // Client ID format: {project_number}.apps.googleusercontent.com | ||
| const projectNumber = GOOGLE_OAUTH_CLIENT_ID?.split('.')[0]; | ||
|
|
||
| // Create the picker | ||
| const pickerBuilder = new googlePicker.PickerBuilder(); | ||
| const builtPicker = pickerBuilder | ||
| .setOAuthToken(accessToken) | ||
| .setDeveloperKey(GOOGLE_API_KEY!) | ||
| .setAppId(projectNumber!) |
There was a problem hiding this comment.
Validate environment variables before using them.
The code extracts projectNumber from GOOGLE_OAUTH_CLIENT_ID using optional chaining (line 181), which means it can be undefined. Then both GOOGLE_API_KEY and projectNumber are used with non-null assertions (lines 187-188). If either environment variable is missing or malformed, this will cause runtime errors.
🔎 Proposed fix
+ if (!GOOGLE_API_KEY) {
+ onLoadingChange(false);
+ throw new Error('Google API key not configured');
+ }
+
// Extract project number from OAuth Client ID for setAppId()
// Client ID format: {project_number}.apps.googleusercontent.com
const projectNumber = GOOGLE_OAUTH_CLIENT_ID?.split('.')[0];
+ if (!projectNumber) {
+ onLoadingChange(false);
+ throw new Error('Invalid or missing Google OAuth Client ID');
+ }
// Create the picker
const pickerBuilder = new googlePicker.PickerBuilder();
const builtPicker = pickerBuilder
.setOAuthToken(accessToken)
- .setDeveloperKey(GOOGLE_API_KEY!)
- .setAppId(projectNumber!)
+ .setDeveloperKey(GOOGLE_API_KEY)
+ .setAppId(projectNumber)🤖 Prompt for AI Agents
In @apps/bubble-studio/src/services/googlePickerService.ts around lines 179 -
188, The code uses GOOGLE_OAUTH_CLIENT_ID and GOOGLE_API_KEY without validating
them: parse GOOGLE_OAUTH_CLIENT_ID into projectNumber safely (e.g., check it
exists and matches the expected pattern like
/^\d+\.apps\.googleusercontent\.com$/ or split and ensure the first segment is
numeric) and ensure GOOGLE_API_KEY is present before calling
googlePicker.PickerBuilder().setOAuthToken(...).setDeveloperKey(...).setAppId(...).
If either env var is missing or malformed, throw or return a controlled error
(or log and abort) instead of using non-null assertions; update the usage around
projectNumber, GOOGLE_API_KEY, and the googlePicker.PickerBuilder/setAppId call
to only proceed when validated.
- Add DocsView with setIncludeFolders and setSelectFolderEnabled for folder type - Update AI prompts to guide folder_id naming pattern for Google Drive fields - Ensure folder picker allows selecting folders, not just browsing Closes bubblelabai#204
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/bubble-studio/src/services/googlePickerService.ts:
- Line 218: Remove the debug console.log in googlePickerService.ts that prints
file selection; specifically delete or disable the line logging "File selected:"
with { fileId, fileName } (or replace it with a proper production logger at
debug level if needed) inside the function handling the picker selection so the
browser console is no longer cluttered.
🧹 Nitpick comments (1)
packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts (1)
204-213: Clarify the exact pattern-matching rules.The documentation states that "flexible naming patterns" like
folder2_id,source_folder_id, andoutput_folder_idwork, but doesn't specify the exact matching logic. Does the frontend match any field containing"folder_id"? Any field ending with"_id"or"Id"preceded by specific keywords?Consider adding explicit pattern-matching rules (e.g., "matches any field name containing 'folder_id' or 'folderId' as a substring") to ensure AI agents and developers have consistent expectations.
🔎 Suggested clarification
GOOGLE DRIVE INTEGRATION: -When a field represents a Google Drive folder or file ID, use these naming patterns to automatically enable the Google Picker UI: +When a field represents a Google Drive folder or file ID, use these naming patterns to automatically enable the Google Picker UI. +The frontend matches field names containing these keywords (case-insensitive, with or without underscores): - folder_id or folderId: For Google Drive folder IDs (enables folder picker with selection) - spreadsheet_id or spreadsheetId: For Google Sheets IDs (enables spreadsheet picker) - document_id or documentId: For Google Docs IDs (enables document picker) - file_id or fileId: For generic Drive file IDs (enables file picker) Always add @canBeFile false to these ID fields since they are identifiers, not content. -The naming pattern is flexible - folder2_id, source_folder_id, output_folder_id all work. +The naming pattern is flexible - folder2_id, source_folder_id, output_folder_id, targetFolderId all work as long as they contain the base keyword.Additionally, consider showing examples for all four picker types (folder, spreadsheet, document, file) in the examples section to help AI agents understand the full range of supported patterns.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/bubble-studio/src/services/googlePickerService.tspackages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/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/bubbleflow-generation-prompts.ts
packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts
📄 CodeRabbit inference engine (.cursor/rules/bubblelab.mdc)
Refer to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts for documentation on how bubble flow is supposed to be generated
Files:
packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts
🧠 Learnings (4)
📚 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-shared-schemas/src/bubbleflow-generation-prompts.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/bubbleflow-generation-prompts.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:
apps/bubble-studio/src/services/googlePickerService.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:
apps/bubble-studio/src/services/googlePickerService.ts
🧬 Code graph analysis (1)
apps/bubble-studio/src/services/googlePickerService.ts (2)
apps/bubble-studio/src/env.ts (2)
GOOGLE_OAUTH_CLIENT_ID(36-37)GOOGLE_API_KEY(34-35)apps/bubble-studio/src/utils/googlePickerCache.ts (1)
cachePickerToken(67-84)
🔇 Additional comments (3)
packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts (3)
228-230: LGTM! Clear and helpful field documentation.The updated example provides clear, user-friendly instructions for obtaining a Google Drive folder ID, following the established pattern of explaining what the field is and where to find the information.
276-280: LGTM! Proper implementation of Google Drive folder ID field.The
folder_idfield demonstrates correct usage of the newly documented Google Drive integration pattern: optional field with clear documentation,@canBeFile falsedirective, and helpful instructions for users to locate the folder ID.
204-280: Runpnpm build:coreafter these changes.As per coding guidelines, modifications to shared schemas require rebuilding since it is a separate package and types need to be regenerated.
| const fileId = file.id; | ||
| const fileName = file.name; | ||
|
|
||
| console.log('File selected:', { fileId, fileName }); |
There was a problem hiding this comment.
Remove debug console.log statement.
This debug log should be removed to avoid cluttering the browser console in production environments.
🔎 Proposed fix
const fileId = file.id;
const fileName = file.name;
- console.log('File selected:', { fileId, fileName });
onSelect(fileId, fileName);
onLoadingChange(false);📝 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.
| console.log('File selected:', { fileId, fileName }); | |
| const fileId = file.id; | |
| const fileName = file.name; | |
| onSelect(fileId, fileName); | |
| onLoadingChange(false); |
🤖 Prompt for AI Agents
In @apps/bubble-studio/src/services/googlePickerService.ts at line 218, Remove
the debug console.log in googlePickerService.ts that prints file selection;
specifically delete or disable the line logging "File selected:" with { fileId,
fileName } (or replace it with a proper production logger at debug level if
needed) inside the function handling the picker selection so the browser console
is no longer cluttered.
3 participants
Summary
Implements native Google Drive file picker integration for Bubble Studio. Users can now browse and select Drive files directly from input fields using Google's Picker UI.
Features:
spreadsheet_id,document_id,folder_id,file_id)Implementation:
Related Issues
Closes #204
Type of Change
Checklist
pnpm checkand all tests passAdditional Context
New Files:
apps/bubble-studio/src/components/GoogleFilePicker.tsx- Main picker componentapps/bubble-studio/src/hooks/useGooglePicker.ts- API loading hookapps/bubble-studio/src/services/googlePickerService.ts- OAuth & Picker logicapps/bubble-studio/src/utils/googlePickerCache.ts- Token caching utilityModified Files:
apps/bubble-studio/src/components/InputFieldsRenderer.tsx- Integration pointapps/bubble-studio/src/env.ts- Config exportsapps/bubble-studio/.env.example- Setup documentationapps/bubble-studio/index.html- Picker API scriptsUser Flow:
Deployment Notes:
Required Setup for Google Picker Feature:
Create Google Cloud Project:
Enable APIs:
Create API Key:
VITE_GOOGLE_API_KEYGet OAuth Client ID:
GOOGLE_OAUTH_CLIENT_IDvalueVITE_GOOGLE_OAUTH_CLIENT_IDAdd to Environment:
# Frontend .env VITE_GOOGLE_API_KEY=your_api_key VITE_GOOGLE_OAUTH_CLIENT_ID=your_client_idSummary by CodeRabbit
New Features
Documentation / Config
✏️ Tip: You can customize this high-level summary in your review settings.