Skip to content

Commit d5950c9

Browse files
authored
Merge pull request #10 from apollographql/jeffrey/parse-comment-description
Parse the comments before the operation as it’s description
2 parents 34b3ed4 + 3a34cac commit d5950c9

File tree

3 files changed

+169
-78
lines changed

3 files changed

+169
-78
lines changed

Cargo.lock

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/mcp-apollo-server/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ tokio = { version = "1.44.2", features = [
3232
] }
3333
tracing.workspace = true
3434
tracing-subscriber = { version = "0.3.19", features = ["env-filter"] }
35+
regex = "1.11.1"
3536

3637
[dev-dependencies]
3738
insta = "1.42.2"

crates/mcp-apollo-server/src/operations.rs

+167-78
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use apollo_compiler::{
44
ast::{Definition, OperationDefinition, Type},
55
parser::Parser,
66
};
7+
use regex::Regex;
78
use rmcp::{
89
model::Tool,
910
schemars::schema::{
@@ -14,6 +15,7 @@ use rmcp::{
1415
};
1516
use rover_copy::pq_manifest::ApolloPersistedQueryManifest;
1617
use serde::Serialize;
18+
use std::collections::HashMap;
1719

1820
use crate::custom_scalar_map::CustomScalarMap;
1921
use crate::errors::{McpError, OperationError};
@@ -47,15 +49,36 @@ impl Operation {
4749
.parse_ast(source_text, "operation.graphql")
4850
.map_err(|e| OperationError::GraphQLDocument(Box::new(e)))?;
4951

50-
let mut operation_defs = document.definitions.iter().filter_map(|def| match def {
51-
Definition::OperationDefinition(operation_def) => Some(operation_def),
52-
Definition::FragmentDefinition(_) => None,
53-
_ => {
54-
tracing::error!(
55-
spec=?def,
56-
"Schema definitions were passed in, only operations and fragments are allowed"
57-
);
58-
None
52+
let mut last_offset: Option<usize> = Some(0);
53+
let mut operation_defs = document.definitions.iter().filter_map(|def| {
54+
let description = match def.location() {
55+
Some(source_span) => {
56+
let description = last_offset
57+
.map(|start_offset| &source_text[start_offset..source_span.offset()]);
58+
last_offset = Some(source_span.end_offset());
59+
description
60+
}
61+
None => {
62+
last_offset = None;
63+
None
64+
}
65+
};
66+
67+
match def {
68+
Definition::OperationDefinition(operation_def) => {
69+
Some((operation_def, description))
70+
}
71+
Definition::FragmentDefinition(_) => None,
72+
_ => {
73+
eprintln!(
74+
// This string needs to be broken into multiple lines or it will break cargo fmt
75+
"
76+
Schema definitions were passed in,
77+
only operations and fragments are allowed
78+
"
79+
);
80+
None
81+
}
5982
}
6083
});
6184

@@ -68,7 +91,7 @@ impl Operation {
6891
})
6992
.collect();
7093

71-
let operation = match (operation_defs.next(), operation_defs.next()) {
94+
let (operation, comments) = match (operation_defs.next(), operation_defs.next()) {
7295
(None, _) => return Err(OperationError::NoOperations),
7396
(_, Some(_)) => {
7497
return Err(OperationError::TooManyOperations(
@@ -86,7 +109,8 @@ impl Operation {
86109
})?
87110
.to_string();
88111

89-
let description = Self::tool_description(graphql_schema, operation, &fragment_defs);
112+
let description =
113+
Self::tool_description(comments, graphql_schema, operation, &fragment_defs);
90114

91115
let object = serde_json::to_value(get_json_schema(
92116
operation,
@@ -124,85 +148,104 @@ impl Operation {
124148

125149
/// Generate a description for an operation based on documentation in the schema
126150
fn tool_description(
151+
comments: Option<&str>,
127152
graphql_schema: &GraphqlSchema,
128153
operation_def: &Node<OperationDefinition>,
129154
fragment_defs: &[&Node<FragmentDefinition>],
130155
) -> String {
131-
let mut tree_shaker = TreeShaker::new(graphql_schema, fragment_defs);
132-
let descriptions = operation_def
133-
.selection_set
134-
.iter()
135-
.filter_map(|selection| {
136-
match selection {
137-
Selection::Field(field) => {
138-
let field_name = field.name.to_string();
139-
let operation_type = operation_def.operation_type;
140-
if let Some(root_name) = graphql_schema.root_operation(operation_type) {
141-
// Find the root field referenced by the operation
142-
let root = graphql_schema.get_object(root_name)?;
143-
let field_definition = root
144-
.fields
145-
.iter()
146-
.find(|(name, _)| {
147-
let name = name.to_string();
148-
name == field_name
149-
})
150-
.map(|(_, field_definition)| field_definition.node.clone());
151-
152-
// Add the root field description to the tool description
153-
let field_description = field_definition
154-
.clone()
155-
.and_then(|field| field.description.clone())
156-
.map(|node| node.to_string());
157-
158-
// Add information about the return type
159-
let ty = field_definition.map(|field| field.ty.clone());
160-
let type_description = ty.as_ref().map(Self::type_description);
161-
162-
// Retain the return type in the tree shaker
163-
if let Some(ty) = ty {
164-
let type_name = ty.inner_named_type();
165-
if let Some(extended_type) =
166-
graphql_schema.types.get(type_name.as_str())
156+
let comment_description = comments.and_then(|comments| {
157+
let content = Regex::new(r"(\n|^)\s*#").ok()?.replace_all(comments, "$1");
158+
let trimmed = content.trim();
159+
160+
if trimmed.is_empty() {
161+
None
162+
} else {
163+
Some(trimmed.to_string())
164+
}
165+
});
166+
167+
match comment_description {
168+
Some(description) => description,
169+
None => {
170+
let mut tree_shaker = TreeShaker::new(graphql_schema, fragment_defs);
171+
let descriptions = operation_def
172+
.selection_set
173+
.iter()
174+
.filter_map(|selection| {
175+
match selection {
176+
Selection::Field(field) => {
177+
let field_name = field.name.to_string();
178+
let operation_type = operation_def.operation_type;
179+
if let Some(root_name) =
180+
graphql_schema.root_operation(operation_type)
167181
{
168-
tree_shaker.retain(
169-
type_name.clone(),
170-
extended_type,
171-
&field.selection_set,
182+
// Find the root field referenced by the operation
183+
let root = graphql_schema.get_object(root_name)?;
184+
let field_definition = root
185+
.fields
186+
.iter()
187+
.find(|(name, _)| {
188+
let name = name.to_string();
189+
name == field_name
190+
})
191+
.map(|(_, field_definition)| field_definition.node.clone());
192+
193+
// Add the root field description to the tool description
194+
let field_description = field_definition
195+
.clone()
196+
.and_then(|field| field.description.clone())
197+
.map(|node| node.to_string());
198+
199+
// Add information about the return type
200+
let ty = field_definition.map(|field| field.ty.clone());
201+
let type_description = ty.as_ref().map(Self::type_description);
202+
203+
// Retain the return type in the tree shaker
204+
if let Some(ty) = ty {
205+
let type_name = ty.inner_named_type();
206+
if let Some(extended_type) =
207+
graphql_schema.types.get(type_name.as_str())
208+
{
209+
tree_shaker.retain(
210+
type_name.clone(),
211+
extended_type,
212+
&field.selection_set,
213+
)
214+
}
215+
}
216+
217+
Some(
218+
vec![field_description, type_description]
219+
.into_iter()
220+
.flatten()
221+
.collect::<Vec<String>>()
222+
.join("\n"),
172223
)
224+
} else {
225+
None
173226
}
174227
}
175-
176-
Some(
177-
vec![field_description, type_description]
178-
.into_iter()
179-
.flatten()
180-
.collect::<Vec<String>>()
181-
.join("\n"),
182-
)
183-
} else {
184-
None
228+
_ => None,
185229
}
186-
}
187-
_ => None,
188-
}
189-
})
190-
.collect::<Vec<String>>()
191-
.join("\n---\n");
230+
})
231+
.collect::<Vec<String>>()
232+
.join("\n---\n");
192233

193-
// Add the tree-shaken types to the end of the tool description
194-
let mut lines = vec![];
195-
lines.push(descriptions);
234+
// Add the tree-shaken types to the end of the tool description
235+
let mut lines = vec![];
236+
lines.push(descriptions);
196237

197-
let mut shaken = tree_shaker.shaken().peekable();
198-
if shaken.peek().is_some() {
199-
lines.push(String::from("---"));
200-
}
201-
for ty in shaken {
202-
lines.push(ty.serialize().to_string());
203-
}
238+
let mut shaken = tree_shaker.shaken().peekable();
239+
if shaken.peek().is_some() {
240+
lines.push(String::from("---"));
241+
}
242+
for ty in shaken {
243+
lines.push(ty.serialize().to_string());
244+
}
204245

205-
lines.join("\n")
246+
lines.join("\n")
247+
}
248+
}
206249
}
207250

208251
fn type_description(ty: &Type) -> String {
@@ -1317,6 +1360,52 @@ mod tests {
13171360
);
13181361
}
13191362

1363+
#[test]
1364+
fn tool_comment_description() {
1365+
let operation = Operation::from_document(
1366+
r###"
1367+
# Overridden tool #description
1368+
query GetABZ($state: String!) {
1369+
b {
1370+
d {
1371+
f
1372+
}
1373+
}
1374+
}
1375+
"###,
1376+
&SCHEMA,
1377+
None,
1378+
)
1379+
.unwrap();
1380+
1381+
insta::assert_snapshot!(
1382+
operation.tool.description.as_ref(),
1383+
@r###"Overridden tool #description"###
1384+
);
1385+
}
1386+
1387+
#[test]
1388+
fn tool_empty_comment_description() {
1389+
let operation = Operation::from_document(
1390+
r###"
1391+
#
1392+
1393+
#
1394+
query GetABZ($state: String!) {
1395+
id
1396+
}
1397+
"###,
1398+
&SCHEMA,
1399+
None,
1400+
)
1401+
.unwrap();
1402+
1403+
insta::assert_snapshot!(
1404+
operation.tool.description.as_ref(),
1405+
@r###"The returned value is optional and has type `String`"###
1406+
);
1407+
}
1408+
13201409
#[test]
13211410
fn it_extracts_operations_from_apollo_pq_manifest() {
13221411
// The inner types needed to construct one of these are not exported,

0 commit comments

Comments
 (0)