Skip to content

deduplication of tools#1242

Open
otakup0pe wants to merge 5 commits intokarthink:masterfrom
otakup0pe:tool-conflicts
Open

deduplication of tools#1242
otakup0pe wants to merge 5 commits intokarthink:masterfrom
otakup0pe:tool-conflicts

Conversation

@otakup0pe
Copy link
Copy Markdown

This fixes #1097 through the following mechanisms. Ended up not splitting this into two PRs as the functionality seemed kinda intertwined, I hope that is 🆗

  • (configurable) visual indicators in transient menu of duplicate tools
  • runtime resolution of duplicate tools for presets and chat mode

This PR was created with LLM assistance. I have been running it for about six weeks and it's been behaving solidly during that time.

@karthink
Copy link
Copy Markdown
Owner

@otakup0pe This is a much bigger change than I thought it would be, and it looks like the LLM has thrown away some of my comments. If you can reduce the scope of the PR and not discard my comments -- which are essential for future understanding of the code (by me and others), I will have an easier time reviewing this.

@otakup0pe
Copy link
Copy Markdown
Author

Oh my gosh sorry. I must have misread reviewing the computer hallucination before getting on a flight - it seemed the comments no longer applied given the proposed implementation!

Do you have any preference on how I shrink the scope? if you are not tied to the "disambiguation" part of the PR (showing which tools are duplicates in the transient view) I might just drop that part completely. During my use of this PR it has been less useful than I thought, and the de-duplication could stand alone.

@otakup0pe
Copy link
Copy Markdown
Author

OK I removed some of the transient ux changes - just kept the bit that let's the user (de) select duplicate tools across categories. In the end, the warnings/counts ended up not really being needed outside of troubleshooting. Please let me know if you would like any other changes, or any other tests.

@karthink
Copy link
Copy Markdown
Owner

Thanks. This PR is still on my radar, just busy with some other features atm.

@otakup0pe
Copy link
Copy Markdown
Author

Oh hey thanks for the bump! I'll update from mainline branch, and make sure the tests are updated 💖

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.

Handling of duplicate mcp-derived tool names

2 participants