Stage 2b: serialization, painter and renderer recovery

[tleonhardt]

Stage 2b of the reserved-row toolbar plan: the terminal transaction lock, the operation
recorder, the differential painter, and the prepare/commit/recovery bridge. Nothing is wired
into toolbar rendering yet — there is still no user-visible change and no public API. Stage 3
is the lifecycle integration.

What this adds

Module	Role
cmd2/terminal_transaction.py	L_terminal, the lock-order and wait contract, and the guard that enforces both
cmd2/output_recorder.py	Records a renderer's output operations against an immutable preflight snapshot
cmd2/toolbar_painter.py	Cell layout for toolbar content, and a painter that writes only what changed
cmd2/prompt_toolkit_bridge.py	Prepares, validates, commits and recovers renderer frames; owns cursor-report correlation

Why each piece is shaped this way

The lock is last, and the rules that make it last are enforced. Nothing higher-level may
be held while taking it, and nothing that can wait may run while holding it. Violations raise
synchronously before the blocking primitive is entered — a guard that fires after a wait has
begun protects nothing, and the tests assert the sentinel was never reached, which also means a
missing guard fails the suite instead of hanging it.

Emission is separated from preparation. Renderer.render() evaluates layout, filters and
styles while it emits, so holding the lock around it would run application callbacks inside the
transaction. The render now runs against a recorder that holds no backend object at all, and
the resulting batch is replayed under one transaction only after its geometry, output, owner
and terminal generations still match.

Discarding the output is only half of discarding the frame. Upstream has already advanced:
_last_screen became a baseline for a frame the terminal never received, and mode flags latch
beside their emission, so a full repaint never re-emits them. Recovery drops the baseline and
re-establishes a small enumerated state contract physically, then tells the renderer what is
true — not a snapshot of upstream fields, and not upstream's reset(), which emits operations
of its own and rewrites available-height bookkeeping.

The painter never erases before it writes. An erase followed by a write is two visible
states, and that pair is the flicker this design exists to remove. A changed run is overwritten
in place; a shortened frame's tail is padded.

Evidence

• Every named design §13.2 test exists and passes, plus the two §13.1 cursor-report regressions
  Stage 2a deferred to the bridge.
• Six mutations applied to production code, each observed to fail the tests that name it:
  stale-commit keeping the diff baseline, baseline-only recovery, an unchecked cursor row, a
  lock that takes nothing, a full-band repaint, and an erase before each paint.
• Races are driven by barriers, not sleeps. The blocked-sink test blocks a real stream and
  asserts emitted byte order and that the blocked writer held no higher-level lock.
• New dynamic-marker harness (pinned_dynamic.py, dynamic_gate.py, kept outside the repo
  with the rest of the harness) passes at 12, 24 and 40 rows: zero disappearances of the stable
  half, the changing half advanced in every window, every paint a partial run, zero erases
  addressed to the band, zero cursor reports from inside it. A frozen toolbar fails that gate,
  which is the control for it.
• Observer controls 23/23. Full acceptance run over 34 commands: 0 disappearances, history
  32/32 complete, exactly once, in order, no toolbar leakage.
• 100% line coverage on all four new modules. Full suite 2274 passed / 6 skipped; make check
  and make docs-test clean; new tests also pass under GIL-enabled CPython 3.13 as well as the
  default free-threaded 3.14t build.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.73%. Comparing base (cd63553) to head (0dc711e).
⚠️ Report is 1 commits behind head on reserved_row_toolbar.

Additional details and impacted files

@@                   Coverage Diff                    @@
##           reserved_row_toolbar    #1757      +/-   ##
========================================================
+ Coverage                 99.68%   99.73%   +0.04%     
========================================================
  Files                        28       32       +4     
  Lines                      6771     7428     +657     
========================================================
+ Hits                       6750     7408     +658     
+ Misses                       21       20       -1     

Flag	Coverage Δ
unittests	99.73% <100.00%> (+0.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[tleonhardt]

All eight findings were real and are fixed in 668294a. Each has a test that fails without its fix; I applied the removal of each fix and confirmed the failure, then reverted.

1 — facts and generations captured apart. Both are now taken inside one transaction, and commit additionally checks the batch's own recorded size against the terminal's current one. The generations can agree while the frame was laid out for a different size, and the facts are what the renderer actually branched on, so they are the last word on whether the frame still fits. test_a_batch_prepared_against_a_stale_size_is_not_committed reproduces your injection.

On the second half of that finding: I removed the needs_resynchronization clause from commit rather than adding it. It could not fail — everything that invalidates the terminal already retires the frame in flight, and that retirement is what commit tests. test_a_frame_is_not_committed_while_recovery_is_owed now asserts the retirement itself, so a future path that invalidates without retiring fails there instead of passing an unreachable guard.

2 — recursive preparation and dispatch. _preparing is set before Renderer.render() is invoked, not after it returns, and can_dispatch_input consults it. A second prepare() from inside a content callback returns None. A managed write from inside a layout callback now retires the frame too, since its operations were recorded against a terminal that moved on mid-render.

3 — managed writes between frames. note_managed_write() invalidates the committed baseline rather than only bumping a generation. It takes an optional prompt_anchor, because the layer that emitted the output is the one that knows where it ended; without one, recovery asks the terminal.

4 — painting against a stale geometry. The painter now takes the TerminalDisplay instead of an output plus a geometry, so there is no snapshot to pass. prepare() lays the frame out for the current geometry and records its generation; paint() re-reads the geometry from its owner inside the transaction and refuses if it has changed or the reservation is gone. A refused frame publishes no baseline.

5 — stale anchors. Recovery validates the anchor against the current usable region, forgets it if it no longer fits, and takes the unknown-origin path (a cursor report, or an explicit ReservedModeFailureError when the terminal cannot report).

6 — available height. Recovery sets _min_available_height to U - origin + 1 rather than zero. The cursor was just placed on a known row, so the height below it is known by the same arithmetic a reply would give; the test now asserts the recovered height and height_is_known.

7 — resolved styles. Cells are compared by resolved attributes, not style strings, so a rule change under an unchanged class name repaints. The old test_a_style_only_change_repaints_those_cells used DummyStyle, where bold and "" resolve identically — it was passing for the wrong reason and now uses a real style.

8 — combining marks after wide characters. The mark attaches to the half that carries the text instead of taking a column of its own.

Re-verified: make check, make docs-test clean; 2292 passed / 6 skipped. Fresh isolated coverage (the repo's addopts carries --cov-append, which is what hid line 151 from me — resynchronization_reason now has a test): 100% on all four modules. External gates re-run against the changed painter: dynamic gate PASS at 12/24/40 rows, acceptance.py PASS over 34 commands with history 32/32.

[tleonhardt]

All three fixed in 66dfd22, each with a test that fails without its fix (mutation applied, failure observed, reverted).

1 — a managed write with no origin now forgets the remembered one. note_managed_write() assigns its argument unconditionally, so None means "the cursor moved and I do not know where it ended" rather than "keep believing the old row". Recovery then takes the unknown-origin path and asks the terminal. test_a_managed_write_without_an_origin_forgets_the_old_one asserts the CPR request goes out and no ESC[1;1H does.

Five existing tests drove note_managed_write() and then expected recovery to complete; they now pass prompt_anchor=1, which is what a caller that knows where its output ended would do. That is the honest split: the ones exercising recovery-completes supply an origin, the new one exercises origin discovery.

2 — preparation refuses to publish after emission stops. The post-render check tests reserved_emission_stopped as well as needs_resynchronization. stop_reserved_emission() retires but does not set the recovery flag, so a layout callback that called it left the caller holding an already-retired frame.

3 — the retirement check moved inside the commit transaction. You are right that this is what the earlier removal rested on: retirement can only stand in for an explicit guard if it is read after the terminal has been acquired. Read before the wait it answers a question about a terminal another writer still held, and that writer can retire the batch while the commit queues, changing neither the generations nor the size.

The reproduction injects at the lock handover rather than with two threads: a RetiringLock whose acquire() runs the retirement as it hands the lock over. Two real threads cannot say where the committing thread had got to before the lock was released, and the interleaving that matters is the one where it is already past its own checks — so the handover is the seam. The mutation that moves the check back outside kills the test.

Re-verified: make check clean, 2295 passed / 6 skipped, 100% on all four modules from a cleared .coverage. External gates re-run: dynamic gate PASS at 12/24/40 rows, acceptance.py PASS with history 32/32.

(The test_send_to_paste_buffer failure you saw is a pre-existing environment dependency on a non-empty clipboard, unrelated to this branch — it passes here.)

[tleonhardt]

Both fixed in 7407dd7, each mutation-checked.

1 — recovery's origin is read under the lock. resynchronize() now reads the anchor inside the transaction, not before it. Recovery can queue behind another writer for as long as that writer holds the terminal, and what that writer does meanwhile — emitting output, moving the prompt, resizing — is exactly what changes where the prompt starts, so an origin read beforehand describes a terminal somebody else still owned. Recovery is also published inside the transaction now, in _establish(): whoever takes the terminal next must not find a recovery still owed against work already done. test_recovery_uses_the_origin_it_finds_after_taking_the_terminal uses the same RetiringLock seam; test_recovery_completes_before_the_terminal_is_released pins the ordering.

Only the mode policy is still resolved before the lock, because resolving it runs application filters and the wait contract keeps those off the terminal.

2 — pending cursor requests carry the whole generation tuple. The terminal samples the cursor when it processes the request, so managed output written afterwards moves the very thing the reply describes; a resize was never the only way a reply goes stale. test_a_cursor_report_invalidated_by_managed_output_is_rejected covers your reproduction, and test_replies_still_correlate_by_order_after_one_is_invalidated pins FIFO: the queue is popped whatever the outcome, since dropping a reply without taking it off would answer every later request with its predecessor. The mutation that leaves it on the queue fails that test.

Re-verified: make check and make docs-test clean, 2299 passed / 6 skipped, 100% on all four modules from a cleared .coverage. Dynamic gate PASS at 12/24/40 rows; acceptance.py PASS with history 32/32.

Twenty-three mutations killed on the branch now, seven of them from this and the previous round.

[tleonhardt]

Both fixed in c8a8b32. First, a correction: my previous summary said the three sites were "all read after acquisition". That was wrong about report_cursor_row() — it never took the lock at all, so there was nothing to read it after. You were right to call that out.

1 — cursor reports are validated and published under the lock. report_cursor_row() now runs its whole body in one transaction: queue pop, generation check, band check, anchor publication and the renderer's height update. Split, those are two steps a managed write can land between — the reply passes as current, the write moves the prompt, and the row just recorded is no longer where the prompt is. Managed output reaches the terminal under this same lock, so a reply validated there cannot be overtaken by one.

The request side moved too: the generations are stamped after acquiring the terminal, so a write landing while the request queued no longer makes the reply to a request issued after it look stale. test_a_request_records_the_terminal_it_was_actually_sent_to covers that direction; test_a_reply_cannot_overtake_a_write_that_lands_while_it_waits covers yours.

One deliberate inclusion: settling the renderer's own pending report stays inside the transaction. asyncio.Future.set_result schedules its callbacks on the loop — it is neither a wait nor a synchronous dispatch of application code — so it belongs with the decision it settles rather than after the lock, where it could be reordered against another reply.

2 — recovery rechecks abandoned emission inside its transaction. Rendering can be given up while recovery queues for the terminal, and recovery would otherwise write cursor and mode sequences into a terminal nothing may emit to any more. The pre-lock check was removed rather than kept alongside: with the inner one in place it could not fail, and an unkillable guard is not worth the line.

Re-verified: make check and make docs-test clean, 2302 passed / 6 skipped, 100% on all four modules from a cleared .coverage. Dynamic gate PASS at 12/24/40 rows; acceptance.py PASS with history 32/32. Twenty-six mutations killed on the branch, three from this round.

Four rounds have now found the same shape four times: state read before the lock that the lock exists to protect. The remaining places the bridge reads shared state outside a transaction are prepare() (whose batch is revalidated at commit, which is the design's answer) and the invalidation notes themselves, which are bookkeeping the emitting caller is expected to make inside its own write transaction — Stage 3 wires that, and I would rather it be stated in the Stage 3 contract than assumed.

[tleonhardt]

Fixed in 320b440. request_cursor_position() rechecks abandoned emission inside its own transaction, for the same reason recovery does: before the wait, the answer describes a terminal somebody else still held. Recovery without a known origin gives the terminal back and asks for it again to send the request, so that second acquisition is a second chance to be abandoned.

The handover double now schedules one callback per acquisition (schedule(None, callback)), so an operation that takes the terminal twice can be interrupted at the handover that matters rather than only its first. The test asserts nothing was written and that no request was left queued — a queued request with no emission would answer a later reply that nothing asked for.

Audit of the remaining transaction entry points, so this is checkable rather than assumed. There are six:

• preflight — reads only; if emission is abandoned during its acquisition, the post-render check in prepare() refuses to publish the frame.
• commit — rechecks prepared is self._in_flight inside; stop_reserved_emission() retires, so abandonment is covered by that identity check.
• resynchronize — rechecks stopped inside (round 4).
• cursor position request — rechecks stopped inside (this round).
• cursor position report — validates generations inside (round 4). Deliberately no stopped check: it emits nothing, and the anchor it publishes is bookkeeping that the next emitting operation revalidates. Adding one would be a branch no test could kill.
• paint — revalidates live geometry and generation inside; a released reservation refuses the paint. The painter has no view of reserved_emission_stopped, which is bridge state, and the release that accompanies abandonment is what it sees.

Re-verified: make check and make docs-test clean, 2303 passed / 6 skipped, 100% on all four modules from a cleared .coverage. Dynamic gate PASS at 12/24/40 rows; acceptance.py PASS with history 32/32. Twenty-seven mutations killed on the branch.

Agreed on the Stage 3 invalidation contract — it is recorded in the Stage 2b completion note as an explicit integration requirement, not an inherited assumption: the invalidation notes (note_managed_write and friends) must be made inside the same transaction that emits the managed output, and nothing in Stage 2b can enforce that from where it sits.

[tleonhardt]

Fixed in 0dc711e, and you are right that this invalidates the audit claim — more than that, it overturns my reasoning from the first review round.

I removed the commit-side stopped/recovery guard back then because no test could kill it, and argued retirement was the mechanism it duplicated. That argument holds only while nothing else can publish a frame. A preparation completing concurrently can, and the frame it publishes matches on identity and on every generation, so identity carries no information about whether the terminal is still ours. The guard is back, and now it is killable: test_a_frame_in_flight_while_emission_is_abandoned_is_not_committed and its recovery-owed twin fail without it.

Two changes rather than one:

• The publication window is closed. The post-render check and the assignment to _in_flight are one transaction, so a writer abandoning the reservation between them can no longer have its retirement overwritten. test_a_frame_is_not_published_over_an_abandonment drives it through the handover double at the publication acquisition.
• Commit asks about the terminal's state rather than inferring it. Identity still answers "superseded batch"; the terminal's own state answers "still ours to emit to". Those were conflated.

The two regressions for commit build the inconsistent state directly. That is deliberate: the guard's contract is that a frame in flight while emission is abandoned is uncommittable whatever sequence produced that pair, and a test that could only reach it through today's sequence would go quiet the moment a new one appeared — which is exactly what happened here.

Corrected audit. The claim "preflight and commit are covered by the post-render check and retirement" was wrong. What is true now: preflight reads only, and its frame cannot be published without the publish transaction re-checking; commit checks identity and stopped/recovery state under the lock; resynchronize, cursor position request and publish each recheck the state they depend on inside their own transaction; cursor position report validates generations and emits nothing; paint revalidates live geometry. Seven entry points now, not six.

Re-verified: make check and make docs-test clean, 2306 passed / 6 skipped, 100% on all four modules from a cleared .coverage. Dynamic gate PASS at 12/24/40 rows; acceptance.py PASS with history 32/32. Thirty mutations killed on the branch, three from this round.