feat(ok): custom endpoint embedding models — auto-detected dimensions, model field, connection test (#2883)#818
Merged
Merged
Conversation
…, model field, connection test (#2883)
* feat(ok): auto-detect embedding dimensions + model field and connection test
Semantic search assumed every model returned 1536-dim vectors, so any custom
OpenAI-compatible endpoint whose model has a different native size failed every
response and degraded silently to keyword search.
- Unset `search.semantic.dimensions` now means "whatever the endpoint returns":
the embedder pins the size from the first response, the vector cache persists
it, and restart reuses it. Cache identity keeps an `'auto'` sentinel so the
detected value never re-triggers its own invalidation.
- A size change mid-life (model swapped behind an alias) discards + rebuilds the
corpus instead of scoring two sizes together; bounded per process. Detected on
both the embed and query paths, so an unchanged corpus still recovers.
- New loopback route POST /api/local-op/embeddings/test runs one probe embed of
a fixed string against the SAVED endpoint and returns the detected size or a
classified reason.
- Settings → Search: "Custom endpoint" disclosure with endpoint + free-text
model + Test connection; auto-expands when either is overridden; changing
either warns about the re-index and egress first.
- Account key relabelled "Embeddings API key" (it is sent to the configured
endpoint, not just OpenAI's).
* docs(ok): document auto-detected dimensions + Test connection; tidy exports
- Reference docs: `dimensions` now defaults to auto-detected, the setup steps
name the Settings disclosure + Test connection, and the egress note says the
key goes to whichever endpoint is configured.
- Status panel points at Test connection instead of re-listing places to check.
- Drop exports nothing consumes (knip); log the classified provider reason on a
failed query embed now that it is available.
* fix(ok): pluralize the re-index page count in the provider-change warning
Verified live against a self-hosted-style endpoint: a one-page project read
"The full text of all 1 pages".
* fix(ok): address review — cut off cleanly when the drift budget is spent
- Spending MAX_DIMS_DRIFT_RESETS now drops `capable`, so every later search
short-circuits instead of re-reaching the provider and re-logging the same
error line per query. `runEmbedPass` honours the same flag.
- Reset the drift budget on a provider-fingerprint change. Deliberately in
`applyConfig` and NOT in `resetWarm` (as suggested in review): drift recovery
calls `resetWarm` itself, so refunding there would leave the bound inert.
- Log when the query path declines recovery (explicit dimensions, or budget
spent) — the embed path already did, so a query-first mismatch was the one
route that degraded with no trail.
- Test-connection failures render destructive, not the same amber as the
transient "still saving" notice, and carry role="alert".
- Cover the budget bound: a provider flapping sizes every request stops costing
rebuilds, and a deliberate provider change starts fresh. Verified the test
binds by mutating the constant.
- `ok embeddings set-model` / `clear-model`, mirroring set-url/clear-url. There
is no generic `ok config set`, so the headless persona this feature targets
could set the endpoint but not the model.
* fix(ok): match aria-live to role on the test-connection result; assert capable
- `role="alert"` implies assertive, so the explicit `aria-live="polite"`
contradicted it and screen readers split on which wins.
- Assert `capable` is restored after the provider-change budget reset — the
direct guard against the reset being moved into `resetWarm`.
* fix(ok): treat a non-JSON 200 as a config error, not a retryable network fault
A base URL that lands on a proxy's HTML page or a site root answers 200 with
markup; `res.json()` throws, and the generic catch classified that as `network`
— four retries, then "couldn't reach the endpoint" for a host that answered
instantly. It is now `malformed_response`: fatal, no retries, and the UI copy
tells the user to check the base URL. That is the likeliest misconfiguration
for the persona this feature exists for.
Also from review:
- Free the resident vectors when the drift budget is spent; nothing reads them
again this process and the disk store still survives for a re-hydrate.
- `onTestConnection` gets a `finally`, so a transport that rejects (against its
contract) can't leave the button stuck on "Testing".
- Bracket MAX_DIMS_DRIFT_RESETS instead of just proving the budget is finite:
the test now walks exactly that many size changes, asserts recovery each
time, and asserts give-up on the next. Mutating the constant to 1 or 3 both
fail it.
* fix(ok): drop the try/finally React Compiler cannot lower
The compiler rejects a `try` with a finalizer outright, so the previous commit
built fine under tsc and the DOM tests but failed the real app build. Same
guarantee via catch-and-continue.
* wip(ok): project-level embeddings keys — storage + resolution + CLI (backend)
Checkpoint: nested project->endpoint->key map in ~/.ok/secrets.yml (bytes stay
out of the project tree), single resolveEmbeddingsCredential seam (project key
-> env/flat default-host-only -> keyless loopback), advisory lock for
cross-process writes, project-scoped CLI set/clear + list. Tests + app UI next.
* feat(ok): move embeddings key to the project Search screen + keyless local
UI half of project-level keys:
- The API key now lives on Settings → This project → Search, always-visible next
to its endpoint (not buried in the disclosure) — the Account key screen is
removed. Write-only, per-endpoint, optional. Status shows "not required" for a
localhost endpoint so keyless Ollama/LM Studio users aren't nagged.
- Keyless loopback: a localhost endpoint embeds + tests with no Authorization
header at all; non-loopback still needs a key.
- semantic-status gains `keyNotRequired`; keySource gains `project`.
- Tests rewritten for the project+endpoint model (secrets-store, embedder
resolution, factory set/clear, SearchSection key field, test-route integration
via the real set-key route).
* docs(ok): document project-level embeddings keys + keyless local; spec D16
* fix(ok): trim unused re-exports from the CLI embeddings-key shim (knip)
* fix(ok): apply a newly-set embeddings key without a restart + dev/prod parity
QA surfaced two gaps:
- The key is resolved at warm time and isn't part of the provider fingerprint,
so setting/clearing it left the service warmed with the old credential ("added
a key but search stays lexical"). set/clear-key now call a new
`reloadCredential()` that re-warms on the next search — cheap (cache
re-hydrates from disk, no re-embed).
- The dev server (hocuspocus-plugin) never injected the embeddings key store, so
dev only ever resolved the env key / keyless while production reads
~/.ok/secrets.yml. Inject makeLazyEmbeddingsKeyStore() so dev matches prod
(also what let me verify the stored-key path end-to-end in the browser).
Verified live: keyless loopback embeds with no Authorization header; after
set-key the next embed sends Bearer <key>; clear reverts to keyless.
* fix(ok): strip spec decision markers from source comments (md-audit)
* fix(ok): review round 5 — key-read/write observability + Enter-to-save + test gaps
- Log a warning when the key-store read fails instead of silently mapping to
"no key" (a stored-but-unreadable key otherwise looks absent and the user
re-runs set-key chasing a ghost).
- CLI set-key/clear-key wrap the store mutation in try/catch with a clean error
+ exit 1 (the headless path pipes keys from a secrets manager and must report
a disk-write failure, not dump a stack).
- Log on the secrets-store best-effort unlink failure (a "cleared" key that
stayed on disk) and on a non-EEXIST lock failure (serialization silently off).
- Enter-to-save on the key field, matching the endpoint/model inputs.
- Tests: no_key test-route case (no network call), a reloadCredential-visible
assertion in the factory set-key test (guards the no-restart behavior), and
key-field Enter-to-save + save-failure DOM cases.
---------
GitOrigin-RevId: d9fce206c7919fb33c3b2a6b5545e4a937977c12
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
1 participant
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.
Copybara-translated 1 Inkeep OSS change. Rebase-merge this PR so the prepared commit lands directly on public main.