Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29ce718e51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Literal::Float(value) if !value.is_finite() => Err(CompilerError::typed( | ||
| T3, | ||
| "float literal must be finite and within the F64 range".to_string(), | ||
| )), |
There was a problem hiding this comment.
Adapt the NaN erasure fixture to the new validation
When the canonical compiler tests run, ir::erase::tests::every_expression_variant_preserves_its_fields feeds Literal::Float(f64::from_bits(0x7ff8_0000_0000_0001)) (NaN) into literal_type(...).unwrap() at crates/omnigraph-compiler/src/ir/erase.rs:461-475. This new arm makes that call return Err, so the test deterministically panics and the workspace CI test graph cannot pass; update the fixture/helper to assign the forged literal's F64 type without unwrapping this validator (or otherwise adapt the test) as part of this change.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| Literal::Float(value) if !value.is_finite() => Err(CompilerError::typed( | ||
| T3, | ||
| "float literal must be finite and within the F64 range".to_string(), | ||
| )), |
There was a problem hiding this comment.
Validate nearest vector elements before the dimension shortcut
When a non-finite value appears inside a correctly sized literal vector used by nearest, e.g. order { nearest($d.embedding, [<oversized decimal>.0, 0.0]) }, the special case at typecheck.rs:2285-2300 checks only dimension and returns before this recursive literal_type arm is reached. Consequently typecheck_query_decl reports success, so both the stored-query boot/offline check (crates/omnigraph-server/src/queries.rs:262-266) and cluster config validation (crates/omnigraph-cluster/src/config.rs:1321-1327) admit a broken query; it is rejected only later when lower_query re-types the child at invocation. Validate the vector elements before that early return.
Useful? React with 👍 / 👎.
Closes #920.
Oversized decimal float literals parse as non-finite
f64values and currently fail later with a generic range error. Reject them during query type checking with stable T3 and add an issue-named GQT regression case.