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

Bring back section chunking #322

Merged
merged 3 commits into from
Nov 27, 2024
Merged

Conversation

hugo-nl
Copy link
Collaborator

@hugo-nl hugo-nl commented Nov 27, 2024

🎸 Fix chroma range error bugg
🎸 improved reduce funciton to vision api

Fix #276

@hugo-nl hugo-nl requested a review from irony November 27, 2024 14:14
Copy link
Contributor

@irony irony left a comment

Choose a reason for hiding this comment

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

co-authored by me so approved :)

for (let i = 0; i < markdown.length; i += chunkSize - overlapSize) {
const chunk = markdown.slice(i, i + chunkSize).trim()
if (chunk.length > 0) chunks.push(chunk)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I liked this idea, but you likely have a reason for reverting back to the section-based chunking we used before.

const filename = tablesOnPage[0].filename
const markdown = await extractTextViaVisionAPI(
{ filename, name: `Tables from page ${page_idx}` },
context.slice(-3).join('\n')
lastPageMarkdown.slice(0, 5000)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why limit to 5000 characters here?

Does this mean that we send a larger context compared to before when we only sent the last 3 tables?

@irony irony merged commit 4be1433 into staging Nov 27, 2024
@irony irony deleted the feat/bring-back-chunking-per-section branch November 27, 2024 14:50
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.

RangeError: Invalid string length
3 participants