Skip to content

Added keep_data_uris support for Mcp. #1283

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Sergeant61
Copy link

@Sergeant61 Sergeant61 commented Jun 4, 2025

Added keep_data_uris support for Mcp.

@Sergeant61 Sergeant61 changed the title feat: add keep_data_uris parameter to convert_to_markdown function for mcp-server Added keep_data_uris support for Mcp. Jun 4, 2025
@afourney
Copy link
Member

afourney commented Jun 4, 2025

@Sergeant61 putting this argument here will require the model to turn the feature on each time it calls the tool.

(I'm also super conservative about how/when to expand the action space)

Would a server configuration approach work for your scenario? E.g. a config file, or command line argument for the server that turns this on for all calls?

@Sergeant61
Copy link
Author

Actually, we added it like that at the beginning, but my teammate and I thought that passing it as a parameter would create a more dynamic structure. In some files we want this, in some we don't, then shouldn't we restart the mcp server every time?

@Sergeant61
Copy link
Author

We discovered this (keep_data_uris) feature while reviewing PRs with my friend. We haven't examined all the features yet, but honestly, I’d like to see features like this added to the MCP server. How do you plan to incorporate such features into the MCP server? I can update the PR accordingly. However, it might be necessary to think about this in a broader scope.

@afourney
Copy link
Member

afourney commented Jun 5, 2025

We discovered this (keep_data_uris) feature while reviewing PRs with my friend. We haven't examined all the features yet, but honestly, I’d like to see features like this added to the MCP server. How do you plan to incorporate such features into the MCP server? I can update the PR accordingly. However, it might be necessary to think about this in a broader scope.

My sense is that we can create some config options, or similar, for the server that would set things like this on startup. My concern about adding it to the function definition that the LLMs see is that the LLMs then need to consider it, and guess the right action, on every LLM call. But if we just configure a flag when starting the server, you get deterministic behavior on that front, and it doesn't confuse the LLM.

But this only works if you downstream application ALWAYS or NEVER wants to save data uris. If it's case-by-case, then the current solution is needed. My gut feel, though, is that ALWAYS or NEVER cover most applications.

@Sergeant61
Copy link
Author

Okay then I will update the pr as you say as soon as possible.

@Sergeant61
Copy link
Author

You can now pass parameters via the command line.
For example:

markitdown-mcp --http --host 127.0.0.1 --port 3001 --keep-data-uris

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.

2 participants