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

Author: azimafroozeh

.github/branch-protection.json

@@ -9,7 +9,9 @@
       "Graph Vocabulary Guard",
       "Test omnigraph-server --features aws",
       "Format (rustfmt)",
-      "Lint (clippy)"
+      "Lint (clippy)",
+      "GQ Logic Tests",
+      "Fix Regression Gate"
     ]
   },
   "enforce_admins": false,

.github/workflows/fix-regression-gate.yml

@@ -0,0 +1,58 @@
+name: Fix Regression Gate
+
+# This is a policy check on the pull request, so it must run code the pull
+# request cannot edit. `pull_request_target` takes this workflow file and
+# the scripts it runs from the base branch; the head is fetched only as
+# data for the diff range and is never checked out or executed. Under a
+# plain `pull_request` trigger a PR could replace the check with `true`
+# while keeping the required context name. The metadata events re-run the
+# gate when a body edit or the `no-repro` label changes its answer; this
+# workflow builds nothing, so those re-runs cost seconds.
+on:
+  pull_request_target:
+    types:
+      - opened
+      - synchronize
+      - reopened
+      - edited
+      - labeled
+      - unlabeled
+
+concurrency:
+  group: fix-regression-gate-${{ github.event.pull_request.number }}
+  cancel-in-progress: true
+
+jobs:
+  fix_regression_gate:
+    name: Fix Regression Gate
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    permissions:
+      contents: read
+    steps:
+      # Base branch, full history: the checker and this diff's merge base.
+      - name: Checkout base
+        uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1
+        with:
+          fetch-depth: 0
+
+      # The head as data only. `refs/pull/N/head` resolves for fork PRs too.
+      - name: Fetch the pull request head
+        env:
+          PR_NUMBER: ${{ github.event.pull_request.number }}
+        run: git fetch --no-tags origin "refs/pull/${PR_NUMBER}/head"
+
+      - name: Check closed issues carry regressions
+        env:
+          PR_BODY: ${{ github.event.pull_request.body }}
+          PR_LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }}
+          BASE_SHA: ${{ github.event.pull_request.base.sha }}
+          HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+        run: |
+          set -euo pipefail
+          python3 scripts/check-fix-regression.py --self-test
+          printf '%s' "$PR_BODY" > "$RUNNER_TEMP/pr-body.txt"
+          python3 scripts/check-fix-regression.py \
+            --body-file "$RUNNER_TEMP/pr-body.txt" \
+            --labels "$PR_LABELS" \
+            --range "$BASE_SHA...$HEAD_SHA"

.github/workflows/gq-logic-tests.yml

@@ -0,0 +1,147 @@
+name: GQ Logic Tests
+
+on:
+  # Code-bearing events only. The body and label events belong to
+  # `fix-regression-gate.yml`, which builds nothing; listing them here
+  # would re-run this Rust build on every label change.
+  pull_request:
+    types:
+      - opened
+      - synchronize
+      - reopened
+  push:
+    branches:
+      - main
+  workflow_dispatch:
+
+concurrency:
+  group: gq-logic-tests-${{ github.ref }}
+  # Superseded PR runs are safe to cancel; post-merge main runs stay alive.
+  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+jobs:
+  # A job cannot depend on another workflow's job, so this is a verbatim
+  # copy of `ci.yml`'s `classify_changes`; `ci.yml` is the source of truth,
+  # and an unconditional step at the top of the test job, right after
+  # checkout, asserts the two stay identical.
+  classify_changes:
+    name: Classify Changes (GQ Logic Tests)
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+    outputs:
+      run_full_ci: ${{ steps.filter.outputs.run_full_ci }}
+    steps:
+      - name: Checkout source
+        uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1
+        with:
+          fetch-depth: 0
+
+      - name: Detect documentation-only changes
+        id: filter
+        env:
+          BEFORE_SHA: ${{ github.event.before }}
+          EVENT_NAME: ${{ github.event_name }}
+          PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
+          PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+          REF_TYPE: ${{ github.ref_type }}
+        run: |
+          set -euo pipefail
+
+          if [[ "$EVENT_NAME" == "workflow_dispatch" || "$REF_TYPE" == "tag" ]]; then
+            echo "run_full_ci=true" >> "$GITHUB_OUTPUT"
+            exit 0
+          fi
+
+          if [[ "$EVENT_NAME" == "pull_request" ]]; then
+            base="$PR_BASE_SHA"
+            head="$PR_HEAD_SHA"
+          else
+            base="$BEFORE_SHA"
+            head="$GITHUB_SHA"
+            if [[ "$base" == "0000000000000000000000000000000000000000" ]]; then
+              base="$(git rev-parse "${head}^" 2>/dev/null || true)"
+            fi
+          fi
+
+          if [[ -z "${base:-}" ]]; then
+            echo "run_full_ci=true" >> "$GITHUB_OUTPUT"
+            exit 0
+          fi
+
+          # Disable rename collapsing so moving source into docs still reports
+          # the source-side deletion and cannot become a docs-only false skip.
+          mapfile -t changed < <(git diff --name-only --no-renames "$base" "$head")
+          if [[ "${#changed[@]}" -eq 0 ]]; then
+            echo "run_full_ci=true" >> "$GITHUB_OUTPUT"
+            exit 0
+          fi
+
+          run_full_ci=false
+          for path in "${changed[@]}"; do
+            case "$path" in
+              docs/*.md|docs/*.mdx|docs/*.rst|docs/*.adoc) ;;
+              README.md|AGENTS.md|CLAUDE.md|CHANGELOG.md|CONTRIBUTING.md|CODE_OF_CONDUCT.md) ;;
+              LICENSE|LICENSE.md) ;;
+              *)
+                run_full_ci=true
+                ;;
+            esac
+          done
+
+          printf 'Changed files:\n'
+          printf '  %s\n' "${changed[@]}"
+          echo "run_full_ci=$run_full_ci" >> "$GITHUB_OUTPUT"
+
+  gq_logic_tests:
+    name: GQ Logic Tests
+    needs: classify_changes
+    runs-on: ubuntu-latest
+    timeout-minutes: 45
+    permissions:
+      contents: read
+    env:
+      CARGO_TERM_COLOR: always
+      # Same floor as the Test Workspace job: unoptimized builds keep every
+      # nested async engine frame, deeper than a default 2 MiB thread stack.
+      RUST_MIN_STACK: 16777216
+    steps:
+      # Unconditional: the copy assertion below must run on every PR, the
+      # documentation-only ones included, inside this required context.
+      - name: Checkout source
+        uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1
+
+      - name: Assert the classification copy matches ci.yml
+        run: python3 scripts/check-classify-copy.py
+
+      # Documentation-only change: report success without building, so the
+      # required context is never left pending.
+      - name: Skip for documentation-only changes
+        if: needs.classify_changes.outputs.run_full_ci != 'true'
+        run: echo "Documentation-only change detected; skipping the GQ logic tests."
+
+      - name: Install system dependencies
+        if: needs.classify_changes.outputs.run_full_ci == 'true'
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y protobuf-compiler libprotobuf-dev
+
+      - name: Install Rust stable
+        if: needs.classify_changes.outputs.run_full_ci == 'true'
+        uses: dtolnay/rust-toolchain@4cda84d5c5c54efe2404f9d843567869ab1699d4 # stable
+        with:
+          toolchain: stable
+
+      - name: Cache Rust build data
+        if: needs.classify_changes.outputs.run_full_ci == 'true'
+        uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2
+        with:
+          workspaces: |
+            . -> target
+          # PR runs only restore; saving per-PR caches would evict the
+          # main-branch entries every warm run depends on.
+          save-if: ${{ github.event_name == 'push' }}
+
+      - name: Run GQ logic tests
+        if: needs.classify_changes.outputs.run_full_ci == 'true'
+        run: cargo test -p omnigraph-engine --test gq_logic_tests --locked -- --nocapture

AGENTS.md

@@ -149,8 +149,22 @@ Set `OMNIGRAPH_UPDATE_OPENAPI=1` only when the drift is intentional.
 
 - Preserve unrelated work in a dirty tree. Never discard user changes.
 - Make the smallest coherent change that closes the behavior and test surface.
-- For a bug, reproduce the predicted failure in the existing owning suite,
-  then fix the root cause and prove the regression turns green.
+- For a bug, reproduce the predicted failure at the tier the regression rule
+  below names, then fix the root cause and prove the regression turns green.
+- Query-behavior tests default to `.gqt` logic tests under
+  `crates/omnigraph/tests/gq_logic_tests/`; a Rust test needs a reason the
+  logic test format cannot express (mechanism assertions, scale symptoms,
+  process environment, concurrency).
+- Every issue fix lands a regression test at the cheapest tier that catches
+  the defect: a `.gqt` logic test when the defect is visible in rows, counts,
+  or errors, a `_issue_NNN` Rust test when it needs mechanism or scale
+  assertions; when the reported symptom additionally needs scale to
+  manifest, a second `#[ignore]`d test in a `tests/repro_issue_*.rs` target
+  guards it, and the two cross-reference each other in comments.
+- Every `#[ignore]`d test opens its ignore message with its species
+  (`instrument:`, `hunt:`, `heavy-repro:`, or the environment it needs);
+  expensive regression repros use `heavy-repro:` and thereby enroll in the
+  nightly job.
 - Update user-visible docs in the same change as a flag, endpoint, format,
   schema construct, behavior, or limit.
 - Update current developer guides when architecture or support boundaries

crates/omnigraph/src/exec/query.rs

@@ -2498,6 +2498,7 @@ async fn execute_expand_dispatch(
             mode = "csr",
             "expand mode chosen",
         );
+        crate::instrumentation::record_expand_path(false);
         return execute_expand_bfs(
             wide,
             graph_index,
@@ -2558,6 +2559,7 @@ async fn execute_expand_dispatch(
                     reason = "index coverage degraded",
                     "expand mode chosen",
                 );
+                crate::instrumentation::record_expand_path(false);
                 return execute_expand_bfs(
                     wide,
                     graph_index,
@@ -2589,6 +2591,7 @@ async fn execute_expand_dispatch(
         mode = "indexed",
         "expand mode chosen",
     );
+    crate::instrumentation::record_expand_path(true);
     // Surface the C6 silent scalar-index fallback once, now that coverage is known.
     warn_on_degraded_coverage(&coverage, key_col, edge_type);
     // Per-hop re-decision policy (issue #533): a forced mode is a contract and
@@ -3077,6 +3080,7 @@ async fn execute_expand_bfs(
             };
             if switch {
                 crate::instrumentation::record_traversal_mid_switch();
+                crate::instrumentation::record_expand_path(false);
                 let gi = graph_index.get().await?.ok_or_else(|| {
                     OmniError::manifest("graph index required for CSR traversal".to_string())
                 })?;

crates/omnigraph/src/instrumentation.rs

@@ -113,6 +113,15 @@ pub struct QueryIoProbes {
     /// Lets the switch tests assert the mechanism actually ran — mode
     /// equivalence alone stays green with the switch disabled.
     pub traversal_mid_switches: Arc<AtomicU64>,
+    /// Path commitments per Expand: the indexed scan, the CSR walk chosen up
+    /// front, or the CSR walk switched to mid-traversal (an Expand that
+    /// switches counts once on each). A logic test pinned with
+    /// `# traversal:` asserts the other path's counter stayed zero and, for
+    /// a query that expands, the pinned one moved: the pin is a task-local
+    /// override, and mode equivalence alone cannot see a pin the executor
+    /// ignored or a scope that dropped it.
+    pub expand_indexed_runs: Arc<AtomicU64>,
+    pub expand_csr_runs: Arc<AtomicU64>,
     /// Expand emissions stopped early by a pushed-down `limit` cap. Same
     /// rationale: capped-subset validity alone cannot prove the cap fired.
     pub expand_cap_stops: Arc<AtomicU64>,
@@ -544,6 +553,19 @@ pub(crate) fn record_traversal_mid_switch() {
     let _ = current(|p| p.traversal_mid_switches.fetch_add(1, Ordering::Relaxed));
 }
 
+/// Record which path one Expand ran (`indexed` true = the indexed scan,
+/// false = the CSR walk). No-op when no probes are installed (production).
+pub(crate) fn record_expand_path(indexed: bool) {
+    let _ = current(|p| {
+        let counter = if indexed {
+            &p.expand_indexed_runs
+        } else {
+            &p.expand_csr_runs
+        };
+        counter.fetch_add(1, Ordering::Relaxed)
+    });
+}
+
 /// Record one Expand stopping early at its pushed-down limit cap. No-op when
 /// no probes are installed (production).
 pub(crate) fn record_expand_cap_stop() {