[theothersideofgod]

Why

A tenant who uploads a text file and then searches with plain text gets nothing useful. vectorEmbedding: { text } fails validation, because vector is required. unifiedSearch text and {column}Text mutations do not embed. graphile-llm embedded the text inside a wrapped resolve(). Grafast calls resolve() after the plan's SQL has already run. So the search gets no vector, and the mutation reports success but writes a NULL vector. This change embeds the text while the operation is planned, before Grafast builds the SQL.

What changed

• New embed-inputs.ts. Each input field that carries text is tagged with an embed rule where it is declared. A finalize pass wraps the plan of each field whose arguments can reach a tagged field. The wrapper replaces fieldArgs.getRaw, so connection-filter's where and the create and update args.apply read a step that embeds the text first.
• text-search-plugin.ts and text-mutation-plugin.ts lose their resolve() wrappers and only tag fields. VectorNearbyInput.vector becomes optional when graphile-llm is loaded. On a create input, the vector field becomes optional when its {column}Text companion exists.
• The embedder result is normalized in one place. build.llmEmbedder returns { embedding }, or number[] | null when metering wraps it. Per-database model overrides now apply to mutations too, not only to search.

Scope

• Covers vectorEmbedding.text, unifiedSearch, and {column}Text on create and update. Keeps the current rules: a non-empty vector wins over text, search degrades on a failed embed by default, and a mutation throws on a failed embed.
• Leaves out the metering plugin. It sets its context in a wrapped resolve(), so the embeds this PR moves to plan time run outside it. Today no environment can reach this. On main, GraphileLlmPreset({ metering: true }) fails at schema build with a llmEmbedder naming conflict. With the conflict bypassed, its root-field resolve() wrapper fails planning for connection and mutation fields. The metering redesign is tracked in constructive-io/constructive-planning#2437.
• Leaves out graphile-upload-plugin (multipart *Upload, same resolve() problem, owner decision pending) and rag-plugin.ts, which reads { embedding } and breaks when metering returns an array.

Tradeoffs

• Replacing fieldArgs.getRaw depends on grafast 1.1.2 reading getRaw through the FieldArgs object (operationPlan-input.js). The tests fail loudly if an upgrade changes that.
• Rejected: overriding the where argument's applyPlan. It copies connection-filter's body and does not reach mutations.
• Rejected: an execute middleware that embeds before planning. It must rewrite the document or variables, and it changes plan-cache behavior.

Blast Radius

Only fields whose arguments can reach a tagged field are wrapped. Fields with no vector or unifiedSearch input are untouched. Generated SDKs will see vector, and the create-input vector field, as optional after the next codegen. Clients that send vectors keep working. The public API only grows. embedTextInWhere stays exported unchanged and is marked deprecated, because the plugins no longer call it. Draft #1744 also edits graphile-llm.test.ts and may conflict. A VectorNearbyInput with neither vector nor text is now an error, not a silently dropped filter. That found the graphile-settings kitchen-sink query sending { nearby: { embedding } }. That field does not exist, and real validation rejects it. The test now sends { vector, metric, distance }.

Verification

• graphile-llm: the new plan-time-embedding.test.ts fails 5 of 5 on the test-only commit, then passes 5 of 5 with the fix. Real Postgres and Ollama.
• Full graphile-llm suite: 76 passed. embed-inputs.test.ts runs real graphql.validate and covers both embedder shapes, degrade and throw, and cyclic filter types.
• A local cluster ran this image against a tenant with uploaded text files. Before, a text-only search returned Field "VectorNearbyInput.vector" of required type "[Float!]!" was not provided. After, it ranked the matching file first at distance 0.286627. That matches a client-sent vector from the same Ollama exactly. An update with a {column}Text value stored a 768-dimension vector at cosine distance 0.0 from that Ollama's embedding.

Full record for reviewers (root causes with file:line, mechanism, decisions, test matrix, known gaps, what is not verified): #1953 (comment)

🤖 Generated with Claude Code

[tenki-reviewer]

Review complete. 🟠 1 high

📍 Findings outside the diff (1) — 🟠 1 high — defects on lines GitHub can't attach comments to

🟠 High — Plan-time embedding bypasses metering quota checks · graphile/graphile-llm/src/plugins/metering-plugin.ts

Embedding now runs inside grafast lambda steps during plan execution (graphile/graphile-llm/src/embed-inputs.ts:274), but the metering context is still established only inside a resolve wrapper (graphile/graphile-llm/src/plugins/metering-plugin.ts:222) — and the package's own header documents that resolve runs after the plan has executed. The metered wrapper therefore sees an empty meteringStore and takes the unmetered pass-through branch (graphile/graphile-llm/src/plugins/metering-plugin.ts:119). With LlmMeteringPlugin loaded, check_billing_quota is never consulted and record_usage is never called for plan-time embeddings: quota-exceeded embeddings silently succeed and usage goes unrecorded, making the 'throw'/EMBED_QUOTA_EXCEEDED paths unreachable in production.

🧹 Nitpicks (3) — 🟢 3 low

• 🟢 Remove per-embed console.log on hot path (embed-inputs.ts:88) — Every successful embedding call synchronously writes a log line with dims and latency (graphile/graphile-llm/src/embed-inputs.ts:89), and the metering wrapper logs the same call again (graphile/graphile-llm/src/plugins/metering-plugin.ts:124).
• 🟢 Removing embedTextInWhere export without version bump (index.ts:44) — embedTextInWhere was removed from the package's public API along with its unit tests in this PR (graphile/graphile-llm/src/index.ts:44 no longer exports it), while package.json remains at version 1.21.6 with no bump signaling the removal.
• 🟢 Empty-array embedding bypasses null handling (embed-inputs.ts:49) — toVectorEmbedder treats the two embedder shapes inconsistently: the raw-array path returns [] as-is (graphile/graphile-llm/src/embed-inputs.ts:49), while the {embedding} path returns null for an empty array (graphile/graphile-llm/src/embed-inputs.ts:50).

---

This PR reworks embedding in graphile-llm so user-supplied text arguments are embedded at Grafast plan time instead of inside resolvers, with graceful degradation when no embedder is available.

Files	Change
src/embed-inputs.ts	New plan-time embedding engine: field-argument wrapping, companion text fields, and embedder normalization.
src/plugins/text-search-plugin.ts	Search/nearby filters now use plan-time embedding; degrade paths drop the vector leg when the embedder is missing.
src/plugins/text-mutation-plugin.ts	Mutation inputs gain nullable vector plus companion text fields embedded at plan time.
src/index.ts	Public exports updated; embedTextInWhere resolver-based export removed.
src/embedder.ts, src/plugins/llm-module-plugin.ts	Embedder construction and LLM config plumbing adapted to the new engine.
src/__tests__/*	Tests rewritten for plan-time embedding, companion fields, and degrade behavior.

_{Reviewed commit: c1402b6}

[theothersideofgod]

Review notes

This comment is the full record behind the PR description, for human and agent reviewers. Every claim below cites a file and line, a command and its output, or a PR. Anything not verified is marked not verified.

1. What users get

Before	After
vectorEmbedding: { text } fails validation: Field "VectorNearbyInput.vector" of required type "[Float!]!" was not provided.	It is accepted. The server embeds the text and ranks by it.
vectorEmbedding: { text, vector: [] } passes validation, but nothing is ranked (embeddingVectorDistance: null).	vector: [] counts as absent, so the text is embedded.
unifiedSearch: "..." matches tsvector only.	pgvector participates too.
{column}Text on create or update reports success and stores NULL.	The embedded vector is stored.
A client that sends vector	Unchanged. The embedder is not called.

2. Root causes

1. text-search-plugin.ts and text-mutation-plugin.ts (base 299b3920e) embedded inside a wrapped resolve(). Grafast calls resolve() with the plan result, after SQL has run (grafast 1.1.2 dist/engine/OperationPlan.js:741-746). Measured: the embedder was called 0 times.
2. graphile-search/src/adapters/pgvector.ts:151-155 declares vector as [Float!]!.
3. build.llmEmbedder returns { embedding } (src/types.ts:22). Only the metering wrapper returns number[] | null (src/plugins/metering-plugin.ts:115-134). The plugins cast it to number[]. Production loads GraphileLlmPreset() with no options (graphile-settings/src/presets/constructive-preset.ts:228-229).
4. !value.vector treated [] as present (base text-search-plugin.ts:112).
5. Why tests missed it: graphile-test calls grafast execute() without GraphQL validation (graphile-test/src/context.ts:316). The old unit tests called embedTextInWhere directly, never through Grafast.

3. How the fix works

• Input fields that carry text are tagged with an EmbedRule in extensions (src/embed-inputs.ts). There are three kinds: nearby, unified, and companion.
• A finalize pass (planTimeEmbedding) wraps extensions.grafast.plan on every object field whose arguments can reach a tagged field. Reachability is a fixed point, because filter types are cyclic.
• The wrapper replaces fieldArgs.getRaw on that field's FieldArgs object. It returns lambda({ raw, ctx: context() }, embedValue) for an embeddable argument root, and access below it, built in the root layer plan like grafast's own getRaw (dist/operationPlan-input.js:65-66).
• The replacement reaches every consumer because grafast reads fieldArgs.getRaw as a property at call time. That covers fieldArgs.apply (operationPlan-input.js:145), processAfter for argument applyPlan (:189, :201), the $arg getters (:163), and getBaked (:101). So connection-filter's where applyPlan (graphile-connection-filter/src/plugins/ConnectionFilterArgPlugin.ts:96-120) and the create and update args.apply($insert) (graphile-build-pg PgMutationCreatePlugin.js:201-205) run unmodified and receive the embedded value.
• Risk. This relies on grafast 1.1.2 internals, not a documented contract. If a grafast upgrade changes it, the tests fail loudly: embedder call counts and distances are asserted.
• Rejected alternatives:
  • Override the where applyPlan. This copies connection-filter's body and does not reach mutations. A prototype worked for search only.
  • An execute middleware that embeds before planning. It must rewrite documents or variables.
  • Embed inside buildFilterApply. apply callbacks are synchronous (graphile-search/src/plugin.ts:898-918).

4. Behavior decisions

• A non-empty vector wins over text. The embedder is not called.
• A VectorNearbyInput with neither vector nor text is an error. This used to be a silent no-op. It is how the invalid kitchen-sink query surfaced (see §7).
• With no embedder configured, search text is ignored with no new error. A mutation throws EMBED_NOT_CONFIGURED.
• Search uses onQuotaExceeded: 'degrade' by default, with 'throw' as an option. A mutation throws on a failed embed. An empty vector counts as a failed embed.
• Per-database overrides (ctx.constructive.useLlm() through llmConfigStore.run) now apply to mutations as well as search. They are resolved only when something needs embedding.

5. Public API and schema

• New exports: embedField, planTimeEmbedding, and EmbedRule. Nothing is removed. embedTextInWhere is still exported unchanged and marked @deprecated. Its original tests are restored.
• VectorNearbyInput.vector becomes nullable when graphile-llm is loaded. On a create input, the vector field becomes optional when its {column}Text companion exists.
• Generated SDKs will see both as optional after the next codegen. sdk/constructive-sdk/src/agent/orm/input-types.ts is not regenerated here. Not verified: the effect on any generated client.

6. Verification

Check	Result
plan-time-embedding.test.ts on test-only commit fa8f37f1a	5 failed. On the fix: 5 passed. Real Postgres and Ollama.
embed-inputs.test.ts	26 tests. Runs real graphql.validate, both embedder shapes, degrade and throw, an empty vector, per-database overrides, no embedder, cyclic filter types.
cd graphile/graphile-llm && pnpm test at 047e50374	5 passed suites, 96 passed tests
graphile-settings/__tests__/preset-integration.test.ts	38 passed
CI at 265ebee0e	19 of 19 checks passed
Local kind cluster, tenant with uploaded .txt files, image built from c1402b65c plus constructive-db#4272. Later commits (265ebee0e test only, 54f3648ee empty vector, 047e50374 export kept) were not run on the cluster	Text-only search ranked the matching file first at 0.286627, equal to a client vector from the same Ollama. {column}Text stored a 768-dimension vector at cosine distance 0.0. The server logged [graphile-llm] embed: dims=768.

ai-tests in .github/workflows/run-tests.yaml:441-540 provides Ollama with nomic-embed-text for graphile/graphile-llm.

7. Review history

• An independent review found that the provisional memo in reaches() was wrong on cyclic types. Fixed in c1402b65c, with a test that fails on the old code. Same commit: the getRaw access/object steps moved to the root layer plan, and a guard was added for a VectorNearbyInput without vector.
• CI failure: graphile-settings kitchen-sink sent vectorEmbedding: { nearby: { embedding } }. That field does not exist. graphql.validate returns Field "nearby" is not defined by type "VectorNearbyInput". It passed before only because the harness skips validation and the adapter dropped the filter. The test now sends { vector, metric, distance } (265ebee0e).
• tenki:
  • Empty-array shape: fixed (54f3648ee).
  • embedTextInWhere removal: reverted, kept deprecated (047e50374).
  • Per-embed log: kept, because the old paths also logged once per embed.
  • Metering bypass: see §8.

8. Known gaps, not fixed here

Tracked in constructive-io/constructive-planning#2437.

• Metering. GraphileLlmPreset({ metering: true }) fails at schema build on main, on this branch, and on Establish complete PostgreSQL request context across Graphile execution #1744 head 10574e19f: A naming conflict has occurred - two entities have tried to define the same key 'llmEmbedder'. With that bypassed:

  • its root-field resolve() wrapper fails planning (Query.allDocs ... is forbidden from defining a GraphQL resolver);
  • plan-time embeds run outside its context (0 metered calls);
  • RAG crashes on the number[] shape.

  The embeds this PR moves to plan time are therefore unmetered, but no environment can turn metering on today.

• RAG. rag-plugin.ts:293, 386 destructures { embedding }.

• Multipart *Upload. graphile-upload-plugin has the same resolve() timing bug. Owner decision pending (fix(export): hoist partman registry fixtures into service package; graphile-llm collision; X-Api-Name lookup #1476, fix(pgpm): cascade --no-tty through init template scaffolding #1477).

9. Not verified

• A filter reached under a list parent (backward relation inside a list), and the layer plan placement of the embed step there.
• Upsert, bulk mutations, and aggregates in the full ConstructivePreset, with {column}Text or text filters.
• Concurrent requests with different per-database overrides.
• The behavior when the embedder throws, as opposed to returning null.
• Why the old text-search resolve() wrapper did not trigger the planning error that the metering wrapper triggers.

10. Related

• constructive-io/constructive-db#4272 points the GraphQL servers at ollama-svc (EMBEDDER_BASE_URL). Both PRs are needed for text search on a cluster.
• Draft Establish complete PostgreSQL request context across Graphile execution #1744 also edits graphile-llm.test.ts and metering-plugin.ts. It does not fix the metering gaps above.
• Planning: fix(export): hoist partman registry fixtures into service package; graphile-llm collision; X-Api-Name lookup #1476, fix(pgpm): cascade --no-tty through init template scaffolding #1477 (uploads), chore: update schemas from constructive-db #911 (metering tests), #2292, #2293 (server-side embeddings design).

[theothersideofgod]

Thanks. Triage of each finding:

High: plan-time embedding bypasses metering. The reasoning is right. The embeds now run before the metering context exists. No environment can reach it today, though, because the metering plugin cannot be turned on. On main (299b3920e) and on this branch, GraphileLlmPreset({ metering: true }) fails at schema build:

A naming conflict has occurred - two entities have tried to define the same key 'llmEmbedder'.

metering-plugin.ts:192-194 calls build.extend on a key LlmModulePlugin already set. With that conflict bypassed locally, the plugin's root-field resolve() wrapper fails planning (Query.allDocs ... is forbidden from defining a GraphQL resolver). RAG embedText also records 0 metered calls and crashes on the metered number[] shape. Making metering work needs a redesign, so it can establish its context where each embed runs. This PR does not do that. It is tracked in constructive-io/constructive-planning#2437, and the PR description's Scope now says so.

Low: per-embed console.log. Kept on purpose. The old search and mutation paths each logged one line per embed (Search embed, Mutation embed). This PR merges them into one line. The double line appears only when metering is loaded, and metering cannot load today.

Low: embedTextInWhere removed without a version bump. Fixed in 047e503 by not removing it. The function is exported again, unchanged, and marked deprecated, and its original tests are restored. The public API now only gains embedField, planTimeEmbedding and EmbedRule, so no major bump is needed.

Low: empty-array embedding bypasses null handling. Fixed in 54f3648. An empty vector from either embedder shape is now a failed embed, so search degrades or throws and a mutation throws. A test fails before the fix and passes after it.