Repository navigation
Conversation
The workspace had no lint configuration, no CI, and no declared MSRV, so
nothing prevented the public surface and warning count from drifting. This
adds the gates and, more importantly, a baseline to measure against.
- [workspace.package] for version/edition/license/rust-version/authors,
inherited by all 8 crates. shared, python, cli and kolibrie-http-server
had no license field despite MPL headers in every source file.
- [workspace.lints], all at warn for now. Escalation to deny happens
per crate as each reaches zero.
- default-members now includes datalogmtl and ml; a bare cargo build
never checked them before.
- rust-toolchain.toml pins 1.96 for development. This is separate from
the MSRV, which is 1.85 (the floor imposed by hashbrown, indexmap and
toml_edit) and is enforced by its own CI job rather than just declared.
- CI: test (default + no-default-features), MSRV, rustdoc and the
ml/python build are blocking. fmt, clippy, cargo-deny and
cargo-public-api are advisory for now.
- scripts/warning_report.py so the burn-down number is reproducible
locally, not only in CI.
Baseline on the merged tree: 649 tests pass, 0 fail. 3,022 warnings --
2,713 missing_docs, 144 unreachable_pub, 80 missing_debug_implementations,
50 unused_qualifications, 27 deprecated, 7 unsafe_code.
fmt is advisory because the tree has never been formatted and cargo fmt
would rewrite ~1,590 hunks. That reformat needs a quiet merge window since
it conflicts with every in-flight branch; flipping continue-on-error to
false belongs in the same commit that does it.
Manifests and config only -- no source file is touched and no behaviour
changes.
Five manifest issues, none of which change behaviour. The pyo3 one was a
real bug rather than tidying.
- rumqttc and chrono are used only by examples (verified: zero references
in any src/) but were declared as normal dependencies, so every consumer
of kolibrie pulled in tokio and rustls. Moved to dev-dependencies;
kolibrie-http-server's tree no longer contains either.
- The workspace pinned pyo3 features [extension-module, auto-initialize,
abi3-py310] for all consumers. extension-module is correct for the
cdylib and wrong for ml, which embeds Python -- it suppresses libpython
linking, which is why ml/build.rs hand-rolls per-OS libpython discovery.
Features are now declared per consumer: python gets extension-module +
abi3-py310, ml and kolibrie get auto-initialize.
- ml used pyo3-build-config 0.23.3 against pyo3 0.25.1, so Cargo.lock
carried both generations and the build script configured linkage with a
different pyo3 than the one being compiled. Aligned to 0.25.
- datalog declared shared as a dev-dependency identical to its normal
dependency.
- kolibrie-http-server declared serde and serde_json locally instead of
using the workspace entries.
scan_rejects_repeated_variable_conflicts_before_extending_rows failed
roughly 20% of the time (5 of 25 isolated runs). The code under test was
fine; the test asserted a positional property of a result that has no
defined order.
DatasetIndex::query_graph returns hash order, and Rust re-seeds
RandomState per process, so the four fixture quads come back in a
different order each run. The precondition required a non-self-referential
quad to appear before the last self-referential one:
let last_valid = all.iter().rposition(|(s, _, o)| s == o).unwrap();
assert!(all[..last_valid].iter().any(|(s, _, o)| s != o));
which is false whenever both self-referential quads happen to sort first.
That property cannot be asserted deterministically while the index order
is arbitrary, so the precondition now checks the fixture by content --
that it contains both kinds of quad, which is all it needed -- and the
result is compared with the module's existing sorted() helper instead of
assert_eq! on a Vec.
The test still asserts the same behaviour: an unbound row matches both
self-referential quads, x = a extends while keeping its other bindings,
and x = d is rejected because ?x and are the same variable. Only the
ordering assumption is gone.
0 failures in 30 runs, from 5 in 25.
Eight other tests in this file compare Bindings with assert_eq! on a Vec
and are latently exposed to the same variation. They pass today because
they derive their expected order from the same query, which is luck rather
than design. Whether row order is part of the contract at all is logged as
an open question in the architecture doc, §4.
Commit structure and the changelog requirement were tribal knowledge, so
neither was applied consistently -- the three commits preceding this one
did not update changes.txt because the rule did not exist in writing.
CONTRIBUTING.md defines:
- the commit message structure (third-person subject, required body
explaining why, evidence in the form of test counts and benchmark
deltas, and an explicit note of what was deliberately left undone)
- that every commit must add a numbered entry to changes.txt
- the pre-commit checks, including comparing the test suite against the
known-good baseline rather than against zero
- a short section for AI coding assistants
It lives in CONTRIBUTING.md rather than a tool-specific file for two
reasons: team members use different assistants, and CLAUDE.md is listed
in .gitignore, so anything written there stays on one machine. CLAUDE.md
now carries a pointer to CONTRIBUTING.md instead of a copy of the rules,
which is why it does not appear in this diff.
Tests show the refactor preserves behaviour; nothing showed it preserves
performance.
- 5 microbenchmarks isolating what the refactor changes
- scripts/perf_bench.sh: routine (32 cases, ~350s) and heavy tiers.
cargo bench --workspace is unusable: four parameter sweeps dominate it
and citybench_cross_window_compare ran 36+ min without finishing.
Reducing sample size does not help; the cost is per-iteration workload.
- scripts/perf_report.py (collect, baseline, compare, noise) and
perf-baseline/baseline.json, so results survive cargo clean
- compare uses per-benchmark noise thresholds: measured noise spans
0.1% to 10.7%, so one global threshold is either blind to the quiet
benchmarks or cries wolf on the noisy ones
Rollout step 3. Steps 3-5 shrink the public surface before it gets documented, so the 2,663 remaining missing_docs warnings are not spent on items that should never have been public. - datalog::reasoning_experimental (307 lines) and its only consumers, the hierarchy_reasoning and hierarchy_reasoning_advanced examples - QueryEngineConfig and with_config: a field-less unit struct and a constructor that ignored it, documenting configurability that did not exist - StorageMode::Hybrid, never constructed, so the Static | Hybrid arm was unreachable and callers matched a state they could not observe - IntervalFactStore::coalesce and holds_at_all, the latter carrying a stale comment claiming the Box evaluator used it - CostEstimator::count_bound_variables with its three tests and the helper they orphaned: the cost model never called it, so the tests only showed that unreachable code worked - ShapeIngester's stored-but-never-read dictionary, and the parameter from its constructor, across six call sites Two suppressions were wrong rather than merely stale. The #[allow(dead_code)] on impl SparqlDatabase masked nothing at all -- pub items in a library crate are never dead code -- so a 23-field impl carried a blanket suppression that could only ever have hidden private helpers. The s2r Consumer harness is used by a CSPARQLWindow test, so it is now #[cfg(test)]; the suppression had hidden that it was test infrastructure and not dead code. Kept deliberately: VTreeNode::Leaf::var in shared/src/sdd.rs is never read, since var_to_vtree holds the same mapping in the direction lookups use, but a vtree leaf that does not record its variable is not independently interpretable. The allow is narrowed from the enum to that one field so anything else dying there still warns.
Rollout step 4a. Steps 3-5 shrink the public surface before it gets documented, so the remaining missing_docs warnings are not spent on items that should never have been public. Step 1's unreachable_pub lint drives the analysis; this takes the module-level subset. - delete kolibrie::query_engine (207 lines, 3 tests): no consumer inside or outside the crate, so demoting it would only have made it dead. Removes QueryEngine, QueryExplanation, QueryEngineStats and StorageMode from the public API; the facade gets written deliberately when the facade crate lands - demote 5 kolibrie modules: error_handler, ml_feature_loader, term_order, ml_predict_candle, ml_predict_runtime - demote 4 datalog materialisation submodules: my_naive, semi_naive, semi_naive_parallel, semi_naive_with_repairs Corrects the architecture document on the datalog half. It scoped six submodules; only four have no external path reference. The count came from a module-path grep, which is the wrong test here: these contain inherent blocks, and inherent impls are not namespaced by their module, so callers reach reasoner.infer_new_facts_semi_naive() through the type. semi_naive has 43 external call sites against zero references to its module path. Stated plainly, because it affects how this PR should be read: those four demotions do not hide any method. The methods stay public via Reasoner. What they buy is that nothing outside can couple to these paths, and four modules leave the documentation ledger. The kolibrie half is a real surface reduction. The demotion surfaced two latent problems. ml_feature_loader'
serde_json was declared without . Serialisation uses ryu and is exact, but the default parser takes a fast path that can land 1 ULP off, so every save-then-load cycle could silently perturb stored f64 values. In the ml crate that means model weights changed on load -- a real persistence defect, not only a test problem. byte_loading_checks_all_layer_dimensions compared an original and a reloaded model's forward outputs for exact equality and failed about 1 run in 25. Because new() randomises weights, whether any weight hit the imprecise parse path varied per run, which is what made it intermittent rather than deterministic.
Rollout step 4b. Step 1 set the lint to warn and recorded 144 warnings as the demotion worklist; step 4a cleared the module-level subset. This finishes the items. - 24 items demoted to pub(crate): 22 in library code, mostly cascading from the modules made private in 4a, plus two helpers in example and test support files - 55 others are false positives in 4 files: pub is required there for visibility from a test binary or example main, but nothing can import from those crate roots. tests/common/mod.rs is the clear case -- five test files call common::bag, so removing pub breaks the build. Each allow carries its reason. - scripts/check_unreachable_pub.sh plus a blocking CI job keep the count at zero The lint stays in [workspace.lints] rather than : making a module private is exactly what produces this warning, so a hard local gate would break the build halfway through the module moves steps 5 and 6 require. The gate belongs at PR time. Checked under three feature configurations, not one. Eight of the 24 items were invisible to a default-features check because they sit behind #[cfg(feature = ml)]. unreachable_pub 144 -> 0. Total warnings 2,941 -> 2,765. 643 tests pass, 0 fail, unchanged. check, examples, benches,
Rollout step 6, the last of the agreed first steps. The smallest
change carrying real architectural meaning: matches_rule_pattern and
construct_triple relate a triple pattern to a fact through a variable
binding and contain no reasoning, so no engine should need the Datalog
reasoner to reach them.
They were also the ENTIRE coupling between datalogmtl and datalog --
four imports. cargo tree now shows datalogmtl depending only on
shared, so the temporal and Datalog reasoners are siblings over a
shared vocabulary rather than one layered on the other, which is what
the temporal, N3 and incremental work all need.
datalog::reasoning::{matches_rule_pattern, construct_triple} keep
working as re-exports, so nothing downstream breaks.
Conventions carried over are now written down rather than silently
inherited: bindings are all-or-nothing, unbound variables and quoted
triples resolve to term id 0 instead of reporting an unbound
conclusion, and construct_triple's dictionary parameter is unused but
retained because §4's parse/prepare/execute split gives it a
TermEncoder.
First stage of the encapsulation §8 deferred until the gates and tests were trustworthy. Of 22 public fields, 8 had no user outside kolibrie/src, so they seal with no external churn and establish the pattern; the remaining 14 follow separately, led by .dictionary at 60 external files and .dataset_index at 30. - cached_stats, exact_stats_generation, planning_stats, stats_epoch and stats_rebuild_nanos are used only inside sparql_database.rs and are now fully private, with no accessor - udfs, ml_predict_materialized_triples and implicit_neural_materialization cross a module boundary and get accessors that name the operation: udf() is the lookup the optimizer needs, set_ml_materialized/take_ml_materialized are the ML runtime's record-and-undo pair, and set_implicit_neural_materialization returns the previous value so program.rs save/restores in one call
The baseline recorded noise as the gap between two back-to-back runs. Two samples cannot estimate a spread, and this produced two false positives that each cost an afternoon -- most clearly execution_hot_paths/filter_numeric, flagged at +5.7% against a recorded 0.22% when four runs in each state showed no difference. - baseline takes N runs, records the full observed range, warns below four, and stores each benchmark's run count - compare warns when the baseline has no noise data at all (a collect summary, which is how the filter_numeric case got through) and marks flagged rows [thin baseline] or [within 1.5x of the limit] - baseline.json rebuilt from four routine-tier passes Rebuilding showed 25 of 32 benchmarks had understated noise: micro_dictionary/decode 0.95% -> 38.3%, outlined_accessor 0.03% -> 11.4%, schema_plus_tuples 1.2% -> 11.8%. That corrects the architecture doc. §4 reported the per-row boxed iterator costing a significant +18.9% on the broad scan; it measures +12.5% against that benchmark's own 19.6% spread, so it is not significant. The conclusion is unchanged and better supported: boxing is a fixed per-call cost, +22.4% at 8 quads, +25.5% at 20k, and unmeasurable at 40k. Not claiming this is solved. Noise depends on
Stage 2a of the encapsulation. SparqlDatabase goes from 14 public
fields to 2; only .dictionary (312 sites) and .dataset_index remain,
and those are their own steps because that is where the storage-trait
design lives.
Fields are pub(crate) behind public accessors rather than fully
private. The goal is that no consumer depends on the layout; making
them private too would mean accessors for ~250 internal sites and buys
nothing a consumer can observe.
- quoted_triple_store returns guards (QuotedTriplesRef /
QuotedTriplesMut, both Deref to QuotedTripleStore) instead of the
Arc. P5 forbids Arc<RwLock<_>> in public signatures, and handing back
the Arc would seal the field while freezing the representation and
the locking strategy. Sites holding one lock across several
operations keep working: the guard lives as long as the binding.
- shares_quoted_triples_with replaces Arc::ptr_eq on the raw field,
stating the property the caller tested — whether two databases are
isolated.
- mutations get named operations: add_prefix, extend_prefixes,
take_neural_materialized, add_rule_mapping.
Scope was measured, not grepped. model_decls, probability_seeds and
others are also fields on CombinedQuery, Rule, CompiledProgram and
Reasoner, so field-name matching counted other types' sites. Sealing
the fields and reading the compiler's E0616 list gave 79 external
sites in 20 files; grep's per-field estimates were wrong in both
directions (planning_stats_policy 15 not 0, prefixes 11 not 22).
Leaves dataset_index as the only public field. .dictionary was the
field the storage trait actually depends on, and the one P5 names
explicitly: Arc<RwLock<_>> must not appear in public signatures.
- DictionaryRef / DictionaryMut deref to Dictionary, so
db.dictionary().decode(id) replaces
db.dictionary.read().unwrap().decode(id). The ~80 sites holding one
lock across several operations keep working: the guard lives as long
as the binding.
- shares_dictionary_with replaces Arc::ptr_eq on the raw field.
- dictionary_handle / set_dictionary_handle are the one deliberate
place Arc<RwLock<_>> still reaches the public API. Sharing an
encoding between a database and a Reasoner is a real capability --
the HTTP server and two examples do it -- and Reasoner::dictionary is
typed that way in another crate, so no guard can express it.
Confining it to two documented methods leaves one place to change
when §4 moves encoding behind TermEncoder, against 96 call sites.
- micro_field_access rewritten: its direct-field arm is no longer
expressible. It measured the three arms within 1.3% before sealing,
which is what justified proceeding; it now guards the guard accessor
and the handle path.
Completes the encapsulation §8 deferred until the gates and tests were trustworthy. SparqlDatabase goes from 22 public fields to 0. dataset_index goes behind eight named operations rather than a getter: default_triple_count, quad_count, all_quads, graphs, named_graphs, create_graph, insert_triple, clear_dataset. That is roughly the QuadSource surface §4 wants, so callers depend on that shape instead of a borrow of DatasetIndex, which would freeze the representation the storage trait exists to abstract. Most are thin on purpose: 33 of the 46 external sites only wanted a triple count. Two tests moved into a new test module in sparql_database.rs: exact_statistics_notice_a_write_that_bypassed_the_helpers and a_replacement_index_with_a_lower_generation_is_not_mistaken_for_fresh. Both deliberately write to the field behind the public API to assert that statistics notice a change via DatasetIndex::generation rather than via notification. Switching them to replace_dataset_index would have kept them compiling and silently ended that coverage, since it invalidates the cache itself. The new mutating methods deliberately do NOT invalidate the statistics cache, matching add_triple/add_quad/delete_*. Generation detection handles it -- those two tests prove so -- and invalidating would force a rebuild the check avoids and change the behaviour the rewritten call sites had. Scope: grep claimed 80 sites, the real figure is 46. Reasoner also has a public dataset_index field, so a blanket rewrite broke four Reasoner accesses in combined_rules_example; the compiler caught them and every introduced call was then audited by receiver.
Goal #2 of the maturity work: public crates get documentation so
deny(missing_docs) can be enabled. shared goes first because every
other crate builds on its vocabulary.
shared missing_docs: 495 items -> 349. Everything remaining is in
query.rs (184) and hybrid.rs (165), which are 71% of the crate and
each want their own pass.
The crate root now states the fact a reader needs first: RDF terms are
interned to u32 ids, not stored as strings; the space splits at
0x8000_0000 between ordinary terms and quoted triples; an id is
meaningless without the dictionary that produced it.
Field and variant docs state invariants rather than restating names,
which is the only thing that makes them worth reading:
- Dictionary::merge keeps existing entries, so merging two
independently built dictionaries resolves shared ids to the wrong
terms instead of reporting a conflict
- Dictionary::encode panics rather than colliding with the
quoted-triple range
- Triple's derived Ord sorts by integer id, unrelated to lexical
order, and the indexes rely on it
- DatasetIndex::query_graph returns hash order, which varies per
process -- the source of a 20% flake fixed earlier in this work
- get_matching_triples does not enforce repeated variables, and
RuleIndex::query_candidate_rules over-approximates; both need the
caller to verify
All 184 items. hybrid.rs (165) is now the only undocumented file in
shared; once it lands the crate can take #
The module doc states what the type names cannot: every term keeps its
lexical spelling -- sigils, brackets, quotes, unexpanded prefixes --
because a term only becomes a Term once bound to a dictionary, and a
rule interned against the wrong vocabulary parses, validates, then
silently matches nothing. It also records that CombinedQuery is a
union, not a choice: one request may carry standard SPARQL alongside
the rule, RSP-QL and ML extensions.
Item docs capture semantics a caller can get wrong rather than
restating names:
- Value::Undef leaves a variable unbound, so it does not constrain
the join
- window bounds are half-open: RANGE n holds n timestamps
- SyncPolicy only matters with several windows, and Fallback::Steal
emits results mixing data from different cycles
- SELECT is bag semantics unless distinct is set, and LIMIT without
ORDER BY leaves the surviving rows unspecified
- a declared save_path is not authority to write there; the host's
ML policy decides
First of five groups for kolibrie, which at 540 undocumented items is
larger and more scattered than shared was. 540 -> 470. The remainder
splits by subsystem: optimizer 147, stream processing 130, program.rs
110, parser.rs 46, ML modules 37.
The crate root gets an overview rather than a module list: how a query
flows from text through the lexical syntax tree to a costed physical
plan, what each subsystem is for, and that reasoning lives outside
this crate -- datalog and datalogmtl are siblings over the shared
vocabulary, not layers on this engine.
SparqlDatabase documents why it has no public fields, since that is
what four steps of sealing established: interning, index maintenance
and generation-based statistics caching are invariants a direct field
write would break, and encoded ids mean nothing without this store's
dictionary.
Item docs record behaviour a caller would otherwise find by
experiment:
- parse_rdf skips malformed input rather than reporting it, so a
partial parse looks like a smaller successful one
- parse_ntriples parses without adding; parse_ntriples_and_add is
the usual entry point
- union returns a third store, because the two dictionaries assigned
ids independently and other's have to be translated
- PlanningStatsPolicy::Bounded scales its tolerance with store size
rather than using a fixed count
- utils::run_server is a single-connection demo that panics on any
I/O error, not the kolibrie-http-server binary
- TripleFilter::Custom panics when cloned
All 46 items. 470 -> 424 for the crate; the optimizer (147), stream
processing (130), program.rs (110) and the ML modules (37) remain.
Two facts matter more than the per-function docs:
- Of 46 public functions only 8 are used by real consumers. The rest
are grammar fragments, public solely because tests/parser_test.rs is
an integration test and so lives outside the crate -- exercising a
combinator requires exporting it. Moving those cases to unit tests
would let the fragments become pub(crate). The module doc says so
and names the five entry points a caller actually wants.
- nom::IResult appears in every public signature, so a nom major bump
is a breaking change for anyone naming one. That is a live P5
violation, recorded where the parser crate split will meet it.
convert_combined_rule is documented as the prepare step: its result is
only meaningful against the dictionary it interned against, and a rule
lowered against the wrong one parses, validates, then matches nothing.
Restores information from four comments the mechanical pass replaced:
the BIND syntax example, the note that the PREFIX and LIMIT primitives
share the request path's parsers, and how the two ORDER BY condition
parsers relate.
Adds a documentation section to CONTRIBUTING.md. Adding a to
parser.rs re-created the trap just fixed in shared -- a module with its
own must not also carry an outer on its pub mod
declaration, or rustdoc resolves its bare links in lib.rs's scope. That
cost 7 broken links here and 8 in shared; the blocking docs job caught
both. The section also records that bracketed notation needs backticks,
and that a comment restating an item's name passes the lint while
telling a reader nothing.
All 110 items, the crate's largest file. 424 -> 314; the optimizer
(147), stream processing (130) and the ML modules (37) remain.
Workspace missing_docs 1,337 -> 1,113.
The module doc explains why compiling and executing are separate: a
CompiledProgram holds its terms as text, not dictionary ids, and
interns them only when it runs, so one compiled program can execute
against several stores.
Item docs record what a caller can get wrong:
- frozen_predictions exists for reproducibility, and a snapshot
carries the artifact hash it came from, so replaying it against a
changed model is detected rather than silently accepted
- an uncovered required choice is a gap in the answer, not a zero;
Coverage::confirmed is what to check first
- ProgramError::Resource says nothing about correctness -- the same
program may succeed with a larger budget
- fact_probability returning None means the fact is absent from the
result; Some(0.0) means it was derived and found impossible.
answer_probability collapses absence to zero only when coverage
was confirmed, and errors otherwise rather than reporting an
undecided question as a certainty that it will not happen
- ProgramError's PROGRAM_* prefixes reach callers over the HTTP API,
so those strings are contract, not diagnostics
Also makes the CONTRIBUTING rule about module docs actionable: delete
the outer /// in the same change that adds the //./target/release/frogql examples/movies.gdb The rule was
written one commit ago and broken immediately, because every module in
lib.rs carries a one-line outer doc and each group that gains a //!
trips it. That is three occurrences; the blocking docs job caught all
three.
All 132 items across rsp_engine.rs and the eight rsp/ modules.
kolibrie/src 314 -> 184; only the optimizer (147) and the ML modules
(37) remain. Workspace missing_docs 1,113 -> 849.
rsp/mod.rs gets the subsystem overview: a continuous query cannot
evaluate over an unbounded stream, so S2R windows it, R2R evaluates
over the window, and R2S decides which solutions are published.
The docs concentrate on what decides whether a consumer sees results:
- a coordinator is needed only when results must be joined across
sources -- several windows, or static patterns alongside one
- under SingleThread nothing is delivered until stop() is called, so
an engine that is never stopped appears to produce nothing
- ReportStrategy conditions combine with AND, so each one added makes
a window report less often; no strategies means report always
- Report::report is not pure: OnContentChange records what it saw
- flush ignores the report strategies and merges every active window,
so it publishes content that had not met its condition; stop closes
the channel but not a registered callback
- ProbabilisticOccurrence carries its seed id because one arrival may
land in several overlapping windows and must stay one seed
- R2ROperator needs Send (windows run on their own threads) and
AsAnyMut (downcasting for implementation state)
- WindowResult and WindowTriple hold decoded text, since a result
crossing windows cannot assume one dictionary
643 tests pass, 0 fail, unchanged. All configs clean; rustdoc builds
with -D broken_intra_doc_links on the first attempt, because each //!
was added in the same change that removed its module's outer ///.
Note for future mechanical edits: a derive-dedupe pass deleted
WindowTriple's five derives and produced 305 errors. Compiling is not
sufficient evidence -- the inverse mistake, a leftover duplicate
All 147 items across 14 files. kolibrie/src 184 -> 37, and the
remainder is entirely the two ML modules. Workspace missing_docs
849 -> 555.
The subsystem doc says why the optimizer exists: a query has many
execution strategies with very different costs, and the order it was
written in is not usually the cheapest. The logical-to-physical
mapping is one-to-many, and that is the point -- one logical Join
becomes a hash, nested-loop, star or bind join depending on
cardinalities the query text cannot express.
Item docs record the things that read as arbitrary until someone
changes them:
- COST_PER_ROW_NESTED_LOOP is deliberately 5x COST_PER_ROW_JOIN,
which is what makes hashing the default unless the inner side is
tiny
- an unmeasured graph is costed non-zero, so an empty or newly
created graph does not make a plan look free
- TableScan and IndexScan share an executor and differ only in cost
- StarJoin avoids an intermediate result per pair; BindJoin re-enters
its right side per left solution with bindings pushed into probes
- the cost model reads cached statistics that may lag the data under
the store's bounded policy
- a graph in the store but absent from a query's DatasetView is
invisible to it, and FROM replaces the default graph
- a subquery's LIMIT applies before the join
- memo keys on a canonical form of the logical subplan, and
with_cached_stats is preferred because collecting statistics scans
the store
now reports zero in all three feature configurations and takes its own
blocking lint-gates line, making it the second crate scope held at zero
after shared.
The default build showed 37 items for this step. The real figure was 89:
52 live behind #[cfg(feature = ml)] and are invisible to a
default-features check. Checking all three configurations before starting
rather than after is the one process change here — the same blind spot
has produced six separate misses earlier in this rollout, and each cost a
follow-up commit.
- ml_policy (32), execute_ml_train (32), neural_relations (10),
execute_ml (8), neural_relations_disabled (5), and the two ml-gated
pub mod declarations in lib.rs
- three missing_docs --only kolibrie/src lines in the lint-gates job,
mirroring the existing unreachable_pub configurations
ml_policy's doc presents it as the authority boundary rather than a
settings struct, because the refusals only make sense with their reasons
attached: verified_bytes refuses bytes not matching the pinned SHA-256, so
replacing a model file disables the model instead of silently changing its
behaviour; trusted_local refuses an allowlist inside the output directory,
because training could otherwise rewrite the registry deciding what is
approved; allow_pickle defaults off because unpickling executes arbitrary
code; ApprovedModel uses deny_unknown_fields so a registry typo errors
rather than leaving a model less constrained than intended. MlError's ML_*
strings are recorded as API contract, not prose.
On the neural side the documented limitation is not visible from the
signatures: these entry points bind the argmax label, and a distribution
over competing labels needs the mutually exclusive seed groups in
program. materialize_neural_relations_for_patterns is named as the
implicit path, where querying a neural predicate runs its model without
the query asking, with the note that execute_program deliberately
switches it off.
c8b04a3 documented the RSP modules and, as a side effect, rewrote kolibrie/src/rsp/builder.rs and simple_r2r.rs from CRLF to LF across 374 and 127 lines. The content was unaffected; the damage is to git blame, which now attributes every line in both files to a documentation commit. Restored per line rather than by converting the files. simple_r2r.rs was already mixed at 127 CRLF to 93 LF, so a blanket conversion would have changed lines c8b04a3 never touched. Every pre-existing line is now byte-identical to its state at c8b04a3^; the doc lines added by that commit take the ending of the item they attach to. The repository has no line-ending policy to appeal to: .gitattributes carries only LFS rules, and the tree is genuinely mixed at 15 CRLF to 221 LF .rs files. Normalising is therefore defensible, but as a deliberate repo-wide change, not as a side effect of a docs pass. 643 tests passed, 0 failed. Content is identical to HEAD ignoring line endings, verified per file.
datalogmtl/src now reports zero missing_docs and takes its own blocking lint-gates line, the fourth crate scope held at zero after shared, kolibrie/src and datalog/src. One gate line again: datalogmtl declares no Cargo features either. Only ml is left before the gate can cover the whole workspace. 85 items across 11 files: 7 module docs plus the crate doc, the 21 interval and operator fields in syntax.rs, the 16 tick counters in metrics.rs, the 6 TemporalStore trait methods and 8 further store items, and the remainder in stream, evaluator, parser and the three automata modules. The crate doc gains a table contrasting the two evaluation strategies, because choosing between them is the first thing a reader needs and it was written down nowhere. The tick evaluator answers over integer points and works on a stream; the interval-native automata engine answers over the real line, supports future operators and unbounded time through omega-materialisation, and needs the whole timeline. They do not agree in general, and neither is wrong. Four facts recorded because the code does not show them and someone has already paid to learn them: - Box steps one integer at a time across its window. With millisecond timestamps and a one-second sampling period, Box[0,5000] asks for a fact at 5001 consecutive milliseconds, finds one at every thousandth, and derives nothing - which is also what a correctly unsatisfied rule does. Express time in units of the sampling period. This is the bug that broke the drone demo. - The same stepping makes a rule's cost proportional to its window width in whatever unit the timestamps use, not to the number of facts in the window, so box_evals times the window width is the useful figure rather than box_evals alone. - Eviction drops everything older than t minus w_max, where nesting adds (Box[0,5]Diamond[0,3] reaches back 8) while separate rules take the maximum rather than the sum. That is the distinction a past max-versus-sum bug in compute_w_max got wrong. - Box and the phi side of Since make no variable safe, which is why validate rejects instead of deriving nothing at every tick: a universal operator asserts something about every point of a window rather than finding a point, so it has no witness to take a value from. Five single-line temporal variants in syntax.rs were expanded to multi-line form so their interval and inner fields can carry docs. The fields and their types are unchanged, and those five lines are the only non-doc deletions in the diff. Line endings are preserved per file this time. lib.rs and metrics.rs are CRLF and stayed CRLF; the insertion script reads bytes and matches each file's existing ending rather than going through Python's universal newlines, which is what silently normalised a file two commits ago.
ml/src now reports zero missing_docs and takes its own blocking lint-gates line. All five library crates are now at zero under their src, so the five gate lines together cover the whole library surface. 43 items across lib.rs and candle_model.rs. One configuration again: ml declares no Cargo features, and the single #[cfg(windows)] block in candle_model.rs guards statements inside a private function rather than an item, so 43 is the whole scope on every platform. Three things the docs now record because reading the code does not show them and a caller would otherwise find them the hard way: - MLHandler::predict ignores its model_name argument whenever best_model is set, which compare_models sets as a side effect and nothing clears. A handler that has compared models predicts with the winner no matter what it is asked for. - compare_models scores on cost and not accuracy: 0.5 * cpu percent plus 0.4 * memory MB plus 0.1 * prediction time, with no accuracy term. Its two dominant terms are in different units, so the weights do not transfer between machines. And a model whose schema file fails to parse is cached with Default all-zero metrics, scores zero, and therefore always wins; the failure only logs KOLIBRIE_OPERATION_FAILED to stderr. - MlpNeuralPredicate::varmap is dead. A candle VarMap is constructed and never read or written anywhere in the workspace, the parameters live in a private Vec<Vec<f64>>, and the arithmetic does not go through candle at all. Documented as unused rather than removed: dropping a public field is an API change and does not belong in a documentation commit. Also documented: that ml grants no authority, since nothing in it checks whether a model may run and the trust decisions all live in kolibrie's ml_policy; the forward/backward queue contract in candle_model, which is a runtime obligation with no type to enforce it, where forward_with_grads queues one activation set, surrogate_backward pops the oldest, gradients accumulate until zero_grads clears them, and zero_grads also discards queued forwards because that is how an aborted batch is recovered from; that optimizer_step's example_count must be the samples that actually contributed rather than the batch size, because surrogate_backward skips samples whose gradients are all zero; that to_bytes round-trips floats bit-exactly only because serde_json is built with float_roundtrip, whose absence made persistence lossy and produced a 1-in-25 flake; and why load_model validates its names, which is that the model directory's parent is prepended to sys.path. The five gate lines stay separate rather than collapsing into one workspace check, for two reasons: cli, kolibrie-http-server and python still hold six items between them, and the per-crate form names which crate regressed when one does. Build scripts stay outside the scopes deliberately - the gates cover <crate>/src, and ml/build.rs and python/build.rs are each one crate-level comment on code that is not part of any public surface.
cli, kolibrie-http-server and kolibrie-python are now at zero, which means every crate's src and every build script in the workspace is. The whole remainder of the burn-down is crate-level comments on test, example and bench targets. Three items, not the six the workspace count implied: the figure was two instances each, one for the binary and one for its test harness. Same double-counting recorded one commit ago. What the crate docs now say, none of it visible from the code: - The HTTP server binds 0.0.0.0 with no authentication of any kind. It resolves the ML context once at startup and exits if it is invalid, rather than starting and refusing requests later, and over HTTP never holds trusted-local authority, so it does not train, write artifacts or accept model declarations from a query. - The CLI's --version is hard-coded in its clap attributes rather than read from the workspace version. It matches today; nothing keeps it matching. - python/src sets doc = false for two reasons, not one: the lib must be named kolibrie for Python to import it under that name, which collides with the kolibrie crate's rustdoc output, and the documentation that matters there is the Python API rather than the Rust shim. - ml/build.rs is long because that crate embeds Python rather than being loaded by it, so the interpreter must be linked rather than deferred. macOS sysconfig can report a framework path through LDLIBRARY even when the prefix does not look like one, which is what the fallback chain is for. The gate lines went from six to one. scripts/check_lint.sh gains a --crate-src mode scoping a lint to <crate>/src/** across every crate at once, and lint-gates now runs that plus the two kolibrie feature configurations in place of seven per-crate lines.
varmap was a public field holding a candle VarMap, constructed in one place and never read or written anywhere in the workspace. The parameters live in a private Vec<Vec<f64>> and the arithmetic never went through candle, so the field was public API and nothing else. Found while documenting ml/src one commit ago: writing down what it did made clear it did nothing. Documented as unused there rather than removed, because dropping a public field is an API change and did not belong in a documentation commit. candle-nn goes with it, VarMap having been its only use. candle-core stays for the Tensor the forward pass returns. Cargo.lock loses the candle-nn entry and nothing else, so its transitive dependencies were all reachable through candle-core already. Two notes for whoever is next in this file. The struct now has no public fields at all, so the parameter representation is a private matter and can change without an API break; that is stated in its documentation. And removing the field unblocks a Debug derive that VarMap had prevented - adding one compiles, verified - which is left for the missing_debug_implementations burn-down so that axis stays one change rather than a scattering.
missing_debug_implementations is now zero under every crate's src and
has its own lint-gates lines, across the same three feature
configurations as missing_docs.
38 types, not the 76 the warning count showed: two instances each for
the lib and its test harness, the same double-counting as the last two
steps. 16 take a plain derive. The other 22 do not, for two different
reasons.
Six cannot derive at all. ResultConsumer and QueryBuilder hold dyn Fn,
CSPARQLWindow a dyn FnMut, RSPBuilder and RSPEngine trait objects, and
SddOperationBudget a &mut dyn FnMut. None of them invokes the closure it
holds: calling the deadline check would spend the budget being reported
on, and calling a result consumer would publish a solution.
Sixteen hold a dataset, and for those a derive would print the data
instead of describing the value. SimpleR2R and PySparqlDatabase embed a
whole SparqlDatabase, CompiledSdd an SddManager, MlpNeuralPredicate the
weight matrices, and the datalogmtl stores their entire fact base. Each
one prints counts and configuration under finish_non_exhaustive, so
SimpleR2R reads as
SimpleR2R { quads: 3, execution_mode: Standard, rules: 0,
derived_triples: 0, hybrid: false,
last_hybrid_error: None, .. }
Every one of those impls records why it is not a derive, so a later
reader does not replace a deliberate summary with #[derive(Debug)] and
silently turn each log line into a data dump.
No impl can block or panic. try_borrow and try_lock throughout, printing
<locked> or <in use> on contention, and RSPEngine takes no lock at all.
A Debug impl that blocks can deadlock against the thread already holding
the lock, and a value being inspected mid-run is exactly when that
happens.
Verified by running them, not by compiling them. A temporary example
printed QueryBuilder, Streamertail and SimpleR2R over a three-quad
fixture, confirming one readable line each and that the count paths
work. It also caught that the fixture was empty on the first attempt,
because parse_ntriples parses without adding and parse_ntriples_and_add
is the inserting one. Probe removed.
Two problems this step caused, both fixed here.
Qualifying the trait as std::fmt::Debug in the five files that already
carry pushed unused_qualifications from 50 to 60.
The trait is now unqualified there while Formatter and Result stay
qualified, since those are not in scope, and the count is back to 50.
The edit script converted kolibrie/src/rsp/simple_r2r.rs and
rsp/builder.rs wholesale from mixed line endings to one ending - the
same defect in the same two files as three commits ago. Nothing was
lost, verified byte-for-byte against HEAD ignoring endings, but the diff
showed ~500 lines rewritten and buried the real change. Restored per
line; both now show additions only. CONTRIBUTING gains a section on
this, since twice is a pattern: reading a file as text and writing it
back converts it, because universal newlines apply on the way in and one
ending on the way out, and simple_r2r.rs is mixed within the file so
even the dominant ending is wrong. The check is that a comment-only
change shows additions and no deletions in git diff --numstat.
unused_qualifications is now zero across the whole workspace and has its own lint-gates lines. Four lints are now held at zero by CI: unreachable_pub, missing_docs and missing_debug_implementations under every crate's src, and this one everywhere. 29 sites, not the 50 the warning count showed - two instances each for the lib and its test harness, the same double-counting as the last three steps. Every one is a redundant module prefix on a name already in scope, so each fix deletes a prefix and nothing else. No line was added or removed and the diff contains no semantic change. 18 of them are in parser.rs, where and make the qualifications redundant; the rest are std::collections::BTreeMap in sparql_database.rs and optimizer.rs, pyo3::exceptions::PyRuntimeError in py_query_builder.rs, and four singletons. Applied rustc's own suggestion spans rather than matching paths by hand. Whether a name is in scope is the compiler's judgement and not something grep can answer, and the lint reports the exact byte range to delete. The first extraction read the primary span's text against the child span's replacement and printed empty strings, which looked like a suggestion to delete the whole path; the child spans cover only the prefix. Checked all three feature configurations before starting, which found three sites the default build cannot see, two of them in the fraud_detection_system example. The gate covers the feature configurations for that reason. The gate is a plain --workspace check with no --crate-src, unlike missing_docs: this lint had nothing to say about tests, examples or benches that was not worth fixing, so there is no reason to scope it. Left alone: sparql_database.rs now wraps a BTreeMap<String, BTreeMap<String, Vec<String>>> over four lines where it would fit on one. The deferred commit normalises that along with ~1,590 other hunks, and a hand edit does not belong in an otherwise mechanical change.
Taking a copy to decode many ids raises a fair question: what happens if something writes to its copy? The answer is worse than losing the terms. A copy's next id starts wherever the shared space's stood, so a term interned into a copy can be handed a number the shared space later gives to a different term. An id minted from a copy is fiction, and a fact built from one decodes to someone else's term -- corruption, not loss. Audited, eight of the nine copies are read-only. The ninth is semi_naive_parallel, and its comment was wrong about its own severity: it said the interned ids never reach the shared space. They do. Line 191 inserts those facts into dataset_index and returns them, so the strategy fabricates ids and stores them as if real. Two rayon workers in one run also mint the same id for different terms, so its results are not even internally consistent. It has no callers, and the comment now says all of that instead of reading like a note about efficiency. Documentation had not stopped that, so the type now does. TermView is a read-only copy carrying decode and nothing else, and SharedTerms::view and Reasoner::term_view hand one out in place of an owned Dictionary, whose encode was sitting right there for anyone to reach. The five remaining hand-rolled copies -- build_cross_window_sds twice and three comparison benchmarks -- went through it too; each was doing Dictionary::new plus a clone, which is exactly the mutable copy this removes. Staleness is the other half and a view cannot prevent it: a term interned after the view is taken is missing from it, so decoding that id yields None. That is sound rather than wrong -- a blank, not another term -- and every caller here is safe for one reason, now written down where the next one will see it: the ids it decodes come from facts already stored, so they predate the view. Both halves are tests rather than claims. One shows two dictionaries from a common starting point minting the same id for different terms; one shows a view blind to a later term. The first had to be written against Dictionary directly, because after this change it cannot be written through the reasoner's API at all, which is the clearest evidence the hazard is actually gone.
Its evaluator carried a public Arc<RwLock<Dictionary>>, and its parsers took one through eleven parameters. Both speak SharedTerms now, so the handle appears in no public signature of kolibrie, shared, datalog or datalogmtl -- only in the three bridge items of shared::term_encoder, which say so in their documentation. Most of those parameters only threaded the handle through to the five places that actually took the lock, so the work was mostly types, with one thing worth keeping. Three of datalogmtl's parsers reported a poisoned lock as a recoverable error, and the HTTP server turns that into a response. with_terms unwraps, which would have converted a request-level error into a panicked handler, so SharedTerms gained try_with_terms and those three keep their contract. TermView derefs to &Dictionary. That keeps the hazard closed -- encode takes &mut self and a Deref only yields & -- while the read-only helpers that already take &Dictionary, meteor_fmt's formatting and the comparison output, need no change. The view earned its keep during this commit rather than in principle. Converting the HTTP server with a blanket substitution turned four interning sites into views, and the compiler refused all four: tokenize_and_encode_ntriples, the static rules parse, and two fact parses. Every one of them interns, and a view has no encode. Before the previous commit those would have compiled and quietly interned into a copy, which is exactly the corruption that commit was about.
SparqlDatabase held its term space as Arc<RwLock<Dictionary>>, and shared::term_encoder kept from_handle and handle alive only so that field could be converted to and from a SharedTerms. Those were the last two signatures in the workspace naming the handle, and the 67 direct uses of the field were what stopped them being deleted. The field is a SharedTerms now, and the two methods are gone. What replaces them is narrower: from_dictionary shares a space that already has terms in it, and shares_space_with answers whether two components are isolated, which is the property Arc::ptr_eq was being used to test. Cloning the encoder already expressed everything from_handle did. Most of the 58 converted call sites hold a borrow that does not fit a closure, so SharedTerms grows read and write, handing out TermsRef and TermsMut: opaque borrows that deref to Dictionary and name no lock. kolibrie's own DictionaryRef and DictionaryMut become aliases for them, leaving one guard type over the term space rather than two. with_terms_read is new for the same reason. The engine takes a read lock per chunk inside a rayon iterator; with_terms is exclusive and would have serialised those chunks, and view would have copied a 40,000-term dictionary about twenty times per query. Arc<RwLock<Dictionary>> now appears in no signature of shared, kolibrie, datalog or datalogmtl -- only as the newtype's private field. Verified: 689 tests pass, 0 failures. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings 106, below the 108 baseline. Performance-neutral, and the first measurement said otherwise. It reported 18-28% regressions on four execution_hot_paths arms; a second run of the identical tree moved those same arms by -11% to -20%, so the first was a cold run. Settled figures are -2.1% to +4.5%, against +0.8% to +3.4% on the five arms micro_dictionary's module doc marks as usable anchors.
rsp_ql_multi_window_integration failed about one run in 24 under eight test threads, and had done so for a while. The two-second sleep after engine.stop() was not slack in a passing test; it was standing in for a guarantee stop() never made, and the defect underneath it was not a matter of waiting longer. stop() flushed every window and returned. Flushing only queues content for that window's thread, which forwards a result to the coordinator, which joins and calls the consumer -- two handoffs nobody awaited. Both threads were spawned with their handles dropped, so there was nothing to join. They are kept now and joined in order, with the engine's own result sender dropped in between so the coordinator sees the channel close. A channel yields what is queued before reporting disconnection, so neither join can lose a result. That alone did not fix it, and the trace said why. With SyncPolicy::Wait the cycle completes as soon as every window has fired once, using whatever each window's latest result is at that instant. On a loaded machine the coordinator is often not scheduled until stop() has already flushed everything, so it drains a queue in which one window's first result -- frequently empty, being the half-open window's first report -- completes the cycle and emits nothing, and that window's real results arrive afterwards with no second firing left to pair them against. The engine was holding a complete, non-empty set for every window and threw it away on exit. It publishes it now, guarded on a window having fired since the last emission so an idle shutdown does not republish. The six sleeps are gone. The file asserts immediately after stop and runs in 0.02s instead of 12s, which makes every assertion in it stronger than it was. Verified: 1100 runs of the file with no failure -- 700 at eight test threads, 400 at sixteen with ten spinning background processes -- against one-in-24 before. Full workspace green.
The Dictionary keys on a term's text, so the spelling is the identity: two terms sharing a spelling share an id and can never be told apart again. The N-Triples reader decoded a literal and returned its lexical form alone, so "25"^^xsd:integer, "25"^^xsd:string and plain "25" all interned as 25, with the datatype gone. That is data loss rather than imprecision -- the graph could no longer be written back out -- and a language tag fared no better, glued on as hello@en, which is also how a plain literal of those eight characters was stored. shared::rdf_term now decides the spelling. Bare where it can be, so an IRI and a plain literal keep exactly the text they had and the overwhelming majority of terms, and the results built from them, are byte-identical. Quoted where it must be, so a typed literal is "25"^^<...#integer> and a tagged one is "hello"@en; a well-formed IRI cannot begin with a quote, so nothing quoted can collide with one. An xsd:string literal is spelled as the plain literal it is. The datatype was being dropped twice, in two functions. encode_term_star re-parsed the reader's output and stripped it back off, so fixing the reader alone changed nothing. Both call one canonicaliser now, which is idempotent so that it can sit on a path that may already have run. Counting the rest, one term had five spellings. Turtle produced 30"^^xsd:integer -- a stray quote, and a prefix never expanded, because the branch that could expand it split the term into the prefix "30"^^xsd and gave up. resolve_term emitted "25"^^http://... without angle brackets. tag_store and hybrid already wrote the correct one, so a probability written as "0.8"^^<...#double> could never match a 0.8 read from a file. Four of the five agree now. RDF/XML is the fifth and is untouched: it reads neither rdf:datatype nor xml:lang, so it has no datatype to keep, and giving it one is its own change. Keeping the datatype is only half of it, because a literal's number lives in its lexical form. Seven places parsed the spelling instead, so SUM over typed data returned nothing and ORDER BY put 7 after 21, both silently. They read the value now: aggregate, term_order, the four arithmetic resolvers in filter evaluation, and the engine's ML feature path. Comparison already read through the literal, which is why it and arithmetic disagreed about whether a typed literal was a number. Verified: 710 tests pass. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings back to the 106 baseline after deleting decode_ntriples_literal, whose escapes -- \uXXXX, \b, \f and \' -- were ported first and are pinned by tests, since deleting it before porting would have turned a valid literal into a different one with no error. Worth stating plainly: the suite passed with the datatype preserved and before any of the value-reading fixes. Typed literals appear 12 times in the whole corpus and never in an aggregate, an ORDER BY or a FILTER, so passing meant the path was untested. The two tests written next both failed immediately.
The RDF/XML reader looked at neither rdf:datatype nor xml:lang, so every literal it produced was a plain one whatever the document said. It was the fifth and last of the five spellings one term had across this codebase, and the only reader still losing a datatype outright. A literal's text arrives as its own event, after the property element that declares what the text means, so the datatype is remembered when the element opens and spent when the text arrives. It does not carry to the next property element, which has a test of its own because element-scoped state is the obvious way to get this wrong. xml:lang needed more than that. It applies to an element and everything inside it, and documents routinely declare the language once on the root, so the tag in force is a stack: each start element pushes its own tag or the one it inherits, each end element pops. A field would have handled only a tag written directly on the property element, which is the case that matters least. Popping an empty stack is a no-op, so malformed nesting cannot panic, and an empty element never pushes so it must not pop. Three tests cover the scope, because one would have passed on two broken implementations. A tag not outliving its element already passed before the stack existed -- resetting on every element satisfies it trivially -- so it says nothing without the other two: a tag inherited from an enclosing element, and an inner tag overriding an outer one with the outer resuming afterwards. Verified: 720 tests pass. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings at the 106 baseline. Still outside this: rdf:parseType, property attributes on rdf:Description, and typed node elements are RDF/XML features this reader does not implement, and none of them is about what a term means.
A blank node label identifies a node only within its own document (RDF 1.1 section 3.4), so two files each saying _:b describe two different nodes. The readers interned the label itself, so the two became one node and merging the files silently fused unrelated statements. It is the one defect in this family that invents a fact rather than losing one. Relabelling is forced rather than chosen: the dictionary keys on a term's text, so two distinct nodes must have two distinct spellings. One scope is taken per read, before the parallel chunks fan out so every chunk relabels alike, and the form follows what SPARQL updates already minted for the same reason. All four readers that can carry a label are scoped -- N-Triples, Turtle, N-Quads including its graph name, and N3 -- and the rewrite recurses through quoted triples, since scoping only the outermost term would leave a nested label bare and two documents would still share the node a quoted triple asserts about. RDF/XML is untouched: it never reads rdf:nodeID, so it produces no blank nodes at all. A scope replaces a previous one rather than nesting inside it. Reading a graph back from its own serialisation is a new document and scopes it again, so a label would otherwise collect a prefix per round trip and grow without bound. The node is still a new node each time; only the spelling stays a sane length. That changed one assertion in nquads_round_trip_keeps_default_and_named _quads, which pinned the _:kolibrie-update- prefix after a serialise and re-read. It asserted a label's lexical form across a boundary RDF makes insignificant. Its intent, readable from the sibling assertion that the serialised form contains no <_:, is that the node survives as a node rather than becoming an IRI -- that is what it checks now, together with the written label still being legible. The scoping tests also surfaced a silent data loss in parse_n3 with nothing to do with blank nodes. Each chunk parsed into its own database and its triples were inserted carrying that database's ids, while only the dictionaries were merged, so the ids meant nothing in the target. An empty target numbered terms in the same order, which is why a first read looked correct; a second read reinterpreted its ids as the first document's terms and stored a copy of the first document in place of the second. Two reads of two unrelated IRI-only documents left one triple. Ids are translated through reencode_term_id now, which also carries the chunk's quoted triples across, as the merge never did. Verified: 735 tests pass. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings at the 106 baseline.
The previous commit scoped blank node labels per document, and a stream is many documents. Each arriving element is its own document in RDF's terms, so scoping per element is the correct reading of the spec -- but the dictionary never forgets a term, so a fresh scope per element minted a term per element and grew it without bound. Measured: 200 elements naming one blank node interned 204 terms, against 5 for the same stream without one. Dictionary::encode asserts rather than wraps when ids reach the quoted-triple range, so a long-running stream would not merely leak, it would eventually panic. Before scoping existed the same stream interned `_:b` once and reused it forever, so this was a regression and not a pre-existing limit. A stream therefore holds one scope for its whole life, taken when its R2R operator is built. Bounded within a stream, still separate between streams and from every file, and the same sharing the engine already had -- so stream semantics do not change, only what the labels are spelled as. parse_ntriples_in_scope and its encoding counterpart are how a caller whose document boundary is not one call says so. Both halves are pinned, the bounded one as a regression guard rather than a feature: 200 elements naming one blank node intern no more terms than the same stream without one, and two streams reading the same label get two nodes. The two other repeated-parse paths keep a fresh scope per call, which is right for both: add_static_ntriples loads a background document, and the HTTP server builds its database per request, so neither accumulates. Verified: 737 tests pass, and 300 runs of rsp_engine_test at eight test threads with no failure, since this touches the streaming path. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings at the 106 baseline.
Five commits changed what a term is spelled as and nothing demonstrated it. The rdf_terms example is the guided tour: what an IRI, a blank node and each kind of literal look like, which spellings RDF says denote one term and which two, how to read a value back out of a term rather than out of its spelling, and what a blank node scope does across two documents. It runs and prints, so each claim is visible rather than asserted: that `"25"` and `"25"^^xsd:string` are one term and `"25"^^xsd:integer` is another, that `"1"` and `"01"` are two terms denoting one value, that `_:b` read in two documents is two nodes and twice in one document is one. It ends by putting the same terms through a store and summing two xsd:integer ages to 42 -- the property that broke when the datatype was first kept and the engine was still parsing spellings. Writing it made the case for one addition. Building a literal had spell_literal, while building a blank node meant knowing both halves of a convention: that the term carries a `_:` prefix, and that a bare label collides with every other document's until it is scoped. spell_blank_node does both, from the label as written. There is deliberately no equivalent for an IRI, and the example says why: an IRI is spelled as itself, so a constructor would be the identity function and would only suggest that something is being done to it. It uses `shared::rdf_term` directly, as the other 54 examples use `kolibrie::` and `shared::` directly. Porting the examples to the kolibrie-api facade is still open, and doing it for one example would only split the convention. Verified: 738 tests pass, and the example smoke suite runs 52 of 55 examples with no failure, this one among them -- `scripts/ smoke_examples.py` reads from cargo metadata, so it needed no registration. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings at the 106 baseline.
A canonical spelling is deliberately not a wire form. An IRI is stored bare and a plain literal unquoted, which is what keeps identity cheap and left every existing term byte-identical when the datatype started being kept. Handing that text to a serialiser was wrong in four ways, and the worst of them is mine. A typed literal was quoted a second time, so the output read "\"30\"^^<...#integer>" -- a plain string literal whose lexical form is the forty-eight characters of a typed one. Valid N-Triples, and read as a string by every other RDF tool. It went unnoticed because these readers re-canonicalise what they read, so a round trip through Kolibrie recovered the datatype; only a second implementation would have seen it, which is exactly the case that matters for exchanging data between systems. Three faults in generate_ntriples predate that and the same writer closes them: a blank node subject became <_:b>, which is an IRI and not a blank node; an IRI whose scheme was not http or https became "urn:a", which is a literal; and a literal was never escaped, so an embedded quote produced output that does not parse. generate_nquads had the first two right, generate_turtle had all three wrong. to_ntriples_term is the one writer, and every serialiser goes through it. escape_ntriples_literal and looks_like_absolute_iri are deleted, checked before removing rather than after: escape_lexical is a strict superset of the first, adding \b and \f, and the second is the same scheme test under another name. The tests assert the emitted text, not what re-reading gives back. Asserting on a re-read is precisely what let this hide, because the reader repaired the writer's output on the way in. Verified: 747 tests pass. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings at the 106 baseline.
A blank node label means nothing outside the document it came from, and the scope counter is process-local, so two instances both mint _:kolibrie-doc-1-b for their first document. Merging their output makes one node of two: the collision document scoping removed, reappearing at the distribution boundary, where it matters most because the systems cannot see each other's counters. RDF 1.1 section 3.5 answers it by replacing blank nodes with IRIs, which are global. rdf_term::skolemise mints one under the /.well-known/genid/ path RFC 8615 reserves for exactly this, so it cannot collide with an IRI an application chose and a consumer that recognises the path knows what the IRI stands for. deskolemise recovers the node for a consumer that would rather have it, recognising the IRI by its path rather than its authority so it works on any publisher's. The base authority is required rather than defaulted. A Skolem IRI is unique because the authority minting it is, and this crate has no identity of its own to offer; defaulting it would mint the same IRI on two machines and put the collision straight back. generate_ntriples_skolemised and generate_nquads_skolemised are new rather than replacing the plain ones, because skolemisation is a choice made at an exchange boundary and not a change to how terms are stored: a blank node is still a blank node inside one store, and both forms are valid RDF. The plain serialisers now say that their labels are document-local and point at the skolemised form. It skolemises per term. The first attempt rewrote the serialised text, which is shorter and wrong: a literal reading "see _:b here" contains a label that is not a term, and the rewrite would have replaced it. A test holds exactly that literal. Verified: 759 tests pass. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings at the 106 baseline.
Term ids are only meaningful against the dictionary that minted them, so anything crossing out of a store has to carry text instead. Two places had worked that out independently: the parsers' return type, which said Vec<Vec<(String, String, String)>> -- three strings in a particular order, with nothing saying which order or that they are terms rather than a pattern -- and the windows' item type, which was its own three-field struct for the same stated reason. TermTriple is both of them. It carries Hash and Ord so it can serve as a window's item, converts to and from the tuple it replaces so the migration is not all-or-nothing, and writes a line of N-Triples through the term writer, so an IRI gets its brackets and a literal keeps its datatype. WindowTriple stays as an alias, because a window's item type reads better for the name. I claimed five representations of one thing when I proposed this. Two is the honest count, and the correction matters because consolidating the other three would have been wrong. WindowResult's results are solution mappings -- a row, not a triple. CompiledProgram holds rules and declarations. WindowedTriple's predicate is documented as a local name under its window IRI rather than a full IRI, and it carries a timestamp, so it is an SDS fact and not a triple of terms. Of the thirty-one places naming (String, String, String), most are triple patterns holding variables, filter comparisons of the shape (lhs, op, rhs), or diagnostics; folding those in would have conflated a variable with a term named ?s. The fifteen field renames from s/p/o were done through the fully qualified text, not a regex on `.s`, which would have matched a great deal more than these. Verified: 761 tests pass. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings at the 106 baseline.
WindowResult's solutions were Vec<HashMap<String, String>>, the last place in the workspace carrying terms in an unnamed shape. It is also exactly the shape Bindings replaced on the encoded side after measuring a 4.7x difference on the join, and the costs are worse here because the keys are strings rather than u32: a variable was hashed once per row per column instead of being resolved to a column once for the sequence. TextBindings is the unencoded sibling. A cell is Option<String> because a solution mapping is partial and neither of the other tricks is available: a map says so by leaving a key out, Bindings says so with a sentinel id, and the empty string is a perfectly good literal. The coordinator's join path moves over with it -- both last_materialized maps, join_window_results, emit_results and the cross-window materialiser. join_window_results no longer clones every row of every window before joining any of them, which it did only to build an owned Vec it could drain. natural_join is gone; TextBindings::join carries the semantics and resolves the shared columns once from the two schemas. It still compares rather than hashes, and that is deliberate. An unbound cell is compatible with any value, because two solution mappings are compatible where both are defined (SPARQL 1.1 section 18.5), and a hash join on the shared columns cannot express that -- it would silently drop or invent solutions. A test holds the case: one row leaves the shared variable unbound, so it joins every row of the other side, and the merged row takes the bound side's value. A hash join with those rows handled separately is a later change, and one to measure rather than assume. The single-thread path accumulates each window's results while the multi-thread coordinator replaces them. That asymmetry is pre-existing and not this change's to settle, so extend concatenates rather than joins, widening the schema and leaving rows that predate a column unbound -- including realigning cells by their variable's name rather than its position, which has its own test. Verified: 773 tests pass, and 300 runs of rsp_engine_test at eight test threads with no failure, since this rewrote the coordinator's join. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings at the 106 baseline.
The term work left the Python bindings behind. Their signatures were unaffected -- the bindings call parse_ntriples_and_add, which did not change -- but their behaviour was: a query now hands back a typed literal as its canonical spelling, and Python could read a value out of it only by parsing that string by hand, which is exactly what the Rust side stopped doing. Eleven functions mirror shared::rdf_term: term_kind, lexical_value, numeric_value, literal_datatype, literal_language, make_literal, new_document_scope, make_blank_node, to_ntriples_term, skolemise and deskolemise, with the XSD_STRING and SKOLEM_PATH constants. There is no make_iri, for the same reason there is none in Rust: an IRI is spelled as itself. CI built the cdylib and stopped there, so nothing in this repository's automation had ever executed a line of Python -- the same gap smoke_examples.py closes on the Rust side, where it found three examples that compiled and did not run. scripts/smoke_python.py builds the module, stages it as kolibrie.so beside the examples so their relative paths resolve, and runs each one. It replaces the bare build as the CI step. All five pass, the four that predate this included. The new example asserts rather than prints: a typed literal keeps its datatype, a plain literal stays bare, a language tag is not glued to the text, an IRI is an IRI whatever its scheme, a prefixed name is expanded, and the engine's SUM agrees with Python's own arithmetic over the same terms -- the property that breaks silently if either side reads a spelling instead of a value. Running it also found something compiling never would. clean_turtle_term stripped an IRI's angle brackets, and those brackets are the only thing distinguishing an absolute IRI from a prefixed name once the text is bare. A bare urn:a reaching resolve_query_term was read as the prefix urn, found undeclared, and reported as KOLIBRIE_OPERATION_FAILED: every non-http IRI in a Turtle document logged one for its subject and one for its predicate, two per triple, while parsing correctly regardless. The brackets are kept now and resolve_query_term strips them itself. Verified: 773 tests pass, and all five Python examples run. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings at the 106 baseline.
Python could read N-Triples from a file and nothing else: text held in memory had to be written to disk first, and there was no way to get a graph back out at all. load_ntriples is the counterpart to parse_turtle, and to_ntriples, to_nquads and their skolemised forms are the way out. The skolemised forms take the base authority with no default, for the reason the Rust ones do: a Skolem IRI is globally unique because its authority is, and defaulting it would mint the same IRI on two machines and put back the collision it exists to remove. All five raise on a poisoned lock rather than silently doing nothing, matching exec_query rather than parse_turtle. The example now covers the exchange itself -- two stores publishing the same document under different authorities, merged into a third, and a round trip that keeps every datatype and language tag. One of its assertions had to be weakened to be honest. Merging alice's and bob's output gives two nodes in a single process whether or not anything was skolemised, because the two stores drew consecutive scopes from one process-local counter; the assertion looked like it proved skolemisation and proved only that. What carries the weight is the separate one beside it: a single label under two authorities gives two IRIs, which is what two machines would actually rest on. Verified: 773 tests pass, and all five Python examples run. Clean on --all-targets and under the ml feature. Warnings at the 106 baseline.
Eighteen locks, handled three ways: two different messages for the same condition, and in ten places no handling at all. A swallowed lock did nothing and said nothing. `count` answered zero, the four accessors answered an empty list, and `add_stream_triple` returned success having stored nothing. None of those is distinguishable by a caller from a query that simply matched nothing -- which is the ambiguity `exec_query` was changed to avoid -- and reporting success for a write that did not happen is worse than either. One helper now takes every lock and turns a poisoned one into an exception with one message. Seven signatures become PyResult so the failure has somewhere to go: add_triple, parse_turtle, get_decoded_triples, get_subjects, get_predicates, get_objects and count, along with the three group_by methods that call the first of them. Nothing changes for a Python caller. pyo3 unwraps Ok transparently, so count() still answers 2 and add_triple() still answers None; the only visible difference is that a poisoned lock raises instead of lying. Verified: 773 tests pass, and all five Python examples run, with the accessors checked by hand to confirm they still answer plain values rather than wrappers. Clean on --all-targets and under the ml feature. Warnings at the 106 baseline.
Five examples, each asserted rather than printed, so they fail loudly
instead of showing something wrong: 01_terms builds and reads terms,
02_parsing covers the four readers and two writers, 03_sparql covers
SELECT, joins, FILTER, aggregates, ORDER BY, the query builder and
updates, 04_streaming covers windows and joins across streams, and
05_datalog covers one rule, rules feeding each other to a fixpoint, and
backward chaining.
Three old ones go. contradictions.py demonstrated
semi_naive_with_repairs, which is not what this should show;
kolibrie_usage.py never touched the query API; test_windowing.py
exercised a shim. test_kg.py is superseded by 05_datalog.
The streaming one needed an engine that did not exist. What Python had
was not the RSP engine: QueryBuilder::as_stream builds a StreamingQuery
that accumulates triples in a list and applies limit, offset and
distinct -- no windows, no RSP-QL, no joining across streams. A filter
with a stream-shaped name, which is why that example was outdated
rather than merely dated. RspEngine takes an RSP-QL query, opens a
stream per window and joins across them as the windows fire. Its
results collect into a buffer rather than calling a Python function,
because the Rust consumer runs on the engine's own threads and calling
into Python from one would mean taking the GIL per solution. Results
are complete when stop() returns, so the example contains no sleep.
Writing the parsing example found a bug compiling would not.
parse_rdf_from_file is an independent copy of parse_rdf -- its own event
loop, 157 lines -- so teaching one of the two about rdf:datatype and
xml:lang left the other discarding both, and the same document read
from a file and from a string produced different terms. They remain two
readers: a benchmark reads a file through this one, and turning a
streaming reader into one that slurps is a change to measure rather
than to slip into a correctness fix.
Writing the datalog example found two gaps. backward_chaining answers
in the graph's own terms and there was no inverse of encode_term, so its
bindings read Term.Constant(0); and the Term variants carried pyo3's
unnamed _0 rather than id and name. The example prints
{'who': 'alice', 'whose': 'carol'} now.
Verified: 773 tests pass, all five Python examples run, and all 52
runnable Rust examples still do. Clean on --all-targets, under the ml
feature, and for the examples and benches. Warnings at the 106
baseline.
A rule had to be built: `Rule(premise=[TriplePattern(Term.Variable("X"),
Term.Constant(graph.encode_term("...")), ...)])`, with every constant
named by the integer id the graph gave it. That is reasonable for a rule
a program generates and tedious for one a person wrote.
Both syntaxes parse from a string now. KnowledgeGraph::add_n3_rules
takes an N3 document -- `{ premise } => { conclusion }` under @Prefix
declarations -- and SparqlDatabase::add_rule takes a Kolibrie RULE
block, runs it, and answers the triples it derived. Both intern against
the store they are added to, so a parsed rule matches the facts already
there.
Doing that found parse_n3_document rejecting the conventional spelling
of an N3 document. Every N3 statement ends with a dot,
parse_unresolved_rule stops at the conclusion's closing brace, and the
document parser requires the whole input to be consumed -- so a trailing
dot failed with Eof. No test caught it because the single-rule parser
leaves leftover input behind and therefore looked correct.
The repairs API is gone from the bindings: query_with_repairs,
infer_new_facts_semi_naive_with_repairs and add_constraint. The Rust
side keeps them, since nothing asked for that and a materialisation
strategy is not a binding's to retire.
05_datalog is rewritten around the text syntaxes: one N3 rule, two
sharing a prefix block with the second consuming what the first
derives, a CONSTRUCT rule run and then queried, backward chaining from
a built goal, and last the assembled form, kept because a generated
rule is easier to build than to render as text.
Verified: 773 tests pass and all five Python examples run. Clean on
--all-targets and under the ml feature. Warnings at the 106 baseline.
Two modules, reasoning::repairs and materialisation::semi_naive_with_ repairs, 145 lines between them, plus Reasoner's constraints field, add_constraint, and the two private helpers only those used -- violates_constraints and compute_repairs. Nothing else in the workspace reached any of it, and no test covered a line of it. Inconsistency-tolerant reasoning was carried for several releases without a single assertion about what it answers, which is its own argument for not keeping it. What documented it goes too: the contradictions example and its registration, the inconsistency function in the knowledge_graph example, the bullet in datalog's crate docs -- an intra-doc link that would otherwise have broken -- and the three method sections in README.md and the Dutch, Spanish and German translations. The materialisation module's own documentation needed correcting twice over. Its table listed the repairing strategy, and its prose said the strategies differ in cost and in what they record "and the repair variant additionally alters the fact base before it starts, so it is the one that can disagree about the result". With it gone every remaining strategy agrees about which facts follow, which is worth saying positively, and "the first four live in pub(crate) modules" is now three. Verified: 773 tests pass, rustdoc is clean across the workspace, all five Python examples run and all 51 runnable Rust examples do. Clean on --all-targets, under the ml feature, and for the examples and benches. Warnings down to 105 from 106, the example that went taking one with it.
Sixteen of the README's twenty-three Rust examples did not compile. The reasons read as a catalogue of how a README rots: `execute_query` after it became `execute`, indexing a `Solutions` as though it were a `Vec`, `Reasoner.dictionary` and `SparqlDatabase.dictionary` after both became private in this same body of work, a `Rule` built without its `negative_premise`, and `datalog::knowledge_graph` under a name it no longer has. scripts/readme_examples.py extracts every block and compiles it. Compile-only on purpose: whether an example *runs* is what smoke_examples.py answers about kolibrie/examples, and what rots here is whether it still says something the compiler accepts. Most blocks are fragments shown mid-narrative -- `db.parse_turtle(..)` with the `db` from two paragraphs up -- which is how a README should read, so a section can declare setup in an HTML comment and every block after it compiles with that prepended, the way rustdoc hides setup behind `#` lines. The hand-written API documentation goes: 371 lines restating signatures, which had drifted exactly as such a list does. Three removed methods were documented after the fact, here and in four translations, and nothing could have noticed. In its place, two generated references -- `cargo doc`, which is the authority, and docs/api.md, the index: 1,132 public items across six crates, from scripts/api_surface.py. That script reads the source lexically rather than rustdoc JSON or cargo public-api, both of which want nightly, which this project does not take. The cost is stated in the file: no type resolution, no macro expansion, a re-export listed where it is written, and a `pub` item in a private module listed although nothing can reach it. Both are in verify.sh and in CI, which is the point. A generated listing nobody regenerates is a listing nobody can trust. The README also now mentions the public API it never did: typed terms, blank node scoping, skolemisation, the kolibrie-api facade, and what the Python bindings cover. The Volcano optimizer section stays, contrary to the request that prompted this. It reads as internal and it is not: streamertail_optimizer is `pub mod`, Streamertail is publicly re-exported, and two examples construct one the way a consumer would. The condition was to remove it if it were not public API, and it is. Verified: verify.sh passes end to end -- 773 tests, 51 runnable examples, 25 README blocks, rustdoc clean, api.md up to date. Warnings at 105.
Five of them -- Dutch, German, Spanish, French and Japanese -- each carrying its own copy of the hand-written API section that was just deleted from the English README, documenting the same three methods that no longer exist, plus its own copies of examples that stopped compiling. Nothing checked any of it, and the drift was invisible in a way the English README's was not: no reader of this file would ever see it. Repairing them means five more copies of every example for scripts/readme_examples.py to compile, and five more surfaces to update for every API change, in languages no maintainer here reads well enough to keep honest. The language bar at the top goes with them, and the sentence about the drift now says five translations and why they are gone. Every local markdown link in the repository resolves. Verified: the README's 25 examples still compile, docs/api.md is still up to date, and no reference to any removed file remains.
Eleven files in the crate root distinguished by an `ml_` or `neural_` prefix -- a prefix doing by hand what a module does by language. Seven move into `kolibrie/src/ml/` and lose it: `ml_runtime` is `ml::runtime`, `execute_ml` is `ml::execute`. The real gain is the feature gating. Eight `#[cfg(feature = "ml")]` attributes were scattered through lib.rs's module list, and it took reading all eight to see which half of the ML layer was conditional. They are in one table now. ml-gated code is invisible to a default `cargo check`, which has twice let a breakage reach a commit in this work. Three files stay in the crate root: ml_policy, ml_feature_loader and neural_registry. They are the ML *contract* rather than its implementation -- the engine depends on them and they reach back no further than the store, while everything inside `ml` depends on the engine. lib.rs already said so in prose, that the crate root is the one place that may name both the abstraction and the implementation; the layout says it now too. That line was found rather than guessed, and the first attempt got it wrong. Moving all ten broke the acyclicity gate: `scripts/module_graph.py` counts a directory as one node -- by design, and consistently with `rsp` and `streamertail_optimizer` -- so a leaf the engine depends on and a consumer that depends on the engine became the same node, and seven modules collapsed into one strongly-connected component. Measuring each submodule's direction, with doc links excluded as the script excludes them, showed exactly three with no edge into the engine. With those three outside, the graph is acyclic at 23 modules and 66 edges, down from 30 and 94: the grouping made it smaller rather than only rearranging it. One hazard, documented in the module: kolibrie depends on a crate called `ml`, and five files inside `kolibrie::ml` write `use ml::...` for it. It resolves today, but it is one name for two things in one scope. Also worth knowing, found on the way and left alone: under `--features ml` there are five `unnecessary qualification` warnings and three broken intra-doc links, all pre-existing. Neither `warning_report.py` nor the rustdoc gate covers that feature configuration. Verified: verify.sh passes end to end -- 773 tests, 51 runnable examples, 25 README blocks, rustdoc clean, api.md regenerated, acyclic. All three feature configurations build. Five Python examples run. Warnings at 105.
I proposed consolidating rsp and rsp_engine and the measurement refuses it. rsp_engine names rule_execution::process_rule_definition, and rule_execution names crate::rsp::r2s and crate::rsp::s2r, so merging the two makes rsp -> rule_execution -> rsp: a two-module cycle the acyclicity gate would reject. The split is not an accident. `rsp` is the RSP-QL operator library -- S2R, R2R, R2S -- and `rsp_engine` is the engine that orchestrates them and reaches into rule execution. That is a layer boundary, and the layout already expresses it. Measured before moving anything this time, which is the lesson of the ml module. What the area did have was a dead file. kolibrie/src/rsp/lib.rs was never compiled: only the crate root's lib.rs is special, so a lib.rs inside a module directory is an ordinary file nothing includes. rsp/mod.rs is the real root and declares all five submodules; the dead copy declared four, one of them `pub mod stream;` for a stream.rs that does not exist. It would not have compiled had anything included it, which is the proof that nothing did. And rsp_engine.rs becomes rsp_engine/mod.rs, so all four directory modules -- ml, rsp, rsp_engine, streamertail_optimizer -- use one convention. No path, API or graph change; it removes the inconsistency that made the RSP layer look split three ways when it is two layers and a stray file. Verified: verify.sh passes end to end -- 773 tests, 51 runnable examples, 25 README blocks, rustdoc clean, acyclic at 23 modules and 66 edges. All three feature configurations build. 200 runs of rsp_engine_test at eight test threads with no failure, since this moved the engine's file. Warnings at 105.
`verify.sh` is meant to run on every change, and did not: a one-line edit to `sparql_database.rs` cost 436 seconds to rebuild and test. Most of that was never the tests. The profile was building release-grade binaries for every `cargo test`, and what remained was a macOS cost that no amount of work in this repository can remove. - Retune `[profile.dev]` for the edit-build-test cycle. It was `opt-level = 2`, `lto = "thin"`, `incremental = false`; it is now `opt-level = 1` with incremental on and no LTO. `debug-assertions` and `overflow-checks` stay on, being the reason a dev build is worth running. - Stop two examples sleeping for no reason. `automate_policy` slept 21 seconds in a loop whose window has no `TICK` clause, so its firing is driven by the counter it timestamps with, not by wall time; `continious_stream` builds no `RSPEngine` and calls `add_to_stream` nowhere, so its pacing was narrative. - Make the README check incremental. It deleted and recreated its generated crate every run, so every run compiled 25 blocks cold. - Add per-phase timing to `verify.sh`, and per-example timing, a median and floor, and `--dump-timings` to `smoke_examples.py`. None of the above was findable without them: a first attempt timed each cargo command standing alone, summed to 42 seconds, and missed most of the run. - Derive the Python library path in `scripts/python_env.sh` instead of hardcoding it in three scripts. They each named an exact Homebrew patch version and `$HOME/miniconda3`, which breaks on `brew upgrade`, on any machine laid out differently, and on Linux, where the loader does not read `DYLD_LIBRARY_PATH` at all. - Take `--all-targets` off the CI test step, keeping it on the build step, and run the examples there rather than only compiling them. Evidence. The edit-and-test cycle is 149 seconds, from 436, with 773 tests still passing. The examples lost 45 seconds of sleeping for 12, with `automate_policy` byte-identical at 39 lines and 39 window firings. The README check went 27 seconds to 4. The remaining floor is macOS Gatekeeper, not this repository. Of a 185-second example run only ~16 seconds is the examples doing anything, 13 of that in three of them, and the rest is a flat ~3.3s per binary charged on an example's first execution after it is linked -- the fastest example in the suite costs 2.2s the first time and 0.0s the second. Over ten freshly linked binaries: 36 seconds wall, `syspolicyd` 30.1s of CPU and `XprotectService` 9.5s, while the examples reported 0.00 user and 0.00 sys. Blocked, not computing. The assessment is keyed on content, so a byte-identical copy of an assessed binary costs 0.18s and every rebuild pays again. The fix is a machine setting, exempting the terminal that runs cargo under Privacy & Security > Developer Tools, so it is documented in `smoke_examples.py` rather than worked around. Three approaches were measured and rejected, each recorded beside the setting it concerns so none is retried on intuition: - `opt-level = 0` is slower both ways: 162 seconds for the same edit against 149, and 328 cold against 295. The tests run slowly enough unoptimised to outweigh what the codegen saves. - `debug = 1` shrinks the example binaries by a third, 4.5MB mean to 3.0MB, and saves nothing: 160 seconds for the edit against 149, 282 cold against 295, and the example run 182 against 124. Two of three moved the wrong way, because the assessment is not proportional to debug sections dyld never loads. Size was the wrong lever. - Running the examples concurrently saves 8%, 170 seconds against 185, because the daemon serialises: every example's own duration rose to ~53s as all 51 queued behind it. All 51 still passed, but contention took `continious_stream` from 7.4s to 42 and `replay_profile` from 6.4 to 56, and their RSP windows are wall-clock. Passing once at six times the duration is not evidence they stay correct there. On Linux the floor does not exist, which is what makes this suite fit for a GitHub Action. The example run takes 18 seconds there against 185, and 18 again on a second pass where macOS needs 185 then 16. A control confirms the mechanism rather than the correlation: 20 freshly compiled 5.4MB binaries with full debug info cost 8 milliseconds in total to launch on Linux, against 3,300 milliseconds each on macOS. That CI work is two changes, both measured in a 4-CPU container matching `ubuntu-latest`. `--all-targets` in a test run pulls in the 13 `harness = false` bench targets, which criterion runs in test mode, and `citybench_cross_window_compare` does not survive it: SIGKILLed in a 7.7GB container, and 12.3GB resident after ten minutes on a 48GB host while still growing, against a runner's 16GB. The build step already compile-checks every target, so dropping it loses no coverage and gains doc tests, which `--all-targets` excludes -- six doc-test targets had never run in CI. The examples now run, reusing that build, gated to the default leg because the script skips every example carrying `required-features`. A failing example's captured output is printed, because the script reports a path that dies with the runner; tested both ways, since a step guarded by `failure()` otherwise ships untested. The hardcoded library path was also wrong in a way that worked. It named Homebrew's python@3.11 first while the binaries here link against miniconda's 3.11 -- `otool -l` shows pyo3's baked rpath pointing at `$HOME/miniconda3/lib` -- and `DYLD_LIBRARY_PATH` takes precedence over rpath, so it was shadowing the libpython the build chose with a different copy of the same ABI. The requirement is also narrower than either document claimed: the default suite needs no library path at all, all 773 tests passing with the variable unset, because pyo3 bakes an absolute rpath into the binaries it links. The `ml` feature does need one, that rpath not being emitted transitively, and without it `cargo test -p kolibrie --features ml` fails 34 targets and `kolibrie-http-server` exits 127. `CONTRIBUTING.md` now says which builds need it; `CLAUDE.md` is gitignored, so that edit stays local. Verified. `verify.sh` green on macOS: 773 tests, 51 of 51 examples, every phase at zero errors. Both CI legs as defined, in the container -- default: build 120s, test 4s with 773 passing across six doc-test targets, examples 51 of 51 in 17s; no-default: build 8s, test 4s, 773 passing. Deliberately not done. A clean Linux build leaves 12GB in `target/debug` per runner, which is where `debug = 1` would earn its keep -- for disk rather than the speed it was rejected for -- as `CARGO_PROFILE_DEV_DEBUG` in the workflow env, leaving the local profile alone. It is not set here because the real free disk on a runner was not measured, so it is insurance rather than a fix. Also left alone: the no-default matrix leg tests less than its name suggests, since `kolibrie`'s default feature set is empty and `--no-default-features` is not a narrower configuration for it.
`cargo fmt --all --check` is a blocking CI gate and the branch did not pass it: 48 files, 148 hunks. The drift accumulated across this branch's own work, chiefly the files the term model added or rewrote -- `rdf_term.rs`, `terms.rs`, `term_encoder.rs`, `sparql_database.rs`, `rsp_engine/mod.rs`, `py_rsp.rs` -- because `fmt` was never run as those landed. Formatting only; no behaviour change. That invalidated `docs/api.md`, whose own gate is also blocking. `scripts/api_surface.py` records public signatures as they are written in source, and rustfmt rewrapped four of them: `encode_triples`, `join_rule` and `parse_facts_shared` collapsed onto one line, `sort_by` expanded onto several. No item was added, removed or changed. Both failures would have blocked the merge to `main`, the second for a purely cosmetic reason. Verified: `verify.sh` green -- 773 tests, 51 of 51 examples, every phase at zero errors, `cargo fmt --all --check` and the api-surface check both clean.
`scripts/check_lint.sh unused_qualifications -- -p kolibrie --features ml --all-targets` is a blocking CI gate and found 5, all in the `MlRuntime` impl in `neural_relations.rs`. It wrote `std::collections::HashMap` twice, plus `shared::query::TrainNeuralRelationDecl`, `shared::query::MLPredictClause` and `shared::triple::Triple` in full, while `HashMap`, `TrainNeuralRelationDecl`, `MLPredictClause` and `Triple` are all imported at the top of the same file. Each replacement is exactly what rustc suggested. `crate::query_error::QueryError` and `shared::query::CombinedRule` keep their full paths: they are genuinely not in scope, which is why the lint flags five and not seven. Having now fixed CI gates one at a time twice over, this runs all of them instead: every one of the 18 `check_lint.sh` invocations across the workspace, `ml` and `no-default` configurations, plus the module graph, api surface and README example checks. 21 of 21 pass. The rustdoc job passes too, exit 0 with zero errors; its one remaining warning is `private_intra_doc_links` -- `ml`'s public documentation links to a private item named `syntax` -- which `-D rustdoc::broken_intra_doc_links` does not deny, so it is latent rather than breaking. Verified: 472 tests pass under `--features ml`, `cargo fmt --all --check` clean, and the api-surface check still matches.
`cargo machete` reported `serde_json` in `shared`, and `serde` and `rayon` in `datalogmtl`. Confirmed rather than taken on trust, because machete's usual false positive is a crate reached only through a derive macro: neither crate mentions any of the three anywhere -- not in `src`, not in tests, not in `datalogmtl/examples` or `datalogmtl/comparison`, and not as a `Serialize` or `Deserialize` derive. This does not touch the `float_roundtrip` guarantee from changes.txt entry 37. That feature is declared on the workspace dependency, and `ml`, which holds the test guarding it, declares `serde_json` for itself. Worth recording that the `supply-chain` job is `continue-on-error: true`, so unlike the rustfmt and lint gates this was advisory and would not have blocked the merge. Fixed anyway: an unused dependency is real cruft and three were cheap to drop. Verified in every configuration, because dropping a dependency can break one feature set and not another: `cargo check` passes for `--workspace --all-targets`, the same with `--no-default-features`, and `-p kolibrie --features ml --all-targets`. Tests: 773 default, 773 no-default, 472 under `ml`. `cargo fmt --all --check`, the api-surface check, the module graph and `cargo machete` are all clean. One false alarm along the way, mine: the first run of that configuration loop reported all three as failing, because zsh does not word-split an unquoted `$cfg` holding several flags and passed each set as a single argument. It contradicted a passing 773-test run in the same output, which is what gave it away. The same mistake is recorded at entry 684 in a timing loop.
ladroid
self-requested a review
November 3, 2026 12:18
ladroid
approved these changes
Nov 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.