Conversation
…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
…y store Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
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
…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
Contributor
There was a problem hiding this comment.
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.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P9BNWi77MsouBBexxHpT65
…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>
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>
Member
Author
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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). OneDerivedIndexRuntimeper database wakes on the root store'scommittedevent; 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 withexactStart; a boundary the log cannot resolve, a corrupt frame, a removed log or areloadmarker 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 optionalreset(epoch).batch.recordsis the last-write-wins view over distinct keys;batch.throughis the cursor vector the batch completes and is withheld until an oversized transaction's closing chunk. Every hook is required — a backend that completes insidedeliver()implements them trivially — because work that survives a method return is what an ownership handoff has to fence.Rebuild.
rebuildingis 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 asthrough, and replay resumes after it. Bounded retry with backoff;unavailableafter the budget, honoured by peers.Shared readiness. State, reason code, attempt count, rebuild request, lag flag and the owner-epoch counter in one
getUserSharedBufferallocation per index, read with plainAtomics.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:
flushreturned no promise, and a quiescence path for that undeclared promise. No shipped or planned backend is synchronous.DerivedIndexReadinessReasoncode, 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 ofAtomicsover rocksdb-js shared buffers.lastCommittedPositiononly 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, itsDate.now()comparison against transaction timestamps, the shared reload word and the backward-clock residual are gone; a reload marker is met exactly once.unavailable: a backend withoutreset, a runtime withoutscanRecords, a condemnation marker that could not be written, and entering a rebuild (no cursor to guard; readers act onrebuilding). 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.partialdropped fromDerivedIndexTransaction(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.oldestSequenceNumber: 0withfileCount: 0for it; the=== 1test 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
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.vectorIndexPlane.test.js22 passing against@harperfast/hnsw0.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