Skip to content

test(gq): add the GQ logic test harness, corpus, and fix-regression gate - #596

Merged
azimafroozeh merged 8 commits into
ModernRelay:mainfrom
azimafroozeh:gq-logic-tests
Sep 2, 2026
Merged

azimafroozeh merged 8 commits into
ModernRelay:mainfrom
azimafroozeh:gq-logic-tests

Conversation

@azimafroozeh

@azimafroozeh azimafroozeh commented Sep 1, 2026 •

Copy link
Copy Markdown
Collaborator

What & why

Implements RFC 0045 (GQ logic tests, #584): a query-level regression becomes one small text file instead of a Rust test, and issue-closing PRs are held to carrying one.

  • crates/omnigraph/tests/gq_logic_tests.rs walks tests/gq_logic_tests/*.gqt: one self-contained case per file (issue anchor and red_on: provenance, then schema, seed, and query / mutate / restart steps with per-step params and expect).
  • Each case runs on the real engine: fresh store, init, load, index builds only when the case needs them, steps in order under a pinned traversal mode.
  • Expect modes: unordered (multiset), ordered, error: <substring>, affected: nodes=<N> edges=<M>; number normalization never routes integers through f64.
  • The parser is fail-closed: every malformed input is a refusal, and every refusal has a self-test.
  • OMNIGRAPH_GQ_BLESS=1 rewrites a failing case's expect rows in place; OMNIGRAPH_GQ_LOGIC_TESTS=<substring> filters cases by file name.
  • First corpus, five cases: the two performance: bm25/rrf ranked read + a join materializes the joined column corpus-wide → Arrow 2 GB offset overflow at ~700k+ rows #563 regressions, an ordering-refusal pin, a foreach + mutation + restart round-trip, and a typed mutation error.
  • Two new required PR checks in gq-logic-tests.yml: GQ Logic Tests runs the target pre-merge (the workspace job keeps covering it post-merge), and Fix Regression Gate (scripts/check-fix-regression.py) holds every issue the body closes by keyword to a matching issue_N test or .gqt case in the diff, waivable per PR with the no-repro label.
  • AGENTS.md gains the contract sentences: logic tests are the default query-behavior medium, every issue fix lands a regression at the cheapest tier that catches it, and #[ignore] messages open with their species.

The nightly heavy-repro job stays deferred until its first tests/repro_issue_*.rs member lands (the job fails on an empty glob by design).

Backing issue / RFC

Checklist

  • Change is focused (one test target, its corpus, its two CI checks, the AGENTS.md contract, dev-doc edits; no src/ changes)
  • Tests added/updated for behavior changes (84 harness tests: 5 corpus cases plus one self-test per refusal; the gate script carries a self-test the CI job runs first)
  • Public docs updated if user-facing surface changed (dev-facing only: testing.md, ci.md, branch-protection.md, AGENTS.md)
  • Reviewed against docs/dev/invariants.md (test-and-CI diff; no invariant or deny-list item touched)

Local verification

  • cargo test -p omnigraph-engine --test gq_logic_tests — 84/84
  • cargo clippy -p omnigraph-engine --all-targets -- -D warnings -W clippy::dbg_macro, cargo fmt --all — clean
  • scripts/check-fix-regression.py --self-test — ok; plus three scenario runs against this branch's own diff: closing performance: bm25/rrf ranked read + a join materializes the joined column corpus-wide → Arrow 2 GB offset overflow at ~700k+ rows #563 passes (the corpus matches), closing an issue with no test exits 1, the no-repro label waives
  • scripts/check-docs.py, scripts/check-agents-md.sh, scripts/check-workflow-action-pins.py — all OK
  • cargo test --workspace — not run locally (no src/ changes); CI covers it

Notes for reviewers

  • The only engine seam used is the test-only with_traversal_mode pin.
  • Two maintainer actions after merge: create the no-repro label, then run scripts/apply-branch-protection.sh (running it earlier leaves open PRs pending on contexts they cannot receive; both new checks already report on this PR).
  • The gate reads GitHub's keyword forms fixes #N, fixes: #N, and fixes:#N (leading zeros normalized); URL, cross-repo, GH-N, and no-space fixes#N closings pass unexamined and stay review's job.
  • The edited/labeled/unlabeled PR types re-run the gate when a body edit or the label changes its answer; the test job re-runs on them too (accepted cost).
  • The PR cache restores but never saves (save-if: push), so per-PR caches cannot evict main's.
  • Stricter than the RFC on small points (traversal pin forces index builds, 10000-iteration loop cap, exact-spelling numeric tokens, CRLF and foreign corpus files refused); each flagged on docs(rfc): add RFC 0043, GQ logic tests #584.

Greptile Summary

The PR adds a GQ logic-test harness and corpus together with required CI and branch-protection checks. It also introduces a fix-regression gate, but the gate’s attempted executable-shape validation still accepts ordinary Rust functions.

  • Adds the .gqt parser, runner, expectations, filtering, blessing, and initial regression corpus.
  • Adds pre-merge GQ testing and issue-fix regression enforcement.
  • Documents the new testing tiers and branch-protection requirements.

Confidence Score: 4/5

The PR is not yet safe to merge because the required fix-regression check can pass when an issue-closing change adds no executable regression test.

The gate accepts any suitably named function definition in Rust source files, including ordinary production or helper functions, while the pre-merge workflow only executes the GQ corpus and therefore does not close that enforcement gap.

Files Needing Attention: scripts/check-fix-regression.py

Important Files Changed

Filename Overview
scripts/check-fix-regression.py Adds the issue-fix regression gate, but Rust matches remain naming checks that can accept non-test functions.
crates/omnigraph/tests/gq_logic_tests.rs Adds the parser and execution harness for query-level logic-test cases.
.github/workflows/gq-logic-tests.yml Adds required pre-merge logic-test and regression-gate jobs.
AGENTS.md Defines the repository contract for selecting and naming regression-test tiers.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  PR[Pull request] --> Body[Parse closing keywords]
  Body --> Gate[Fix Regression Gate]
  Gate --> Corpus[Match added .gqt case]
  Gate --> Rust[Match added Rust function name]
  Corpus --> Pass[Required check passes]
  Rust --> Pass
  Corpus --> GQ[Run GQ logic corpus]
Loading

Fix all with Greploop Fix All in Claude Code

Reviews (3): Last reviewed commit: "test(gq): bound the walker, refuse unsaf..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Comment thread scripts/check-fix-regression.py Outdated
Comment thread scripts/check-fix-regression.py

@greptile-apps greptile-apps Bot 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.

Your trial has ended. Reactivate Greptile to resume code reviews.

@aaltshuler aaltshuler 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.

Technical review with five inline findings. The focused harness passes 93/93 tests, including all six corpus cases. The canonical workspace suite remains unverified on this SHA because the PR job was skipped and the PR notes that it was not run locally.

Comment thread crates/omnigraph/tests/gq_logic_tests.rs Outdated
Comment thread .github/workflows/gq-logic-tests.yml Outdated
Comment thread crates/omnigraph/tests/gq_logic_tests.rs Outdated
Comment thread scripts/check-fix-regression.py Outdated
Comment thread .github/workflows/gq-logic-tests.yml Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants