Skip to content

Commit fd23d1a

Browse files
authored
Fix streaming performance bottlenecks in thinking blocks and redisplay (dnouri#153)
* Fix streaming performance bottlenecks in thinking blocks and redisplay Thinking-block rendering was O(n) in accumulated content: every delta deleted and reinserted the entire blockquote region. Track previously rendered text and append only the new suffix when possible, falling back to full rewrite only when normalization alters earlier content. Bind inhibit-redisplay in the process filter so that multiple JSON lines delivered in one read() produce a single redisplay cycle. Filter function-based entries from treesit-range-settings when harvesting embedded-mode configs in md-ts-mode. Such entries create host-level parsers that re-parse the whole buffer on every redisplay. * pi-coding-agent-render.el (pi-coding-agent--render-thinking-content): Use incremental suffix insertion; track state in new variable pi-coding-agent--thinking-prev-rendered. (pi-coding-agent--reset-thinking-state): Clear it. * pi-coding-agent-ui.el (pi-coding-agent--thinking-prev-rendered): New. * pi-coding-agent-core.el (pi-coding-agent--process-filter): Bind inhibit-redisplay around the dispatch loop. * md-ts-mode.el (md-ts--add-config-for-mode): Keep only query-based range settings; drop function-based ones. * test/pi-coding-agent-core-test.el: Test that inhibit-redisplay is bound during dispatch. * test/pi-coding-agent-render-test.el: Test incremental suffix path, tool-header short-circuit, and range-settings filtering. * Sync md-ts-mode.el with upstream (dnouri/md-ts-mode bd2ac57) Picks up two fixes from upstream that affect streaming sessions: Workaround for tree-sitter < 0.25.0 integer underflow in local parsers. After an edit outside a local parser's included range, unsigned subtraction in length_sub/point_sub corrupts point fields and makes incremental reparse silently return zero query matches. Inline markup (backticks, bold, links) fails to fontify when content is inserted line by line, as happens during LLM streaming. The workaround recreates each local parser when the buffer has been modified since the parser was last updated, forcing a full reparse. Filter function-based range settings when harvesting embedded mode configs. Function-based entries create host-level parsers that re-parse the whole buffer as the embedded language on every redisplay cycle. The range-settings filter test moves to the upstream test suite; remove the local copy from pi-coding-agent-render-test.el.
1 parent 3d4510b commit fd23d1a

6 files changed

Lines changed: 162 additions & 19 deletions

‎md-ts-mode.el‎

Lines changed: 78 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,38 @@ BEG, END, OFFSET, and RANGE-FN are passed through."
342342
(md-ts--treesit-query-range
343343
parser query beg end offset range-fn)))))
344344

345+
;; WORKAROUND: tree-sitter < 0.25.0 integer underflow in
346+
;; `length_sub'/`point_sub'. After an edit outside a local parser's
347+
;; included range, unsigned subtraction underflows, corrupting point
348+
;; fields and making incremental reparse silently produce zero query
349+
;; matches. Fixed upstream by commit f3d50f27 (ts 0.25.0, 2025-01-31).
350+
;;
351+
;; Workaround: replace the parser with a fresh one so
352+
;; `ts_parser_parse' does a full (non-incremental) reparse.
353+
;;
354+
;; REMOVAL: once tree-sitter >= 0.25.0 can be assumed, delete this
355+
;; function and its two call-sites (in
356+
;; `md-ts--treesit--update-ranges-local' and
357+
;; `md-ts--refresh-local-parsers').
358+
(defun md-ts--recreate-local-parser (ov old-parser)
359+
"Delete OLD-PARSER on OV and create a fresh replacement.
360+
Return the new parser, or nil if creation fails."
361+
(let ((lang (treesit-parser-language old-parser))
362+
(embed-level
363+
(when (fboundp 'treesit-parser-embed-level)
364+
(funcall (intern "treesit-parser-embed-level") old-parser))))
365+
(treesit-parser-delete old-parser)
366+
(let ((new-parser
367+
(condition-case nil
368+
(md-ts--parser-create lang nil t 'embedded)
369+
(treesit-load-language-error nil))))
370+
(when new-parser
371+
(when embed-level
372+
(funcall (intern "treesit-parser-set-embed-level")
373+
new-parser embed-level))
374+
(overlay-put ov 'treesit-parser new-parser))
375+
new-parser)))
376+
345377
(defun md-ts--treesit--update-ranges-local
346378
(query embedded-lang modified-tick &optional beg end offset range-fn)
347379
"Update ranges for local parsers between BEG and END.
@@ -365,11 +397,19 @@ OFFSET, and RANGE-FN control overlay timestamps and range computation."
365397
(parser-lang (treesit-parser-language
366398
embedded-parser)))
367399
(when (eq parser-lang lang)
368-
(treesit-parser-set-included-ranges
369-
embedded-parser `((,beg . ,end)))
370-
(move-overlay ov beg end)
371-
(overlay-put ov 'treesit-parser-ov-timestamp
372-
modified-tick)
400+
(let ((ov-tick (overlay-get
401+
ov
402+
'treesit-parser-ov-timestamp)))
403+
(when (not (eql ov-tick modified-tick))
404+
(setq embedded-parser
405+
(md-ts--recreate-local-parser
406+
ov embedded-parser))))
407+
(when embedded-parser
408+
(treesit-parser-set-included-ranges
409+
embedded-parser `((,beg . ,end)))
410+
(move-overlay ov beg end)
411+
(overlay-put ov 'treesit-parser-ov-timestamp
412+
modified-tick))
373413
(throw 'done t)))))))
374414
(when (not has-parser)
375415
(let ((embedded-parser
@@ -447,6 +487,32 @@ If BEG and END are non-nil, only update ranges in that region."
447487
(fset (car pair) (symbol-function (cdr pair))))
448488
(setq md-ts--range-shims-installed t))
449489

490+
;; On Emacs 31+ the native `treesit--update-ranges-local' is used
491+
;; (our shims are not installed), so the workaround from
492+
;; `md-ts--recreate-local-parser' must be applied via :before advice
493+
;; on `treesit-update-ranges'. It must be :before because the native
494+
;; function stamps overlays with the current tick.
495+
(unless md-ts--range-shims-installed
496+
(defun md-ts--refresh-local-parsers (&optional beg end)
497+
"Replace local parsers whose buffer was modified since last update.
498+
Workaround for tree-sitter < 0.25.0 integer underflow — see
499+
`md-ts--recreate-local-parser' for details.
500+
Must run as :before advice on `treesit-update-ranges'."
501+
(let ((tick (buffer-chars-modified-tick))
502+
(beg (or beg (point-min)))
503+
(end (or end (point-max))))
504+
(dolist (ov (overlays-in beg end))
505+
(when-let* ((old-parser (overlay-get ov 'treesit-parser))
506+
((treesit-parser-language old-parser))
507+
(ov-tick (overlay-get ov 'treesit-parser-ov-timestamp)))
508+
(when (not (eql ov-tick tick))
509+
(when-let* ((new-parser
510+
(md-ts--recreate-local-parser ov old-parser)))
511+
(treesit-parser-set-included-ranges
512+
new-parser `((,(overlay-start ov) . ,(overlay-end ov))))))))))
513+
(advice-add 'treesit-update-ranges :before
514+
'md-ts--refresh-local-parsers))
515+
450516
;; Emacs 29 font-lock polyfill — local parser support.
451517
;;
452518
;; Emacs 29's `treesit-font-lock-fontify-region' calls
@@ -986,9 +1052,13 @@ Return non-nil on success, nil if MODE could not be harvested."
9861052
(setq treesit-simple-indent-rules
9871053
(append treesit-simple-indent-rules
9881054
(plist-get configs :simple-indent)))
989-
(setq treesit-range-settings
990-
(append treesit-range-settings
991-
(plist-get configs :range)))
1055+
;; Only keep query-based range settings; function-based entries
1056+
;; create host-level parsers that re-parse the whole buffer.
1057+
(let ((safe-ranges (seq-filter
1058+
(lambda (s) (not (functionp (nth 0 s))))
1059+
(plist-get configs :range))))
1060+
(setq treesit-range-settings
1061+
(append treesit-range-settings safe-ranges)))
9921062
(setq-local indent-line-function #'treesit-indent)
9931063
(setq-local indent-region-function #'treesit-indent-region)
9941064
t)))

‎pi-coding-agent-core.el‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,9 @@ Returns nil on timeout."
131131

132132
(defun pi-coding-agent--process-filter (proc output)
133133
"Handle OUTPUT from pi PROC.
134-
Accumulates output and processes complete JSON lines.
135-
Uses process property for per-process partial output storage."
136-
(let* ((partial (or (process-get proc 'pi-coding-agent-partial-output) ""))
134+
Accumulates output and dispatches complete JSON lines."
135+
(let* ((inhibit-redisplay t)
136+
(partial (or (process-get proc 'pi-coding-agent-partial-output) ""))
137137
(result (pi-coding-agent--accumulate-lines partial output))
138138
(lines (car result)))
139139
(process-put proc 'pi-coding-agent-partial-output (cdr result))

‎pi-coding-agent-render.el‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -217,14 +217,28 @@ Returns non-nil when meaningful content remains after normalization."
217217
(end (marker-position pi-coding-agent--thinking-marker))
218218
(normalized (pi-coding-agent--thinking-normalize-text
219219
pi-coding-agent--thinking-raw))
220-
(rendered (pi-coding-agent--thinking-blockquote-text normalized)))
220+
(rendered (pi-coding-agent--thinking-blockquote-text normalized))
221+
(prev pi-coding-agent--thinking-prev-rendered))
221222
(when (<= start end)
222-
(let ((existing (buffer-substring-no-properties start end)))
223-
(unless (equal existing rendered)
224-
(goto-char start)
225-
(delete-region start end)
226-
(insert rendered)
227-
(set-marker pi-coding-agent--thinking-marker (point)))))
223+
(cond
224+
;; Fast path: new rendered text extends previous — just append suffix.
225+
((and prev
226+
(not (string-empty-p prev))
227+
(string-prefix-p prev rendered))
228+
(let ((suffix (substring rendered (length prev))))
229+
(unless (string-empty-p suffix)
230+
(goto-char end)
231+
(insert suffix)
232+
(set-marker pi-coding-agent--thinking-marker (point)))))
233+
;; Slow path: full rewrite; skip if buffer already matches.
234+
(t
235+
(let ((existing (buffer-substring-no-properties start end)))
236+
(unless (equal existing rendered)
237+
(goto-char start)
238+
(delete-region start end)
239+
(insert rendered)
240+
(set-marker pi-coding-agent--thinking-marker (point))))))
241+
(setq pi-coding-agent--thinking-prev-rendered rendered))
228242
(and (<= start end)
229243
(not (string-empty-p normalized))))))
230244

@@ -262,7 +276,8 @@ separated from preceding content."
262276
(set-marker pi-coding-agent--thinking-start-marker nil))
263277
(setq pi-coding-agent--thinking-marker nil
264278
pi-coding-agent--thinking-start-marker nil
265-
pi-coding-agent--thinking-raw nil))
279+
pi-coding-agent--thinking-raw nil
280+
pi-coding-agent--thinking-prev-rendered nil))
266281

267282
(defun pi-coding-agent--display-thinking-start ()
268283
"Insert opening marker for thinking block (blockquote)."

‎pi-coding-agent-ui.el‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,12 @@ Used to rewrite thinking content in place after whitespace normalization.")
662662
"Accumulated raw thinking deltas for the current thinking block.
663663
Normalized and re-rendered incrementally to avoid excess whitespace.")
664664

665+
(defvar-local pi-coding-agent--thinking-prev-rendered nil
666+
"Previously rendered blockquote text for the current thinking block.
667+
Used for incremental rendering: when the new rendered text extends the
668+
previous text, only the suffix is inserted instead of replacing the
669+
entire region. Reset by `pi-coding-agent--reset-thinking-state'.")
670+
665671
(defvar-local pi-coding-agent--line-parse-state 'line-start
666672
"Parsing state for current line during streaming.
667673
Values:

‎test/pi-coding-agent-core-test.el‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,5 +622,22 @@ Mocks `make-process' to capture :command, binding
622622
'("npx" "pi") '("-e" "/path/to/ext.ts"))
623623
'("npx" "pi" "--mode" "rpc" "-e" "/path/to/ext.ts"))))
624624

625+
;;;; Process Filter Tests
626+
627+
(ert-deftest pi-coding-agent-test-process-filter-inhibits-redisplay ()
628+
"Process filter binds `inhibit-redisplay' to t during dispatch.
629+
This batches N JSON lines delivered in one read() into a single
630+
redisplay cycle instead of triggering N separate redraws."
631+
(let ((captured-inhibit nil)
632+
(fake-proc (start-process "cat" nil "cat")))
633+
(unwind-protect
634+
(progn
635+
(process-put fake-proc 'pi-coding-agent-display-handler
636+
(lambda (_e) (setq captured-inhibit inhibit-redisplay)))
637+
(pi-coding-agent--process-filter
638+
fake-proc "{\"type\":\"agent_start\"}\n")
639+
(should (eq captured-inhibit t)))
640+
(delete-process fake-proc))))
641+
625642
(provide 'pi-coding-agent-core-test)
626643
;;; pi-coding-agent-core-test.el ends here

‎test/pi-coding-agent-render-test.el‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,29 @@ Models may send \\n\\n before thinking content too."
192192
(should (equal before (buffer-string)))
193193
(should (= before-tick (buffer-chars-modified-tick))))))
194194

195+
(ert-deftest pi-coding-agent-test-thinking-incremental-appends-suffix-only ()
196+
"Consecutive thinking deltas use the fast path: insert suffix, not full rewrite.
197+
After the first delta stabilizes, subsequent deltas that extend the
198+
rendered text should only insert the new suffix. We verify by placing
199+
a text property in the existing content — the fast path preserves it
200+
because it inserts at the end, while a full rewrite would lose it."
201+
(with-temp-buffer
202+
(pi-coding-agent-chat-mode)
203+
(pi-coding-agent--display-agent-start)
204+
(pi-coding-agent--display-thinking-start)
205+
(pi-coding-agent--display-thinking-delta "First thought.")
206+
(should pi-coding-agent--thinking-prev-rendered)
207+
;; Place a marker property inside the rendered thinking region
208+
(let ((think-start (marker-position pi-coding-agent--thinking-start-marker))
209+
(inhibit-read-only t))
210+
(put-text-property think-start (1+ think-start) 'test-marker t)
211+
;; Second delta extends the text — fast path should preserve the property
212+
(pi-coding-agent--display-thinking-delta " Second thought.")
213+
;; Property should survive (fast path doesn't delete existing region)
214+
(should (get-text-property think-start 'test-marker))
215+
;; And the new content appears
216+
(should (string-match-p "Second thought" (buffer-string))))))
217+
195218
(ert-deftest pi-coding-agent-test-thinking-paragraph-spacing-no-runaway-blank-lines ()
196219
"Thinking paragraphs keep a single readable separator, not multiple blanks."
197220
(with-temp-buffer
@@ -3944,6 +3967,18 @@ This validates that our hook-based tests are meaningful."
39443967

39453968

39463969

3970+
;;;; Tool Header Short-Circuit
3971+
3972+
(ert-deftest pi-coding-agent-test-tool-update-header-skips-when-unchanged ()
3973+
"display-tool-update-header does not modify buffer when header is unchanged.
3974+
Avoids unnecessary delete+insert cycles on repeated toolcall_delta
3975+
events where the header text hasn't changed."
3976+
(pi-coding-agent-test--with-toolcall "read" '(:path "/tmp/foo.py")
3977+
(let ((tick-before (buffer-modified-tick)))
3978+
;; Send delta with same args — header should be identical
3979+
(pi-coding-agent--display-tool-update-header "read" '(:path "/tmp/foo.py"))
3980+
(should (= (buffer-modified-tick) tick-before)))))
3981+
39473982
;;;; Built-in Slash Command Dispatch
39483983

39493984
(ert-deftest pi-coding-agent-test-dispatch-builtin-compact ()

0 commit comments

Comments
 (0)