Skip to content

Conversation

esilverm
Copy link
Contributor

In #346, it is mentioned that our tools aren't properly picking up deprecated directives when building and executing operations. This led me to dig deeper and find that our minify handling for the builtin tools does not include any directives in the minified output. This can potentially be harmful since it means the LLM doesn't know to exclude certain fields when building operations with introspect.

In this PR, I've added the ability to minify deprecated directives with the inclusion of their reason argument to give better direction to the LLM when it works within this problem space.
Additionally, I've added some snapshot tests to the minify file so we can assert that the minified schema is built properly and includes the things we expect it to include.

I've also made sure to add proper coverage for the directive since it has recently been updated to be allowed to be placed on INPUT_FIELD_DEFINITION and ARGUMENT_DEF (though these technically fail schema validation with our current graphql-rs parser.

@apollo-librarian
Copy link

apollo-librarian bot commented Sep 17, 2025

✅ Docs preview ready

The preview is ready to be viewed. View the preview

File Changes

0 new, 2 changed, 0 removed
* (developer-tools)/apollo-mcp-server/(latest)/config-file.mdx
* (developer-tools)/apollo-mcp-server/(latest)/define-tools.mdx

Build ID: 462d8d6d1cd4f59f3f55af73
Build Logs: View logs

URL: https://www.apollographql.com/docs/deploy-preview/462d8d6d1cd4f59f3f55af73

@esilverm esilverm marked this pull request as ready for review September 17, 2025 15:36
@esilverm esilverm requested a review from a team as a code owner September 17, 2025 15:36
Copy link
Contributor

@DaleSeo DaleSeo left a comment

Choose a reason for hiding this comment

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

We should update the relevant section in our docs:

2025-09-18 at 09 51 35

https://www.apollographql.com/docs/apollo-mcp-server/define-tools#minification

@esilverm esilverm requested a review from a team as a code owner September 18, 2025 16:04
@esilverm esilverm requested a review from DaleSeo September 18, 2025 16:04
@esilverm esilverm force-pushed the evan/minify/include-builtin-directives branch from 50ad170 to 8d83382 Compare September 18, 2025 16:30
Copy link
Contributor

@DaleSeo DaleSeo left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my feedback! 🙇

@esilverm esilverm merged commit 131a9e0 into develop Sep 23, 2025
11 checks passed
@esilverm esilverm deleted the evan/minify/include-builtin-directives branch September 23, 2025 18:49
@apollo-bot2 apollo-bot2 mentioned this pull request Sep 24, 2025
@DaleSeo DaleSeo mentioned this pull request Sep 24, 2025
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