[SCAL-323294] changes for fetch_data tool by saharsh-ts · Pull Request #179 · thoughtspot/mcp-server

saharsh-ts · 2026-07-09T15:49:58Z

No description provided.

gemini-code-assist

Code Review

This pull request introduces two new tools, search_objects and fetch_data, to the MCP server for searching and retrieving data from ThoughtSpot objects (Answers and Liveboards). It includes new schemas, gRPC-backed API clients, terminology mapping for synonyms, and comprehensive unit tests. The reviewer identified a critical issue in src/servers/mcp-server.ts where the asynchronous validateConnection method is not awaited in the Ping tool case, causing the check to always evaluate to true.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-07-09T15:53:17Z

 			case ToolName.Ping: {
 				if (this.ctx.props.accessToken && this.ctx.props.instanceUrl) {
-						if (!this.getThoughtSpotService(recorder).validateConnection()) {
+					if (!this.getThoughtSpotService(recorder).validateConnection()) {


The validateConnection method is asynchronous and returns a Promise<boolean>. Since it is not awaited here, the condition !promise will always evaluate to false because any Promise object is truthy. This effectively bypasses the connection validation check. Please add await to correctly evaluate the promise result.

Suggested change

if (!this.getThoughtSpotService(recorder).validateConnection()) {

if (!(await this.getThoughtSpotService(recorder).validateConnection())) {

References

When a method returns a Promise, ensure it is awaited to prevent race conditions and potential data loss, especially when subsequent operations depend on its completion.

added await

mouryabalabhadra · 2026-07-16T04:40:31Z

@copilot review the pr

Copilot · 2026-07-16T04:48:39Z

@copilot review the pr

Reviewed the PR and addressed one issue: fetch_data now rejects empty visualization_ids instead of treating it as “fetch all”. Added validation coverage in tests as well. Fixed in commit 352f265.

saharsh-ts · 2026-07-16T13:11:10Z

@copilot review the pr

Rifdhan · 2026-07-22T00:21:13Z

+		),
+	columns: z.array(z.string()).describe("The column names, in order."),
+	data_rows: z
+		.array(z.array(z.unknown()))


Is there a way to add a proper type contract

Tightened to string | number | boolean | null datatypes.

Rifdhan · 2026-07-22T00:23:50Z

+
+// Round numeric cells to 2 decimals: collapses FP noise and trims payload.
+// Non-numbers and non-finite values pass through untouched.
+function roundCell(value: unknown): unknown {


This seems arbitrary, what if the customer wants more precision? I feel we should return the full value for now, and not prematurely optimize this.

Removed — cells now returned at full precision.

Rifdhan · 2026-07-22T00:29:44Z

+		// treats it as "must hold the whole viz" and 500s when it's too small.
+		// Start at maxRows; on that error, bump to the required count and refetch,
+		// then cap rows client-side below. Attempts are bounded (a full Liveboard
+		// can report a larger viz on each retry) to avoid a runaway loop.


This seems like a hacky design, we should make the API call such that it is successful on the first try. Is there no way to do that using this API?

Reworked to one call — Answers pass record_size = max_rows,
Liveboards request the max 32-bit int.
Verified on local instance.

saharsh-ts · 2026-08-06T04:10:01Z

@copilot review the PR

Rifdhan · 2026-08-10T22:36:02Z

+		description: [
+			"Fetch the full data (columns and rows) of a saved Answer or Liveboard, identified by its GUID.",
+			"CALL THIS WHENEVER the user wants to explain, describe, summarize, analyze, interpret, or ask what a specific object contains or shows: any explanation of what an object contains must be grounded in the data this tool returns, so fetch it first with the object's `id`, then answer from the returned data.",
+			"Returns the object's data exactly as saved — it does not run new queries or change the object's question, filters, or columns. The result is shaped to the object's type: an Answer returns a single tabular result, a Liveboard returns one tabular result per visualization (each with its visualization id and name). To pull a single visualization pinned on a Liveboard, pass the Liveboard GUID as `object_id` and the visualization GUID in `visualization_ids`. Each result includes the column names and data rows. Use the optional `max_rows` to bound the rows returned per visualization (defaults to 25).",


This description is a bit misleading, since TS will execute the saved query against the latest state of the data, so the tool may return different data compared to when the user first saved the answer. It WILL also run a new database query, so not sure why this says it does not run new queries.

updated tool definitions.

Rifdhan · 2026-08-10T22:37:16Z

+	type: z
+		.string()
+		.describe("The resolved object type: either 'ANSWER' or 'LIVEBOARD'."),
+	description: z.string().describe("The description of the object."),


There are a lot of fields here duplicated from the search_objects response. Let's make it more concise and only include truly NEW data which the agent doesn't already know.

Trimmed output to just data. Removed id/name/type/description — all already known from search_objects.

Rifdhan · 2026-08-10T22:37:47Z

+		.describe(
+			"The object's data. A single entry for an Answer; one entry per visualization for a Liveboard.",
+		),
+	request_id: z


Let's remove like last time

Removed request_id. Still sent as x-request-id upstream for tracing.

Rifdhan · 2026-08-10T22:42:31Z

+	row_count: z
+		.number()
+		.optional()
+		.describe("Number of rows actually returned in `data_rows`."),


Isn't this redundant? The agent can see how many rows there are

Removed — agent can count data_rows. Kept total_row_count (upstream total, not derivable when capped).

Rifdhan · 2026-08-10T22:54:05Z

+
+// Only Answers and Liveboards expose fetchable data.
+const ANSWER_TYPE = "ANSWER";
+const LIVEBOARD_TYPE = "LIVEBOARD";


Should these be an enum

Done — enum ObjectType { Answer, Liveboard }.

Rifdhan · 2026-08-10T22:56:20Z

+		const headers = buildHeaders(token, undefined, undefined, { requestId });
+
+		// Step 1: resolve the object's type — it decides the data endpoint.
+		const metaData = await postJson(


Why do we need to do /metadata/search again? I thought we already did this in search_objects tool and returned all the necessary details to directly call the relevant /data API here.

Added optional object_type — pass the type from search_objects and we skip the lookup (one call). Kept optional so a bare GUID still resolves.

saharsh-ts added 2 commits July 9, 2026 11:52

[SCAL-319676] terminologies added

d7f8fbd

[SCAL-323294] changes for search_objects tool

57e354a

gemini-code-assist Bot reviewed Jul 9, 2026

View reviewed changes

[SCAL-323294] merge main and resolve conflicts

9396681

saharsh-ts force-pushed the SCAL-323294_fetch-data branch from 2b7918b to de4930a Compare July 13, 2026 07:47

Copilot started work on behalf of mouryabalabhadra July 16, 2026 04:40 View session

Copilot finished work on behalf of mouryabalabhadra July 16, 2026 04:49

Copilot AI requested a review from mouryabalabhadra July 16, 2026 04:49

saharsh-ts force-pushed the SCAL-323294_fetch-data branch from 352f265 to 6454d11 Compare July 16, 2026 05:14

[SCAL-323294] added schema for search_objects response format

959eeed

saharsh-ts force-pushed the SCAL-323294_fetch-data branch from 6454d11 to ddaf1fa Compare July 20, 2026 09:47

Rifdhan reviewed Jul 22, 2026

View reviewed changes

saharsh-ts added 2 commits July 22, 2026 14:07

[SCAL-323294] refactoring and updates to search-objects tool

09bd735

[SCAL-323294] fetch_data refactoring and updates

cb87e5c

saharsh-ts force-pushed the SCAL-323294_fetch-data branch from 349b5d8 to cb87e5c Compare July 22, 2026 08:45

saharsh-ts added 4 commits July 24, 2026 14:00

[SCAL-323294] added schema check for searhc_objects, and refactored

7fd9463

Merge branch 'SCAL-323294_search-objects' into SCAL-323294_fetch-data

1d9164f

[SCAL-323294] merge main

6e603fe

[SCAL-323294] updated on review comments

7488bc3

Rifdhan reviewed Aug 10, 2026

View reviewed changes

saharsh-ts added 4 commits August 11, 2026 08:40

Merge branch 'main' into SCAL-323294_fetch-data

33c01f9

[SCAL-323294] check for data download privileges

f9cc421

[SCAL-323294] update mcp-server-base.ts

8f19484

[SCAL-323294] updated tool definitions and on comments.

31e061b

	if (!this.getThoughtSpotService(recorder).validateConnection()) {
	if (!(await this.getThoughtSpotService(recorder).validateConnection())) {

Uh oh!

Conversation

saharsh-ts commented Jul 9, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mouryabalabhadra commented Jul 16, 2026

Uh oh!

Copilot AI commented Jul 16, 2026

Uh oh!

saharsh-ts commented Jul 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saharsh-ts commented Aug 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants