Skip to content

Widget owns tail-follow transitions for native anchor paths (Toad #64) - #136

Closed
trissim wants to merge 2 commits into
mainfrom
fix/anchor-owned-bindings
Closed

trissim wants to merge 2 commits into
mainfrom
fix/anchor-owned-bindings

Conversation

@trissim

@trissim trissim commented Oct 16, 2026 •

Copy link
Copy Markdown

Repair at the owner — OpenHCSDev/toad#64 (kept open)

Actual PageUp trace:119 of120 binding notifications came from HistoryWindow.watch_scroll_y → Window.tail_follow_changed → refresh_bindings, not focus changes. Stop the source emission; Footer projection/comparison/cache is untouched.

Fact / behavior Before After
Tail-follow predicate Toad HistoryWindow reads native flags One declared property on native Widget; Toad inherits it
Tail transition notification Toad anchor/release/every scroll offset Widget's one state writer; native anchor/release/check/scroll-end all consume it
Jump/action projection Window override Same Window override consumes native hook; mount still projects initial state
Reader intent/source/preparation HistoryWindow and existing owners Unchanged revisions, restoration scopes, source-tail checks and fences

Twelve Toad production lines deleted; no new class, store, cache, worker, timer or cap. All anchor writes and consumers enumerated before editing. Actual search rg -n 'def follows_tail' src/toad ../textual-main/src/textual finds exactly one declaration, textual/widget.py:692. Toad no longer writes either native anchor flag; native initialization and _set_anchor_state own writes.

Paired Textual#136 2fea115fc / Toad#685 0d91afbeb. Pin and lock use that dependency; current main integrated normally, retaining Core493878cde and unrelated Send/Vim/status work. No installed package changed.

Actual application measurements

Plain st150×47/:197, normal launcher, original saved core-route-counts; one10s run per phase, physical120-key/40ms held PageUp. Growing saved state, not frozen-source controlled A/B/full acceptance. Original Textual18f4 and Textual-main134 have identical trees; the measured pair excludes unrelated native changes. End in the focused editor is caret movement, not source-tail reset.

Before After
Frames / paired inputs 999 /119 977 /120
Tail-triggered binding refreshes 1450 1 legitimate release
Frame median/p95/worst ms 6.5 /22.7 /40.2 4.6 /18.0 /46.0
Input-to-paint median/p95 ms 7.8 /25.3 7.0 /28.5

No snapshot overlaps. Causal stacks after repair contain one release plus one unrelated backend/prompt update, no scroll-watcher emission. Median/p95 callback work improved here; input p95 and worst frame did not. 7ms/full throughput target remains unmet. Retained fixed-key scopes17.81/54.93ms cover22/92 callbacks and different intervening work, neither Footer reconciliation: not an equal-work ratio or subtraction of the old3.31ms footer sample.

Compositor report — OpenHCSDev/toad#61, no compositor patch

The old6.49ms sample's51 arrange_widget calls are traversal including leaves, not51 child-layout computations. Its23 Widget.arrange calls total0.27ms; cached-resource restoration2.32ms includes projection/logical membership. Eight warm reflows visit51–52 widgets, call container arrange24–25 times, recompute0–7 arrangements. Recomputations are chrome ancestors: WorkspaceScreen, Horizontal, Container, MainScreen, Center, Vertical; ChannelsSidebar once. Other container calls include Conversation/AgentWindow, prompt/groups, GoalBar, VerticalScroll, SessionDetails, sidebars, header/tabs/history controls.

Warm captures do not newly arrange transcript pages. One cold complete61-visit capture includes TranscriptHistory/two TranscriptPageViews/HistoryEdge; the older59-row page(y−127) and one-row edge(y−128) are outside clip y3–27. Cached logical membership still runs: not zero offscreen work or scaling acceptance.

Verification / limits

  • Actual source application: PageUp release; explicit source-latest; native scroll_end; natural PageDown rejoin; anchor(False); sidebar focus and prompt return. Jump bar/action match native state. Focus changes footer six prompt actions→two sidebar actions→six. Draft/selection preserved empty. Profiler/switch interval/GC restored as found.
  • Current-main source smoke uses actual installed Core493878cde, not Core source overlay; module receipt retained. Footer pair remains source-verified, not installed. Final current-main smoke is separate from timing comparison: PageUp release→source-latest83/83/no-newer/empty draft→clean quit. All owned frontend/ACP/observer/pool PIDs exited; no traceback/reset/EBADF/unraisable text. Core#946 is merged/installed; this bounded actual-Core retirement passes, not the original exact GC ordering or full status latency.
  • AST: Toad282 / Textual254 modules before/after and after main merge, no parse omissions. Relevant declarations, imports/inheritance, writes, decisions and consumers retained. Dynamic/C/event-loop behavior not statically certified.
  • Final Toad ratchet: no positive deltas. Native Widget GodClassExcess+14 for load-bearing owner behavior; no zero-debt claim.
  • No automated Toad tests, synthetic application, provider input, original replay or build. Original sessions/BLOCKED/UNKNOWN untouched.

Canonical results: docs/performance/scroll-series.md. Evidence: .artifacts/footer-11046/ and /home/ts/.cache/agent-scratch/ux-footer-11046/ (series, causal traces, profiles, compositor census, real-app receipts). Also corrects the earlier81.97ms figure: summed callbacks across486ms between paints, largest individual callback10.21ms—not one82ms block. OpenHCSDev/toad#61 and OpenHCSDev/toad#64 stay open.

@trissim
trissim marked this pull request as ready for review October 16, 2026 05:59
Copilot AI balanced review requested due to automatic review settings October 16, 2026 05:59

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 reached their quota limit.

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.

@trissim
trissim marked this pull request as draft October 16, 2026 07:46
@trissim

trissim commented Oct 21, 2026

Copy link
Copy Markdown
Author

Closing under maintainer P402's explicit rule: the named PageUp binding-refresh storm no longer exists on current main.

Quoted actual current-main evidence, Textual d752616156b11ed3756ddef861a148c3284f6c8d, current Toad source 996dc0172, runtime core-d62f11177-lock-f98aa284dfb54016:

physical PageUp: 120 keys, 40 ms spacing
scroll_calls: 676
tail_calls: 677
tail_binding_refreshes: 1
all_binding_refreshes: 1
follows_tail: true -> false
exit: 0

This is real Toad/Linux driver in plain st on isolated owned Xvfb, a private Core thread containing saved history, actual binding via CommsUpdated, and original methods observed rather than replaced. Current Window.tail_follow_changed in src/toad/widgets/conversation.py already contains:

jump = self.can_jump_to_latest
if self.jump_bar.display == jump:
    return

The binding call follows that return, so unchanged scroll offsets no longer emit bindings. A passing exit alone was not used as evidence: the original refresh calls were counted.

The original PR is preserved, not rewritten: merged current fork main into fix/anchor-owned-bindings, pushed head 9cba314d29bae6a7abf12bb38f42aa532aee795e, verified git ls-remote equals it. The same real 120-key run on that branch gives scroll_calls649 / tail_calls651 / tail_binding_refreshes1 / all_binding_refreshes1, follows_tail true->false, exit0. There is no second reduction in the named defect to ship here. This comparison is a refresh-count proof, not an equal-work performance claim.

Receipts: /home/ts/.cache/agent-scratch/fork-external/P402-136-main.json and P402-136-original.json; physical st I/O logs alongside. Existing whole-root refactor-audit inventory and semantic reading cover the anchor writers/consumers and Window's override. Dynamic dispatch/native behavior are not inferred from AST.

No tests added, local suite/ratchets run, provider input, installed package edits or Toad edits. The original native ownership refactor remains published on its branch; this closure does not claim all tail-follow architecture or scroll-performance targets are solved. Toad #64 stays open. AI-assisted qualification by fork-external.

@trissim trissim closed this Oct 21, 2026
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