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

Feat/discord-forum retriever #27

Closed
wants to merge 9 commits into from
Closed

Conversation

amindadgar
Copy link
Member

Added the forum retriever

@amindadgar amindadgar linked an issue Dec 25, 2023 that may be closed by this pull request
@amindadgar amindadgar changed the title Feat/forum retriever Feat/discord-forum retriever Dec 25, 2023
@amindadgar amindadgar requested a review from TjitsevdM December 25, 2023 14:44
def query_discord_auto_filter(
community_id: str,
query: str,
similarity_top_k: int = 20,

Choose a reason for hiding this comment

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

Is this the top_k for the summaries or or the raw messages? Ideally, we have a separate parameter for each (k1 and k2 in card description under low level design). Also, is there a place where the d parameter is passed to a function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for asking, The similarity_top_k in for the query_discord_auto_filter is the k2, I'll see how the k1 can be adjusted in the secondary search (which would be in the function query_discord.
For the parameter d I was thinking to just include all the given date from metadata and not the time interval, which I'll fix and add based on the card.

Copy link
Member Author

Choose a reason for hiding this comment

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

The two missing (d and k1) parameters added and can be read from the .env file. Please let me know if you had any more questions.

Copy link

@TjitsevdM TjitsevdM left a comment

Choose a reason for hiding this comment

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

See added comments, the PR doesn't correspond fully to the card description but this is a good start!

@amindadgar amindadgar requested a review from TjitsevdM December 27, 2023 06:09
Copy link

@TjitsevdM TjitsevdM left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comments so quickly. Everything looks good now. The discourse implementation will be very similar. You can see if you prefer to finish that first or if you'd like to start with 3c instead

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.

feat: hivemind ETL discord summary retriever
3 participants