private-gpt icon indicating copy to clipboard operation
private-gpt copied to clipboard

feat: Context Template from settings for QueryDocs Allowing the text …

Open juan-m12i opened this issue 1 year ago • 5 comments

For the QueryDocs use case, allow to customise the template used to collate the context pulled from the vector database.

  • Added new config to Settings as a str? in rag
  • Added context_template as parameter in chat_engine, stream_chat, and chat. If context_template is None, we load it from the new settings (and if there is no setting, it will get the default from llama_index as it was the previous behaviour)

Github project link Discord thread

though process

juan-m12i avatar Dec 13 '23 11:12 juan-m12i

This addition seems similar to the recent work we've done around (default_)system_prompt in the sense that this context_template is a static value that will be applied to every queries.

I'll let @imartinez decide what is the best way forward

I see this kind of customization as a refinement of the default RAG pipeline, so I proposed to create a new rag section in settings for this - in the future it could host other straightforward customizations like the size of the chunk generated during ingestion.

My overall view is:

  • Allow some simple modifications to the default RAG pipeline through this new set of settings.
  • For more complex customizations, direct the users to fork the repo and modify the code of Services.

imartinez avatar Dec 16 '23 18:12 imartinez

Hi! I applied the requested changes.

The only difference from what @pabloogc mentioned is that I had to do: settings().rag.default_context_template instead of settings.rag.default_context_template, not sure if I should have done it differently, but it wouldn't work without parenthesis (and mypy would also complain)

juan-m12i avatar Dec 19 '23 20:12 juan-m12i

Hi, thanks!

I replicated the settings approach from LLMcomponent and Embedding component (there were some differences as the setting is used in a private function of the class, but I believe the approach I took -creating an internal variable- should work).

I removed context_template from the params and function callings because as requested (I guess following YAGNI). My thinking process was to leave this prepared in such a way that if someone would use the class (ui, api, any other entrypoint) it could pass the parameter and keep a consistent interface, but to enable that back is such a small change that it's all good).

Only drawback is that by removing that, the conditional is now a bit more verbose, as the default None is not assigned:

            if self.default_context_template is not None:
                context_template = self.default_context_template
            else:
                context_template = None

Alternatively we could consider this one lines, but it's less readable IMO context_template = self.default_context_template if self.default_context_template is not None else None

EDIT: I just realised that if rag.default_context_template is missing it will be converted to None in the Settings object, right? So I could just pass self.default_context_template when invoking _from_defaults:

return ContextChatEngine.from_defaults(
                system_prompt=system_prompt,
                retriever=vector_index_retriever,
                service_context=self.service_context,
                node_postprocessors=[
                    MetadataReplacementPostProcessor(target_metadata_key="window"),
                ],
                context_template=self.default_context_template,
            )

and remove the conditional altogether

juan-m12i avatar Dec 20 '23 15:12 juan-m12i

Stale pull request

github-actions[bot] avatar Jan 05 '24 05:01 github-actions[bot]

@juan-m12i could you update the branch to latest changes? I still think it is a valuable contribution!

imartinez avatar Mar 11 '24 22:03 imartinez