This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-mcp.git
The following commit(s) were added to refs/heads/main by this push:
new e4428af feat(search): append remediation hints to common Solr query
errors (#166)
e4428af is described below
commit e4428af15b7f31a21fc03860728f2da4e9a0a44b
Author: Aditya Parikh <[email protected]>
AuthorDate: Tue Aug 18 17:43:21 2026 -0400
feat(search): append remediation hints to common Solr query errors (#166)
* feat(search): append remediation hints to common Solr query errors
MCP clients receive tool exception messages verbatim, making the error
message part of the tool's API surface. Raw Solr errors like 'undefined
field foo' leave the client to retry blind; naming the follow-up tool
lets it self-correct in the next call.
search now wraps SolrException with a next-step hint for three common
failures: undefined field (call get-schema; mentions dynamic suffixes),
query syntax errors (Lucene syntax, escaping, local params), and
missing collection (call list-collections). Unrecognized Solr errors
propagate unchanged. The collection-not-found error in CollectionService
gets the same list-collections hint.
Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: adityamparikh <[email protected]>
* test(search): pin remediation hints against real Solr; fix dead 404 branch
The hints classify Solr's error text, but that text is generated by
solr-core,
which is not on the classpath — only solr-solrj is. Nothing couples the
matchers
to Solr's wording at compile time, so the mock-based unit tests could only
prove
that a stubbed message produces a hint, never that Solr still emits such a
message. Testcontainers tests now provoke each failure on a real server.
Writing them surfaced a branch that never fired: an unknown collection is a
404
whose body is Solr's HTML page, so SolrJ reports it as
"Expected mime type in [application/json, text/plain] but got text/html" —
text containing none of "collection not found", "can not find", or "404".
The
unit test passed only because it asserted against a fixture string. Match
SolrException.ErrorCode.NOT_FOUND.code instead; the status code is the only
signal that survives (getMetadata() is null for this response), and it also
removes the "404" substring false-positive risk.
Undefined fields turned out to produce three distinct messages depending on
where the field appears — "undefined field x" (q/fq), "undefined field:
\"x\""
(facet), and "sort param field can't be found: x" (sort) — each now covered.
- extract Solr match tokens and hint text to named constants; tests assert
on
the hint constants, never on the token the matcher uses, so an assertion
cannot pass by tautology if Solr rewords a message
- integration tests: undefined field in q/fq/facet/sort, unparseable query,
missing collection, and an unrecognised error propagating without a hint
- unit tests: add not-found and undefined-sort-field cases
Signed-off-by: Aditya Parikh <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Signed-off-by: adityamparikh <[email protected]>
---------
Signed-off-by: adityamparikh <[email protected]>
Co-authored-by: Claude Fable 5 <[email protected]>
Co-authored-by: Eric Pugh <[email protected]>
---
.../mcp/server/collection/CollectionService.java | 3 +-
.../solr/mcp/server/search/SearchService.java | 75 +++++++++++++++++-
.../search/SearchServiceIntegrationTest.java | 88 ++++++++++++++++++++++
.../solr/mcp/server/search/SearchServiceTest.java | 68 +++++++++++++++++
4 files changed, 232 insertions(+), 2 deletions(-)
diff --git
a/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
b/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
index 8a259e6..48c69b4 100644
--- a/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
+++ b/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java
@@ -523,7 +523,8 @@ public class CollectionService {
// Validate collection exists
if (!validateCollectionExists(actualCollection)) {
- throw new
IllegalArgumentException(COLLECTION_NOT_FOUND_ERROR + actualCollection);
+ throw new
IllegalArgumentException(COLLECTION_NOT_FOUND_ERROR + actualCollection
+ + ". Hint: call list-collections to see
available collections.");
}
// Index statistics using Luke
diff --git a/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
b/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
index 0619fec..57eaa6c 100644
--- a/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
+++ b/src/main/java/org/apache/solr/mcp/server/search/SearchService.java
@@ -30,6 +30,7 @@ import org.apache.solr.client.solrj.response.FacetField;
import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.FacetParams;
import org.apache.solr.mcp.server.util.PromptNames;
import org.springaicommunity.mcp.annotation.McpArg;
@@ -113,6 +114,38 @@ public class SearchService {
* map.
*/
public static final String SORT_ORDER = "order";
+
+ /**
+ * Fragments of Solr's own error text that identify a failure we can
advise on.
+ *
+ * <p>
+ * These are matched rather than imported because they are produced by
+ * <em>solr-core</em>, which is not on this server's classpath — only
+ * {@code solr-solrj} is. Nothing couples them to Solr at compile time,
so
+ * {@code SearchServiceIntegrationTest} pins each one against a real
Solr
+ * server: if a future Solr rewords a message, that test fails rather
than the
+ * hint silently disappearing. Prefer a structured signal (see
+ * {@link SolrException#code()} below) whenever one exists.
+ */
+ static final String UNDEFINED_FIELD_TOKEN = "undefined field";
+ /** @see #UNDEFINED_FIELD_TOKEN */
+ static final String SORT_FIELD_NOT_FOUND_TOKEN = "field can't be found";
+ /** @see #UNDEFINED_FIELD_TOKEN */
+ static final String SYNTAX_ERROR_TOKEN = "syntaxerror";
+ /** @see #UNDEFINED_FIELD_TOKEN */
+ static final String CANNOT_PARSE_TOKEN = "cannot parse";
+
+ /**
+ * Remediation hint naming the {@code get-schema} tool; takes the
collection.
+ */
+ static final String GET_SCHEMA_HINT_FORMAT = ". Hint: call get-schema
on collection '%s' to see the fields that"
+ + " exist; schemaless collections often store values
under dynamic-suffix names such as name_s or price_d.";
+ /** Remediation hint for an unparseable {@code q}. */
+ static final String LUCENE_SYNTAX_HINT = ". Hint: the q parameter uses
Lucene query syntax; quote or escape"
+ + " special characters and check any {!...} local
params.";
+ /** Remediation hint naming the {@code list-collections} tool. */
+ static final String LIST_COLLECTIONS_HINT = ". Hint: call
list-collections to see available collections.";
+
private final SolrClient solrClient;
/**
@@ -306,7 +339,12 @@ public class SearchService {
solrQuery.setRows(rows);
}
- final QueryResponse queryResponse =
solrClient.query(collection, solrQuery);
+ final QueryResponse queryResponse;
+ try {
+ queryResponse = solrClient.query(collection, solrQuery);
+ } catch (SolrException e) {
+ throw withRemediationHint(e, collection);
+ }
// Add documents
final SolrDocumentList documents = queryResponse.getResults();
@@ -349,6 +387,41 @@ public class SearchService {
return new SolrQuery.SortClause(field, parsed);
}
+ /**
+ * Wraps common Solr query failures with a next-step hint. MCP clients
receive
+ * the exception message as the tool error, so naming the follow-up
tool lets
+ * them self-correct instead of retrying blind.
+ *
+ * @param e
+ * the Solr exception raised by the query
+ * @param collection
+ * the collection that was queried
+ * @return an exception carrying the original message plus a
remediation hint,
+ * or the original exception when no hint applies
+ */
+ private static RuntimeException withRemediationHint(SolrException e,
String collection) {
+ final String message = String.valueOf(e.getMessage());
+
+ // An unknown collection is a 404 whose body is Solr's HTML
"not found" page,
+ // so SolrJ reports it as a mime-type mismatch and leaves
getMetadata() null.
+ // The status code is the only signal that survives; match it
rather than the
+ // message text, which mentions neither the collection nor
"404".
+ if (e.code() == SolrException.ErrorCode.NOT_FOUND.code) {
+ return new IllegalArgumentException(message +
LIST_COLLECTIONS_HINT, e);
+ }
+
+ // Everything below is a 400 carrying a generic SolrException,
indistinguishable
+ // except by Solr's message text.
+ final String lower = message.toLowerCase(Locale.ROOT);
+ if (lower.contains(UNDEFINED_FIELD_TOKEN) ||
lower.contains(SORT_FIELD_NOT_FOUND_TOKEN)) {
+ return new IllegalArgumentException(message +
GET_SCHEMA_HINT_FORMAT.formatted(collection), e);
+ }
+ if (lower.contains(SYNTAX_ERROR_TOKEN) ||
lower.contains(CANNOT_PARSE_TOKEN)) {
+ return new IllegalArgumentException(message +
LUCENE_SYNTAX_HINT, e);
+ }
+ return e;
+ }
+
/**
* MCP prompt that guides the client through translating a
natural-language
* question into a Solr query: inspect the schema, build the query, run
the
diff --git
a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
index 35e273c..bb3c842 100644
---
a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
@@ -26,6 +26,7 @@ import java.util.OptionalDouble;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.common.SolrException;
import org.apache.solr.mcp.server.TestcontainersConfiguration;
import org.apache.solr.mcp.server.indexing.IndexingService;
import org.junit.jupiter.api.BeforeEach;
@@ -184,6 +185,93 @@ class SearchServiceIntegrationTest {
assertEquals(10, documents.size());
}
+ /**
+ * Remediation hints classify Solr's error text, which this server
cannot see at
+ * compile time — the strings are produced by solr-core, and only
solr-solrj is
+ * on the classpath. These tests therefore provoke each failure on a
real Solr
+ * server and assert the hint survives: they are the only thing
standing between
+ * a reworded Solr message and a hint that silently stops appearing.
+ *
+ * <p>
+ * Each asserts on the hint constant, never on the token being matched
— an
+ * assertion routed through the same token the matcher uses would pass
even if
+ * Solr changed its wording, which is exactly the regression being
guarded.
+ */
+ @Test
+ void searchWithUndefinedFieldInQueryReturnsGetSchemaHint() {
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class, () -> searchService
+ .search(COLLECTION_NAME,
"definitely_not_a_field:value", null, null, null, null, null));
+
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted(COLLECTION_NAME)),
+ () -> "expected get-schema hint, got: " +
e.getMessage());
+ }
+
+ @Test
+ void searchWithUndefinedFieldInFilterQueryReturnsGetSchemaHint() {
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class, () -> searchService
+ .search(COLLECTION_NAME, "*:*",
List.of("definitely_not_a_field:value"), null, null, null, null));
+
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted(COLLECTION_NAME)),
+ () -> "expected get-schema hint, got: " +
e.getMessage());
+ }
+
+ /** Faceting words it differently: {@code undefined field: "name"}. */
+ @Test
+ void searchWithUndefinedFacetFieldReturnsGetSchemaHint() {
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class, () -> searchService
+ .search(COLLECTION_NAME, "*:*", null,
List.of("definitely_not_a_field"), null, null, null));
+
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted(COLLECTION_NAME)),
+ () -> "expected get-schema hint, got: " +
e.getMessage());
+ }
+
+ /**
+ * Sorting words it differently again: {@code sort param field can't be
found},
+ * which is why the matcher carries a second undefined-field token.
+ */
+ @Test
+ void searchWithUndefinedSortFieldReturnsGetSchemaHint() {
+ List<Map<String, String>> sort = List
+ .of(Map.of(SearchService.SORT_ITEM,
"definitely_not_a_field", SearchService.SORT_ORDER, "asc"));
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
+ () -> searchService.search(COLLECTION_NAME,
"*:*", null, null, sort, null, null));
+
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted(COLLECTION_NAME)),
+ () -> "expected get-schema hint, got: " +
e.getMessage());
+ }
+
+ @Test
+ void searchWithUnparseableQueryReturnsLuceneSyntaxHint() {
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
+ () -> searchService.search(COLLECTION_NAME,
"name:(", null, null, null, null, null));
+
assertTrue(e.getMessage().contains(SearchService.LUCENE_SYNTAX_HINT),
+ () -> "expected Lucene syntax hint, got: " +
e.getMessage());
+ }
+
+ /**
+ * An unknown collection is a 404 whose body is Solr's HTML page, so
the message
+ * names neither the collection nor "404" — it reads
+ * {@code Expected mime type in
+ * [application/json, text/plain] but got text/html}. Matched on
+ * {@link org.apache.solr.common.SolrException#code()} instead.
+ */
+ @Test
+ void searchOnMissingCollectionReturnsListCollectionsHint() {
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
+ () ->
searchService.search("definitely_not_a_collection", "*:*", null, null, null,
null, null));
+
assertTrue(e.getMessage().contains(SearchService.LIST_COLLECTIONS_HINT),
+ () -> "expected list-collections hint, got: " +
e.getMessage());
+ }
+
+ /**
+ * A Solr error we have no advice for must reach the client untouched.
Negative
+ * {@code rows} is structurally identical to the undefined-field
failures — a
+ * 400 carrying a generic {@code SolrException} — so it also pins that
the text
+ * matching is not over-broad.
+ */
+ @Test
+ void searchWithUnrecognizedSolrErrorPropagatesWithoutHint() {
+ SolrException e = assertThrows(SolrException.class,
+ () -> searchService.search(COLLECTION_NAME,
"*:*", null, null, null, null, -5));
+ assertFalse(e.getMessage().contains("Hint:"), () -> "expected
no hint, got: " + e.getMessage());
+ }
+
@Test
void testSearchWithQuery() throws SolrServerException, IOException {
SearchResponse result = searchService.search(COLLECTION_NAME,
"name:\"Game of Thrones\"", null, null, null,
diff --git
a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
index b0c6cd6..798348b 100644
--- a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
+++ b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceTest.java
@@ -32,6 +32,7 @@ import org.apache.solr.client.solrj.response.FacetField;
import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledInNativeImage;
@@ -47,6 +48,73 @@ class SearchServiceTest {
assertNotNull(localService);
}
+ /*
+ * These stub Solr's error text rather than observe it, so they can
only show
+ * that a matching message produces a hint — never that Solr still
emits such a
+ * message. The fixture strings below are verbatim samples captured
from a real
+ * Solr 9.9; SearchServiceIntegrationTest is what keeps them honest.
+ */
+
+ private static SearchService serviceThrowing(SolrException e) throws
Exception {
+ SolrClient mockClient = mock(SolrClient.class);
+ when(mockClient.query(eq("test_collection"),
any(SolrQuery.class))).thenThrow(e);
+ return new SearchService(mockClient);
+ }
+
+ @Test
+ void search_WithUndefinedField_ShouldHintGetSchema() throws Exception {
+ SearchService localService = serviceThrowing(
+ new
SolrException(SolrException.ErrorCode.BAD_REQUEST, "undefined field bogus"));
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
+ () -> localService.search("test_collection",
"bogus:x", null, null, null, null, null));
+ assertTrue(e.getMessage().contains("undefined field bogus"),
"original Solr message must be preserved");
+
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted("test_collection")));
+ }
+
+ @Test
+ void search_WithUndefinedSortField_ShouldHintGetSchema() throws
Exception {
+ SearchService localService = serviceThrowing(
+ new
SolrException(SolrException.ErrorCode.BAD_REQUEST, "sort param field can't be
found: bogus"));
+ List<Map<String, String>> sort = List
+ .of(Map.of(SearchService.SORT_ITEM, "bogus",
SearchService.SORT_ORDER, "asc"));
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
+ () -> localService.search("test_collection",
"*:*", null, null, sort, null, null));
+
assertTrue(e.getMessage().contains(SearchService.GET_SCHEMA_HINT_FORMAT.formatted("test_collection")));
+ }
+
+ @Test
+ void search_WithQuerySyntaxError_ShouldHintLuceneSyntax() throws
Exception {
+ SearchService localService = serviceThrowing(new
SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ "org.apache.solr.search.SyntaxError: Cannot
parse 'name:('"));
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
+ () -> localService.search("test_collection",
"name:(", null, null, null, null, null));
+
assertTrue(e.getMessage().contains(SearchService.LUCENE_SYNTAX_HINT));
+ }
+
+ /**
+ * A missing collection is matched on the 404 status, not the message —
Solr's
+ * 404 body is an HTML page, so SolrJ surfaces it as the mime-type
mismatch
+ * stubbed here, which mentions neither the collection nor "404".
+ */
+ @Test
+ void search_WithNotFoundStatus_ShouldHintListCollections() throws
Exception {
+ SearchService localService = serviceThrowing(new
SolrException(SolrException.ErrorCode.NOT_FOUND,
+ "Expected mime type in [application/json,
text/plain] but got text/html."));
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
+ () -> localService.search("test_collection",
"*:*", null, null, null, null, null));
+
assertTrue(e.getMessage().contains(SearchService.LIST_COLLECTIONS_HINT));
+ }
+
+ @Test
+ void search_WithUnrelatedSolrError_ShouldPropagateUnchanged() throws
Exception {
+ SearchService localService = serviceThrowing(
+ new
SolrException(SolrException.ErrorCode.SERVER_ERROR, "internal failure"));
+ SolrException e = assertThrows(SolrException.class,
+ () -> localService.search("test_collection",
null, null, null, null, null, null));
+ assertTrue(e.getMessage().contains("internal failure"));
+ assertFalse(e.getMessage().contains("Hint:"));
+ }
+
@Test
void search_WithNullQuery_ShouldDefaultToMatchAll() throws Exception {
SolrClient mockClient = mock(SolrClient.class);
