Add SentinelLocalIndex.from_texts() to build an index in one line - #35
Open
wxiao0421 wants to merge 2 commits into
Open
Add SentinelLocalIndex.from_texts() to build an index in one line#35wxiao0421 wants to merge 2 commits into
wxiao0421 wants to merge 2 commits into
Conversation
4 tasks
Building an index is currently eight manual steps, documented in the README, and
two of them fail silently when skipped. Omit normalize_embeddings=True and the
similarity maths quietly returns wrong numbers; omit the corpus and explanations
degrade to row numbers. No crash, no warning, just subtly wrong results.
from_texts() does those steps inside the library, where they are tested, so a
caller cannot forget a step they never have to write:
index = SentinelLocalIndex.from_texts(
positive_texts=[...], negative_texts=[...], neg_to_pos_ratio=5.0, seed=42
)
Details worth reviewing:
- Keeps both return values of get_sentence_transformer_and_scaling_fn. Dropping
scale_fn silently changes scores for models like E5.
- Hoists the encoding defaults into DEFAULT_ENCODING_KWARGS, used by both the
constructor and from_texts, rather than writing normalize_embeddings=True in a
second place where the two copies could later disagree.
- Always passes the corpus through, which is half the point of the method.
- Applies neg_to_pos_ratio through the shared _take_rows helper with the same
local-torch.Generator seeding convention as the rest of the library.
- Validates input before any encoding happens, including a bare string passed
where a list is expected. A string is iterable, so without that check it would
be encoded one character at a time: confusing, slow and entirely silent.
README: "Creating a New Index" now leads with the one-liner, with the manual
recipe kept below under "Advanced: building an index manually" for people who
need custom encoding, and its silent traps called out explicitly.
Adds 12 tests: the built index is usable, the corpus is kept, embeddings are
normalized without asking, the ratio downsamples reproducibly and stays aligned,
the result survives save/load, and each input mistake raises.
Co-authored-by: Cursor <cursoragent@cursor.com>
Both this PR and the subsample() PR appended a test class to the end of test_sentinel_local_index.py, which produced a merge conflict between two adjacent parametrize blocks - the kind that resolves wrongly if skimmed. A separate file removes the conflict entirely and lets the stack merge in any order. No test content changed. Co-authored-by: Cursor <cursoragent@cursor.com>
wxiao0421
force-pushed
the
feat/persist-corpus-and-deterministic-load
branch
from
July 29, 2026 20:07
d64f37e to
cb513d9
Compare
wxiao0421
force-pushed
the
feat/index-from-texts
branch
from
July 29, 2026 20:07
983dd9b to
d261206
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Building an index today is eight manual steps (the README's "Creating a New Index" recipe), and two of them fail silently when skipped:
normalize_embeddings=True→ the similarity maths quietly returns wrong numbers.Neither raises. Neither warns. You just get subtly wrong or unexplainable results, and nothing tells you why. That is the real cost here, not keystrokes: doing these steps inside the library, where they are tested, means a caller cannot forget a step they never have to write.
Verified end to end — the resulting index scores correctly, its embeddings are unit-length, and its explanations name real sentences that survive a save/load round trip:
Implementation details worth reviewing
get_sentence_transformer_and_scaling_fn. Droppingscale_fnis a silent scoring bug for models like E5.DEFAULT_ENCODING_KWARGS, now used by both the constructor andfrom_texts, rather than writingnormalize_embeddings=Truein a second place. Two copies of a default eventually disagree, and this one disagreeing would be invisible.neg_to_pos_ratiovia the shared_take_rowshelper from Persist the corpus and make index loading deterministic #32, with the same private-torch.Generatorseeding convention used everywhere else in the library.positive_texts="some text"would otherwise be encoded one character at a time: confusing, slow, and entirely silent.What breaks if this is wrong
normalize_embeddingsstopped being applied, every index built this way would score incorrectly with no error anywhere. Asserted directly by checking the embedding norms are 1.0, rather than just checking the kwarg was passed.scale_fnwere dropped, scores change silently for scaled models.Nothing existing changes behaviour: this is a new classmethod, plus a refactor that moves an existing default into a named constant without altering its value.
Docs
README's Creating a New Index now leads with the one-liner. The manual recipe is kept directly below under "Advanced: building an index manually" for anyone who needs custom encoding, a different backend, or precomputed vectors — with its two silent traps called out explicitly so the advanced path is safer too.
Test plan
12 new tests.
neg_to_pos_ratiodownsamples and the surviving negatives keep their own textpytest tests/— 79 passed (67 on the base branch)flake8 --config=.flake8output byte-identical to base: zero new findingsdocs/check_docs_sync.pyreports in syncThe validation tests run without loading a model, since they raise before any encoding; the rest are marked
integrationin line with the existing suite.Note on CI:
.github/workflows/test.ymlonly triggers on pull requests targetingmain, so stacked PRs show no checks. Run locally against Python 3.10.