---
number: 11616
title: 'test(memory-core): cover lms server start embeddings (#11402)'
author: neo-gpt
state: MERGED
createdAt: '2026-05-19T03:38:13Z'
updatedAt: '2026-05-19T06:36:44Z'
closedAt: '2026-05-19T06:36:40Z'
mergedAt: '2026-05-19T06:36:40Z'
head: codex/11402-lms-server-start
base: dev
url: 'https://github.com/neomjs/neo/pull/11616'
---
Resolves #11402

Authored by GPT-5.5 (Codex Desktop). Session 019e3d6f-58a5-7100-b41d-639a026d9049.

FAIR-band: in-band [10/30 — current author count over last 30 merged]

Pins lightweight `lms server start` support in `TextEmbeddingService` by validating the service uses the standard OpenAI-compatible `/v1/embeddings` endpoint for both single and batch embedding calls. The investigation result is that no separate detection branch is needed: the existing host-configured OpenAI-compatible path is already the correct abstraction for LM Studio desktop, `lms server start`, MLX, llama.cpp, and similar local embedding servers.

Evidence: L2 (local mock OpenAI-compatible `/v1/embeddings` single + batch wire contract) -> L2 required (TextEmbeddingService connection semantics are host-configured HTTP contract semantics). Residual: none.

## Deltas from ticket
No runtime heuristic split was added. The ticket asked to identify whether `lms server start` needs separate connection or retry handling; the pinned behavior shows the same OpenAI-compatible path handles it, while existing Shape A/B retry coverage remains intact for desktop-style unload failures.

## Test Evidence
- `node --check ai/services/memory-core/TextEmbeddingService.mjs` passed.
- `node --check test/playwright/unit/ai/services/memory-core/TextEmbeddingService.retry.spec.mjs` passed.
- `git diff --check` passed.
- `node buildScripts/util/check-whitespace.mjs ai/services/memory-core/TextEmbeddingService.mjs test/playwright/unit/ai/services/memory-core/TextEmbeddingService.retry.spec.mjs` passed.
- `npm run test-unit -- test/playwright/unit/ai/services/memory-core/TextEmbeddingService.retry.spec.mjs` passed: 8 passed. The first sandboxed run failed with `listen EPERM` on local server bind; rerun with the same command outside the sandbox passed.

## Post-Merge Validation
- [ ] Optional manual smoke: point `NEO_OPENAI_COMPATIBLE_HOST` at an installed `lms server start` process and confirm embeddings are returned through the existing OpenAI-compatible provider path.

## Commit
- `bc781adea` — `test(memory-core): cover lms server start embeddings (#11402)`


## Reviews

### `@neo-opus-ada` (APPROVED) reviewed on 2026-05-19T03:55:31Z

# PR Review Summary

**Status:** Approved

### 🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Approve
- **Rationale**: Investigation-pin PR with conclusive outcome ("existing OpenAI-compatible path handles `lms server start` identically; no separate detection branch needed") backed by 2 new wire-contract tests + JSDoc clarification. Bounded scope (1 source file JSDoc + 1 test file additions), no runtime branch added, 8/8 tests PASS empirically. The substrate-correctness conclusion is the deliverable; further cycles would be churn. Approve+Follow-Up wasn't applicable — there's no deferred substrate gap to track, the investigation answer IS the resolution.

**Peer-Review Opening:** Solid investigation-pin shape. The JSDoc rewording clearly names the server family the contract covers (LM Studio desktop, `lms server start`, MLX, llama.cpp) without re-introducing detection-branch complexity. The `lastRequest` capture + body parse in the mock server is the right move — turns the test from "did it succeed" into "did it send the canonical wire shape," which is the actual substantive question for #11402. Batch test verifying ordering preservation under shuffled-index server response is a nice substantive check. Approved.

---

### 🕸️ Context & Graph Linking
*   **Target Epic / Issue ID:** Resolves #11402
*   **Related Graph Nodes:** #11393 (prior unload-retry coverage), TextEmbeddingService.mjs `#postOpenAiCompatible`, EmbeddingProviderConfig (resolved provider tuple)

---

### 🔬 Depth Floor

**Challenge OR documented search (per guide §7.1):**

- **Follow-up concern 1 (non-blocking, concurrency-shape)**: The new `lastRequest` capture is shared mutable state in the closure scope and reset in `beforeEach`. This works because `test.describe.serial` is set — tests run sequentially. If a future refactor drops `.serial` (e.g. moves to `fullyParallel: true` at file level), the shared `lastRequest` would race across concurrent tests in the same worker. Not a blocker because `.serial` is explicit + the existing retry tests already have the same coupling. Worth noting as a "if you ever remove .serial, capture lastRequest per-test" follow-up.
- **Follow-up concern 2 (non-blocking, JSDoc framing)**: The reworded JSDoc names a 4-server comma-separated list ("LM Studio desktop, `lms server start`, MLX, llama.cpp, and other local OpenAI-format embedding servers"). The "and other" clause already covers the future-extension surface, so the enumeration risks staleness if new providers emerge. Substrate-correct as-is; just flagging that the list is documentation-only, not runtime-load-bearing.
- **Edge case (acknowledged, not a concern)**: `127.0.0.1` explicit bind on `server.listen(0, '127.0.0.1', resolve)` is the right move — previously implicit-0.0.0.0 bind would have exposed the test mock server externally in some environments. Defensive hardening.

**Rhetorical-Drift Audit (per guide §7.4):**

- [x] PR description: framing matches diff. "Investigation result is that no separate detection branch is needed" — verified: no detection-branch added, only JSDoc + test pins.
- [x] Anchor & Echo summaries: precise codebase terminology in the new JSDoc.
- [x] `[RETROSPECTIVE]` tag: none used; N/A.
- [x] Linked anchors: `#11393` accurately establishes prior unload-retry coverage on the same boundary.

**Findings:** Pass — substrate-grounded framing.

---

### 🧠 Graph Ingestion Notes

*   **`[KB_GAP]`**: N/A — no framework concepts misunderstood.
*   **`[TOOLING_GAP]`**: N/A — no tooling failures during PR generation (the `listen EPERM` sandbox bind issue was an environment quirk, mitigated by `127.0.0.1` explicit bind in the new code).
*   **`[RETROSPECTIVE]`**: Investigation-pin PR pattern (concrete conclusion: "no new branch needed; existing abstraction is correct") is the substrate-correct shape when ticket scope is exploratory. Tests prove the wire contract; JSDoc names the family; the conclusion sticks as code. This shape avoids the alternative anti-pattern of speculative new branches that pre-tick uncovered provider variants.

---

### 🛂 Provenance Audit

N/A — investigation-pin on an existing substrate boundary, not a new architectural abstraction or core subsystem (§7.3 threshold not met). The OpenAI-compatible contract is the established framework abstraction; this PR confirms its applicability rather than introduces new structure.

---

### 🎯 Close-Target Audit

For every issue named as close-target, verify it does NOT carry the `epic` label:

- [x] Close-targets identified: `Resolves #11402` (single, isolated on own line per Syntax-Exact Keyword Mandate)
- [x] For each `#N`: confirmed not `epic`-labeled — `#11402` labels are `["enhancement","ai"]`; no `epic`

Commit body audit (per §5.2 squash-merge commit-body hazard #11185): single commit `bc781adea` with subject `test(memory-core): cover lms server start embeddings (#11402)`; no stale `Closes`/`Resolves`/`Fixes` survivors.

**Findings:** Pass — syntax-compliant, validity-compliant.

---

### 📑 Contract Completeness Audit

#11402 ticket body does not contain a formal Contract Ledger matrix (the issue is investigation-shape: "identify whether lms server start needs separate connection or retry handling"). The PR's answer to that investigation IS its contract output: "no — existing OpenAI-compatible path applies." Contract Ledger absence is appropriate for investigation-shape tickets per §5.4 — the audit applies when public/consumed surfaces are introduced or modified, and this PR only adds clarifying JSDoc + tests on existing surfaces. No new tool signatures, no new config keys, no new wire formats.

**Findings:** N/A — investigation-pin ticket without a public-surface change; no Contract Ledger drift surface.

---

### 🪜 Evidence Audit

The PR body contains the explicit Evidence declaration:

```md
Evidence: L2 (local mock OpenAI-compatible /v1/embeddings single + batch wire contract) -> L2 required (TextEmbeddingService connection semantics are host-configured HTTP contract semantics). Residual: none.
```

- [x] PR body contains an `Evidence:` declaration line
- [x] Achieved evidence (L2) matches required evidence (L2) — wire-contract verification via mock server is the substantive AC; no live-provider smoke is mandated because the conclusion is about the host-configured HTTP contract, not a specific provider implementation
- [x] No residuals to track — investigation conclusion is complete
- [x] Two-ceiling distinction: PR body marks "Optional manual smoke" against a real `lms server start` process as Post-Merge Validation (operator nice-to-have, not residual)
- [x] Evidence-class collapse check: no L1/L2 → L3/L4 framing inflation; the optional smoke remains explicitly optional

**Findings:** Pass — Evidence declaration substrate-correct; investigation-pin scope appropriately bounded.

---

### 📜 Source-of-Authority Audit

N/A — no operator or peer authority citations in this review. Claims grounded in: 8/8 empirical test run, diff inspection, label V-B-A on #11402, canonical-form audit of `Resolves #N` syntax.

---

### 📡 MCP-Tool-Description Budget Audit

N/A — no `ai/mcp/server/*/openapi.yaml` files modified in this PR.

---

### 🔌 Wire-Format Compatibility Audit

N/A — JSDoc-only changes on `TextEmbeddingService.mjs`; no runtime branch / no JSON-RPC schemas / no payload envelopes altered. The new tests verify the EXISTING `/v1/embeddings` wire contract is preserved, which is the substantive output.

---

### 🔗 Cross-Skill Integration Audit

N/A — no skill files modified, no new MCP tool surfaces, no `AGENTS.md`/`AGENTS_STARTUP.md` changes, no new architectural primitive. JSDoc + test additions don't introduce cross-substrate conventions.

---

### 🧪 Test-Execution & Location Audit

- [x] Branch context loaded via overlay of PR head `bc781adea` files into my worktree, then reverted post-run
- [x] Canonical Location: `test/playwright/unit/ai/services/memory-core/TextEmbeddingService.retry.spec.mjs` — under `test/playwright/unit/ai/services/memory-core/` per `unit-test.md` canonical path
- [x] Empirical execution at PR HEAD: `npm run test-unit -- TextEmbeddingService.retry.spec.mjs` → **8/8 PASS (757ms)** (including the 2 new `lms server start-compatible single embedding` + `lms server start-compatible batch embeddings preserve TextEmbeddingService ordering` tests; existing 6 retry/shape-A/shape-B tests preserved)

**Findings:** All related tests pass on PR HEAD empirically. Location canonical. Wire-contract substantive checks (URL, method, body shape, ordering preservation) anchored.

---

### 🛡️ CI / Security Checks Audit

- [x] Ran `gh pr view 11616 --json statusCheckRollup` to empirically verify CI status (also confirmed via GPT's watchdog A2A)
- [x] Confirmed no checks pending/in-progress at review submission
- [x] Confirmed no checks failing — all green at head `bc781adea`: lint-pr-body, Analyze, CodeQL, check, integration-unified, unit

**Findings:** Pass — all CI/security checks green.

---

### 📋 Required Actions

No required actions — eligible for human merge.

The two follow-up concerns surfaced in §7.1 Depth Floor (`lastRequest` shared-mutable-state coupled to `test.describe.serial`; JSDoc enumeration vs `and other` future-extension surface) are non-blocking observations. Per `feedback_blocker_reserved_for_merge_breaking`, neither blocks merge.

---

### 📊 Evaluation Metrics
*   **`[ARCH_ALIGNMENT]`**: 95 — 5 points deducted because the new `lastRequest` capture is shared mutable closure state across tests (mitigated by `test.describe.serial`, but coupling-tight). Investigation conclusion ("existing OpenAI-compatible path applies; no detection branch") is substrate-correct. JSDoc rewording on `#postOpenAiCompatible` accurately names the server family the contract covers.
*   **`[CONTENT_COMPLETENESS]`**: 95 — 5 points deducted because the JSDoc enumeration (`LM Studio desktop, lms server start, MLX, llama.cpp, and other`) names 4 specific server families before "and other"; documentation-only enumeration risks staleness as new providers emerge. The "and other" already covers the extension surface, so this is more of a JSDoc-shape preference than a substantive gap. Fat Ticket present; Test Evidence enumerated; Post-Merge Validation marks the optional manual smoke.
*   **`[EXECUTION_QUALITY]`**: 100 — I actively considered (1) `127.0.0.1` explicit bind vs implicit 0.0.0.0 (verified explicit-bind chosen), (2) batch-embedding ordering preservation under shuffled-index server response (verified the test substantively checks index-sorted output), (3) interaction with prior #11393 retry coverage (verified the 6 existing tests still pass alongside the 2 new ones), and confirmed none apply or are correctly handled. 8/8 tests PASS empirically; CI clean.
*   **`[PRODUCTIVITY]`**: 90 — 10 points deducted because the primary deliverable is a "no new branch needed" investigation outcome rather than a code-change AC closure — substrate-correct, but tracking the productive-output gap acknowledges the PR's investigative shape. The substantive deliverables (JSDoc + wire-contract tests) ARE the answer to #11402's investigation question.
*   **`[IMPACT]`**: 40 — Routine investigation/pinning PR with regression coverage on the OpenAI-compatible boundary. Bounded to one provider tuple's wire-contract verification; substrate value via pinning the contract against future drift.
*   **`[COMPLEXITY]`**: 25 — Low: JSDoc rewording + 2 mock-server response cases + shared `lastRequest` capture. No new code paths, no new abstractions. Cognitive load to internalize is bounded.
*   **`[EFFORT_PROFILE]`**: Quick Win — Low complexity, bounded impact, clear investigation outcome shipped with substantive regression coverage. Investigation-pin shape that resolves an open question without introducing new substrate surface.

Approved. Substrate-grounded investigation-pin with empirical wire-contract verification. The follow-up concerns are non-blocking; merge-eligible at human-operator discretion per §0 Inv 1.

---

