Conversation
A location with both a value and a child is two same-key slots in a LineListNode. The previous-sibling lookup only checked the first matched slot for the child, so the zipper landed with child_count 0. Check both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019R2H8fnco29asY2v3TPbtF
|
https://github.com/Adam-Vandervorst/PathMap/actions/runs/35158141349#summary-105008105822 Again, +57.2% superdense_drop_bench/200, but overall result not changed? |
…st macro that is implemented for all zipper types; slightly broadening covered cases
|
@imlvts I promoted your test to all zippers and added another test. That turned up an ACTZipper bug. I committed a fix for that bug, but I wanted your review in case the fix isn't agreeable. I factored it the way I did because having the focus at a non-existent path is an edge case in practice and I didn't want the case to slow down anything that is doing iteration over existing items. |
I feel like the drop bench results are so noisy, I'm of a mind to remove those benchmarks altogether. If I thought this was the final shape of the TrieNode contract, I'd optimize the function more, but I don't think it's worth any time right now because there are a number of changes I want to make. We don't really have any benchmarks that exercise backwards iteration, but maybe we should. |
|
This conflicts with #113, and I will update that later. The way you did it is better. |
A location with both a value and a child is two same-key slots in a LineListNode. The previous-sibling lookup only checked the first matched slot for the child, so the zipper landed with child_count 0. Check both.