Repository navigation
refactor(storage): separate tool bodies and compact semantic directories - #6520
Conversation
Generated-by: Codex
Preserve exact canonical bytes with immutable shared projection bodies and header slots. Resolve frozen ranges on the original SQLite worker using its existing statement cache, and keep full replay, transfer, and collection in the same owner lifecycle. Generated-by: Codex
Keep one body reference on each event row, enforce atomic collection, and measure prefix API time separately from complete result validation. Generated-by: Codex
Prepare canonical headers and original projection bytes on the SQLite worker, then decode owned events on a joined blocking task. Drain both stages before releasing the original transaction, preserving prefix budgets and raw evidence. Verify checkpoint tool history after source collection and import. Record chronological benchmark samples and isolate RootStore opening cost. Generated-by: Codex
Decode borrowed headers and original projection slices before advancing each SQLite row. Remove the temporary body string, transfer queue and independent decoder while preserving the original snapshot, budgets and raw evidence. Align the cleanup scheduling test with upstream's readable Root contract, retaining sibling drainage, confirmed outcomes and failed shutdown evidence. Generated-by: Codex
Order canonical headers and references before resolving bodies by primary key on the same SQLite worker and transaction. Reuse one body statement while decoding borrowed rows and recording original canonical evidence. Reject missing bodies without publishing partial canonical bytes. Generated-by: Codex
Read tool projection ranges directly from one immutable body and remove unused cross-header streaming. Share captured source validation and range dispatch through CanonicalRead on the original SQLite worker. Cover original and malformed canonical artifact tokens through the real capture path, including continued reads after malformed-token rejection. Generated-by: Codex
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
This is a focused migration/SQL-constraint and release-inventory check at a2b7d44a9a37aa56f5d78ffc1836e8a307d98e89; it is not a complete review of the 138-file storage rewrite. I inspected migrations 0013/0014, their registration/cleanup path, the canonical migration and recipe code, and the changed Cargo inputs. One confirmed P2 remains: the three new dependency-input changes are absent from the source-bound notice inventory, as described inline.
The base already has two stale SDK fingerprints. Correcting only those inherited fingerprints in an isolated source copy still leaves the actual notice verifier failing on this PR: Cargo.toml, Cargo.lock and platform/event-log/Cargo.toml are three additional mismatches. Correcting all five recorded fingerprints makes the same verifier pass. This distinguishes the new inventory gap from the inherited one; it does not validate license-generation output.
I also executed the unchanged material triggers from migration 0014 in nine isolated SQLite fixture checks: shared-body lifetime, last-reference clearing/deletion, rollback, immutable bodies/references, foreign-key protection and invalid length/offset/kind rejection all passed. The fixture uses a minimal event table and supplies already separated material; it does not run the Rust migration functions, Postcard conversion, SQLx upgrade, real prepared-checkpoint recovery or production readers. An actual merge calculation against Rust dfc556ebf50ee5f6cfd71ddfa2d1c097faf27fa6 is clean.
This machine has no Rust toolchain. The reported 382 tests, v12 checkpoint upgrade and performance measurements remain the contributor's evidence. I have not established the safety or performance of the full rewrite. Hosted CI is still running; passing dependency/Computer Use checks does not override the locally reproduced inventory failure.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
| [workspace.dependencies] | ||
| mxc-sdk = { git = "https://github.com/microsoft/mxc", rev = "05a430bc17a3e1b3fa2131524fc52e4319e07298" } | ||
| flatbuffers = "25.12.19" | ||
| postcard = { version = "1.1.3", default-features = false, features = ["alloc"] } |
There was a problem hiding this comment.
[P2] Refresh the notice inventory for the new dependency inputs
This dependency change, the corresponding lockfile change and platform/event-log/Cargo.toml are all hashed inputs of the release notice verifier, but products/maka/cli/notices.json is unchanged. They introduce three mismatches beyond the two stale SDK entries already present on the base. Running the actual verifier after correcting only the inherited SDK entries still fails; correcting these three additional fingerprints in a disposable copy makes it pass. Please regenerate and review the source-bound inventory against the final dependency graph; otherwise normal checks and release preparation remain blocked even after the base's SDK mismatch is repaired.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
Regenerate notices against the final dependency graph and bundled product. The license document is unchanged; refresh the three Cargo inputs and two inherited SDK fingerprints. Generated-by: Codex
My earlier source-inventory P2 is resolved by #6520 commit The later #6523 replaces that mechanism with actual component/license coverage checks. The completed product worker for Rust
|
The final-head run 41273119557 has finished: 1,980 passed, one failed, 26 skipped. The failure is #6529 changes the launcher fixture's watchdog scope; its source change is not a completed runtime validation. Current Rust CI is still running. My earlier inventory P2 remains resolved, while full storage/migration acceptance remains outside the scope of that fix check.
|
Summary
Large successful tool projections currently travel inside canonical event JSON and SQLite sort records. Store their exact original JSON tokens once in immutable, content-addressed
event_bodies; keep headers and references inevent_log. Prefix reads sort metadata before borrowing each body on the original SQLite worker. Exact-byte consumers reconstruct the original JSON; typed readers decode without joining it first. Atomic publication, frozen evidence, authorization, copy/import and last-reference collection retain the original ownership contracts.Encode semantic directory descriptors once with Postcard and pack fixed locators into caller-charged pages. Compact JSON, Postcard and FlatBuffers were compared, as were raw and framed bodies; retain Postcard/raw only. This is not general-purpose body compression.
Verification
Following review, fully regenerated release notices (1360 components / 693 texts); the 2,681,462-byte notice document is unchanged. Updated five stale source fingerprints, including the three Cargo inputs and two inherited SDK inputs. Notice verification/regression test, dependency inventory, ASF headers and pre-commit checks pass. The follow-up commit changes only
products/maka/cli/notices.json; storage source and measured binary provenance remain unchanged.Event-log: 382/382 tests passed, covering migration, exact replay, shared-body lifetime, frozen reads, copy/import, bundle transfer and checkpoints.
Related engine filters: 49/49; native filters: 6/6. Following the earlier upstream refresh, native 6/6 and the affected cleanup test 1/1 passed again. The new real canonical-artifact case rejects an incomplete token with a success-returning callback, then successfully reuses the original token.
Event-log clippy all targets/all features with warnings denied; rustdoc; workspace format; ASF headers; manual pre-commit hooks; release benchmark build passed. Format/ASF passed again after the final rebase. A workspace-wide test run and baseline red-run of every new test are not claimed.
A genuine upstream v12 Prepared checkpoint, killed at the prepared boundary, upgraded to schema 14 and recovered the same manifest/receipt, four exact events, tool bytes and prefix digest. Bundle validation, foreign keys, reopen and idempotent retry passed; temporary probe hooks were restored byte-for-byte.
Final fixed comparison: 48 fresh processes, four AB/BA-balanced pairs × six scenes, 100 mixed iterations and 13 operations. All 234 descriptive quantiles, raw outputs, fixtures, hashes and execution order independently audited; dedicated advisor accepted the explicit UX tradeoffs below.
Read performance and tradeoffs
Each percentage is the median of paired candidate/baseline ratios; milliseconds are median paired differences. They are separate statistics. Four pairs are descriptive evidence, with no confidence interval or universal equivalence claim.
prefix_readprefix_readmodel_viewsequential_alltail_64_allLarge prefix gains hold in every pair. Dense
prefix_readp50 improves by 10.09–11.21 ms; large export p50 improves by 60.44–69.14 ms.The slower rows are supported costs, not dismissed as noise. Actual 100-call totals give small
model_viewmedian extra costs of 1.132 ms repeated / 4.564 ms entropy.sequential_allscans all 64 small bodies: 100 complete batches cost a median 19.944 ms extra (pair range +0.522 to +176.896 ms).tail_64_allreads four tool-body tails: its 100-batch total difference has median −0.047 ms, while its p99 is consistently slower. These are aggregate passes/model-preparation calls, not per-frame or per-token costs. Current production search finds nowith_sequential_sourcecaller; the API remains supported.Dense range/sequential and repeated-large text-leaf tails vary across pairs/order. 17/234 median ratios exceed 1.05; the other 217 are not statistical passes. Existing-database reopen shows no consistently slower scene; entropy-large has median ratio +4.99% / median delta +14.00 ms, with one +198.82 ms pair. Admission used the declared quiet-load gate, but whole-process pressure includes the benchmark itself and some outside work returned. Every observation is retained, with no causal attribution or sample replacement. Earlier 120-process cohorts and the user-requested process-114 repeat remain separate.
All 17 median-ratio flags above +5%
first_64_allsequential_alltail_64_allfirst_64_allfirst_64_allmodel_viewmodel_viewsequential_allsequential_allcross_frame_repeated_64_alldescriptor_leaf_textmodel_viewtail_64_allfirst_64_allmodel_viewmodel_viewtail_64_allStorage and migration costs
RootStore::openmedians +19.4–31.2% (+0.82–1.74 s), slower in 29/30 pairs; dense write sums +18–21%, many other write sums +5–13%. These are descriptive creation costs, not ordinary existing-database reopen. The 60 creation reports used the earlier two-table cohort ondfc0e2323, baseline binary90a3d87d…/ candidate740c34c0…; subsequent reader optimizations preserve the storage/write format. Pair gaps, boot cohorts and resource-observer limitations remain disclosed rather than treated as causal proof.Review focus and reproduction
The complete diff is 139 files, +5179/−1002 (net +4177). Many query sites switch to header-only predicates. The final simplification removes the unused cross-segment reader and duplicate canonical-range dispatch (net −103, including a real regression test). The two-file, 928-line manual benchmark is retained as an independent regression/reproduction tool.
Review migrations 0013/0014,
canonical/{recipe,write,source,read,event}.rs,prefix.rs, directory encoding and captured readers. No experimental codecs, second SQL owner, body cache or additional publication state machine remains.Benchmark sources: Rust fixture and byte oracles, manual comparison driver. Copy the identical example into the baseline checkout (harness-only addition), then build each variant with
cargo build --locked --release -p maka-event-log --example event_storage_bench; the manual driver acceptsoriginal=/path/to/baseline candidate=/path/to/candidate --output /fresh/path --iterations 100 --body-pattern repeated(repeat withentropy). Its standard mode creates fresh fixtures with at least five runs; the reported final check instead used the private fixed four-pair schedule on retained matching fixtures.Measured candidate source:
ee2a979fe, release SHA-25668f5701493d715eff9b19427489f9d0dfa20b38787dbf47124c81b7de698b52b; baseline source:2ba23ef3e, SHA-2565d767d3ca440bfaa99b4c8b04abaf77437430592c25025efdb84d6ef5db3e7ba. Linked source/dependency comparisons prove equivalence through the final basedfc556ebfand heada2b7d44a9; all 138 changed files are byte-identical across the final rebase. Raw reports/protocol/audits are retained locally under~/.cache/maka-event-storage-layout, not uploaded with this PR. The tables above are the reviewer-visible evidence summary.AI use
Codex authored implementation, tests and benchmark workflow. A dedicated GPT-6-astra high advisor reviewed correctness, deletion necessity, validation scope, final results and source equivalence. Commits retain
Generated-by: Codextrailers.Checklist
Does this PR entail a change in behavior?