feat(cli): env-selectable embedder — offline sentence-transformers path (closes #2) - #3
Merged
Conversation
Closes #2. make_memory only wired the LiteLLM default embedder, so every write-path CLI invocation (retain/index/sweep) required an OpenAI key + network. The library already ships an offline embedder (prospecta.embed.sentence_- transformers, 384-dim, no key) but nothing selected it from the CLI. cli/_common.py: - New _resolve_embedder() reads PROSPECTA_EMBEDDER (case-insensitive): unset/default/litellm → defaults.make_default_embedder() (unchanged behavior); sentence-transformers/st → embed.sentence_transformers(model) with PROSPECTA_EMBED_MODEL (default all-MiniLM-L6-v2). Unknown values raise SystemExit(1) with the valid set — no silent fallback, since a wrong-embedder mismatch corrupts a bank's vector space. - make_memory now resolves embed and llm INDEPENDENTLY. Previously llm resolved first inside the same try, so a missing litellm aborted before embed. Decoupled: an offline embedder works even with litellm absent. Paired with `retain --index-text "..."` (P4, skips LLM index_text gen), this is a fully offline, keyless retain. No breaking change — default behavior is identical when PROSPECTA_EMBEDDER is unset. tests/test_cli.py: 5 tests — default→litellm, explicit litellm, st routes to the offline factory with default + custom model, unknown errors loud. README: "Offline / keyless retain" quickstart section + the dim-must-match caveat (a 384-dim sentence-transformers bank is not interchangeable with a 1536-dim OpenAI bank). Verified end-to-end against a live substrate: created a dim-384 bank, PROSPECTA_EMBEDDER=sentence-transformers retain with --index-text landed a real document (2 memory_items, dim=384, llm_generated=false) with no API key — the sentence-transformers model encoded the vectors locally. 24 tests pass (test_cli + test_embed_helpers).
There was a problem hiding this comment.
Pull request overview
Adds an environment-selectable embedder to the CLI so write-path commands can run fully offline (sentence-transformers) without requiring LiteLLM/OpenAI, aligning CLI behavior with the already-shipped offline embed helper and unblocking keyless dev/CI flows.
Changes:
- Introduces
_resolve_embedder()inprospecta.cli._commonto select an embedder viaPROSPECTA_EMBEDDER(+PROSPECTA_EMBED_MODELfor sentence-transformers). - Updates
make_memoryto resolveembedandllmindependently so offline embedding can work even when LiteLLM isn’t installed. - Adds CLI tests for embedder selection and documents an “Offline / keyless retain” workflow in the README.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
prospecta/cli/_common.py |
Adds env-based embedder resolution and decouples embedder vs LLM construction in make_memory. |
tests/test_cli.py |
Adds coverage for default/litellm/sentence-transformers embedder resolution and unknown-value failure. |
README.md |
Documents the offline/keyless retain path and dimensionality caveat. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses both Copilot review comments on #3. 1. make_memory swallowed ImportError from _resolve_embedder, hiding the actionable install hint when a user selects PROSPECTA_EMBEDDER= sentence-transformers but hasn't installed the extra — it devolved into a confusing downstream RuntimeError about Memory(embed=...). Now the offline branch catches its own ImportError and re-raises SystemExit(1) with the factory's install hint. The default/litellm path keeps its soft-fail (stats/config must work without litellm), so the behavior is asymmetric by design and documented as such. 2. The unknown-value error message omitted the `st` alias the resolver actually accepts, disagreeing with the README. Now reads: "valid: default | litellm | sentence-transformers (alias: st)". tests: +4 — unknown lists st alias; explicit-st-missing fails loud with the install hint; make_memory propagates that SystemExit (doesn't swallow it). Re-verified live: happy-path offline retain still lands (doc e02f2385, dim-384, no key); unknown value prints the corrected message.
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.
Closes #2.
What
make_memoryonly wired the LiteLLM default embedder, so every write-path CLI invocation (retain/index/sweep) required an OpenAI key + network. The library already ships an offline embedder (prospecta.embed.sentence_transformers, 384-dim, no key) but nothing selected it from the CLI.Changes
cli/_common.py— new_resolve_embedder()readsPROSPECTA_EMBEDDER(case-insensitive): unset/default/litellm→ existingdefaults.make_default_embedder()(unchanged);sentence-transformers/st→embed.sentence_transformers(model)withPROSPECTA_EMBED_MODEL(defaultall-MiniLM-L6-v2). Unknown values raiseSystemExit(1)with the valid set — no silent fallback, since a wrong-embedder mismatch corrupts a bank's vector space.make_memorynow resolvesembedandllmindependently. Previouslyllmresolved first inside the sametry, so a missing litellm aborted beforeembed. Decoupled — an offline embedder works even when litellm is absent. Paired withretain --index-text "..."(P4, skips LLM index_text generation), this is a fully offline, keyless retain.tests/test_cli.py— 5 tests: default→litellm, explicit litellm,stroutes to the offline factory with default + custom model, unknown errors loud.No breaking change — default behavior is identical when
PROSPECTA_EMBEDDERis unset.Verification
Verified end-to-end against a live substrate, no API key:
Substrate confirms: source
issue-2-verification, 2 memory_items, dim=384 (sentence-transformers, not the dim-32 hash),llm_generated=false(caller-supplied index_text, no LLM call). The sentence-transformers model encoded the vectors locally.24 tests pass (
test_cli+test_embed_helpers).Why this is filed from the prospecta-tui side
The TUI manual-ops surface (#1) shells out to
prospecta retainfor its retain form. Without an offline embedder, that path can't be live-verified without provisioning an OpenAI key + dim-1536 bank. This unblocks end-to-end verification of the TUI manual ops — and, more broadly, gives prospecta a keyless local-dev/CI retain path.⚒️ Forge