Skip to content
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

[WIP] make retriever page content just the content #7

Closed
wants to merge 1 commit into from

Conversation

lgabs
Copy link

@lgabs lgabs commented Jun 20, 2024

Addresses this dialog issue. This retriever is only used in lcel.py for now, for instance:

https://github.com/talkdai/dialog/blob/14e19f619155a710de4c350c0a78adc1942ae9d6/src/dialog/llm/agents/lcel.py#L60-L61

Users that already uses this project as-is, leaving questions in the question field and answers in the content csv field would have to be informed that this change will demand them to either join question/answer before into the content field or chose another format_docs function (which will be coerced into a runnable), in this case by using a plugin.

@vmesel
Copy link
Member

vmesel commented Jun 23, 2024

@lgabs coding wise it makes sense, but on the optimization side: don't we need the title/question for the content to be more relevant? Or is it added to the context as well?

@lgabs
Copy link
Author

lgabs commented Jun 28, 2024

Yes, the title and question are relevant together, but my point about flexibility is that the retrieval phase should only focus on the task of returning the content and their metadata (in which case title is a metadata). Then, when combining documents later in the chain, users are free to combine as they wish, for instance the following prompt template strings could all be valid prompt (compared to that example in [dialog])(https://github.com/talkdai/dialog/blob/main/src/dialog/llm/agents/default.py#L53):

"Question: {question}\nAnswer{content}"
"Question: {question}\nCategory:{category}\nAnswer{content}"
"Question: {question}\nCategory:{category}\nSubcategory:{subcategory}\nAnswer{content}"

@vmesel
Copy link
Member

vmesel commented Aug 14, 2024

Closing this PR until we have any news here.

@vmesel vmesel closed this Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants