You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@solr.apache.org by "adityamparikh (via GitHub)" <gi...@apache.org> on 2026/09/12 16:39:26 UTC

[PR] feat(indexing)!: fold per-format inline tools into index-documents [solr-mcp]

adityamparikh opened a new pull request, #197:
URL: https://github.com/apache/solr-mcp/pull/197

### Summary
Replaces the four inline indexing tools (`index-json-documents`, `index-csv-documents`, `index-xml-documents`, `index-markdown-documents`) with one tool:

```
index-documents(collection, content, format)
```

`format` is required and accepts `json`, `csv`, `xml`, `markdown` or the alias `md`, case-insensitively. The four per-format methods stay as Java entry points; the new tool dispatches to them.

### Why one tool
- **Same shape as file ingestion.** #194 adds `index-file(collection, path, format)`. With this change inline and file indexing share one calling convention instead of one tool plus four differently named siblings.
- **Smaller tool catalog.** Tool schemas are re-sent every session; three near-identical schemas go away.
- **One home for guidance.** Schema-first, sanitisation and payload advice live in one description.
- **Explicit format, not sniffing.** Inline payloads have no filename, and CSV and Markdown are both plain text with no safe distinguishing prefix, so the model sets a four-value argument instead of the server guessing. `index-file` keeps extension inference because a filename is real signal.

### What changed
- `IndexingService`: new `@McpTool index-documents`; the per-format methods lose their `@McpTool`; `resolveIndexTool` becomes `normalizeFormat`; the `index-data` prompt tells the model to call `index-documents` with the matching `format`.
- README tool table and prompt row, THREAT_MODEL XML note, observability test README.
- Tests: `McpToolRegistrationTest` asserts `index-documents` is the only `index-*` tool and that all three arguments are required; `IndexingServiceTest` covers each format, the `md` alias, case and whitespace normalisation, and rejection of unknown or missing formats before Solr is touched; `McpClientIntegrationTestBase` and `SampleClient` call the new tool over MCP and assert the old names are gone.

### Breaking change
Clients and saved prompts that call the four per-format tools must call `index-documents` with a `format` argument. At 0.0.2-SNAPSHOT this is the cheapest point to make the rename.

### Not done here
A typed JSON `documents` array (native JSON in the tool call instead of an escaped string) would save tokens for JSON payloads. It needs a second parameter or a second tool and is left for a follow-up.

### Verification
`./gradlew build` on Java 25: 411 tests, 0 failures, 7 skipped (the OTLP suite, skipped on `main` too). All Testcontainers-based integration classes ran. Spotless applied.

### Related
- Conflicts trivially with #194 and #196, which edit the same tool descriptions; whichever merges second needs a small rebase.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_019zJ9WA8nNyA5Yxb8ueM7vV

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org

Re: [PR] feat(indexing)!: fold per-format inline tools into index-documents [solr-mcp]

Posted by "adityamparikh (via GitHub)" <gi...@apache.org>.

adityamparikh commented on PR #197:
URL: https://github.com/apache/solr-mcp/pull/197#issuecomment-5658284142

Closing in favour of keeping the per-format tools and optimising each signature for its format.

What changed my mind is a measurement, not the design. Indexing 61 shows through a live server took over two minutes; Solr and the server accounted for under a second of that. The rest was the model emitting the payload as tool-call arguments, and the cost differs sharply by wire shape: the same 61 documents are ~9,500 tokens as JSON escaped inside a string, ~8,300 as a native JSON array, ~5,100 as CSV text. One tool with a `format` string forces every format through the worst shape (a string), and the catalog saving it buys (a few hundred tokens once per session) is two orders of magnitude smaller than a single call's payload.

So instead, three small PRs against `main`: `index-json-documents` takes a typed `documents` array (no escaping, schema-validated); `index-markdown-documents` starts a new document at every YAML front-matter block so many documents are one call; the XML creator stops prefixing fields with the record element name. #196's guidance stays and gets one sharper sentence. The `index-file` idea is not coming back in any transport-specific form.