Skip to content

Shared derived-index runtime for native backends - #2548

Closed
kriszyp wants to merge 88 commits into
codex/fulltext-storage-adapterfrom
feat/derived-index-native-backend-runtime
Closed

kriszyp wants to merge 88 commits into
codex/fulltext-storage-adapterfrom
feat/derived-index-native-backend-runtime

Conversation

@kriszyp

@kriszyp kriszyp commented Sep 9, 2026

Copy link
Copy Markdown
Member

The shared post-commit derived-index runtime for RocksDB tables: lock-elected delivery of committed transaction-log mutations to a native index backend, exact cursor resume, rebuild from primary records, cross-worker readiness, and opt-in writer backpressure. This PR carries the whole stack — #2533 (runtime foundation), #2535 (RocksDB storage surface) and the native-backend additions — so one review covers the derived index end to end and one merge lands it. #2430 (native HNSW) is rebuilt on this branch and is the first consumer; nothing here has a production caller until it lands.

What is here

Runtime (resources/derivedIndexRuntime.ts). One DerivedIndexRuntime per database wakes on the root store's committed event; each registered backend has its own process-wide lock, runner, cursor vector (log name → completed transaction timestamp) and readiness. The elected runner reads committed entries from every physical log, collects identities per transaction, resolves each changed record once against the primary store after its last occurrence, projects only the registered attributes, and delivers bounded batches. Resume proves every saved cursor with exactStart; a boundary the log cannot resolve, a corrupt frame, a removed log or a reload marker condemns the generation and rebuilds. Nothing on the commit path is added beyond the log entry that already exists and a local-only eviction marker for registered caching tables.

Backend contract. One interface: attach(host) (the owner-epoch fence), deliver(batch) (accept/defer/fail), flush(reason) (barrier request), shutdown(epoch) (quiescence), getDurableCursor(), onStateChange(), and optional reset(epoch). batch.records is the last-write-wins view over distinct keys; batch.through is the cursor vector the batch completes and is withheld until an oversized transaction's closing chunk. Every hook is required — a backend that completes inside deliver() implements them trivially — because work that survives a method return is what an ownership handoff has to fence.

Rebuild. rebuilding is published, the previous epoch is quiesced, reset(newEpoch) runs, the committed tail of every log is captured, primary records are scanned and delivered in bounded chunks, the final chunk carries the tail as through, and replay resumes after it. Bounded retry with backoff; unavailable after the budget, honoured by peers.

Shared readiness. State, reason code, attempt count, rebuild request, lag flag and the owner-epoch counter in one getUserSharedBuffer allocation per index, read with plain Atomics.loads on any worker (readDerivedIndexReadiness). Same dependency as primary-key allocation, blob holds and HNSW node ids already carry.

Lag policy (opt-in, maxLagMilliseconds). The owner measures cursor distance, time parked, unread-log age and undurable-work age; past the budget every worker rejects local user writes to the index's tables with a retryable 503 (DERIVED_INDEX_LAGGING) at the staging layer (_writeUpdate / _writeDelete / _writeInvalidate / _writeRelocate), never canonical-source applies, replay or replication notifications. Cleared with hysteresis once the owner proves catch-up, and on every park the runtime cannot leave on its own.

Storage surface (resources/RocksDerivedIndexStorage.ts, from #2535). Binary, WAL-backed key/value for a native index that keeps its state in RocksDB (Tantivy); HNSW does not use it.

Simplifications made in review

The design was cut down before asking for review, each item verified against source rather than inferred:

  • One backend contract instead of a synchronous/asynchronous split with a discriminant, registration-time hook validation for one side, a runtime check that a "synchronous" flush returned no promise, and a quiescence path for that undeclared promise. No shipped or planned backend is synchronous.
  • Plain words instead of a sequence lock. Nothing read the free-form reason string the seqlock existed to publish; the shared record carries a DerivedIndexReadinessReason code, the epoch counter moved into the same buffer, and the 512-byte record became 32 bytes with no spin. This removed the only construct in the runtime that was novel with respect to Harper's existing use of Atomics over rocksdb-js shared buffers.
  • Committed-tail rebuild anchor instead of the oldest retained entry. rocksdb-js advances lastCommittedPosition only to the earliest still-uncommitted write (TransactionLogStore::commitFinished, uncommittedTransactionPositions.front()), so a committed read is a contiguous physical prefix and nothing committed after the capture can sit behind the tail. The whole-retention-window replay after every rebuild, the capture-time reload-marker suppression, its Date.now() comparison against transaction timestamps, the shared reload word and the backward-clock residual are gone; a reload marker is met exactly once.
  • Lag latch clears on every terminal park, not only on unavailable: a backend without reset, a runtime without scanRecords, a condemnation marker that could not be written, and entering a rebuild (no cursor to guard; readers act on rebuilding). Before this, three paths released ownership with the latch set and nothing scheduled to clear it, so writes stayed 503 indefinitely; one of them needed a commit wake to retry, and commits were what was being shed.
  • partial dropped from DerivedIndexTransaction (nothing consumed it; the withheld cursor already says what is certified), and the per-log cursor checks that could not fail after the whole-vector match removed.
  • A log that has never written a file retains its beginning. rocksdb-js reports oldestSequenceNumber: 0 with fileCount: 0 for it; the === 1 test read every brand-new database as a retention gap and failed every rebuild attempt. Found by restacking Native HNSW index: file-primary mmap graph as a backend on the shared derived-index runtime #2430 and defining a fresh table — the first real consumer exercising the runtime.

Verification

  • Derived suites (derivedIndexRuntime, derivedIndexRuntimeNativeBackend, derivedIndexRuntimeRocks, derivedIndexRegistry, rocksDerivedIndexStorage): 95 passing. The native-backend suite covers coalescing, oversized transactions, deferral, cadence, rebuild/retry/exhaustion, handoff with an apply scheduled and with a flush pending, shutdown failure holding the lock, condemnation across a simulated restart, lag trip/clear/hysteresis, the three terminal-park cases (each proven red without the fix), tail-anchored rebuilds across several logs, and a real worker thread reading the owner's publication through the binding.
  • End to end on real audited RocksDB tables: commit → runtime → queued backend → barrier → readiness; primary scan and replay; write shedding at the staging layer with canonical-source bypass.
  • Native HNSW index: file-primary mmap graph as a backend on the shared derived-index runtime #2430 restacked on this branch: vectorIndexPlane.test.js 22 passing against @harperfast/hnsw 0.2.1 — two workers into one native file, a committed write replayed after its worker dies, reload markers, a cursor outside retention, an empty index answering no results, a populated index that lost its file answering 503.
  • npm run build, tsc --noEmit, lint:required, prettier: clean.

Refs #2489. Supersedes #2533 and #2535, whose commits this branch contains.

🤖 Generated with Claude Code

kriszyp and others added 22 commits September 8, 2026 10:39
…testing

#2276 landed the deflake this branch was opened for — a per-index `random` seam and an
eight-seed sweep — so what is left here are the two executable guards that keep the
comparison from passing vacuously, rebased onto that sweep.

A pinned graph raises the opposite problem to a random one: on roughly 40% of graphs
layer 0 alone reaches the true neighbours from any entry point, and there the
comparison passes with the descent deleted outright. The test now deletes the descent
on every seed and requires at least one of the eight to change its results. Requiring
it per seed would be wrong — descent-sensitivity is a property of the graph, so it
would fail on the insensitive members of a legitimately chosen seed list.

It also records the ef the layers above 0 actually receive and requires it to be
ROUTING_EF. Without that the comparison survives the optimization being removed: hand
those layers the full ef in production and both sides search identically, leaving a
test named for greedy routing green while nothing routes greedily.

Finally, the seeded stream now counts its draws and asserts one per node. The seeds
name specific graphs only while that holds; a second consumer of the stream would
shift every level after it and silently re-pin all eight to graphs none of the
recorded measurements were taken on.

Fixes #2372

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017h3UiYpKgssZiQEjwu2Rij
Pre-push review nit, both comment-only: the `resolveAuditPosition` JSDoc inventoried its own
parameters before reaching the rule that earns it, and the ROUTING_EF assertion's comment argued
to a reviewer rather than telling the next reader what the assertion is for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017S9s2kMDR5BL9CD33PhoUn
`resolveAuditPosition`'s refs-present-but-unresolved return is the branch this change altered and
nothing exercised it: both existing divergent-head cases resolve a ref and return early. Pins it to
the position the record reports for itself.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017S9s2kMDR5BL9CD33PhoUn
…al config (#2484)

* fix(sql): wire sql.* config keys to the real config layer

sqlEngine/config.ts read engine/allowFullScan/maxSortRows/maxHashRows
from globalThis.harperConfig.sql, which nothing in production ever
assigned — only three unit-test files set it as scaffolding. A value
set under sql.* in harperdb-config.yaml never reached the SQL engine;
sql.engine only appeared to work because it also has a
HARPER_SQL_ENGINE env fallback.

Register the four keys in CONFIG_PARAMS (utility/hdbTerms.ts) and read
them via configUtils.getConfigValue(), the same accessor every other
config domain uses. getConfigValue() returns undefined pre-boot rather
than eagerly initializing from disk, preserving the "works without a
fully booted config" property the globalThis branch existed for, and
it self-initializes correctly per worker thread with no new boot hook
to wire in. Delete the globalThis branch entirely.

Switch the three scaffolding test files (join/mutation/aggregate) from
mutating globalThis.harperConfig to configUtils.updateConfigObject(),
the already-sanctioned in-memory config override unit tests use
elsewhere. Add router.test.js coverage proving sql.engine/allowFullScan/
maxSortRows/maxHashRows are actually read from Harper config (not just
the env var), and that the env var still wins for sql.engine.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ytbJQ44LZsFyZ1C1fFdNk

* fix(sql): validate sql.* config and harden tests per plan review

Addresses a plan-mode cross-model review (Framing-Verdict:
better-alternative-exists) of the prior commit's accessor-swap fix:

- Add a scoped `sql` Joi schema (validation/configValidator.ts) so a
  malformed sql.* value (bad engine enum, wrong-typed allowFullScan,
  non-positive/non-integer row caps, an unknown key) is rejected loudly
  at boot or on set_configuration, instead of silently keeping the
  default — the top-level schema's allowUnknown:true previously let an
  entire malformed `sql:` section through unvalidated.
- Correct sqlEngine/PLAN.md's stale `sql.engine.allowFullScan` /
  `sql.engine.maxSortRows` / `sql.engine.maxHashRows` phrasing to match
  the actual sibling-key shape SqlEngineConfig has always used — the
  review flagged this as a real doc/code contradiction an operator
  could be misled by.
- Switch the sql.* test scaffolding (join/mutation/aggregate.test.js,
  and this fix's own router.test.js coverage) from blindly resetting to
  `undefined` to snapshot/restore, so a suite doesn't clobber a value
  set by another one sharing the same mocha process.
- Add getSqlEngineConfig() coverage for wrong-typed/unrecognized config
  values (defense-in-depth: Joi's coercion at validate time is never
  written back into flatConfigObj, so the accessor's own typeof guards
  are what actually protect a live read).
- Add registration + set_configuration rejection tests
  (unitTests/config/setConfigurationSql.test.js) and Joi schema tests
  (unitTests/validation/configValidator.test.js), following existing
  precedents (replicationReceiveQueueParam.test.js's registration
  pattern, the blob-gap-floor schema tests) rather than exercising
  setConfiguration()'s full success path, which would write to this
  box's shared on-disk test config.

Deliberately not adopted, with disqualifiers recorded in the PR body's
"For the human reviewer" section: resolving one config snapshot per
SQL statement (the review's hot-path suggestion), and a full HTTP
integration boot test for sql.engine/allowFullScan specifically (the
'auto' engine's legacy fallback masks the config-driven difference at
the HTTP-observable level, so a naive version of that test would pass
on both the fixed and the reverted code).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ytbJQ44LZsFyZ1C1fFdNk

* fix(sql): close full-review findings on the sql.* config validation

Adopts the concrete, fixable findings from the full pre-push review
(gemini + cursor-composer + Harper domain adjudication) of the
previous two commits:

- sqlSchema now sets convert:false, so a quoted allowFullScan:"true"
  or maxSortRows:"500" in harperdb-config.yaml is REJECTED at boot
  instead of silently passing Joi (which coerces it) and then being
  dropped by getSqlEngineConfig()'s typeof guard — validateConfig()
  never writes the coerced value back into configDoc for sql the way
  it does for threads/logging/storage, so leaving convert:true on
  would have made the new schema's strictness a no-op for exactly the
  scenario it exists to catch.
- Tighten maxSortRows/maxHashRows's defense-in-depth guard from
  typeof === 'number' to isPositiveInteger (rejects NaN/negative/
  fractional caps too — NaN in particular defeats PhysicalSort's
  `buf.length >= cap` guard entirely, since every comparison against
  NaN is false).
- router.test.js: clear the four sql.* keys before each test instead
  of only snapshotting, so the default-value assertions can't go red
  on a machine whose own harper-config.yaml already sets one of them;
  add a flattenConfig() unit test covering the nested-to-flat key
  derivation the other tests bypass via updateConfigObject().
- join.test.js: drop three per-test SQL_ALLOWFULLSCAN=true
  reassignments already covered by the describe's beforeEach.
- Trim added comments that narrated intent/history rather than
  documenting a non-obvious invariant, per Harper's zero-new-comments
  default.

Not adopted, both already covered as open decisions carried into the
PR body's "For the human reviewer": resolving one config snapshot per
SQL statement instead of per-call reads (unchanged from the plan
review — no per-statement context exists at the router/optimizer
layer to hang it on), and registering sql_engine's bare-env-var
reachability (SQL_ENGINE), which the domain leg flagged as colliding
with a common external convention (e.g. Django) — a real, if
graduated, availability risk shared in kind with ~150 other existing
bare CONFIG_PARAMS names, surfaced to the task owner rather than
resolved unilaterally.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ytbJQ44LZsFyZ1C1fFdNk

* chore(sql): trim comments narrating history per delta-review nits

Both the codex and gemini legs of the delta review flagged several
added comments as narrating test/bug history or restating what the
test names already say rather than documenting a non-obvious
invariant. Trims those; keeps the two comments codex specifically
called out as explaining a real invariant (the sql Joi schema's
convert:false rationale, and why the set_configuration rejection test
needs no on-disk config fixture).

Also independently re-verified (not adopted) two other delta-round
gemini findings against the actual code and the passing test suite:
the claimed ReferenceError from bare string/boolean/number in
configValidator.ts (destructured from Joi.types() at the top of the
file — 508 tests exercising that schema all pass) and the claimed
Joi abortEarly:true truncating the maxSortRows/maxHashRows rejection
test (validateConfig() explicitly passes abortEarly: false — the test
asserting both messages together already passes).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ytbJQ44LZsFyZ1C1fFdNk

* chore(sql): drop the last narrating comment (delta review round 3)

Codex's third delta round flagged the remaining setConfigurationSql.test.js
preamble as restating what the parameterized test names already say.
Also independently verified (not adopted) gemini's round-3 "blocker"
claim that getConfigValue()/flattenConfig() have a casing mismatch —
both explicitly lowercase before the flatConfigObj lookup
(config/configUtils.ts's getConfigValue return line and flattenConfig's
squashObj), and this PR's own flattenConfig() derivation test already
exercises and passes that exact path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ytbJQ44LZsFyZ1C1fFdNk

* fix(sql): use plain node:assert per repo style (gemini bot review)

gemini-code-assist flagged unitTests/config/setConfigurationSql.test.js's
node:assert/strict import as against repo house style (.gemini/styleguide.md:
plain node:assert + explicit assert.strictEqual/deepStrictEqual). The file
already only calls .strictEqual/.rejects, so the swap is semantically a
no-op.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ytbJQ44LZsFyZ1C1fFdNk

* test(sql): isolate config tests from ambient state

Snapshot and clear HARPER_SQL_ENGINE alongside the sql.* config keys so external environment settings cannot override the config-integration assertions. Also isolate the existing router tests from a machine-local sql.engine value now that the router reads live Harper config.

Co-Authored-By: GPT-5 Codex <noreply@openai.com>

* fix(sql): reserve `sql` as a component name so a deploy cannot break boot

`sql` is now a validated core config section, but the root config namespace is
shared with application entries: `deploy_component project=sql package=x` wrote
`sql: {package: x}` and reported success, and the next restart failed config
validation with no way out but hand-editing the YAML.

Reserve the name at every ingress that creates a component under it — the
deploy/add validators, `set_component_file` (creation only), and
`set_configuration`'s `<component>_package`/`_port` escape, which maps straight
into a root entry without passing through either operation. `force` does not buy
the name: there is no core component to overwrite, only config to break.

An application deployed under the name before it was reserved still boots. The
`sql` entry validates as an application entry when it carries one of the keys a
deploy writes, and as the settings schema otherwise, so a typo'd setting still
fails loudly; boot warns to rename. The two shapes cannot be mixed.

Co-Authored-By: Claude Opus <noreply@anthropic.com>

* fix(sql): close the remaining component-creation ingresses for a reserved name

Review round 2. `set_env_value` creates the project directory the same way
`set_component_file` does, and `harper deploy setup=true` would seal a credential
for a component name the server then refuses — both now go through the
reservation. The grandfather check treats an unresolvable components root as
"not there" so it fails closed to the reservation instead of erroring, which is
also what the Windows unit job (no ambient install) exercises.

Widens the legacy-application key list to every deployment key componentLoader
reads off a root entry, so a grandfathered entry cannot be mistaken for engine
settings and fail boot. Drops the sinon/rewire tests the house style forbids: the
deploy handler cases now call the real operation, and the file-writer cases pin
a temporary components root and cover both sides of the grandfather check.

Co-Authored-By: Claude Opus <noreply@anthropic.com>

* fix(sql): keep a disabled `sql` entry bootable and tighten the reservation

Review round 3. `sql: false` (and `sql:`) is how componentLoader spells a
disabled component, so an operator who had already turned a pre-reservation
`sql` application off would have hit the very boot failure this change exists to
prevent; both are now accepted. The reservation matches case, like the config
param lookup it protects.

Co-Authored-By: Claude Opus <noreply@anthropic.com>

* fix(sql): keep the reservation case-sensitive, like the YAML key it protects

Review round 4: matching case-insensitively refused a redeploy of an existing
component named `SQL` — a distinct root key that collides with nothing — and did
it with a message naming a configuration section that does not exist.

Co-Authored-By: Claude Opus <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
…ble and defineTable (#2523)

* feat(branches): declare tables into a branch through @table, ensureTable and defineTable (#2264)

A branched application's `schema.graphql`, `scope.ensureTable()` and `defineTable()` all resolved
through the process-global `table()` factory, so its declared tables would have landed in the base
database while its code read and wrote the branch. Those paths were fenced (`branchGuard.ts`);
this replaces the fence with real branch-scoped declaration.

`table()` is now the global binding of `declareTable(target, definition)`: a `TableTarget` supplies
the root store, the `tables` graph a class is published into, the reload after a lost create race,
and who owns the column-family wrappers a declaration opens. The global target is the same code with
the same objects behind it, so an unbranched application still receives `table` by identity.
`scopedTableFactory(branches)` routes each declaration to the branch of the database it names, or
to `table()` for a database the application did not branch; GraphQL, `ensureTable` and
`defineTable` (via `defineTableUsing`) take that factory from the application scope.

A branch's schema-change signal carries the branch path; a thread holding that branch open reloads
its catalog (`reloadBranchAt`) instead of rescanning the global map. Branch classes announce to no
global `updateTable` subscriber. Every wrapper a declaration opens is recorded on the branch so
`close()` releases it, and table classes are cleaned up before their stores.

The Table statics that resolve the global schema by logical name (`dropTable`, `addAttributes`)
stay refused through a branch.

Implemented by the dispatch dev-agent (task harper-2264); brought onto main after harper#2517.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix(branches): clean up table classes when a branch open fails part-way

The failure path of openBranchDatabase closed the stores a partial open had created but never ran
Table.cleanup on the classes built over them, leaving expiration and eviction timers and reclamation
handlers alive for a branch that no longer exists. (Review finding on #2523.)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* style: separate DESIGN.md entries

Co-Authored-By: GPT-5 Codex <noreply@openai.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
…vent loop on the RocksDB path (harper#2536)

runIndexing never used its resume checkpoint: `start` began undefined and the
guard `compareKeys(lastIndexedKey, start) < 0` could never be true because
ordered-binary sorts undefined lowest, so every retrigger rescanned from the
first record. It also never yielded the event loop on a plain RocksDB index:
`outstanding` was decremented synchronously because RocksIndexStore.put is
putSync, so none of the outstanding-based yields ever fired and a large
backfill ran as one uninterrupted turn until the worker was terminated.

- resumeStartKey() computes the minimum persisted checkpoint across the
  attributes being built, or undefined (full scan) when any attribute has none.
- The loop yields every INDEXING_YIELD_INTERVAL (100) scanned entries,
  deletion entries included, independent of write-completion timing.
- Because a checkpoint is now actually consumed, it must certify a fully
  indexed prefix: it is written once the index writes it covers have settled
  and stops advancing after any record fails, so the retry re-covers that
  record instead of resuming past it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EVCC8xu4zk7eirYMg1ZHPb
…t, yield independent of the backpressure await

- A child process seeds a table, starts the backfill, flushes and SIGKILLs
  itself at its first persisted checkpoint; the parent resumes from that
  checkpoint through the PID-mismatch trigger. The flush is what a clean
  shutdown does: RocksDB data/index stores open without a WAL while the
  descriptor store has one, so an unflushed hard kill is out of contract.
- The record-count yield no longer sits behind the `outstanding > MAX`
  await, and the RocksDB yield test asserts the exact 100-record cadence.
- Plain `node:assert` per house style; narrating comments trimmed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EVCC8xu4zk7eirYMg1ZHPb
…persist the interrupt checkpoint synchronously

- Deletion entries now flow through the checkpoint, interrupt and yield
  path instead of `continue`-ing past it, so a dense tombstone region
  still checkpoints and still honours a worker restart.
- The interrupt path awaits the last write and persists its checkpoint
  before returning, so a thread restart on LMDB cannot drop it.
- The RocksDB yield assertion is a range, not an exact turn count.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EVCC8xu4zk7eirYMg1ZHPb
* Hot reload the models config block when the config file changes

An orchestrator can now rotate a credential or re-point a baseUrl by rewriting
the models block of harperdb-config.yaml; each worker watches the file (the
same RootConfigWatcher pattern the logger uses) and reprojects its registry
with no restart. Changed entries are rebuilt through the same factories boot
uses and swapped in atomically; removed entries stop serving; programmatic
registrations that overrode a config entry keep their documented precedence;
fallback routing is rebuilt with the block.

If HARPER_DEFAULT_CONFIG / HARPER_CONFIG / HARPER_SET_CONFIG also defines
models, the file alone is not authoritative for the block and hot reload stays
off — which doubles as the compatibility gate: an orchestrator still injecting
models through HARPER_SET_CONFIG keeps today's restart behavior, one that
writes the file instead gets live reload.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Coalesce boot and reload applies in separate lanes

A watcher event landing while a re-bootstrap sat queued overwrote the
pending apply wholesale, silently demoting boot to reload semantics and
losing its occupant-overwrite contract (found by review). Merging the
flag instead would launder the raw watcher block through boot
semantics, past reload validation and the missing-key no-op. Each lane
coalesces latest-wins on its own; boot drains first, the newest reload
then refines it under its own rules.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Assert each env layer independently disables models hot reload

The gate test only exercised HARPER_SET_CONFIG; a typo in either other
layer name would silently un-gate that layer. Parametrized over all
three, mutation-verified (misspelling the untested names fails two
tests). Found by CI review.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Cover the boot-cancels-settle-timer guard deterministically

An onSnapshotObserved test seam marks the moment a watcher snapshot is
observed, so the test can arm the settle timer, boot before it fires,
and assert the pre-boot content is discarded — converting the disclosed
review-covered guard into a mutation-verified one. Suggested by CI
review.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address review: boot composition order, helper restoration, shared watcher

Three findings from review. Boot publishes each entry as it is built,
in config order, so a later module factory observes earlier entries (a
wrapper resolves its base) exactly as before this feature; reload keeps
the staged atomic publish but runs factories for built-ins only, since
module factories may compose across entries and staged construction
cannot honor that — changing one keeps restart semantics. A helper
whose name a config entry claims is suppressed rather than forgotten,
and restored the moment the claiming entry is removed, so the live
registry matches a restart with the final config. Models rides the
isolate-shared RootConfigWatcher instead of opening a second native
watcher per worker; logging shares the same instance.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Restore a retained module-backed slot's fallback routing on removal

A module-backed primary removed or renamed out of the config block is
retained and keeps serving (restart-managed removal), but clearFallbackGroups()
wiped its routing and neither rebuild loop could restore it — the slot is absent
from both desiredKeys and presentKeys. Failover was silently dropped while the
primary kept answering. Track retained module keys and restore their recorded
fallback group after the clear.

Co-Authored-By: Claude Opus <noreply@anthropic.com>

* Refuse a module-backed→built-in change on reload too

The restart-only guard checked only the incoming backend, so rewriting a
module-backed entry to a built-in passed staging and publishEntry, live-replacing
the custom backend and retiring its helpers with none of the disposal a restart
performs — contradicting the InstalledSlot invariant that module-backed entries
require a restart to add, change, OR remove. Extend the guard to also refuse when
the currently-installed slot is module-backed, keeping the module and its fallback
routing until a restart.

Co-Authored-By: Claude Opus <noreply@anthropic.com>

* Apply a snapshot the shared watcher already holds at subscription

The singleton's one-time 'ready' usually fires for the logger before
models subscribes, and EventEmitter does not replay — a rewrite landing
in that gap stayed invisible until the next write. A snapshot the
watcher already holds is now applied directly on subscription. Found by
CI review; mutation-verified via a caller-owned pre-warmed watcher.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Release a suppressed helper even when its claiming entry never won the name

claimHelperOccupant suppresses a factory helper before the claiming
entry's own swap is known to win. When that swap loses to an application
override, the entry is recorded without a backend, and the removal path
only restored the helper `if (slot.backend)` — so dropping the entry left
the helper suppressed with no way back once the override retired. The
restore now runs unconditionally; it is already a no-op while the name
is held or nothing matching is suppressed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
LMDB commits the deferred checkpoint writes asynchronously, so a descriptor read
right after the interrupted pass could see the previous checkpoint and then
watch the last one land. Wait for the expected checkpoint, read the failed-pass
checkpoint only once it has stopped changing, and accept a later committed
checkpoint after the child-process kill on LMDB.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EVCC8xu4zk7eirYMg1ZHPb
…bility

Every checkpoint put is queued by the time runIndexing resolves, so waiting for
the write queue to flush makes the descriptor read authoritative.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EVCC8xu4zk7eirYMg1ZHPb
Adds to the shared derived-index runtime (#2489) what a backend with
milliseconds-per-mutation apply cost and an msync barrier needs:

- a coalesced last-write-wins `records` view beside `transactions`
- identity-first bounded collection with partial chunks for oversized
  transactions and no cursor publication mid-transaction; per-registration
  turn, chunk, cadence and rebuild options
- a runtime-scheduled durability cadence (age, thresholds, shutdown) through
  an optional backend `flush(reason)`
- rebuild as a runtime phase on the conservative log boundary with capped
  backoff, a shared attempt budget and an observable `unavailable` end state
- shutdown-before-unlock handoff, an owner-epoch fence for backends, and a
  new epoch per rebuild attempt
- sequence-locked shared readiness readable on every worker

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
- a fault found mid-drain can no longer publish `ready` over `rebuilding`
- resolution checks the chunk byte and time bounds after every key and
  carries the remainder, so the turn budget covers both phases
- reload markers committed before a rebuild's capture are covered by its scan
- an owner acquiring on shared `needs-rebuild`/`rebuilding` rebuilds
- `stop()`/unregister reject after a failed backend shutdown; the held lock
  is revivable through `requestRebuild`; one `shutdown` per epoch
- `requestRebuild` from a non-owner travels through a shared request word;
  a request during a rebuild no longer dangles
- promise-returning `reset`/`flush` are awaited or fail closed

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
- a held lock is revived under the same epoch and re-quiesced before any
  successor epoch or reset
- stop()/unregister return one cached promise, wait for every backend, and
  release table registrations only after the backend settled
- a rebuild consumes the shared rebuild request at start and on success; a
  non-owner never publishes readiness
- flush rejections are generation-fenced; the age timer re-arms while
  accepted work is not durable
- discarded log iterators are closed; scan tombstones are skipped
- rebuild proven against a real audited RocksDB table

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
- a failed unregister shutdown stays in the runtime-wide stop() wait
- a shared rebuild request reaches an owner parked on backpressure or backoff
- an inherited exhausted budget spends no further attempt; a backend that
  cannot rebuild parks on a condemned generation
- reload suppression uses the wall clock transaction timestamps use
- an unindexable reason carries the error class and status, not its message

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
- the readiness buffer carries a notify callback so a non-owning worker's
  requestRebuild wakes the owner directly
- readiness views are cached per buffer; epoch minting sits inside the
  acquisition error boundary
- a two-log rebuild with an empty log proves the boundary omits it safely

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
- a release waits for an in-flight reset before quiescing the epoch
- a stopped runner holding the lock after a failed shutdown is revivable
  through the runtime's requestRebuild
- a latched unavailable status clears once a peer revived the index
- the reload-suppression bound is shared with the next owner
- shared readiness reasons never carry backend error messages; live
  tombstones resolve to absent; unflushed counters reset with the cursor

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
…capabilities

Adopted from the planning recheck: a backend that queues declares
`queued: true` and registration rejects it unless it implements attach,
flush and shutdown. Also guards the idle-release cursor read and the
cleanup hooks, and keeps a failed shutdown's message out of the shared
readiness reason.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
…al store

- latestSeen is recorded at collection and cleared with the cursor;
  stalledMilliseconds reports time parked on backpressure or the ceiling
- the per-key wall-time check samples every 16 records
- the real-RocksDB test drives a second runtime's rebuild request through
  the native shared buffer
- documents the wall-clock-step residual of reload suppression

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
…by iteration

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
@kriszyp kriszyp added this to the v5.3 milestone Sep 9, 2026
…y checkpoint cannot leave an index gap

RocksDB data and index stores open without a WAL while the descriptor store has
one, so a checkpoint written every 100 records could outlive the index entries it
certifies after a SIGKILL/OOM. A checkpoint is now persisted at most once per
indexingCheckpointPeriodMs (5s) and only after the RocksDB store is flushed, with
at most one in flight; the interrupt, failure and completion paths drain it first.

Every checkpoint written this way carries checkpointCertified. The trigger only
resumes a stamped checkpoint: earlier releases advanced lastIndexedKey past
failed and unflushed index writes (both the indexingFailed and the interrupted
exits), so an unstamped one is a full rebuild. No field cleanup is needed.

setIndexingCheckpointPeriod() is the test seam; the crash test no longer flushes
by hand, so it now exercises the real hard-kill contract.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EVCC8xu4zk7eirYMg1ZHPb

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces native-backend readiness additions to the derived-index runtime, implementing a coalesced delivery view, bounded chunked collection and resolution, scheduled durability cadence, a robust rebuild phase, generation fencing, and sequence-locked shared cross-worker readiness. It also includes comprehensive unit tests and benchmarks. The feedback suggests enhancing the documentation to explicitly describe what the 'attach' method provides to future implementers of the 'QueuedDerivedIndexBackend' interface.

Comment thread docs/derived-index-runtime-stage-1.md Outdated
kriszyp and others added 4 commits September 9, 2026 12:23
…hare one flush per root store

- A record's non-last index puts (multi-value attributes, a second attribute)
  now attach a rejection handler, so a rejected LMDB put anywhere in the record
  freezes the checkpoint instead of being certified past.
- Concurrent backfills on one database share a single in-flight flush, and the
  flush uses the default (non-stalling) option.
- persistCheckpoint awaits its descriptor puts, so the interrupt path returns
  only once the checkpoint is committed on LMDB and a rejection is counted.
- The checkpoint period is paced with performance.now().

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EVCC8xu4zk7eirYMg1ZHPb
…to its key, flush before the ready descriptor

- A flush only covers writes issued before it started, so a checkpoint never
  joins a flush in flight: it joins the next one, shared by every backfill on
  that database asking meanwhile (at most one in flight and one queued).
- checkpointCertified now repeats the checkpoint key; the trigger resumes only
  when it matches lastIndexedKey, so a descriptor advanced by an older binary
  that round-trips the field cannot be mistaken for a certified one.
- The completion path flushes the tail written since the last checkpoint before
  persisting the ready descriptor, and parks the index if that flush fails: a
  kill right after completion previously lost every unflushed entry (0 of
  10,000 survived in the new child-process test on the parent commit).
- reindex reason 'uncertified-checkpoint' is logged for the legacy fallback.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EVCC8xu4zk7eirYMg1ZHPb
…le advance

Item 7 as ruled (option b): a registration's maxLagMilliseconds makes the
owner publish a shared lag word; every worker's runner registers an
admission check for the index's tables and Table.update()/delete() throw a
retryable 503 DerivedIndexLagError while it is set. Replication apply and
cache fills are never gated.

Also from the adjudicated doc round: `ready` and the retry budget settle on
the first durable advance (a queued backend under sustained ingest never
idles); a chunk the projection rejects entirely fails closed; the latched
unavailable check reads one word.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
The lag latch is process-shared memory that only an owning runner clears. Three
paths gave up ownership while it was still set and scheduled nothing that could
clear it, so every worker kept rejecting writes to the index's tables with a
retryable 503 indefinitely:

- `#needsRebuild()` when the backend has no `reset` or the runtime has no
  `scanRecords`. Nothing will ever rebuild, so nothing will ever catch up.
- `#acquired()` inheriting that same shared `needs-rebuild` state.
- `#deferForCondemnation()`, whose retry needs a wake, and wakes come from
  commits -- the very writes being shed.

This is the rule `#becomeUnavailable()` and the failed-shutdown hold already
followed; these three were the paths that missed it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kriszyp and others added 22 commits September 9, 2026 20:49
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Track native range activity without extending idle write holders. Report expired snapshots before native access, and keep the restart purge regression focused on allocated bytes reclaimed.

Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Forget read ownership when a live handle enters the commit retry loop, while existing range wrappers retain their expired-reader guard. Preserve iterable iterator and return-value contracts; cover a real conflict retry and key-only iteration.

Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Restore the RocksDB musl lockfile entry
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
…ive-flake

Make long-lived chain-link active reporting tests deterministic
… died

The catch in `next()` is only reached once `checkActive()` has already thrown,
which means the snapshot the underlying iterator was opened against is gone.
Closing it there is the likeliest moment for the native layer to object, and
because `DbiIterator.return()` forwards to the native handle with no guard of
its own, that error would propagate in place of the ReadSnapshotExpiredError —
replacing the named 503 with exactly the raw iterator error this wrapper
exists to stop surfacing. Cleanup failures are now swallowed; the error being
propagated is the actionable one.

Cleanup also goes through the closure rather than `this`, so a destructured
`next` still cleans up instead of throwing a TypeError on the way out.

`throw()` still does not delegate to `iterator.throw`: DbiIterator.throw
closes and rethrows the same error, so delegation buys nothing, and doing it
after the close above would run it against an iterator already closed.

`return`'s value parameter is now optional, which it always was at every call
site — the type only went unchecked while these calls went through `this`.

Addresses review thread r3975211616 (points 1 and 2 taken, 3 declined).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…failures

Keep active RocksDB scans alive during eviction
Replace raw manifest-spec equality with semver compatibility and canonical module-resolution checks so intentional caret ranges can still prove a single shared instance.

Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
…ce-guard

Keep shrinkwrap guard compatible with hoisted dependencies
One backend contract. No shipped or planned backend is synchronous (the HNSW
plane and Tantivy both queue and barrier), so the `asynchronous` discriminant,
the registration branch that validated hooks only for declared-asynchronous
backends, the check that a "synchronous" flush did not return a promise, and
the quiescence path for that undeclared promise are gone. `attach`, `flush`
and `shutdown` are required of every backend; a fake that completes inside
`deliver()` implements them trivially.

Shared readiness is plain words, not a sequence lock. Nothing in production
read the free-form reason string the seqlock existed to publish; the shared
record now carries a `DerivedIndexReadinessReason` code in one Int32 word,
the owner-epoch counter lives in the same buffer instead of a second
`getUserSharedBuffer` key, and a read is four `Atomics.load`s with no spin.
The full message stays in the owner's local status and log. This removes the
one construct in the runtime that was novel with respect to how Harper already
uses `Atomics` over rocksdb-js shared buffers (primary-key allocation, blob
holds, HNSW node ids).

The rebuild anchors at the committed tail, not the oldest retained entry.
rocksdb-js advances `lastCommittedPosition` only to the earliest
still-uncommitted write (`TransactionLogStore::commitFinished`,
`uncommittedTransactionPositions.front()`), so a committed read is a
contiguous physical prefix and nothing committed after the capture can sit
behind the tail. The whole-retained-log replay after every rebuild, the
capture-time reload-marker suppression, its `Date.now()` comparison against
transaction timestamps, the shared reload word and the backward-clock
residual all go with it: a reload marker is met exactly once.

The lag latch clears on entering a rebuild (no durable cursor to guard) and on
every park the runtime cannot leave on its own, not only on `unavailable`.

`partial` is dropped from `DerivedIndexTransaction`: nothing consumed it and a
backend cannot act on it; the withheld cursor already says what is certified.
The per-log cursor checks in `#reconcileDurableCursor` that could not fail
after the whole-vector match are gone; the repeat-detection set stays, since
transaction timestamps are unique per log but not physically monotone.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nning

rocksdb-js reports `oldestSequenceNumber: 0` with `fileCount: 0` for a log
that has no files yet, so the `=== 1` retention test rejected every brand-new
database: the first runner found no durable cursor, started a rebuild, and
`#captureBoundary` failed the attempt with "retains no committed transaction
and has lost its beginning" — on every attempt, until the budget parked the
index unavailable. Found by restacking the native HNSW index on the runtime
and defining a fresh table.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@kriszyp kriszyp changed the title Make the derived-index runtime safe and efficient for native backends Shared derived-index runtime for native backends Sep 10, 2026
@kriszyp

kriszyp commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

Continued as #2567 against main: GitHub's stack membership kept refusing a base change on this PR even after #2533 and #2535 were closed, so the same head branch is re-opened there. Nothing else differs. — Claude Fable 5.1

@kriszyp kriszyp closed this Sep 10, 2026
kriszyp added a commit that referenced this pull request Sep 10, 2026
`resources/DerivedIndexBackend.ts`, this branch's own delivery runtime, is
deleted. The file-primary HNSW index is now a `DerivedIndexBackend` on the
shared runtime from #2548:

- `resources/indexes/hnswDerivedIndex.ts` adds `HnswDerivedIndexBackend` —
  `deliver()` queues, an applier drains in 5 ms slices, the plane's `msync`
  is the barrier, and the one cursor vector is published after the pending
  mappings under `Symbol.for('derived-index-cursor')`; `reset(epoch)` removes
  the cursor before the file and the mappings — and `attachDerivedIndexes`,
  which registers a table's post-commit indexes with one runtime per database
  on every worker.
- The commit path only validates (a malformed vector is still the client's
  400); nothing is staged on the transaction and the `aftercommit` targets
  argument is gone. Writer backpressure is the runtime's lag policy
  (`maxLagMilliseconds`, 30 s default on a `nativePlane` attribute).
- Search readiness is the runtime's shared record on every worker, not the
  per-worker `isIndexing`; a failed search detaches and asks the owner for a
  rebuild instead of unlinking a file a peer may already have replaced.
- `vectorIndexPlane.test.js` is ported: 22 passing against the real native
  package, including the two-worker, worker-death, reload-marker, retention
  and lost-file cases. The ingest bench's repeated-key shape now measures
  265 native applies for 1,000 commits over 50 keys.

`hnsw-native-plane.md` §8, §10, §11 and §13 record the outcome; six of the
§10 open items are closed by the shared runtime.

Refs #2489

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
kriszyp added a commit that referenced this pull request Sep 11, 2026
`resources/DerivedIndexBackend.ts`, this branch's own delivery runtime, is
deleted. The file-primary HNSW index is now a `DerivedIndexBackend` on the
shared runtime from #2548:

- `resources/indexes/hnswDerivedIndex.ts` adds `HnswDerivedIndexBackend` —
  `deliver()` queues, an applier drains in 5 ms slices, the plane's `msync`
  is the barrier, and the one cursor vector is published after the pending
  mappings under `Symbol.for('derived-index-cursor')`; `reset(epoch)` removes
  the cursor before the file and the mappings — and `attachDerivedIndexes`,
  which registers a table's post-commit indexes with one runtime per database
  on every worker.
- The commit path only validates (a malformed vector is still the client's
  400); nothing is staged on the transaction and the `aftercommit` targets
  argument is gone. Writer backpressure is the runtime's lag policy
  (`maxLagMilliseconds`, 30 s default on a `nativePlane` attribute).
- Search readiness is the runtime's shared record on every worker, not the
  per-worker `isIndexing`; a failed search detaches and asks the owner for a
  rebuild instead of unlinking a file a peer may already have replaced.
- `vectorIndexPlane.test.js` is ported: 22 passing against the real native
  package, including the two-worker, worker-death, reload-marker, retention
  and lost-file cases. The ingest bench's repeated-key shape now measures
  265 native applies for 1,000 commits over 50 keys.

`hnsw-native-plane.md` §8, §10, §11 and §13 record the outcome; six of the
§10 open items are closed by the shared runtime.

Refs #2489

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants