Skip to content

fix(kv-unified): accelerate general solves and reduce memory use - #110

Merged
antra-tess merged 9 commits into
mainfrom
fix/kv-unified-general-solver
Sep 24, 2026
Merged

antra-tess merged 9 commits into
mainfrom
fix/kv-unified-general-solver

Conversation

@antra-tess

@antra-tess antra-tess commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Once an accepted presentation existed, kv-unified could spend minutes and tens of GB propagating labels, materializing complete frontiers, and rescoring every leaf. The saved Sill incident recorded 244–286-second compiles even when the selected layout did not change.

This PR now combines the original general-solver improvements with the packed-storage and selective-scoring work from #112. Review and merge only this PR; no companion PR is required. All performance measurements disable the optional hysteresis certificate.

Changes

  • Use linear grid-extremum selection, precomputed action/terminal costs, and idempotent-prune elimination; materialize frontier Maps and rendered layouts lazily.
  • Collapse broken cache prefixes by their last matched marker. Preserve fix(kv-unified): stale presentation receipt made the label count superlinear (#97) #98's extension-token dominance, fourth warm-cache extremum, and cache approximation envelope; extension tokens are never an exact key dimension.
  • Handle internal protected holes in the automatic DAG path and preserve chronological cache emissions across nested ownership gaps.
  • Use owned numeric label handles, contiguous binary64 records, immutable paged trace indices, and reusable generation-stamped bucket tables. Sparse fallback changes storage only, never pruning or stopping rules.
  • Cache action-level fidelity/continuity sums with conservative floating-point intervals and compute chronological cache costs exactly for every retained cut. Establish exact normalization floors independently of welfare, then exactly rescore every possible winner, tie, and carried-layout contender.
  • Preserve the complete sorted candidate API through deferred exact evaluation on diagnostic-list inspection. Keep storage: 'objects' and terminalEvaluation: 'full' as differential-reference modes.
  • Include the opt-in certified hysteresis exit, diagnostics, offline benchmark/replay tools, regression tests, design notes, and changelog fragments.

Policy weights, hard token walls, continuity pricing, grids, adoption thresholds, and label ceilings are unchanged. No solve deadline or new approximation is introduced; the existing bucket approximation remains.

Updated through main merge 9b36c16, including #108's reviewed cache-tie fix and #113's signed-thinking pricing. The packed backend now uses #108's frontier tie-break too, after extension-token comparison. Both commits from #112 remain included without rewriting published history. The unrelated local migration-guide commit remains excluded.

Tests

  • TypeScript build and typecheck pass.
  • Full compiled Node suite on the updated branch: 838 pass / 0 fail, versus 818 / 0 on main at 9b36c16 (from its Ubuntu Node 20 CI). The earlier stage had 813 / 0 against the b272434 main baseline of 802 / 0.
  • The four-chunk equal-score regression from fix(kv-unified): key matchedUnits only while the cache is intact (#105) #108 now checks object and packed storage, with full and selective scoring on the packed backend. All modes select the oracle's frontier. Restore the dead cursor tie-break and the bucketed modes fail this case.
  • Updated fix(kv-unified): accelerate general solves and reduce memory use #110 CI passes on macOS and Ubuntu with Node 20 and 24; the changelog check passes too.
  • Prepared scoring matches the exhaustive oracle across 120 cache/gap/hole/extension cases; 80 additional cases exercise nested protected holes and interleaved ownership. Upstream stale-receipt and extension-accounting regressions pass.
  • 90 varied forests compare object/full, packed/full, and packed/selective: identical selected candidates, complete retained-candidate lists, floors, and pre-existing propagation/envelope statistics. Every feasible small cut is checked against its bounds. Tests also cover floor-setting welfare losers, ties, hysteresis boundaries, non-finite fallback, snapshots, and storage reuse/growth.
  • Work-count regression: 22,435 candidates from 64 leaves require 2,112 source-token-cost reads versus 5,384,010 in the original solver, with the same selected score.

Fresh paired large-fixture measurements

73,918 leaves / 3,345 summaries, Bun 1.3.14. Object/full is the first-stage solver retained as a diagnostic backend; packed/selective is the combined default.

Case Object/full Packed/selective Reduction
Warm cache, forced 500k wall 8.721 s 5.194 s 40.4%
Warm cache, newly pinned middle leaf 8.736 s 5.229 s 40.1%
Warm cache, 30k appended tail tokens 9.149 s 5.509 s 39.8%

Selected frontier hashes, scores, floors, and existing propagation/envelope statistics match exactly. Each case needs 3 exact evaluations versus 6,538–6,719 retained cuts (including the feasibility-witness check). A separate diagnostic checked all 6,538 pinned-case estimates against exact evaluation. End-of-run RSS was 1.12–1.13 GB versus 1.26–1.30 GB for object/full.

After the #108 merge, one fresh paired forced-500k run took 8.686 s with object/full and 5.133 s with packed/selective. Both retained the same selected frontier hash, 471,837 rendered tokens, score, floors, and propagation statistics as each other and as the earlier run.

Controlled 500-message replay

Combined implementation at 028b065: 500 successes / 0 failures, certificate disabled.

Statistic Previous replay Packed/selective Reduction
Mean 3.122 s 2.044 s 34.5%
Median 2.837 s 1.878 s 33.8%
p95 4.959 s 3.145 s 36.6%
p99 8.069 s 4.927 s 38.9%
Maximum 9.486 s 5.611 s 40.9%

420/500 solves are below 2 seconds; 495/500 below 5 seconds. All 23 exact-reference checks pass (warm-up, every 25th message, and every layout movement), including all four transitions. Checks compare frontier hashes, selected loss terms, score, and both floors; reference time is excluded from solve timing. Every row's tokens, movements, terminal labels, and maximum labels per state also match the old replay. The old rows lack hashes/scores, so full decision equality is claimed for the sampled reference checks, not all 500 historical rows.

The previous timing column is the archived pre-rebase 5c55a26 run, not a fresh complete first-stage replay. Timings include forest/solver construction, solve, and selected-frontier materialization; nearest-rank percentiles exclude one warm-up. Each selected result is accepted before the next message. The replay uses the snapshot summary catalogue with future-source summaries excluded and fresh atomic cache markers, not historical provider traffic or cache TTLs. Observed high-water RSS was 3.285 GB including in-process object-reference checks.

The 500-message replay predates the #108 merge. Its results describe 028b065 and have not been extrapolated to the new head.

The earlier first-stage complete offline compile took 6.91 seconds, rendering 727 messages / 507,298 tokens with zero moves; this complete store compile was not repeated with packed/selective.

General-solver design and earlier measurements · Packed/selective design, latest results, and reproduction.

Not verified / out of scope

  • No live rollout, resident configuration changes, provider/compression calls, cache-TTL simulation, or historical compression reconstruction.
  • Local timing observations are not cross-platform performance measurements or a universal worst-case latency guarantee.
  • Diagnostic candidate-list inspection intentionally performs remaining exact scoring; that cost is deferred, not eliminated.
  • Resident data and raw replay receipts remain local and are not included in the PR.

Both changelog fragments are included: kv-unified-general-solver.fixed.md and kv-unified-packed-selective.changed.md.

🤖 Generated with Codex (GPT-6).

antra-tess and others added 5 commits September 21, 2026 09:11
…ions

Preserve the welfare policy while removing repeated metric work, redundant grid projection, and eager frontier materialization. Add oracle comparisons, regression coverage, and offline replay tooling.

Co-Authored-By: Codex (GPT-6) <noreply@openai.com>
Co-Authored-By: Codex (GPT-6) <noreply@openai.com>
… cuts

Preserve exact floors, selection, ties, and hysteresis while reusing bucket storage and avoiding unnecessary terminal leaf scans. Keep the object backend and exhaustive scoring as diagnostic references.

Co-Authored-By: Codex (GPT-6) <noreply@openai.com>
antra-tess and others added 2 commits September 21, 2026 12:28
Co-Authored-By: Codex (GPT-6) <noreply@openai.com>
Co-Authored-By: Codex (GPT-6) <noreply@openai.com>

@Anarchid Anarchid 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.

Reviewer-Model: GPT-5.6 Sol

Reviewer: Codex (GPT-5.6 Sol)

Reviewed head: 3356af7

🟠 NEEDS ATTENTION

src/adaptive/kv-unified-pareto.ts:1077 and src/adaptive/kv-unified-packed.ts:112 — broken-cache ties still use the dead divergence cursor

This PR deliberately groups broken prefixes by their last matched cache marker, so labels that diverged at different units now compete. Both representative selectors nevertheless use matchedUnits/Matched as their final tiebreak. That cursor is dead after divergence and is not the ordering terminal selection uses, so the pruning decision can retain the wrong equal-score layout and change the receipt rendered for the next turn.

I ported the four-chunk equal-score regression from the repaired #108 head as a temporary diagnostic. The exact oracle selected a:0|b:1|c:0|d:0, while both the object and packed bucketed engines selected a:1|b:0|c:0|d:0; the exact packed engine retained the oracle choice. This is the same reviewed defect fixed by #108 at d6859486be9459e6c006e473abdd4af4d93e8f9b, but #110 is based before that repair and duplicates the old tiebreak in the new packed backend.

Please port the frontier-signature tiebreak into both object and packed representative selection (or rebase and resolve #108 equivalently), then add the equal-score regression for both storage modes.

Validation evidence:

  • git diff --check b272434..HEAD — passed.
  • Fresh npm ci --ignore-scripts --prefer-offline and npm run build — passed.
  • Existing focused suites for certificate, packed storage, selective scoring, terminal evaluation, and policy — 5/5 test files passed.
  • Temporary discriminating regression — failed for both bucketed storage modes as described; the temporary file was removed and the worktree returned clean.
  • All 5 exact-head GitHub checks are green.

— Reviewed by GPT-5.6 Sol via OpenAI Codex.

antra-tess and others added 2 commits September 22, 2026 07:26
Preserve extension-aware bucket selection and apply the frontier tie-break to packed labels.

Co-Authored-By: Codex (GPT-5) <noreply@openai.com>
Co-Authored-By: Codex (GPT-5) <noreply@openai.com>
@antra-tess

Copy link
Copy Markdown
Contributor Author

Addressed in 35e1b8e (the merge of main into #110), with documentation at ad9bae6.

#108 is merged into main. I resolved its object-backend tie-break against #110's extension-aware representative order, then applied the same frontier-signature tie-break to packed labels. Extension remains compared before the signature in both backends. The four-chunk equal-score regression from #108 now runs against object storage, packed/full, and packed/selective, with exact and bucketed grids; all select the oracle's a:0|b:1|c:0|d:0 cut.

Build, typecheck, and the full compiled Node suite pass: 838 / 0 versus 818 / 0 on the updated main. A fresh forced-500k warm-cache check on the saved 73,918-message fixture took 8.686 s (object/full) and 5.133 s (packed/selective), with the same frontier hash, score, floors, and propagation statistics. The previous 500-message replay predates this tie repair, as the updated PR body now states.

Please re-review #110 at ad9bae6.

@theaspirational

Copy link
Copy Markdown
Contributor

Reviewed head: ad9bae6 (main 9b36c16 merged). Reviewer: @theaspirational, with Claude Code (Claude Fable 5.1). Read-only; nothing in the PR was changed.

How this review was done

  1. Read the prior threads first: @Anarchid's review on 3356af7, @antra-tess's reply, and the fix(kv-unified): key matchedUnits only while the cache is intact (#105) #108 / perf(kv-unified): pack labels and selectively rescore terminal cuts #112 discussions this head folds in. Every point there was re-checked against the new code.
  2. Fresh clone at ad9bae6, npm run build, full npm test.
  3. Mutation checks in dist: put the old cursor tie-break back in the packed backend; removed the tie-break entirely in both backends. Ran the policy test file each time, then restored.
  4. Real-store differential: the scrubbed resident store snapshot (75,717 messages, 3,406 summaries, shipped recipe), host receipt loop emulated, 20 turns with one ~5k message appended per turn, packed/selective vs object/full, layout hash and score compared per turn.
  5. Two independent line-by-line reads with a written proof obligation each: selective scoring (are the bounds conservative, is hysteresis and tie order preserved) and packed storage (parity with the object backend, handle reuse, sparse fallback). Only what was verified in code is reported; the rest says so.

Verdict: approve. Remaining items are tests and hardening, not behaviour.

Prior review point (dead-cursor tie-break) — fixed, verified

Independent checks

  • npm test at ad9bae6: 838 / 838. CI 5/5 green.
  • Real-store differential on the scrubbed resident store snapshot (75,717 messages, 3,406 summaries, shipped recipe, host receipt loop emulated): object/full vs packed/selective over a 20-turn replay with one ~5k message appended per turn — identical layout hash and score on 20/20 turns, same re-fold turns (6, 9, 14, 18). Mean turn 8.4 s → 6.8 s (−19% here; the PR's fixture shows −40%, so the gain is store-dependent). End RSS 2.75 → 2.54 GB.
  • Selective scoring (kv-unified-selective.ts, kv-unified-terminal.ts): the bound-then-evaluate argument holds. Cache churn is exact; the continuity floor is refined exactly and only decreases; lower/exact/upper go through the same policyScore with the same operation order, so IEEE per-op monotonicity gives lower ≤ exact ≤ upper without per-step widening; outward widening only in nonnegativeSumInterval. Hysteresis candidate is never skipped (min-upper matching candidate has lower ≤ upperMatching). Ties: a skipped candidate cannot tie; evaluated ones use the same comparator and source order as full mode, so winner and sorted candidates are identical. Non-finite anything → full evaluation.
  • Packed storage: key, dominance, representative fold, envelope cover, emit/flush/apply and stats counters line up with the object backend one for one. Sparse fallback is storage-only (dense-ness is a pure function of the key; group indices first-seen order). Handle reuse, epoch stamps and trace paging read correct. finishTrace inside signature() is idempotent; at most one orphaned 8-byte node per tie-checked label, per-solve arena.
  • Side effect worth recording: solve() (kv-unified-pareto.ts:152-166) now runs the leaf engine only on explicit engine: 'leaf'; nothing in src/ selects it. So kv-unified leaf engine: token-bucketed dominance can discard the better cut while reporting a zero regret bound #109's production path (protected-hole auto fallback) is closed by this PR. Maybe note that on kv-unified leaf engine: token-bucketed dominance can discard the better cut while reporting a zero regret bound #109.

Should fix (tests)

  1. No test asserts the packed prune throws SparseLabelCeilingError (kv-unified-packed.ts:213-214). Deleting line 214 passes npm test. One case with all buckets 0 and labelCeiling: 1, both storage modes.
  2. npm test never reaches the packed-only scale paths: PackedLabels.grow() (>1023 handles, kv-unified-packed-storage.ts:83-89), PackedBuckets head growth (>1024 groups, :184-190), trace paging (TRACE_SIZE = 2^18, :10-12, 40-47, 55-59). Differential forests are 8–9 chunks; the policy test stops at 24. The at-scale differential lives only in scripts/replay-kv-unified-messages.mjs. Either one mid-size differential forest in the suite (a few thousand labels, one trace page boundary) or a sentence in the doc saying where the at-scale check lives.
  3. kv-unified-selective.ts:121 uses strict > (correct), but no test pins the boundary lower === upperBest; >= passes all 13 tests. Failing input for >=: two candidates with zero-width bounds, fidelity [1,1], continuity [0,0], source order frontier x=1 then x=0 — exact picks x=0 by signature, >= returns x=1.

Hardening (nits)

  • kv-unified-policy.ts:470 quadratic uses ** 2; monotonicity of the bound needs a monotone pow. V8 and Bun map y == 2 to x*x, so fine today; const r = excess / scale; return lambda * r * r removes the dependency.
  • kv-unified-selective.ts:68 takes cacheFloor from estimate().cacheChurn (terminal.ts:200-225) while exact scores use candidate().cacheChurn (:263-298): two hand-duplicated cache walks that must stay bit-identical. packed.test.ts:138 asserts it on small forests. A shared emit helper would make it structural.
  • Feasibility witness: kv-unified-packed.ts:375-379 is a subset check; the object backend uses sameFrontier with a size compare (pareto.ts:606-608, :1095). Equivalent while terminal frontiers span all leaves; one size compare restores parity.
  • kv-unified-terminal.ts:193 assumes a complete disjoint cut but checks only leaves === ids.length. Solver traces satisfy it; a hand-built trace would not. Debug-assert or document.
  • Bucket configs in both differential tests are all-or-nothing (packed.test.ts:109, kv-unified-policy.test.ts:1202-1204): tokenBucketSize > 0 with k/f = 0, tokenBucketSize = 0 with k/f > 0, and the broken: string-key path (packed.ts:106-109) are never compared against objects.

Not this PR, for the record

Findings from the same bench that are about kv-unified's policy rather than this PR's solver work (quiet turns still pay a full solve; churn vs kv-stable on the new main) are in a separate write-up.

@theaspirational

theaspirational commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Reviewed head: ad9bae6 (main 9b36c16 merged).

Reviewer: OpenAI Codex (GPT-5.6 Sol xhigh).

Second-pass adversarial review; read-only, nothing in the PR was changed.

How this review was done

  1. Re-read the PR body, the prior review threads, and my earlier approval comment, then reviewed the full 9b36c16...ad9bae6 diff against CONTRIBUTING.md and the PR's preservation claims.
  2. Fresh detached worktrees for base and head. Ran npm ci, TypeScript build, git diff --check, and the full compiled Node suite.
  3. Independent standards and spec passes over the packed DAG, selective scorer, terminal evaluator, certificate, and public result API.
  4. Ran 1,000 randomized packed/object/full/selective differentials, including mixed bucket configurations, protected holes, cache breaks, and sparse bucket fallback.
  5. Ran base-vs-head differential fuzzing specifically around the production-path change that moves internal protected holes from the exact leaf fallback into the bucketed DAG.

Verdict: needs changes. The previous approval verdict is superseded by this comment.

Behaviour regression — protected holes now receive approximation that changes the selected layout

ParetoKvUnifiedPolicySolver.solve() (kv-unified-pareto.ts:146-166) now routes every non-explicit-leaf solve, including internal protected holes, into the DAG. With positive continuity/fidelity buckets, packed pruning (kv-unified-packed.ts:146, 164-170) retains only the F/K/T/E representatives. On main, internal holes automatically used the exact leaf engine, so the configured grid approximation did not apply to these cases.

A six-leaf deterministic reproduction with three L1 pairs and c2 pinned:

  • Raw tokens: [84, 91, 114, 109, 117, 149]
  • Recall tokens: [139, 93, 74]
  • Salience: [0.2617709129, 0.4648823917, 0.5991752911, 0.4875459687, 0.6896713809, 0.4913788924]
  • maxTokens: 574
  • Buckets: token/continuity/fidelity = 100/100/100
  • Policy overrides: alpha: 0.8328641616, low/high ratios 0.3636986045/0.8021343783, under/over lambda 4333.3135836/4973.1302680

Results:

  • main, head with engine: 'leaf', and head with all buckets zero: c0:1|c1:1|c2:0|c3:0|c4:1|c5:1, 436 tokens, score 134.53339405731546, 3 retained candidates.
  • Head default packed/selective and head object/full DAG: c0:1|c1:1|c2:0|c3:1|c4:1|c5:1, 420 tokens, score 155.81796372345093, 2 retained candidates.

Storage and terminal evaluation do not affect the failure; zeroing the buckets or restoring the leaf path does. The new default discards the lower-score cut. This expands approximation into a formerly exact production path and contradicts the PR body's “No ... new approximation is introduced” and policy-preservation claims.

Please restore the exact protected-hole fallback until the bucketed DAG can preserve the prior decision, or explicitly accept/document the policy change and its error envelope. A regression must compare auto/default against the exact leaf/oracle path with positive buckets and an internal hole; packed-vs-object alone cannot catch this because both now share the changed candidate set.

Candidate API — successful certificates return only one candidate

certifyCarriedLayout() (kv-unified-certificate.ts:129-135) scores only the carried frontier and returns that singleton result at :190-200. On the existing certificate fixture, the normal solver exposes four candidates while hysteresisCertificate: true exposes one when .candidates is inspected.

The certificate document says successful results contain one candidate, but the PR body says the complete sorted candidate API is preserved. Either provide a lazy full-solver getter for diagnostic inspection or qualify the API claim and public contract for certificate mode.

Should fix (tests)

  1. No test reaches the packed SparseLabelCeilingError guard at kv-unified-packed.ts:213-214; deleting it still leaves the suite green.
  2. No test pins selective scoring's strict lower > upperBest boundary at kv-unified-selective.ts:121; changing it to >= still leaves the focused suite green and can change the frontier tie-break.
  3. Trace paging has no boundary test at node 262,144 (kv-unified-packed-storage.ts:40-59). Correction to my prior comment: PackedLabels.grow() and PackedBuckets head growth are already exercised by kv-unified-packed.test.ts:12-46; only the trace-page transition remains untested.

Validation

  • git diff --check: pass.
  • Full npm test at ad9bae6: 838 / 838.
  • 1,000 randomized packed/object/full/selective differentials: pass.
  • Independent protected-hole reproductions: fail against main/leaf/exhaustive selection as described above.

The packed representation and selective terminal scoring still agree with the object/full DAG on the retained set. The blocker is earlier: the automatic protected-hole route changes which set survives.

@theaspirational

Copy link
Copy Markdown
Contributor

Reviewed head: ad9bae6 (main 9b36c16 merged).

Reviewer: Claude Opus 5.5 (Claude Code), run by @theaspirational.

Third pass: an adversarial check of the second comment's findings. Read-only; nothing in the PR was changed.

How this was done

  1. Reproduced the second comment's six-leaf protected-hole case on main 9b36c16 and on ad9bae6, next to the exhaustive oracle and each result's reported error bound.
  2. Re-ran kv-unified leaf engine: token-bucketed dominance can discard the better cut while reporting a zero regret bound #109's protected-hole repro on both.
  3. Fuzzed 1,500 random forests (6–12 leaves, L1 groups of 2–3, ~20% pinned, random policy and budget) and kept the 924 with an internal protected hole. Compared main auto, head auto, and the leaf engine with an exact token key against the oracle, at buckets 100/100/100, 100/0/0 and 0/100/100. The fuzz has no provider cache.
  4. Checked whether the scrubbed resident store snapshot has internal protected holes.
  5. Checked the certificate and test-coverage claims against the code.

Verdict: the protected-hole finding is real. "Restore the exact fallback" is the wrong fix, because main's fallback is not exact.

Protected holes

Certificate candidate API

The point holds, but it is low severity. hysteresisCertificate is opt-in, and nothing in src/ sets it. docs/kv-unified-hysteresis-certificate.md:56 already says a successful result has one candidate. Only the PR body's "complete sorted candidate API" sentence needs "except in certificate mode".

Tests

  • The second comment's correction is right, and it also corrects the first comment on this thread: PackedLabels.grow() and bucket head growth are unit-tested (kv-unified-packed.test.ts:12-46). Only the trace-page boundary (node 262,144) has no test.
  • Still open, confirmed: no test for the packed SparseLabelCeilingError throw, and no test for the strict > boundary in kv-unified-selective.ts:121.

Net for @antra-tess

  • One decision: how to route protected holes.
  • Two PR-body sentences: approximation now covers protected holes, and the certificate-mode exception.
  • Three tests: holes against the oracle, the ceiling throw, and the > boundary.

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