fix(subagents): publish live progress only at widget-visible milestones - #2213
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces disruptive terminal redraws caused by the in-process subagent runner publishing AgentProgress on high-frequency child session events (e.g., streaming deltas). It introduces an explicit progress-emission profile so progress updates are only published at milestones that actually change what the expanded live widget renders, preventing unnecessary above-fold repaints that can trigger pi-tui’s scrollback-clearing full redraw.
Changes:
- Added an exported
progressEmissionFor()table andProgressEmissioncontract to gate which child session events publish progress. - Updated the in-process runner subscription loop to emit progress only for the allowed milestones (forced vs throttled vs none).
- Added unit tests that pin the emission profile and directly measure scrollback-clearing redraw behavior; updated changelogs to document the fix.
Show a summary per file
| File | Description |
|---|---|
| test/unit/subagents-live-widget-scrollback.test.ts | Adds geometry-based tests that count scrollback-clearing writes per publish, validating reduced redraws. |
| test/unit/subagents-inprocess-progress-emission.test.ts | Pins the progressEmissionFor() emission table and “milestones only” behavior across realistic event streams. |
| packages/subagents/src/runs/inprocess/runner.ts | Introduces progressEmissionFor() and applies it to suppress catch-all progress publishing. |
| packages/subagents/CHANGELOG.md | Documents the subagents-side fix in the Unreleased changelog. |
| packages/coding-agent/CHANGELOG.md | Documents the user-visible behavior change in the coding-agent Unreleased changelog. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
|
|
||
| ### Fixed | ||
|
|
||
| - Reduced redundant live subagent repaints by publishing progress only for widget-visible milestones. The in-process runner published `AgentProgress` from a catch-all over every child session event, including the high-frequency `message_update` streaming deltas and `tool_execution_update`, and each publish rewrites the elapsed fields the widget renders. With a subagent expanded via Ctrl+O, that made the live widget repaint for the whole run rather than when its contents changed, and each repaint of a row above the terminal fold costs a destructive pi-tui full redraw that clears terminal scrollback. Progress is now published only on events that change what the widget shows — `agent_start`, `tool_execution_start`, `tool_execution_end`, and (throttled) `message_end` — which is the emission profile foreground subagents had before the in-process runner. Measured against a 20-second replay in the tight geometry, scrollback clears drop from 23 to 3; a live tmux run against the built CLI dropped from 8 clears (7 of them repaints that changed nothing visible) to 3 (none of them). **Genuine above-fold milestone changes can still require a pi-tui redraw**, so a short terminal whose editor/footer region plus the live widget exceed the terminal height will still lose scrollback at tool boundaries; that path is upstream (earendil-works/pi#4785, #7194) and outside this package. The 400 ms throttle, the forced-emit milestones, the depth guard, and the typed status contract are unchanged ([#2205](https://github.com/bastani-inc/atomic/pull/2205)). |
|
|
||
| ### Fixed | ||
|
|
||
| - Reduced redundant repaints of a Ctrl+O-expanded live subagent widget, which were scrolling the chat window to the bottom and clearing terminal scrollback during a run. The widget was republished on every child session event, including assistant streaming deltas, so it repainted continuously instead of when its contents changed; each repaint of a row above the terminal fold costs a pi-tui full redraw that clears scrollback. Progress is now published only at milestones that change what the widget shows. A genuine above-fold milestone change can still require that redraw when the editor/footer region plus the live widget exceed the terminal height — that path is upstream (earendil-works/pi#4785, #7194) and is not fixed here ([#2205](https://github.com/bastani-inc/atomic/pull/2205)). |
flora131
added a commit
that referenced
this pull request
Aug 6, 2026
Copilot review on #2213: the entries linked only to #2205, the regression source, so a reader of the release notes could not trace the fix to the PR that implemented it. Both entries now lead with #2213 and keep #2205 as the named regression source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A Ctrl+O-expanded subagent scrolled the chat to the bottom and wiped terminal scrollback throughout a run. Not present in 0.9.12; introduced by e4aa7ec (#2205). Measured mechanism: pi-tui's TUI.doRender() compares the whole rendered line array and, when the earliest changed row is above previousViewportTop, falls back to fullRender(true), which writes \x1b[2J\x1b[H\x1b[3J -- clear screen, home, clear scrollback. #2205's in-process runner published AgentProgress from a catch-all over every child session event, including message_update streaming deltas, and every publish rewrites the elapsed fields the widget renders. The live widget therefore presented pi-tui with a changed above-fold row continuously rather than when its contents changed. Progress now publishes only for events that change what the widget shows, via an exported progressEmissionFor() table: agent_start, tool_execution_start and tool_execution_end forced, message_end throttled, everything else silent. That is the emission profile foreground subagents had before the in-process runner. The 400 ms throttle, the forced-emit milestones, the depth guard, and the typed status contract are unchanged, and no renderer is touched. Paired live capture against the built CLI under tmux at 20 rows, same task and script, differing only by this change: 73 destructive full redraws (66 of which repainted content the user could not see) drops to 20 (10). Restricted to the subagent's own run, 66 drops to 12, all on genuine content changes. Residual: a genuine above-fold change still costs one pi-tui redraw when rowsBelowWidget + widgetRows > terminalRows. pi-tui 0.83.0 is the newest release and still has this branch; upstream earendil-works/pi#4785 and #7194 track it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot review on #2213: the entries linked only to #2205, the regression source, so a reader of the release notes could not trace the fix to the PR that implemented it. Both entries now lead with #2213 and keep #2205 as the named regression source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
flora131
force-pushed
the
fix/2205-progress-emission
branch
from
August 6, 2026 21:33
8659692 to
dfd67b7
Compare
flora131
added a commit
that referenced
this pull request
Aug 7, 2026
…terior greptile on #2214 reproduced a viewport jumping from entry-10..19 to entry-13..22 after three entries above the reader were removed. The finding is correct, and the defect is larger than the report suggests. rowsShiftedAboveAnchor compared one scalar row count per component and broke at the first component whose end passed anchorRow, on the stated assumption that a spanning component's changes happen at or below the anchor. In production that assumption never holds: the chat host renders the ENTIRE transcript as a single component in slot 0 (chat-session-host-rendering.ts:23-25), so every scrolled-up reader's anchor is inside it, the loop broke immediately, and the compensation this PR added returned 0 for all of them. It engaged only when the anchor sat past the transcript, parked on the trailing spacer. A production path deletes from that component's interior: compaction_end -> refreshCompactedTranscript -> replaceMessages, which splices the entry array in place (chat-message-renderer.ts:160-164). Windowed components may now report a row map, `rowSegments(width)`, identifying each run of rows by the entry that produced it. For a spanning component the shift is the distance the anchored segment itself moved, rather than a height delta. Segments are matched by object identity, not by cache key. Keys embed the entry index (chat-session-host-rendering.ts:167-173), so a splice renumbers every survivor; the entry objects move through it unchanged. Known limits, deliberate: - A real compaction_end rebuilds entry objects via chatEntriesFromAgentMessages, so only extraEntries keep identity. A viewer anchored in the rebuilt region falls back to the nearest surviving segment above, then below, and lands close rather than exact. - No cross-component identity; duplicate ids resolve to first occurrence. - One small object per entry per measured frame, unmemoized. - Components without rowSegments keep the previous break semantics. Two tests added, both verified to fail with only the consuming branch reverted to `break;` -- entry-10 -> entry-13 and entry-10 -> entry-16 -- and to pass once restored byte-identically. All six existing anchor tests are unchanged and still pass; the only pre-existing line touched in that file is an import. Also repairs the [Unreleased] changelog structure in this file. My conflict resolutions while rebasing #2213/#2218 concatenated bullets without respecting headings, leaving `### Removed` with no blank line before it and two Fixed bullets underneath it. That malformed section is already on main; all six bullets are preserved and no released section is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
flora131
added a commit
that referenced
this pull request
Aug 7, 2026
…terior greptile on #2214 reproduced a viewport jumping from entry-10..19 to entry-13..22 after three entries above the reader were removed. The finding is correct, and the defect is larger than the report suggests. rowsShiftedAboveAnchor compared one scalar row count per component and broke at the first component whose end passed anchorRow, on the stated assumption that a spanning component's changes happen at or below the anchor. In production that assumption never holds: the chat host renders the ENTIRE transcript as a single component in slot 0 (chat-session-host-rendering.ts:23-25), so every scrolled-up reader's anchor is inside it, the loop broke immediately, and the compensation this PR added returned 0 for all of them. It engaged only when the anchor sat past the transcript, parked on the trailing spacer. A production path deletes from that component's interior: compaction_end -> refreshCompactedTranscript -> replaceMessages, which splices the entry array in place (chat-message-renderer.ts:160-164). Windowed components may now report a row map, `rowSegments(width)`, identifying each run of rows by the entry that produced it. For a spanning component the shift is the distance the anchored segment itself moved, rather than a height delta. Segments are matched by object identity, not by cache key. Keys embed the entry index (chat-session-host-rendering.ts:167-173), so a splice renumbers every survivor; the entry objects move through it unchanged. Known limits, deliberate: - A real compaction_end rebuilds entry objects via chatEntriesFromAgentMessages, so only extraEntries keep identity. A viewer anchored in the rebuilt region falls back to the nearest surviving segment above, then below, and lands close rather than exact. - No cross-component identity; duplicate ids resolve to first occurrence. - One small object per entry per measured frame, unmemoized. - Components without rowSegments keep the previous break semantics. Two tests added, both verified to fail with only the consuming branch reverted to `break;` -- entry-10 -> entry-13 and entry-10 -> entry-16 -- and to pass once restored byte-identically. All six existing anchor tests are unchanged and still pass; the only pre-existing line touched in that file is an import. Also repairs the [Unreleased] changelog structure in this file. My conflict resolutions while rebasing #2213/#2218 concatenated bullets without respecting headings, leaving `### Removed` with no blank line before it and two Fixed bullets underneath it. That malformed section is already on main; all six bullets are preserved and no released section is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
flora131
added a commit
that referenced
this pull request
Aug 7, 2026
…#2214) * fix(coding-agent): keep the scroll anchor stable when content shrinks Separate pre-existing defect, not a #2205 regression: chat-transcript.ts is byte-identical between 0.9.12 and 0.9.13-alpha.1. Found while falsifying the initial hypothesis for the Ctrl+O autoscroll bug, which the layer below this one actually fixes. ScrollableComponentViewport stores its offset as a distance from the bottom and compensated it only when content grew: if (this.scrollFromBottom > 0 && this.lastWidth === width && lineCount > this.lastLineCount) { this.scrollFromBottom += lineCount - this.lastLineCount; } A component that loses rows -- a live subagent widget dropping its current-tool row at the end of a tool call -- left scrollFromBottom unchanged, and clampScroll() then pulled the view toward the bottom against a smaller maxScroll. Compensation is now symmetric in both directions, so a user scrolled up stays anchored whether content grows or shrinks, while a user already at the bottom still sticks to it. Scope note: ScrollableComponentViewport is used by ChatSessionHost, not by the normal interactive chat transcript, so this does not by itself change the reported Ctrl+O behavior. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(coding-agent): keep the anchor when a component shrinks in its interior greptile on #2214 reproduced a viewport jumping from entry-10..19 to entry-13..22 after three entries above the reader were removed. The finding is correct, and the defect is larger than the report suggests. rowsShiftedAboveAnchor compared one scalar row count per component and broke at the first component whose end passed anchorRow, on the stated assumption that a spanning component's changes happen at or below the anchor. In production that assumption never holds: the chat host renders the ENTIRE transcript as a single component in slot 0 (chat-session-host-rendering.ts:23-25), so every scrolled-up reader's anchor is inside it, the loop broke immediately, and the compensation this PR added returned 0 for all of them. It engaged only when the anchor sat past the transcript, parked on the trailing spacer. A production path deletes from that component's interior: compaction_end -> refreshCompactedTranscript -> replaceMessages, which splices the entry array in place (chat-message-renderer.ts:160-164). Windowed components may now report a row map, `rowSegments(width)`, identifying each run of rows by the entry that produced it. For a spanning component the shift is the distance the anchored segment itself moved, rather than a height delta. Segments are matched by object identity, not by cache key. Keys embed the entry index (chat-session-host-rendering.ts:167-173), so a splice renumbers every survivor; the entry objects move through it unchanged. Known limits, deliberate: - A real compaction_end rebuilds entry objects via chatEntriesFromAgentMessages, so only extraEntries keep identity. A viewer anchored in the rebuilt region falls back to the nearest surviving segment above, then below, and lands close rather than exact. - No cross-component identity; duplicate ids resolve to first occurrence. - One small object per entry per measured frame, unmemoized. - Components without rowSegments keep the previous break semantics. Two tests added, both verified to fail with only the consuming branch reverted to `break;` -- entry-10 -> entry-13 and entry-10 -> entry-16 -- and to pass once restored byte-identically. All six existing anchor tests are unchanged and still pass; the only pre-existing line touched in that file is an import. Also repairs the [Unreleased] changelog structure in this file. My conflict resolutions while rebasing #2213/#2218 concatenated bullets without respecting headings, leaving `### Removed` with no blank line before it and two Fixed bullets underneath it. That malformed section is already on main; all six bullets are preserved and no released section is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(coding-agent): let a static transcript report its rows to the anchor greptile raised the same drift a third time after the previous push, this time naming ScrollableChatTranscriptComponent. The thread arrived marked resolved; the code disagreed, and the reviewer was right. That class builds its transcript WITHOUT a cache key (:505), so supportsRowWindow is false, rowSegments returned [] for anything not windowed, and isRowWindowComponent gated the viewport away from asking. The previous commit therefore fixed the ChatSessionHost path -- which does pass a cache key -- and left the exported class exactly as broken. Passing it a cache key would have been the obvious fix and is wrong: chat-message-renderer.test.ts:188-198 pins that a transcript without one reflects entries mutated in place, which is precisely what caching by key would miss. The absent key is the feature. So identifying rows is decoupled from windowing. renderAllRows already walks every entry and knows each block's height, so it records them as it goes -- no extra render, and it cannot go stale against an in-place mutation the way a key can. rowSegments returns those on the static path, guarded by the width they were measured at. The viewport reads segments from static components too, after rendering them, since the recording is a by-product of that render. Two tests added against the exported class: entries removed above the viewer keep the anchored entry, and an entry mutated in place is still reflected -- the second guards the regression the cache-key approach would have caused. Reverting only the static-segment plumbing fails the first with entry-4 -> entry-7 while the other nine pass, so it is specific to this gap. npm run check clean; 626 files / 5968 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Norin Lavaee <nlavaee@umich.edu> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A Ctrl+O-expanded subagent scrolled the chat to the bottom and wiped
terminal scrollback throughout a run. Not present in 0.9.12; introduced
by e4aa7ec (#2205).
Measured mechanism: pi-tui's TUI.doRender() compares the whole rendered
line array and, when the earliest changed row is above previousViewportTop,
falls back to fullRender(true), which writes \x1b[2J\x1b[H\x1b[3J --
clear screen, home, clear scrollback. #2205's in-process runner published
AgentProgress from a catch-all over every child session event, including
message_update streaming deltas, and every publish rewrites the elapsed
fields the widget renders. The live widget therefore presented pi-tui with
a changed above-fold row continuously rather than when its contents changed.
Progress now publishes only for events that change what the widget shows,
via an exported progressEmissionFor() table: agent_start,
tool_execution_start and tool_execution_end forced, message_end throttled,
everything else silent. That is the emission profile foreground subagents
had before the in-process runner. The 400 ms throttle, the forced-emit
milestones, the depth guard, and the typed status contract are unchanged,
and no renderer is touched.
Paired live capture against the built CLI under tmux at 20 rows, same task
and script, differing only by this change: 73 destructive full redraws
(66 of which repainted content the user could not see) drops to 20 (10).
Restricted to the subagent's own run, 66 drops to 12, all on genuine
content changes.
Residual: a genuine above-fold change still costs one pi-tui redraw when
rowsBelowWidget + widgetRows > terminalRows. pi-tui 0.83.0 is the newest
release and still has this branch; upstream earendil-works/pi#4785 and
#7194 track it.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Stack created with GitHub Stacks CLI • Give Feedback 💬
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Greptile Summary
The in-process subagent runner now publishes live progress only when the expanded widget has meaningful new information to display. Runtime replay and focused widget tests confirm that agent start and tool boundaries still publish immediately, message completion remains throttled, and streaming updates no longer trigger redundant redraws.
Confidence Score: 5/5
Safe to merge: the changed progress-emission behavior preserves visible lifecycle and tool updates while reducing unnecessary terminal redraws.
No blocking failure remains.
What T-Rex did
Reviews (3): Last reviewed commit: "docs(changelog): attribute the emission ..." | Re-trigger Greptile