Fix debug_assert panic in the m1 plan walker when a merge child is already visited

[bfs]

The m1 plan walker asserts that any child found in the scan window c[e.state.next..] is unvisited (debug_assert_eq!(e2.state.visited, false) in make_m1_plan). But a merge node appears in the child list of each of its parents, and only the parent it is first descended from consumes its slot via the swap/next += 1 bookkeeping. When another parent is later re-scanned off the stack (or the child was taken via the deferred b_children path), the child legitimately shows up visited in an unconsumed slot and the assert fires.

Release builds happened to tolerate this (the !e.state.visited guard at the top of the loop keeps every node processed exactly once), but any debug build panics on affected graph shapes. We hit it via a randomized simulation harness in a downstream project; test_data/plan_visited_regression.dt is a minimized 334-byte real oplog that reproduces it, and the included test also asserts the merge result equals a fresh tip checkout.

The fix retires visited child slots the same way descents do (visited never reverts), so rescans of the node don't re-walk them and the node isn't re-pushed onto the stack on their account. cargo test is green including the new regression test.

Happy to adjust anything about the approach or the test placement.

[josephg]

Thanks for finding this. Can you make a minimal reproducing test case? I want to go through the logic myself to make sure this is the right fix.

[bfs]

yup, sorry - that fixture was raw output from a simulation harness, not something a human can read. reworked the branch: the first commit is now a minimal repro you can trace by hand (7 single-op entries via Graph::from_simple_items, in the existing plan.rs test module) - it panics at the assert if you check out that commit alone. second commit is the fix.

the shape: node 6 is a merge child of three concurrent roots (0, 4, 5), so it's in all three child lists, but only the parent you first descend from consumes its slot via the swap / next += 1 bookkeeping. merging from a branch at [2] toward [2, 3, 6], one of 6's other parents gets rescanned off the stack after 6 was already visited, and the scan finds it in an unconsumed slot. the fix retires those slots the same way descents do, since visited never reverts. happy to reshape if you'd rather handle it another way.

[josephg]

Thanks for getting back to me so fast! Taking a look now.