Skip to content

test(gq): add a mandatory --- expect shape section and a result-schema check - #635

Merged
azimafroozeh merged 2 commits into
ModernRelay:mainfrom
azimafroozeh:0134-gqt-executed-schema-check
Sep 5, 2026
Merged

test(gq): add a mandatory --- expect shape section and a result-schema check#635
azimafroozeh merged 2 commits into
ModernRelay:mainfrom
azimafroozeh:0134-gqt-executed-schema-check

Conversation

@azimafroozeh

Copy link
Copy Markdown
Contributor

What & why

This PR makes a .gqt rows step state and check its result columns, so a wrongly typed result column fails the case even when every affected cell is null. Follow-up to #596 and #607; motivated by the fix for #623, whose zero-row aggregate case could not go red: since #627 the JSON writer omits a null cell's key, so {"n": 0} renders from a Float64 column and a Date32 column alike, and the type assertion had to live in a Rust test.

  • Every rows step (--- expect unordered / --- expect ordered) carries a mandatory --- expect shape section: one <name>: <type> line per result column in .pg property syntax, ? permitting a null cell (p.age: I32?, total: I64, p.embedding: Vector(3)). A missing section is a parse refusal naming both routes (write it from the .pg schema, or fill it with OMNIGRAPH_GQ_BLESS=1 and review the diff).
  • The section is compared against the executed QueryResult::schema() before the rows: column count, executed column name (executed_column_name), Arrow DataType through PropType::to_arrow, and no null cell in a column written without ?. Messages spell both sides in .pg where the executed type has a spelling (expected I32, the executor returned F64).
  • A second, computed check compares the executed schema against the compiler's infer_query_result_schema for the step's declaration: count, name, DataType, and no null in a column the compiler infers non-nullable. It costs the author nothing and ties lint's promise to the executed result; the shape section exists because this check cannot see a rule the compiler and the executor share.
  • Bless fills an empty section or rewrites a mismatching one from the executed schema, writing ? exactly when the column holds a null cell (the executor's own nullable flag is never read: aggregate_return sets every column nullable). Bless refuses to rewrite a shape whose executed type the compiler disputes, and refuses a type with no .pg spelling, naming it.
  • PropType::from_arrow / ScalarType::from_arrow are added to the compiler as the inverse of to_arrow over its image, with a round-trip test over every scalar, list and nullable form; executed_column_name becomes pub.
  • The 16 rows steps in the corpus gained their sections (9 cases); RFC 0045 is amended inline (File format, Comparison semantics, Bless mode, Execution semantics, Compatibility, Evidence, dated decision-log entry with a Superseded list); docs/dev/testing.md, the crate README, AGENTS.md's regression-tier sentence and the Fix Regression Gate's printed skeleton follow.

Backing issue / RFC

  • Implements RFC 0045, GQ logic tests (docs/rfcs/0045-gq-logic-tests.md), amended in this PR with the shape section and the result-schema check; no separate issue.

Checklist

  • Change is focused (the .gqt runner's result-schema checking: shape section, computed check, bless, and the docs and corpus that carry them)
  • Tests added/updated for behavior changes (116 runner self-tests, 22 of them new: section placement and refusals, mismatch messages, bless convergence and refusals, shape-before-rows ordering; compiler from_arrow round-trip; the 11 corpus cases run with their 16 shape sections; disabling the check turns three wiring pins red)
  • Public docs updated if user-facing surface changed (docs/dev/testing.md, crates/omnigraph-gqt/README.md, AGENTS.md, RFC 0045; no product surface changes)
  • Reviewed against docs/dev/invariants.md — no Hard Invariant weakened, no deny-list item hit (test harness and one compiler visibility change plus a pure type mapping; the product .pg grammar and the query surface are untouched)

Local verification

  • cargo check --workspace --all-targets — green
  • cargo test -p omnigraph-gqt — green: 116 self-tests, 11 corpus cases
  • cargo test -p omnigraph-compiler types:: — green: 5 tests
  • cargo clippy -p omnigraph-gqt -p omnigraph-compiler --all-targets -- -D warnings -W clippy::dbg_macro — green
  • cargo fmt --all --check — green
  • python3 scripts/check-fix-regression.py --self-test — ok
  • scripts/check-agents-md.sh — green (45 links, 43 docs)
  • python3 scripts/check-docs.py — green (125 files)
  • cargo run -p omnigraph-vocabulary-guard -- check --surface rust-string --base <merge base> ... — not conclusive: the audit is disabled in CI (VOCABULARY_AUDIT_ENABLED: "false") and the merge base itself reports unclassified occurrences
  • cargo test --workspace — not run: the engine and server crates are not touched; the compiler change is a visibility keyword and a new type mapping covered by its own tests

Notes for reviewers

  • Landing order: a rows step whose zero-row result carries a non-count aggregate is red until the fix for bug: min and max refuse Date and Bool columns #623 lands (the executor types the column Float64); the shape line written from the .pg schema is right, the message says so, and bless refuses to pin the executor's type. No corpus case carries one.
  • Landing order: a bare node projection (return { $p }) is red under the computed check until the bare-node projection fix lands (the compiler infers the node object, the executor returns the id column); no corpus case carries one, and the failure message names the form.
  • Open PRs whose cases have rows steps go red on rebase until their sections are added; bless fills them in one run per step, and the reviewed diff is where each blessed line is checked against its .pg declaration. test(gq): add the Kuzu-derived .gqt case corpus #617 is the largest.
  • Honest routes the docs give an author: write the section by hand from the .pg schema and the return clause; or run, hit the refusal, bless, review the diff. p: Person for a bare node is refused until the bare-node fix lands. A .pg-nullable property may be written without ? when the step's data holds no null; that is the stronger assertion and is accepted.
  • What the rule does not stop, accepted: blind bless of a wrong type (the reviewed diff is the stop; a shape rewrite outside a migration PR is a review flag); a hand-written ? on a never-null column (nothing mechanical; bless never writes one the data does not justify, so it is visibly hand-made); aliasing every column to dodge the name spelling (legitimate, the types are still checked); a shape-only rebless in an issue_N case satisfying the Fix Regression Gate's added-body-line rule (a review flag).
  • Column names follow the executed spelling (p.name for an unaliased property); the compiler's inferred schema and lint --json still spell it name, and whether that folds into the executed spelling is a separate compiler decision, out of scope here.
  • Two comparators share one null-count walk rather than one comparator with two inputs: the shape check reports in .pg vocabulary against the author's lines, the computed check in Arrow vocabulary against the compiler's fields.
  • Engine footprint: one pub on executed_column_name, from_arrow on PropType and ScalarType; omnigraph-gqt gains arrow-schema (dep) and arrow-array (dev-dep).

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

@azimafroozeh azimafroozeh changed the title test(gq): add a mandatory --- expect shape section and a result-sch… test(gq): add a mandatory --- expect shape section and a result-schema check Sep 5, 2026
@azimafroozeh
azimafroozeh merged commit 50f3a6b into ModernRelay:main Sep 5, 2026
23 checks passed
@azimafroozeh
azimafroozeh deleted the 0134-gqt-executed-schema-check branch September 5, 2026 15:32
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.

1 participant