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

#128 and #125: RAG Improvements #129

Merged
merged 14 commits into from
Jan 29, 2025
Merged

#128 and #125: RAG Improvements #129

merged 14 commits into from
Jan 29, 2025

Conversation

zateutsch
Copy link
Contributor

@zateutsch zateutsch commented Jan 25, 2025

fixes #125
fixes #128

Summary of changes:

  • Removed RAG indexing progress indicator as it wasn't updating the UI properly, only takes 5-10 seconds, and complicates the sample to have it in there
  • Added a "Select PDF" button to the prompting view to allow users to select a new PDF without reloading the sample
  • Moved key functions up in the source so that viewing the code shows you IndexPDF() and DoRAG() towards the top of the file.

@zateutsch zateutsch requested a review from nmetulev January 25, 2025 22:26
@nmetulev
Copy link
Member

On my machine, I indexed war and piece and it took a minute to index. Without the progress tracking, I wasn't sure if the sample is working. I think we need some progress tracking in there - can you think of a way to add it that works?

@nmetulev
Copy link
Member

Also, the close button on the pdf overlaps the select pdf button
image

Copy link
Member

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

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

left comments on the pr

@zateutsch zateutsch requested a review from nmetulev January 28, 2025 22:26
@niels9001
Copy link
Contributor

niels9001 commented Jan 29, 2025

Thought: should we have the PDF reader thing inline with the text so it's side-by-side?

Something like this, but then with clickable page numbers to navigate to the pages?
image

@nmetulev
Copy link
Member

nmetulev commented Jan 29, 2025

@niels9001, should we block this PR on adding the side by side, or can we add it another time?

@niels9001
Copy link
Contributor

@niels9001, should we block this PR on adding the side by side, or can we add it another time?

No let's get this in, we can add it later.

@nmetulev nmetulev merged commit 1c0ae00 into main Jan 29, 2025
3 checks passed
@nmetulev nmetulev deleted the zt/RAG-improvements branch January 29, 2025 20:10
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.

[FEATURE] RAG sample should allow new PDF select [BUG] RAG loading indicator doesn't update
3 participants