From 37a758cf1eae9f222f0a6ab1de6f874823c4f5fa Mon Sep 17 00:00:00 2001 From: Mani Sarkar Date: Wed, 7 Oct 2026 01:58:30 +0100 Subject: [PATCH 1/3] docs(sie): apply superlinked/sie#241 review fixes to sie-setup.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two amendments from the upstream SIE PR #241 review (code-owner @fm1320 and CodeRabbit) that were missing from the project's own user-facing guide: 1. Bounded poll loop: replace unbounded `until` loop in Step 3 with a 60-attempt cap (40 s × 60 ≈ 40 min). 502 exits immediately as a terminal load failure; 503/504 are treated as retryable. Loop wrapped in a subshell so that `exit 1` does not close an interactive terminal when pasted. 2. HF_TOKEN optionality: soften "required for model downloads" to "optional for ungated models (BGE-M3, Stella-v5); required for gated models such as naver/splade-v3" — updated prerequisites table, Path B summary row, Step B note, and Path B setup checklist. --- docs/plan/slices/PROGRESS.md | 2 ++ docs/user-guide/sie-setup.md | 46 ++++++++++++++++++++++++++---------- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/docs/plan/slices/PROGRESS.md b/docs/plan/slices/PROGRESS.md index 2976d87..b380a86 100644 --- a/docs/plan/slices/PROGRESS.md +++ b/docs/plan/slices/PROGRESS.md @@ -729,6 +729,7 @@ Implement the 4 stubbed chunkers (fixed, token, sentence, semantic), add sparse/ | Date | Slice | Decision | Why | |------|-------|----------|-----| +| 2026-10-07 | 21/22 — ad-hoc doc fix | **`sie-setup.md` amended per superlinked/sie#241 review comments.** (1) Bounded poll loop: replaced unbounded `until` loop with 60-attempt cap (40 s × 60 = 40 min max); 502 exits immediately as terminal load failure; 503/504 retried; loop wrapped in `( ... )` subshell to prevent `exit 1` closing interactive terminals. (2) `HF_TOKEN` optionality: softened "required for model downloads" to "optional for ungated models (BGE-M3, Stella-v5); required for gated models such as `naver/splade-v3`" across prerequisites table, Path B summary, Step B note, and Path B checklist. | Upstream SIE code-owner (Filip Makraduli @fm1320) and CodeRabbit identified all three loop issues and the HF_TOKEN overstatement in PR review; final commit `08b4c1a` incorporated them in the SIE-side docs guide. Applying the same corrections to the project's own user-facing guide ensures parity and prevents users from running an infinite poll or setting `HF_TOKEN` unnecessarily for BGE-M3/Stella-v5. | | 2026-10-01 | N/A — ad-hoc doc audit | **Elasticsearch/Redis documentation-parity audit + remediation (PR #222).** Found ES/Redis under-routed at every upstream entry doc (TRAVERSAL.md, CLAUDE.md, badges, docs/README.md tables) despite complete dedicated setup guides; accepted ADR-008, added ADR-009 (embedding-cache backend port, distinct from ADR-007); fixed 7+ stale "STORAGE_BACKEND defaults to mongodb permanently" claims against the actual shipped sqlite default; added a regression test asserting ES/Redis adapters structurally cannot satisfy `StorageBackend`. Reviewed across 5 independent `nw-*-reviewer` passes (documentarist ×3, solution-architect, software-crafter) — every finding fixed or confirmed already covered. | A new user/agent following any single entry doc could reach the end without learning Elasticsearch or Redis exist, and `CLAUDE.md`/`architecture.md` actively contradicted the shipped sqlite default — a correctness bug, not just a discoverability gap. | | 2026-10-01 | N/A — ad-hoc doc audit | **`getting-started.md`'s Configure section collapsed from 5 duplicated inline per-backend config blocks to one table linking to each backend's dedicated setup guide** (reviewed by nw-documentarist-reviewer first; Postgres cloud-vs-local warning and Redis vector-store/embedding-cache role distinction preserved verbatim as flagged-blocking). Net: page shortened from 323 to 274 lines (below its pre-work 272-line baseline) while covering 2 more backends. | Every backend already had a complete setup guide; duplicating that content inline added scrolling cost for every reader without adding information — the opposite of progressive disclosure. | | 2026-10-01 | 54 | **DECISIONS #273 — Option (a): one shared Redis instance, volatile-lru eviction.** Cache keys `rpf:emb:*` carry TTL (7-day default); vector keys `rpf:chunk:*` have no TTL. Both key families share one Redis instance. Confirmed no namespace collision (different prefix + different eviction tier). | Option (b) (separate Redis instance for cache) would increase operational cost and complexity for a Could-tier feature; volatile-lru is purpose-built for this use case and already part of the Slice 53 Redis deployment. | @@ -1055,6 +1056,7 @@ Tracks skill runs across slices and sessions. Appended automatically by `/verify | Date | Branch | Skill | Slice | Outcome | Notes | |---|---|---|---|---|---| +| 2026-10-07 | docs/sie-setup-pr241-amendments | /sync-docs | 21/22 sie-setup doc fix | APPLIED | Amended `docs/user-guide/sie-setup.md`: (1) bounded poll loop with 502 terminal exit + subshell wrap; (2) `HF_TOKEN` softened to optional for ungated models. Decision log entry added. Docs-only — no code changes. | | 2026-07-29 | slice/22-sie-scooter | /sync-docs | Doc technology badges | APPLIED | Tracker-only after `a56bf87`: PROGRESS maintenance + decision + skill log. Entry/user/contributor guides + CHANGELOG already matched (feature commit). CLAUDE/AGENTS/TRAIL/GAP/ADRs NO_CHANGE. Evidence **IMPLEMENTED**; Meterian badge endpoints HTTP 200 observed earlier in session. | | 2026-07-29 | slice/22-sie-scooter | /sync-docs | Slice 22 post-verify | APPLIED | Tracker-only: HANDOFF + SLICE-22 Gate Status + PROGRESS header/skill log now cite `/verify-slice` COMPLETE + `383541b`. User-guide/CLAUDE/CHANGELOG NO_CHANGE (already matched shipped API). Evidence **VERIFIED** (unit/API-mock); live SIE optional; PR confirm pending. | | 2026-07-29 | slice/22-sie-scooter | /verify-slice | Slice 22 | COMPLETE | Re-run after hermetic fix: 6/6 exit criteria PASS; 17/17 targeted tests; docs current; live SIE NOT_APPLICABLE. | diff --git a/docs/user-guide/sie-setup.md b/docs/user-guide/sie-setup.md index 55df25f..acd7d24 100644 --- a/docs/user-guide/sie-setup.md +++ b/docs/user-guide/sie-setup.md @@ -47,7 +47,7 @@ Three server `.env` variables control SIE. They are **not** split into “local | Path | When to use | What you need | |---|---|---| | **A — Remote gateway** *(recommended if available)* | Your org runs SIE (Helm/K8s, managed gateway, hackathon endpoint) | `SIE_ENABLED=true`, `SIE_ENDPOINT`, `SIE_API_KEY` in server `.env` — **no Docker** | -| **B — Self-hosted Docker** | No remote gateway; you run SIE locally on `:8720` | Docker Desktop, `HF_TOKEN`, warm-up per [§ Self-hosted Docker](#self-hosted-docker-optional) below | +| **B — Self-hosted Docker** | No remote gateway; you run SIE locally on `:8720` | Docker Desktop, `HF_TOKEN` (required for gated models; optional for BGE-M3/Stella-v5), warm-up per [§ Self-hosted Docker](#self-hosted-docker-optional) below | ### Path A — Remote gateway (no Docker) @@ -92,7 +92,7 @@ first-run model download and warm-up can take 10–30+ minutes on Apple Silicon. | Requirement | Notes | |---|---| | Docker Desktop | 4 GB+ of free disk space | -| `HF_TOKEN` | HuggingFace read token — required for model downloads inside the container | +| `HF_TOKEN` | HuggingFace read token — **optional** for ungated models (BGE-M3, Stella-v5); **required** for gated models such as `naver/splade-v3` | | Apple Silicon (M1/M2/M3) | Extra `--platform linux/amd64` flag required (see below) | > **`HF_TOKEN` is only for the Docker path.** Remote gateways manage their own model weights; @@ -111,6 +111,10 @@ export HF_TOKEN=hf_xxxxxxxxxxxxxxxxxxxx > **Note:** `HF_TOKEN` is only needed by the SIE container for model downloads. It does not > go into the server's `.env` (the server never calls HuggingFace directly). You can pass it > as a shell export or add `HF_TOKEN=hf_...` to `.env` and source it. +> +> **Ungated models** (BGE-M3 `BAAI/bge-m3`, Stella-v5 `dunzhang/stella_en_1.5B_v5`) do not +> require `HF_TOKEN`. **Gated models** such as `naver/splade-v3` require an accepted HuggingFace +> license agreement and a token with read access to that repo. --- @@ -196,17 +200,32 @@ When tailing `docker logs -f sie-server`, you will typically see this progressio If phase 2 appears without phase 3 within a few minutes, confirm the volume mount: `-v sie-hf-cache:/app/.cache/huggingface` (see [Disk cache warning on first start](#disk-cache-warning-on-first-start)). -Poll until the model is ready: +Poll until the model is ready (bounded: 60 attempts × 40 s ≈ 40 min max): ```bash -until curl -sf -o /dev/null -X POST http://localhost:8720/v1/encode/BAAI/bge-m3 \ - -H "Content-Type: application/json" \ - -d '{"items":[{"text":"readiness probe"}]}'; do - echo "SIE not ready yet — waiting 10s..." - sleep 10 -done -echo "SIE model ready" -``` +( + attempt=0; max=60 + until [ "$attempt" -ge "$max" ]; do + status=$(curl -s -o /dev/null -w "%{http_code}" -X POST http://localhost:8720/v1/encode/BAAI/bge-m3 \ + -H "Content-Type: application/json" \ + -d '{"items":[{"text":"readiness probe"}]}') + if [ "$status" = "200" ]; then echo "SIE model ready"; exit 0; fi + if [ "$status" = "502" ]; then + echo "SIE returned 502 — terminal load failure. Check: docker logs sie-server" + exit 1 + fi + attempt=$((attempt + 1)) + echo "SIE not ready (HTTP $status) — attempt $attempt/$max, waiting 40s..." + sleep 40 + done + echo "Timed out after $((max * 40))s. Check: docker logs -f sie-server" + exit 1 +) +``` + +> **503 and 504 are retryable** (model loading / gateway timeout). **502 is terminal** (load +> failed permanently — restart required). The loop is wrapped in a subshell `( ... )` so that +> `exit 1` does not close your terminal if you paste the block into an interactive session. Watch the container logs for: ``` @@ -543,7 +562,7 @@ docker run -d \ ghcr.io/superlinked/sie-server:latest-cpu-default ``` -Confirm `HF_TOKEN` is set before Step B — downloads require HuggingFace read access. +Confirm `HF_TOKEN` is set before Step B if using gated models (e.g. `naver/splade-v3`); it is optional for BGE-M3 and Stella-v5. Watch `docker logs -f sie-server` until `Fetching 30 files` completes **without** the `Background writer channel closed` error, then poll encode to 200. @@ -818,7 +837,8 @@ python3 -c "import os; print(os.getenv('SIE_ENDPOINT', 'http://localhost:8720')) ``` [ ] Docker Desktop installed and running -[ ] HF_TOKEN exported: export HF_TOKEN=hf_... +[ ] HF_TOKEN exported if using gated models (e.g. naver/splade-v3): export HF_TOKEN=hf_... + (optional for ungated models BGE-M3 / Stella-v5) [ ] SIE container started (with --platform linux/amd64 on Apple Silicon) [ ] curl http://localhost:8720/healthz → ok [ ] Model warm-up poll passes (HTTP 200 from POST /v1/encode/BAAI/bge-m3) From e0e914c124039da5e28cd4f5f2323d29621d8575 Mon Sep 17 00:00:00 2001 From: Mani Sarkar Date: Wed, 7 Oct 2026 11:10:33 +0100 Subject: [PATCH 2/3] docs(sie): extend pr241 fixes to .env.example, configuration.md; fix 504 retryable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply remaining gaps identified by post-merge sync-docs audit against superlinked/sie#241: - .env.example: mark HF_TOKEN optional for ungated models (BGE-M3, Stella-v5); required for gated models (e.g. naver/splade-v3); harmonise "Docker path only" wording across both comment lines - docs/user-guide/configuration.md: same optionality note at the commented env block (line 558) and the env-var reference table (line 665) - server/core/embedding/sie_embedder.py: add "504" and "gateway timeout" to _is_retryable_sie_error() — 504 was documented as retryable but missing from code Reviewed and approved by nw-documentarist-reviewer (PASS) and nw-software-crafter-reviewer (PASS) before commit. --- .env.example | 4 ++-- docs/user-guide/configuration.md | 4 ++-- server/core/embedding/sie_embedder.py | 2 ++ 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.env.example b/.env.example index 0e5a07d..033e105 100644 --- a/.env.example +++ b/.env.example @@ -120,12 +120,12 @@ VOYAGE_TPM_LIMIT=10000 # When server runs in Docker and SIE on the host: # SIE_ENDPOINT=http://host.docker.internal:8720 # SIE_API_KEY= # usually empty for local Docker -# HF_TOKEN=hf_... # HuggingFace token — Docker container only +# HF_TOKEN=hf_... # HuggingFace token — Docker path only; optional for ungated models (BGE-M3, Stella-v5); required for gated models (e.g. naver/splade-v3) # # SIE_ENABLED=false # SIE_ENDPOINT=http://localhost:8720 # SIE_API_KEY= -HF_TOKEN=your_huggingface_token_here # Docker path only — not used by server for remote gateway +HF_TOKEN=your_huggingface_token_here # Docker path only — optional for ungated models (BGE-M3, Stella-v5); required for gated models (e.g. naver/splade-v3) # ── Port reference (static — chosen to avoid common conflicts) ─────────────── # SERVER_URL=http://localhost:8001 # FastAPI backend (8001 — uncommon, not a framework default) diff --git a/docs/user-guide/configuration.md b/docs/user-guide/configuration.md index 198db57..e0bc054 100644 --- a/docs/user-guide/configuration.md +++ b/docs/user-guide/configuration.md @@ -555,7 +555,7 @@ SIE_ENABLED=false # Path B — self-hosted Docker on :8720 (only when no remote gateway): # SIE_ENDPOINT=http://localhost:8720 # SIE_ENDPOINT=http://host.docker.internal:8720 # server in Docker, SIE on host -# HF_TOKEN=hf_... # Docker path only — HuggingFace token for container model downloads +# HF_TOKEN=hf_... # Docker path only — optional for ungated models (BGE-M3, Stella-v5); required for gated models (e.g. naver/splade-v3) # Aim experiment tracking (OPTIONAL — UI via ./scripts/docker/aim-ui.sh) # AIM_REPO=.aim # Docker sets /app/.aim automatically @@ -662,7 +662,7 @@ profile already includes `--maxmemory-policy volatile-lru` — see | `SIE_ENABLED` | `false` | **Master on/off** for SIE — not local-vs-remote; set `true` when you want the server to use SIE (either path) | | `SIE_ENDPOINT` | `http://localhost:8720` | **Where** to connect — remote gateway URL or local Docker (`host.docker.internal:8720` when server is in Docker) | | `SIE_API_KEY` | — | **Auth** — Bearer token when gateway requires it; usually empty for local Docker | -| `HF_TOKEN` | — | HuggingFace token for **self-hosted Docker only** — not used when pointing at a remote gateway | +| `HF_TOKEN` | — | HuggingFace token for **self-hosted Docker only** — **optional** for ungated models (BGE-M3, Stella-v5); **required** for gated models such as `naver/splade-v3` | | `AIM_REPO` | `.aim` | Path to Aim experiment repo (Docker: `/app/.aim`; UI: `./scripts/docker/aim-ui.sh`) | | `HEALTH_CHECK_MONGODB_TIMEOUT_MS` | `5000` | MongoDB ping timeout for `/healthz` (ms) | diff --git a/server/core/embedding/sie_embedder.py b/server/core/embedding/sie_embedder.py index 7882ce3..d8a6033 100644 --- a/server/core/embedding/sie_embedder.py +++ b/server/core/embedding/sie_embedder.py @@ -81,6 +81,8 @@ def _is_retryable_sie_error(exc: Exception) -> bool: or "rate limit" in message or "too many requests" in message or "503" in message + or "504" in message + or "gateway timeout" in message or "unavailable" in message or "service unavailable" in message ) From 386794aee4a3931fd4ffeaddabe9e801a0c7b410 Mon Sep 17 00:00:00 2001 From: Mani Sarkar Date: Wed, 7 Oct 2026 11:25:18 +0100 Subject: [PATCH 3/3] test(sie): add 504 retry test to match existing 503 coverage nw-software-crafter-reviewer flagged that the 504 addition in _is_retryable_sie_error() had no test proving the path executes. Mirrors test_embed_documents_retries_on_503_like_errors exactly, using "504 Gateway Timeout" as the transient error. --- .../core/embedding/test_sie_embedder.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/server/core/embedding/test_sie_embedder.py b/tests/server/core/embedding/test_sie_embedder.py index f4c6f26..2a7384b 100644 --- a/tests/server/core/embedding/test_sie_embedder.py +++ b/tests/server/core/embedding/test_sie_embedder.py @@ -248,6 +248,37 @@ def _encode_side_effect(*_args: object, **_kwargs: object) -> list[dict]: assert len(result) == 1 assert attempts == 2 + def test_embed_documents_retries_on_504_like_errors(self, monkeypatch: pytest.MonkeyPatch): + """ + Scenario: embed documents retries on 504 like errors. + Slice: 45 — GWT-on-touch (module theme separation) + Given SIE encode returns a transient 504 Gateway Timeout error + When embed_documents_sie is called + Then requests are retried and eventually succeed. + """ + ### Given + ### When + ### Then + attempts = 0 + + def _encode_side_effect(*_args: object, **_kwargs: object) -> list[dict]: + nonlocal attempts + attempts += 1 + if attempts == 1: + raise Exception("504 Gateway Timeout") + return [{"dense": np.zeros(1024, dtype=np.float32)}] + + monkeypatch.setattr("server.core.sie_embedder._SIE_INITIAL_RETRY_DELAY_S", 0.0) + with patch("server.core.sie_embedder.SIEClient") as mock_client_cls: + mock_client_cls.return_value.encode.side_effect = _encode_side_effect + + from server.core.sie_embedder import embed_documents_sie + + result = embed_documents_sie(["test"], "bge-m3") + + assert len(result) == 1 + assert attempts == 2 + def test_embed_documents_retries_on_429_with_retry_after_header( self, monkeypatch: pytest.MonkeyPatch ):