refactor: `eval_comparison` reintroduces the impossible-operator arm `OrderingOp` removed

[shunichironomura]

Warning

This content was written by an AI agent and must be verified by a human developer. After human verification, this alert may be removed.

Summary

crates/graphcal-eval/src/eval_expr/arithmetic.rs deliberately introduces a narrowed OrderingOp type to remove an "impossible operator" fallback — then routes quantity comparisons back through a function that re-widens to BinOp and needs exactly that fallback.

The stated design

arithmetic.rs:183-194:

Restriction of BinOp to the four ordering comparison operators.

Carrying this typed subset lets apply_ordering dispatch without an "impossible" arm — the type system forbids non-ordering ops at the call site rather than checking at runtime.

The path that undoes it

Both eval_equality_values (arithmetic.rs:141) and eval_ordering_values (arithmetic.rs:178) funnel the quantity case through eval_comparison(op: BinOp, ...):

#[expect(clippy::float_cmp, reason = "DSL equality uses exact comparison")]
fn eval_comparison(op: BinOp, l: f64, r: f64, ctx: &EvalContext<'_>, span: Span)
    -> Result<bool, GraphcalError>
{
    match op {
        BinOp::Eq => Ok(l == r),
        BinOp::Ne => Ok(l != r),
        BinOp::Lt => Ok(l < r),
        BinOp::Gt => Ok(l > r),
        BinOp::Le => Ok(l <= r),
        BinOp::Ge => Ok(l >= r),
        _ => Err(ctx.internal_error(format!("unexpected operator {op:?} in comparison"), span)),
    }
}

arithmetic.rs:220-238. The _ => arm is the runtime check OrderingOp exists to eliminate.

Impact

No wrong results — purely a type-safety and consistency gap. But it leaves an unreachable error path that must be kept alive and tested (see #1395, which tracks untested defensive internal-error guards in the same area).

Suggested fix

Widen the narrowed type to ComparisonOp { Eq, Ne, Lt, Gt, Le, Ge } and have both callers narrow once at entry. The fallback arm then disappears, matching the design the file already documents.

Provenance

Found during a full-workspace code review at 6e462341a (v0.0.1-alpha.27). Baseline at that commit: cargo clippy --workspace --all-targets clean, cargo test --workspace 2926 passed / 0 failed, cargo deny check advisories ok.