Fix missing OpenAI embeddings key handling - #1007
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
rcjasub
left a comment
rcjasub
left a comment
There was a problem hiding this comment.
Requesting changes — the fix moves the missing-key check to module load time instead of scoping it to where the key is actually used, which introduces a regression.
packages/db/src/utils/get-embedding.ts now throws as soon as the module is imported, not when getEmbedding() is called. queries.ts statically imports getEmbedding, and its getDefaultDropdownItems() (lines 38–77) never calls it — it only queries IMPs/seed products directly. So the nav-bar's default (empty-query) search dropdown now hard-crashes whenever OPENAI_EMBEDDINGS_KEY is missing, even though that path never touched embeddings before. It also bypasses the graceful-degradation try/catch already in queries.ts/search.ts, since those only wrap runtime calls, not module evaluation.
Suggestion: move the check inside getEmbedding() (or lazily construct the client on first call), consistent with how connection.ts handles DATABASE_URL — checked lazily, at use time, not at import.
Otherwise the fix is correct and the error message is clear for #1004 — just needs to be scoped narrower before merging. Let me know how that goes.
…ns (#1065) ## The problem **The Database Compatibility Check has failed on every push to `main` since #1054 merged, so staging has not been migrated since 11 Aug.** That workflow is what applies committed migrations to staging; while it is red, staging drifts behind `main` — the exact failure mode the workflow's own comments were written to prevent. It is not caused by any individual PR. It fires on `push: main`, which is why PR checks stay green and the failure only appears after a merge. ## Root cause `migrate-remote-db.ts` imports `importer-lib` for `getArg`, a CLI-flag helper. `importer-lib` imports `getEmbedding`. And #1054 added a module-scope throw to `get-embedding.ts`: ``` migrate-remote-db.ts → lib/importer-lib.ts → src/utils/get-embedding.ts → throws at import ``` The workflow's env block has no `OPENAI_EMBEDDINGS_KEY`, and correctly so — applying migrations has nothing to do with embeddings. So a migration job dies on a key it will never use, before it reaches a line of its own code. ## The fix Move the check into the client factory. The rule is unchanged and still fails by name; only its timing moves, from *anything imports this module* to *something asked for an embedding*. ## Verification Both directions, against the real import chain: - With this change, importing exactly what `migrate-remote-db.ts` imports resolves with `OPENAI_EMBEDDINGS_KEY` unset. - Stashing it reproduces the CI error verbatim from the same import. Two tests pin it: the module must import cleanly with no key, and `getEmbedding()` must still reject by name when called without one. `format:check`, `lint`, `type-check`, and the full suite pass. ## Note for #1007 #1007 is still open and proposes this same module-scope throw. Its base already has it (#1054 landed the identical change), so merging it as written would reintroduce this failure. It can be closed as superseded, or rebased onto this. ## After merge The first push to `main` should turn the Database Compatibility Check green and apply the migrations staging has been missing. Worth watching that run rather than assuming it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
3 participants
Description
Fixes #1004
Removes the
NOTAKEYfallback forOPENAI_EMBEDDINGS_KEYin the OpenAI embeddings client setup.Previously, when
OPENAI_EMBEDDINGS_KEYwas missing, the app initialized the OpenAI client with the placeholder valueNOTAKEY, causing inference search to fail later with a generic OpenAI/auth error. This change fails early with a clear missing environment variable error, making the root cause easier to diagnose in logs.Checklist