Skip to content

fix: expand chunk IDs in remove() when chunking is enabled #13

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.

Sign up for GitHub

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

Merged
harlan-zw merged 3 commits into from
Mar 30, 2026
+42 −1
Merged

fix: expand chunk IDs in remove() when chunking is enabled #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d4918e6
fix: expand chunk IDs in remove() when chunking is enabled
oritwoen Mar 30, 2026
b82d62d
fix: fail fast when chunked remove() has no listIds() driver
oritwoen Mar 30, 2026
7ea07ae
Merge branch 'main' into fix/remove-with-chunking
harlan-zw Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/db/sqlite-fts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,11 @@ export async function sqliteFts(config: SqliteFtsConfig = {}): Promise<SearchPro
}
},

async listIds() {
const rows = db.prepare('SELECT id FROM documents_meta').all() as { id: string }[]
return rows.map(r => r.id)
},

async clear() {
db.exec('DELETE FROM documents_fts')
db.exec('DELETE FROM documents_meta')
Expand Down
18 changes: 17 additions & 1 deletion src/retriv.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,25 @@ export async function createRetriv(options: RetrivOptions): Promise<SearchProvid
},

async remove(ids: string[]) {
let removeIds = ids
if (chunker) {
const lister = drivers.find(d => d.listIds)
if (!lister)
throw new Error('remove() with chunking requires a driver that implements listIds()')
const allIds = await lister.listIds!()
Comment on lines +223 to +228
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 30, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Handle empty remove() input as a fast no-op.

At Line 224, chunk-aware expansion runs even when ids is empty. With chunking enabled and no listIds() driver, remove([]) throws instead of returning a no-op result.

Suggested fix 
    async remove(ids: string[]) {
+      if (ids.length === 0)
+        return { count: 0 }
       let removeIds = ids
       if (chunker) {
         const lister = drivers.find(d => d.listIds)
         if (!lister)
           throw new Error('remove() with chunking requires a driver that implements listIds()')
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/retriv.ts` around lines 223 - 228, If remove([]) is called with an empty
ids array, avoid running the chunker expansion and potential listIds lookup; add
an early no-op return when ids is empty at the start of remove() (before the
chunker logic). Specifically, in remove() check if ids is falsy or length === 0
and return immediately so the subsequent code that creates lister via
drivers.find(d => d.listIds) and calls lister.listIds() is skipped, preventing
the error when no listIds-capable driver exists.

const idSet = new Set(ids)
removeIds = allIds.filter((id) => {
if (idSet.has(id))
return true
const sep = id.indexOf('#chunk-')
return sep >= 0 && idSet.has(id.substring(0, sep))
})
}
const results = await Promise.all(
drivers.filter(d => d.remove).map(d => d.remove!(ids)),
drivers.filter(d => d.remove).map(d => d.remove!(removeIds)),
)
for (const id of ids)
parentDocs.delete(id)
return { count: results[0]?.count ?? 0 }
},

Expand Down
20 changes: 20 additions & 0 deletions test/retriv.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,26 @@ describe('createRetriv', () => {
await retriv.close?.()
})

it('removes chunked documents by parent id', async () => {
const retriv = await createRetriv({
driver: sqliteFts({ path: ':memory:' }),
chunking: markdownChunker({ chunkSize: 20, chunkOverlap: 0 }),
})

await retriv.index([
{ id: 'doc1', content: 'First part.\n\nSecond part.\n\nThird part.' },
{ id: 'doc2', content: 'Keep this.\n\nStill here.\n\nNot removed.' },
])

await retriv.remove?.(['doc1'])

const doc1Results = await retriv.search('part', { limit: 10 })
const doc2Results = await retriv.search('keep', { limit: 10 })

expect(doc1Results.filter(r => r.id.startsWith('doc1'))).toHaveLength(0)
expect(doc2Results.length).toBeGreaterThanOrEqual(1)
})

it('extracts snippets with highlights', async () => {
const retriv = await createRetriv({
driver: sqliteFts({ path: ':memory:' }),
Expand Down
Loading