fix(jq): forward demand across path()'s reindex bridge, not just collect

docs/compliance/jq/limitations.md

@@ -1658,15 +1658,21 @@ answers `["b"]` — and classified the two residuals appended below):
   `path(.a[])`) always agreed — nothing in it is observable per element — and keeps #2061/
   #2168's no-materialization cursor walk untouched.
 
-  **Two shapes still collect**, neither introduced by that work and both tracked as
-  [#2925](https://github.com/rust-works/succinctly/issues/2925). A document the reindex bridge
-  will not round-trip identically (any `Float`, or a number literal too large to survive the
-  trip) takes the bridge, which collects — so the *document*, not the filter, selects the
-  route: `[limit(1; path((.a|stderr),(.b|stderr)))]` writes `1` on `{"a":1,"b":2}` and `12`
-  once a 300-digit literal is added. And the stop reaches the branch producer but not a
-  generator in *index* position: `[limit(1; path(.[("a"|stderr),("b"|stderr)]))]` writes `ab`
-  where jq writes `a`, which is `resolve_index_expr`'s eager key evaluation — the same function
-  #2032 sits in.
+  **Two shapes still collected after that work, neither introduced by it, both tracked and
+  now closed as [#2925](https://github.com/rust-works/succinctly/issues/2925).** A generator
+  in *index* position closed with [#2267](https://github.com/rust-works/succinctly/issues/2267):
+  `E[K]`/`E[S:T]` are native `resolve_node_sink` arms now, so a bound reaches the key and
+  bound generators too (`[limit(1; path(.[("a"|stderr),("b"|stderr)]))]` writes `a`, matching
+  jq, where it used to write `ab`). And a document the reindex bridge will not round-trip
+  identically (a number literal past `REINDEX_LITERAL_LEN_CAP`, 256 characters, or a NaN
+  spelling — a bare `Float` has been bridge-identity since #2902 and no longer selects this
+  route) now forwards demand across the bridge the same way, instead of collecting every path
+  first: `[limit(1; path((.a|stderr),(.b|stderr)))]` writes `1` whether or not a 300-digit
+  literal sits elsewhere in the document. `to_json_for_reindex`'s own respelling of an
+  over-cap literal (`1E+300` for the literal above) is a separate, still-open value-fidelity
+  gap on the same bridge — [#3025](https://github.com/rust-works/succinctly/issues/3025) —
+  unaffected by this fix, since it changes only which entry point the bridge hands the
+  document to, not what the bridge does to the document itself.
 - **`recurse(f)`/`recurse(f; cond)` past its native stack budget finishes one node's own `f`
   before descending.**
   `resolve_recurse_sink` (#2235) streams each visited node to a bounded consumer as soon as

src/jq/eval.rs

@@ -45999,9 +45999,15 @@ fn builtin_path<'a, W: Clone + AsRef<[u64]>, S: EvalSemantics>(
     // consulted `optional` via `suppress_or_raise` -- this function was the
     // one place the pattern hadn't been carried over. Reachable both via the
     // public `succinctly::jq::eval::eval` library API and, via the CLI,
-    // whenever `eval_generic.rs`'s `Builtin::Path` fallback re-enters the
-    // full evaluator for a non-`reindex_bridge_is_identity` value (a `Float`
-    // anywhere in the document).
+    // whenever `eval_generic.rs`'s *eager* `eval_single` arm for
+    // `Builtin::Path` re-enters the full evaluator for a non-
+    // `reindex_bridge_is_identity` value (a number literal past
+    // `REINDEX_LITERAL_LEN_CAP`, or a NaN spelling -- a bare `Float` has
+    // been bridge-identity since #2902). The *lazy* `eval_each_generic` arm
+    // for the same builtin no longer reaches here on that route (#2925): it
+    // hands the same non-identity case to `eval_each_owned` instead, which
+    // re-enters through `eval_each`'s own lazy `Builtin::Path` arm
+    // (`each_path`), not this eager one.
     let owned = to_owned_or_suppress!(&value, optional);
     builtin_path_on_owned::<W, S>(expr, &owned, optional)
 }

src/jq/eval_generic.rs

@@ -9146,8 +9146,8 @@ fn eval_each_generic<S: EvalSemantics, V: DocumentValue>(
         // navigation, nothing observable per output, and the #2061/#2168
         // route that avoids materializing the document at all), and a
         // document the reindex bridge would not round-trip identically
-        // keeps the bridge -- and so keeps the collecting behaviour, a
-        // recorded residual (#2925).
+        // keeps the bridge -- which now forwards demand too (#2925), so
+        // every route here streams.
         Expr::Builtin(Builtin::Path(path_expr))
             if !(cursor.is_some() && path_expr_is_cursor_navigable(path_expr)) =>
         {
@@ -9165,20 +9165,31 @@ fn eval_each_generic<S: EvalSemantics, V: DocumentValue>(
                 Err(e) => return Flow::Escaped(Control::Error(e)),
             };
             if !reindex_bridge_is_identity(&owned) {
-                // Hand the bridge the document already materialized above,
-                // the way the eager arm does -- re-entering `eval_single`
-                // would run `RootWitness::of` + `demote_for_reentry` +
-                // `to_owned_with_cursor` a second time over the whole
-                // document to rebuild exactly these values.
-                let owned_builtin_expr = Expr::Builtin(Builtin::Path(path_expr.clone()));
-                return drain_result_generic(
-                    eval_on_owned::<S, _>(
-                        &owned_builtin_expr,
-                        owned,
-                        optional,
-                        Reentry::Against(root),
-                    ),
-                    sink,
+                // #2925: demand-forwarding, not `eval_on_owned`'s eager
+                // collect -- a consumer wrapping this arm (`limit`, `first`,
+                // `label`/`break`) can now stop the walk early on a document
+                // the reindex bridge would not round-trip identically (an
+                // over-256-char number literal, a NaN spelling), matching
+                // jq's own generator order instead of over-running it.
+                // `eval_each_owned`, not `eval_on_owned` -- `eval.rs`'s
+                // `eval_each` has its own lazy `Builtin::Path` arm (#2908),
+                // the same demand-forwarding entry point
+                // [`bridge_to_each_owned_flow`] above uses (same contract:
+                // `Reentry::Against(root)` + a sink-wrapping closure).
+                // `expr` itself, not a rebuilt `owned_builtin_expr` -- it
+                // already *is* `Builtin::Path(path_expr)` (this arm's own
+                // match scrutinee), so passing it directly avoids cloning
+                // the subtree `reentry.reroot` is about to walk anyway.
+                // Passing the whole wrapper (not bare `path_expr`, as
+                // `demoted` above does) is what lets the precheck see the
+                // resolver-reaching node itself -- the reason `demoted`
+                // reroots `path_expr` directly instead, per its own comment.
+                return eval_each_owned::<S>(
+                    expr,
+                    &owned,
+                    optional,
+                    Reentry::Against(root),
+                    &mut |v| sink.push(GenericItem::Owned(v)),
                 );
             }
             each_path_on_owned::<S>(&demoted, &owned, false, &mut |v| {

tests/jq_cli_tests.rs

@@ -49338,6 +49338,20 @@ fn path_mode_fold_resolves_init_by_demand_2903() -> Result<()> {
 fn path_results_stream_to_their_consumer_2908() -> Result<()> {
     let abc = r#"{"a":1,"b":2,"c":3}"#;
     let probes = "(.a|stderr), (.b|stderr), (.c|stderr)";
+    // #2925: the reindex bridge's own lazy `Builtin::Path` arm, exercised on
+    // a document it will not round-trip identically (a number literal past
+    // `REINDEX_LITERAL_LEN_CAP`, #2902), so `path(f)` takes the bridge
+    // instead of the cursor walk above. Every probe below reads only a
+    // small named field (`.a`/`.b`/`.c`/`.x`), never the oversized `n`
+    // field itself -- printing *that* to stderr hits a separate, unrelated
+    // bug (the bridge respells an over-cap literal, #3025), which this row
+    // set deliberately does not exercise.
+    let abc_big = format!(r#"{{"a":1,"b":2,"c":3,"n":{}}}"#, "9".repeat(300));
+    let label_big = format!(r#"{{"a":1,"c":1,"n":{}}}"#, "9".repeat(300));
+    let prefix_big = format!(r#"{{"a":{{"b":1}},"c":1,"n":{}}}"#, "9".repeat(300));
+    let altq_big = format!(r#"{{"x":[true,false],"n":{}}}"#, "9".repeat(300));
+    let cap256 = format!(r#"{{"a":1,"b":2,"n":{}}}"#, "9".repeat(256));
+    let cap257 = format!(r#"{{"a":1,"b":2,"n":{}}}"#, "9".repeat(257));
     for (input, filter, want_out, want_err, want_code) in [
         // A bound outside `path()` now stops the generator inside it.
         (
@@ -49487,39 +49501,108 @@ fn path_results_stream_to_their_consumer_2908() -> Result<()> {
             String::new(),
             0,
         ),
+        // #2925: the reindex-bridge route (a document `reindex_bridge_is_identity`
+        // rejects) now forwards demand the same way the cursor-navigable route
+        // above always did, instead of collecting every path before the bound
+        // outside `path()` ever saw one. Every row below is captured whole from
+        // jq 1.7.1 -- stdout, stderr and exit code -- on a document holding a
+        // 300-character number literal (well past `REINDEX_LITERAL_LEN_CAP`,
+        // #2902), which is what selects this route; the identical filter on the
+        // cap-sized documents above stays on this same table, matching jq.
+        (
+            abc_big.as_str(),
+            "[limit(1; path((.a|stderr), (.b|stderr), (.c|stderr)))]".to_string(),
+            "[[\"a\"]]\n".to_string(),
+            "1".to_string(),
+            0,
+        ),
+        (
+            abc_big.as_str(),
+            "[first(path((.a|stderr), (.b|stderr), (.c|stderr)))]".to_string(),
+            "[[\"a\"]]\n".to_string(),
+            "1".to_string(),
+            0,
+        ),
+        (
+            abc_big.as_str(),
+            "[limit(2; path((.a|stderr), (.b|stderr), (.c|stderr)))]".to_string(),
+            "[[\"a\"],[\"b\"]]\n".to_string(),
+            "12".to_string(),
+            0,
+        ),
+        // A `label`/`break` bound reaches it too, same as the cap-sized row.
+        (
+            label_big.as_str(),
+            "[label $o | path((.a|stderr),(.c|stderr)) | ., break $o]".to_string(),
+            "[[\"a\"]]\n".to_string(),
+            "1".to_string(),
+            0,
+        ),
+        // #2680's prefix rule still holds on this route: a resolved sibling
+        // already emitted survives a later one erroring.
+        (
+            prefix_big.as_str(),
+            "path(.a.b, .c.d)".to_string(),
+            "[\"a\",\"b\"]\n".to_string(),
+            "jq: error (at <stdin>:0): Cannot index number with string \"d\"\n".to_string(),
+            5,
+        ),
+        (
+            prefix_big.as_str(),
+            "first(path(.a.b, .c.d))".to_string(),
+            "[\"a\",\"b\"]\n".to_string(),
+            String::new(),
+            0,
+        ),
+        (
+            prefix_big.as_str(),
+            "[path(.a.b, .c.d)?]".to_string(),
+            "[[\"a\",\"b\"]]\n".to_string(),
+            String::new(),
+            0,
+        ),
+        // The `?//` retry row from the table above, over a document that
+        // takes the bridge -- `.x` narrows away from the oversized `n`
+        // field before the `foreach`/`stderr` runs, so this still probes
+        // #2925's routing without also probing #3025's unrelated respelling.
+        (
+            altq_big.as_str(),
+            "[limit(1; path(.x | foreach (1 as $x ?// $y | (stderr|1)) as $v (.; .)))]"
+                .to_string(),
+            "[[\"x\"],[\"x\"]]\n".to_string(),
+            "[true,false][true,false]".to_string(),
+            0,
+        ),
+        // End-to-end agreement at the cap boundary: 256 characters is
+        // `REINDEX_LITERAL_LEN_CAP` itself (stays on the identity/cursor
+        // route), 257 is one past it (takes the bridge route above) -- both
+        // now stream identically, so this CLI-level pair cannot by itself
+        // tell which route ran (both produce byte-identical output). The
+        // boundary itself is what `test_reindex_bridge_identity_predicate_agrees_1909`
+        // pins directly, against `reindex_bridge_is_identity`; these two
+        // rows are here for jq-agreement coverage on either side of it, not
+        // as that guard's substitute.
+        (
+            cap256.as_str(),
+            "[limit(1; path((.a|stderr),(.b|stderr)))]".to_string(),
+            "[[\"a\"]]\n".to_string(),
+            "1".to_string(),
+            0,
+        ),
+        (
+            cap257.as_str(),
+            "[limit(1; path((.a|stderr),(.b|stderr)))]".to_string(),
+            "[[\"a\"]]\n".to_string(),
+            "1".to_string(),
+            0,
+        ),
     ] {
         let (stdout, stderr, code) = run_jq_full(&["-c", &filter], Some(input))?;
         assert_eq!(code, want_code, "{filter}: stdout: {stdout:?} stderr: {stderr:?}");
         assert_eq!(stdout, want_out, "{filter}");
         assert_eq!(stderr, want_err, "{filter}");
     }
 
-    // #2925's one remaining shape. The second one it recorded -- a generator
-    // in index position -- closed with #2267 and is in the table above now,
-    // which is why this is a single case rather than the loop it used to be.
-    //
-    // A document the reindex bridge will not round-trip identically takes
-    // the bridge, which collects -- so the *document* selects the route, not
-    // the filter. The same filter on `{"a":1,"b":2}` is in the table above,
-    // matching jq. jq 1.7.1 writes `1`; this writes `12`, and the assertions
-    // below carry both so that closing the gap trips this test rather than
-    // passing quietly.
-    let big = format!(r#"{{"a":1,"b":2,"n":{}}}"#, "9".repeat(300));
-    let filter = "[limit(1; path((.a|stderr),(.b|stderr)))]";
-    let (jq_err, our_err) = ("1", "12");
-    let (stdout, stderr, code) = run_jq_full(&["-c", filter], Some(&big))?;
-    assert_eq!(code, 0, "{filter}: stdout: {stdout:?} stderr: {stderr:?}");
-    assert_eq!(stdout, "[[\"a\"]]\n", "{filter}");
-    assert_ne!(
-        our_err, jq_err,
-        "{filter}: this row exists because the two differ -- if they no longer do, \
-         move it into the table above"
-    );
-    assert_eq!(
-        stderr, our_err,
-        "{filter}: #2925's residual changed -- if it closed, move this case into the \
-         table above with jq's own stderr ({jq_err:?})"
-    );
     Ok(())
 }