Feat/mxbai embeddings and hardening - #1
Merged
Conversation
Make the embedding model selectable from a registry where choosing a model
auto-derives its vector dimension and the services' Docker memory limits, and
upgrade the default to a stronger model. Also add lint/type tooling, harden
sitemap parsing, and add parity guards.
Embedding model registry
- config/models.yaml is the single source of truth (model -> dim, memory,
query/passage prompts). Default is mixedbread-ai/mxbai-embed-large-v1 (1024d),
a quality upgrade over bge-small (384d).
- `make configure MODEL=<name>` resolves the selection into .env
(EMBEDDING_*, {INGESTION,MCP}_MEM_LIMIT) and renders db/init/01_schema.sql's
vector(N) from a template. `make reindex` re-embeds after a switch.
- Services read the model/dim/prompts from env with defaults matching the
registry default; docker-compose derives memory limits and build-bakes the
model. FastEmbed applies no query/passage prefix for these models, so the
per-model prompt is applied manually around embed() (mxbai: query-only; e5:
both sides). The chunker's tokenizer now follows the embedding model.
Tooling + CI
- ruff (lint) and mypy configured; `make lint` / `make typecheck` and new CI
jobs. mypy quarantines a documented pre-existing typing backlog so the gate
enforces types on new/changed code. Coverage reporting added to `make test`.
- CI configures the small 384d model so the suite doesn't pull the 1.2GB
default on every run (and exercises `make configure`).
Security / robustness
- Sitemap XML now parsed with defusedxml (forbids DTD/entity expansion/XXE)
against untrusted upstream sources.
Parity guards
- tests assert the committed schema matches the rendered template, the service
defaults match the registry, and the duplicated SSRF _addr_is_private helper
stays byte-identical across the two services.
Housekeeping
- Add LICENSE; small B904/B905 fixes surfaced by the new lint gate.
Bring the docs in line with the registry-driven model selection: README and the runbook still described the model as fixed at BAAI/bge-small-en-v1.5 / 384-dim. - README: embeddings are selectable via config/models.yaml (default mxbai-embed-large-v1, 1024d); add `make configure` to the quickstart and `make lint`/`make typecheck` to development; link the LICENSE file. - Runbook: the offline-model FAQ now refers to the configured model and notes that switching it requires rebuilding both images (the model is baked in via the EMBEDDING_MODEL_NAME build arg). - ADR-004: new record for the registry decision — single source of truth, derived dimension/memory, the default upgrade, manual prompting, and the re-embed consequence. - ADR-001: inline supersede note pointing at ADR-004, matching the file's existing convention for the n8n note. The decision itself is left intact. - IMPLEMENTATION_PLAN: correct the claim that FastEmbed applies the BGE passage:/query: prefixes — it does not for any non-multitask model, so queries were previously embedded with no instruction prefix.
The schema parity test asserted db/init/01_schema.sql always equals the registry-DEFAULT render, but CI legitimately runs `make configure MODEL=BAAI/bge-small-en-v1.5` (to avoid pulling the 1.2GB default model on every run), which rewrites that file to vector(384). The test therefore contradicted CI's own setup step and failed. Split it into the two invariants that actually matter: - The working-tree schema must be a faithful render of the template at the ACTIVE dimension (EMBEDDING_DIM, which `make configure` writes to .env and the Makefile exports; falls back to the registry default). This still catches hand-editing — verified by temporarily editing the SQL and watching it fail — while tolerating a reconfigured checkout. - The schema COMMITTED TO GIT must be the default-model render, so a fresh clone gets the advertised default. Read via `git show HEAD:...` so a rewritten working copy is never mistaken for drift; skips if git is unavailable. Verified at both dimensions: parity suite passes with EMBEDDING_DIM=1024 and, after configuring bge-small, with EMBEDDING_DIM=384. Full e2e suite: 22 passed, 2 skipped.
AdamRussak
approved these changes
Jul 21, 2026
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
Makes the embedding model selectable from a registry, where picking a model auto-derives everything downstream (vector dimension, container memory limits, query/passage prompts, the pre-baked image layer), and upgrades the default to a stronger model. Also adds the lint/type gate the repo was missing, hardens sitemap XML parsing, and adds parity guards against the duplicated-code drift the codebase already warned about in comments.
Embedding model
config/models.yamlis the single source of truth.make configure MODEL=<name>resolves the selection into.envand re-rendersdb/init/01_schema.sqlfrom a template sovector(N)always matches the model.mixedbread-ai/mxbai-embed-large-v1(new default)intfloat/multilingual-e5-largeBAAI/bge-base-en-v1.5BAAI/bge-small-en-v1.5(previous default, used by CI)Still FastEmbed / ONNX / CPU — no torch, no GPU.
Bug found while implementing
The previous code (and ADR-001) assumed FastEmbed's
passage_embed()/query_embed()apply the BGEpassage:/query:prefixes. They don't — in FastEmbed 0.8 both delegate straight toembed()for every non-multitask model (bge, mxbai, e5), so queries were being embedded with no instruction prefix at all. Prompts are now applied explicitly from the registry (mxbai/bge instruct the query only; e5 prefixes both sides). The chunker's tokenizer also follows the embedding model now, so chunk sizing matches the model that actually embeds the text.Other changes
make lint/make typecheck, new CI job. mypy quarantines a documented pre-existing typing backlog so the gate enforces types on new/changed code without a risky refactor. Coverage added tomake test.make configure MODEL=BAAI/bge-small-en-v1.5so the suite doesn't pull 1.2GB on every run — which also continuously exercises the configure path.defusedxml(forbids DTD/entity expansion/XXE); these documents come from untrusted upstream sources.Other changes
make lint/make typecheck, new CI job. mypy quarantines a documented pre-existing typing backlog so the gate enforces types on new/changed code without a risky refactor. Coverage added tomake test.make configure MODEL=BAAI/bge-small-en-v1.5so the suite doesn't pull 1.2GB on every run — which also continuously exercises the configure path.defusedxml(forbids DTD/entity expansion/XXE); these documents come from untrusted upstream sources._addr_is_privatehelper stays byte-identical across services.LICENSEadded; smallB904/B905fixes surfaced by the new lint gate.IMPLEMENTATION_PLAN.mdcorrected.The default dimension changes 384 → 1024, so stored vectors are invalid and the column width changes. Existing deployments need a rebuild + re-embed:
Same-dimension model swaps can use
make reindexinstead. Full procedure: Runbook → switch the embedding model.Verification
make lint(ruff) clean;make typecheck(mypy) clean across both packages + scripts.make configureround-trip verified: selecting bge-small yields dim 384 + 1500m/1g in.envandvector(384)in the schema; the default yields 1024 + 2g/2g andvector(1024).make configureround-trip verified: selecting bge-small yields dim 384 + 1500m/1g in.envandvector(384)in the schema; the default yields 1024 + 2g/2g andvector(1024).docker compose --profile full configconfirms both services resolve to the same model with the derived memory limits and prompts.EntitiesForbidden) while a normal sitemap still parses.One pre-existing test (
test_startup_fails_fast_on_unreachable_db) fails locally under WSL2 — it expects a fastECONNREFUSEDfrom127.0.0.1:1, which this sandbox hangs instead. Unrelated to these changes; it should pass in CI.Not done deliberately: MCP
host_origin_protectionis left disabled — enabling it risks breaking Traefik-proxied access, which is out of scope here.