fix(compiler): normalize projected search order keys

[azimafroozeh]

What & why

Secondary search ordering rejected return aliases after nearest(...) and failed when a returned rank expression was repeated directly in order.

• Withdraw T18, allowing return aliases after nearest(...).
• Normalize secondary order expressions to the score references used by return projections. Preserve the leading search expression for retrieval planning.

Backing issue / RFC

• None; fixes existing return-alias and expression binding for secondary search keys.

Checklist

• Focused: bind secondary search keys consistently to returned values.
• Tests: GQT covers alias tie-breaking, expression and alias keys for nearest, bm25 and rrf, recursive Boolean expressions, nullable results, and repeated raw scores. Expression and alias cases run on engine and server.
• Public docs: docs/user/queries/index.md already describes secondary aliases and returned expressions.
• Release note: updated changelog.d/nearest-alias-order-key.changed.md.
• Invariants: reviewed docs/dev/invariants.md; typed query semantics remain intact, no Hard Invariant weakened, no deny-list item hit.

Local verification

The workspace run needed two environment corrections: the benchmark and engine integration binaries passed with CI's RUST_MIN_STACK=16777216; the three CLI session-cache failures passed when the fixture tests could create their OS session locks outside the sandbox.

The disabled Rust-string vocabulary audit stops on stale and missing inventory entries at the merge base. The OpenAPI audit and documentation checker pass.

Notes for reviewers

T18 is withdrawn from the diagnostic catalog; its number remains reserved.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[ragnorc]

Approve at db08fbacb3a66eaa06771e19c39371c97f9fbc53. I found no blocking defect or optional change to request.

This PR makes search queries obey the existing rule for secondary order keys. A caller can return a value, then sort by its alias or repeat its expression. Before this fix, nearest rejected aliases with T18. Repeated score expressions could pass type checking but fail during execution. The PR removes that obsolete refusal and gives secondary keys the same normalization as returned values. The leading search call still selects retrieval. Users can now use these query forms without changing their schema or rebuilding data.

The fix addresses the cause at the compiler layer. Order-key lowering preserves the first rank call and routes other keys through the existing projection lowering. That closes the mismatch for bare nearest, bm25, and rrf scores and for scores inside comparisons, not, is null, in, and nullable Boolean expressions. T33 still requires a projected rank call to match the leading retrieval. T42 still checks that other expressions belong to the return list. The planner can therefore bind the normalized expression to its executed result column. This satisfies the documented search-order contract.

The tradeoff is favorable. The compiler must distinguish retrieval from a score used as a value. It now makes that distinction at one existing boundary. It adds no public API, stored state, execution operator, cache, or storage invariant. It removes a special refusal and avoids a second expression evaluator. Five similar changes can reuse the same normalization rule instead of adding separate sort exceptions. The larger test diff buys observable coverage; the liability reduction comes from shared semantics, not line count.

For workloads that combine ranked retrieval with displayed scores, Boolean flags, or name tie-breaks, this fixes correctness without adding another retrieval pass. Existing candidate limits and ranking costs remain. I made no latency or throughput measurement.

I traced the common query compilation door, planner binding, execution sort keys, score projection, and RRF output. I also checked the complete relevant Lance search documentation against the pinned Lance 11.0.0 source (ab6b5bbe46009ed78746b444df8db59a8bc5d842). Its scanner supplies F32 distance and relevance columns; OmniGraph owns the F64 fused score. This change consumes those existing values. It changes neither index coverage nor storage publication. The score/type regressions remain useful guards for future Lance upgrades; current upstream documentation is not evidence that a newer capability exists in the pinned version.

I surveyed the other 32 open PRs. This fits the search contract work in #793, shared query vocabulary in #805, plan acceptance in #839, and projected-value ordering needed by #957. It adds no competing binding mechanism. I did not build combinations of those branches, so this is an assessment from code and scope, not integration-test evidence.

Validation

I used a clean detached checkout and copied the three checked-in GQT files from the reviewed head outside it. I ran the same, byte-identical files on both commits:

• Before: d334dc3c0510481a2484ad0ad0b65d1b23c99d8c (the merge base).
• After: db08fbacb3a66eaa06771e19c39371c97f9fbc53 (the reviewed head).

GQT case	Before	After
nearest_alias_secondary_key	Step 1 fails with T18, the obsolete alias refusal	Pass, 1.84 s
rank_call_nested_in_a_boolean_projection	Step 3 fails: “the planned sort key is not a property or an alias”	Pass, all steps, 1.85 s
issue_640_search_score_projection	Step 1 fails with that same sort-key error	Pass, all steps, 2.49 s

These are behavioral failures, not compilation failures or empty test selections. The nearest alias case reverses the ID order for two equal-distance rows, so ignoring the secondary key cannot pass. The other cases cover raw scores, aliases, recursive expressions, nulls, and result types.

Build command at each commit:

RUSTUP_TOOLCHAIN=1.97.1 CARGO_TARGET_DIR=/tmp/review-928/target \
RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable' RUSTC_WRAPPER= \
cargo build -p omnigraph-gqt --locked --bin omnigraph-gqt

The test command was the following for each row. CASE is the table's case name; OUT was /tmp/review-960/before-alias, before-expression, or before-score for the three before runs, and /tmp/review-960/head-verified-$CASE for each after run.

/tmp/review-928/target/debug/omnigraph-gqt \
  /tmp/review-960/regressions/$CASE.gqt \
  --target omnigraph-engine --artifacts "$OUT"

I rebuilt the GQT runner after finding stale commit metadata in the shared cache. All six final reports identify the correct source revision. An earlier local run of the Boolean case hit its 10-second budget once; an unchanged retry passed. No timeout was counted as the before-fix proof. No test source or timeout was changed. The checkout is clean and restored to the reviewed head.

CI Gate and GQ Logic Tests are green for this head. GQT's checkout log records GitHub's synthetic merge 9e54037d5b9e3a77ffe4e05db95ee0bcd4cc764a; I verified that its tree is identical to the reviewed head (ee5ecfd0951a9058f0517e20bf9b1c59b7ec6012). Local proof covers engine execution on local storage. These three cases declare only the engine target, so the green served CI job does not establish HTTP execution of these exact cases. I did not run local cloud, concurrency, or performance checks; the production change stays in the shared compiler path.