Op-authoring helper + full backfill of existing memories (phase B, unwired)#526
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
SCIP oracle — resolution reportHeuristic→compiler edge resolution per corpus. Δ compares resolved-after to the
resolved = |
|
| Branch | feat/oplog-authoring |
| Testbed | ubuntu-latest |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
🚨 3 Alerts
| Benchmark | Measure Units | View | Benchmark Result (Result Δ%) | Upper Boundary (Limit %) |
|---|---|---|---|---|
| rag_pipeline::pipeline::index cargo_resolver:resolver_config() | Instructions instructions x 1e9 | 📈 plot 🚷 threshold 🚨 alert (🔔) | 1.67 x 1e9(+59.50%)Baseline: 1.05 x 1e9 | 1.07 x 1e9 (156.37%) |
| rag_pipeline::pipeline::query_cold cargo_resolver:resolver_built_config() | Instructions instructions x 1e6 | 📈 plot 🚷 threshold 🚨 alert (🔔) | 232.30 x 1e6(+19.23%)Baseline: 194.83 x 1e6 | 198.73 x 1e6 (116.89%) |
| rag_pipeline::pipeline::query_warm cargo_resolver:resolver_index() | Instructions instructions x 1e6 | 📈 plot 🚷 threshold 🚨 alert (🔔) | 224.07 x 1e6(+18.79%)Baseline: 188.62 x 1e6 | 192.40 x 1e6 (116.46%) |
Click to view all benchmark results
| Benchmark | Estimated Cycles | cycles x 1e6 | Instructions | Benchmark Result instructions x 1e6 (Result Δ%) | Upper Boundary instructions x 1e6 (Limit %) | L1 Hits | hits x 1e6 | LL Hits | hits x 1e6 | RAM Hits | hits x 1e3 | Total read+write | reads/writes x 1e6 |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| rag_pipeline::pipeline::index cargo_resolver:resolver_config() | 📈 view plot | 2,542.34 x 1e6 | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 1,669.90 x 1e6(+59.50%)Baseline: 1,046.98 x 1e6 | 1,067.92 x 1e6 (156.37%) | 📈 view plot | 2,279.33 x 1e6 | 📈 view plot | 43.63 x 1e6 | 📈 view plot | 1,281.87 x 1e3 | 📈 view plot | 2,324.24 x 1e6 |
| rag_pipeline::pipeline::query_cold cargo_resolver:resolver_built_config() | 📈 view plot | 349.44 x 1e6 | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 232.30 x 1e6(+19.23%)Baseline: 194.83 x 1e6 | 198.73 x 1e6 (116.89%) | 📈 view plot | 333.70 x 1e6 | 📈 view plot | 2.96 x 1e6 | 📈 view plot | 27.14 x 1e3 | 📈 view plot | 336.68 x 1e6 |
| rag_pipeline::pipeline::query_warm cargo_resolver:resolver_index() | 📈 view plot | 336.87 x 1e6 | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 224.07 x 1e6(+18.79%)Baseline: 188.62 x 1e6 | 192.40 x 1e6 (116.46%) | 📈 view plot | 321.92 x 1e6 | 📈 view plot | 2.82 x 1e6 | 📈 view plot | 24.98 x 1e3 | 📈 view plot | 324.76 x 1e6 |
|
| Branch | feat/oplog-authoring |
| Testbed | ubuntu-latest |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result seconds (s) (Result Δ%) | Upper Boundary seconds (s) (Limit %) |
|---|---|---|---|
| index_time/full_rebuild_cargo | 📈 view plot 🚷 view threshold | 5.49 s(-2.12%)Baseline: 5.61 s | 7.93 s (69.31%) |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f48f45083c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
f48f450 to
6a20c9c
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a20c9c7c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
6a20c9c to
26b3681
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26b36811a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
26b3681 to
38d08f8
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38d08f82ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for row in read_memory_rows(conn, &repo_id)? { | ||
| // `all_edges_from`, not `edges_from`: a backfilled obsolete/rejected memory still carries | ||
| // its persisted edges into the complete-history log (the live reader hides them). | ||
| let edges = all_edges_from(conn, &row.memory_id)?; | ||
| ops.extend(memory_to_ops(&row, &repo_id, &edges)?); |
There was a problem hiding this comment.
Make the backfill snapshot atomic with the chain write
When this backfill runs while another writer creates or updates a memory after read_memory_rows/all_edges_from have been read but before author_batch opens its IMMEDIATE transaction, the signed batch captures only the older snapshot and then leaves the local chain non-empty; every later backfill returns at the chain-tail/idempotency gate, so the concurrent row is permanently omitted from the op-log until some separate live op happens to cover it. Because this helper is intended to produce the complete pre-live history, the memory/edge reads need to be protected by the same write transaction or otherwise block memory writes until the batch is committed.
Useful? React with 👍 / 👎.
| if repo_id == crate::index::schema::LEGACY_REPO_ID { | ||
| return Ok(()); |
There was a problem hiding this comment.
Skip backfill for upgradeable local repo IDs
When the active scope is a shallow-clone local: repo id, register_repo later upgrades that id to a portable repo id and re-points the scoped memory rows; this guard only skips the legacy placeholder, so backfill_memory_oplog can sign an immutable owner stream derived from the temporary local: id. After the upgrade, future memory ops use the portable owner stream and the backfilled history remains stranded under the retired stream, the same class of problem this placeholder check is avoiding.
Useful? React with 👍 / 👎.
… (phase B, unwired) Freeze the machinery that turns memories into signed op-log entries — the minting primitive, the row→op translation, and the one-time full backfill — without yet touching the live memory write path. - store::author_op / author_batch: mint entries by allocating the next Lamport from the (stream, device) chain tail, signing, and inserting. author_op is one op in its own txn; author_batch is a whole sequence in ONE atomic txn (an interrupted backfill rolls back and re-runs clean) with a single projection re-fold. append's reproject branch is factored into a shared helper. - oplog gains its first crate-internal API surface: a curated pub(crate) re-export (the minting fns + the op vocabulary), the one seam query::memory reaches through — a one-way dependency, so oplog never depends back on the memory subsystem. - query::memory::authoring translates a memory into NodeCreate [+ NodeStatus when non-active] [+ EdgeAdd per typed node-edge]; code-anchor bindings stay out (they are per-device derived resolution state). backfill_memory_oplog authors every one of the active repo's memories into its owner stream — idempotent (a completed atomic batch leaves a non-empty chain), scope-gated (a no-op on an unscoped DB), deterministically ordered. The log becomes the complete signed history. The minting primitives take a &LocalDevice, keeping DeviceSecret oplog-private. Still unwired: the next increment calls backfill + author inside each mutation's transaction (making create_memory/update_memory transactional). Fixes #524
38d08f8 to
1324921
Compare
Seventh phase-B op-log increment, after #480/#489/#495/#497/#500/#503/#504/#509/#511/#513/#523. Freezes the machinery that turns memories into signed log entries — the minting primitive, the row→op translation, and the one-time full backfill — WITHOUT touching the live memory write path.
What landed
store::author_op/store::author_batch— mint entries by allocating the next Lamport from the(stream, device)chain tail, signing, and inserting.author_opis one op in its ownIMMEDIATEtxn;author_batchis a whole sequence in ONE atomic txn (an interrupted backfill rolls back and re-runs clean) with a single projection re-fold at the end.append's reproject branch is factored into a sharedreproject_after_writehelper. The primitives take a&LocalDevice, soDeviceSecretstays oplog-private.pub(crate)re-export inoplog/mod.rs(the minting fns + the op vocabulary). Every submodule is otherwise private, so this is the ONE seamquery::memoryreaches through — and the only direction of the dependency (oplognever depends back onquery::memory, which the next increment's reverse call would otherwise cycle).query::memory::authoring— translates a memory intoNodeCreate[+NodeStatuswhen non-active] [+EdgeAddper outgoing typed node-edge, inedge_keyorder]. Code-anchor bindings stay out of the log: they are per-device derived resolution state, re-validated locally, never part of the shared node graph.backfill_memory_oplogauthors every one of the active repo's memories into its owner stream — idempotent (a completed atomic batch leaves a non-empty chain, so a re-run is a no-op), scope-gated (a no-op on an unscoped DB), deterministically ordered(created_at_ms, id). The log becomes the complete signed history from genesis.Still not wired to the live write path —
mod oplogstays dead-code-allowed. The next increment callsbackfill_memory_oplog+author_opinside each mutation's transaction (makingcreate_memory/update_memorytransactional), and reusesmemory_to_opsfor the mutations themselves.Tests / gates
author_opmints a genesis→1→2 chain and projects;author_batchwrites N ops in one txn with sequential lamports (empty batch is a no-op; continues a non-empty chain).memory_to_opstranslates content + non-active status + edges in order; end-to-end backfill over a 3-memory fixture (one obsolete, one edge) yields 3 projected nodes + the right status + the edge, and a second backfill is a no-op; unscoped and empty-repo backfills are no-ops.--no-default-features, nightly fmt clean.Fixes #524