[PR #9] [MERGED] fix embedding sort comparator in generateEmbedding #10

New issue

Closed

opened 2026-10-15 13:28:03 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 13:28:03 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/SillyTavern/Extension-WebLLM/pull/9
Author: @Reithan
Created: 4/13/2026
Status: ✅ Merged
Merged: 4/19/2026
Merged by: @Cohee1207

Base: main ← Head: fix-embedding-sort-comparator

📝 Commits (1)

772ff0f fix embedding sort comparator in generateEmbedding

📊 Changes

2 files changed (+2 additions, -2 deletions)

View changed files

📝 dist/index.js (+1 -1)
📝 src/index.js (+1 -1)

📄 Description

Summary

generateEmbedding used .sort(x => x.index) as the comparator, which is a single-argument function. JavaScript's Array.prototype.sort expects a two-argument comparator (a, b) => number; passing a single-argument function causes V8 to use a.index as the comparison value while ignoring b, producing non-deterministic (but in practice consistently wrong) reordering of the embeddings array for batches larger than 1.
The correct comparator is .sort((a, b) => a.index - b.index).
This bug caused vector embeddings to be stored with the wrong text keys during ingest: the first item in each batch received the correct embedding, while items at positions 1–N received each other's embeddings in reversed order. Queries (always single-item batches) were unaffected, so retrieval produced incorrect rankings rather than no results.

Test plan

Ingest a Data Bank file with multiple chunks (> 1 chunk, so batchSize = 5 produces at least one full batch)
Query with text that should match a known chunk that is not the first chunk in its batch
Verify the correct chunk ranks above unrelated content

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/SillyTavern/Extension-WebLLM/pull/9 **Author:** [@Reithan](https://github.com/Reithan) **Created:** 4/13/2026 **Status:** ✅ Merged **Merged:** 4/19/2026 **Merged by:** [@Cohee1207](https://github.com/Cohee1207) **Base:** `main` ← **Head:** `fix-embedding-sort-comparator` --- ### 📝 Commits (1) - [`772ff0f`](https://github.com/SillyTavern/Extension-WebLLM/commit/772ff0f09053b49c278448f7af4748cc24c1dfcf) fix embedding sort comparator in generateEmbedding ### 📊 Changes **2 files changed** (+2 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `dist/index.js` (+1 -1) 📝 `src/index.js` (+1 -1) </details> ### 📄 Description ## Summary - `generateEmbedding` used `.sort(x => x.index)` as the comparator, which is a single-argument function. JavaScript's `Array.prototype.sort` expects a two-argument comparator `(a, b) => number`; passing a single-argument function causes V8 to use `a.index` as the comparison value while ignoring `b`, producing non-deterministic (but in practice consistently wrong) reordering of the embeddings array for batches larger than 1. - The correct comparator is `.sort((a, b) => a.index - b.index)`. - This bug caused vector embeddings to be stored with the wrong text keys during ingest: the first item in each batch received the correct embedding, while items at positions 1–N received each other's embeddings in reversed order. Queries (always single-item batches) were unaffected, so retrieval produced incorrect rankings rather than no results. ## Test plan - [x] Ingest a Data Bank file with multiple chunks (> 1 chunk, so `batchSize = 5` produces at least one full batch) - [x] Query with text that should match a known chunk that is **not** the first chunk in its batch - [x] Verify the correct chunk ranks above unrelated content --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-15 13:28:03 +00:00

closed this issue
added the
pull-request
label

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

SillyTavern/Extension-WebLLM#10

No description provided.

Rows
Columns