Skip to content

[front] Remove ToolGeneratedFileType in favor of blob resources #13508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from

Conversation

philipperolet
Copy link
Contributor

@philipperolet philipperolet commented Jun 17, 2025

Summary

  • Removed ToolGeneratedFileType, isToolGeneratedFile, and ToolGeneratedFileSchema in favor of standard blob resources
  • Extracted CSV and JSON output generation logic from file upload functionality into separate helper functions
  • Updated all MCP servers (run_dust_app, tables_query, process) to return blob resources directly
  • Simplified UI components to work with cleaner file resource structure
  • Removed unused INTERNAL_MIME_TYPES.TOOL_OUTPUT.FILE constant

Risks

Blast radius: MCP tool outputs and file generation across the platform
Risk: low

Tests

Local tests: mcp actions process, run dust app, table query

Deploy Plan

  • Deploy front service

philipperolet and others added 2 commits June 18, 2025 11:38
- Extract CSV/JSON generation logic from file creation functions
- Update MCP servers to return blob resources instead of ToolGeneratedFile
- Remove isToolGeneratedFile checks and special handling
- Update UI components to work with simplified file data
- Remove INTERNAL_MIME_TYPES.TOOL_OUTPUT.FILE constant

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
## Summary
- Extracted CSV and JSON output generation from file upload logic
- Updated all MCP servers to return blob resources instead of ToolGeneratedFile
- Removed ToolGeneratedFileType, isToolGeneratedFile, and related schemas
- Simplified UI components to work with simpler file resource structure
- Removed unused INTERNAL_MIME_TYPES.TOOL_OUTPUT.FILE constant

## Risks
- All MCP servers now return blob resources which are handled uniformly
- UI components updated to work with simpler props structure
- No breaking changes to external APIs

## Tests
- TypeScript compilation passes
- ESLint shows no errors
- Code properly formatted

## Deploy Plan
- Deploy front service

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@philipperolet philipperolet force-pushed the pr/177-remove-file-output branch from f4f0132 to 2d8a9b1 Compare June 18, 2025 09:53
Copy link

github-actions bot commented Jun 18, 2025

Fails
🚫

Files in **/sdks/js/ have been modified. Changing the types defined in the SDK could break existing client.
Additions (new types, new values) are generally fine but removals are NOT OK : it would break the contract of the Public API.
Please add the sdk-ack label to acknowledge that your are not breaking the existing Public API contract.

Generated by 🚫 dangerJS against ae4cbb6

@philipperolet philipperolet force-pushed the pr/177-remove-file-output branch from 54088b4 to c192b2a Compare June 18, 2025 11:39
philipperolet

This comment was marked as resolved.

- Rename generateJSONOutput to generateJSONSnippet (only returns snippet)
- Remove generateJSONFileAndSnippet function entirely
- Remove EXTRACT_RESULT from INTERNAL_MIME_TYPES
- Use ActionGeneratedFileType in MCPToolOutputDetails
- Add uploadToConversationDataSource flag to handleBase64Upload
- Use BlobCallToolResultBlock type in run_dust_app.ts
- Move name field outside resource object
- Add snippet field to blob resources
- Move generateSectionFile to tables_query.ts module
isSupportedFileContentType(block.resource.mimeType)
) {
if (isBlobResource(block)) {
const fileName = isResourceWithName(block)
? block.name
: `generated-file-${Date.now()}.${extensionsForContentType(block.resource.mimeType as SupportedFileContentType)[0]}`;

// For extract_data server, we need to upload to conversation data source
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove that part. here we always want to upload to conversation ds

@@ -4,18 +4,28 @@ import type { TextContent } from "@modelcontextprotocol/sdk/types.js";
import type { ZodRawShape } from "zod";
import { z } from "zod";

// Define the BlobCallToolResultBlock type
Copy link
Contributor Author

Choose a reason for hiding this comment

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

delete that and use the type with the same name from output_schemas.ts

const content: {
type: "resource";
name: string;
resource: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

use BlobCallToolResultBlock type

contentType: plainTextFile.contentType,
snippet: plainTextFile.snippet,
text: `Generated text file: ${fileTitle}`,
uri: `data:text/plain;base64,${textContent.substring(0, 100)}...`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here, as well as in 3 other places in this PR where this uri scheme was introduced, replace ${textcontent.substring... by "transient-content"

resource: TablesQueryOutputResources;
}
| {
type: "resource";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use the resource field of BlobCallToolResultBlock here

- Remove uploadToConversationDataSource flag check - always upload to conversation DS
- Replace data URI substring with 'transient-content' placeholder
- Use BlobCallToolResultBlock type in tables_query server_v2
- Fix blob resource structure in server_v2 with name field outside resource
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant