Drop a state answer asked on a revision the page has left - #97
Drop a state answer asked on a revision the page has left#97leaf-agent wants to merge 2 commits into
Conversation
A state answer carries the browser view of the revision its request named and of the active revision the page may activate into, and of no other. A forced activation between the ask and the answer therefore leaves the page standing on a revision the answer says nothing about, and composition holds it from activating into the newer one. The commit boundary judged sequence and active revision but not that view, so the projection threw "state browser has no matching revision view", the page rolled the read back, and the runtime reported the fault to the log. Judge the view at the same boundary: an answer with no view of the revision the page will be showing is stale, like the ones the gates above drop. The reading it carries still differs from the page's, so the news stream asks again with the revision the page holds now. The regression test arranges the race rather than waiting for it, holding one read taken on the first revision open across the chip press that moves the page to the second.
leaf-agent
left a comment
leaf-agent
left a comment
There was a problem hiding this comment.
The runtime change reads correctly to me — showing predicts exactly what apply looks up, and dropping instead of throwing stays live because a drop never advances runtime.reading, so the next /api/news word (repeated every five seconds regardless of change) makes the page ask again with the revision it now holds.
The regression test is the part I'd change before this lands. As written it detects the bug about a third of the time: with the guard removed from state-application.js and everything else at this commit, I ran the test ten times on the CI runner and it failed 3/10. The cause is the barrier after the held read is released — ticked(page) waits for the next lf-actions, and the heartbeat dispatches one on its own cadence, so in most runs the assertions run before the released answer has been judged at all. The comment above it ("the page's next pass is after it") is the claim that doesn't hold: the next pass is whichever pass comes next, and usually that is the heartbeat's.
Swapping that one barrier for told(page) — the page has taken in the reading the server holds, which it can only reach by judging the answer just released — makes it a real net: 6/6 fail without the guard, 6/6 pass with it. With the second suggestion below applied too, 4/4 each way. Worth updating the PR body as well, which says it fails on main three runs out of three; that is this machine's result rather than a property of the arrangement.
lint being red here is not this PR: main at the merge base (47f1c35d) fails the same hook, on tests/interact_support.py, tests/test_interact_*.py and three others that this diff doesn't touch. #96 is the fix for it.
How the rates were measured
All runs on the CI runner, uv run pytest tests/test_render_projection.py::test_an_answer_asked_on_a_revision_the_page_has_left_is_stale_rather_than_broken -q -n0 --run-nightly, one process at a time.
| runtime | test | result |
|---|---|---|
| this PR | this PR | 10/10 pass |
| guard removed | this PR | 7/10 pass, 3/10 fail |
| guard removed | told(page) barrier |
0/6 pass, 6/6 fail |
| this PR | told(page) barrier |
6/6 pass |
| guard removed | both suggestions | 0/4 pass, 4/4 fail |
| this PR | both suggestions | 4/4 pass |
Every failure was the intended one — assert errors == [] with leaf: read failed: state browser has no matching revision view — so the arrangement is specific; it was only the barrier that let the run finish before the answer was judged.
Instrumenting the runtime with a page global recorded at receiveState entry showed the passing runs never seeing an answer whose views were ["1", "3"] by the time the assertions ran, while the answers before it (["1"], ["1", "2"], ["2", "3"]) were all there — the released answer had simply not been processed yet.
The regression test's barrier after the held read is released was `ticked`, which waits for the next `lf-actions`. The heartbeat dispatches one on its own cadence, so the assertions usually ran before the released answer had been judged at all, and the test passed on the broken runtime most of the time. Wait on `told` instead: the page has taken in the reading the server holds, which it reaches only by judging the answer just released. Release the held route by reading the list rather than popping it, too — the list is both the record and the armed flag, so emptying it re-arms the route and holds the page's next read for good.
|
Landed on
|
2 participants
Problem
The
testjob on run 33185431703 failed intest_the_live_page_defers_for_typing_then_adopts_without_a_press, where the runtime reportedTypeError: state browser has no matching revision viewfromstate-application.js. The same test passed on the two commits that landed beside it, so this is a race the CI machine's load arranged rather than something that commit changed — and it is the runtime's own fault report, not a test-side wait: the page rolled the read back and posted anerrorevent to the log.A state answer carries the browser view of the revision its request named and of the active revision the page may activate into, and of no other (
browser.py). The commit boundary re-judges a late answer against the sequence and the active revision it finds, but not against that view. So an answer asked while the page stood on r1, and overtaken by the chip press that activated r2, comes back naming r3 as active: it cannot activate, because the reader is mid-composition, and the page is standing on a revision it holds no view of.Solution
Judge the view at the same boundary as the other two coordinates: an answer with no view of the revision the page will be showing once this application is done is stale, and is dropped the way the gates above it drop one. Nothing is lost by dropping it — the reading it carries still differs from the page's, so the news stream asks again with the revision the page holds now, and that answer projects it. The invariant throw stays where it is, for a state that is malformed rather than late.
Testing
test_an_answer_asked_on_a_revision_the_page_has_left_is_stale_rather_than_brokenarranges the race instead of waiting for it: it holds one/api/stateread open while the page still names r1, presses the chip to r2 under it, publishes r3, and then releases the held read into a page that is composing. Its barrier after the release istoldrather thanticked: the heartbeat dispatcheslf-actionson its own cadence, so waiting for the next pass let the assertions run before the released answer had been judged, and the test passed on the broken runtime most of the time. Waiting instead until the page holds the reading the server holds makes it a net. Measured on this CI runner, one process at a time: 5/5 fail with the guard removed, every failure the intendedleaf: read failed: state browser has no matching revision view, and 3/3 pass with it. Those are this machine's rates, not a property of the arrangement.uv run pytest tests/test_render_projection.py --run-nightly(75 passed),tests/test_render_drafts.py tests/test_render_controls.py --run-nightly(97 passed),uv run pytest tests(657 passed, 6 skipped), andpre-commiton both files. Re-run after the barrier change:tests/test_render_projection.py --run-nightly(75 passed),uv run pytest tests(657 passed, 6 skipped), andpre-commiton both files.The interleaving, step by step
Leaf-View-Revision: 1.index.htmlbecomes v2, then v3. The server answers the held request against the log of that moment: requested r1, active r3, sobrowser.viewsholds{"1", "3"}.runtime.currentRevisionis 2.willActivateis false, because the reader is mid-composition and this is not a forced activation.applylooks upviews["2"]and finds nothing, so the projection throws, the read rolls back, andreportPageErrorposts the fault.Step 4 is why the deferring test is the one that catches it: without the composition hold the answer activates into r3, whose view it does carry.
Automated fix for failed run