An experimental Markdown renderer#597
Conversation
xenodium
force-pushed
the
inline-markdown
branch
2 times, most recently
from
52c62cc to
f2f1298
Compare
|
Seems work nicely! If your 
I'm using the |
|
Oh yeah it also seems like I'm not getting the fancy table behavior described, it's just showing the markdown table. |
Ah, it seems to work with claude, but not codex, is the thing |
@mplanchard that's because the LLM embedded it in a code block. Ask it to give you the table without a code fences. |
eyyy yeah that does it, although the heading alignment is a bit funky 
|
|
Ah thanks. I'll need to see the traffic that generated the Markdown to look into it. https://github.com/xenodium/agent-shell?tab=readme-ov-file#how-do-i-viewget-agent-client-protocol-traffic |
Good to know. I'll see if I can find a more subtle default. Having said that, the new renderer offers faces that can be overriden. |
Roger, here you go traffic output13:14:13.690 → request session/list 13:14:13.692 → request session/prompt 13:14:13.754 ← response result 13:14:15.454 ← notification session/update 13:14:15.455 ← notification session/update 13:14:15.482 ← notification session/update 13:14:15.483 ← notification session/update 13:14:15.514 ← notification session/update 13:14:15.515 ← notification session/update 13:14:15.548 ← notification session/update 13:14:15.572 ← notification session/update 13:14:15.572 ← notification session/update 13:14:15.593 ← notification session/update 13:14:15.594 ← notification session/update 13:14:15.620 ← notification session/update 13:14:15.639 ← notification session/update 13:14:15.658 ← notification session/update 13:14:15.677 ← notification session/update 13:14:15.724 ← notification session/update 13:14:15.748 ← notification session/update 13:14:15.749 ← notification session/update 13:14:15.770 ← notification session/update 13:14:15.770 ← notification session/update 13:14:15.796 ← notification session/update 13:14:15.797 ← notification session/update 13:14:15.838 ← notification session/update 13:14:15.856 ← notification session/update 13:14:15.875 ← notification session/update 13:14:15.876 ← notification session/update 13:14:15.897 ← notification session/update 13:14:15.918 ← notification session/update 13:14:15.938 ← notification session/update 13:14:15.958 ← notification session/update 13:14:15.977 ← notification session/update 13:14:15.996 ← notification session/update 13:14:16.112 ← notification session/update 13:14:16.113 ← notification session/update 13:14:16.115 ← notification session/update 13:14:16.118 ← notification session/update 13:14:16.120 ← notification session/update 13:14:16.123 ← notification session/update 13:14:16.149 ← notification session/update 13:14:16.149 ← notification session/update 13:14:16.182 ← notification session/update 13:14:16.221 ← notification session/update 13:14:16.222 ← notification session/update 13:14:16.224 ← notification session/update 13:14:16.227 ← notification session/update 13:14:16.254 ← notification session/update 13:14:16.328 ← notification session/update 13:14:16.329 ← notification session/update 13:14:16.332 ← notification session/update 13:14:16.334 ← notification session/update 13:14:16.361 ← notification session/update 13:14:16.362 ← notification session/update 13:14:16.365 ← notification session/update 13:14:16.392 ← notification session/update 13:14:16.419 ← notification session/update 13:14:16.420 ← notification session/update 13:14:16.449 ← notification session/update 13:14:16.473 ← notification session/update 13:14:16.474 ← notification session/update 13:14:16.514 ← notification session/update 13:14:16.515 ← notification session/update 13:14:16.551 ← notification session/update 13:14:16.552 ← notification session/update 13:14:16.585 ← notification session/update 13:14:16.587 ← notification session/update 13:14:16.619 ← notification session/update 13:14:16.620 ← notification session/update 13:14:16.657 ← notification session/update 13:14:16.659 ← notification session/update 13:14:16.682 ← notification session/update 13:14:16.683 ← notification session/update 13:14:16.713 ← notification session/update 13:14:16.713 ← notification session/update 13:14:16.748 ← notification session/update 13:14:16.773 ← notification session/update 13:14:16.793 ← notification session/update 13:14:16.935 ← notification session/update 13:14:16.936 ← notification session/update 13:14:16.948 ← notification session/update 13:14:16.952 ← notification session/update 13:14:16.956 ← notification session/update 13:14:16.960 ← notification session/update 13:14:16.963 ← notification session/update 13:14:16.967 ← notification session/update 13:14:16.996 ← notification session/update 13:14:16.997 ← notification session/update 13:14:17.000 ← notification session/update 13:14:17.037 ← notification session/update 13:14:17.037 ← notification session/update 13:14:17.041 ← notification session/update 13:14:17.045 ← notification session/update 13:14:17.068 ← notification session/update 13:14:17.090 ← notification session/update 13:14:17.112 ← notification session/update 13:14:17.114 ← notification session/update 13:14:17.144 ← notification session/update 13:14:17.173 ← notification session/update 13:14:17.175 ← notification session/update 13:14:17.217 ← notification session/update 13:14:17.258 ← notification session/update 13:14:17.286 ← notification session/update 13:14:17.287 ← notification session/update 13:14:17.397 ← notification session/update 13:14:17.403 ← notification session/update 13:14:17.430 ← notification session/update 13:14:17.431 ← notification session/update 13:14:17.435 ← notification session/update 13:14:17.439 ← notification session/update 13:14:17.443 ← notification session/update 13:14:17.447 ← notification session/update 13:14:17.451 ← notification session/update 13:14:17.455 ← notification session/update 13:14:17.491 ← notification session/update 13:14:17.492 ← notification session/update 13:14:17.495 ← notification session/update 13:14:17.526 ← notification session/update 13:14:17.528 ← notification session/update 13:14:17.532 ← notification session/update 13:14:17.557 ← notification session/update 13:14:17.559 ← notification session/update 13:14:17.583 ← notification session/update 13:14:17.715 ← notification session/update 13:14:17.717 ← notification session/update 13:14:17.722 ← notification session/update 13:14:17.725 ← notification session/update 13:14:17.729 ← notification session/update 13:14:17.767 ← notification session/update 13:14:17.768 ← notification session/update 13:14:17.772 ← notification session/update 13:14:17.777 ← notification session/update 13:14:17.781 ← notification session/update 13:14:17.813 ← notification session/update 13:14:17.815 ← notification session/update 13:14:17.840 ← notification session/update 13:14:17.864 ← notification session/update 13:14:17.866 ← notification session/update 13:14:17.870 ← notification session/update 13:14:17.915 ← notification session/update 13:14:17.916 ← notification session/update 13:14:17.947 ← notification session/update 13:14:17.948 ← notification session/update 13:14:18.087 ← notification session/update 13:14:18.089 ← notification session/update 13:14:18.094 ← notification session/update 13:14:18.100 ← notification session/update 13:14:18.105 ← notification session/update 13:14:18.109 ← notification session/update 13:14:18.114 ← notification session/update 13:14:18.119 ← notification session/update 13:14:18.152 ← notification session/update 13:14:18.153 ← notification session/update 13:14:18.156 ← notification session/update 13:14:18.195 ← notification session/update 13:14:18.196 ← notification session/update 13:14:18.224 ← notification session/update 13:14:18.226 ← notification session/update 13:14:18.231 ← notification session/update 13:14:18.384 ← notification session/update 13:14:18.387 ← notification session/update 13:14:18.392 ← notification session/update 13:14:18.398 ← notification session/update 13:14:18.403 ← notification session/update 13:14:18.469 ← response result 13:14:18.475 → request session/list 13:14:18.540 ← response result full shell outputCodex> same output but not in a code block ▼ Notices |
|
thanks! almost there... The buffer that has this: Press C-x C-s (acp-traffic-save-to) to get the actual content of each one of those items |
|
Oh lol, yeah, I thought that output didn't seem very useful. It's okay, one of these days I'll learn to read. weird that GH doesn't let you upload |
|
Thanks that helps. Made some changes. Mind trying it out and see if you still have issues with headers on Codex? |
Yeah that seems to have done the trick! 
|
|
Awesome. Thanks for reporting back! |
|
i'm testing this branch, but all tool use are expanded |
@liaowang11 oh. interesting. i've not reproduced this yet myself. coincidentally, it looks like the bash section/node icon is collapsed. what happens when you expand it? |
I let codex to find the root cause using your emacsclient skill(super useful!) and here are the findings: agent-shell--experimental-renderer breaks folding state for collapsed tool-call bodies. Observed behavior: Root cause: Recommended fix: |
|
I would like to add a feature request related to the leaking of Markdown code when copy/pasting. It would be nice to be able to influence the copy of the markdown. Both have it's use:
After thinking about it, stripping markdown is not a solution to the problem. You need to be able to control and switch between the two modes. Two Elisp functions that can be used by users. It is probably the best solution. And one variable that holds default behavior. This is just an idea. |
@jcubic this will be theoretically possible with the current approach. The text in the buffer, while it has the markdown stripped, it has metadata which can be used to regenerate markup. This has the advantage of being markup agnostic, so we can potentially have commands to "copy as Markdown" or "copy as Org", but copying always defaults to stripped text, which is the primary use that's often tripped (copy and paste in shell). |
@liaowang11 Nice to hear! It's very handy that it can inspect buffers for you, get text properties, etc.
I can't reproduce neither manually nor with a test: https://gist.github.com/xenodium/c3f9ce717eef920ccf289971ec968e0c Could you please manually reproduce in your with some data you don't mind sharing? Post
|
This seems to only happen with pi-agent, i don't encounter the problem with codex-acp. The screenshot after model output: After toggle: After toggle again: The traffic log: agent-shell-tool-use-expand-by-default => nil |
Reported in PR #597: pi-acp tool-call fragments rendered with the indicator showing `▶' (collapsed) but the body fully visible.
Skip re-rendering already-processed prefix on each call Streaming use of `agent-shell-markdown-replace-markup' calls the renderer once per chunk, so every pass was re-walking the entire buffer from `point-min' on each call — O(N^2) over N chunks. Track a per-buffer "watermark": the position before which content is fully rendered and stable. Stored as an `agent-shell-markdown-watermark' text property on the first character (so a propertized string returned from `agent-shell-markdown-convert' carries it without a buffer-local variable). Re-stamped at the end of each render to: - start of the last line in the buffer; clamped back to - start of any open fence (so a future closing ``` still matches), - start of any rendered table whose extension is still possible (so streamed continuation rows still fold in). The next call narrows to (watermark, point-max) and every pass runs inside the narrow. `:force' on `agent-shell-markdown-replace-markup' drops the watermark and re-renders the whole buffer.
Reported in PR #597: pi-acp tool-call fragments rendered with the indicator showing `▶' (collapsed) but the body fully visible.
|
Hey - have been trying this out and although I've seen some beautiful things render, I've also seen, well, nothing render. Even when folded. It's been a journey to dig into it - here's a compressed explanation with some screenshots. Note that I'm using a new ACP I built myself and I understand there's different results for different ACPs but I think the issue is something that can be fixed in this branch. Could be from These two screenshots are taken at the same point - notice that the tool call has not been unfolded. In between the screenshots I ran this: (let ((inhibit-read-only t))
(remove-text-properties (point-min) (point-max) '(invisible nil)))Here's the before / after evaluating that elisp: 
A more sensible explanation than I'm able to give un-
|
|
Thanks a lot for this. @codeluggage are you able to reproduce on the latest from branch (please also update shell-maker). If you could also share the traffic file and a screenshot of the offending case, I'd be able to replay the traffic to render locally and investigate. |
|
Buffer is still open - but alas, has a private repo in the mix so let me reproduce with another. I believe I'm on the latest because I just got things brought up to date before posting this (results were the same as I was reproducing) but perhaps I am not on the latest shell-maker. Will check and give you a small repro with the nice back and forth arrows and the other log. |
|
@codeluggage In addition to eval what's listed in #597 (comment) do shell-maker.el too please |
The inline-markdown branch (xenodium#597) uses the aligned-under-section style for `add-text-properties' plists, e.g. `(agent-shell-ui-section label-left help-echo ,qualified-id read-only t front-sticky (read-only)) instead of the flat indent. Reformatting our touched call sites to match avoids gratuitous diff churn when the experimental renderer lands upstream, and brings em-dashes back into the folding-command docstrings to match the surrounding prose style. No behaviour change.
|
@xenodium ha that was a confusing ride - turns out it was stale things locally for me in-between the recent force pushes. Confirmed fixed by cc9fea4 on the latest inline-markdown tip. I was on the old pre-force-push base (19a3c22). Now that I've rebased my fork onto 38b62b5 I thankfully can't reproduce. Unique situation for me as I put folding things (#608) on top of this branch. |
|
Ah good to hear. Thanks for confirming.
I'm hoping I can merge this branch soon, so should be ok for you. Having said that, I did force push to my branch so maybe that wasn't so fun for ya :/ |
5 participants
Experimenting with inline text properties to render Markdown text into agent-shell buffers.
agent-shell's Markdown renderer today is powered by overlays. While overlays have served us well for some time, they have some limitations, primarily around performance (so far has been good enough-ish) but also with text navigation/selection.
This branch is an experiment to see if we can achieve an improved experience around Markdown rendering using inlined text properties (no overlays).
Two initial areas of focus:
Table navigation and content selection (not currently possible in today's overlay implementaion)
Cell navigation
M-x agent-shell-markdown-table-next-cellM-x agent-shell-markdown-table-previous-cellMore performant code block rendering
Enabling experimental renderer
To try out the experimental renderer in this branch use:
Report bugs
This is fairly experimental, so please do report bugs.