Png image #54

shikharbsar · 2025-07-29T18:46:49Z

No description provided.

cloudflare-workers-and-pages · 2025-07-29T18:46:57Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
❌ Deployment failed View logs	png-image	`0dc21cc`	Aug 11 2025, 09:32 PM

mnk-blr · 2025-07-29T18:46:59Z

🎉 Snyk checks have passed. No issues have been found so far.

✅ security/snyk check is complete. No issues have been found. (View Details)

✅ license/snyk check is complete. No issues have been found. (View Details)

gemini-code-assist

Summary of Changes

Hello @shikharbsar, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces the capability to generate and serve PNG images of data visualizations directly from ThoughtSpot. It establishes a new API endpoint for image retrieval, backed by a temporary token system utilizing Cloudflare KV for secure access to session details. The changes also enhance the server's context to facilitate environment access and update various configurations and type definitions to support the new functionality.

Highlights

Image Retrieval Endpoint: Introduced a new GET endpoint /data/img in src/handlers.ts to serve PNG images of ThoughtSpot visualizations. This endpoint uses a temporary token to retrieve necessary session and access details from a KV store.
Temporary Token Management: Implemented a system in src/servers/mcp-server.ts and src/servers/openai-mcp-server.ts to generate unique tokens, store ThoughtSpot session ID, generation number, instance URL, and access token in Cloudflare KV, and provide a URL for image retrieval in the API response.
Server Context Enhancement: Modified the Context interface in src/servers/mcp-server-base.ts and refactored instrumentedMCPServer in src/utils.ts to allow Durable Objects to access the Cloudflare Workers environment (env), enabling KV interactions.
ThoughtSpot Integration for Images: Added a new getAnswerImagePNG method to src/thoughtspot/thoughtspot-service.ts to interact with the ThoughtSpot API and export answer reports as PNG images.
Configuration and Type Updates: Updated environment variable access in src/index.ts, added HOST_NAME to wrangler.jsonc and worker-configuration.d.ts, and performed various type definition cleanups in worker-configuration.d.ts.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces functionality to generate and serve PNG images of ThoughtSpot answers. It adds a new endpoint /data/img which uses a token-based mechanism with KV store to retrieve image data. The changes touch multiple files to support this flow, including modifications to the MCP servers to generate image URLs, and updates to the ThoughtSpot service to fetch the image data. The PR also includes some refactoring to pass environment variables to server instances and updates to dependency configurations.

My review focuses on ensuring the new endpoint is robust, identifying potential runtime errors, and suggesting improvements for code clarity, consistency, and maintainability. I've pointed out a critical issue with a method call that would lead to a runtime error, and a high-severity issue regarding a potentially accidental removal of a configuration block. Other comments focus on removing debug logs, improving type safety, and standardizing naming conventions.

ashubham · 2025-07-29T22:25:58Z

src/servers/mcp-server.ts

+                accessToken: this.ctx.props.accessToken
+            };
+            await putInKV(token, tokenData, this.ctx.env);
+            tokenUrl = `${this.ctx.env?.HOST_NAME}/data/img?token=${token}`;


Maybe we call token something like uniqueId or tempId to not make the agent believe its a token protected resource and should not be opened

ashubham · 2025-07-29T22:27:53Z

src/servers/openai-mcp-server.ts

+            const token = crypto.randomUUID();
+            const tokenData = {
+                sessionId: answer.session_identifier,
+                generationNo: answer.generation_number,
+                instanceURL: this.ctx.props.instanceUrl,
+                accessToken: this.ctx.props.accessToken
+            };
+            await putInKV(token, tokenData, this.ctx.env);
+            tokenUrl = `${this.ctx.env?.HOST_NAME}/data/img?token=${token}`;


this seems to be duplicate code

ashubham · 2025-07-29T22:28:40Z

src/servers/openai-mcp-server.ts

+            tokenUrl = `${this.ctx.env?.HOST_NAME}/data/img?token=${token}`;
+        }
+        let content = answer.data;
+        if (tokenUrl) {


will tokenUrl be not available in any case ? If not we can remove this conditionality

ashubham · 2025-07-29T22:29:30Z

src/servers/openai-mcp-server.ts

+**CHART VISUALIZATION AVAILABLE:**
+There is a data visualization chart for this query available at: "${tokenUrl}".
+
+This URL contains a PNG image showing the visual representation of the above data for the question: "${question}". Please fetch and display this image to provide a complete analysis with both the data and its visual representation. The image must be displayed in the response of this question. 
+Use just this image in the response, do not use any other images or visualizations or charts. It may take time to get the image, so please wait for the image to be fetched and displayed in the response.`;


Can we make this consistent across different mcp servers ? Maybe move to the common service layer.

ashubham · 2025-07-29T22:31:23Z

wrangler.jsonc

 		"HONEYCOMB_DATASET": "",
-		"HONEYCOMB_API_KEY": ""
+		"HONEYCOMB_API_KEY": "",
+		"HOST_NAME": "https://agent.thoughtspot.app"


We have created aliases for this, and might also change this in the future. Lets not hardcode this. This should be available in the request payload headers.

Yeah, added it as part of props, let me know if I need to change it.

ashubham · 2025-08-07T00:02:53Z

src/handlers.ts

+    async getAnswerImage(request: Request, env: Env) {
+        const span = getActiveSpan();
+        const url = new URL(request.url);
+        const token = url.searchParams.get('token') || url.searchParams.get('uniqueId') || '';


remove token

Rename the variable as well

ashubham · 2025-08-07T00:04:01Z

src/handlers.ts

+        const tokenData = await getFromKV(token, env);
+        if (!tokenData) {
+            span?.setStatus({ code: SpanStatusCode.ERROR, message: "Token not found" });
+            return new Response("Token not found", { status: 404 });


Can we create an error response utility for the handlers as well.

ashubham · 2025-08-07T00:06:17Z

src/handlers.ts

+        const sessionId = (tokenData as any).sessionId;
+        const generationNo = (tokenData as any).GenNo || (tokenData as any).generationNo; // Handle both field names
+        const instanceURL = (tokenData as any).instanceURL;
+        const accessToken = (tokenData as any).accessToken;


const { sessionId, generationNo, instanceURL, accessToken } = tokenData as any;

ashubham · 2025-08-07T00:08:46Z

src/servers/api-server.ts

+    @WithSpan('api-get-answer-image')
+    async getAnswerImage(props: Props, sessionId: string, generationNo: number) {
+        const service = this.getThoughtSpotService(props);
+        return await service.getAnswerImage(sessionId, generationNo);
+    }


this might not be needed.

ashubham · 2025-08-07T00:10:09Z

src/servers/mcp-server-base.ts

+    }
+
+    protected async getAnswerContent(answer: any, question: string) {
+        const tokenUrl = await this.getTokenUrl(answer.session_identifier, answer.generation_number);


createImageUrl

ashubham · 2025-08-07T00:10:47Z

src/servers/mcp-server-base.ts

+                instanceURL: this.ctx.props.instanceUrl,
+                accessToken: this.ctx.props.accessToken
+            };
+            await putInKV(token, tokenData, this.ctx.env);


should not put in a get function. Lets rename this createImageUrl

ashubham · 2025-08-07T00:12:15Z

src/servers/mcp-server-base.ts

+    protected async getTokenUrl(sessionId: string, generationNo: number) {
+        let tokenUrl = "";
+        // Generate token and store in KV store
+        if (this.ctx.env?.OAUTH_KV) {


Lets put this in a try ... catch. The putInKV method should throw if OAUTH_KV is not available. And we should handle that condition in the catch

ashubham · 2025-08-07T00:15:19Z

src/servers/mcp-server-base.ts

+            };
+            await putInKV(token, tokenData, this.ctx.env);
+            const hostName = this.ctx.props.hostName;
+            if (hostName && (hostName.startsWith("http") || hostName.startsWith("https"))) {


handle this while setting the prop

ashubham · 2025-08-07T00:16:35Z

src/thoughtspot/thoughtspot-service.ts

    }

+    @WithSpan('get-answer-image')
+    async getAnswerImagePNG(sessionId: string, GenNo: number): Promise<HttpFile> {


ashubham · 2025-08-07T00:32:23Z

src/utils.ts

+        private _server: T | undefined;
+        private _env: Env;
+
+        // Lazy getter for server that creates it when first accessed
+        // 
+        // WHY THIS APPROACH:
+        // Originally we passed 'this' directly as Context: `server = new MCPServer(this)`
+        // This worked when Context was just { props: Props }, but broke when we added env.
+        // 
+        // PROBLEMS WITH ORIGINAL APPROACH:
+        // 1. McpAgent's 'env' property is protected, but Context expects public
+        // 2. TypeScript error: "Property 'env' is protected but public in Context"
+        // 
+        // WHY NOT CONSTRUCTOR CREATION:
+        // We tried creating server in constructor: `new MCPServer({ props: this.props, env })`
+        // But this.props is undefined during constructor - it gets set later by McpAgent._init()
+        // Runtime error: "Cannot read properties of undefined (reading 'instanceUrl')"
+        // 
+        // SOLUTION - LAZY INITIALIZATION:
+        // - Store env from constructor (available immediately)
+        // - Create server only when first accessed (after props are set by McpAgent lifecycle)
+        // - Combine both props and env into proper Context object
+        get server(): T {
+            if (!this._server) {
+                const context: Context = {
+                    props: this.props, // Available after McpAgent._init() sets it
+                    env: this._env     // Stored from constructor
+                };
+                this._server = new MCPServer(context);
+            }
+            return this._server;
+        }


declare public env: Env;

And then move creation of the server inside the init method.

gemini-code-assist bot reviewed Jul 29, 2025

View reviewed changes

ashubham requested changes Jul 29, 2025

View reviewed changes

ashubham force-pushed the png-image branch from 188b5de to 60c1e09 Compare July 30, 2025 16:14

ashubham reviewed Aug 7, 2025

View reviewed changes

shikharbsar added 23 commits August 7, 2025 15:05

Add png image formation to MCP

c6c23ab

remove value from log line

b3dab1e

add png image to chatgpt mcp server

9dac59b

fix tool response text

88b981a

add url in text to get png-image

d931b05

change return text

4819bfc

use a comprehensive prompt to fetch image

4f50566

add logs and prompt

fd3f9a9

add question in tool response

e4a2545

make some prompt changes

f7d6f7f

make url prominent

1725a76

return image blob

30757ea

add tests

f692cf4

restore wrangler

29588ea

add HOST_NAME to envirnoment variables

e6c0b7a

remove env vars

370d1cd

create single implementation for getTokenURL

d1fa96c

comment migrations

f9a9702

use uniqueId as well for fetching image from token url

3b222a7

address comments

2b32547

add await to get token url before using

6477e7a

fix tests

b5f4482

address comments

0dc21cc

ashubham force-pushed the png-image branch from 25cca71 to 0dc21cc Compare August 11, 2025 21:32

Png image #54

Are you sure you want to change the base?

Png image #54

Uh oh!

Conversation

shikharbsar commented Jul 29, 2025

Uh oh!

cloudflare-workers-and-pages bot commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying with Cloudflare Workers

Uh oh!

mnk-blr commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎉 Snyk checks have passed. No issues have been found so far.

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

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!

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

cloudflare-workers-and-pages bot commented Jul 29, 2025 •

edited

Loading

mnk-blr commented Jul 29, 2025 •

edited

Loading