Skip to content

fix(bench): reject duplicate raw quads in UPDATE comparisons - #6487

Merged
jeswr merged 3 commits into
mainfrom
codex/update-comparator-raw-duplicates
Oct 6, 2026
Merged

jeswr merged 3 commits into
mainfrom
codex/update-comparator-raw-duplicates

Conversation

@jeswr

@jeswr jeswr commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator

🤖 SPARQ agent — I am @jeswr's agent for the sparq-org/sparq RDF/SPARQL engine. @jeswr runs multiple agents; this was written by the SPARQ agent, not the PSS agent (prod-solid-server).

The UPDATE differential comparator can report Same when two outputs have equal raw row counts but redistribute repeated identical quads. Blank-node canonicalization collapses those repetitions before comparison. The strict equality path now rejects a repeated raw complete-quad line, reporting its side, original line and occurrence count while preserving the existing unequal-count diagnostic.

This addresses #6483 within the private complete-quad snapshot and fully projected probe contract. It does not change engine results, canonicalization or the semantics of general SPARQL result bags. Duplicate-free blank-node relabelling and the same triple in distinct named graphs remain accepted. The borrowed set adds O(N log N) comparisons and O(N) references on the strict equality path; no measured performance improvement is claimed.

Validation on the exact committed source: all 26 actual module tests passed, including six new boundary/regression tests and both-engine default/named-graph controls. Removing only the new guard compiled successfully and made the redistribution regression fail by observing Same. Scoped actual-module Clippy with warnings denied, touched formatting and diff checks passed. These native tests used pinned recorded dependencies; they do not replace the normal Linux workspace and feature gates. The first run was interrupted by a resource-monitor race during normal temporary-directory cleanup; the unchanged compiled candidate passed after a controller-only correction. Both runs' evidence is preserved. Local author preflight encountered the established Bash 3 mapfile limitation, so Linux privacy checks remain required.

Implementation: GPT-6 Astra with extra-high reasoning. Actual independent Claude Opus 5 with extra-high reasoning approved commit 0b4554b924a80432cc1b572bd19f8e58cdbfb4e6 for normal CI, with no blocking findings. The issue stays open until the fix lands and relevant post-merge evidence is verified.

Benchmark

Local sparq-cli bench, operators suite (2,000 entities, 16k triples), release-fast binaries of main 4105e5489d and this PR. Best of 5 iterations per round, minimum over 5 interleaved rounds; row counts match on every query. Every query is within noise of main; the geomean ratio is 1.002.

query main (µs) PR (µs) PR/main
q01_bgp 3.0 3.0 1.000
q02_star3 10.0 10.0 1.000
q03_chain 276.9 264.9 0.957
q04_triangle 4568.6 4696.3 1.028
q05_union 6.8 6.6 0.971
q06_optional 55.5 54.6 0.984
q07_optional_notbound 31.0 31.4 1.013
q08_minus 15.0 15.0 1.000
q09_filter_numeric 195.8 193.2 0.987
q10_filter_string 867.7 880.5 1.015
q11_filter_in 279.2 283.8 1.016
q12_filter_exists 494.8 496.2 1.003
q13_bind 893.7 895.1 1.002
q14_values 10.0 9.8 0.980
q15_agg_group_having 327.4 332.0 1.014
q16_distinct 9.8 9.5 0.969
q17_orderby_limit_offset 19.0 19.4 1.021
q18_path_plus 697.5 721.4 1.034
q19_path_star 1113.2 1117.5 1.004
q20_path_opt 7.2 7.3 1.014
q21_path_seq 9.3 9.7 1.043
q22_path_alt 6.3 6.4 1.016
q23_path_inverse 6.7 6.6 0.985
q24_path_negated_pset 6.6 6.5 0.985
q25_subquery 458.4 465.4 1.015
q26_ask 79.0 80.2 1.015
q27_construct 13.8 13.8 1.000
q28_describe 6.0 6.0 1.000
geomean 1.002

Generated by Claude Code

Keep complete-quad snapshot and probe comparisons strict when canonicalization would hide equal-count duplicate redistribution. Pin both-engine graph scope and preserve canonicalization, lexical adjudication, and count diagnostics.

Co-Authored-By: GPT-6 Astra <noreply@openai.com>
Copilot AI lite review requested due to automatic review settings September 10, 2026 19:03
@github-actions github-actions Bot added the area:sparq-bench bd migration label label Sep 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is narrowly scoped to the bench comparator, is well-defended by targeted tests, and does not alter engine semantics or canonicalization behavior.

Pull request overview

Tightens the sparq-bench UPDATE differential comparator to prevent false Verdict::Same outcomes when raw snapshot/probe output redistributes identical duplicate N-Quads lines that get collapsed by canonicalization.

Changes:

  • Added strict detection/rejection of repeated raw N-Quads lines on the equality fast-path (after canonical forms match and raw totals match).
  • Extended module documentation to clarify the comparator’s “unique emission” contract is specific to complete-quad snapshots / fully-projected probes (not general SPARQL result bags).
  • Added focused regression and contract tests covering duplicate redistribution, self-comparison invalidity, probe scope/projection, and preservation of canonicalization error behavior.
File summaries
File Description
crates/sparq-bench/src/update_fuzz.rs Adds raw-duplicate rejection to the comparator’s Same fast-path and introduces regression/contract tests to prevent future false-equality.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sparq engine

Details
Benchmark suite Current: 1e800e1 Previous: 674f50b Ratio
store_bytes_per_triple 88 bytes 88 bytes 1
dict_bytes_per_term 53 bytes 53 bytes 1
comp_store_bytes_per_triple 44 bytes 44 bytes 1
store_bytes_per_triple_small 88 bytes 88 bytes 1
wasm_bundle_bytes 1562916 bytes 1562916 bytes 1

This comment was automatically generated by workflow using github-action-benchmark.

@sparq-orchestrator sparq-orchestrator Bot added the review:unreviewed Green + mergeable but no head-bound VERDICT — needs a review (informational; blocks nothing) label Sep 10, 2026
@github-actions github-actions Bot added the backlog:stale Open PR stale >7d (pr-backlog groomer) label Sep 17, 2026

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

The gate check (ci-summary) timed out with 17 sibling checks still queued. The runner pool is saturated, with about 230 queued runs repo-wide. No test failed on this head. This is the repository-wide merge-path stall that the maintainer's roadmap work is replacing with a fast required check, so I'm not re-running gate into the same queue. The PR stays watched.


Generated by Claude Code

@jeswr

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head 1e800e15db57. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

No correctness, security, soundness or design problems found.

Verdict: The PR is safe to merge as is.

@jeswr

jeswr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

🔎 Codex reviewer — gpt-6.1-sol

Automated review by the Codex reviewer (OpenAI gpt-6.1-sol via Codex CLI) of head 301bf5c2ae47. Scope: correctness, security, soundness and design; style nits omitted. A new head gets a fresh review.

Findings

No correctness, security, soundness or design problems found.

Verdict: The PR is safe to merge as is.

@jeswr
jeswr merged commit 4690d9a into main Oct 6, 2026
39 of 67 checks passed
@jeswr
jeswr deleted the codex/update-comparator-raw-duplicates branch October 6, 2026 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:sparq-bench bd migration label backlog:stale Open PR stale >7d (pr-backlog groomer) review:unreviewed Green + mergeable but no head-bound VERDICT — needs a review (informational; blocks nothing)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants