fix(query): correct three fast paths that diverged from the generic pipeline

[bplatz]

Closes #1652.

All three reported shapes reproduce as fast != generic under the
FLUREE_DISABLE_QUERY_FAST_PATHS kill switch, on small indexed ledgers — no
DBLP-scale data needed. Two of the three fixes are arithmetic/carry corrections
that keep the lane; the third is an eligibility narrowing in the style of #1628.

1. Star top-k dropped join multiplicity

GroupByObjectStarTopKOperator treated the star's filter triples as an
existence semi-join — the merge-join lane deduplicated filter subjects, the
multi-predicate lane intersected them into a set — so

SELECT ?o1 (COUNT(?s) AS ?count) {
  ?s :bibtexType ?o1 .
  ?s :hasSignature ?o2 .
} GROUP BY ?o1 ORDER BY DESC(?count) LIMIT 10

counted qualifying subjects instead of joined rows. On a 20-triple ledger
(one publication with three signatures, one with two): fast 2, generic 5.
Reproduces at any leaflet size, no overlay required.

The merge-join lane now consumes a whole filter-subject run into an
(s, count) group, the multi-predicate lane folds filter predicates into a
subject -> product-of-counts map, and AggStateStar::observe takes that
multiplicity — COUNT adds it, MIN/MAX/SAMPLE ignore it (duplicate-insensitive,
and they were already correct, which is why only the count column was wrong).

2. Chain fold lost rows at POST leaflet boundaries

Not the tail-weight machinery the issue suspected. PostObjectGroupCountIter
restarted an object group at every leaflet boundary, emitting the same o_key
twice with partial counts; execute_chain seeks PSOT forward-only, so the
repeated (non-increasing) key read as absent and every row of the second
fragment was dropped. Confirmed by instrumentation — a
duplicate POST object group emitted immediately followed by a seek miss.

Two consequences worth noting against the report:

• it hits the plain inner-join chain too, so "plain is correct" there was
  layout luck — a 6-row chain gives fast 5 vs generic 6 at 3-row leaflets;
• OPTIONAL, MINUS and EXISTS inherit it from the shared driver, which is why
  their deltas were identical (-434).

The iterator now carries the open group across leaflet and batch refills, the
way PsotSubjectCountIter already did. Found alongside: a homogeneous non-IRI
leaflet terminated the stream instead of being skipped, and since POST sorts
non-IRI o_types below IRI_REF, that truncated every IRI group behind one on a
mixed-object predicate.

3. Composite (s,o) join compared non-identifying keys

count_composite_join_pairs merge-joins ?s <p1> ?o . ?s <p2> ?o on
(s_id, o_type, o_key) across two different predicates. A
NUM_BIG_OVERFLOW o_key is a handle into a per-predicate arena, not a term
identity, so equal xsd:decimals and overflow integers under the two
predicates never matched — the same trap #1628 gated for the whole-graph
COUNT(DISTINCT ?o); this lane never had it. A 20-row ledger where createdBy
carries a wider big-value set than authoredBy: fast 3, generic 9.

Probing the fix surfaced a second hazard in the same key: list rows. The
generic pipeline's list-element join semantics (a list element does not match a
plain ref; aligned duplicate lists pair per-row) are not expressible in a key
that drops o_i.

Both now decline through predicate_unsafe_for_cross_predicate_o_key_join,
which answers from leaflet directory metadata (HAS_O_I flag, o_type_const)
and decodes an o_type column only for a mixed leaflet whose key range straddles
NUM_BIG_OVERFLOW — inline numerics sort below it and langstrings above, so
the range test alone over-declines ordinary int+string+langstring predicates
(the must-fire assertion caught exactly that). The overlay lane declines
row-wise on a live o_i, covering novelty list rows the directories have not
seen.

Testing

New standalone binary it_fastpath_1652_regression.rs (own process: it toggles
the global kill switch and asserts routing): 16 cases over 5 ledgers, with the
chain ledger indexed at three-row leaflets to force boundary-straddling groups
at test scale. Every case pins three things — the fast lane's answer, the
generic pipeline's answer (both against hand-computed values, so a generic
regression fails as loudly as a fast-path one), and the engine's fast-path outcome stamps: must-fire for every shape that keeps its lane, so a fix
cannot silently degrade into a disable, and must-not-fire for the composite
shapes that now decline.

The differential harness gains a group_by_object_star_topk case across base,
overlay and novelty.

Green locally: 1440 fluree-db-query unit tests, all 422 grp_query tests
(differential harness included), #1628's it_repeated_var_fast_path_guards and
it_count_datatype_pin, it_query_explain, it_distinct_object_numbig_gate,
plus clippy --all-features --all-targets -D warnings and cargo fmt --check.

Follow-up in this PR

Review surfaced a fourth divergence in the same operator: detect_group_by_object_star_topk
admitted star shapes whose filter triples share an object variable. It destructured
(sv, pred, ov) but pushed only pred into filter_preds, so filter object vars were never
compared to each other — and for a shared var the product of per-subject counts is not the join
multiplicity, it's the size of the value intersection.

?s ex:bibtexType ?o1 . ?s ex:refA ?x . ?s ex:refB ?x returned 57/57/57/57/21 against the
generic pipeline's 14/14/14/14/5, lane stamped proceed. The admission predates this PR (the
base's existence semantics mis-answers the shape differently), but the product fold made it a
certified-lane divergence, so filter object vars must now be pairwise distinct and the shape
declines. Distinct vars over the same predicate stay eligible — there the product is the
multiplicity — pinned by a must-fire companion case. Both probes ride on the star ledger, whose
refA/refB include every-17th-subject disjoint values so the generic row-drop is covered too.

Also trims the case-3 memory fact to the 750-char cap repo_memory_lint enforces (was 806,
the sole red job); the o_type-ordering detail it drops is already in that fact's rationale.

[aaj3f]

This is the strongest of the fast-path correctness PRs so far, @bplatz, and I'm eager to have it.

Two things may warrant consideration before merge, both small.

First, CI's test job is red on this PR itself: the case-3 memory fact at .fluree-memory/repo.ttl:2993 is 806 chars against repo_memory_lint's 750-char cap — one string to tighten.

Second, and the real one: detect_group_by_object_star_topk (operator_tree.rs:814-832) admits star shapes whose filter triples share an object variable, and for those the new product-of-counts is not the join multiplicity — at HEAD, ?s ex:pg ?o . ?s ex:f1 ?x . ?s ex:f2 ?x returns fast [T1:5, T2:1] vs generic [T1:2] with the lane stamped proceed. Your code didn't introduce the admission (the base's existence semantics is wrong on that shape too, differently), but this PR is precisely the fast≠generic fix for this operator, and a pairwise-distinctness check on the filter object vars in the detector — declining to generic like the composite shapes — closes it for a few lines plus one must-not-fire probe case.

Adherence to repo commitments:

• Patterns/abstractions: ✔ reuses the carry pattern and the #1628 metadata-gate style; extends AggStateStar::observe rather than adding a parallel path; correctness-preserving fallback honored (Ok(None) declines, no partial results).
• Performance (speed first, memory second): ✔ hot operators touched but no per-row allocation added; run-length counting replaces dedupe; the gate is a once-per-query metadata pass in front of a full-scan join; bench-compare passed.
• Testing: ⚠️ excellent where it looks — dual-lane hand-pinned values, routing stamps, 3-row leaflets, differential-harness case, all verified locally and mutation-checked — but the shared-filter-variable shape is uncovered and diverging, and CI is currently red on the memory lint.
• Conventions: ✔ thorough multi-line commit body; fmt and clippy clean on the changed crates at HEAD ("fifteen cases" in the body is actually 14 — cosmetic).

Verified locally at branch HEAD (0438ac3): fluree-db-query --lib 1402 passed; it_fastpath_1652_regression pass; grp_query it_differential_fastpath pass; mutation of observe fails exactly the 4 star cases; adversarial shared-var probe diverges at HEAD and at base; fmt + clippy clean. CI run 32240014671: 11129/11130 pass, sole failure is the repo-memory lint above.

I know this is a sacked PR and other PRs in the stack may already do this to some degree, but this makes me wonder where else we could adopt the routing-stamp pattern within other fast-path suites for similar improvements/guarantees

[aaj3f]

🔴 Blocking — the detector admits star shapes whose filter triples share an object variable, and for those this product-of-counts is not the join multiplicity.

Claude caught this while probing the detector with adversarial star shapes: detect_group_by_object_star_topk (operator_tree.rs:814-832) admits two filter triples that share one object variable, and ?s ex:pg ?o . ?s ex:f1 ?x . ?s ex:f2 ?x is a join on ?x — the true per-subject multiplicity is the size of the f1/f2 value intersection, not count(f1) × count(f2). The detector's loop only checks sv == ov (via validate_simple_triple) and ov == group_var; it never checks filter object vars against each other.

Verified at HEAD on a 3-subject ledger: fast returns [T1:5, T2:1] where generic returns [T1:2] — the count is wrong AND a phantom group appears (s4's f1/f2 values are disjoint, so the generic join drops T2 entirely), with group_by_object_star_topk stamped proceed, i.e. this is the same silent-200 divergence class this PR exists to close. To be fair about provenance: the admission is pre-existing — at the merge base the existence semantics mis-answers other shared-var datasets (it doesn't even require the shared ?x values to match) — but this PR's fix model bakes in the independence assumption, and the new must-fire stamps now certify the lane on a shape it cannot answer.

The fix is a few lines in the detector: collect the filter ovs and return None on a repeat, so the shape declines to the generic pipeline like the composite shapes now do. (Duplicate identical triples are deduped upstream and agree — verified — so only the distinctness check is needed.) A probe case in it_fastpath_1652_regression.rs with a must-not-fire stamp would pin it.

(Commenting here on the product-fold because operator_tree.rs:814 is not in this diff.)

[aaj3f]

🔴 Blocking (mechanical) — CI's test job is red on this PR, and it's this line.

The case-3 memory fact fact-01m0bredy3qtxvftbwass43qkc has an 806-char mem:content, over the 750-char cap enforced by fluree-db-memory/tests/repo_memory_lint.rs:166 (repo_memory_blocks_are_well_formed). The fact is introduced by this PR's single commit, and it's the only failure in the 11130-test run; clippy, fmt, testsuite-sparql, and bench-compare all passed.

Tightening that one string (the o_type-ordering detail could live in mem:rationale, which already covers half of it) turns the job green.

[aaj3f]

Optional. The comment at count_plan_exec.rs:3505-3509 is now stale in a way that could mislead the next reader: it justifies the predicate_objects_all_iri guard by "PostObjectGroupCountIter, which terminates on a homogeneous non-IRI leaflet" — after this PR it skips, not terminates — and its parenthetical "POST orders such leaflets before IRI_REF" is only true for inline/string types (langstring and NUM_BIG sort after).

The guard itself is still needed and correct — a literal ?b survives the OPTIONAL with multiplier 1, and the IRI-only iterator would now silently skip those rows rather than terminate, which is still an undercount — so this is purely a comment refresh

(Commenting here because count_plan_exec.rs:3505 is not in this diff.)

[aaj3f]

Praise. The must-fire / must-not-fire routing assertions are the piece these fast-path fixes have been missing — a fix can no longer silently degrade into a disable, and a decline can't silently un-decline. I mutation-checked the suite (reverted observe to saturating_add(1)): exactly the four star cases fail, with the proceeded-sites list right in the failure message. This pattern is worth carrying into every future fast-path PR.

[aaj3f]

Praise. The straddle-only o_type decode is the right call — I verified the range test alone would over-decline an ordinary int+string+langstring predicate (inline numerics sort below NUM_BIG_OVERFLOW, langstrings above), and the must-fire assertion on the clean composite ledger is what keeps that from regressing.

[bplatz]

Thanks @aaj3f — both blocking items are fixed, plus the optional comment refresh. Pushed as 3ecb8ac8e (fixes) and b3d327949 (comments); CI run 32325826441 is green on all five jobs, including the test job that was the sole red one.

Shared filter object variable

Confirmed, and the mechanism is exactly as you describe: the detector destructures (sv, pred, ov) but pushes only pred into filter_preds, so ov is discarded and filter object vars are never compared to each other.

Filter object vars must now be pairwise distinct; a repeat returns None and the shape declines. I checked the neighbouring cases so the guard does not over-decline — ov == group_var twice and sv == ov already declined elsewhere, and distinct vars over the same predicate stay eligible, since there the product genuinely is the multiplicity.

Two probe cases, on new refA/refB predicates in the star ledger (adding them left every existing hand-pinned value untouched):

• star topk shared filter object var declines — must-not-fire, expects 14/14/14/14/5
• star topk distinct filter object vars keep the lane — must-fire, expects 57/57/57/57/21

Every 17th subject gets disjoint refA/refB values, so the probe covers the generic row-drop behind the phantom group you saw, not only the inflated count. Mutation-checked both ways: with the guard reverted the fast lane returns 57/57/57/57/21 against the generic pipeline's 14/14/14/14/5, stamped proceed; with it in place, 16/16 cases pass.

Memory lint

Trimmed to 693 chars. The dropped clause (o_type ordering / range-alone over-declines) was already carried by that fact's mem:rationale, as you suggested. Added a fact recording the pairwise-distinctness invariant, since the convention here is that each fixed case carries one.

Stale comment

Fixed, and the sweep turned up two more. Your reading was right on both halves — the iterator now skips rather than terminates, and langstring/NUM_BIG sort after IRI_REF, not before — and the guard itself stays: I traced the shape (?a p1 ?b . OPTIONAL { ?b p2 ?c . ?c p3 ?d } with multiplier max(1, …)), so a literal ?b must still count once and skipped rows undercount just as a truncated stream did. Only the reasoning changed.

The two extras were made incomplete by the fix above rather than by the original PR: detect_group_by_object_star_topk and GroupByObjectStarTopKOperator both described the product of per-subject counts as the join multiplicity without recording that this holds only for pairwise-distinct filter object vars. Both now state the precondition and point at each other — otherwise the new guard reads as incidental, which is how it gets deleted later.

fast_path_common.rs's own iterator docs are already accurate post-fix, and its "POST sorts o_type before o_key" line is a claim about column order rather than about which types come first, so it stays as written.

Routing stamps elsewhere

Filed #1669. Not covered by anything in the current stack — I checked the open PRs and issues.

Sizing it turned up more than expected: only ~8 sites stamp at all, 5 of 8 fast-path suites assert on them, and it_differential_fastpath is among the ones that don't — the harness whose entire purpose is fast-vs-generic agreement currently cannot distinguish "the fast path agreed" from "the fast path never ran." There is also a second, parallel surface: fast_path_common.rs:3833 still emits "fast path produced result" next to the structured stamp purely to keep it_minmax_fast_path_fired working. The issue also ties this to the TODO(PR-3) GateVerdict seam already noted in fast_path_outcome.rs, since the test-side adoption is what would keep that honest once it exists.

Also updated the PR body — the case count is now 16, which incidentally settles the "fifteen" discrepancy you spotted.