[FOR SHARING PURPOSES ONLY] RAG ingestion and chat pipelines

[dmartinol]

TODO list before marking the draft as ready:

• fix mypy static analysis
• fix pytest
• introduce factory for DocumentStore
• introduce factory for Retriever
• introduce factory for Document Embedders
• ilab data process should use the DoclingConverter instead of sdg DocumentChunker
• add taxonomy navigation for default ilab data ingest pipeline
• add unit tests
• integration tests (maybe a different PR)
• Integrate PaRAGon

Hi team! I’ve put together some initial code that can serve as a starting point for our discussion on the topic in subject.
Given the current limitations on updating the CLI interface in the current version, the approach leverages configuration options to enable the ingestion and chat pipelines without requiring new commands
Details below.

Note:
Since we're using a development version of instructlab-sdg package, the following steps are needed for now:

• Clone the instructlab/sdg repo
• Import the local package like pip install -e /path/to/sdg/repo

RAG document transformation and ingestion

• Transform user docs using available SDG modules (no need to define any qna.yaml knowledge document)
• Generate embeddings and ingest them in a configured DB

No configuration

Flags and env vars

Reflect the design document RAG ingestion and chat pipelines

Command

From pre-processed documents:

ilab data process --rag /path/to/pre-processsed/docs

Inclusing pre-processing pipeline:

ilab data process --rag --transform --transform-output /path/to/pre-processsed/docs /path/to/user/docs

Output in the configured DB. Supported DB: MilvusLite.

RAG Chat

Configuration

chat:
  rag:
    enable: false
    retriever:
      top_k: 20
      embedder:
        model_name: sentence-transformers/all-minilm-l6-v2
    document_store:
      type: milvuslite
      uri: embeddings.db
      collection_name: Ilab

Command

ilab model chat

Includes additional marker to display the activation status of the RAG function:

% ilab model chat                        
╭───────────────────────────────────────────────────────────────────────────── system ─────────────────────────────────────────────────────────────────────────────╮
│ Welcome to InstructLab Chat w/ GRANITE-7B-LAB-Q4_K_M.GGUF (type /h for help)                                                                                     │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
>>>                                                                                                                                                [RAG][S][default]

[cdoern]

left some lengthy comments. I know this is in a draft state but I figured I would get a review in. Many of my comments are concerns over config defaulting, reading, and how we are using these new RAG options.

Also, we will need e2e tests for this since it is a major change.

[cdoern]

I am pretty sure new requirements like these need to be run by a specific set of people.

[dmartinol]

could you please elaborate more on the "specific set of people" part? (for sure I miss something here)

[cdoern]

same here for new requirements

[cdoern]

This should be its own class with specific arguments we can validate and run through tests rather than a dictionary that can contain anything. If we are putting something directly into our config, we need to have control over validation.

if we were going to separate config yaml route, we could just blindly read it in, but I think this approach is better.

[dmartinol]

The way it's been designed follows the initial conversation we had 2 weeks ago, when we decided not to update the configuration to avoid compatibility issues in case we would have changed any of these commands, as their definition seemed "unstable".
I'd be happy if we instead decide to adopt the regular config definitions, as it is a more straightforward implementation.

[cdoern]

so we are implicitly passing these in?

the way we do this with config entries is map the nested classes to click.options where the defaults are funneled in via cls=clickext.ConfigOption. I would prefer to

1. make the config options a class not a dictionary
2. add associated flags to override in ilab model chat where the defaults use the above class ConfigOption to read defaults from the config

[dmartinol]

1. ok, as you also requested in another comment
2. sure, I'll use the regular design then

[cdoern]

will this wrapper apply these to any ilab command using the @rag_options? I would really prefer for ilab commands directly using rag and allowing users to configure RAG, that they have their own validated set of options that read from the config.

A second option I would be ok with, is if these rag_options use the cls=clickext.ConfigOption class to read their defaults from the config file.

[dmartinol]

will this wrapper apply these to any ilab command using the @rag_options?

This was meant to be used only in the chat command, where it makes sense to add these options.

I would really prefer for ilab commands directly using rag and allowing users to configure RAG, that they have their own validated set of options that read from the config

👍

[cdoern]

things like this for example should be reading from the config, we try not to specify defaults at this level

[cdoern]

yeah, this should have some sane defaults or else users will not know how to use this

[dmartinol]

@cdoern I already noticed the mypy issues detected in this file, but I was wondering what I had to do with them. Should I try to fix them even if I did not update this code (w/o introducing any side effect, of course) or can we skip these warnings?