feat(models): @embed directive + write-time hook + auto-HNSW (Phase 5 of #510)

[heskew]

Summary

Phase 5 of #510. @embed schema directive that auto-fires embedding on writes, stores the vector on the record, and HNSW-indexes the column — no app code.

type Document @table @export {
  id: ID @primaryKey
  content: String
  embedding: Vector @embed(source: "content", model: "default")
}

Three lines replace a custom embedding pipeline.

What ships

• Parser (resources/graphql.ts): @embed(source, model) branch added alongside @computed. Vector added to PRIMITIVE_TYPES. Validates both args are present and string-typed. Auto-attaches @indexed(type: "HNSW") when no explicit @indexed.
• API (resources/Table.ts): Table.setEmbedAttribute(name, embedder) mirrors setComputedAttribute for component-author overrides. Default embedder auto-registered at schema load.
• Write-time hook (resources/Table.ts, resources/models/embedHook.ts): chained into the existing preCommitBlobsForRecordBefore slot at the main put/patch site. Commit blocks on the embedder (sync-by-default).
• Vector coerceType case: JSON Array<number> and Float32Array both accepted and normalized to Float32Array for storage.
• Replicated-write predicate: skips on options.isNotification === true (cluster-subscribe), context.replicateFrom === false (REST x-replicate-from: none), and context.alreadyLogged === true (audit replay). Receivers store the originator's vector as-is — preserves cross-cluster embedding-space consistency.
• Embedder error sanitization: backend URLs / model IDs / API-key tails stay in server logs; the caller sees Failed to compute embedding for attribute "<name>".
• Schema-reload safety: the static embedAttributes filter is recomputed in updatedAttributes() so in-place Table.attributes.splice() at databases.ts:940 doesn't leave a stale snapshot.

Open decision (resolved per Kris's #632 comment)

Sync-by-default per Kris's preference — fits the existing write.before pattern with zero new infrastructure and is expected to be the overwhelming common case ("I would expect that's overwhelmingly the mode that should and would be used").

Low-latency follow-up direction

Per Kris's #632 comment, the right way to minimize write latency is not queued-via-server/jobs/ (the issue body's original suggestion). It's a derived caching table pattern: the origin table commits immediately without waiting for the embedder; a separate derived table holds the vector and follows sync mode against the origin. Origin write latency stays at LMDB-speed; derived-table writes do the embedding work asynchronously.

The same primitive lands re-embed-on-model-change backfill — the derived table is the natural place to iterate-and-rewrite when the schema's @embed(model: ...) changes.

Model-change invalidation — partial

The parser sets property.version = "embed:<model>", so databases.ts:1111's version-change-driven reindex fires when the model changes. But that pathway re-INDEXES existing vectors; it does NOT re-EMBED source fields through the new model. New writes after a model swap pick up the new model; existing rows keep their old-model vectors until re-written. Full re-embed-on-model-change backfill lands with the derived-table follow-up (above).

Verifier cadence

Per the phase-cadence plan, Phase 5 was scheduled for /ultrareview (write-time hook + auto-indexing is non-trivial new behavior). Locally I ran deep-review --multipass (4 domain agents × 2 runs = 8 agents) over the working tree pre-PR. The multipass surfaced:

• (fixed pre-PR) cluster-subscribe replication path doesn't set replicateFrom/alreadyLogged — predicate now also checks options.isNotification === true.
• (fixed pre-PR) Vector was added to PRIMITIVE_TYPES but lacked a coerceType case — added.
• (fixed pre-PR) stale embedAttributes snapshot on schema reload — recomputed in updatedAttributes().
• (fixed pre-PR) @embed(...) with missing source: or model: silently degraded to a no-op — now errors with directive location.
• (fixed pre-PR) embedder backend errors could leak URLs / key tails — sanitized.

/ultrareview recommended pre-merge for multi-model cross-check (this PR's domain is new-behavior class).

Test plan

• Unit: 17 tests in unitTests/resources/models/embedHook.test.js — default embedder, replication predicate (all 3 signals), source-presence semantics, error sanitization.
• Build: npm run build clean.
• Lint: oxlint --deny-warnings clean for touched files.
• Integration: end-to-end with a fake Ollama HTTP server inside the test (integrationTests/server/embed-directive.test.ts) — happy path, source-unchanged PATCH (no re-embed), REST x-replicate-from: none (receiver path) skip.
• CI green (3 Node versions).
• /ultrareview cross-model pass before merge.

Files

Path	Change
resources/graphql.ts	extended — parse @embed, add 'Vector', validate args, auto-HNSW
resources/Table.ts	extended — setEmbedAttribute, write-time hook, Vector coerceType, embedAttributes refresh in updatedAttributes()
resources/models/embedHook.ts	new — default embedder + pre-commit builder + replication predicate + error sanitization
unitTests/resources/models/embedHook.test.js	new — 17 tests

Out of scope (deferred follow-ups)

• Derived caching table for low-latency writes (Kris's preferred direction — see above) — tracked at [Models] Derived caching table for @embed (low-latency writes + model-change backfill) #750. Same primitive lands re-embed-on-model-change backfill.
• Cross-model migration tooling (harper kb reembed --from X --to Y).
• Multimodal embeddings (image, audio).
• Pre-existing tech debt at resources/graphql.ts:170 — the unknown-directive warning condition reads server.knownGraphQLDirectives.includes(directiveName) (warns when the directive IS known). Not touched in this PR.

Tracking: #510
Closes #632

🤖 Generated with Claude Code

[claude]

Default embedder not refreshed on model change.

The !this.userEmbedders[attribute.name] guard correctly avoids clobbering a component-author override set via setEmbedAttribute, but it also prevents the auto-generated default embedder from being updated when the schema is reloaded with a different model: via the databases.ts:940 in-place attributes.splice() path.

updatedAttributes() is called on the existing Table class (not a fresh makeTable()), so userEmbedders carries over. After a @embed(model: "modelB") schema reload, userEmbedders["embedding"] still holds the modelA embedder, and new writes silently use the wrong model. The PR body claims "New writes after a model swap pick up the new model" — the code doesn't support that claim for in-place schema reloads.

To fix without clobbering user overrides, track which names were explicitly set by setEmbedAttribute (e.g., a static userSetEmbedders: Set<string> = new Set()) and skip refresh only for those:

Suggested change

	if (attribute.embed && !this.userEmbedders[attribute.name]) {
	if (attribute.embed && !TableResource.userSetEmbedders?.has(attribute.name)) {

…then add static userSetEmbedders: Set<string> = new Set() alongside userEmbedders and have setEmbedAttribute populate it.

[heskew]

Applied — setEmbedAttribute now tracks user-set names in static userSetEmbedders: Set<string>; defaults refresh on every updatedAttributes() call, user overrides are preserved. Pushed at b108804.

[claude]

ArrayBuffer.isView branch reinterprets bytes rather than converting values.

new Float32Array(value.buffer) copies the raw bytes of the backing ArrayBuffer from offset 0. If value is a non-Float32Array typed array — e.g., Float64Array([1.0, 2.0, 3.0]) — the eight-byte-per-element encoding is reinterpreted as four-byte float32s, producing 6 values of garbage. The same issue exists for typed arrays with a nonzero byteOffset (subarrays/slices).

The fix is to convert element values rather than reinterpret bytes:

Suggested change

	throw new SyntaxError();
	if (ArrayBuffer.isView(value)) return Float32Array.from(value as any);

Float32Array.from iterates the view's elements and produces the correct float32 representation of each numeric value. (DataView is not iterable, but it is also not a plausible input here.)

[heskew]

Applied — switched to Float32Array.from(value) which iterates element values rather than reinterpreting bytes. Float64Array / subarray inputs now convert correctly. Pushed at b108804.

[claude]

2 blockers found (new findings on 47bd103c4).

1. Source-changing PATCH (the CRITICAL fix) has no test

File: resources/Table.ts lines ~1248, ~1257
What: update() now wraps both _writeUpdate call sites with when(...) to propagate the embed promise. The integration test only exercises the skip-path of update() — PATCH without the source field, where buildEmbedBefore returns undefined and _writeUpdate returns synchronously. No test exercises PATCH with the source field present, which is the only path that exercises the async branch the fix was written for.
Why it matters: This was the CRITICAL fix in 47bd103c4 — silent data loss / "transaction already closed" crash on source-changing PATCHes. The fix is plausible but the production path is unverified end-to-end.
Suggested fix: Add an integration test case: seed a record, PATCH it with { content: 'updated text' }, verify embed fires once and the stored vector matches deterministicVector('updated text').

2. getFromSource caching-table embed path has no test

File: resources/Table.ts ~line 4509
What: The MEDIUM fix from Kris's review wires buildEmbedBefore into the getFromSource write path (the path that bypasses _writeUpdate). Only the false branch of if (embedBefore) has pre-existing coverage; the true branch (embed fires on a caching table) has none.
Why it matters: Kris called this "one of the most compelling use cases." A silent mis-wire here would mean caching tables with @embed never embed — the same failure mode the fix was supposed to close.
Suggested fix: Add an integration test: a source table + a caching table with @embed, write to the source, verify the derived-table record has a populated vector.

[kriszyp]

It seems like we are entirely missing support for embeddings with getFromSource writes. And using embeddings with caching tables actually seems like one of the most compelling uses cases.

My Gemini review caught a number of things:

1. Unhandled Promise in TableResource.update leads to Transaction Commits before Embed resolves — CRITICAL (Confirmed)

File: Table.ts
Domain: concurrency
What: The update(updates, fullUpdate) instance method of TableResource calls this._writeUpdate(...) which returns a Promise when @embed is active. However, update() completely ignores this promise and returns this
synchronously.
Why it matters: Because update() returns synchronously, the calling application will immediately proceed to run await instance.save() . However, the asynchronous embedder operation is still pending, meaning the write has not been
registered in the transaction's write list yet. The transaction will either commit without saving the updated record (causing silent data loss) or crash with a "Transaction already closed" error when the embedder finally resolves and
tries to invoke addWrite() on a committed transaction.
Code:

// lines 1243-1247                                                                                                                                                                                                                     
return when(loading, () => {                                                                                                                                                                                                           
    this.#changes = updates;                                                                                                                                                                                                              
    this._writeUpdate(id, this.#changes, false);                                                                                                                                                                                          
    return this;                                                                                                                                                                                                                          
});                                                                                                                                                                                                                                    
...                                                                                                                                                                                                                                    
// line 1251-1252                                                                                                                                                                                                                      
this._writeUpdate(id, this.#changes, fullUpdate);                                                                                                                                                                                      
return this;                                                                                                                                                                                                                           

Suggested fix:
Wrap both invocations of _writeUpdate inside the update method with Harper's when() helper so that the promise is properly propagated back to the caller when an asynchronous hook is active:

return when(loading, () => {                                                                                                                                                                                                           
    this.#changes = updates;                                                                                                                                                                                                              
    return when(this._writeUpdate(id, this.#changes, false), () => this);                                                                                                                                                                 
});                                                                                                                                                                                                                                    
...                                                                                                                                                                                                                                    
return when(this._writeUpdate(id, this.#changes, fullUpdate), () => this);                                                                                                                                                             

──────

2. coerceType for Vector returns Float32Array which is mangled by the Record Encoder — HIGH (Confirmed)

File: Table.ts
Domain: data-integrity
What: coerceType converts incoming Vector attribute values into a Float32Array . However, as documented in createDefaultEmbedder , Harper's record encoder ( updateAndFreeze ) does not cleanly serialize typed arrays, instead
mangling them into {0: val1, 1: val2, ...} objects in the database.
Why it matters: While the default embedder circumvents this by casting its result back to Array , any user-supplied vectors (e.g., replication receiver paths, programmatic writes, or manual API uploads) will pass through
coerceType and be coerced to a Float32Array . These will get stored in the database in a mangled object format, corrupting the vector column and breaking subsequent HNSW indexing and search operations.
Code:

case 'Vector':                                                                                                                                                                                                                         
    if (value === null || value === 'null') return null;                                                                                                                                                                                  
    if (value instanceof Float32Array) return value;                                                                                                                                                                                      
    if (Array.isArray(value)) return Float32Array.from(value);                                                                                                                                                                            
    if (ArrayBuffer.isView(value)) return Float32Array.from(value as any);                                                                                                                                                                
    throw new SyntaxError();                                                                                                                                                                                                              

Suggested fix:
Coerce vectors to standard JavaScript arrays ( number[] ) instead of Float32Array s, ensuring they bypass record-encoder mangling while remaining compatible with HNSW:

case 'Vector':                                                                                                                                                                                                                         
    if (value === null || value === 'null') return null;                                                                                                                                                                                  
    if (value instanceof Float32Array) return Array.from(value);                                                                                                                                                                          
    if (Array.isArray(value)) return value.map(Number);                                                                                                                                                                                   
    if (ArrayBuffer.isView(value)) return Array.from(value as any);                                                                                                                                                                       
    throw new SyntaxError();                                                                                                                                                                                                              

──────

3. Model Change via Schema Reload causes Dimension Mismatches or Mixed-Model Vectors in HNSW — MEDIUM (Confirmed)

File: graphql.ts and embedHook.ts (comments)
Domain: data-integrity / config
What: Modifying the @embed(model: "...") argument in a schema file updates the attribute version, triggering a schema reload and an HNSW reindex. However, existing rows are not automatically re-embedded through the new model.
Why it matters: If a user changes their embedding model (e.g., swapping a 384-dimension model for a 1536-dimension model), existing rows retain their old-dimension vectors. Loading these mixed-dimension vectors into the same HNSW
index
will either trigger a crash during index construction or severely pollute the embedding space, returning garbage query results.
Suggested fix:

1. Document this operational hazard explicitly.
2. Consider adding validation to block schema updates that change model definitions on populated tables unless the vector attribute is cleared, or plan a background job pathway to invalidate and backfill the vector column when a
   model version change is detected.
   ──────

4. Malformed @embed Directives Fail Silently during Schema Deployment — LOW (Confirmed)

File: graphql.ts
Domain: api / config
What: When a malformed @embed directive is encountered (e.g., omitting source or model , or supplying non-string literals), the schema compiler logs via console.error and skips registering the @embed property metadata, but
continues compiling.
Why it matters: Because no exception is thrown, the overall schema installation succeeds with a 200 OK . The field is silently registered as a regular, un-embedded Vector attribute. Users receive no indication that their @embed
directive was discarded unless they inspect the server console logs.
Code:

if (!embedDefinition.source || !embedDefinition.model) {
    console.error(
        `@embed on "${property.name}" requires both "source" and "model" arguments, at`,
        directive.loc
    );
}

Suggested fix:
Throw a descriptive ClientError to halt compilation and fail the schema deployment loudly, returning a helpful error message to the client:

if (!embedDefinition.source || !embedDefinition.model) {
    throw new ClientError(
        `@embed on "${property.name}" requires both "source" and "model" arguments`,
        400
    );
}

──────

5. Dependency on globalThis.logger in embedHook.ts — LOW (Likely)

File: embedHook.ts
Domain: config
What: Failure logging in embedHook.ts relies on resolving the logger dynamically via (globalThis as any).logger rather than using standard module imports.
Why it matters: Relying on globalThis for logger references deviates from ES module standards, bypasses static type checks, and can lead to unhandled runtime exceptions if the logger isn't populated on globalThis in specific
worker threads.
Code:

const logger = (globalThis as any).logger;
logger?.error?.(`Embedder for attribute "${attr.name}" failed:`, err);

Suggested fix:
Import the standard Harper logger statically:

import { logger } from '#src/utility/logging/logger.ts';

[kriszyp]

Sequential? Why not in parallel?

[kriszyp]

So paranoid. We have prototype pollution protection (freezing). (not a blocker, just a nit)

[kriszyp]

This is a clever idea. But does it really mean only Float32? Or should this also allow a Float64 and Float16? (seems like it should). (I would actually love to move to using Float16 in HNSW)