fix(common): contain jaq parser panics in jq::parse_file, and wire the fuzz dictionaries - #1209

_{Comment @coderabbitai help to get the list of available commands.}

Yes — and it turns out to be a clean upstream bug, so thanks for asking. I dug in.

It is not our call site. jaq_json::read::parse_single panics on its own, with none of our code in the picture and no jq program involved — just parsing a document.

Root cause is in jaq-json src/num.rs: Num::cmp compares exactly within a variant pair but lossily through f64 across variants.

(Self::Int(x),    Self::Int(y))    => x.cmp(y),                          // exact
(Self::Int(i),    Self::Float(f))  => float_cmp(*i as f64, *f),          // lossy
(Self::BigInt(x), Self::BigInt(y)) => x.cmp(y),                          // exact
(Self::BigInt(x), Self::Float(y))  => float_cmp(x.to_f64().unwrap(), *y) // saturates to inf

So equality is not transitive, and it needs nothing exotic to show:

let a = Num::Int(1_000_000_000_000_000_001);
let b = Num::Int(1_000_000_000_000_000_002);
let f = Num::Float(1e18);      // f64 spacing at 1e18 is 128
// a == f, b == f, yet a < b

The BigInt arm is the same defect with a wider mouth: every integer above f64::MAX is Equal to Float(inf). Our fuzz artifact hits it because Map<Val, Val> means object keys are Vals, and both Hash for Val and Ord for Val sort an object entries by key — so hashing an object-as-key runs Num::cmp over numbers straddling the boundary. Delta-minimised 1461 -> 799 bytes.

Separately, Float(NAN).cmp(&Float(NAN)) is Less — a value strictly less than itself. The comment there explains the jq-compatible nan-sorts-smallest intent, which seems reasonable, but Ord is also what BTreeMap and binary_search stand on. Returning Equal for that one pair keeps the jq behaviour and restores reflexivity.

Not reported upstream (checked the tracker; #232 is unrelated jq-compat stuff) and 2.0.2 is the latest release, so it is live.

I have an issue drafted with both reproducers and a suggested fix — compare Int/BigInt against Float exactly, since every finite f64 is a dyadic rational, which is contract-correct and more accurate than today. Will file it against 01mf02/jaq unless you would rather send it.

Worth being explicit that this does not change the case for this PR: catch_unwind here is containment, not a fix. Even with an upstream release we would want to keep it, since the guard is what makes the Err contract true regardless of what the parser does.

Went back and checked the jaq tracker properly — my last comment said "not reported upstream" off a keyword search, which was too thin an answer to give you. Enumerating all 460 issues+PRs (state=all) instead of relying on GitHub fuzzy search turned up two things that change the report.

Still nothing tracking it, confirmed properly this time: no issue or PR mentioning total order, Ord, transitivity or a sort panic. I also cloned the repo — master (6e82c3c, Aug 7) is byte-identical to 2.0.2 in the Ord impl, so it is live rather than fixed-but-unreleased.

There is a sharper bug than the one I described. PR #343 ("Simplify integer-float equality", merged Oct 2025) gave PartialEq an f.is_finite() guard. Ord::cmp, twenty lines below, never got it:

// PartialEq (num.rs:336)
(BigInt(i), Float(f)) | (Float(f), BigInt(i)) => f.is_finite() && float_eq(i.to_f64().unwrap(), *f),
// Ord (num.rs:364)
(BigInt(x), Float(y)) => float_cmp(x.to_f64().unwrap(), *y),

So a == inf is false while a.cmp(&inf) is Equal, and Ord requires those to agree. That is one containment rule with two implementations and only one of them hardened — the same shape as the tar and path issues we have been working through, which I did not expect to find waiting for me in a dependency. It now leads the report, because unlike the precision question it needs no design debate and the fix is one line.

#343 also saw this coming and priced it wrong, which is the most useful thing to tell an upstream maintainer. Its own description anticipates the collision — "if you store many very large integers that are quite close to each other and that fit into f64 as object keys... many of them map to the same hash, and therefore looking up such entries will be rather slow" — and weighs it purely as a performance risk. The same collision is what makes Eq non-transitive and Ord non-total. So the report is not "you missed this", it is "you considered this and the cost was larger than it looked".

One correction to my last comment: I said the nan < nan behaviour looked like a reasonable jq-compat intent. It is more than intent — PR #248 ("NaN compatibility") was merged specifically to enforce it. So I have demoted that from a bug to a note about what the decision costs in std-library terms, and I am not proposing to change the observable semantics.

Worth noting for our own purposes: jaq does have fuzz targets (#262), but they cover jaq-core lexer/parser, not the jaq-json reader. That is the gap our parse_single harness landed in.

Draft is rewritten and ordered by how unarguable each point is. Nothing here changes this PR — the guard is still the right call regardless of what upstream decides.

going to merge and deal with the notification separately