Prefer Vertex AI Advanced Service for Gemini calls#75
Prefer Vertex AI Advanced Service for Gemini calls#75Benjamin-Sayaque wants to merge 17 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Vertex AI Advanced Service path for Gemini requests with UrlFetch fallback, updates README prerequisites and testing docs, exposes GenAIApp.resetGeminiAuthState(), and adds a test harness to validate API-key vs Vertex AI authentication modes. ChangesVertex AI Advanced Service with Fallback
Sequence DiagramsequenceDiagram
participant ChatRun as Chat.run()
participant CallVertexAi as _callVertexAi()
participant AdvService as VertexAI.generateContent
participant UrlFetch as _callGenAIApi()
ChatRun->>CallVertexAi: endpointUrl, payload
CallVertexAi->>AdvService: attempt generateContent (advanced service)
alt Advanced Service succeeds
AdvService-->>CallVertexAi: normalized response (candidate content)
else Advanced Service fails
AdvService-->>CallVertexAi: error / missing method
CallVertexAi->>UrlFetch: fallback to UrlFetch-based API call
UrlFetch-->>CallVertexAi: API response
end
CallVertexAi-->>ChatRun: unified message response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 552e1c2db5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/code.gs`:
- Around line 1448-1456: The _getVertexAiAdvancedService() function currently
only checks global VertexAI/vertexai and misses other userSymbol names; update
it to first attempt VertexAI/vertexai, then iterate over properties on
globalThis to find any object whose structure matches the Vertex AI Advanced
Service (e.g., has Endpoints and Endpoints.generateContent), and return that
object; fall back to throwing the existing error only if no matching global
symbol is found; reference _getVertexAiAdvancedService, globalThis, and the
Endpoints.generateContent signature when implementing the detection so any
enabledAdvancedServices[].userSymbol is supported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 40cbb577-ea6c-463c-a73a-bd7669d7ed42
📒 Files selected for processing (2)
README.mdsrc/code.gs
📜 Review details
🔇 Additional comments (1)
src/code.gs (1)
1523-1526: 🏗️ Heavy liftFix Vertex AI Advanced Service tool-calling config key mapping
Insrc/code.gs(1523-1526),_buildVertexAiAdvancedServicePayload(payload)deep-clonespayload, deletesmodel, and forwards the tool-calling config keys unchanged (e.g.,tool_config.function_calling_config.allowed_function_names). Vertex AI’s tool-calling config usestoolConfig/functionCallingConfig/allowedFunctionNames, so the advanced-service path may ignore the function-calling restrictions. Convert the forwarded tool-calling config to the expected camelCase schema before sending the request. https://cloud.google.com/vertex-ai/generative-ai/docs/reference/rest/v1/projects.locations.publishers.models/generateContent
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/code.gs`:
- Around line 1456-1460: The globalThis scan only recognizes services that match
the old shape; update the detection in the for-loop (and the similar blocks
around the other occurrences) to treat candidates that expose any supported
Vertex AI method path as matches: instead of only calling
_isVertexAiAdvancedService(candidateService), check for
endpoints?.generateContent or the nested
projects?.locations?.publishers?.models?.generateContent path (or simply call
_getVertexAiGenerateContentMethod(candidateService) and accept a non-null
result) and return that candidateService when found so we don’t silently fall
back to UrlFetchApp; apply the same change to the other two occurrences
referenced in the review.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 108b764c-862d-47fb-a332-178bafedf416
📒 Files selected for processing (1)
src/code.gs
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/code.gs (2)
1479-1484: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winRemove unused
_isVertexAiAdvancedServicefunction.After the fix on Line 1459 that now uses
_getVertexAiGenerateContentMethodfor detection, this function is no longer called anywhere and can be deleted.♻️ Suggested removal
- /** - * Checks whether an object matches the Apps Script Vertex AI Advanced Service - * shape exposed for any enabledAdvancedServices[].userSymbol. - * - * `@private` - * `@param` {Object} service - A global object candidate from globalThis. - * `@returns` {boolean} - True when the object exposes the Endpoints.generateContent method. - */ - function _isVertexAiAdvancedService(service) { - return !!service - && typeof service === 'object' - && !!service.Endpoints - && typeof service.Endpoints.generateContent === 'function'; - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1479 - 1484, The function _isVertexAiAdvancedService is now unused after switching to _getVertexAiGenerateContentMethod; delete the entire _isVertexAiAdvancedService function definition from the file, ensure there are no remaining references to it elsewhere, and run tests/lint to confirm no breakages; if any call sites exist, replace them to use _getVertexAiGenerateContentMethod instead.
1538-1541: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueUnreachable
!locationcheck.The ternary on Line 1538 always produces either
'global'(when!regionor model includes 'gemini-3') orregion(which must be truthy to reach the else branch). Theif (!location)guard can never be true.♻️ Remove unreachable check
const location = (!region || payload.model.includes('gemini-3')) ? 'global' : region; - if (!location) { - throw new Error('Missing Vertex AI location for Advanced Service call.'); - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1538 - 1541, The check for `if (!location)` is unreachable because `const location = (!region || payload.model.includes('gemini-3')) ? 'global' : region` always yields a non-empty string ('global' or the truthy `region`); remove the redundant guard and its Error throw. Locate the assignment to `location` and the subsequent `if (!location)` block and delete the `if` branch (or replace it with an assertion/log only if you want defensive diagnostics), ensuring callers rely on `location` being either 'global' or the provided `region`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/code.gs`:
- Around line 1479-1484: The function _isVertexAiAdvancedService is now unused
after switching to _getVertexAiGenerateContentMethod; delete the entire
_isVertexAiAdvancedService function definition from the file, ensure there are
no remaining references to it elsewhere, and run tests/lint to confirm no
breakages; if any call sites exist, replace them to use
_getVertexAiGenerateContentMethod instead.
- Around line 1538-1541: The check for `if (!location)` is unreachable because
`const location = (!region || payload.model.includes('gemini-3')) ? 'global' :
region` always yields a non-empty string ('global' or the truthy `region`);
remove the redundant guard and its Error throw. Locate the assignment to
`location` and the subsequent `if (!location)` block and delete the `if` branch
(or replace it with an assertion/log only if you want defensive diagnostics),
ensuring callers rely on `location` being either 'global' or the provided
`region`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 212c4764-2752-459c-9798-a77a414be0c0
📒 Files selected for processing (1)
src/code.gs
📜 Review details
🔇 Additional comments (2)
src/code.gs (2)
1456-1466: LGTM!
1400-1408: LGTM!Also applies to: 1419-1427, 1569-1595
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/code.gs (1)
1476-1478: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueJSDoc return type is outdated.
The function now returns
{method: Function, path: string}but the JSDoc still documents@returns {Function}.📝 Suggested fix
/** * Resolves the generated Apps Script method for projects.locations.publishers.models.generateContent. * Apps Script Advanced Service names can differ by release/casing, so support the known variants. * * `@private` * `@param` {Object} service - The Vertex AI Advanced Service object. - * `@returns` {Function} - A generateContent method bound to its collection object. + * `@returns` {{method: Function, path: string}} - The generateContent method and its collection path. * `@throws` {Error} If the method is not exposed by the enabled Advanced Service. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1476 - 1478, Update the JSDoc for the function that returns the generateContent binding so the `@returns` reflects the new shape: change from `@returns` {Function} to `@returns` {{method: Function, path: string}} and update the descriptive text to say it returns an object with a method (the bound generateContent function) and the API path; locate the docblock associated with the generateContent binding code (the function that accepts the service parameter and throws if the method isn't exposed) and modify the `@returns` line and description accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/code.gs`:
- Around line 1543-1548: In _buildVertexAiAdvancedServicePayload, avoid directly
deleting advancedServicePayload.tool_config.includeServerSideToolInvocations
because tool_config may be undefined on new call paths; guard the nested delete
by first checking advancedServicePayload.tool_config exists (e.g., if
(advancedServicePayload.tool_config) { delete
advancedServicePayload.tool_config.includeServerSideToolInvocations }) or use
optional chaining semantics before deleting the nested property while preserving
the existing removal of advancedServicePayload.model.
---
Outside diff comments:
In `@src/code.gs`:
- Around line 1476-1478: Update the JSDoc for the function that returns the
generateContent binding so the `@returns` reflects the new shape: change from
`@returns` {Function} to `@returns` {{method: Function, path: string}} and update
the descriptive text to say it returns an object with a method (the bound
generateContent function) and the API path; locate the docblock associated with
the generateContent binding code (the function that accepts the service
parameter and throws if the method isn't exposed) and modify the `@returns` line
and description accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e08181f7-88f5-455f-944b-07d11a824eab
📒 Files selected for processing (1)
src/code.gs
📜 Review details
🔇 Additional comments (3)
src/code.gs (3)
516-521: LGTM!
1419-1427: LGTM!
1481-1504: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/code.gs`:
- Around line 1543-1549: _buildVertexAiAdvancedServicePayload currently
preserves Gemini-style keys under tool_config (snake_case) which Vertex AI
Advanced Service expects as camelCase under
toolConfig.functionCallingConfig.allowedFunctionNames; update the function to
detect payload.tool_config, convert it into a toolConfig object with camelCased
nested names (toolConfig.functionCallingConfig.allowedFunctionNames) by mapping
tool_config.function_calling_config.allowed_function_names to
toolConfig.functionCallingConfig.allowedFunctionNames, drop/include other
properties as before (still remove includeServerSideToolInvocations), delete the
original tool_config, and return the transformed advancedServicePayload so
Vertex receives camelCase keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2599235e-b07e-4567-bdad-19798a02285e
📒 Files selected for processing (1)
src/code.gs
📜 Review details
🔇 Additional comments (1)
src/code.gs (1)
1477-1477: LGTM!
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/code.gs (1)
1543-1549:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTranslate
tool_configto the Vertex API field names before calling the Advanced Service.Apps Script advanced services are thin wrappers over the underlying Google API request schema, and the Vertex AI function-calling request is documented with
toolConfig.functionCallingConfig.allowedFunctionNames. This adapter still forwards Gemini-styletool_config.function_calling_config.allowed_function_names, so forced tool-call constraints can be ignored on the preferred Advanced Service path. (developers.google.com)♻️ Minimal fix
function _buildVertexAiAdvancedServicePayload(payload) { const advancedServicePayload = JSON.parse(JSON.stringify(payload || {})); delete advancedServicePayload.model; if (advancedServicePayload.tool_config) { - delete advancedServicePayload.tool_config.includeServerSideToolInvocations; + const toolConfig = advancedServicePayload.tool_config; + delete toolConfig.includeServerSideToolInvocations; + + if (toolConfig.function_calling_config) { + toolConfig.functionCallingConfig = toolConfig.function_calling_config; + delete toolConfig.function_calling_config; + + if (toolConfig.functionCallingConfig.allowed_function_names !== undefined) { + toolConfig.functionCallingConfig.allowedFunctionNames = + toolConfig.functionCallingConfig.allowed_function_names; + delete toolConfig.functionCallingConfig.allowed_function_names; + } + } + + advancedServicePayload.toolConfig = toolConfig; + delete advancedServicePayload.tool_config; } return advancedServicePayload; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1543 - 1549, The adapter currently leaves Gemini-style tool_config keys unchanged so _buildVertexAiAdvancedServicePayload forwards tool_config.function_calling_config.allowed_function_names which the Vertex Advanced Service expects as toolConfig.functionCallingConfig.allowedFunctionNames; update _buildVertexAiAdvancedServicePayload to map/translate tool_config.function_calling_config.allowed_function_names -> toolConfig.functionCallingConfig.allowedFunctionNames (and any other snake_case function_calling_config fields) before returning advancedServicePayload so the Advanced Service receives the correct Vertex API field names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/code.gs`:
- Around line 1543-1549: The adapter currently leaves Gemini-style tool_config
keys unchanged so _buildVertexAiAdvancedServicePayload forwards
tool_config.function_calling_config.allowed_function_names which the Vertex
Advanced Service expects as
toolConfig.functionCallingConfig.allowedFunctionNames; update
_buildVertexAiAdvancedServicePayload to map/translate
tool_config.function_calling_config.allowed_function_names ->
toolConfig.functionCallingConfig.allowedFunctionNames (and any other snake_case
function_calling_config fields) before returning advancedServicePayload so the
Advanced Service receives the correct Vertex API field names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f1c34cc2-5318-4eec-a0e9-021e2ad8b0d2
📒 Files selected for processing (1)
src/code.gs
📜 Review details
🔇 Additional comments (1)
src/code.gs (1)
1448-1504: LGTM!
|
@coderabbitai create an issue for this PR |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: gh issue create \
--repo scriptit-fr/GenAIApp \
--title "Prefer Vertex AI Advanced Service for Gemini calls" \
--assignee aubrypaul \
--body "## Overview
This issue tracks the work introduced in PR `#75` to prefer the Google Apps Script **Vertex AI Advanced Service** for Vertex AI/Gemini requests when available, while keeping the existing \`UrlFetchApp\` path as a transparent fallback.
## Background
PR: https://github.com/scriptit-fr/GenAIApp/pull/75
Requested by: `@aubrypaul`
## Motivation
- Use the native Apps Script Vertex AI Advanced Service for Gemini calls instead of raw \`UrlFetchApp\` HTTP calls when the service is available.
- Retain existing behavior, response shape, and error handling as a fallback.
## Changes Introduced
- Added \`callVertexAi(endpoint, payload)\` dispatcher that tries the Advanced Service first and falls back to \`_callGenAIApi()\` (UrlFetchApp path).
- Added Advanced Service helpers: \`_getVertexAiAdvancedService()\`, \`_getVertexAiGenerateContentMethod()\`, \`_getVertexAiModelResource()\`, \`_buildVertexAiAdvancedServicePayload()\`, \`_normalizeVertexAiResponse()\`, \`_logVertexAiAdvancedServiceFallback()\`.
- Routing: Gemini requests using \`setGeminiAuth()\` (GCP-authenticated, no API key) are now sent through \`callVertexAi()\`. API-key Gemini calls and all other UrlFetchApp calls remain unchanged.
- Updated \`README.md\` to document the Advanced Service preference and required OAuth scopes.
## Notes
- No automated unit tests were added; the fallback preserves existing runtime/retry behavior.
- A syntax check was performed by running \`node --check\` on the source (succeeded).
- An autofix commit (\`97a7711\`) was applied to \`src/code.gs\` as part of the review process.
"Length of output: 1845
It covers the motivation, changes introduced, and notes from PR |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/code.gs (1)
1400-1408:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Function name mismatch will cause ReferenceError on fallback.
Line 1406 calls
callVertexAiWithUrlFetchFallback(no underscore) but the function is defined as_callVertexAiWithUrlFetchFallback(with underscore) on line 1437. This will throwReferenceError: callVertexAiWithUrlFetchFallback is not definedwhenever the Advanced Service call fails and the fallback is triggered.🐛 Proposed fix
function _callVertexAi(endpoint, payload) { try { return _callVertexAiWithAdvancedService(endpoint, payload); } catch (err) { _logVertexAiAdvancedServiceFallback(err); - return callVertexAiWithUrlFetchFallback(endpoint, payload); + return _callVertexAiWithUrlFetchFallback(endpoint, payload); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1400 - 1408, The fallback call uses the wrong function name and will throw a ReferenceError; update the fallback invocation in _callVertexAi to call the correctly defined function _callVertexAiWithUrlFetchFallback (replace callVertexAiWithUrlFetchFallback with _callVertexAiWithUrlFetchFallback) and ensure the referenced helper _logVertexAiAdvancedServiceFallback and _callVertexAiWithAdvancedService remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/code.gs`:
- Around line 1400-1408: The fallback call uses the wrong function name and will
throw a ReferenceError; update the fallback invocation in _callVertexAi to call
the correctly defined function _callVertexAiWithUrlFetchFallback (replace
callVertexAiWithUrlFetchFallback with _callVertexAiWithUrlFetchFallback) and
ensure the referenced helper _logVertexAiAdvancedServiceFallback and
_callVertexAiWithAdvancedService remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c3451c60-58ed-4d35-9aa5-c2724b75bbd8
📒 Files selected for processing (1)
src/code.gs
📜 Review details
🔇 Additional comments (9)
src/code.gs (9)
1543-1550: Thetool_configtotoolConfigcamelCase translation issue flagged in a prior review remains unaddressed. Snake_case keys (tool_config,function_calling_config,allowed_function_names) are passed to the Advanced Service which expects camelCase (toolConfig,functionCallingConfig,allowedFunctionNames). This may cause function-calling constraints to be silently dropped.
516-521: LGTM!
1419-1427: LGTM!
1437-1439: LGTM!
1448-1469: LGTM!
1480-1504: LGTM!
1515-1534: LGTM!
1562-1588: LGTM!
1596-1601: LGTM!
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/code.gs (1)
1570-1576:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent fallback on valid Vertex AI “blocked” responses (promptFeedback / empty candidate content).
In
src/code.gs,_normalizeVertexAiResponse()throws wheneverfirstCandidate?.contentis falsy (around lines 1570-1576). For Vertex AI, blocked-input responses can come back withpromptFeedbackand no candidates, and blocked-output responses can include a candidate whosecontentis empty (withfinishReason, oftenSAFETY)—both cases should be treated as a valid “no usable text” outcome instead of an error. Because_callVertexAi()catches any exception and immediately falls back to_callVertexAiWithUrlFetchFallback()(calling_callGenAIApi()), these “blocked” responses currently trigger an unnecessary retry through the UrlFetch path instead of preserving the expected Gemini-likenullbehavior.♻️ Minimal fix
const firstCandidate = response.candidates?.[0]; const responseMessage = firstCandidate?.content || null; const finish_reason = firstCandidate?.finishReason || null; - if (!responseMessage) { - throw new Error('Vertex AI Advanced Service returned no candidate content.'); + if (!firstCandidate) { + if (response.promptFeedback) { + return null; + } + throw new Error('Vertex AI Advanced Service returned no candidate content.'); + } + if (!responseMessage) { + if (finish_reason === 'SAFETY' || finish_reason === 'BLOCKLIST' || finish_reason === 'PROHIBITED_CONTENT') { + return null; + } + throw new Error('Vertex AI Advanced Service returned no candidate content.'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1570 - 1576, The current _normalizeVertexAiResponse() throws when firstCandidate?.content is falsy, causing blocked Vertex AI responses to trigger the UrlFetch fallback; instead, detect Vertex AI "blocked" signatures (response.promptFeedback present or a candidate with empty/undefined content but with a finishReason like "SAFETY") and return a normalized "no usable text" outcome (e.g., text: null and finish_reason set to firstCandidate?.finishReason or null) rather than throwing; update the logic around firstCandidate, response.candidates, promptFeedback and finish_reason so _callVertexAi() will not throw and fall back to _callVertexAiWithUrlFetchFallback()/_callGenAIApi() for these valid blocked responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/code.gs`:
- Around line 1570-1576: The current _normalizeVertexAiResponse() throws when
firstCandidate?.content is falsy, causing blocked Vertex AI responses to trigger
the UrlFetch fallback; instead, detect Vertex AI "blocked" signatures
(response.promptFeedback present or a candidate with empty/undefined content but
with a finishReason like "SAFETY") and return a normalized "no usable text"
outcome (e.g., text: null and finish_reason set to firstCandidate?.finishReason
or null) rather than throwing; update the logic around firstCandidate,
response.candidates, promptFeedback and finish_reason so _callVertexAi() will
not throw and fall back to _callVertexAiWithUrlFetchFallback()/_callGenAIApi()
for these valid blocked responses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fa1aade2-0fe8-4935-90ec-907c9cf07d50
📒 Files selected for processing (1)
src/code.gs
Fixed 2 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
…r-authentication-modes Add Gemini authentication-mode test entrypoint and README docs
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/code.gs (1)
1543-1549:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConvert Gemini
tool_configto VertextoolConfigbefore Advanced Service call.Line 1546 still forwards Gemini snake_case config. This can cause Advanced Service calls to miss function-calling constraints and behave differently from the UrlFetch fallback path.
♻️ Minimal patch
function _buildVertexAiAdvancedServicePayload(payload) { const advancedServicePayload = JSON.parse(JSON.stringify(payload || {})); delete advancedServicePayload.model; if (advancedServicePayload.tool_config) { - delete advancedServicePayload.tool_config.includeServerSideToolInvocations; + const toolConfig = advancedServicePayload.tool_config; + delete toolConfig.includeServerSideToolInvocations; + + if (toolConfig.function_calling_config) { + const functionCallingConfig = toolConfig.function_calling_config; + if (functionCallingConfig.allowed_function_names !== undefined) { + functionCallingConfig.allowedFunctionNames = functionCallingConfig.allowed_function_names; + delete functionCallingConfig.allowed_function_names; + } + toolConfig.functionCallingConfig = functionCallingConfig; + delete toolConfig.function_calling_config; + } + + advancedServicePayload.toolConfig = toolConfig; + delete advancedServicePayload.tool_config; } return advancedServicePayload; }For Google Apps Script Vertex AI Advanced Service `generateContent`, are snake_case keys like `tool_config.function_calling_config.allowed_function_names` accepted, or is camelCase `toolConfig.functionCallingConfig.allowedFunctionNames` required?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1543 - 1549, The _buildVertexAiAdvancedServicePayload function currently forwards Gemini's snake_case tool_config; convert it to Vertex's camelCase shape before returning: map tool_config -> toolConfig, tool_config.function_calling_config -> toolConfig.functionCallingConfig, and tool_config.function_calling_config.allowed_function_names -> toolConfig.functionCallingConfig.allowedFunctionNames (preserving values), while still deleting model and removing includeServerSideToolInvocations as currently done; update the function to construct/assign the camelCase keys from the existing snake_case nested objects so Advanced Service generateContent receives the expected Vertex field names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/testFunctions.gs`:
- Around line 61-67: The requireAuthTestCredential function should treat
whitespace-only strings as missing: obtain the config via
getAuthTestConfigValue(name, "") and trim the result before validating; if the
trimmed value is empty or falsy, throw the existing Error message, otherwise
return the trimmed value. Update references to value in
requireAuthTestCredential to use the trimmed string so tests fail fast on blank
credentials.
---
Duplicate comments:
In `@src/code.gs`:
- Around line 1543-1549: The _buildVertexAiAdvancedServicePayload function
currently forwards Gemini's snake_case tool_config; convert it to Vertex's
camelCase shape before returning: map tool_config -> toolConfig,
tool_config.function_calling_config -> toolConfig.functionCallingConfig, and
tool_config.function_calling_config.allowed_function_names ->
toolConfig.functionCallingConfig.allowedFunctionNames (preserving values), while
still deleting model and removing includeServerSideToolInvocations as currently
done; update the function to construct/assign the camelCase keys from the
existing snake_case nested objects so Advanced Service generateContent receives
the expected Vertex field names.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 74fe1049-93a0-4aa3-a1e2-39a452f198af
📒 Files selected for processing (3)
README.mdsrc/code.gssrc/testFunctions.gs
📜 Review details
🔇 Additional comments (3)
README.md (1)
36-36: LGTM!Also applies to: 465-490
src/code.gs (1)
516-521: LGTM!Also applies to: 1400-1427, 1448-1504, 1515-1534, 1562-1601, 2674-2683
src/testFunctions.gs (1)
5-59: LGTM!Also applies to: 69-141, 145-147
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/testFunctions.gs (2)
124-125:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTreat empty model output as a test failure.
String(response).lengthis non-zero forundefinedandnull, so both auth helpers can report success without a usable reply. Throw unlesschat.run()returns a non-empty string before logging completion.💡 Suggested fix
function testApiKeyAuthentication() { const geminiApiKey = requireAuthTestCredential(AUTH_TEST_CONFIG_KEYS.GEMINI_API_KEY, "API key authentication"); @@ const chat = GenAIApp.newChat(); chat.addMessage("Reply with exactly: API key authentication ok"); const response = chat.run({ model: GEMINI_MODEL, max_tokens: 64 }); + if (typeof response !== "string" || !response.trim()) { + throw new Error("[GenAIApp tests] API key authentication returned no text response."); + } console.log(`[GenAIApp tests] API key authentication completed with response length ${String(response).length}.`); } @@ function testVertexAiAuthentication() { @@ const chat = GenAIApp.newChat(); chat.addMessage("Reply with exactly: Vertex AI authentication ok"); const response = chat.run({ model: GEMINI_MODEL, max_tokens: 64 }); + if (typeof response !== "string" || !response.trim()) { + throw new Error("[GenAIApp tests] Vertex AI authentication returned no text response."); + } console.log(`[GenAIApp tests] Vertex AI authentication completed with response length ${String(response).length}.`); }Also applies to: 139-140
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/testFunctions.gs` around lines 124 - 125, The test currently treats undefined/null response as success because String(response).length > 0; update the logic after chat.run({ model: GEMINI_MODEL, max_tokens: 64 }) to validate that the returned response is a non-empty string (e.g., typeof response === 'string' && response.trim().length > 0) and throw an Error (or reject) if not before calling console.log; apply the same check for the other auth helper usage (the similar chat.run call later) so empty model outputs are considered test failures rather than successes.
145-146:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore Vertex auth before the shared Gemini matrix runs.
Line 146 clears all Gemini auth, but Lines 161-166 only restore the API-key path. In a setup that provides
VERTEX_AI_GCP_PROJECT_IDand noGEMINI_API_KEY, every later Gemini case inrunTestAcrossModels()executes unauthenticated.💡 Suggested fix
function runTestAcrossModels(testName, setupFunction, runOptions = {}) { - const geminiApiKey = getAuthTestConfigValue("GEMINI_API_KEY", ""); + const geminiApiKey = getAuthTestConfigValue(AUTH_TEST_CONFIG_KEYS.GEMINI_API_KEY, ""); + const vertexProjectId = getAuthTestConfigValue(AUTH_TEST_CONFIG_KEYS.VERTEX_AI_GCP_PROJECT_ID, ""); + const vertexRegion = getAuthTestConfigValue(AUTH_TEST_CONFIG_KEYS.VERTEX_AI_GCP_REGION, ""); const openAiApiKey = getAuthTestConfigValue("OPEN_AI_API_KEY", ""); + GenAIApp.resetGeminiAuthState(); if (geminiApiKey) { GenAIApp.setGeminiAPIKey(geminiApiKey); } + else if (vertexProjectId) { + GenAIApp.setGeminiAuth(vertexProjectId, vertexRegion); + } if (openAiApiKey) { GenAIApp.setOpenAIAPIKey(openAiApiKey); }Also applies to: 160-170
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/testFunctions.gs` around lines 145 - 146, The reset call GenAIApp.resetGeminiAuthState() is clearing all Gemini auth before the shared Gemini matrix in runTestAcrossModels(), causing Vertex-only setups (VERTEX_AI_GCP_PROJECT_ID present, GEMINI_API_KEY absent) to run unauthenticated; modify the sequence so Vertex auth is preserved or restored before the shared matrix: either remove or move GenAIApp.resetGemminiAuthState() until after tests that rely on Vertex, or change the reset to only clear the API-key path while leaving Vertex credentials intact; update testConfiguredAuthenticationModes() / the block around testConfiguredAuthenticationModes() and the subsequent restore logic to explicitly restore Vertex auth when VERTEX_AI_GCP_PROJECT_ID is set and only clear GEMINI_API_KEY, ensuring runTestAcrossModels() sees valid Vertex credentials.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/testFunctions.gs`:
- Around line 124-125: The test currently treats undefined/null response as
success because String(response).length > 0; update the logic after chat.run({
model: GEMINI_MODEL, max_tokens: 64 }) to validate that the returned response is
a non-empty string (e.g., typeof response === 'string' && response.trim().length
> 0) and throw an Error (or reject) if not before calling console.log; apply the
same check for the other auth helper usage (the similar chat.run call later) so
empty model outputs are considered test failures rather than successes.
- Around line 145-146: The reset call GenAIApp.resetGeminiAuthState() is
clearing all Gemini auth before the shared Gemini matrix in
runTestAcrossModels(), causing Vertex-only setups (VERTEX_AI_GCP_PROJECT_ID
present, GEMINI_API_KEY absent) to run unauthenticated; modify the sequence so
Vertex auth is preserved or restored before the shared matrix: either remove or
move GenAIApp.resetGemminiAuthState() until after tests that rely on Vertex, or
change the reset to only clear the API-key path while leaving Vertex credentials
intact; update testConfiguredAuthenticationModes() / the block around
testConfiguredAuthenticationModes() and the subsequent restore logic to
explicitly restore Vertex auth when VERTEX_AI_GCP_PROJECT_ID is set and only
clear GEMINI_API_KEY, ensuring runTestAcrossModels() sees valid Vertex
credentials.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0ddd5b79-5ec7-41c1-a333-bc673b211ad3
📒 Files selected for processing (1)
src/testFunctions.gs
📜 Review details
🔇 Additional comments (1)
src/testFunctions.gs (1)
3-11: LGTM!Also applies to: 53-71, 77-123, 128-138, 153-153, 193-206
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/testFunctions.gs (1)
283-283:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent API key handling:
OPEN_AI_API_KEYaccessed as global instead of viagetAuthTestConfigValue.
testInputTokenWarning,testCodeInterpreterExcel, andtestCodeInterpreterPDFaccessOPEN_AI_API_KEYdirectly as a global constant, whilerunTestAcrossModelsusesgetAuthTestConfigValue("OPEN_AI_API_KEY", ""). This inconsistency could causeReferenceErrorif the global is not defined.🛠️ Suggested fix for consistency
function testInputTokenWarning() { - GenAIApp.setOpenAIAPIKey(OPEN_AI_API_KEY); + const openAiKey = getAuthTestConfigValue("OPEN_AI_API_KEY", ""); + if (!openAiKey) { + console.log("[GenAIApp tests] Skipping testInputTokenWarning: OPEN_AI_API_KEY not configured."); + return; + } + GenAIApp.setOpenAIAPIKey(openAiKey); ... function testCodeInterpreterExcel(driveFileId) { - GenAIApp.setOpenAIAPIKey(OPEN_AI_API_KEY); + GenAIApp.setOpenAIAPIKey(getAuthTestConfigValue("OPEN_AI_API_KEY", "")); ... function testCodeInterpreterPDF(driveFileId) { - GenAIApp.setOpenAIAPIKey(OPEN_AI_API_KEY); + GenAIApp.setOpenAIAPIKey(getAuthTestConfigValue("OPEN_AI_API_KEY", ""));Also applies to: 307-307, 320-320
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/testFunctions.gs` at line 283, Replace direct global accesses of OPEN_AI_API_KEY in testInputTokenWarning, testCodeInterpreterExcel, testCodeInterpreterPDF (and the GenAIApp.setOpenAIAPIKey call) with the same getter used in runTestAcrossModels: call getAuthTestConfigValue("OPEN_AI_API_KEY", "") to retrieve the key, then pass that value into GenAIApp.setOpenAIAPIKey and any other places using OPEN_AI_API_KEY; this ensures consistent handling and avoids ReferenceError when the global constant is undefined.src/code.gs (3)
318-333:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical syntax error:
enableCodeInterpreteris missing its closing brace and return statement.The
enableCodeInterpretermethod at line 318 opens a function body but never closes it. TheenableToolCombinationmethod at line 330 appears to be defined insideenableCodeInterpreter, which will cause a syntax error at runtime. The method also lacks areturn this;statement for fluent chaining.🐛 Proposed fix
this.enableCodeInterpreter = function (containerId) { this._codeInterpreterEnabled = true; if (containerId) { this._codeInterpreterContainerId = containerId; } - - /** OPTIONAL + return this; + }; + + /** OPTIONAL * * Enable or disable server-side tool invocations for Gemini (Tool Combination). * `@param` {boolean} enabled - True to enable tool combination. * `@returns` {Chat} - The current Chat instance. */ this.enableToolCombination = function (enabled) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 318 - 333, Close the open function body for enableCodeInterpreter: add the missing closing brace and a "return this;" so enableCodeInterpreter sets _codeInterpreterEnabled, optionally _codeInterpreterContainerId, and returns the Chat instance; then move the enableToolCombination definition out of enableCodeInterpreter (ensure enableToolCombination is defined at the top level on the same prototype/object, sets tool_combination_enabled, and also returns this) so both functions are separate, syntactically correct, and support fluent chaining.
1873-1874:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMultiple potential null reference errors when
payloadis null.Similar to the network error case, several log statements and the final error message reference
payload.model. Whenpayloadis null (GET requests), these will throw. Apply defensive access throughout.🐛 Proposed fix for all occurrences
- console.error(`[GenAIApp] - Request to ${payload.model} failed with response code ${responseCode} - ${response.getContentText()}`); + console.error(`[GenAIApp] - Request to ${payload?.model ?? endpoint} failed with response code ${responseCode} - ${response.getContentText()}`); ... - console.warn(`[GenAIApp] - Rate limit reached when calling ${payload.model}, retrying (${retries}/${maxRetries}).`); + console.warn(`[GenAIApp] - Rate limit reached when calling ${payload?.model ?? endpoint}, retrying (${retries}/${maxRetries}).`); ... - console.warn(`[GenAIApp] - Request to ${payload.model} failed with response code ${responseCode} - ${response.getContentText()}, retrying (${retries}/${maxRetries})`); + console.warn(`[GenAIApp] - Request to ${payload?.model ?? endpoint} failed with response code ${responseCode} - ${response.getContentText()}, retrying (${retries}/${maxRetries})`); ... - console.error(`[GenAIApp] - Request to ${payload.model} failed with response code ${responseCode} - ${response.getContentText()}`); + console.error(`[GenAIApp] - Request to ${payload?.model ?? endpoint} failed with response code ${responseCode} - ${response.getContentText()}`); ... - throw new Error(`[GenAIApp] - Failed to call ${payload.model} after ${retries} retries.`); + throw new Error(`[GenAIApp] - Failed to call ${payload?.model ?? endpoint} after ${retries} retries.`);Also applies to: 1881-1882, 1890-1891, 1895-1896, 1901-1902
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1873 - 1874, Log statements reference payload.model without null checks, causing crashes when payload is null (e.g., on GET requests); update all occurrences (the console.error at the shown diff and the lines noted at 1881-1882, 1890-1891, 1895-1896, 1901-1902) to defensively access model (use a safe check or default like payload?.model or (payload && payload.model) or fallback string) when composing messages that include responseCode or response.getContentText(); ensure every place that currently interpolates payload.model handles a null/undefined payload and keeps the same contextual message text.
1830-1836:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPotential null reference when
payloadis null during retry logging.Line 1830 logs
payload.modelon network error, but_callGenAIApinow acceptspayload = nullfor GET requests (line 1815-1817). Whenpayloadis null and a network error occurs, this will throwCannot read property 'model' of null.🐛 Proposed fix
catch (err) { if (verbose) { - console.warn(`[GenAIApp] - Network error calling ${payload.model}: ${err.message}. Retrying (${retries + 1}/${maxRetries})`); + console.warn(`[GenAIApp] - Network error calling ${payload?.model ?? endpoint}: ${err.message}. Retrying (${retries + 1}/${maxRetries})`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1830 - 1836, The retry logging uses payload.model but _callGenAIApi allows payload to be null for GET requests, which can cause a null reference; update the retry log in the catch block (the console.warn that references payload.model) to safely handle a null payload by using a fallback or optional chaining (e.g., derive modelName = payload?.model ?? '<no-payload>' or include the request method/URL) so the log never attempts to read .model from null and still provides useful context during retries.
♻️ Duplicate comments (1)
src/code.gs (1)
1709-1716:⚠️ Potential issue | 🟠 MajorPayload key casing mismatch:
tool_configshould be converted totoolConfigfor Vertex AI.This helper deletes
includeServerSideToolInvocationsbut preserves the snake_casetool_configkey. The Vertex AI Advanced Service expects camelCasetoolConfig.functionCallingConfig. This was flagged in a prior review and appears unaddressed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1709 - 1716, The _buildVertexAiAdvancedServicePayload helper currently removes includeServerSideToolInvocations but leaves the snake_case tool_config key; update this function to rename tool_config to camelCase toolConfig and move its inner properties accordingly (e.g., map or remove includeServerSideToolInvocations and ensure any functionCallingConfig exists under toolConfig if required by Vertex AI). Specifically: deep-clone payload as before, if payload.tool_config exists create payload.toolConfig = payload.tool_config with includeServerSideToolInvocations removed (or translated into functionCallingConfig structure if needed), then delete the original tool_config and return the modified advancedServicePayload.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/testFunctions.gs`:
- Around line 153-162: The two global constants
TEST_CODE_INTERPRETER_XLSX_DRIVE_FILE_ID and
TEST_CODE_INTERPRETER_PDF_DRIVE_FILE_ID are undefined; update the code to
retrieve these values via getAuthTestConfigValue (same pattern as other config
values) before using them in the conditional calls to testCodeInterpreterExcel
and testCodeInterpreterPDF. Specifically, add local variables (e.g., const
xlsxId = getAuthTestConfigValue('TEST_CODE_INTERPRETER_XLSX_DRIVE_FILE_ID') and
const pdfId = getAuthTestConfigValue('TEST_CODE_INTERPRETER_PDF_DRIVE_FILE_ID'))
and then change the if checks to use xlsxId and pdfId so ReferenceError is
avoided and behavior matches other tests.
---
Outside diff comments:
In `@src/code.gs`:
- Around line 318-333: Close the open function body for enableCodeInterpreter:
add the missing closing brace and a "return this;" so enableCodeInterpreter sets
_codeInterpreterEnabled, optionally _codeInterpreterContainerId, and returns the
Chat instance; then move the enableToolCombination definition out of
enableCodeInterpreter (ensure enableToolCombination is defined at the top level
on the same prototype/object, sets tool_combination_enabled, and also returns
this) so both functions are separate, syntactically correct, and support fluent
chaining.
- Around line 1873-1874: Log statements reference payload.model without null
checks, causing crashes when payload is null (e.g., on GET requests); update all
occurrences (the console.error at the shown diff and the lines noted at
1881-1882, 1890-1891, 1895-1896, 1901-1902) to defensively access model (use a
safe check or default like payload?.model or (payload && payload.model) or
fallback string) when composing messages that include responseCode or
response.getContentText(); ensure every place that currently interpolates
payload.model handles a null/undefined payload and keeps the same contextual
message text.
- Around line 1830-1836: The retry logging uses payload.model but _callGenAIApi
allows payload to be null for GET requests, which can cause a null reference;
update the retry log in the catch block (the console.warn that references
payload.model) to safely handle a null payload by using a fallback or optional
chaining (e.g., derive modelName = payload?.model ?? '<no-payload>' or include
the request method/URL) so the log never attempts to read .model from null and
still provides useful context during retries.
In `@src/testFunctions.gs`:
- Line 283: Replace direct global accesses of OPEN_AI_API_KEY in
testInputTokenWarning, testCodeInterpreterExcel, testCodeInterpreterPDF (and the
GenAIApp.setOpenAIAPIKey call) with the same getter used in runTestAcrossModels:
call getAuthTestConfigValue("OPEN_AI_API_KEY", "") to retrieve the key, then
pass that value into GenAIApp.setOpenAIAPIKey and any other places using
OPEN_AI_API_KEY; this ensures consistent handling and avoids ReferenceError when
the global constant is undefined.
---
Duplicate comments:
In `@src/code.gs`:
- Around line 1709-1716: The _buildVertexAiAdvancedServicePayload helper
currently removes includeServerSideToolInvocations but leaves the snake_case
tool_config key; update this function to rename tool_config to camelCase
toolConfig and move its inner properties accordingly (e.g., map or remove
includeServerSideToolInvocations and ensure any functionCallingConfig exists
under toolConfig if required by Vertex AI). Specifically: deep-clone payload as
before, if payload.tool_config exists create payload.toolConfig =
payload.tool_config with includeServerSideToolInvocations removed (or translated
into functionCallingConfig structure if needed), then delete the original
tool_config and return the modified advancedServicePayload.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cf61b525-8acb-4928-9e24-aae5f475c132
📒 Files selected for processing (2)
src/code.gssrc/testFunctions.gs
📜 Review details
🔇 Additional comments (7)
src/code.gs (4)
1780-1817: LGTM!
55-59: LGTM!Also applies to: 465-467, 556-566, 657-659, 676-678, 791-807, 811-913
1566-1605: LGTM!Also applies to: 1614-1635, 1646-1670, 1681-1700, 1728-1754, 1762-1767, 2846-2856
543-548: Confirm Vertex AI routing matches PR intent.In
src/code.gs,_buildGeminiPayloadsetstool_config.includeServerSideToolInvocationstotool_combination_enabled(initialized tofalse), and the routing condition checks!payload?.tool_config?.includeServerSideToolInvocations—so it calls_callVertexAiwhen server-side tool invocation metadata is not requested (default) and falls back to_callGenAIApiwhen it is requested. This aligns with “VertexAI does not support server-side tool invocation metadata in the response”. Additionally,_buildVertexAiAdvancedServicePayloadremovesincludeServerSideToolInvocationsfrom the Vertex Advanced Service request payload.src/testFunctions.gs (3)
5-71: LGTM!Also applies to: 77-141, 144-150, 167-177, 196-214
306-330: LGTM!
3-3: Confirmgemini-3.5-flashmodel availabilityconst GEMINI_MODEL = "gemini-3.5-flash";
gemini-3.5-flashis a valid Vertex AI/Gemini model ID and is generally available (GA). Ensure you use a supported region/endpoint for your Vertex AI project/deployment.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/testFunctions.gs (1)
283-283:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent API key handling:
OPEN_AI_API_KEYaccessed as global instead of viagetAuthTestConfigValue.
testInputTokenWarning,testCodeInterpreterExcel, andtestCodeInterpreterPDFaccessOPEN_AI_API_KEYdirectly as a global constant, whilerunTestAcrossModelsusesgetAuthTestConfigValue("OPEN_AI_API_KEY", ""). This inconsistency could causeReferenceErrorif the global is not defined.🛠️ Suggested fix for consistency
function testInputTokenWarning() { - GenAIApp.setOpenAIAPIKey(OPEN_AI_API_KEY); + const openAiKey = getAuthTestConfigValue("OPEN_AI_API_KEY", ""); + if (!openAiKey) { + console.log("[GenAIApp tests] Skipping testInputTokenWarning: OPEN_AI_API_KEY not configured."); + return; + } + GenAIApp.setOpenAIAPIKey(openAiKey); ... function testCodeInterpreterExcel(driveFileId) { - GenAIApp.setOpenAIAPIKey(OPEN_AI_API_KEY); + GenAIApp.setOpenAIAPIKey(getAuthTestConfigValue("OPEN_AI_API_KEY", "")); ... function testCodeInterpreterPDF(driveFileId) { - GenAIApp.setOpenAIAPIKey(OPEN_AI_API_KEY); + GenAIApp.setOpenAIAPIKey(getAuthTestConfigValue("OPEN_AI_API_KEY", ""));Also applies to: 307-307, 320-320
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/testFunctions.gs` at line 283, Replace direct global accesses of OPEN_AI_API_KEY in testInputTokenWarning, testCodeInterpreterExcel, testCodeInterpreterPDF (and the GenAIApp.setOpenAIAPIKey call) with the same getter used in runTestAcrossModels: call getAuthTestConfigValue("OPEN_AI_API_KEY", "") to retrieve the key, then pass that value into GenAIApp.setOpenAIAPIKey and any other places using OPEN_AI_API_KEY; this ensures consistent handling and avoids ReferenceError when the global constant is undefined.src/code.gs (3)
318-333:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical syntax error:
enableCodeInterpreteris missing its closing brace and return statement.The
enableCodeInterpretermethod at line 318 opens a function body but never closes it. TheenableToolCombinationmethod at line 330 appears to be defined insideenableCodeInterpreter, which will cause a syntax error at runtime. The method also lacks areturn this;statement for fluent chaining.🐛 Proposed fix
this.enableCodeInterpreter = function (containerId) { this._codeInterpreterEnabled = true; if (containerId) { this._codeInterpreterContainerId = containerId; } - - /** OPTIONAL + return this; + }; + + /** OPTIONAL * * Enable or disable server-side tool invocations for Gemini (Tool Combination). * `@param` {boolean} enabled - True to enable tool combination. * `@returns` {Chat} - The current Chat instance. */ this.enableToolCombination = function (enabled) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 318 - 333, Close the open function body for enableCodeInterpreter: add the missing closing brace and a "return this;" so enableCodeInterpreter sets _codeInterpreterEnabled, optionally _codeInterpreterContainerId, and returns the Chat instance; then move the enableToolCombination definition out of enableCodeInterpreter (ensure enableToolCombination is defined at the top level on the same prototype/object, sets tool_combination_enabled, and also returns this) so both functions are separate, syntactically correct, and support fluent chaining.
1873-1874:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMultiple potential null reference errors when
payloadis null.Similar to the network error case, several log statements and the final error message reference
payload.model. Whenpayloadis null (GET requests), these will throw. Apply defensive access throughout.🐛 Proposed fix for all occurrences
- console.error(`[GenAIApp] - Request to ${payload.model} failed with response code ${responseCode} - ${response.getContentText()}`); + console.error(`[GenAIApp] - Request to ${payload?.model ?? endpoint} failed with response code ${responseCode} - ${response.getContentText()}`); ... - console.warn(`[GenAIApp] - Rate limit reached when calling ${payload.model}, retrying (${retries}/${maxRetries}).`); + console.warn(`[GenAIApp] - Rate limit reached when calling ${payload?.model ?? endpoint}, retrying (${retries}/${maxRetries}).`); ... - console.warn(`[GenAIApp] - Request to ${payload.model} failed with response code ${responseCode} - ${response.getContentText()}, retrying (${retries}/${maxRetries})`); + console.warn(`[GenAIApp] - Request to ${payload?.model ?? endpoint} failed with response code ${responseCode} - ${response.getContentText()}, retrying (${retries}/${maxRetries})`); ... - console.error(`[GenAIApp] - Request to ${payload.model} failed with response code ${responseCode} - ${response.getContentText()}`); + console.error(`[GenAIApp] - Request to ${payload?.model ?? endpoint} failed with response code ${responseCode} - ${response.getContentText()}`); ... - throw new Error(`[GenAIApp] - Failed to call ${payload.model} after ${retries} retries.`); + throw new Error(`[GenAIApp] - Failed to call ${payload?.model ?? endpoint} after ${retries} retries.`);Also applies to: 1881-1882, 1890-1891, 1895-1896, 1901-1902
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1873 - 1874, Log statements reference payload.model without null checks, causing crashes when payload is null (e.g., on GET requests); update all occurrences (the console.error at the shown diff and the lines noted at 1881-1882, 1890-1891, 1895-1896, 1901-1902) to defensively access model (use a safe check or default like payload?.model or (payload && payload.model) or fallback string) when composing messages that include responseCode or response.getContentText(); ensure every place that currently interpolates payload.model handles a null/undefined payload and keeps the same contextual message text.
1830-1836:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPotential null reference when
payloadis null during retry logging.Line 1830 logs
payload.modelon network error, but_callGenAIApinow acceptspayload = nullfor GET requests (line 1815-1817). Whenpayloadis null and a network error occurs, this will throwCannot read property 'model' of null.🐛 Proposed fix
catch (err) { if (verbose) { - console.warn(`[GenAIApp] - Network error calling ${payload.model}: ${err.message}. Retrying (${retries + 1}/${maxRetries})`); + console.warn(`[GenAIApp] - Network error calling ${payload?.model ?? endpoint}: ${err.message}. Retrying (${retries + 1}/${maxRetries})`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1830 - 1836, The retry logging uses payload.model but _callGenAIApi allows payload to be null for GET requests, which can cause a null reference; update the retry log in the catch block (the console.warn that references payload.model) to safely handle a null payload by using a fallback or optional chaining (e.g., derive modelName = payload?.model ?? '<no-payload>' or include the request method/URL) so the log never attempts to read .model from null and still provides useful context during retries.
♻️ Duplicate comments (1)
src/code.gs (1)
1709-1716:⚠️ Potential issue | 🟠 MajorPayload key casing mismatch:
tool_configshould be converted totoolConfigfor Vertex AI.This helper deletes
includeServerSideToolInvocationsbut preserves the snake_casetool_configkey. The Vertex AI Advanced Service expects camelCasetoolConfig.functionCallingConfig. This was flagged in a prior review and appears unaddressed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1709 - 1716, The _buildVertexAiAdvancedServicePayload helper currently removes includeServerSideToolInvocations but leaves the snake_case tool_config key; update this function to rename tool_config to camelCase toolConfig and move its inner properties accordingly (e.g., map or remove includeServerSideToolInvocations and ensure any functionCallingConfig exists under toolConfig if required by Vertex AI). Specifically: deep-clone payload as before, if payload.tool_config exists create payload.toolConfig = payload.tool_config with includeServerSideToolInvocations removed (or translated into functionCallingConfig structure if needed), then delete the original tool_config and return the modified advancedServicePayload.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/testFunctions.gs`:
- Around line 153-162: The two global constants
TEST_CODE_INTERPRETER_XLSX_DRIVE_FILE_ID and
TEST_CODE_INTERPRETER_PDF_DRIVE_FILE_ID are undefined; update the code to
retrieve these values via getAuthTestConfigValue (same pattern as other config
values) before using them in the conditional calls to testCodeInterpreterExcel
and testCodeInterpreterPDF. Specifically, add local variables (e.g., const
xlsxId = getAuthTestConfigValue('TEST_CODE_INTERPRETER_XLSX_DRIVE_FILE_ID') and
const pdfId = getAuthTestConfigValue('TEST_CODE_INTERPRETER_PDF_DRIVE_FILE_ID'))
and then change the if checks to use xlsxId and pdfId so ReferenceError is
avoided and behavior matches other tests.
---
Outside diff comments:
In `@src/code.gs`:
- Around line 318-333: Close the open function body for enableCodeInterpreter:
add the missing closing brace and a "return this;" so enableCodeInterpreter sets
_codeInterpreterEnabled, optionally _codeInterpreterContainerId, and returns the
Chat instance; then move the enableToolCombination definition out of
enableCodeInterpreter (ensure enableToolCombination is defined at the top level
on the same prototype/object, sets tool_combination_enabled, and also returns
this) so both functions are separate, syntactically correct, and support fluent
chaining.
- Around line 1873-1874: Log statements reference payload.model without null
checks, causing crashes when payload is null (e.g., on GET requests); update all
occurrences (the console.error at the shown diff and the lines noted at
1881-1882, 1890-1891, 1895-1896, 1901-1902) to defensively access model (use a
safe check or default like payload?.model or (payload && payload.model) or
fallback string) when composing messages that include responseCode or
response.getContentText(); ensure every place that currently interpolates
payload.model handles a null/undefined payload and keeps the same contextual
message text.
- Around line 1830-1836: The retry logging uses payload.model but _callGenAIApi
allows payload to be null for GET requests, which can cause a null reference;
update the retry log in the catch block (the console.warn that references
payload.model) to safely handle a null payload by using a fallback or optional
chaining (e.g., derive modelName = payload?.model ?? '<no-payload>' or include
the request method/URL) so the log never attempts to read .model from null and
still provides useful context during retries.
In `@src/testFunctions.gs`:
- Line 283: Replace direct global accesses of OPEN_AI_API_KEY in
testInputTokenWarning, testCodeInterpreterExcel, testCodeInterpreterPDF (and the
GenAIApp.setOpenAIAPIKey call) with the same getter used in runTestAcrossModels:
call getAuthTestConfigValue("OPEN_AI_API_KEY", "") to retrieve the key, then
pass that value into GenAIApp.setOpenAIAPIKey and any other places using
OPEN_AI_API_KEY; this ensures consistent handling and avoids ReferenceError when
the global constant is undefined.
---
Duplicate comments:
In `@src/code.gs`:
- Around line 1709-1716: The _buildVertexAiAdvancedServicePayload helper
currently removes includeServerSideToolInvocations but leaves the snake_case
tool_config key; update this function to rename tool_config to camelCase
toolConfig and move its inner properties accordingly (e.g., map or remove
includeServerSideToolInvocations and ensure any functionCallingConfig exists
under toolConfig if required by Vertex AI). Specifically: deep-clone payload as
before, if payload.tool_config exists create payload.toolConfig =
payload.tool_config with includeServerSideToolInvocations removed (or translated
into functionCallingConfig structure if needed), then delete the original
tool_config and return the modified advancedServicePayload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cf61b525-8acb-4928-9e24-aae5f475c132
📒 Files selected for processing (2)
src/code.gssrc/testFunctions.gs
📜 Review details
🔇 Additional comments (7)
src/code.gs (4)
1780-1817: LGTM!
55-59: LGTM!Also applies to: 465-467, 556-566, 657-659, 676-678, 791-807, 811-913
1566-1605: LGTM!Also applies to: 1614-1635, 1646-1670, 1681-1700, 1728-1754, 1762-1767, 2846-2856
543-548: Confirm Vertex AI routing matches PR intent.In
src/code.gs,_buildGeminiPayloadsetstool_config.includeServerSideToolInvocationstotool_combination_enabled(initialized tofalse), and the routing condition checks!payload?.tool_config?.includeServerSideToolInvocations—so it calls_callVertexAiwhen server-side tool invocation metadata is not requested (default) and falls back to_callGenAIApiwhen it is requested. This aligns with “VertexAI does not support server-side tool invocation metadata in the response”. Additionally,_buildVertexAiAdvancedServicePayloadremovesincludeServerSideToolInvocationsfrom the Vertex Advanced Service request payload.src/testFunctions.gs (3)
5-71: LGTM!Also applies to: 77-141, 144-150, 167-177, 196-214
306-330: LGTM!
3-3: Confirmgemini-3.5-flashmodel availabilityconst GEMINI_MODEL = "gemini-3.5-flash";
gemini-3.5-flashis a valid Vertex AI/Gemini model ID and is generally available (GA). Ensure you use a supported region/endpoint for your Vertex AI project/deployment.
🛑 Comments failed to post (1)
src/testFunctions.gs (1)
153-162:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winUndefined global constants
TEST_CODE_INTERPRETER_XLSX_DRIVE_FILE_IDandTEST_CODE_INTERPRETER_PDF_DRIVE_FILE_ID.These constants are referenced but never declared in the provided file. Unlike other config values that use
getAuthTestConfigValue, these are accessed directly as globals, which will throwReferenceErrorif not defined elsewhere.🐛 Proposed fix: use getAuthTestConfigValue for consistency
// OpenAI-only tests - require valid Drive file IDs. - if (TEST_CODE_INTERPRETER_XLSX_DRIVE_FILE_ID) { - testCodeInterpreterExcel(TEST_CODE_INTERPRETER_XLSX_DRIVE_FILE_ID); + const xlsxFileId = getAuthTestConfigValue("TEST_CODE_INTERPRETER_XLSX_DRIVE_FILE_ID", ""); + if (xlsxFileId) { + testCodeInterpreterExcel(xlsxFileId); } - if (TEST_CODE_INTERPRETER_PDF_DRIVE_FILE_ID) { - testCodeInterpreterPDF(TEST_CODE_INTERPRETER_PDF_DRIVE_FILE_ID); + const pdfFileId = getAuthTestConfigValue("TEST_CODE_INTERPRETER_PDF_DRIVE_FILE_ID", ""); + if (pdfFileId) { + testCodeInterpreterPDF(pdfFileId); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/testFunctions.gs` around lines 153 - 162, The two global constants TEST_CODE_INTERPRETER_XLSX_DRIVE_FILE_ID and TEST_CODE_INTERPRETER_PDF_DRIVE_FILE_ID are undefined; update the code to retrieve these values via getAuthTestConfigValue (same pattern as other config values) before using them in the conditional calls to testCodeInterpreterExcel and testCodeInterpreterPDF. Specifically, add local variables (e.g., const xlsxId = getAuthTestConfigValue('TEST_CODE_INTERPRETER_XLSX_DRIVE_FILE_ID') and const pdfId = getAuthTestConfigValue('TEST_CODE_INTERPRETER_PDF_DRIVE_FILE_ID')) and then change the if checks to use xlsxId and pdfId so ReferenceError is avoided and behavior matches other tests.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/code.gs (1)
1711-1718:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
tool_configsnake_case keys are not translated to Vertex AI's expected camelCase.
_buildGeminiPayload()createstool_config.function_calling_config.modeandtool_config.function_calling_config.allowed_function_names(snake_case), but the Vertex AI Advanced Service expectstoolConfig.functionCallingConfig.modeandtoolConfig.functionCallingConfig.allowedFunctionNames(camelCase). This payload builder only removes the internal flag without converting the key casing, so function-calling constraints may be silently ignored when using the Advanced Service path.🔧 Suggested fix to translate keys
function _buildVertexAiAdvancedServicePayload(payload) { const advancedServicePayload = JSON.parse(JSON.stringify(payload || {})); delete advancedServicePayload.model; if (advancedServicePayload.tool_config) { - delete advancedServicePayload.tool_config.includeServerSideToolInvocations; + const tc = advancedServicePayload.tool_config; + delete tc.includeServerSideToolInvocations; + + // Translate snake_case to camelCase for Vertex AI + if (tc.function_calling_config) { + const fcc = tc.function_calling_config; + const functionCallingConfig = { + mode: fcc.mode + }; + if (fcc.allowed_function_names !== undefined) { + functionCallingConfig.allowedFunctionNames = fcc.allowed_function_names; + } + advancedServicePayload.toolConfig = { functionCallingConfig }; + } + delete advancedServicePayload.tool_config; } return advancedServicePayload; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/code.gs` around lines 1711 - 1718, _buildVertexAiAdvancedServicePayload currently deep-clones the payload and strips internal flags but doesn't convert snake_case tool_config keys to Vertex AI's camelCase, so toolConfig.functionCallingConfig.* fields (mode, allowedFunctionNames) are ignored; update _buildVertexAiAdvancedServicePayload to detect payload.tool_config (or nested tool_config.function_calling_config) and map those keys to camelCase equivalents (toolConfig, functionCallingConfig, allowedFunctionNames, mode) before removing includeServerSideToolInvocations, ensuring existing values are preserved and copied into the new camelCase properties so the Advanced Service receives the correct field names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/testFunctions.gs`:
- Around line 120-134: Remove the unused OpenAI dependency from
testApiKeyAuthentication: stop calling requireAuthTestCredential with
AUTH_TEST_CONFIG_KEYS.OPEN_AI_API_KEY and remove the
GenAIApp.setOpenAIAPIKey(...) call so the test only obtains and sets the Gemini
key; keep requireAuthTestCredential(AUTH_TEST_CONFIG_KEYS.GEMINI_API_KEY, "API
key authentication"), GenAIApp.setGeminiAPIKey(geminiApiKey), and the rest of
the test (GenAIApp.setGeminiAuth, newChat(), chat.run(...)) unchanged.
---
Duplicate comments:
In `@src/code.gs`:
- Around line 1711-1718: _buildVertexAiAdvancedServicePayload currently
deep-clones the payload and strips internal flags but doesn't convert snake_case
tool_config keys to Vertex AI's camelCase, so toolConfig.functionCallingConfig.*
fields (mode, allowedFunctionNames) are ignored; update
_buildVertexAiAdvancedServicePayload to detect payload.tool_config (or nested
tool_config.function_calling_config) and map those keys to camelCase equivalents
(toolConfig, functionCallingConfig, allowedFunctionNames, mode) before removing
includeServerSideToolInvocations, ensuring existing values are preserved and
copied into the new camelCase properties so the Advanced Service receives the
correct field names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5ad3bd63-9761-4217-aa3e-f0685e3b0e46
📒 Files selected for processing (2)
src/code.gssrc/testFunctions.gs
📜 Review details
🔇 Additional comments (28)
src/code.gs (17)
55-59: LGTM!
305-324: LGTM!
545-550: LGTM!
558-568: LGTM!
659-662: LGTM!Also applies to: 678-681
792-809: LGTM!
813-916: LGTM!
1568-1576: LGTM!
1587-1595: LGTM!
1616-1637: LGTM!
1648-1672: LGTM!
1683-1702: LGTM!
1730-1756: LGTM!
1764-1769: LGTM!
1782-1819: LGTM!
1842-1845: LGTM!
2848-2857: LGTM!src/testFunctions.gs (11)
3-5: LGTM!
7-14: LGTM!
20-57: LGTM!
59-73: LGTM!
75-88: LGTM!
96-118: LGTM!
136-149: LGTM!
152-171: LGTM!
175-200: LGTM!
203-222: LGTM!
314-338: LGTM!
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
Motivation
UrlFetchAppcalls.UrlFetchApppath as a transparent fallback so user-visible behavior, response shape, and errors remain unchanged.Description
callVertexAi(endpoint, payload)that triescallVertexAiWithAdvancedService(...)first and falls back tocallVertexAiWithUrlFetchFallback(...)which delegates to the existing_callGenAIApi(...)implementation._getVertexAiAdvancedService(),_getVertexAiGenerateContentMethod(),_getVertexAiModelResource(),_buildVertexAiAdvancedServicePayload(),_normalizeVertexAiResponse(), and_logVertexAiAdvancedServiceFallback()to detect, invoke, and normalize the Advanced Service response to the preexisting UrlFetchApp response shape.setGeminiAuth(...)(i.e., GCP-authenticated requests wheremodel.includes('gemini') && !geminiKey) callcallVertexAi(...), while Gemini API-key calls and all otherUrlFetchAppcalls remain unchanged.README.mdto note that the library prefers the Apps Script Vertex AI Advanced Service when enabled, and to retain the required OAuth scopes for Vertex AI usage.Testing
tmp=$(mktemp /tmp/code-XXXXXX.js) && cp src/code.gs "$tmp" && node --check "$tmp", which succeeded for the modified file.node --check src/code.gs(without temp copy) is not applicable in this environment because Node does not recognize the.gsextension, so that check is informational only.Codex Task