JSTPRV-166 conformance harness#285
Open
millioner wants to merge 23 commits into
Open
Conversation
WalkthroughAdds a new ONNX conformance harness crate with generators, fixtures, ONNX model builder, runner, tolerance logic, tests, and CI; updates Rust nightly to nightly-2025-06-20 across CI and toolchain; adjusts several EvalResult signatures to include a lifetime; removes crate-level AVX512 feature gates; plus small conditional/formatting refactors. Changes
Sequence Diagram(s)sequenceDiagram
participant TestHarness as TestHarness
participant Generator as Generator
participant OnnxBuilder as OnnxBuilder
participant Runner as ConformanceRunner
participant Tract as TractReference
participant JSTProve as JSTProve
participant Tolerance as Tolerance
TestHarness->>Generator: request TestCase(s)
activate Generator
Generator->>OnnxBuilder: build_single_op_model(...)
activate OnnxBuilder
OnnxBuilder-->>Generator: onnx_bytes
deactivate OnnxBuilder
Generator-->>TestHarness: TestCase(onnx_bytes, inputs, tolerance)
deactivate Generator
TestHarness->>Runner: run(TestCase)
activate Runner
Runner->>Tract: execute ONNX reference
activate Tract
Tract-->>Runner: flattened i64 reference outputs
deactivate Tract
Runner->>JSTProve: compile + execute jstprove
activate JSTProve
JSTProve-->>Runner: flattened i64 jstprove outputs
deactivate JSTProve
Runner->>Tolerance: check(expected, actual) per element
activate Tolerance
Tolerance-->>Runner: pass/fail
deactivate Tolerance
alt all within tolerance
Runner-->>TestHarness: Pass
else mismatches
Runner-->>TestHarness: Fail(mismatches)
else backend error
Runner-->>TestHarness: Error
end
deactivate Runner
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
rust/jstprove_remainder/tests/lenet_dump.rs (1)
33-33: Consider applying inline format capture consistently throughout the function.Line 33 now uses modern inline format capture (
{k},{v:?}), but otherprintln!statements in the same function (lines 10, 14, 18-24, 28-30, 40-46) still use the older explicit argument style. For better consistency and readability, consider updating all format strings in this test to use inline capture.♻️ Suggested refactor for consistency
fn test_dump_lenet_structure() { let parsed = parser::parse_onnx(Path::new("models/lenet.onnx")).unwrap(); println!("\n=== ONNX inputs ==="); for inp in &parsed.inputs { - println!(" {} shape={:?}", inp.name, inp.shape); + println!(" {} shape={:?}", inp.name, inp.shape); } println!("=== ONNX outputs ==="); for out in &parsed.outputs { - println!(" {} shape={:?}", out.name, out.shape); + println!(" {} shape={:?}", out.name, out.shape); } println!("=== Initializers ==="); for (name, td) in &parsed.initializers { - println!( - " {} dims={:?} floats={} ints={}", - name, - td.dims, - td.float_data.len(), - td.int_data.len() - ); + println!( + " {name} dims={:?} floats={} ints={}", + td.dims, + td.float_data.len(), + td.int_data.len() + ); } println!("=== Nodes ==="); for node in &parsed.nodes { - println!( - " [{}] op={} inputs={:?} outputs={:?}", - node.name, node.op_type, node.inputs, node.outputs - ); + println!( + " [{}] op={} inputs={:?} outputs={:?}", + node.name, node.op_type, node.inputs, node.outputs + ); for (k, v) in &node.attributes { println!(" attr {k}: {v:?}"); } } let graph = LayerGraph::from_parsed(&parsed).unwrap(); println!("=== Topo order ==="); for layer in graph.iter_topo() { - println!(" [{}] {} ({:?})", layer.id, layer.name, layer.op_type); - println!(" inputs: {:?}", layer.inputs); - println!(" outputs: {:?}", layer.outputs); - println!( - " weights: {:?}", - layer.weights.keys().collect::<Vec<_>>() - ); + println!(" [{}] {} ({:?})", layer.id, layer.name, layer.op_type); + println!(" inputs: {:?}", layer.inputs); + println!(" outputs: {:?}", layer.outputs); + println!( + " weights: {:?}", + layer.weights.keys().collect::<Vec<_>>() + ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/jstprove_remainder/tests/lenet_dump.rs` at line 33, The test's println! calls are mixed between old explicit-argument formatting and the newer inline capture used in the line with `println!(" attr {k}: {v:?}");`; update all other println! invocations in the same test function (the ones printing inputs, layers, shapes, params, etc.) to use inline format capture (e.g., "{name}" or "{var:?}") instead of positional/explicit arguments so formatting is consistent across the function; look for every println! in the test (including prints for `name`, `shape`, `params`, `bias`, `weights`, and similar variables) and replace their format strings to reference the local identifiers inline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust/jstprove_circuits/src/circuit_functions/layers/pad.rs`:
- Around line 261-276: The current check only validates constant_value when
get_w_or_b::<f64, _>(...) succeeds, allowing integer initializers or unreadable
named initializers to bypass validation; change the pad input[2] handling so
that if a constant_value name is present (layer.inputs.get(2) non-empty) you
must attempt to load the initializer and reject the layer unless you can confirm
every element is exactly zero for any stored numeric type—i.e., try to read the
initializer from layer_context.w_and_b_map for both float and integer
representations (or inspect the stored value generically) and if loading fails
or any element is non-zero return Err(LayerError::Other { layer: LayerKind::Pad,
msg: ... }) instead of silently assuming zero; update the block using
get_w_or_b, cv_name, LayerError::Other and LayerKind::Pad to implement this
stricter validation.
In `@rust/jstprove_conformance/src/fixtures.rs`:
- Around line 640-650: The comment above the x_vals vector incorrectly states
"Distinct values" while x_vals contains ALPHA twice (and the sorted list shows
two 1s); update the comment to accurately reflect that values include a
duplicate (e.g., remove "Distinct" or state "Contains a duplicate to test tie
behavior") or change the data in x_vals to make all entries unique; locate the
x_vals declaration in fixtures.rs (symbol: x_vals) and either edit the comment
text or replace one ALPHA occurrence so the data and comment are consistent.
- Around line 235-277: The test fixture f05_div_near_zero_divisor is using a
trivial divisor of all ones so it never exercises truncation/remainder logic;
change the Initializer passed to build_single_op_model (Initializer { name: "B",
dims: vec![8], data: vec![... ] }) to use a non-trivial constant divisor pattern
(e.g. include 2 or 3 values such as vec![2,2,2,2,2,2,2,2] or a mixed small
divisor set) and keep the dividend vector (dividend: Vec<i64> = vec![7, -7, 3,
-3, 1, -1, 0, 100]) so negative divisions like -7/2 reveal
truncation-vs-euclidean differences; also update the failure_description string
if needed to reflect the new divisor choice.
In `@rust/jstprove_conformance/src/generator/builder.rs`:
- Around line 50-57: The sampled_shapes generation currently samples each input
independently; change the .scan state from () to Option<Vec<usize>> so the first
input's sampled shape is stored and returned/cloned for subsequent inputs when
the spec requires same-shape binary ops (e.g., elementwise/comparison cases).
Concretely, replace spec.inputs.iter().scan((), ...) with
spec.inputs.iter().scan(None, |state: &mut Option<Vec<usize>>, t| { if
state.is_none() { *state = Some(t.shape.sample(&mut rng)); }
Some(state.as_ref().unwrap().clone()) }) so later code that calls
compute_output_shape sees the same sampled shape for all inputs. Ensure the code
still samples independently for specs that do not require same-shape semantics
by guarding the scan behavior behind the same-shape condition if one exists on
spec.
In `@rust/jstprove_conformance/src/generator/cases_float.rs`:
- Around line 924-942: The test builds a TopK model that declares both "values"
and "indices" but the harness/runner compares all tract outputs while JSTProve
only returns the values, causing a mismatch; fix by making the test only declare
the single output you intend to compare (remove "indices" from the outputs list
in this TopK case built by build_single_op_model) or alternatively update the
comparison logic in TestCase/ConformanceRunner to select/filter outputs by name
or expected dtype (e.g., only compare the "values" output / FLOAT outputs) so
multi-output operators like TopK are not compared against outputs JSTProve does
not produce.
In `@rust/jstprove_conformance/src/generator/op_specs.rs`:
- Around line 154-189: The docstring for split_initializers is incorrect: it
claims the function returns dynamic input names but the implementation returns
positional indices (see split_initializers, init_indices and dynamic_indices)
and callers like build_single_op_model expect indices; update the function
documentation to state the second tuple element is Vec<usize> of non-initializer
input indices (rename the doc token from "dynamic_input_names" to
"dynamic_input_indices" and explain they are positional indices used for shape
lookup), or alternatively change the return to actually provide names
(Vec<String>/Vec<&str>) throughout split_initializers, Initializer construction,
and any call sites such as build_single_op_model—pick one approach and make the
docs and return type consistent.
In `@rust/jstprove_conformance/src/runner.rs`:
- Around line 245-275: The scalar (rank-0) case currently uses
Array1::from_vec(...).into_dyn(), producing a 1-D tensor; update the three
branches in the function that builds tensors (the BOOL branch that creates
`bools`, the FLOAT branch that creates `floats` with ALPHA scaling, and the
INT64 branch that uses `vals`) so that when `shape.is_empty()` you construct a
rank-0 ndarray via `tract_ndarray::arr0(...)` (using `bools[0]`, `floats[0]`,
and `vals[0]` respectively) and convert that to a Tensor, otherwise keep the
existing from_shape_vec logic; ensure you still handle reshape errors the same
way in the non-scalar branches.
In `@rust/jstprove_conformance/src/tolerance.rs`:
- Around line 36-67: Add a regression test named something like
extreme_value_wraparound that uses integer boundary values to ensure no
overflow/wraparound lets mismatches pass: e.g. assert that
Tolerance::EXACT.check(i64::MIN, i64::MAX) is false, and then construct a
Tolerance (use the Tolerance struct and its check method) with a very large abs
tolerance that legitimately should accept those extremes and assert it returns
true; this verifies distance is computed safely (no wrapping) and that both
failing and succeeding behaviors at extremes are covered.
- Around line 20-28: In check, avoid signed overflow by computing delta as an
unsigned difference instead of subtracting i64s and calling unsigned_abs;
replace `let delta = (expected - actual).unsigned_abs() as i64;` with a branch
that produces an unsigned delta (e.g. `let delta_u = if expected >= actual {
(expected - actual) as u64 } else { (actual - expected) as u64 };`), then
compare `delta_u` against `self.abs` cast to u64, and use `delta_u` (cast to
f64) for the relative check using `expected.unsigned_abs()` (u64) so no signed
wraparound can occur; keep the rest of the logic in `check` but use these
unsigned-safe values for both the absolute (`self.abs`) and relative
(`self.rel`) comparisons.
In `@rust/jstprove_conformance/tests/conformance.rs`:
- Around line 66-75: The current TestResult::Error branch incorrectly treats any
case where ref_runner.run(case) returns Pass as a "reference_only" skip; change
this to only skip when the failing case is explicitly known as a reference-only
placeholder/unsupported op by adding a predicate (e.g.,
is_known_reference_only_placeholder(op_name) or a case-level placeholder flag)
and use that to gate the skip logic around reference_only_skipped and the log
(instead of directly calling ref_runner.run(case)); update the branch that
currently checks ref_runner.run(case) to call the new predicate on case.op_name
(or case.placeholder flag) and fall through to the error handling path for all
other errors to avoid hiding real JSTProve regressions.
---
Nitpick comments:
In `@rust/jstprove_remainder/tests/lenet_dump.rs`:
- Line 33: The test's println! calls are mixed between old explicit-argument
formatting and the newer inline capture used in the line with `println!("
attr {k}: {v:?}");`; update all other println! invocations in the same test
function (the ones printing inputs, layers, shapes, params, etc.) to use inline
format capture (e.g., "{name}" or "{var:?}") instead of positional/explicit
arguments so formatting is consistent across the function; look for every
println! in the test (including prints for `name`, `shape`, `params`, `bias`,
`weights`, and similar variables) and replace their format strings to reference
the local identifiers inline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f081a95-e125-459a-8ba7-542d47d32a3e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (48)
.github/workflows/conformance.yml.github/workflows/e2e-tests.yml.github/workflows/release.yml.github/workflows/unit-integration-tests.ymlCargo.tomlcompiler/expander_compiler/src/circuit/ir/dest/mod.rscompiler/expander_compiler/src/circuit/ir/hint_less/mod.rscompiler/expander_compiler/src/circuit/ir/hint_normalized/mod.rscompiler/expander_compiler/src/circuit/ir/source/mod.rsprover/arith/babybear/src/lib.rsprover/arith/gf2/src/lib.rsprover/arith/gf2_128/src/lib.rsprover/arith/goldilocks/src/lib.rsprover/arith/mersenne31/src/lib.rsprover/arith/src/lib.rsprover/gkr/src/lib.rsrust-toolchain.tomlrust/jstprove_circuits/bin/jstprove.rsrust/jstprove_circuits/src/circuit_functions/gadgets/autotuner.rsrust/jstprove_circuits/src/circuit_functions/hints/gelu.rsrust/jstprove_circuits/src/circuit_functions/hints/pow.rsrust/jstprove_circuits/src/circuit_functions/layers/clip.rsrust/jstprove_circuits/src/circuit_functions/layers/pad.rsrust/jstprove_circuits/src/circuit_functions/layers/slice.rsrust/jstprove_circuits/src/circuit_functions/layers/topk.rsrust/jstprove_circuits/src/circuit_functions/utils/onnx_model.rsrust/jstprove_circuits/src/runner/holographic.rsrust/jstprove_circuits/src/runner/main_runner.rsrust/jstprove_conformance/CONTRIBUTING.mdrust/jstprove_conformance/Cargo.tomlrust/jstprove_conformance/src/fixtures.rsrust/jstprove_conformance/src/generator/builder.rsrust/jstprove_conformance/src/generator/cases_float.rsrust/jstprove_conformance/src/generator/cases_int.rsrust/jstprove_conformance/src/generator/mod.rsrust/jstprove_conformance/src/generator/op_specs.rsrust/jstprove_conformance/src/generator/shapes.rsrust/jstprove_conformance/src/generator/values.rsrust/jstprove_conformance/src/lib.rsrust/jstprove_conformance/src/onnx_builder.rsrust/jstprove_conformance/src/runner.rsrust/jstprove_conformance/src/tolerance.rsrust/jstprove_conformance/src/tolerance_table.mdrust/jstprove_conformance/tests/conformance.rsrust/jstprove_onnx/src/quantizer.rsrust/jstprove_pyo3/src/lib.rsrust/jstprove_remainder/tests/e2e_tests.rsrust/jstprove_remainder/tests/lenet_dump.rs
💤 Files with no reviewable changes (7)
- prover/arith/mersenne31/src/lib.rs
- prover/arith/babybear/src/lib.rs
- prover/arith/src/lib.rs
- prover/arith/gf2/src/lib.rs
- prover/arith/goldilocks/src/lib.rs
- prover/arith/gf2_128/src/lib.rs
- prover/gkr/src/lib.rs
… validation, correct TopK multi-output comparison, and add same_shape_inputs to test builder
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
rust/jstprove_conformance/tests/conformance.rs (1)
66-81:⚠️ Potential issue | 🟠 MajorSame concern still open:
TestResult::Erroris skipped for any op whose tract path is healthy.This gate only checks that tract passes — which is true for virtually every valid ONNX model, not just reference-only placeholders. A real JSTProve compile/witness regression on Add, Conv, MatMul, etc. will be swallowed into
reference_only_skippedwith aninfo!log and the group will still report green.Either add an explicit placeholder flag to
TestCase(parallelingRegressionFixture::allow_jstprove_error) or gate on a narrow allowlist of unimplemented ops (InstanceNormalization,GroupNormalization,Mul/LeakyRelu/HardSwishper the Group B comment). Otherwise this test harness cannot fail on JSTProve regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/jstprove_conformance/tests/conformance.rs` around lines 66 - 81, The current TestResult::Error branch wrongly treats any case whose ref_runner.run(case) returns Pass as a "reference_only" skip; change the gate to use an explicit marker instead: add a boolean placeholder flag (e.g., reference_only or allow_jstprove_error) to the TestCase struct (mirroring RegressionFixture::allow_jstprove_error) or implement a narrow op allowlist of unimplemented ops, then replace the ref_runner.run(case) check with a direct check of that new flag (or membership in the allowlist) before incrementing reference_only_skipped and logging; otherwise panic as before. Ensure you update places constructing TestCase to set the flag appropriately (or populate the allowlist) and keep the log and panic branches unchanged except for the new condition.
🧹 Nitpick comments (2)
rust/jstprove_conformance/src/generator/builder.rs (1)
175-261: Optional: consolidate candidate construction and treatErroroutcomes inshrink.Three near-identical
TestCase { inputs: ..., onnx_bytes: best.onnx_bytes.clone(), op_name, seed, tolerance, ignore_extra_reference_outputs: false }blocks. Also:
ignore_extra_reference_outputsis hard-coded tofalse, silently dropping the flag fromoriginal— if a failing TopK case entersshrink, the candidate re-evaluation will trip the length-mismatch path instead of the real mismatch. Propagatebest.ignore_extra_reference_outputsthrough each candidate.is_still_failingtreatsTestResult::Erroras "not failing". If mutating an input flipsFail → Error, the reduced case silently loses the signal. Consider also preservingError-producing candidates (or at least halting phase).♻️ Proposed helper + flag propagation
+fn with_inputs(best: &TestCase, inputs: Vec<Vec<i64>>) -> TestCase { + TestCase { + inputs, + onnx_bytes: best.onnx_bytes.clone(), + op_name: best.op_name, + seed: best.seed, + tolerance: best.tolerance.clone(), + ignore_extra_reference_outputs: best.ignore_extra_reference_outputs, + } +}Then each phase becomes
let candidate = with_inputs(&best, new_inputs);.rust/jstprove_conformance/src/tolerance.rs (1)
3-32: Minor: negativeabssilently accepts every delta.
self.abs: i64→self.abs as u64(line 24) wraps a negative value to a huge unsigned, which makescheckalways returntrue. All current constructors use non-negativeabs, so this is only a defensive concern, but sinceabsispub i64any externalTolerance { abs: -1, .. }would be a silent footgun.🛡️ Proposed guard
- let delta_u = expected.abs_diff(actual); - if delta_u <= self.abs as u64 { + let delta_u = expected.abs_diff(actual); + let abs_bound = self.abs.max(0) as u64; + if delta_u <= abs_bound { return true; }Alternatively, change
pub abs: i64topub abs: u64to make the invariant type-enforced.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/jstprove_conformance/src/tolerance.rs` around lines 3 - 32, The bug is that casting a negative pub field Tolerance::abs (i64) to u64 makes large unsigned values and causes check() to accept any delta; fix by changing Tolerance.abs from i64 to u64 (update the struct declaration, the EXACT constant to use 0u64, and the check method to compare delta_u <= self.abs without casting). If you prefer to avoid the breaking type change, add a defensive guard in check (e.g. if self.abs < 0 { return false; } before casting) so negative abs values do not wrap to huge unsigned values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust/jstprove_conformance/src/generator/op_specs.rs`:
- Around line 86-158: The Gemm spec currently allows independent sampling of
dims so A and B can have mismatched contraction dims; in gemm_spec update the
TensorSpec ShapeSpec for A, B, and C to pin each axis to the exact sizes (use
min_dim = max_dim = m/k/n as appropriate) and update max_elements to the exact
product ((m*k) for A, (k*n) for B, (n) for C) so that A is [M,K], B is [K,N],
and C is [N]; change the ShapeSpec for A (min_dim/max_dim -> m and k per axis),
for B (k and n), and for C (n) to ensure valid contraction during
build_single_op_model and compute_output_shape.
In `@rust/jstprove_conformance/src/runner.rs`:
- Around line 99-120: The failure path computes delta with (ref_val -
jst_val).unsigned_abs() as i64 which can overflow; change the arithmetic to use
abs_diff (e.g., ref_val.abs_diff(jst_val)) and store delta in an unsigned wider
type by updating Failure::delta to u64 (or otherwise saturate into u64) so
mismatches never panic or wrap; update the element-wise loop in runner.rs to
compute delta via abs_diff and push a Failure with the u64 delta, and apply the
same fix to the synthetic length-mismatch Failure (use abs_diff on the two usize
lengths and store that in Failure::delta as u64).
- Around line 254-283: The scalar branch unconditionally indexes into bools[0],
floats[0], and vals[0] when shape.is_empty(), which panics if vals is empty; add
a guard in the code paths that handle scalar inputs (check if vals.is_empty()
when shape.is_empty()) and return a clean error instead of panicking (e.g.
convert to a TestResult::Error or return Err(anyhow!(...)) with a clear
message). Apply this check before constructing arr in the BOOL (bools), FLOAT
(floats) and INT64 (vals) branches so empty scalar inputs are reported as errors
rather than causing a panic.
---
Duplicate comments:
In `@rust/jstprove_conformance/tests/conformance.rs`:
- Around line 66-81: The current TestResult::Error branch wrongly treats any
case whose ref_runner.run(case) returns Pass as a "reference_only" skip; change
the gate to use an explicit marker instead: add a boolean placeholder flag
(e.g., reference_only or allow_jstprove_error) to the TestCase struct (mirroring
RegressionFixture::allow_jstprove_error) or implement a narrow op allowlist of
unimplemented ops, then replace the ref_runner.run(case) check with a direct
check of that new flag (or membership in the allowlist) before incrementing
reference_only_skipped and logging; otherwise panic as before. Ensure you update
places constructing TestCase to set the flag appropriately (or populate the
allowlist) and keep the log and panic branches unchanged except for the new
condition.
---
Nitpick comments:
In `@rust/jstprove_conformance/src/tolerance.rs`:
- Around line 3-32: The bug is that casting a negative pub field Tolerance::abs
(i64) to u64 makes large unsigned values and causes check() to accept any delta;
fix by changing Tolerance.abs from i64 to u64 (update the struct declaration,
the EXACT constant to use 0u64, and the check method to compare delta_u <=
self.abs without casting). If you prefer to avoid the breaking type change, add
a defensive guard in check (e.g. if self.abs < 0 { return false; } before
casting) so negative abs values do not wrap to huge unsigned values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b090a781-c04c-4e53-9b08-56206128fdc1
📒 Files selected for processing (10)
rust/jstprove_circuits/src/circuit_functions/layers/pad.rsrust/jstprove_conformance/src/fixtures.rsrust/jstprove_conformance/src/generator/builder.rsrust/jstprove_conformance/src/generator/cases_float.rsrust/jstprove_conformance/src/generator/cases_int.rsrust/jstprove_conformance/src/generator/op_specs.rsrust/jstprove_conformance/src/runner.rsrust/jstprove_conformance/src/tolerance.rsrust/jstprove_conformance/tests/conformance.rsrust/jstprove_remainder/tests/lenet_dump.rs
✅ Files skipped from review due to trivial changes (2)
- rust/jstprove_remainder/tests/lenet_dump.rs
- rust/jstprove_circuits/src/circuit_functions/layers/pad.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- rust/jstprove_conformance/src/fixtures.rs
- rust/jstprove_conformance/src/generator/cases_int.rs
…alar empty guard, allow_jstprove_error flag, u64 tolerance
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
rust/jstprove_conformance/tests/conformance.rs (2)
38-38: Stale "reference_only" naming — these are nowallow_jstprove_errorskips.Since the Error branch no longer runs
ref_runnerto decide whether to skip (good — matches the flag-based design now), the variablereference_only_skippedand the summary message "(reference_only placeholders)" are misleading. A grepping engineer looking for a genuine reference-only skip will land here and be confused. Rename tojstprove_errors_skipped/ "(allow_jstprove_error)" so the log and the flag line up.♻️ Proposed rename
- let mut reference_only_skipped = 0usize; + let mut jstprove_errors_skipped = 0usize; @@ - if case.allow_jstprove_error { - reference_only_skipped += 1; + if case.allow_jstprove_error { + jstprove_errors_skipped += 1; @@ - if reference_only_skipped > 0 { - eprintln!( - "[{group_name}] {reference_only_skipped} case(s) skipped (reference_only placeholders)" - ); - } + if jstprove_errors_skipped > 0 { + eprintln!( + "[{group_name}] {jstprove_errors_skipped} case(s) skipped (allow_jstprove_error)" + ); + }Also applies to: 65-65, 81-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/jstprove_conformance/tests/conformance.rs` at line 38, The variable name reference_only_skipped is stale and should be renamed to jstprove_errors_skipped and the summary text "(reference_only placeholders)" changed to "(allow_jstprove_error)"; update every occurrence (declaration, increments, and the final summary/print block in conformance.rs) so increments and logging use jstprove_errors_skipped and the summary string reflects "(allow_jstprove_error)" to match the flag-based behavior and avoid confusion.
40-62:shrinkruns on every failure even in non–fail-fast mode, potentially blowing up runtime.When
CONFORMANCE_FAIL_FAST=0(used to enumerate all regressions locally), every failing case triggers up to 50 extra runner invocations inshrink(line 54). For a genuinely broken op where most generated cases fail, that is50 × Nextra tract+JSTProve runs, each of which recompiles the circuit. Consider either (a) only shrinking the first failure per op_name, or (b) skipping shrink entirely when!fail_fast. Not a correctness issue — just a UX/perf cliff when debugging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/jstprove_conformance/tests/conformance.rs` around lines 40 - 62, The current loop calls shrink(case, &full_runner) for every failing case which can hugely increase runtime; change it to only run shrink when either fail_fast is true or this is the first failure for that operation name: introduce a local HashSet (e.g. shrunk_ops) before iterating cases, and inside the TestResult::Fail arm only call shrink and print "minimized inputs" if fail_fast || !shrunk_ops.contains(&case.op_name); when you do run shrink insert case.op_name into shrunk_ops so subsequent failures for the same op_name skip shrinking.rust/jstprove_conformance/src/generator/builder.rs (1)
82-89: Hard-coded INT64 element type limitsTestCaseBuilderto INT64 ops.The builder pins both dynamic input
elem_type(line 87) and the synthesised output("output", &output_shape, 7)(line 108) to7(INT64). FLOAT-typed ops (those incases_float.rsrelying on α-scaling) cannot be produced through this builder at all; they must be hand-constructed viabuild_single_op_model{_ordered}. Worth surfacing the element type onOpInputSpec(per-input and per-output) so the builder can grow to support FLOAT specs without a parallel code path. Not blocking — just calling out the constraint so it's intentional.Also applies to: 108-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/jstprove_conformance/src/generator/builder.rs` around lines 82 - 89, The builder currently hard-codes elem_type = 7 (INT64) in the input_shapes construction and the synthesized output tuple, which prevents building FLOAT cases; update the data model and builder to read per-input/output element types instead of using 7. Add an elem_type field to OpInputSpec (and the corresponding output spec), propagate that through spec.inputs and spec.outputs, and replace the hard-coded 7 in the input_shapes map (the closure that builds (spec.inputs[i].name, dims, 7i32)) and the output tuple ("output", &output_shape, 7) to use the specified elem_type value from the spec for each item. Ensure types remain i32 where required and fallback to a default if elem_type is absent for backward compatibility.rust/jstprove_conformance/src/runner.rs (2)
125-142: Add parentheses to the length-check condition for clarity.
!(lengths_match || ref_longer && case.ignore_extra_reference_outputs)is parsed as!(lengths_match || (ref_longer && ignore)), which is the intended behaviour, but mixing||and&&without parens inside a negation is the kind of thing that gets silently broken by a future tweak. A quick explicit grouping makes intent obvious.♻️ Proposed fix
- if !(lengths_match || ref_longer && case.ignore_extra_reference_outputs) { + if !(lengths_match || (ref_longer && case.ignore_extra_reference_outputs)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/jstprove_conformance/src/runner.rs` around lines 125 - 142, The conditional combining lengths_match, ref_longer and case.ignore_extra_reference_outputs is ambiguous; replace the current check in runner.rs with an explicit grouping so it reads !(lengths_match || (ref_longer && case.ignore_extra_reference_outputs)) to make the intended precedence clear; update the if condition that currently uses lengths_match and ref_longer to use the parenthesized form in the error/Failure push block.
427-439: Minor: per-input intermediateVecallocation insideflat_map.
v.iter().map(move |&x| x as f64 / scale).collect::<Vec<_>>()materialises a temporaryVec<f64>for each input tensor only to immediately flatten it. For small conformance inputs this is negligible, but you can keep the iterator lazy:♻️ Proposed fix
- .flat_map(|(idx, v)| { - let is_float = input_info - .get(idx) - .map(|i| i.elem_type == 1) - .unwrap_or(false); - let scale = if is_float { ALPHA } else { 1.0 }; - v.iter().map(move |&x| x as f64 / scale).collect::<Vec<_>>() - }) + .flat_map(|(idx, v)| { + let is_float = input_info + .get(idx) + .map(|i| i.elem_type == 1) + .unwrap_or(false); + let scale = if is_float { ALPHA } else { 1.0 }; + v.iter().map(move |&x| x as f64 / scale) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/jstprove_conformance/src/runner.rs` around lines 427 - 439, The per-input closure in the construction of activations currently collects each mapped iterator into a temporary Vec via .collect::<Vec<_>>(), causing unnecessary allocations; change the closure passed to flat_map to return the iterator directly (remove the inner .collect call) so flat_map can lazily flatten the per-input iterators — i.e., inside the flat_map over case.inputs use v.iter().map(move |&x| x as f64 / scale) (with the same is_float/scale logic using input_info and ALPHA) and rely on the final .collect() into activations: Vec<f64>.rust/jstprove_conformance/src/generator/cases_float.rs (1)
510-512: CI truncation drops later cases — consider ordering by coverage priority.Under the
cifeature,default_case_count()returns 5, so only the first 5 entries of eachVec<TestCase>run in CI (perrust/jstprove_conformance/src/generator/mod.rs). Inrescaling_casesthat means CI currently runs Gemm×2, MatMul, Conv×2 and never exercises ConvTranspose, BatchNorm, Div, or Pow in the CI loop. Same concern applies totranscendental_cases(Erf/Sqrt/later ops dropped) andspatial_cases. Worth either interleaving cases so each op gets at least one CI slot, or making the truncation op-aware (e.g. "first case per distinctop_name") so CI retains baseline coverage of every op.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/jstprove_conformance/src/generator/cases_float.rs` around lines 510 - 512, CI truncation via cases.truncate(default_case_count()) drops later ops; modify the generator functions (e.g., rescaling_cases, transcendental_cases, spatial_cases) so truncation preserves at least one case per distinct operation name (op_name) before applying default_case_count(): implement an op-aware selection step that groups TestCase entries by op_name and picks the first case from each group (or interleaves groups) to form a prioritized list, then apply cases.truncate(default_case_count()) to that prioritized list to ensure each op has baseline CI coverage while keeping the existing truncation mechanism.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust/jstprove_conformance/src/generator/builder.rs`:
- Around line 134-157: compute_output_shape currently returns the first dynamic
input shape for any non-"Gemm" op, which is only valid for element-wise ops and
will produce wrong ONNX output shapes for
MatMul/Conv/ConvTranspose/Reduce*/Pooling/Reshape etc.; update
compute_output_shape to explicitly handle each non-elementwise op your specs may
produce (e.g., implement MatMul using sampled_shapes[0] and sampled_shapes[1] →
[M,N], Conv/ConvTranspose using kernel/stride/pad attributes and input/weight
shapes, Reshape using the provided shape input, Reduce* using the reduced axes,
pooling using input dims and kernel/strides), and only fall back to
first_dynamic_shape for true element-wise ops (or replace the fallback with a
debug_assert!(is_elementwise(spec.op_name))/panic to fail fast). Reference
compute_output_shape, first_dynamic_shape, and spec.op_name when adding the
explicit match arms or the assert so future specs cannot silently produce
mismatched shapes.
In `@rust/jstprove_conformance/src/generator/shapes.rs`:
- Around line 77-89: The code currently clamps every sampled dimension with
d.max(lo), which undoes the singleton branch when min_dim > 1; update the logic
in the shape generation loop (the variables/function: shape, rank, lo =
self.min_dim, hi = self.max_dim, d, rng, self.allow_singleton) so that the
min_dim clamp is only applied when we did not choose the singleton branch —
i.e., if we picked the allow_singleton path (rng.gen_bool), keep d==1 even if
lo>1, otherwise sample/clip the random value and apply d = d.max(lo) (or
equivalent) to enforce min_dim for non-singleton cases.
In `@rust/jstprove_conformance/src/runner.rs`:
- Around line 325-343: The fallback branch mistakenly assumes an ONNX FLOAT
(onnx_elem_type == 1) can always be read with out.to_array_view::<f32>(), but
out.datum_type() may be a different float type (e.g., F16/BF16) so that call can
fail; update the else branch in the logic around out.datum_type(), output_info,
onnx_elem_type and out_idx to inspect the actual tract datum type (match on
out.datum_type() variants) and handle each float case explicitly (convert
F32/F64 and convert or cast F16/BF16 to f32/f64 before scaling by ALPHA), or
remove the onnx_elem_type == 1 special-case and instead use a generic conversion
path that reads the element as the produced datum type then applies the ALPHA
scaling and rounding into result.
---
Nitpick comments:
In `@rust/jstprove_conformance/src/generator/builder.rs`:
- Around line 82-89: The builder currently hard-codes elem_type = 7 (INT64) in
the input_shapes construction and the synthesized output tuple, which prevents
building FLOAT cases; update the data model and builder to read per-input/output
element types instead of using 7. Add an elem_type field to OpInputSpec (and the
corresponding output spec), propagate that through spec.inputs and spec.outputs,
and replace the hard-coded 7 in the input_shapes map (the closure that builds
(spec.inputs[i].name, dims, 7i32)) and the output tuple ("output",
&output_shape, 7) to use the specified elem_type value from the spec for each
item. Ensure types remain i32 where required and fallback to a default if
elem_type is absent for backward compatibility.
In `@rust/jstprove_conformance/src/generator/cases_float.rs`:
- Around line 510-512: CI truncation via cases.truncate(default_case_count())
drops later ops; modify the generator functions (e.g., rescaling_cases,
transcendental_cases, spatial_cases) so truncation preserves at least one case
per distinct operation name (op_name) before applying default_case_count():
implement an op-aware selection step that groups TestCase entries by op_name and
picks the first case from each group (or interleaves groups) to form a
prioritized list, then apply cases.truncate(default_case_count()) to that
prioritized list to ensure each op has baseline CI coverage while keeping the
existing truncation mechanism.
In `@rust/jstprove_conformance/src/runner.rs`:
- Around line 125-142: The conditional combining lengths_match, ref_longer and
case.ignore_extra_reference_outputs is ambiguous; replace the current check in
runner.rs with an explicit grouping so it reads !(lengths_match || (ref_longer
&& case.ignore_extra_reference_outputs)) to make the intended precedence clear;
update the if condition that currently uses lengths_match and ref_longer to use
the parenthesized form in the error/Failure push block.
- Around line 427-439: The per-input closure in the construction of activations
currently collects each mapped iterator into a temporary Vec via
.collect::<Vec<_>>(), causing unnecessary allocations; change the closure passed
to flat_map to return the iterator directly (remove the inner .collect call) so
flat_map can lazily flatten the per-input iterators — i.e., inside the flat_map
over case.inputs use v.iter().map(move |&x| x as f64 / scale) (with the same
is_float/scale logic using input_info and ALPHA) and rely on the final
.collect() into activations: Vec<f64>.
In `@rust/jstprove_conformance/tests/conformance.rs`:
- Line 38: The variable name reference_only_skipped is stale and should be
renamed to jstprove_errors_skipped and the summary text "(reference_only
placeholders)" changed to "(allow_jstprove_error)"; update every occurrence
(declaration, increments, and the final summary/print block in conformance.rs)
so increments and logging use jstprove_errors_skipped and the summary string
reflects "(allow_jstprove_error)" to match the flag-based behavior and avoid
confusion.
- Around line 40-62: The current loop calls shrink(case, &full_runner) for every
failing case which can hugely increase runtime; change it to only run shrink
when either fail_fast is true or this is the first failure for that operation
name: introduce a local HashSet (e.g. shrunk_ops) before iterating cases, and
inside the TestResult::Fail arm only call shrink and print "minimized inputs" if
fail_fast || !shrunk_ops.contains(&case.op_name); when you do run shrink insert
case.op_name into shrunk_ops so subsequent failures for the same op_name skip
shrinking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f99929d-ff67-46c5-ba71-8bb6125b04e2
📒 Files selected for processing (9)
rust/jstprove_conformance/src/fixtures.rsrust/jstprove_conformance/src/generator/builder.rsrust/jstprove_conformance/src/generator/cases_float.rsrust/jstprove_conformance/src/generator/cases_int.rsrust/jstprove_conformance/src/generator/op_specs.rsrust/jstprove_conformance/src/generator/shapes.rsrust/jstprove_conformance/src/runner.rsrust/jstprove_conformance/src/tolerance.rsrust/jstprove_conformance/tests/conformance.rs
✅ Files skipped from review due to trivial changes (1)
- rust/jstprove_conformance/src/generator/cases_int.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- rust/jstprove_conformance/src/tolerance.rs
- rust/jstprove_conformance/src/fixtures.rs
- rust/jstprove_conformance/src/generator/op_specs.rs
…length-check parens, shrink dedup, rename skipped counter
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust/jstprove_conformance/src/fixtures.rs`:
- Around line 346-389: The fixture f07a_reducemax_overflow_boundary is
inconsistent: its header and failure_description claim an i64 overflow boundary
test but the actual input uses v = 7 (inputs vec![7,6,7,8,5,7]) which cannot
overflow; fix by either (A) making the test actually hit the boundary by setting
v to the intended boundary (e.g. v = i64::MAX / 4 or the exact value your
range-check/shifting logic expects) and updating the inputs vector to use that v
so shifted values can overflow on pre-fix builds, or (B) if you intend to test
the keepdims/tract workaround instead, rewrite the leading comment and
failure_description to remove the overflow framing and clearly describe the
keepdims=0/tract q_sum_t behavior; update the variable name v and inputs, plus
the failure_description string and header comment in
f07a_reducemax_overflow_boundary accordingly.
In `@rust/jstprove_conformance/src/generator/cases_float.rs`:
- Around line 784-809: Replace the informal "XXX: query_id overflow bug in
AveragePool circuit" comments that accompany allow_jstprove_error: true in the
TestCase entries for op_name "AveragePool" and "GlobalAveragePool" in
generator/cases_float.rs with a concrete tracking reference (e.g. "JSTPRV-<n>"
or "GH-<repo>#<n>"); update both TestCase comments so they read like
"JSTPRV-123: query_id overflow bug in AveragePool circuit" (using the actual
ticket number you open), ensuring the skip is auditable and can be removed when
the referenced issue is resolved.
In `@rust/jstprove_conformance/tests/conformance.rs`:
- Around line 227-232: The norm_ops test is vacuous because norm_cases() returns
an empty Vec, so change the test to fail fast when no cases exist: call
norm_cases() first, and if the returned Vec is empty panic with a clear message
(e.g., "norm_cases is empty — remove this test or populate cases"); otherwise
pass the cases into run_group("norm", cases). Alternatively mark the test
#[ignore] with a comment explaining InstanceNorm/GroupNorm are placeholders; as
an additional safety, add a debug_assert! in run_group to ensure non-placeholder
groups are never invoked with an empty cases Vec.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcad7145-2742-43e3-96c3-9188bb7c7e80
📒 Files selected for processing (5)
rust/jstprove_conformance/src/fixtures.rsrust/jstprove_conformance/src/generator/cases_float.rsrust/jstprove_conformance/src/generator/shapes.rsrust/jstprove_conformance/src/runner.rsrust/jstprove_conformance/tests/conformance.rs
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
rust/jstprove_conformance/src/fixtures.rs (2)
346-384:⚠️ Potential issue | 🟡 MinorF-07a still does not test the advertised overflow path.
Line 372 uses
v = 7, so this fixture only checks a smallReduceMaxpath while the id and description still claim an overflow-boundary regression. Consider either renaming/rewriting it as a keepdims/small-input coverage case, or excluding it until a real boundary reproducer is possible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/jstprove_conformance/src/fixtures.rs` around lines 346 - 384, The test f07a_reducemax_overflow_boundary currently uses v = 7 so it does not exercise an overflow boundary despite the id and failure_description claiming an overflow regression; update the fixture to reflect what it actually tests by renaming the id (e.g., change RegressionFixture.id from "reducemax_overflow_boundary" to something like "reducemax_keepdims0_small_input") and update the failure_description text in f07a_reducemax_overflow_boundary to describe that this is a keepdims=0 small-input coverage case rather than an overflow boundary repro (or alternatively remove/skip the fixture until a true boundary reproducer is available); edit the symbols f07a_reducemax_overflow_boundary, RegressionFixture.id, and the failure_description string accordingly.
243-279:⚠️ Potential issue | 🟡 MinorF-05 still cannot exercise truncation-direction behavior.
Line 265 uses divisor
1for every element, so negative dividends never produce a remainder and cannot expose truncation-toward-zero vs euclidean/floor quotient issues. If non-trivial divisors are currently unfixed, this fixture should be renamed/described as an identity-divisor smoke test rather than a historical truncation regression.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/jstprove_conformance/src/fixtures.rs` around lines 243 - 279, The fixture f05_div_near_zero_divisor uses a constant divisor vector of all 1s (Initializer name "B") so it cannot exercise truncation-vs-euclidean behavior; either change the Initializer data to include non-trivial divisors (e.g., mix of 2, 3, -2, etc., preserving dims vec![8]) so negative dividends produce non-zero remainders and trigger truncation-path logic in f05_div_near_zero_divisor (also verify build_single_op_model still expects INT64 divisor initializer), or if non-trivial divisors are intentionally unsupported, rename the fixture id and update failure_description to call this an identity-divisor smoke test rather than a truncation regression (update id "div_near_zero_divisor" and the failure_description string accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust/jstprove_conformance/src/fixtures.rs`:
- Around line 443-529: The fixtures f08a_matmul_overflow_boundary and
f08b_gemm_overflow_boundary keep one operand at 1.0 so the accumulation never
reaches the i64 overflow threshold; change the test inputs so both operands are
the large boundary value (32767 * ALPHA) to actually trigger the overflow path.
In f08a update b_vals (currently vec![ALPHA; 16]) to the large quantized value
like a_vals (vec![32767 * ALPHA; 16]); in f08b update a_vals (currently
vec![ALPHA; 16]) to the large value to match b_data (32767.0_f32) so the
per-term product becomes ~32767 * ALPHA^2 and the 4-term dot product can
overflow; keep types (Vec<i64> for quantized inputs) and adjust any explanatory
text if needed.
In `@rust/jstprove_conformance/tests/conformance.rs`:
- Around line 69-77: In TestResult::Error(e) when case.allow_jstprove_error is
true, don't immediately count as a skipped jstprove error; re-run the case using
the reference-only execution (i.e., invoke the runner that executes only the
reference/ONNX path) and only increment jstprove_errors_skipped and log the SKIP
if that reference-only run succeeds; otherwise treat the original error as a
real failure (do not skip) and propagate or log it accordingly. Ensure you call
the existing reference-only runner/path for the verification (rather than
assuming the initial Error came from jstprove), reference the
TestResult::Error(e) branch and case.allow_jstprove_error and use the
reference-only run result to decide skipping.
---
Duplicate comments:
In `@rust/jstprove_conformance/src/fixtures.rs`:
- Around line 346-384: The test f07a_reducemax_overflow_boundary currently uses
v = 7 so it does not exercise an overflow boundary despite the id and
failure_description claiming an overflow regression; update the fixture to
reflect what it actually tests by renaming the id (e.g., change
RegressionFixture.id from "reducemax_overflow_boundary" to something like
"reducemax_keepdims0_small_input") and update the failure_description text in
f07a_reducemax_overflow_boundary to describe that this is a keepdims=0
small-input coverage case rather than an overflow boundary repro (or
alternatively remove/skip the fixture until a true boundary reproducer is
available); edit the symbols f07a_reducemax_overflow_boundary,
RegressionFixture.id, and the failure_description string accordingly.
- Around line 243-279: The fixture f05_div_near_zero_divisor uses a constant
divisor vector of all 1s (Initializer name "B") so it cannot exercise
truncation-vs-euclidean behavior; either change the Initializer data to include
non-trivial divisors (e.g., mix of 2, 3, -2, etc., preserving dims vec![8]) so
negative dividends produce non-zero remainders and trigger truncation-path logic
in f05_div_near_zero_divisor (also verify build_single_op_model still expects
INT64 divisor initializer), or if non-trivial divisors are intentionally
unsupported, rename the fixture id and update failure_description to call this
an identity-divisor smoke test rather than a truncation regression (update id
"div_near_zero_divisor" and the failure_description string accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 920d677a-7448-49e9-9f01-a522a31621a3
📒 Files selected for processing (3)
rust/jstprove_conformance/src/fixtures.rsrust/jstprove_conformance/src/generator/cases_float.rsrust/jstprove_conformance/tests/conformance.rs
✅ Files skipped from review due to trivial changes (1)
- rust/jstprove_conformance/src/generator/cases_float.rs
…/f08b to non-overflowing inputs and document unfixed overflow
millioner
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Related Issue
Type of Change
Checklist
Deployment Notes
Additional Comments
Summary by CodeRabbit
New Features
Tests
Chores
Documentation