Description
expand_operators (src/giql/expander.py:380-399) collects all opted-in operator nodes in one top-down walk(), then replaces them in collection (outer-first) order. When two flagged operators nest (ancestor/descendant), replacing the ancestor detaches the descendant's subtree; the descendant's later node.replace() is then a silent no-op on the detached tree, so the inner operator is never expanded (it serializes as raw GIQL or is dropped, with no error). This is dormant today (the pass is a strict no-op — nothing is flagged and the registry is empty) but becomes a live bug once two nestable operators are flagged in the migration steps; the nesting is reachable in the grammar, e.g. a predicate inside a DISJOIN/NEAREST reference := (SELECT … WHERE x INTERSECTS …) subquery. The current walk tests do not pin the collect-then-mutate invariant — re-running the pass with the unsafe mid-walk form leaves every walk test passing. Follow-up to #138 (PR #151) and a prerequisite for the migration steps that flag nestable operators (#141, #142, #143).
Expected behavior
Every flagged operator expands regardless of nesting; an inner flagged operator nested inside an outer one is replaced correctly, and a test exists that fails under the naive outer-first / mid-walk-replace form.
Root cause
expand_operators replaces the collected pending nodes in outer-first walk order, so an ancestor replacement orphans a collected descendant before its turn. Replace deepest-first (sort pending by node depth descending) and/or skip any node already detached (node.parent is None and not the root) before node.replace. Add a discriminating test: an outer flagged operator whose expander returns AST that does not contain the inner node, plus an inner flagged operator — assert both expand, and that the test fails under outer-first ordering.
Description
expand_operators(src/giql/expander.py:380-399) collects all opted-in operator nodes in one top-downwalk(), then replaces them in collection (outer-first) order. When two flagged operators nest (ancestor/descendant), replacing the ancestor detaches the descendant's subtree; the descendant's laternode.replace()is then a silent no-op on the detached tree, so the inner operator is never expanded (it serializes as raw GIQL or is dropped, with no error). This is dormant today (the pass is a strict no-op — nothing is flagged and the registry is empty) but becomes a live bug once two nestable operators are flagged in the migration steps; the nesting is reachable in the grammar, e.g. a predicate inside aDISJOIN/NEARESTreference := (SELECT … WHERE x INTERSECTS …)subquery. The current walk tests do not pin the collect-then-mutate invariant — re-running the pass with the unsafe mid-walk form leaves every walk test passing. Follow-up to #138 (PR #151) and a prerequisite for the migration steps that flag nestable operators (#141, #142, #143).Expected behavior
Every flagged operator expands regardless of nesting; an inner flagged operator nested inside an outer one is replaced correctly, and a test exists that fails under the naive outer-first / mid-walk-replace form.
Root cause
expand_operatorsreplaces the collectedpendingnodes in outer-first walk order, so an ancestor replacement orphans a collected descendant before its turn. Replace deepest-first (sortpendingby node depth descending) and/or skip any node already detached (node.parent is Noneand not the root) beforenode.replace. Add a discriminating test: an outer flagged operator whose expander returns AST that does not contain the inner node, plus an inner flagged operator — assert both expand, and that the test fails under outer-first ordering.