Skip to content

Record which walk owns focus_iter_token, so to_next_val stops resuming stale state - #74

Open
imlvts wants to merge 1 commit into
Adam-Vandervorst:masterfrom
imlvts:bugfix/iteration-token-ownership
Open

Record which walk owns focus_iter_token, so to_next_val stops resuming stale state#74
imlvts wants to merge 1 commit into
Adam-Vandervorst:masterfrom
imlvts:bugfix/iteration-token-ownership

Conversation

@imlvts

@imlvts imlvts commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

to_next_get_val resumes from focus_iter_token whenever it is not NODE_ITER_INVALID, rather than starting from the focus, and so does to_next_k_path. So the token is not scratch space: it is a promise that an iteration is in progress and positioned where the reader expects. Several movement operations broke that promise, and after each a following to_next_val carried on from the wrong place and reported that nothing was left:

  • descend_first_byte (and so to_next_step) stored the token next_items had already advanced past the item it descended into, and NODE_ITER_FINISHED on the branch that did not move.
  • descend_first_k_path leaves a token that belongs to the k-path walk, which to_next_k_path legitimately resumes from.
  • reset cleared the token only on the branch that popped an ancestor, so a zipper whose whole subtrie lives in one node kept a spent one.
  • ascend, ascend_byte, ascend_until and ascend_until_branch restored or left a token that no longer described the focus.

Clearing the token everywhere is the obvious fix and costs ~10% on a to_next_step walk, because to_next_sibling_byte resumes from the same field and to_next_step alternates the two. Instead IterOwner records which walk the token belongs to: the k-path pair claims it, every ascent and descend_first_byte disown it, reset clears it, and to_next_get_val restarts unless the token is its own. The sibling path keeps its fast resume. descend_first_byte stores the token before its child-descent block, which may replace it with the child's own; storing after clobbers it. No API change.

Squashed from five commits on the bugfixes branch, all one defect. Regression test: read_zipper_to_next_val_after_every_movement, nine routes onto the same location followed by a full value walk, plus the reset and four-ascent sequences and a k-path walk. Fails before this change.

lean/FINDINGS.md finding 2 on the lean-fuzzer-restage branch.

…g stale state

`to_next_get_val` *resumes* from `focus_iter_token` whenever it is not
`NODE_ITER_INVALID`, rather than starting from the focus, and so does
`to_next_k_path`.  So the token is not scratch space: it is a promise
that an iteration is in progress and positioned where the reader
expects.  Several movement operations broke that promise, and after
each a following `to_next_val` carried on from the wrong place and
reported that nothing was left:

* `descend_first_byte` (and so `to_next_step`) stored the token
  `next_items` had already advanced past the item it descended into,
  and `NODE_ITER_FINISHED` on the branch that did not move.
* `descend_first_k_path` leaves a token that belongs to the k-path
  walk, which `to_next_k_path` legitimately resumes from.
* `reset` cleared the token only on the branch that popped an ancestor,
  so a zipper whose whole subtrie lives in one node kept a spent one.
* `ascend`, `ascend_byte`, `ascend_until` and `ascend_until_branch`
  restored or left a token that no longer described the focus.

Clearing the token everywhere is the obvious fix and costs ~10% on a
`to_next_step` walk, because `to_next_sibling_byte` resumes from the
same field and `to_next_step` alternates the two.  Instead `IterOwner`
records which walk the token belongs to: the k-path pair claims it,
every ascent and `descend_first_byte` disown it, `reset` clears it, and
`to_next_get_val` restarts unless the token is its own.  The sibling
path keeps its fast resume.  `descend_first_byte` stores the token
*before* its child-descent block, which may replace it with the child's
own; storing after clobbers it.  No API change.

Squashed from five commits on the bugfixes branch, all one defect.
Regression test: `read_zipper_to_next_val_after_every_movement`, nine
routes onto the same location followed by a full value walk, plus the
`reset` and four-ascent sequences and a k-path walk.  Fails before this
change.

lean/FINDINGS.md finding 2 on the lean-fuzzer-restage branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZmoASqM5FUuzvJeJaYQjR
@imlvts

imlvts commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Alternative: store iteration owner as a part of the iteration token, if there's a bit to spare.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant