Skip to content

feat: Llama index milvus rag #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 26 commits into from
Apr 28, 2025

Conversation

aevo98765
Copy link
Contributor

Description

This pull request adds a llama-index RAG implementation. This depends upon ollama local model hosting. The instructions of how to setup have been added.

Suggestions for future improvements:

  • Get docling to do the chunking.
  • Return sources from the search function so the retrieval sources can be displayed on the mcp client.

aevo98765 added 11 commits April 7, 2025 14:31
Signed-off-by: Ash Evans <ash.evans@ibm.com>
…he env vars

Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
@aevo98765 aevo98765 requested a review from PeterStaar-IBM April 8, 2025 16:32
@aevo98765 aevo98765 self-assigned this Apr 8, 2025
Copy link

mergify bot commented Apr 8, 2025

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

@PeterStaar-IBM PeterStaar-IBM added the enhancement New feature or request label Apr 9, 2025
@PeterStaar-IBM
Copy link
Contributor

Hi @aevo98765 , this is a good start. I see that you use the native markdown chunker from RAG. I think we can do better: @vagenas has been working on native chunker on the DoclingDocument and a lot of experience with llama-index. You can find a nice example here: rag_llamaindex.ipynb

  1. @vagenas : Can you have a look at export_docling_document_to_vector_db
  2. @aevo98765 : I dont see in the tools any ollama call. Why do we need the ollama and the llm if we dont use it in the tools?

@PeterStaar-IBM PeterStaar-IBM changed the title Llama index chromadb rag feat: Llama index chromadb rag Apr 10, 2025
@PeterStaar-IBM
Copy link
Contributor

@aevo98765 I dont see where we use the ollama llm. Do we need to define an llm, or can we simply get away with a simple chromadb?

Signed-off-by: Ash Evans <ash.evans@ibm.com>
… to docling node parser

Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
@aevo98765 aevo98765 changed the title feat: Llama index chromadb rag feat: Llama index milvus rag Apr 15, 2025
Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
@aevo98765
Copy link
Contributor Author

@PeterStaar-IBM Changed the code to use the docling native approach. I had to switch to Milvus to achieve this and store this in memory. ChromaDB wouldn't work. Ollama models are used under the hood in the LlamaIndex query engine. It is not explicit at the moment as it is set as a 'SETTING' which reads in like an environment variable for LlamaIndex to use. Hope this makes sense.

Copy link
Contributor

@ceberam ceberam left a comment

Choose a reason for hiding this comment

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

  • To frame this new feature within the purpose of docling-mcp: I would document it in README.md more clearly as an optional feature. We could have a separate section at the end (for instance, Applications) where we explain how docling-mcp can be leveraged in Agentic RAG applications (basically, what you wrote in terms of configuration and examples).
  • In connection with above, I would move the export_docling_document_to_vector_db and search_documents tools to another module. For instance, rag.py
  • In other docling repos, we leverage pydantic settings, since we think they offer some advantages (we can clearly define the options and type-hint them, validate them, leverage aliases, automatically manage nested variables...).

Later on, we can:

  • enable other LLM providers (e.g., watsonx.ai)
  • add more variables (e.g., chunker type and chunking options)

aevo98765 and others added 2 commits April 25, 2025 14:09
Signed-off-by: Ash Evans <70710356+aevo98765@users.noreply.github.com>
…ded at the end in a Applications section

Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
Signed-off-by: Ash Evans <ash.evans@ibm.com>
Copy link

codecov bot commented Apr 25, 2025

Codecov Report

Attention: Patch coverage is 20.68966% with 46 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
docling_mcp/tools/applications.py 0.00% 36 Missing ⚠️
docling_mcp/shared.py 63.15% 7 Missing ⚠️
docling_mcp/server.py 0.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@aevo98765
Copy link
Contributor Author

Also the tests are failing. This will be because the applications milvus RAG code is conditional. We would need to pass env vars to the test pipeline to fix this. Any suggesstions or advice here?

@ceberam
Copy link
Contributor

ceberam commented Apr 28, 2025

Also the tests are failing. This will be because the applications milvus RAG code is conditional. We would need to pass env vars to the test pipeline to fix this. Any suggesstions or advice here?

I have added a conditional import on commit dbde847
Since we will add more applications, I will create a new issue to add test environment variables in the test-package job of the GitHub CI/CD.

ceberam added 3 commits April 28, 2025 11:22
Signed-off-by: Cesar Berrospi Ramis <75900930+ceberam@users.noreply.github.com>
Signed-off-by: Cesar Berrospi Ramis <75900930+ceberam@users.noreply.github.com>
The application tools 'export_docling_document_to_vector_db' and 'search_documents' are
imported in 'server.py' depending on the environmental variables.

Signed-off-by: Cesar Berrospi Ramis <75900930+ceberam@users.noreply.github.com>
@ceberam ceberam force-pushed the llama-index-chromadb-rag branch from a34b0bd to dbde847 Compare April 28, 2025 09:22
@aevo98765
Copy link
Contributor Author

LGTM

Signed-off-by: Ash Evans <ash.evans@ibm.com>
@ceberam ceberam merged commit e21e336 into docling-project:main Apr 28, 2025
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants