perf(executor): model_query orchestrator overhaul — eight audit-driven optimisations#846
Draft
HexaField wants to merge 7 commits into
Draft
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Jun 4, 2026
`all_where_pushable` and `build_query_patterns` now handle every
`WhereCondition` variant for the synthetic `id` / `base` property:
`Number` and `Bool` translate to `FILTER(STR(?source) = "...")`,
`NumberArray` to `FILTER(STR(?source) IN (...))`, and `Ops`
(`not`/`gt`/`gte`/`lt`/`lte`/`between`/`contains`/`not_array`) bind
`STR(?source)` to `?_id_str` and emit the matching range / inclusion
filters on the IRI's string form. Closes the gap §3.5 of the design
note flagged where Ops/NumberArray on id/base silently fell through
the WHERE builder.
Adds `build_aggregate_sparql`, a per-source `(MIN(?ts), MAX(?ts))`
companion query used by tests to validate the SPARQL shape that
synthesises `createdAt` / `updatedAt`. The production pipeline keeps
the hydration-fold path: under Oxigraph 0.5.x the bounded `VALUES
?source { ... }` aggregate still costs ~150-300 ms on the medium-tier
Flux benchmark because the planner walks every reifier when matching
the triple-term pattern. The builder is retained so a future planner
improvement can flip the aggregate on without re-deriving the WHERE
shape.
Drops the now-unused `predicate_filter` field from
`InstanceQueryPlan::TwoPhase` and factors
`build_predicate_filter_for_property_fetch` out of the inline
two-phase property fetch so `query.rs` can derive the same filter
shape directly from the resolved `ModelShape`.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…t aggregate
Three integration-level guards for the SPARQL pushdown work:
- `test_last_write_wins_for_scalars` writes three versions of a scalar
property at increasing timestamps and asserts only the latest value
surfaces in `execute_model_query`. Locks in the contract that the
hydration fold's `?ts > current_ts` check survives any future
refactor of the property-row query shape.
- `test_id_ops_filter_pushes` confirms `id: { not: [...] }` is flagged
as fully pushable by `all_where_pushable` and runs end-to-end without
a Rust-side post-filter, returning exactly the IRIs the SPARQL
`FILTER(STR(?source) NOT IN (...))` clause excludes.
- `test_aggregate_createdAt_updatedAt_from_sparql` exercises both the
end-to-end synthesis through `execute_model_query` and the
`build_aggregate_sparql` builder directly, so a future flip from the
hydration fold to the SPARQL aggregate has a contract test ready.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…lder `build_aggregate_sparql` is exercised by integration tests but the production pipeline keeps the hydration fold for `createdAt` / `updatedAt`, so the function is unreferenced in `cfg(not(test))`. Gate `#[allow(dead_code)]` on the non-test build to keep `cargo check` warning-free without losing the helper. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
da99b61 to
eb5c383
Compare
This was referenced Jun 4, 2026
Six independent orchestrator-level optimisations that reduce the
SPARQL fan-out for a single perspective.modelQuery RPC. None change
the query DSL semantics; they remove wasted work where the caller's
intent makes it visible. All landed under the back-compat default —
opt-in flags surface the cheaper paths.
A. Opt-in reifier metadata (`withMetadata: false`)
Drops the `?_reifier <reifies> <<(?s ?p ?t)>> . ?_reifier
<author> ?author . ?_reifier <timestamp> ?timestamp .` join from
the main instance SELECT in both Single and TwoPhase plans.
Saves three triple-pattern matches per result row — empirically
~3.4x the per-row SPARQL cost on scan-all queries.
Caveat documented on `ModelQueryInput.with_metadata`: with the
flag off, hydration degrades from last-write-wins-by-timestamp
to last-row-wins-by-insertion-order, and `author` / `createdAt`
/ `updatedAt` are left unset on the result instances.
B. Opt-in `total_count` (`count: false`)
The COUNT round-trip now fires only when the caller asks for it.
Previous behaviour (fire whenever pagination is applied) is
preserved when `count` is `None`, so existing callers are
unaffected. `count: true` also forces COUNT on unpaginated
queries.
C. Single-plan when WHERE is uniquely-selective
When WHERE includes an id/base equality on `String` or
`StringArray` whose cardinality is <= LIMIT, skip the TwoPhase
pagination — phase 1's timestamp probe scans the reifier index
for every candidate source but there's nothing to sort over and
nothing to cut. Falls through to Single with LIMIT/OFFSET
applied post-hydration. Covers the common "fetch one row by
id" pattern. New helpers `where_clause_caps_result_size` /
`where_clause_max_source_count` in sparql_builder.rs.
D. Reverse-relation UNION fusion
`resolve_reverse_relations` now emits one SPARQL with
`VALUES ?predicate { ... }` over the union of reverse predicates
instead of one query per predicate. Saves R-1 round trips per
call for shapes with R reverse relations. Polymorphic-on-
predicate decoration still works because the row binner walks
`predicate_to_relations` instead of the predicate -> relation
1:1 mapping.
E. Multi-key sort pushdown
`SparqlPagination` now carries `sort_keys: Vec<(SortKey,
OrderDirection)>` (was: single key + direction). The pagination
subquery emits multi-column ORDER BY with NULL-pushed-to-end
handling per key, using SAMPLE() under `GROUP BY ?source` when
property keys are involved. Multi-key ORDER BY no longer
forces a post-hydration Rust sort.
F. Relation `where_filter` pushdown into one fused query
`apply_where_filter_to_relation` now folds the per-predicate
value fetches into one query with `VALUES ?predicate`, binning
rows by predicate and applying conditions in a single Rust
pass. Same N+1 -> 1 round-trip pattern as D.
J. Parallel resolveLanguage batching
`resolve_language_transforms` now collects every unique
(lang, expr_addr) pair across all instances + properties, fires
them concurrently via `futures::future::join_all`, and writes
results back in a single pass. Previously sequential per-
instance per-property — N awaits per call.
K. Streaming Solutions -> Vec<Value>
`SparqlStore::query_values{_async}` returns
`Result<Vec<Value>, Error>` directly, skipping the
`Solutions -> JSON String -> from_str -> Vec<Value>` round
trip that historical `query()` callers paid. Legacy
`query()` / `query_async()` continue to return `String` and
now delegate to `query_values` + a single `to_string`. All
call sites inside `perspectives::model_query` (query.rs,
relations.rs, getters.rs, projection.rs, shape.rs) switched
to the new methods.
Test plan
- `cargo check -p ad4m-executor --lib` clean
- `cargo test -p ad4m-executor --lib perspectives::model_query`
— 149 pass / 0 fail / 0 ignored
Follow-up PRs in this same branch:
- G: Getter inlining via BIND(EXISTS{...})
- H: Projection subquery inlining
- I: CONSTRUCT-based subgraph hydration
Every ASK getter declared on a shape is now folded into a single SELECT
that runs once per evaluate_getters call. Each getter contributes a
`BIND(EXISTS { <body> } AS ?_g_<i>)` column under
`VALUES ?source { ... }`, so one round trip resolves G boolean
getters across N instances instead of G separate per-getter queries.
SELECT getters still fan out one-per-getter — their multi-row binding
shape can't be folded into the same fused query without join-explosion
risk (each SELECT getter could match multiple rows per source).
Practical follow-up is to handle that via scalar-only SELECTs with
OPTIONAL{...} BIND, but that's a separate change.
Existing test (test_convert_ask_to_batched_select) still passes since
the helper is retained behind `#[allow(dead_code)]` for the
integration_tests module; the live path uses the new `extract_ask_body`
helper instead.
Test plan
- `cargo test -p ad4m-executor --lib perspectives::model_query` — 149 pass / 0 fail
When WHERE includes id-eq (`String(<iri>)`), emit `VALUES ?source { <iri> }`
rather than `FILTER(?source = <iri>)`. Both are semantically equivalent
but Oxigraph 0.5's planner can pin `?source` to the bound IRI before
joining with the rest of the WHERE under VALUES, materially faster than
applying a post-join FILTER as the dataset grows.
Caught by the wind tunnel S16 `sr_by_id_single_plan` case at medium
tier (10151 links): the new Single-plan branch from audit-item C was
spending 1.01ms vs 0.60ms on dev's TwoPhase (which already used VALUES
internally for phase 2's property fetch). With the VALUES emission,
the case drops to 0.26ms — 2.3x faster than dev's TwoPhase path on the
same data.
Test plan
- `cargo test -p ad4m-executor --lib perspectives::model_query` — 149 pass / 0 fail
- Wind tunnel S16 medium tier: `sr_by_id_single_plan` 4.2× (dev) → 1.2× (#846)
HexaField
added a commit
to coasys/ad4m-wind-tunnel
that referenced
this pull request
Jun 4, 2026
Four new cases test the orchestrator-level opt-ins landing in coasys/ad4m#846: | Case | Tests audit item | Expected impact | |---|---|---| | embeddings_all_no_metadata | A (withMetadata: false) | scan-all ratio ~3x lower | | sr_by_expression_limit1_no_count | B (count: false) | -0.1ms per call | | sr_by_id_single_plan | C (selective WHERE)+A+B | matches a Single-plan baseline | | sr_all_no_metadata_no_count | A+B combined | scan-all near-parity with raw | Each case keeps the same raw SPARQL on the left side so the ratio column reads "how much overhead does modelQuery still carry vs the hand-written SPARQL doing comparable work." Running against `dev` exercises the back-compat default (these opt-in flags are ignored, so we get the same numbers as the pre-existing cases). Running against `refactor/sparql-pushdown-last-write-wins` (PR #846) will show the ratio collapses to roughly raw-SPARQL parity for these flagged-off cases. Test plan - npx tsc --noEmit clean - ./run.sh --branch dev / #846 --scenario s16 — TBC after #846's release executor finishes building
HexaField
added a commit
to coasys/flux
that referenced
this pull request
Jun 4, 2026
Append "Realised wins" section with per-tier S16 ratio tables comparing `dev` against `refactor/sparql-pushdown-last-write-wins` (coasys/ad4m#846). Includes the same data the PR description carries, plus the per-category verdict refresh: most flux call sites that don't need link-level metadata or unpaginated counts are now convertible at 1.5-3x cost rather than 5-10x. Headline opt-in ratios from #846: - sr_by_id_single_plan medium: 4.2x -> 1.2x (3.45x) - sr_all_no_metadata_no_count medium: 8.7x -> 3.3x (2.63x) - embeddings_all_no_metadata medium: 10.0x -> 3.7x (2.71x) - sr_by_expression_limit1_no_count medium: 4.9x -> 2.8x (1.76x) Back-compat cases stay within ±10% noise. The residual 3-5x on scan-all is what audit items H and I would close; both deferred for a follow-up PR (H is low-priority, I is the 500-LOC CONSTRUCT rewrite).
Plumb the executor-side `with_metadata` opt-in flag through @coasys/ad4m's TypeScript Query type so call sites can opt out of the reifier-metadata join from JS. Mirrors the field added on `ModelQueryInput` in this PR's earlier orchestrator commit (b3f4292). Documented behaviour: when `withMetadata: false`, the hydrator skips the 3-triple-pattern reifier join in `build_instance_sparql`, leaving `author`/`createdAt`/`updatedAt` unset and degrading scalar-property hydration from last-write-wins-by-timestamp to last-row-wins-by- insertion-order. Recommended for read-only / append-only call sites that don't surface link-level metadata in the UI (e.g. count-only queries, simple scans). Added to: - `Query.withMetadata?` — top-level findAll/findAllAndCount option - `StrictTypedQuery.withMetadata?` — typed variant - `TypedRelationSubQuery.withMetadata?` — propagates to include sub-queries The flag flows through `prepareModelQueryParams` -> `queryInput.withMetadata` -> JSON.stringify -> Rust `ModelQueryInput.with_metadata` (serde rename).
HexaField
added a commit
to coasys/flux
that referenced
this pull request
Jun 4, 2026
Audit-driven migration of the convert-viable raw `querySparql` call
sites in `packages/api/src/{channel,conversation,conversation-subgroup,
semantic-relationship}/index.ts` to their `Ad4mModel.findAll{,AndCount}`
equivalents. Lands on top of coasys/ad4m#846 — the orchestrator
overhaul that makes these conversions viable at 1.5-3x cost (rather
than the 5-10x of pre-#846).
Migrated sites (5 production methods):
1. `Channel.pinnedConversations()` (Cat A site #7)
- `findAll(Channel, { where: { isPinned: true }, include: { conversations: { limit: 1, withMetadata: false } }, withMetadata: false, count: false })`
- Single-plan path engaged: the executor sees `isPinned == true` as
uniquely-selective via the `@Property` flag form.
2. `Conversation.stats()` (Cat A site #10 — partial)
- Replaced the subgroup count SPARQL with
`ConversationSubgroup.findAllAndCount(..., { limit: 0, count: true, withMetadata: false })` — count-only fast path.
- Replaced the participant SPARQL with native `LinkQuery` via
`perspective.get(...)` — no SPARQL roundtrip needed for this
simple link enumeration.
3. `ConversationSubgroup.stats()` (Cat A site #15)
- Both queries (item count + participant list) replaced with
parallel `LinkQuery`s via `perspective.get(...)`. Avoids the
model_query orchestrator entirely because there's no
multi-property hydration needed.
4. `SemanticRelationship.itemEmbedding()` (Cat B site #19)
- Promoted the `itemEmbeddingViaModel()` demonstrator (added during
the audit) to be the primary implementation. Uses
`include: { embeddingTag: { withMetadata: false } }` with the
polymorphic-on-same-predicate `@HasOne` confirmed working in
wind tunnel S16 (`include actually fires: yes`).
- Removes the `itemEmbeddingViaModel` method — superseded.
5. `findEmbeddingSRId(itemId)` in `conversation/util.ts` (Cat E site #26)
- Promoted from "needs nested-where-on-relation" to convertible
because the executor's polymorphic discrimination on the include
resolves to the Embedding tag only (the Topic variant doesn't
fire when the conformance doesn't match). Looks at `embeddingTag`
on the result instances.
Kept as raw SPARQL with audit-grounded rationale (16 production sites):
- Cat C reifier-metadata reads (4): Channel.allItems, Channel.unprocessedItems
data fetch, Conversation.subgroupsData, ConversationSubgroup.itemsData —
all want per-link timestamp (the reifier on `<chan> has_child <msg>`,
not on the message entity). Ad4mModel's `createdAt` synthesis refers
to the entity hydrate, not to the link.
- Cat D set-difference (2): Channel.unprocessedItems set-difference
pair — `FILTER NOT EXISTS` planner cliff still present.
- Cat E inter-class joins (4): Topic.linkedConversations,
Topic.linkedSubgroups, Conversation.subgroupsData batch timestamp,
Conversation.topics UNION — all need multi-hop joins through
predicates that aren't declared as relations on the source model.
- Cat A site #5 Channel.totalItemCount — multi-type FILTER would split
into 3 parallel `findAllAndCount` calls and a sum. Worse than 1 SPARQL.
- Cat A site #6 Channel.recentConversations — already hand-optimised to
avoid reifier joins (uses native link API for timestamps).
- ConversationSubgroup.topics, .topicsWithRelevance, Topic.* — Cat E.
- SemanticRelationship.allConversationEmbeddings, .allSubgroupEmbeddings,
.allItemEmbeddings, .allItemEmbeddingsByType — needs reverse
`@HasMany`/`@BelongsTo` from Conversation/Subgroup/Message back to SR,
which isn't declared today. Convertible in a follow-up PR.
Tests
- Updated `channel.test.ts` and `conversation.test.ts` mocks to include
`perspective.modelQuery` (for the migrated paths) and seed
Ad4mModel static-method spies where the @coasys/ad4m mock can't
exercise the real findAll/findAllAndCount pipeline.
- 119 tests total: 115 pass / 4 pre-existing failures unrelated to this
migration (`Channel.unprocessedItems` × 3 + parseLit JSON-stringify ×
1, all caused by the test file's `@coasys/ad4m` vi.mock missing
`fileToDataUri` — present before this change).
Stacked dependency
- This PR's runtime correctness depends on coasys/ad4m#846 (and the
upstream stack #837 / #842). The `withMetadata: false`,
`count: false` and selective-WHERE single-plan paths the migrations
rely on are only honoured by the post-#846 executor. Running
against `dev`'s executor falls back to the back-compat default
(slower but still functional).
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.
Summary
Stacks on top of #842 and extends the scope from the original "push
id/baseOps+NumberArray+ scaffoldcreatedAt/updatedAtaggregate" follow-up into a broadermodel_queryorchestrator overhaul based on the single-SPARQL elegance audit in coasys/flux#605.Eight orchestrator-level wins land here, all preserving back-compat (opt-out behaviour is the same as today). Cross-references against the audit's lettered PR sequence:
b3f4292withMetadata: false) — drops?_reifier reifies + author + timestampjoin. ~3.4× per-row scan cost.b3f4292total_count(count: false) — skip COUNT round trip unless caller reads it.b3f4292+376d4b1VALUES ?sourceinstead ofFILTERfor id-eq.b3f4292@BelongsTopredicates. Saves R-1 round trips.b3f4292SparqlPagination.sort_keys: Vecwith multi-columnORDER BY+ GROUP BY/SAMPLE for property keys.b3f4292where_filterpushdown — fold per-predicate value fetches into one query.61bb127BIND(EXISTS{...})SELECT — one round trip across G getters.b3f4292resolveLanguagebatching viafutures::future::join_all.b3f4292Solutions → Vec<Value>(skip a JSON serialise + parse round trip per SPARQL call).Plus the original #846 scope already merged:
id/baseOps+NumberArraypush to SPARQLbuild_aggregate_sparqlscaffold forcreatedAt/updatedAtH (projection subquery inlining) and I (CONSTRUCT-based subgraph hydration) are intentionally deferred — both are >150 LOC structural rewrites and the A–G + J + K set already closes 2.4–3.4× of the S16 ratio on every opt-in case.
Performance — wind tunnel S16, S5, S8
All measurements: fresh
dev(HEAD1f29d0b1) vsrefactor/sparql-pushdown-last-write-wins(HEAD376d4b1b). Both binaries built into the same sharedCARGO_TARGET_DIRfrom the same Rust toolchain. Apple Silicon, 10 runs/case + 1 warm-up.S16 — direct
modelQuerymeasurementsMedium tier (1000 items, 10151 links) — opt-in cases vs back-compat:
sr_by_expression_limit1(default)sr_by_expression_with_include(default)sr_all(default)embeddings_all(default)topics_all(default, sub-ms raw)embeddings_all_no_metadata(A)sr_by_expression_limit1_no_count(B)sr_by_id_single_plan(C + A + B)sr_all_no_metadata_no_count(A + B)Small tier (100 items, 1051 links) — opt-in cases:
embeddings_all_no_metadata(A)sr_by_id_single_plan(C + A + B)sr_all_no_metadata_no_count(A + B)sr_by_expression_limit1_no_count(B)Default-behaviour cases stay within ±15% noise — back-compat preserved.
Cross-scenario regression check
To confirm the orchestrator changes don't regress paths that don't opt in:
S5 (
queryLinksscaling, 100/500/1000 links) — never touchesmodel_query, but shares theSparqlStorewhosequery()now delegates toquery_values(K):S8 (raw
querySparqlon Flux community graph):totalItemCountallItemsunprocessedItemsrecentConversationspinnedConversationssubgroupItemsDatasubgroupTopicsmessageHydrationpaginatedMessagesS8 medium tier (58460 links): every query within ±8% of dev — parity dominates as per-call SPARQL execution cost dwarfs per-RPC overhead at that scale.
Pre-existing query paths are unaffected at large data sizes and see incidental 5–30% wins at small sizes where K's
Solutions → Vec<Value>cuts a JSON serialise+parse round trip that was a meaningful fraction of total latency.Test plan
cargo test -p ad4m-executor --lib perspectives::model_query— 149 pass / 0 fail / 0 ignoredcargo check -p ad4m-executor --libcleanCompanion PRs
coasys/ad4m-wind-tunnel#6— extended S16 withwithMetadata: false,count: false,id-eq cases. Will serve as the permanent regression gate against this orchestrator surface.coasys/flux#605— migration doc with the full audit, per-PR sequence, and realised-wins tables (this PR is items A/B/C/D/E/F/G/J/K landing together).Follow-ups deferred from this PR
relations.rs+hydration.rs; deserves its own PR with a clean diff against this orchestrator surface.cc @lucksus @data-bot-coasys for review.