You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "shahzadarain (via GitHub)" <gi...@apache.org> on 2026/06/11 20:44:25 UTC
[PR] feat(indexing): add markdown document indexing [solr-mcp]
shahzadarain opened a new pull request, #144:
URL: https://github.com/apache/solr-mcp/pull/144
## Summary
Adds an `index-markdown-documents` MCP tool that indexes markdown content into Solr, alongside the existing JSON/CSV/XML tools — extracting searchable structure rather than a flat text blob.
Closes #69
## Design
A new `MarkdownDocumentCreator` follows the existing strategy pattern in `indexing/documentcreator/`, parsing with **CommonMark-Java 0.28.0** plus its YAML front matter extension (lightweight, reflection-free, so GraalVM native-image safe — no extra hints needed).
Field extraction:
| Field | Source |
|---|---|
| front matter entries | each entry becomes a field (names sanitized via `FieldNameSanitizer`; block- and flow-style lists become multi-valued fields) |
| `id` | front matter `id` if present, otherwise SHA-256 of the input |
| `title` | front matter `title`, else first level-1 heading |
| `headings` | multi-valued field with every heading text (the document outline) |
| `content` | plain text body, front matter excluded |
The `index-data` MCP prompt also accepts `markdown`/`md` formats.
## Maintainer feedback from #69, addressed
**Stable id / idempotency:** when front matter supplies no `id`, the document id is derived deterministically from a SHA-256 hash of the input, so re-indexing identical markdown overwrites the same document — matching the tool's `idempotentHint`. Since LLM-driven conversion of other formats to markdown is non-deterministic, the tool description also instructs clients to supply a stable front matter `id` when indexing converted content.
**Tool steering:** the tool description now reads: *"Do NOT use for JSON/CSV/XML input; use index-json-documents, index-csv-documents, or index-xml-documents instead. Only convert source content to markdown when there is no dedicated tool for the source format, and supply a stable 'id' in the YAML front matter when doing so."*
## Testing
- `MarkdownIndexingTest` — 10 unit tests: front matter extraction, title resolution (front matter wins over H1), heading collection, plain-text body with formatting stripped, field name sanitization, flow- and block-style YAML lists, front matter id, content-hash id stability, empty-input rejection
- Prompt-path tests for `markdown` and `md` formats in `IndexingServiceTest`
- MCP round-trip added to `McpClientIntegrationTestBase`: index markdown via the tool, then find it by front matter id (runs across all transport × runtime combinations)
- Tool registered in `listToolsReturnsExpectedTools` and behavior hints asserted in `toolsExposeBehaviorHints`
- `./gradlew spotlessApply` clean; unit tests green locally on JDK 25
Docs updated: README tool table, AGENTS.md format/creator lists.
--
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): add markdown document indexing [solr-mcp]
Posted by "adityamparikh (via GitHub)" <gi...@apache.org>.
adityamparikh commented on PR #144:
URL: https://github.com/apache/solr-mcp/pull/144#issuecomment-5341583989
Follow-up: this drifted back into conflict after shahzadarain/solr-mcp#1 was merged on 31 July, because more has landed on `main` since.
I've opened a fresh merge PR — shahzadarain/solr-mcp#3 — against `feature/markdown-indexing`. Merging it makes this PR mergeable again; it's merge-only, no changes to @shahzadarain's markdown code.
Only one real conflict this time, in `AGENTS.md`, where both sides edited the same two lines for different reasons: this branch adds markdown to the `IndexingService` line, while #178 corrected `metadata/` to `collection/` on the `CollectionService` line. I kept both correct halves rather than taking one side.
Post-merge checks, since a clean auto-merge is what hides this class of breakage:
- `MarkdownDocumentCreator` is still injected into and delegated to by `IndexingDocumentCreator` (that file auto-merged).
- No new record types, so no `SolrNativeHints` entry is needed — `index-markdown-documents` returns `String`. Worth stating explicitly now that `main` carries `SolrNativeHintsTest`, since a missing hint would otherwise only fail in a full GraalVM build.
- JDK 25: `compileJava`, `compileTestJava`, `spotlessCheck` all clean.
My approval on the code stands.
--
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): add markdown document indexing [solr-mcp]
Posted by "adityamparikh (via GitHub)" <gi...@apache.org>.
adityamparikh commented on PR #144:
URL: https://github.com/apache/solr-mcp/pull/144#issuecomment-5512221121
Hi @shahzadarain — this one has drifted out of mergeable state against `main`, so
GitHub currently reports it as conflicting. Would you mind rebasing?
The good news is that it's a very small fix. **The only conflict is one hunk in
`AGENTS.md`** — none of the Java changes conflict at all:
```
- **IndexingService** (`indexing/`) - Document indexing supporting JSON, CSV, XML formats
- **CollectionService** (`collection/`) - List collections, get stats, health checks <-- main
- **CollectionService** (`metadata/`) - List collections, get stats, health checks <-- this PR
```
The cause is that `CollectionService` moved package from `metadata/` to `collection/`
on `main` after this branch was cut. The resolution is to keep `main`'s `collection/`
path and carry over your "and markdown" addition to the `IndexingService` line:
```
- **IndexingService** (`indexing/`) - Document indexing supporting JSON, CSV, XML, and markdown formats
- **CollectionService** (`collection/`) - List collections, get stats, health checks
```
I resolved it that way locally and ran the full CI suite on the merge result —
`classes testClasses spotlessCheck`, `unitTest` and `integrationTest` **all pass**, so
resolving that single hunk should be all that's needed. The branch is 17 commits
behind `main` (merge base `582490b`).
--
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): add markdown document indexing [solr-mcp]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh merged PR #144:
URL: https://github.com/apache/solr-mcp/pull/144
--
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): add markdown document indexing [solr-mcp]
Posted by "adityamparikh (via GitHub)" <gi...@apache.org>.
adityamparikh commented on PR #144:
URL: https://github.com/apache/solr-mcp/pull/144#issuecomment-4995475683
Hi @shahzadarain — this PR currently has merge conflicts with `main`: the README tools section was restructured by #151, and #146 added constructor Javadoc on the same lines this branch touches in `IndexingDocumentCreator`.
I've resolved the conflicts by merging current `main` into your branch and opened shahzadarain/solr-mcp#1 against `feature/markdown-indexing`. If you merge that, this PR will update automatically and become mergeable again. The merge result builds clean (`./gradlew build`, 328 tests passing, including the MCP stdio integration test that exercises `index-markdown-documents` end to end).
Of course, feel free to resolve the conflicts your own way instead if you'd prefer different choices — happy to close my PR in that case.
--
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): add markdown document indexing [solr-mcp]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #144:
URL: https://github.com/apache/solr-mcp/pull/144#issuecomment-5635686927
I love it......! Arguably this logic ought to live in the Solr Extraction module versus in a MCP tool, but then, that requires you to have neabled Solr Extraction, which is something you have to do at `bin/solr start` time ;-(. And the world is more and more markdown, so this is awesome.....
Bonus points if you wanted to enhance the Solr Extraction library integration with Tika 4 to do the same thing ;-)......
--
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): add markdown document indexing [solr-mcp]
Posted by "adityamparikh (via GitHub)" <gi...@apache.org>.
adityamparikh commented on PR #144:
URL: https://github.com/apache/solr-mcp/pull/144#issuecomment-5135075452
Hi @shahzadarain — friendly follow-up on the merge conflicts here. The resolution PR on your fork, shahzadarain/solr-mcp#1, is still open: merging it into `feature/markdown-indexing` will make this PR mergeable again. It is still current — only one docs-only commit (#167) has landed on `main` since it was prepared.
If you would rather resolve the conflicts yourself (e.g. by merging upstream `main` into your branch locally), that works just as well. Happy to help if anything is unclear!
--
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): add markdown document indexing [solr-mcp]
Posted by "adityamparikh (via GitHub)" <gi...@apache.org>.
adityamparikh commented on PR #144:
URL: https://github.com/apache/solr-mcp/pull/144#issuecomment-5357777772
Reviewed as part of a sweep over the open PRs. The markdown support is thoughtfully done — the
input size guard, front-matter extraction with a first-`h1` title fallback, and especially the
tool description that routes callers *away* to `index-json-documents` / `-csv` / `-xml` and tells
them only to convert when no dedicated tool exists. Negative routing like that is exactly what
stops an LLM reaching for the wrong tool.
**Blocking: the branch is currently `CONFLICTING` and can't merge.** It also overlaps #105, which
adds a shared `SolrDocumentCreator.requireContent(content, format)` helper for the null/blank
check each creator was doing separately. Worth rebasing *after* #105 lands and reusing that helper
rather than keeping the local check in `createSchemalessDocumentsFromMarkdown`, so the error
message stays identical across all four formats.
**Worth a second look: the content-hash id makes edits additive.** `contentHash` is a SHA-256 of
the entire markdown body, so changing a single character produces a different `id`. Re-indexing an
edited document therefore creates a *second* document rather than updating the first, and the
stale copy stays in the index. For a docs collection re-indexed on every commit, that grows
without bound and silently returns outdated content alongside current content.
The tool description does say to supply a stable front-matter `id`, but only in the
format-conversion case. Since the hash is what makes re-indexing idempotent *only for byte-identical
input*, I'd widen that guidance to say a front-matter `id` is required for any document that will
be re-indexed, and state plainly that without one, edits accumulate rather than replace. The
current wording reads as an optimisation rather than a correctness requirement.
Two minor things:
- `create` measures the input with `markdown.getBytes(UTF_8).length`, which allocates a full byte
copy of a potentially 10 MB string just to size it. A `markdown.length() > MAX_INPUT_SIZE_BYTES`
pre-check is free — UTF-8 bytes can never be fewer than chars — and only needs the exact
computation when that passes.
- `create` returns `List.of()` for blank input, but `createSchemalessDocumentsFromMarkdown` throws
on blank first, so that branch is unreachable through the tool. Either drop it or add a
direct-caller test so it isn't dead code.
The id semantics are the only thing I'd call out as needing a decision rather than a tweak.
--
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): add markdown document indexing [solr-mcp]
Posted by "adityamparikh (via GitHub)" <gi...@apache.org>.
adityamparikh commented on PR #144:
URL: https://github.com/apache/solr-mcp/pull/144#issuecomment-5357868986
I rebased this onto current `main` to check how bad the conflict was — it's small, and the result
builds clean. Pushed to my fork in case it's useful:
[`adityamparikh:rebase-offer/pr144-markdown-indexing`](https://github.com/adityamparikh/solr-mcp/tree/rebase-offer/pr144-markdown-indexing)
```bash
git remote add aditya https://github.com/adityamparikh/solr-mcp.git
git fetch aditya
git reset --hard aditya/rebase-offer/pr144-markdown-indexing
```
Only if you want it — no need to take it, and I haven't touched this PR. **387 tests, 0 failures**,
with all 11 markdown tests passing.
Three conflicts, and I made a judgement call on two that you should check:
**`IndexingDocumentCreator.java`** — mechanical. `main` added a constructor javadoc while this
branch added the `markdownDocumentCreator` field. Kept both, and documented the new
`@param markdownDocumentCreator` since the constructor signature changed here.
**`AGENTS.md`** — this branch listed `CollectionService (metadata/)`, but the package is
`collection/`. Took `main`'s path and kept your markdown addition to the `IndexingService` line.
**`README.md`** — this is the one to review. Your side restructured the flat tool table into
`### Indexing` / `### Collections` sections; `main` meanwhile reworded the `create-collection` row.
I resolved conservatively — kept `main`'s flat table and added only the
`index-markdown-documents` row — on the grounds that a table restructure is a separate change from
markdown support and shouldn't ride in on a rebase. **If the sectioning was deliberate, override
me**; it's a reasonable improvement, just worth being its own decision.
The id-semantics question from my earlier comment is unaffected by any of this and still worth a
look.
--
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