Skip to content

fix(test/alloydbainl): use explicit SQL alias prompt in integration test - #3916

Merged
wangauone merged 2 commits into
mainfrom
fix/alloydbainl-integration-alias-prompt
Aug 31, 2026
Merged

wangauone merged 2 commits into
mainfrom
fix/alloydbainl-integration-alias-prompt

Conversation

@wangauone

Copy link
Copy Markdown
Contributor

#3877 made the AI NL test prompts explicit about the column alias, but only updated alloydb_ai_nl_mcp_test.go. alloydb_ai_nl_integration_test.go kept the vague "return the number 1" prompt while still expecting the number_one column name that #3339 introduced.

With no alias requested, the service generates plain SELECT 1 and Postgres falls back to its default ?column? label, so the assertion can never pass:

got  "[{"execute_nl_query":{"?column?":1}}]"
want "[{"execute_nl_query":{"number_one":1}}]"

This applies the same prompt wording #3877 used, to the seven remaining occurrences in the integration test file.

The alloydb-ai-nl build step currently fails on every PR for this reason. All three subtests in TestAlloyDBAINLToolEndpoints that actually execute a query fail; the rest only pass because they are auth or validation error cases that never reach the database. TestAlloyDBAINLCallTool, in the file #3877 did fix, passes.

@wangauone
wangauone requested review from a team as code owners August 31, 2026 21:28

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the integration tests in tests/alloydbainl/alloydb_ai_nl_integration_test.go to use a more explicit SQL-based question string in the request bodies and arguments. The review feedback correctly identifies a mismatch in one of the test cases where the API endpoint for Invoke my-auth-required-tool without auth token incorrectly points to my-auth-tool instead of my-auth-required-tool.

Comment thread tests/alloydbainl/alloydb_ai_nl_integration_test.go
@wangauone
wangauone enabled auto-merge (squash) August 31, 2026 21:36
@Yuan325 Yuan325 added the release candidate Use label to signal PR should be included in the next release. label Aug 31, 2026
@wangauone
wangauone merged commit 596eaf9 into main Aug 31, 2026
26 checks passed
@wangauone
wangauone deleted the fix/alloydbainl-integration-alias-prompt branch August 31, 2026 22:38
@github-actions

Copy link
Copy Markdown
Contributor

🧨 Preview deployments removed.

Cloudflare Pages environments for pr-3916 have been deleted.

AlexTalreja pushed a commit that referenced this pull request Sep 10, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.11.0](v1.10.0...v1.11.0)
(2026-09-10)


### Features

* Add Toolbox version check on startup
([#3837](#3837))
([7d36de3](7d36de3))
* **alloydb:** Provide actionable error when read-only mode is used on
pre-PG17
([#3902](#3902))
([28ace11](28ace11))
* **MCP Apps:** Add support for MCP Apps
([#4008](#4008))
([9cf3e95](9cf3e95))
* **MCPResources:** Add support for MCP Resources
([#3968](#3968))
([fb227b0](fb227b0))
* **mcp:** Serve groups/list and groups/get as a Toolbox extension
([#3914](#3914))
([eaf2a9c](eaf2a9c))
* **source/bigquery:** Attach SQLCommenter attributes as BigQuery job
labels ([#3843](#3843))
([bf0f1a5](bf0f1a5))
* **sources:** Add ConnectOnce, a helper for connecting on first use
([#3905](#3905))
([16c31fa](16c31fa))


### Bug Fixes

* **docs/cloudgda:** Document context fields, fix PSV example and links
([#3919](#3919))
([ae47535](ae47535))
* **looker:** Update want clause
([#3962](#3962))
([9593321](9593321))
* **source/http:** Block IETF protocol assignments range in default SSRF
guard ([#3909](#3909))
([4302e86](4302e86))
* **sources:** Release the handle when a source fails to connect
([#3921](#3921))
([0001190](0001190))
* **test/alloydbainl:** Use explicit SQL alias prompt in integration
test ([#3916](#3916))
([596eaf9](596eaf9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
social4hyq pushed a commit to social4hyq/homebrew-core that referenced this pull request Sep 20, 2026
mcp-toolbox 1.11.0

Created-by: HarmonybrewBot
Commit-by: HarmonybrewBot
Merged-by: HarmonybrewBot
Description: Created by `brew bump`

---

Created with `brew bump-formula-pr`.<details>
  <summary>release notes</summary>
  <pre>## [1.11.0](googleapis/mcp-toolbox@v1.10.0...v1.11.0) (2026-09-10)


### Features

* Add Toolbox version check on startup ([#3837](googleapis/mcp-toolbox#3837)) ([7d36de3](googleapis/mcp-toolbox@7d36de3))
* **alloydb:** Provide actionable error when read-only mode is used on pre-PG17 ([#3902](googleapis/mcp-toolbox#3902)) ([28ace11](googleapis/mcp-toolbox@28ace11))
* **MCP Apps:** Add support for MCP Apps ([#4008](googleapis/mcp-toolbox#4008)) ([9cf3e95](googleapis/mcp-toolbox@9cf3e95))
* **MCPResources:** Add support for MCP Resources ([#3968](googleapis/mcp-toolbox#3968)) ([fb227b0](googleapis/mcp-toolbox@fb227b0))
* **mcp:** Serve groups/list and groups/get as a Toolbox extension ([#3914](googleapis/mcp-toolbox#3914)) ([eaf2a9c](googleapis/mcp-toolbox@eaf2a9c))
* **source/bigquery:** Attach SQLCommenter attributes as BigQuery job labels ([#3843](googleapis/mcp-toolbox#3843)) ([bf0f1a5](googleapis/mcp-toolbox@bf0f1a5))
* **sources:** Add ConnectOnce, a helper for connecting on first use ([#3905](googleapis/mcp-toolbox#3905)) ([16c31fa](googleapis/mcp-toolbox@16c31fa))


### Bug Fixes

* **docs/cloudgda:** Document context fields, fix PSV example and links ([#3919](googleapis/mcp-toolbox#3919)) ([ae47535](googleapis/mcp-toolbox@ae47535))
* **looker:** Update want clause ([#3962](googleapis/mcp-toolbox#3962)) ([9593321](googleapis/mcp-toolbox@9593321))
* **source/http:** Block IETF protocol assignments range in default SSRF guard ([#3909](googleapis/mcp-toolbox#3909)) ([4302e86](googleapis/mcp-toolbox@4302e86))
* **sources:** Release the handle when a source fails to connect ([#3921](googleapis/mcp-toolbox#3921)) ([0001190](googleapis/mcp-toolbox@0001190))
* **test/alloydbainl:** Use explicit SQL alias prompt in integration test ([#3916](googleapis/mcp-toolbox#3916)) ([596eaf9](googleapis/mcp-toolbox@596eaf9))

| **OS/Architecture**                                                                                       | **Description**                                                                                                          | **SHA256 Hash**                                                     |
| --------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------- |
| [linux/amd64](https://storage.googleapis.com/mcp-toolbox-for-databases/v1.11.0/linux/amd64/toolbox) ([Signature](https://storage.googleapis.com/mcp-toolbox-for-databases/v1.11.0/linux/amd64/toolbox.asc)) | For **Linux** systems running on **Intel/AMD 64-bit processors**.                                                        | 4d86b7c75916e2de721b9dc9dfb681f9a29fe4a8a01779007b2f6b1f5819296c    |
| [darwin/arm64](https://storage.googleapis.com/mcp-toolbox-for-databases/v1.11.0/darwin/arm64/toolbox)     | For **macOS** systems running on **Apple Silicon** (M1, M2, M3, etc.) processors.                                        | 5e94db330b5ec77a916c4670ec1337d97abbb9865b13a705f81f87ca14592307    |
| [darwin/amd64](https://storage.googleapis.com/mcp-toolbox-for-databases/v1.11.0/darwin/amd64/toolbox)     | For **macOS** systems running on **Intel processors**.                                                                   | 4a05bc786302571823f8c10b0ed2ce4f219c4a05c9899e91a9743ca8ec932743    |
| [windows/amd64](https://storage.googleapis.com/mcp-toolbox-for-databases/v1.11.0/windows/amd64/toolbox.exe) | For **Windows** systems running on **Intel/AMD 64-bit processors**.                                                      | 3c8c46d8efad225636e341fcdb137557779ee1c1b0c362d1b4870901c2668ccf    |
| [windows/arm64](https://storage.googleapis.com/mcp-toolbox-for-databases/v1.11.0/windows/arm64/toolbox.exe) | For **Windows** systems running on **ARM 64-bit processors**.                                                            | feb2d0d55c003b30af7a9afa22f11721239c70cc21812a2de66c425de76a2153    |</pre>
  <p>View the full release notes at <a href="https://github.com/googleapis/mcp-toolbox/releases/tag/v1.11.0">https://github.com/googleapis/mcp-toolbox/releases/tag/v1.11.0</a>.</p>
</details>
<hr>

See merge request: Harmonybrew/homebrew-core!19994
wangauone added a commit that referenced this pull request Sep 24, 2026
The AI-NL tests asserted the full response string, which embeds the SQL
column alias chosen by the model. The model does not reliably honour the
requested alias and sometimes returns `result` instead of `number_one`,
failing the test even though the query returned the correct value.

#3877 and #3916 tried to stabilise this by rewriting the prompt. Nothing
in the tool contract binds the alias, so a prompt can only lower the
failure rate.

Match the affected responses with a regex that accepts any single column
set to 1. The row and column counts are still pinned, so the assertion
can still catch a genuine regression.
Yuan325 added a commit that referenced this pull request Sep 28, 2026
#4125)

## Description

These tests asserted the full response string, which includes the SQL
column alias chosen by the model. The model sometimes returns `result`
instead of `number_one`, so the test failed even though the query
returned the correct value.

#3877 and #3916 tried to fix this by rewriting the prompt. Nothing in
the tool contract binds the alias, so a prompt can only lower the
failure rate.

This matches the affected responses with a regex that accepts any single
column set to 1. Row and column counts are still pinned so the assertion
can still catch a real regression. Cases that assert error messages are
unchanged.

Integration tests need live AlloyDB credentials and were not run
locally. `go vet` and `gofmt` are clean. The patterns were checked
against the response shapes from both endpoints, including the exact
string in the failure report.

## PR Checklist

- [x] Make sure you reviewed

[CONTRIBUTING.md](https://github.com/googleapis/mcp-toolbox/blob/main/CONTRIBUTING.md)
- [x] Make sure to open an issue as a

[bug/issue](https://github.com/googleapis/mcp-toolbox/issues/new/choose)
  before writing your code! That way we can discuss the change, evaluate
  designs, and agree on the general idea
- [x] Ensure you have manually reviewed the entire diff before
requesting a
  review
- [x] Ensure the tests and linter pass
- [x] Code coverage does not decrease (if any source code was changed)
- [x] Appropriate docs were updated (if necessary)
- [ ] Make sure to add `!` if this involve a breaking change

🛠️ Fixes #3874

Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release candidate Use label to signal PR should be included in the next release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants