Skip to content

chat: fix bare backtick flicker in streamed agent host markdown#318498

Merged
roblourens merged 1 commit into
mainfrom
agents/vsckb-implement-the-text-created-isolated-worktr-42c3ab69
May 27, 2026
Merged

chat: fix bare backtick flicker in streamed agent host markdown#318498
roblourens merged 1 commit into
mainfrom
agents/vsckb-implement-the-text-created-isolated-worktr-42c3ab69

Conversation

@roblourens
Copy link
Copy Markdown
Member

Fixes a bare-backtick flicker while streaming agent host responses, plus removes a now-stale supportHtml: true from agent host markdown emission.

Symptom

For agent host (AH) sessions, the initial worktree announcement

Created isolated worktree for branch xyz

briefly rendered as

Created isolated worktree for branch `xyz

(bare opening backtick, no <code>) until the closing backtick word arrived. The progressive word renderer (chatWordCounter) deliberately splits at non-backtick boundaries, so the intermediate string ending in a bare ` is expected — fillInIncompleteTokens is supposed to handle that.

Why fillInIncompleteTokens wasn't firing

ChatContentMarkdownRenderer wraps any supportHtml markdown in <body>...</body> (so dompurify keeps leading comments). The lexer then produces:

[ html('<body>'), paragraph('…for branch `xyz'), space, html('</body>') ]

fillInIncompleteTokensOnce only inspects tokens.at(-1) for its paragraph / list / heading fix-ups. The trailing html('</body>') token meant the codespan fix-up never ran.

Why AH was opting into supportHtml in the first place

The // supportHtml is load bearing… comment in agentHostSessionHandler._setupMarkdownPart was stale. It dated back to when AgentHostEditingSession emitted file edits as a markdownContent('\n````\n') + codeblockUri + textEdit + markdownContent('\n````\n') cluster — the supportHtml: true tag on model deltas made canMergeMarkdownStrings reject merging with those bare fence chunks. After #318053 (Connor) replaced that whole thing with a dedicated 'externalEdit' progress part, model deltas have nothing to accidentally merge into.

Fixes

Both layers are addressed:

Defensive (base layer). fillInIncompleteTokensOnce now walks past trailing space / html tokens to find the last paragraph/list, then preserves those trailing tokens around the patched node. The heading branch is intentionally left alone. This means any future consumer of supportHtml: true streaming markdown also gets correct fix-ups.

Root (AH layer). Drop supportHtml: true from:

  • agentHostSessionHandler._setupMarkdownPart (live streaming sink) — and remove the stale "load bearing" comment.
  • stateToProgressAdapter.turnsToHistory (ResponsePartKind.Markdown case).

Also drops the now-unused options parameter from rawMarkdownToString. This aligns the streaming and history paths with the already-different activeTurnToProgress path (which never set the flag), and with vanilla Copilot chat stream.markdown(...) callsites.

Tests

  • Token-level (markdownRenderer.test.ts): asserts fillInIncompleteTokens on the <body>…\xyz\n\n` token list matches the lex of the balanced wrapped string.
  • End-to-end (chatMarkdownRenderer.test.ts): renders a MarkdownString with supportHtml: true containing Created isolated worktree for branch \xyzthrough the fullChatContentMarkdownRenderer.render(md, { fillInIncompleteTokens: true })pipeline and asserts axyz` is in the output with no bare backtick in the text content.

All 213 tests pass across the affected suites (markdownRenderer + chatMarkdownRenderer + stateToProgressAdapter); tsgo --noEmit is clean.

(Written by Copilot)

Streaming the agent host's worktree announcement
("Created isolated worktree for branch `xyz`") briefly rendered a bare
opening backtick because `fillInIncompleteTokens` was not closing the
codespan during progressive rendering. The progressive word renderer in
`chatWordCounter` deliberately excludes backticks from word characters,
so the intermediate state at the end of a streamed chunk is exactly
"…for branch \`xyz" — which is precisely what
`fillInIncompleteTokensOnce` is supposed to patch up.

Two issues conspired:

1. `ChatContentMarkdownRenderer` wraps `supportHtml` markdown in
   `<body>...</body>` so dompurify does not strip leading comments.
   That makes the lexer emit
   `[html('<body>'), paragraph(…`xyz), space, html('</body>')]`, so
   `tokens.at(-1)` is `html`, not `paragraph`, and the
   codespan / list fix-ups never run.

2. The agent host paths were unconditionally setting
   `supportHtml: true` on streamed markdown deltas. The
   "supportHtml is load bearing" comment is now stale: PR #318053
   replaced the old `AgentHostEditingSession` (which emitted bare
   ` ```` ` fences as `markdownContent`) with a dedicated
   `'externalEdit'` progress part, so model deltas have nothing to
   accidentally merge into.

Fixes:

- `fillInIncompleteTokensOnce` now walks past trailing `space` and
  `html` tokens to find the last paragraph/list, then preserves those
  trailing tokens around the patched-up node. Heading branch is
  intentionally left alone.
- Drop `supportHtml: true` (and the stale comment) from both the live
  streaming sink in `agentHostSessionHandler` and the history rebuild
  in `stateToProgressAdapter`. Drop the now-unused `options` parameter
  from `rawMarkdownToString`.

Tests:

- New token-level regression in `markdownRenderer.test.ts` exercises
  the `<body>…</body>`-wrapped + bare-backtick scenario.
- New end-to-end test in `chatMarkdownRenderer.test.ts` runs through
  the full render pipeline (with `supportHtml: true` and
  `fillInIncompleteTokens: true`) and asserts a `<code>` element is
  produced with no leftover bare backtick.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 27, 2026 03:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a rendering flicker in streamed agent-host markdown where partial content ending in a bare backtick briefly shows in the DOM before the closing backtick arrives. It does this by making fillInIncompleteTokens resilient to supportHtml-wrapped markdown (which introduces trailing html tokens like </body>), and by removing stale supportHtml: true usage in the agent host markdown emission paths.

Changes:

  • Make fillInIncompleteTokensOnce skip trailing space/html tokens when deciding which paragraph/list token to patch, while preserving those trailing tokens in the output token list.
  • Remove supportHtml: true from agent-host markdown emission (streaming sink + history adapter) and simplify rawMarkdownToString accordingly.
  • Add regression tests covering codespan completion with supportHtml wrapping at both token-level and end-to-end chat rendering levels.
Show a summary per file
File Description
src/vs/base/browser/markdownRenderer.ts Adjusts incomplete-token fixup logic to operate correctly even when trailing html/space tokens exist (e.g. </body> wrapper).
src/vs/base/test/browser/markdownRenderer.test.ts Adds a token-level regression test for codespan completion when markdown is wrapped in <body>...</body>.
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts Removes supportHtml: true from streamed agent-host markdown delta emission.
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts Removes supportHtml: true from markdown history conversion and drops the unused options parameter from rawMarkdownToString.
src/vs/workbench/contrib/chat/test/browser/widget/chatMarkdownRenderer.test.ts Adds an end-to-end regression test ensuring bare codespans are closed when supportHtml wrapping introduces trailing html tokens.

Copilot's findings

Comments suppressed due to low confidence (1)

src/vs/base/browser/markdownRenderer.ts:1033

  • fillInIncompleteTokensOnce now skips trailing space/html tokens for the list/paragraph fixups, but the heading fixup still uses tokens.at(-1). For supportHtml-wrapped markdown (where </body> is the literal last token), the existing heading completion (used to avoid hello\n- being parsed as a setext heading) will never run. Consider locating the last heading token using the same lastInterestingIdx logic (and preserving trailing tokens) so heading-related fixups also work when trailing html/space tokens are present.
	const lastToken = tokens.at(-1);
	if (lastToken?.type === 'heading') {
		const completeTokens = completeHeading(lastToken as marked.Tokens.Heading, mergeRawTokenText(tokens));
		if (completeTokens) {
			return completeTokens;
		}
  • Files reviewed: 5/5 changed files
  • Comments generated: 0

@roblourens roblourens marked this pull request as ready for review May 27, 2026 03:26
@roblourens roblourens enabled auto-merge (squash) May 27, 2026 03:26
@roblourens roblourens merged commit 2e9dc7f into main May 27, 2026
26 checks passed
@roblourens roblourens deleted the agents/vsckb-implement-the-text-created-isolated-worktr-42c3ab69 branch May 27, 2026 03:47
@vs-code-engineering vs-code-engineering Bot added this to the 1.123.0 milestone May 27, 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.

3 participants