Skip to content

Add SentinelLocalIndex.from_texts() to build an index in one line - #35

Open
wxiao0421 wants to merge 2 commits into
feat/persist-corpus-and-deterministic-loadfrom
feat/index-from-texts
Open

Add SentinelLocalIndex.from_texts() to build an index in one line#35
wxiao0421 wants to merge 2 commits into
feat/persist-corpus-and-deterministic-loadfrom
feat/index-from-texts

Conversation

@wxiao0421

Copy link
Copy Markdown
Contributor

Stacked on #32 (→ #31). Base branch is feat/persist-corpus-and-deterministic-load.

This one branches off the stack rather than sitting on top of it. Its only real dependency is the _take_rows helper from #32, so basing it there lets it merge as soon as #32 lands, without waiting on #33 and #34.

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:

  • Forget normalize_embeddings=True → the similarity maths quietly returns wrong numbers.
  • Forget to pass the corpus → explanations degrade to row 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.

index = SentinelLocalIndex.from_texts(
    positive_texts=[...],
    negative_texts=[...],
    neg_to_pos_ratio=5.0,
    seed=42,
)

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:

built in one call: 3 pos / 3 neg
normalized: True
corpus kept: True

explanation names real sentences, not row numbers:
   [+] 'they are invading our nation'
   [+] 'our people are being replaced'

after save+load, corpus survives: True

Implementation details worth reviewing

  • Keeps both return values of get_sentence_transformer_and_scaling_fn. Dropping scale_fn is a silent scoring bug for models like E5.
  • Hoists the encoding defaults into DEFAULT_ENCODING_KWARGS, now used by both the constructor and from_texts, rather than writing normalize_embeddings=True in a second place. Two copies of a default eventually disagree, and this one disagreeing would be invisible.
  • Always passes the corpus through — half the point of the method.
  • Applies neg_to_pos_ratio via the shared _take_rows helper from Persist the corpus and make index loading deterministic #32, with the same private-torch.Generator seeding convention used everywhere else in the library.
  • Validates input before any encoding happens, including the bare-string case. A string is iterable, so positive_texts="some text" would otherwise be encoded one character at a time: confusing, slow, and entirely silent.

What breaks if this is wrong

  • If normalize_embeddings stopped 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.
  • If scale_fn were dropped, scores change silently for scaled models.
  • If the corpus were not carried through, explanations silently revert to the row-number behaviour that Persist the corpus and make index loading deterministic #32 exists to fix.

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.

  • One call produces a usable index that scores text
  • The corpus is kept automatically
  • Embeddings are unit-length by default (catches a silent normalization regression)
  • neg_to_pos_ratio downsamples and the surviving negatives keep their own text
  • Same seed → identical index
  • The result saves and reloads with explanations intact
  • Bare string instead of a list raises, for both arguments
  • Empty list raises, for both arguments
  • Non-positive ratio raises
  • pytest tests/ — 79 passed (67 on the base branch)
  • flake8 --config=.flake8 output byte-identical to base: zero new findings
  • docs/check_docs_sync.py reports in sync

The validation tests run without loading a model, since they raise before any encoding; the rest are marked integration in line with the existing suite.

Note on CI: .github/workflows/test.yml only triggers on pull requests targeting main, so stacked PRs show no checks. Run locally against Python 3.10.

wxiao0421 and others added 2 commits July 29, 2026 13:02
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
wxiao0421 force-pushed the feat/persist-corpus-and-deterministic-load branch from d64f37e to cb513d9 Compare July 29, 2026 20:07
@wxiao0421
wxiao0421 force-pushed the feat/index-from-texts branch from 983dd9b to d261206 Compare July 29, 2026 20:07
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.

1 participant