Skip to content

Tests wait on owners' receipts, not on the pilot's sleep - #1613

Merged
trissim merged 17 commits into
mainfrom
fix/settle-receipts
Nov 7, 2026
Merged

trissim merged 17 commits into
mainfrom
fix/settle-receipts

Conversation

@trissim

@trissim trissim commented Nov 7, 2026 •

Copy link
Copy Markdown

Toad's tests wait for real receipts, so they pass under a pilot that settles instead of sleeping (OpenHCSDev/textual#257). One product fix rides along: a stale read acknowledgement no longer re-requests work on a suspended source or warns the user.

textual#257 replaces the pilot's 20 ms CPU-time sleeps with a settle on owed work: Textual's suite 40 s → 26.5 s; Toad's keypress-heavy files, serial on one worker, 135.9 s → ~88 s. Under it, 15 Toad tests failed deterministically; each was passing on main only because a sleep happened to cover work no message queue owes. Every fix waits on the owner's receipt for that work; where the receipt was missing or written out by hand, the owner now declares it.

Merge order: textual#257 first (it adds Button.accepts_press, used here), then this PR with the relock.

Receipts the owners now declare

  • StyledRows.rows_ready: a block's rows come from the render processes (styled_rows.py:82). A press landed on AboveThrobber because the response body was still 0 rows tall (test_hover_during_drag). Seven test waits that read _render_worker and _styled from outside now ask the widget.
  • Button.accepts_press (Textual): a Button refuses a press during the previous press's 0.2 s effect. The ACP log test pressed Wrap and Latest twice inside it; one press helper waits on the receipt and replaces the hand-written -active wait.
  • TextualUi.clickable / until_clickable: a click lands on what the published frame shows; the palette's input was clicked before its pushed screen had a frame. One test wrote the check out by hand; textual_steps.command_dialog, a third copy with no caller, is deleted.
  • TextualUi.until_delivery_shown: a sent message's delivery status arrives from Core after its rows and lifts the bottom-anchored page two rows. Two tests pressed a link and released on the status's details link (existing_thread_link, clipboard image).

Waits for the effect the test claims

  • welcome_view: the opened thread's tab is selected (the label mounts first), as test_goal_owner_views already waited.
  • channel_any_mode: the bus records the toggle (the verb runs in Core, off the UI loop).
  • message_notifications: the page measures again once the taller rows land.
  • saved_view_read_only: the chat mounts, then the read-only decision sets status and composer together.
  • tab_scrollbar_top: the underline's slide runs on Toad's effect clock, which wait_for_scheduled_animations never covered; early in a slide from off-strip nothing is highlighted. strip_settled waits on effect_phase(TAB_SLIDE).in_motion.

Product fixes the settle exposed

  • Tab history (test_navigation_keys). The workspace selects a tab early in a switch and recorded the visit only after its later frames, so H then L inside that window counted from the old cursor and landed on the wrong tab (a quick H L does it to a person). TabOrder.navigate moves the cursor when the step is taken; the app's select_session records a visit and show_history_step does not (the step already did), so history_index, threaded through the switch only to re-record it, is deleted and record_visit only appends.
  • Paging on an unbound tab. Uncaptured.read_page raised ValueError("Transcript paging requires a bound thread"), reported as an app error whenever a latest-page request queued before an unbind ran after it. It raises ReadAgain, which execute() already drops.
  • A rewritten session file (transcript_presentation_fencing). When a commit's draw could not publish, its PreparedReplacementViewportRequest was deferred like a reader's request and later drew the commit's rows alone, with nothing recording the painted cursor: the screen showed the new file while reads acknowledged the old one, failed twice and warned "Saved history reloading". A commit's page no longer defers; the commit keeps its demand and retries.

Stale read acknowledgements

TranscriptSource.mark_read runs in the screen's painted-read worker, which suspend() does not join. An ack in flight across retire, close or detach that met a replaced file retried, failed again, called require_commit() on the suspended source and showed "Saved history reloading". The ack now captures generation (the owner of "in-flight view work revoked") and, if the work was revoked while it ran, leaves the read to the work that replaced it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd

A styled block's rows come from the render processes, which no message queue
owes, so a pilot that settles on owed work presses before the rows exist:
test_hover_during_drag's press landed on AboveThrobber because the response
body was still zero rows tall. On Textual main the pilot's 20 ms sleeps hid it.

StyledRows.rows_ready answers it once: the render worker is finished and the
rows are not awaiting styling. Seven test waits restated that fact by reading
_render_worker and _styled from outside; they now ask the widget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
Copilot AI balanced review requested due to automatic review settings November 7, 2026 00:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

…ly mounted

The tab label mounts before the workspace selects its source, so the test read the
welcome's selected session. test_goal_owner_views already waits on the selection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

debt-src at cb2fe5e blocks on 3 lines:

grew BannedVocabulary:src/toad/widgets/styled_rows.py +1
found BannedVocabulary:src/toad/widgets/styled_rows.py at src/toad/widgets/styled_rows.py:56
introduced BannedVocabulary:src/toad/widgets/styled_rows.py at lines [56]

trissim and others added 15 commits November 6, 2026 19:13
The menu's verb runs in Core, off the UI loop, so no settled queue owes it; the
test's claim is the bus state, so it waits on that state in both directions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
Textual's Button refuses a press during the previous press's 0.2 s effect; the
test pressed Wrap and Latest twice inside it once the pilot stopped sleeping.
One press helper waits on Button.accepts_press (OpenHCSDev/textual#257) and
replaces the hand-written -active wait.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
Opening a status's details measures the page once the taller rows return from the
render processes, which no queue owes; the test waits for the measurement it claims.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
…e page

The delivery status arrives from Core after the message rows and lifts the
bottom-anchored page two rows; the click aimed before it pressed the sender
and released on the status's details link.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
The slide is a Toad effect, not a Textual animation, so wait_for_scheduled_animations
never waited for it; early in a 0.18 s slide from off-strip the highlight is not drawn.
strip_settled waits on effect_phase(TAB_SLIDE).in_motion.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
Enter opens the view's tab; its chat mounts after, and the composer is disabled by
the same read-only decision that sets the status, so both are read after it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
Two tests aimed at a message's link before the status row arrived from Core and
lifted the page, so the release toggled the status details instead. One helper,
until_delivery_shown, owns that wait.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
A click lands on what the published frame shows; the palette's input was clicked
before its pushed screen had a frame. TextualUi.clickable owns the check that one
test wrote out by hand; textual_steps.command_dialog, a third copy, had no caller
and goes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
…that replaced it

mark_read runs in the screen's painted-read worker, which suspend() does not join.
An ack that crossed a suspend (retire, close, detach) and met a replaced file
retried, called require_commit() on the suspended source and warned the user.
It now captures generation, the owner of 'in-flight view work revoked', and
returns when the work was revoked while it ran. Traced: an ack started at
generation 3, the source suspended, the ack ended at generation 4.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
…iling the app

Uncaptured answers every other question as 'never current, nothing to do', but its
read_page raised ValueError('Transcript paging requires a bound thread'), which the
source worker reported as an app error whenever a latest-page request queued before
an unbind ran after it. ReadAgain is the owner of 'this answer no longer stands; a
new request owns the advance', and execute() already drops it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
…switch finishes

The workspace selects the new tab early in a switch and records the visit only
after the switch's later frames, so H then L inside that window counted from the
old cursor and landed on the wrong tab (test_navigation_keys failed under the
settled pilot; a quick H L does the same to a person). TabOrder.navigate now moves
the cursor itself. The app's selection and history paths become select_session
(records the visit) and show_history_step (the step is already recorded), so the
history_index threaded through the switch to re-record it is deleted, and
record_visit only appends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
When a commit's draw could not publish, its PreparedReplacementViewportRequest was
deferred like a reader's request and later drew the commit's rows on its own, with
no commit to record the painted cursor. The screen showed the rewritten file while
reads acknowledged the old one: TranscriptChanged twice, then 'Saved history
reloading'. The commit keeps its pending demand and retries itself, so its page no
longer defers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
HistorySourceSnapshot.current rebuilt the owner's snapshot through
query_ancestor(HistoryWindow), so a worker that outlived its history's mount
(comms_chat _refresh_labels -> update_mentions -> source_is_current) raised
NoMatches and failed the app instead of answering 'not current'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
…on owed work)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim
trissim merged commit 9b6b545 into main Nov 7, 2026
2 of 4 checks passed
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.

2 participants