docs: add embedding options capability matrix, fix macOS socket drift - #522
Conversation
Consolidates the scattered embedding-backend guidance (configuration.md, native/local-inference/README.md, ADR-0006/0016, the native-embedder spike) into one page: docs/embedding-options.md. Covers the Swift CoreML sidecar, Infinity, Ollama, vLLM/TEI, and hosted OpenAI-compatible providers, each labeled with its actual verification status per FR-011. Also corrects the drift flagged in the issue: docs/configuration.md never mentioned that the Electron liminis app binds its own per-workspace socket rather than sharing /tmp/liminis-inference.sock with a bare binary. Both docs now state this explicitly. Links the new page from README.md, docs/index.md, and configuration.md's Embedder sidecar section, registers it in the site's page order and the llms-full.txt bundle, and regenerates that bundle. Closes #504
|
🏭 Fabrik — stage: Implement No existing PR — I'll emit the Used 55/100 turns, 5.2M input (100 raw + 5.1M cache-read + 104k cache-write) / 21k output tokens. |
|
🏭 Fabrik — stage: Review Review FindingsReviewed Verified
FixedNone — no issues found. Working tree is already clean and pushed; no changes were needed. BlockingNone. This is a documentation-only PR whose every factual claim I traced back to its cited source and found accurate, every link I verified resolves (both via anchor-slug computation and an actual site build + link checker run), and CI is green. Ready for human review. Reviewed the embedding-options documentation PR: verified all 12 functional requirements against source docs, confirmed every internal link/anchor resolves (via computed slugs and an actual Used 43/100 turns, 3.3M input (78 raw + 3.2M cache-read + 87k cache-write) / 11k output tokens. |
![handarbeit-pruefer[bot]](https://avatars.githubusercontent.com/in/4408765?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the new docs/embedding-options.md capability matrix, its cross-links from README.md/docs/index.md/docs/configuration.md, the socket-path clarification added to docs/configuration.md, and the site plumbing (scripts/generate-docs-llms-full.sh's ORDERED list, site/src/pages-order.mjs, and the regenerated docs/llms-full.txt, which I confirmed reproduces byte-for-byte via bash scripts/generate-docs-llms-full.sh). All internal anchors referenced from the new page (configuration.md#embedder-sidecar, #http-transport-..., #mcp-client-config-recipes, #switching-an-existing-workspaces-embedder, #extractor-local-or-hosted, operations.md#knowledge_status-health-fields) resolve against existing headers, and the ADR/spike files it links exist. One factual-accuracy issue: the Swift CoreML sidecar row's "Verification" cell cites verify-embedding-parity.py as documented "in native/local-inference/README.md," but that README never mentions the script (no "Tests" section reference, no cosine-parity claim) — the citation doesn't back the claim it's attached to, which is notable given this PR's stated goal (FR-011) is to label verification status accurately rather than assert it uniformly.
Note: this sandbox's git history is a shallow, parent-less clone of the PR head commit, so I could not run git diff main...HEAD; I instead read the changed/new files directly and cross-checked their claims against the rest of the repository's current state.
… matrix The Swift CoreML sidecar row cited verify-embedding-parity.py as documented in native/local-inference/README.md, but the README never mentions that script. Point the citation directly at the script file instead, and note it runs manually rather than via CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
🏭 Fabrik — stage: Review (review feedback addressed) Thread resolved. No issue body changes are needed — this was a citation-accuracy fix within the PR, not a spec change. SummaryAddressed the review finding from Threads addressed:
Resolved 1 review thread(s) across 2 comment(s). |
|
🏭 Fabrik — stage: Validate All 6 CI checks pass. Validation is complete — every requirement verified, docs site builds and all links resolve, PR is clean and mergeable. Validation ReportRequirements: 12/12 passed
Test Suite: N/A (documentation-only change)
Regressions: None detectedNo code changes; diff against Branch State
Issues FoundNone. Verdict: READY TO MERGEValidation passed. PR #522 mergeable: MERGEABLE, mergeStateStatus: CLEAN. All 12 functional requirements verified against source docs, docs site builds cleanly with all 377 internal links resolving, llms-full.txt regeneration is drift-free, and all 6 CI checks pass. No issues found. Used 31/100 turns, 1.7M input (40 raw + 1.6M cache-read + 77k cache-write) / 5k output tokens. |
There was a problem hiding this comment.
Reviewed the full diff for PR #522 (docs-only): the new docs/embedding-options.md capability matrix, the socket-drift fix in docs/configuration.md, cross-links from README.md/docs/index.md, and the site-plumbing/llms-full.txt regeneration.
Verified independently: verify-embedding-parity.py's docstring matches its cited description and the prior review thread's citation issue is correctly fixed in the current commit (README no longer claims the script is documented in native/local-inference/README.md); every ADR/spike/anchor link referenced from the new page resolves against the actual file contents (docs/spikes/native-embedder-2026-05.md's candle-NO-GO/ort-GO-with-caveats verdict, docs/operations.md's knowledge_status health-fields anchor, docs/configuration.md's HTTP-transport/MCP-recipes/switching-embedder anchors); the Ollama and vLLM/TEI matrix rows' "listed with a concrete recipe" claims match actual recipe tables in configuration.md; and re-running scripts/generate-docs-llms-full.sh reproduces docs/llms-full.txt byte-for-byte. site/src/pages-order.mjs is confirmed as the single source of truth consumed by astro.config.mjs's nav, so the new page is wired in correctly there too.
No new defects found.
|
🏭 Fabrik merge-train — Landed via batch PR #523. |
1 participant
Closes #504
Summary
Adds
docs/embedding-options.md, a single reference page answering "what embedding backend should I run, on my platform, and what will it cost me?" — consolidating guidance previously scattered acrossdocs/configuration.md,native/local-inference/README.md, ADR-0006/0016, and the native-embedder spike.Key changes
docs/embedding-options.md): capability matrix (Swift CoreML sidecar, Infinity, Ollama, vLLM/TEI, hosted OpenAI-compatible), each row labeled with its actual verification status rather than presented as uniformly verified (FR-011). Includes sections on: the generic OpenAI-compatible extension point, no-bundled-cross-platform-option for Linux/Windows (linking the spike doc), dimension/schema fixing + model-identity invalidation (cross-referencing Recompute embeddings on WAL replay with a content-addressed cache, instead of replaying stored vectors #440), embedding-vs-extraction separation, and the macOS socket-path distinction.docs/configuration.md): adds the previously-missing statement that the Electronliminisapp's sidecar binds a separate per-workspace socket (<workspaceRoot>/.liminis/local-inference.sock), not the bare binary's/tmp/liminis-inference.sockauto-discovery default — these are never shared.README.md,docs/index.md's Reference pages list, anddocs/configuration.md's Embedder sidecar section all link to the new page.scripts/generate-docs-llms-full.sh'sORDEREDlist andsite/src/pages-order.mjs, and regenerateddocs/llms-full.txt.Renaming decision (per Plan)
The issue's proposed "Python sentence-transformers" row is presented as Infinity instead — it's the only Python/sentence-transformers-based server in
configuration.md's verified-shape list; the repo's only literal sentence-transformers service (embedder_server.py) spoke the now-superseded ADR-0006 wire format.How to test
docs/embedding-options.mdand confirm each capability-matrix row states platform, install cost, dimension, flag, offline status, auto-probe support, unreachable-at-startup behavior, and verification status.bash scripts/generate-docs-llms-full.shthengit diff --exit-code docs/llms-full.txt— should be clean (already regenerated in this PR).site/:pnpm install && pnpm build && pnpm links— verifies the new page builds and every internal link resolves (already run locally; 377 internal links checked, all resolve).ci.yml's docs-only fast path applies — no Rust job runs).