refactor(storage_engine): remove the dead materialization machinery by jsam · Pull Request #368 · inputlayer/inputlayer

jsam · 2026-10-05T18:10:33Z

Intent

please leave this query batching functionliaty just refactor things so its clear and optimized for this ... auto materialize needs to be default mode of operation where rules get deployed as views and everything else is a query .. make the docs true with the correct systems design ... this is not accceptable state for production readiness (Context: implement #307 exactly as written.)

Issue #307, "Remove the dead materialization machinery (V0)", part of epic #305 (persistent rules become incrementally maintained views; everything else is a query):

Problem. auto_materialize, refresh_materializations, evaluate_rule, materialize_derived_relation, set_auto_materialize (src/storage_engine/mod.rs), DerivedRelationsManager/CompiledRule (src/derived_relations.rs, 797 lines; clauses always empty per src/storage_engine/catalog_change.rs), the materialized_relations field threaded through snapshots and the change log (src/storage_engine/snapshot.rs; src/storage_engine/precondition.rs), install_catalog's dd.register_rule branch (catalog_change.rs), shadow_write (program_commit.rs) and materialize_tests.rs are reachable only from tests and describe a design that will not be built. They mislead readers (and the architecture doc) and add a lock around every snapshot publish when enabled.

Target. Delete all of the above and the IncrementalEngine derived-relations API (register_rule, set_materialized, notify_base_update, derived_relations()), keeping the worker-thread skeleton (IncrementalEngine::new/worker_loop/insert/delete in src/incremental.rs) as the seed of the later view maintainer. publish_snapshot takes the simple branch only. Docs in src/ rewritten to tell the truth (persistent rules are recomputed on read today).

Acceptance. grep -rn "materializ" src/ returns only comments explaining recompute-on-read; all tests pass; cargo clippy -D warnings clean; snapshot publish has no DerivedRelationsManager lock; binary size and test count reported in the PR.

What Changed

Deleted the test-only materialization path. That covers src/derived_relations.rs (DerivedRelationsManager/CompiledRule), materialize_tests.rs, auto_materialize/set_auto_materialize/refresh_materializations/evaluate_rule/materialize_derived_relation in KnowledgeGraph, the materialized_relations field in snapshots and the change log, the dd.register_rule branch in install_catalog, and shadow_write in program commit. publish_snapshot now has a single path with no DerivedRelationsManager lock, and apply_delta no longer takes a time argument or returns an error.
Reduced IncrementalEngine to the worker-thread skeleton (new, the worker loop, insert, delete), dropping its derived-relations API and the tests for it. Also removed the now-unreachable StorageError::IncrementalEngineError/DDWorkerDisconnected variants, CommitError::Unknown, and the write_run handler arm that mapped CommitError::Unknown.
Updated the architecture and coding-standards internals docs (plus the bundled GUI copy) and the src/ comments. They now say persistent rules are recomputed on read, and that IncrementalEngine is an unused skeleton that the planned view maintainer will replace.

Risk Assessment

✅ Low: The change deletes code that only tests could reach. publish_snapshot, WriteProgram::view, ChangeLog and apply_delta all reduce exactly to their previous materialized-set-empty branches, so server behaviour is unchanged. The docs fix round now matches the code, with no leftover references to the deleted machinery.

Testing

I built the release server and client and ran the server in a throwaway data directory. I drove six live scenarios over the CLI client and raw WebSocket: persistent-rule recompute-on-read (including recursion), conditional delete and rule drop, atomic program commit and rollback, standing-query deltas over a rule-derived relation, session rules, and restart persistence. All passed, with no panics in the server logs. The acceptance grep for materializ in src/ is empty, and the new targeted unit test passes. Transcripts are saved in the evidence directory. There is no UI surface, so no screenshots were taken.

Live validation: ✅ go - 6 of 6 scenarios driven live against the product

Scenario	Result	Live	Evidence
A persistent recursive rule (reachable) gives fresh results on each read after edge inserts and deletes	✅ pass	live	s1-persistent-rules-recompute-on-read.txt: 3 rows, then 6 after +edge(3,4), then 3 after -edge(1,2)
A rule chained on a rule-derived relation (big over reachable) and a query-only session rule both evaluate correctly; .rule list shows only the persistent rules	✅ pass	live	s1-persistent-rules-recompute-on-read.txt
A conditional delete and .rule drop are reflected in later reads	✅ pass	live	s2-conditional-delete-program-drop.txt
An atomic multi-statement program with facts, a rule and a query commits through program_commit; a failing program rolls back all of its writes	✅ pass	live	s3-atomic-program-commit-and-rollback.txt: adult=[alice]; carol not inserted after rollback; dave appears after a later insert
A subscriber to ?adult(N) gets an insert delta for an adult, a retract delta when one is deleted, and no delta for a minor (snapshot publish path without a DerivedRelationsManager)	✅ pass	live	s4-standing-query-over-rule.txt
After a server restart, persistent rules and facts reload and rules keep recomputing on read after new inserts	✅ pass	live	s5-restart-persistence.txt

Evidence: Persistent rules recompute on read (insert/delete/recursion/session rule)

Connecting to server at http://127.0.0.1:18571...
Connected!

Server status: healthy
Authenticated as: admin
Current knowledge graph: default

> .kg create gtest
Knowledge graph 'gtest' created.
Switched to knowledge graph: gtest
> .kg use gtest
Switched to knowledge graph: gtest
> +edge(1, 2)
Inserted 1 fact(s) into 'edge'.
> +edge(2, 3)
Inserted 1 fact(s) into 'edge'.
> +reachable(X, Y) <- edge(X, Y)
Rule 'reachable' registered.
> +reachable(X, Y) <- reachable(X, Z), edge(Z, Y)
Rule 'reachable' registered.
> ?reachable(X, Y)
┌───┬───┐
│ X │ Y │
├───┼───┤
│ 1 │ 2 │
│ 1 │ 3 │
│ 2 │ 3 │
└───┴───┘
3 rows
> +edge(3, 4)
Inserted 1 fact(s) into 'edge'.
> ?reachable(X, Y)
┌───┬───┐
│ X │ Y │
├───┼───┤
│ 1 │ 2 │
│ 1 │ 3 │
│ 1 │ 4 │
│ 2 │ 3 │
│ 2 │ 4 │
│ 3 │ 4 │
└───┴───┘
6 rows
> -edge(1, 2)
Deleted 1 facts from 'edge'.
> ?reachable(X, Y)
┌───┬───┐
│ X │ Y │
├───┼───┤
│ 2 │ 3 │
│ 2 │ 4 │
│ 3 │ 4 │
└───┴───┘
3 rows
> +big(X) <- reachable(X, Y), Y > 3
Rule 'big' registered.
> ?big(X)
┌───┐
│ X │
├───┤
│ 2 │
│ 3 │
└───┘
2 rows
> temp(X, Y) <- edge(X, Y), X < Y
┌───┬───┐
│ X │ Y │
├───┼───┤
│ 2 │ 3 │
│ 3 │ 4 │
└───┴───┘
2 rows
(session: 1 rule(s))
> ?temp(X, Y)
┌───┬───┐
│ X │ Y │
├───┼───┤
│ 2 │ 3 │
│ 3 │ 4 │
└───┴───┘
2 rows
> .rule list
Rules:
  big (1 clause(s))
  reachable (2 clause(s))

Evidence: Conditional delete and rule drop

Connecting to server at http://127.0.0.1:18571...
Connected!

Server status: healthy
Authenticated as: admin
Current knowledge graph: default

> .kg use gtest
Switched to knowledge graph: gtest
> -edge(X, Y) <- edge(X, Y), X = 3
Conditional delete: 1 fact(s) deleted from 'edge'.
> ?reachable(X, Y)
┌───┬───┐
│ X │ Y │
├───┼───┤
│ 2 │ 3 │
└───┴───┘
1 rows
> +person("alice", 30), +person("bob", 12), +adult(N) <- person(N, A), A >= 18
Parse error: Invalid term: '30)'
> ?adult(N)
No results.
> .rule drop big
Rule 'big' dropped.
> .rule list
Rules:
  reachable (2 clause(s))
> ?edge(X, Y)
┌───┬───┐
│ X │ Y │
├───┼───┤
│ 2 │ 3 │
└───┴───┘
1 rows

Evidence: Atomic program commit and rollback over WebSocket

>>> PROGRAM:
+person("alice", 30)
+person("bob", 12)
+adult(N) <- person(N, A), A >= 18
?adult(N)
<<< {"type":"result","columns":["N"],"rows":[["alice"]],"errors":[]}
>>> PROGRAM:
+person("carol", 40)
.rule drop missing
<<< {"type":"result","columns":["message"],"rows":[["Rule 'missing' not found: Failed to drop rule: Rule 'missing' does not exist (rolled back: none of the program's 2 write statements 0-1 was applied)"]],"errors":[{"index":1,"code":"not_found","message":"Rule 'missing' not found: Failed to drop rule: Rule 'missing' does not exist (rolled back: none of the program's 2 write statements 0-1 was applied)"}]}
>>> PROGRAM:
?person(N, A)
<<< {"type":"result","columns":["N","A"],"rows":[["alice",30],["bob",12]],"errors":[]}
>>> PROGRAM:
?adult(N)
<<< {"type":"result","columns":["N"],"rows":[["alice"]],"errors":[]}
>>> PROGRAM:
+person("dave", 50)
?adult(N)
<<< {"type":"result","columns":["N"],"rows":[["alice"],["dave"]],"errors":[]}

Evidence: Standing query deltas over a persistent rule

SUBSCRIBER <<< {"type":"result","id":"s","columns":["N"],"rows":[["alice"],["dave"]],"row_count":2,"total_count":2,"truncated":false,"execution_time_ms":0,"errors":[],"subscribed":{"subscription":"adults","generation":1,"revision":16}}
WRITER >>> +person("erin", 33)
SUBSCRIBER <<< {"type":"persistent_update","knowledge_graph":"gtest","relation":"person","operation":"insert","count":1,"timestamp_ms":1791229161020,"seq":14}
WRITER <<< {"type":"persistent_update","knowledge_graph":"gtest","relation":"person","operation":"insert","count":1,"timestamp_ms":1791229161020,"seq":14}
WRITER <<< {"type":"result","columns":["message"],"rows":[["Inserted 1 fact(s) into 'person'."]],"row_count":1,"total_count":1,"truncated":false,"execution_time_ms":100,"row_provenance":["unknown"],"errors":[],"statements":[{"index":0,"kind":"insert","inserted":1,"deleted":0}],"revision":17}
SUBSCRIBER <<< {"type":"subscription_delta","subscription":"adults","generation":1,"knowledge_graph":"gtest","seq":1,"revision":17,"columns":["N"],"inserted":[["erin"]],"retracted":[]}
WRITER >>> -person("alice", 30)
SUBSCRIBER <<< {"type":"persistent_update","knowledge_graph":"gtest","relation":"person","operation":"delete","count":1,"timestamp_ms":1791229161787,"seq":15}
WRITER <<< {"type":"result","columns":["message"],"rows":[["Deleted 1 facts from 'person'."]],"row_count":1,"total_count":1,"truncated":false,"execution_time_ms":166,"row_provenance":["unknown"],"errors":[],"statements":[{"index":0,"kind":"delete","inserted":0,"deleted":1}],"revision":18}
WRITER <<< {"type":"persistent_update","knowledge_graph":"gtest","relation":"person","operation":"delete","count":1,"timestamp_ms":1791229161787,"seq":15}
SUBSCRIBER <<< {"type":"subscription_delta","subscription":"adults","generation":1,"knowledge_graph":"gtest","seq":2,"revision":18,"columns":["N"],"inserted":[],"retracted":[["alice"]]}
WRITER >>> +person("kid", 5)
SUBSCRIBER <<< {"type":"persistent_update","knowledge_graph":"gtest","relation":"person","operation":"insert","count":1,"timestamp_ms":1791229162467,"seq":16}
WRITER <<< {"type":"result","columns":["message"],"rows":[["Inserted 1 fact(s) into 'person'."]],"row_count":1,"total_count":1,"truncated":false,"execution_time_ms":141,"row_provenance":["unknown"],"errors":[],"statements":[{"index":0,"kind":"insert","inserted":1,"deleted":0}],"revision":19}
WRITER <<< {"type":"persistent_update","knowledge_graph":"gtest","relation":"person","operation":"insert","count":1,"timestamp_ms":1791229162467,"seq":16}

Evidence: Rules and facts survive a server restart

>>> PROGRAM:
.rule list
<<< {"type":"result","columns":["message"],"rows":[["Rules:"],["  adult (1 clause(s))"],["  reachable (2 clause(s))"]],"errors":[]}
>>> PROGRAM:
?reachable(X, Y)
<<< {"type":"result","columns":["X","Y"],"rows":[[2,3]],"errors":[]}
>>> PROGRAM:
?adult(N)
<<< {"type":"result","columns":["N"],"rows":[["dave"],["erin"]],"errors":[]}
>>> PROGRAM:
+edge(3, 9)
?reachable(X, Y)
<<< {"type":"result","columns":["X","Y"],"rows":[[2,3],[2,9],[3,9]],"errors":[]}

Numbers

Measured against the merge base 09d1d0cb, release profile, same machine:

	base	this PR	change
`inputlayer-server` binary	42,225,456 B	42,196,472 B	-28,984 B (-0.07%)
`inputlayer` CLI binary	14,473,560 B	14,517,776 B	+44,216 B (+0.3%; no CLI code changed, codegen noise)
`#[test]` functions in `src/`, `tests/`, `benches/`	4,265	4,202	-63 (64 removed, 1 added)

Removed tests exercised only the deleted paths: the incremental-engine mirror (test_dd_*), the materialization pipeline, snapshot materialization, and the DerivedRelationsManager / derived-API unit tests. Kept and re-pointed: the two SCC rule tests (now on a plain storage engine) and the residency test (its in-memory state is now a vector index instead of the incremental engine). Added: test_snapshot_evaluates_persistent_rules_on_read.

cargo test --workspace --all-features: 4,343 passed, 0 failed, 0 ignored. cargo clippy --all-targets -- -D warnings: clean.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

🔧 **Rebase** - 4 issues found → auto-fixed ✅

⚠️ src/derived_relations.rs - merge conflict rebasing onto origin/main
⚠️ src/incremental.rs - merge conflict rebasing onto origin/main
⚠️ src/statement/mod.rs - merge conflict rebasing onto origin/main
⚠️ src/storage_engine/mod.rs - merge conflict rebasing onto origin/main

🔧 Fix applied.
✅ Re-checked - no issues remain.

🔧 **Review** - 1 issue found → auto-fixed ✅

⚠️ docs/content/docs/internals/architecture.mdx:80 - The user intent says to "make the docs true", but the architecture docs still describe code this change deletes. docs/content/docs/internals/architecture.mdx:80 lists the auto_materialize: bool field. Line 95 describes materialized_relations. Lines 97-103 have section 2.4 on IncrementalEngine/DerivedRelationsManager, enable_incremental() and set_auto_materialize(). Line 579 has a file-table row for src/derived_relations.rs. Line 651 names "the shadow_write hook", which is now removed. The same stale derived_relations.rs # DerivedRelationsManager entry also appears at docs/internals/README.md:67, docs/internals/coding-standards.md:395 and docs/content/docs/internals/coding-standards.mdx:395. Fix: rewrite section 2.4 to describe only the remaining IncrementalEngine worker skeleton (unused by the server; seed of the Epic: persistent rules are incrementally maintained views; everything else is a query (E0) #305 view maintainer). Drop the deleted fields and file rows. Reword line 651 so it says the commit path will need a new hook instead of pointing at the deleted shadow_write.

🔧 Fix applied.
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

Live validation: ✅ go - 6 of 6 scenarios driven live against the product

Scenario	Result	Live	Evidence
A persistent recursive rule (reachable) gives fresh results on each read after edge inserts and deletes	✅ pass	live	s1-persistent-rules-recompute-on-read.txt: 3 rows, then 6 after +edge(3,4), then 3 after -edge(1,2)
A rule chained on a rule-derived relation (big over reachable) and a query-only session rule both evaluate correctly; .rule list shows only the persistent rules	✅ pass	live	s1-persistent-rules-recompute-on-read.txt
A conditional delete and .rule drop are reflected in later reads	✅ pass	live	s2-conditional-delete-program-drop.txt
An atomic multi-statement program with facts, a rule and a query commits through program_commit; a failing program rolls back all of its writes	✅ pass	live	s3-atomic-program-commit-and-rollback.txt: adult=[alice]; carol not inserted after rollback; dave appears after a later insert
A subscriber to ?adult(N) gets an insert delta for an adult, a retract delta when one is deleted, and no delta for a minor (snapshot publish path without a DerivedRelationsManager)	✅ pass	live	s4-standing-query-over-rule.txt
After a server restart, persistent rules and facts reload and rules keep recomputing on read after new inserts	✅ pass	live	s5-restart-persistence.txt

cargo build --release --bin inputlayer-server --bin inputlayer-client
Started a release inputlayer-server on port 18571 with a throwaway --data-dir in /tmp and a bootstrap API key set by environment variables
inputlayer-client -k <key> s1.iql: recursive persistent rule reachable, then insert, delete, chained rule, session rule and .rule list
inputlayer-client -k <key> s2.iql: conditional delete, .rule drop, then a reachable query
WebSocket client (node): multi-statement program with facts, rule and query; a failing program rolled back (.rule drop missing)
WebSocket client (node): .subscribe adults ?adult(N) while a second connection inserts and deletes facts, checking the subscription deltas
Restarted the server on the same data dir, then queried .rule list, ?reachable, ?adult and a new insert
grep -rn materializ src/ (empty)
cargo test --lib test_snapshot_evaluates_persistent_rules_on_read

✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Persistent rules are stored definitions that every read evaluates from the snapshot's base facts. The materialization path built around the incremental engine was reachable only from tests and described a design that will not be built: auto_materialize recomputed every rule on each publish, CompiledRule.clauses was always empty, and no server path ever enabled the incremental engine. Removed: auto_materialize, set_auto_materialize, refresh_materializations, evaluate_rule, materialize_derived_relation, enable_incremental and the KnowledgeGraph's incremental engine, shadow_write, install_catalog's register_rule branch and compile_for_dd, DerivedRelationsManager/CompiledRule (src/derived_relations.rs), the IncrementalEngine derived-relations API, materialized_relations in snapshots, the change log and staged views, materialize_tests.rs, the storage error variants only those paths produced, and CommitError::Unknown, which only a failed shadow write produced. publish_snapshot now takes the single plain path with no DerivedRelationsManager lock. IncrementalEngine keeps its worker-thread skeleton (new, worker_loop, insert, delete and the base-relation reads) as the seed of the per-graph view maintainer. Comments in src/ now describe recompute-on-read.

…tion machinery

…lization removal

This comment has been minimized.

Sign in to view

jsam added 3 commits October 5, 2026 19:31

no-mistakes(review): Update internal docs to drop removed materializa…

07061ef

…tion machinery

no-mistakes(document): Apply rustfmt to residency_tests after materia…

be56809

…lization removal

jsam force-pushed the fm/m9-v0-dead-code branch from 5dd5add to be56809 Compare October 5, 2026 19:44

jsam changed the title ~~refactor(engine): remove the dead materialization machinery (#307)~~ refactor(storage_engine): remove the dead materialization machinery Oct 5, 2026

jsam marked this pull request as ready for review October 5, 2026 20:36

jsam merged commit 67a9782 into main Oct 5, 2026
37 of 39 checks passed

jsam deleted the fm/m9-v0-dead-code branch October 5, 2026 20:38

jsam mentioned this pull request Oct 5, 2026

Remove the dead materialization machinery (V0) #307

Closed

Navigation Menu

Repository navigation

refactor(storage_engine): remove the dead materialization machinery - #368

refactor(storage_engine): remove the dead materialization machinery#368
jsam merged 3 commits into
mainfrom
fm/m9-v0-dead-code

jsam commented Oct 5, 2026 •

edited

Loading

Uh oh!

This comment has been minimized.

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jsam commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Intent

What Changed

Risk Assessment

Testing

Numbers

Pipeline

Uh oh!

This comment has been minimized.

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jsam commented Oct 5, 2026 •

edited

Loading