perf(memory): scan graph source once per settled reconciliation - #551
Conversation
… skips Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…rable Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
AdmittedProjectionArtifactV1::from_lifecycle_install read and SHA-256 hashed all five artifact members - including the whole model file - on every authority construction, then discarded the bytes. Session open repeats the identical read-and-verify when it consumes the bytes, so every scheduled projection's artifact load and every serving restore attempt paid a redundant full model read. Construction now checks each member's structural pin (declared entry, normalized relative path, regular non-symlink file, exact length) without reading bytes, matching the artifact-store authority whose admission also defers digest checks to reads. Byte digests remain verified by read_member_bytes at every session open, so digest- mismatched bytes still can never open a session or serve a vector. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Keep 5s/30s as omitted-deadline fallbacks only. Prove BudgetExceeded stays Partial and that a semantic miss does not take down exact, lexical, or graph.
A successfully parsed LCM JSON payload whose object keys carry credential material was collapsed into StructuredSanitizationError::SanitizerUnavailable and then mapped to DetectionError::Receipt, so the sanitizer's own fail-closed quarantine surfaced as 'privacy sanitizer receipt construction failed' in every projection-drain refusal. Give quarantine findings their own typed state (CredentialKeyQuarantine -> StructuredQuarantine), keep SanitizerUnavailable for detector initialization failure (-> Initialization, with InvalidLimits), and reserve Receipt for real canonical/receipt construction faults (new CanonicalEncoding). Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
begin_maintenance_drain is only for run_maintenance_checkpoint and crate tests. A public unchecked drain would close admission with no permit. Admission stays closed after Truncate; Database will not call this on the live submit path.
|
RetrievalBudget.deadline_micros is a request/profile bound, not publication or activation. new_admitted still hard-wires None for the daemon first-query caller.
Codex P2: key quarantine no longer shares the "ambiguous structured document" message. Parse ambiguity stays StructuredQuarantine.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6a2677b1d
ℹ️ 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".
Lifecycle authority construction is structural-only, so an install path that never opens a session could mark a same-length digest-mismatched model Current. schedule_generation's AlreadyPublished resume installed the pointer without warming, and a resume reporting every batch already committed embedded nothing and installed the same way. Both install paths now warm one pooled candidate session before staging publication, exactly as prepare_restore warms before commit: a cold open reads and digest-verifies every member, so corrupt bytes stay Failed/degraded and never become Current or serving. After real embedding the warm reuses the idle pooled session without re-reading the artifact. Tests prove a digest-mismatched model never becomes Current after an AlreadyPublished resume (publication staging never runs) or restore. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
The credential-key quarantine test asserts variant equality directly, which requires PartialEq on the public error enum. Matches the derive set already carried by StructuredSanitizationError. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…nce-checkpoint-port-2ce6' into codex/pr541-review-fix
A stale snapshot inventory is request-local. Checking blockers after begin_maintenance_drain closed admission and left the writer Draining, so Blocked tore down exact-SQL the same way a foreign permit used to. Leave admission open and the writer Ready.
…udget-backoff-9603' into td/pr538-update
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
begin_maintenance_drain is only for run_maintenance_checkpoint and crate tests. A public unchecked drain would close admission with no permit. Admission stays closed after Truncate; Database will not call this on the live submit path.
A stale snapshot inventory is request-local. Checking blockers after begin_maintenance_drain closed admission and left the writer Draining, so Blocked tore down exact-SQL the same way a foreign permit used to. Leave admission open and the writer Ready.
…idence-budget-backoff-9603
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
code-index consumer view (source / load)
We are not a caller of load_source / project_memory_graph. Same class of leftover as our graph-manifest memo: a cheaper recheck must not look like a seat.
What looks good:
- Stamp is captured in the same read snapshot as the source (
load_source→lineage_stamp_tx). source_unchanged_since: absent stamp isfalse(reload). Equal stamp is the old watermark-matchOk. Moved stamp falls through to fullload_source+ watermark compare.finish_reconciliation_watermarkstamp-equal returns after this pass already published. It does not skip close/reopen or recovered-digest on our port.- Serve path still
GraphConflicts if the verified generation does not match the first-load expectation, or if the stamp moved and the reloaded watermark differs. - Tests drive
publish_project_memory_graph_after_write, not a test-only load port. Codex P1 (load_source_for_test/finish_reconciliation_watermark_for_test) is gone ond011d96ff.
Not a silent reinterpret of seating. Stamp skip ≠ Plan 39 seating.
What’s blocking ready
Nothing must-fix from this crate. Still draft. Isolated store::memory tests are the evidence; don’t retrigger CI. Leave the base. Don’t flatten.
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Cross-review of draft d011d96ff. Comments only. Not editing runtime-core. Not changing the base. Not retriggering CI.
Crate — not blocking ready: load_source still returns a lineage_stamp; source_unchanged_since is the cheap recheck. Stamp-equal Ok in finish_reconciliation_watermark does not swallow pending — worker clears pending when a pass starts and reruns if it is set again during the pass (memory_graph_reconciliation_pending only suppresses GraphConflict on watermark mismatch). Earlier inline on that was wrong; correction is on the thread and I resolved it. load_source_for_test / finish_reconciliation_watermark_for_test are gone; remaining *_for_test helpers are #[cfg(test)] + pub(in crate::store::memory). Codex P1 on those ports is outdated (resolved).
Blocking ready: draft; mergeable unknown; no checks on the rollup. Ready watch is empty until Zack names one.
Leftover: none in this crate from this pass.
…t-ea75' into cursor/mount-lexical-budget-725a Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…' into cursor/semantic-cheap-authority-construction-36ae Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
…raph-db-hydrate-digest-off-gate-13bf Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Materializing a foreign/WAL read-snapshot copy previously ran a full rusqlite backup of the scratch copy into a second standalone file, rewriting every main-database page after the family was already copied. Leave WAL journal mode on the exclusively owned copy instead: that checkpoints only the WAL-resident pages in place, drops the second file, and keeps peak scratch near one family copy. Isolated fixture (38 MiB main + 4 MiB WAL, no reflink): 79,918,280 bytes written on the previous tip vs 46,338,756 bytes now, with the same readable snapshot rows. DirectImmutable stays no-copy. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
The projected memory-graph read and reconciliation paths loaded the full canonical owner source twice, re-parsing every payload_json and event_json a second time only to rehash the same ID-derived watermark. load_source now captures the highest lineage event_sequence inside the same read snapshot; because the lineage table is append-only and every source-affecting mutation records a lineage event in the same transaction on both fact write paths, an unchanged stamp proves the watermark is unchanged. A moved or absent stamp falls back to the full reload and watermark compare, preserving exact conflict semantics. A settled reconciliation pass over eight 16KiB payload facts drops from 48 source rows / 278656 bytes loaded to 24 rows / 139328 bytes. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Drop the cfg(test) ports load_source_for_test and finish_reconciliation_watermark_for_test. The 2-to-1 source-load claim now baselines against an unmounted publication pass, which records exactly one canonical source load before failing at the graph mount, and compares a settled publish_project_memory_graph_after_write pass against it through the same reconciliation telemetry. The stale-stamp conflict invariant is proven by parking a publication inside the verified-graph reconcile, committing a fact mid-pass, and releasing it: the finish reloads the mutated source, surfaces GraphConflict with nothing pending, and the publisher schedules exactly one republication that settles with one further load. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
d011d96 to
185aaa8
Compare
Problem
Project memory graph projection (
project_memory_graph) and reconciliation (reconcile_project_memory_graph_pass→finish_reconciliation_watermark) each scanned the full owner source twice per operation, including everypayload_jsonandevent_json. The second scan existed only to recompute the ID-derivedsource_watermarkand compare it against the first — but the watermark's MENTIONS relations derive from payload entities, so "just rehash IDs" still forced a full payload reload and parse. On a live daemon with a large project memory this doubles source work, bytes materialized, and transient RSS on every settled pass and every graph read.Change (runtime-core only)
load_sourcenow also captures the highestmemory_v2_lineage_events.event_sequenceinside the same read snapshot (LoadedMemoryGraphSource.lineage_stamp). Both recheck sites first asksource_unchanged_since, a single indexed-scalar read on a fresh snapshot:memory_graph_reconciliation_pending()remains consulted only on the mismatch/fallback path, where it has always suppressed the conflict);load_source+ watermark compare, byte-identical to the previous behavior.Why the stamp is a sound change token:
memory_v2_lineage_eventsis append-only (schema triggers reject UPDATE/DELETE) with anAUTOINCREMENTsequence that is never reused, and every committed mutation that can change the projected source — new facts, assertions, evidence, payload rows, curated relations, active-assertion or payload-access transitions — records at least one lineage event in the same transaction on both fact write paths (the facadecommit_fact_tx, which derives everymemory_v2_current_factschange frombatch.events(), and the storage-runtimeFactExecutor::execute_write, which rejects event-less batches). Source-neutral writes (retrieval/feedback counters, superseded-payload purges) never touch rows the source reads; unrelated writes only advance the stamp and trigger the conservative fallback.The durable
graph_transaction_generationmetadata counter was considered and rejected: it is only bumped by theDatabasefacade commit and is bypassed by the storage-runtime writer path, so it cannot prove source stability.Evidence (isolated crate tests, production journeys, tempdir fixtures only)
store::memory::graph_reconciliation_tests::reconciliation_pass_loads_large_payload_source_onceseeds 8 facts with 16 KiBpayload_jsoneach, baselines one canonical source load via an unmounted publication pass (which records exactly one load before failing at the graph mount), then runs one settledpublish_project_memory_graph_after_writepass and asserts it performs exactly one load through the same telemetry:Before-numbers captured by temporarily forcing the fallback on this branch; the test fails on the legacy double-load behavior (
left: 48, right: 24).store::memory::graph_reconciliation_tests::source_mutation_during_publication_conflicts_and_republishesparks a publication inside the verified-graph reconcile, commits a fact mid-pass (which settles through the ordinary write journey with exactly one load), then releases the parked pass: with a stale stamp, a stale watermark, and nothing pending, the finish reloads the canonical source, surfacesGraphConflict, and the publisher schedules exactly one republication — the post-release tail is exactly two loads of the mutated source (the conflicted finish's fallback reload plus the scheduled republication's own load).Verification
cargo test -p tracedecay-runtime-core --lib store::memory— 129 passed, 0 failed.cargo clippy -p tracedecay-runtime-core --all-features --tests— clean;cargo fmt -p tracedecay-runtime-core --check— clean.store_runtime::resolver::tests::project_graph_locator_...fails_closedfails on the clean base too (overlayfs locality check). The head CI Format failure is inherited from the base branch (crates/tracedecay-sessions/.../jsonl_observation_admission/tests.rs,src/commands/daemon.rs— both byte-identical to the base tip and last touched by base commits).Files
crates/tracedecay-runtime-core/src/store/memory/graph.rscrates/tracedecay-runtime-core/src/store/memory/graph_reconciliation_tests.rs