Skip to content

Commit 59f705e

Browse files
committed
Fix tool-card row estimate over-counting (args + args_streaming double-count); over-count is the unsafe direction for the mid-run trim
1 parent 8f77ae7 commit 59f705e

1 file changed

Lines changed: 44 additions & 26 deletions

File tree

src/runtime/app/update/frozen.cpp

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,12 @@ std::size_t wrapped_rows(std::string_view body, int cols) {
151151
// tree. A tool card renders its big payload (write `content`, edit
152152
// `edits[].new_text`/`old_text`, etc.) one row per source line, so the
153153
// real rendered height tracks newlines in those strings — NOT the JSON
154-
// byte length. Summing per-string wrapped_rows captures multi-line
155-
// payloads accurately and biases toward OVER-counting (the safe
156-
// direction for the trim's "provably above the viewport" proof).
157-
// Non-string scalars contribute nothing (they render inline in the
158-
// header, not the body). Cheap: no allocation, no dump().
154+
// byte length. Summing per-string wrapped_rows counts each payload's
155+
// real source lines (plus wrap), which tracks the rendered height
156+
// closely without OVER-counting — the safe side for the mid-run trim's
157+
// keep-loop (see estimate_msg_rows). Non-string scalars contribute
158+
// nothing (they render inline in the header, not the body). Cheap: no
159+
// allocation, no dump().
159160
std::size_t estimate_json_string_rows(const nlohmann::json& j, int cols) {
160161
switch (j.type()) {
161162
case nlohmann::json::value_t::string:
@@ -185,30 +186,36 @@ std::size_t estimate_msg_rows(const Message& mm) {
185186
if (!mm.streaming_text.empty()) rows += wrapped_rows(mm.streaming_text, cols);
186187

187188
for (const auto& tc : mm.tool_calls) {
188-
// The RENDERED body of a settled tool card is one ROW PER SOURCE
189-
// LINE (line-numbered write, per-hunk edit diff, read/grep
190-
// output), not bytes/width. The old `body_bytes / w` estimate
191-
// catastrophically UNDER-counted any body with many short lines:
192-
// a 300-line write of ~20-char lines is ~300 rendered rows but
193-
// bytes/width put it at ~79. That under-count is the root of the
194-
// scrollback duplication — trim_frozen_above_viewport's "provably
195-
// above the viewport" proof reads these row counts, and an
196-
// under-counted (still on-screen) entry gets dropped, re-emitting
197-
// committed scrollback rows shifted = the turn appears twice.
198-
// Count newlines in the rendered text sources instead; wrapping
199-
// only ADDS rows, so newline-count is a safe LOWER bound that's
200-
// far closer to reality, and we keep the byte term as an extra
201-
// over-count cushion for wrapped long lines.
189+
// The RENDERED body of a tool card is one ROW PER SOURCE LINE
190+
// (line-numbered write, per-hunk edit diff, read/grep output),
191+
// not bytes/width — a 300-line write of ~20-char lines is ~300
192+
// rendered rows, which bytes/width put at ~79.
193+
//
194+
// This count must NOT OVER-estimate. It feeds two trims, and the
195+
// mid-run one (trim_frozen_above_viewport) walks the NEWEST
196+
// entries summing rows until it has "kept a viewport", then drops
197+
// the rest as off-screen. If a kept entry over-counts, the keep
198+
// sum reaches the viewport margin before the real rows do, so the
199+
// proof drops an entry whose real rows are STILL on screen —
200+
// re-emitting committed scrollback rows = the duplication ghost.
201+
// An UNDER-count only keeps more (a taller canvas, never a
202+
// dropped on-screen entry), so under/exact is the safe side.
203+
//
204+
// The body shows up in exactly one source per tool state:
205+
// • settled → parsed `args` (write content / edit hunks) or
206+
// `output()` (read/grep result). args wins; we do
207+
// NOT also add args_streaming, which holds the
208+
// SAME bytes in raw-JSON form (append-only, never
209+
// cleared) and would double the count.
210+
// • streaming→ args not parsed yet; the live card renders from
211+
// args_streaming, so count that instead.
202212
std::size_t tool_rows = 0;
203-
if (!tc.output().empty())
204-
tool_rows += wrapped_rows(tc.output(), cols);
205-
if (!tc.args_streaming.empty())
206-
tool_rows += wrapped_rows(tc.args_streaming, cols);
207-
// Args JSON: the big body is a string field (write `content`,
208-
// edit `edits[].new_text`/`old_text`). Count newlines across all
209-
// string values so a multi-line payload counts its real lines.
210213
if (!tc.args.is_null())
211214
tool_rows += estimate_json_string_rows(tc.args, cols);
215+
else if (!tc.args_streaming.empty())
216+
tool_rows += wrapped_rows(tc.args_streaming, cols);
217+
if (!tc.output().empty())
218+
tool_rows += wrapped_rows(tc.output(), cols);
212219
rows += tool_rows;
213220
// Header / footer / chrome rows per tool card (~4 rows even
214221
// for an empty body — title, divider, status, blank). Fixed
@@ -917,6 +924,17 @@ maya::Cmd<Msg> trim_frozen_above_viewport(Model& m) {
917924
// floor) so the canvas stays near a single screen during a long run
918925
// — per-frame cost (clear + verify + blit) stays flat and minimal —
919926
// while never racing the visible region.
927+
//
928+
// PROOF DEPENDS ON: frozen_rows[k] never OVER-counting an entry's
929+
// real rendered height. The keep-loop stops once the ESTIMATED sum
930+
// of kept entries reaches kViewportKeepRows; if an estimate were
931+
// high, the loop would stop early and the kept entries' REAL rows
932+
// could fall short of a viewport, leaving a dropped entry on screen
933+
// (the duplication ghost). estimate_msg_rows is built to under-count
934+
// or hit exactly, never over (see its body), so estimated-kept >=
935+
// margin implies real-kept >= margin >= term_h. An under-count only
936+
// keeps a few extra entries — a slightly taller canvas, never a
937+
// corrupt one.
920938
// Query terminal geometry through the SAME helper the row estimate
921939
// uses (term_dims) so the keep-margin and the per-entry row counts
922940
// are computed against one consistent terminal state — a width/height

0 commit comments

Comments
 (0)