Repository navigation
UI Consistency: 1 rendering regression found
apps/web/src/components/ChatMarkdown.tsx — Adding remarkDirective to CHAT_MARKDOWN_REMARK_PLUGINS and CHAT_MARKDOWN_REMARK_PLUGINS_WITH_BREAKS changes the markdown grammar for all chat message text, regressing two paths:
remarkNormalizeListItemIndentation'sINLINE_PARSE_PREFIXsentinel (t3-markdown-inline-prefix:) is broken: with directives enabled,prefix:<letter…>parses as atextDirective, the prefix check fails, and recovered over-indented list content falls back to a raw text node — losing inline code, links, and emphasis in exactly the case that plugin exists for. Its own test file renders withoutremarkDirective, so it does not catch this.restoreDirectiveSourcerebuildsleafDirective/containerDirectivenodes as a paragraph containing the raw source slice, so markdown inside a:::block renders literally (asserted by the new testrestores flow directives as paragraphs), and an unclosed:::nameswallows the remainder of the message into one literal paragraph.
Suggested smallest fix: keep the chat grammar unchanged and pre-transform the text with the shared replaceCodexFileCitationsWithMarkdownLinks helper already used by mobile and the copy path, letting the existing markdown-link → file-chip path render the result.
Details
Note
Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.
Scope: apps/web/src/**/*.{ts,tsx,css}. Files examined in scope: apps/web/src/components/ChatMarkdown.tsx, apps/web/src/components/ChatMarkdown.test.tsx, apps/web/src/components/chat/MessagesTimeline.logic.ts. Out-of-scope files read for context: apps/web/src/markdown-list-indentation.ts, apps/web/src/markdown-links.ts, packages/client-runtime/src/codexFileCitations.ts, packages/client-runtime/src/codexFileCitationMarkdown.ts, mobile SelectableMarkdownText.
Checked and found acceptable:
- Percent-encoding produced by
resolveCodexFileCitationLink(%25,%23,%3F) round-trips correctly throughresolveMarkdownFileLinkTarget, which splits?/#before a singlesafeDecode, and#L<n>is converted to a:linesuffix. - Citation links reuse the existing
MarkdownFileLinkchip, so environment-scoped actions (open in editor, reveal, open in panel) still resolve from the samethreadRef/environmentIdplumbing as other file links; no new active-environment fallback is introduced. - Prior findings from earlier runs (unknown-directive nodes rendering as empty
<div>, directives inside link text, missing<p>wrapper for flow directives) are addressed by theinsideLinktraversal flag and theparagraphrestoration path. - No new raw controls, class-string overrides of primitive contracts, or global CSS changes in this PR.
🔍 Agent activity — 10 tool calls
💭 I'm checking the current state of ChatMarkdown.tsx and MessagesTimeline.logic.ts within the apps/web scope to see if the previously flagged issues around insideLink handling and flow directives have been resolved, then I'll double-check the environment routing rules too. Now I'm tracing how ChatMarkdown converts a citation into a link, and whether that link renders as a file chip that triggers openFile against the active environment.
1. git_tools — codex|remarkDirective|resolveCodexFileCitationLink|insideLink
2. git_tools — chat-markdown-file-link
💭 I'm tracing how the file chip anchor logic resolves the active environment when a Codex citation becomes a link, since that path already existed before this change—the new piece is just reusing the existing anchor rendering rather than introducing a fresh unscoped call site.
3. browse_code — apps/web/src/components/ChatMarkdown.tsx
💭 I'm examining how the file citation link anchor gets rendered—checking whether the href produced by resolveCodexFileCitationLink correctly percent-encodes characters like
#and?, and how the anchor component parses that href back into a target file path and line number. Since node.url is set directly in the markdown AST rather than parsed from raw markdown text, I need to check if the file-link resolution logic actually decodes those encodings—if it doesn't call decodeURIComponent, a path containing#would end up with a literal%23in it, breaking the file open or pointing to the wrong path. I should look at the file-link resolution code to confirm.
4. git_tools — decodeURI|resolveMarkdownFileLink|targetPath|parseFileLink
💭 I'm noting that the precomputed href memo won't include Codex citations since they aren't raw markdown links, but that's fine since the anchor renderer already falls back to resolving the metadata directly at render time—just a minor perf cost from the memo miss, not a correctness issue. Now I'm worried about percent-encoding: web citations get their href injected directly into the AST rather than parsed from markdown text, so characters like
%23or%3Fwon't get decoded the way normal markdown parsing would handle them. IfresolveMarkdownFileLinkMetadoesn't perform its own decoding, paths containing#,?, or%could resolve to the wrong file when a citation chip is clicked, so I need to check what that function actually does with the raw href.
5. browse_code — Show the implementation of resolveMarkdownFileLinkMeta and normalizeMarkdownLinkDestination / normalizeMarkdownLinkHrefKey (in client-runtime markdown links module). Does it percent-decode the href (decodeURI/decodeURIComponent) before resolving the file path, and how does it parse trailing #L42 line fragments?
💭 Good, this confirms the decode-once approach works correctly — the
%23in a path segment gets restored to#safely, since fragments are split off before decoding, andstripSearchAndHashsplits off the query string before decoding too, which is exactly why the%3Fencoding exists to preserve literal?characters in paths. I check how ChatMarkdown handles the citation URL through its urlTransform and file-URI rewriting — an absolute path should pass through fine, and the chip rendering test should confirm that. The environment routing for MarkdownFileLink stays unchanged, deriving from existing props. Now I want to pull up ChatMarkdown.tsx around the restore logic to verify what's changed and check for remaining issues.
6. browse_code — apps/web/src/components/ChatMarkdown.tsx
💭 I'm checking that text directives restore correctly using the source slice so literal syntax is preserved, and thinking about whether restoring flow directives as a paragraph with a single text child works properly when nested inside list items or blockquotes. But there's a real regression here: a container directive's content now becomes raw literal text rather than being parsed, so any bold, inline code, or links inside a
:::noteblock lose their formatting and render as plain literal syntax. Worse, since container directives can wrap multiple block-level elements, something like a bulleted list inside:::detailswould previously render as an actual list but now collapses into literal text too. Even more concerning, an unclosed:::block extends all the way to the end of the document — meaning if a message opens with:::noteand never closes it, everything that follows, including headings, code blocks, and citations, gets swallowed into raw literal text, which would be especially disruptive during streaming. Confirmed: an unclosed container directive swallows everything to the end of the message as one literal paragraph, which is a real regression compared to before this PR — previously that content would render with normal formatting. There's also a concern about blank lines collapsing within the restored literal paragraph, losing paragraph breaks, and I should check whether directives inside table cells cause similar issues. I'm also confirming remarkGithubAlerts stays unaffected, and that unclosed container directives extend to the end of the parent container like fenced code without a closing fence — which means directives nested inside a swallowed block, like a citation marker, never get visited or converted since visit stops at the outer container. I'm weighing how often triple-colon blocks would actually show up in chat output versus pasted file content, and considering whether a safer fix would be to scope directive parsing narrowly to just the citation syntax rather than enabling the full directive extension globally. The isolated transformer would run remark parsing twice, adding a perf cost on streaming re-renders, though mobile already accepts that tradeoff. I'm wary of pushing for a bigger redesign here and want to keep the fix minimal, but I also need to weigh whether it's worth flagging at all given prior reviewers already raised issues in this same function and the author has already iterated on it. I decide the container-directive flattening is a genuine regression worth reporting: before the change,:::noteblocks rendered as normal paragraphs with properly parsed inline markdown and separate blocks for blank-line-separated content, but afterward the entire directive body collapses into one literal paragraph until the closing:::or EOF. I verify leaf directives restore correctly, but I'm now checking whether other directive types like text directives interact badly withremarkBreaks, since text directives can't span newlines and the citation restoration runs before line breaks are processed. I confirm restored paragraph text with embedded newlines gets converted to<br/>nodes properly, and citations inside links, headings, or tables all restore fine sinceinsideLinkprevents unwanted conversion. My remaining concern is whether earlier plugins likeremarkNormalizeListItemIndentationmutate the source text itself rather than just the AST, which would throw off the position offsets used for slicing citations from the original source.
7. git_tools — function remarkNormalizeListItemIndentation
8. git_tools — remarkNormalizeListItemIndentation
💭 I want to check markdown-list-indentation.ts to see if it rebuilds subtrees without positions, since that could cause the citation restore logic to fall back to a bare
:nameand lose the attribute text when a directive lands inside a newly constructed node without position data.
9. browse_code — apps/web/src/markdown-list-indentation.ts
💭 I realize plugin registration order doesn't actually control parse timing here—remarkDirective's micromark extension gets applied during initial parsing regardless of where it sits in the plugin list, so directives are already structured in the tree before other transformers run. But I'm noticing this normalizer step re-parses content, which could interact with that in unexpected ways. The recovered content gets re-parsed against a prefixed source string, so its node positions are relative to that different string entirely. If an over-indented list item's recovered content happens to look like a directive token, remarkDirective will parse it into a textDirective node with offsets pointing into the prefixed string—but remarkCodexFileCitations slices the original document using those foreign offsets, producing wrong substituted text. The mismatch is essentially the prefix length offset against wherever that content actually sits in the real source. I'm weighing whether this niche bug is worth flagging versus a bigger issue: container/leaf directive restoration flattening inner markdown structure, and unclosed
:::fences swallowing the rest of a message as literal text. That flattening is a genuine regression—restoring a container as a plain paragraph loses blank-line paragraph separation, which wasn't a problem before. I'm now thinking about how plausible:::syntax is in real agent chat, like when discussing Docusaurus or VitePress admonition blocks in markdown files. I'm leaning toward flagging both the container flattening issue and the offset mismatch during re-parsing as separate comments, but want to nail down the offset bug more precisely first. Looking at how blocks are reconstructed from indented code, only the first block inherits the correct original position while its children and subsequent blocks retain positions computed relative to a prefixed source string. Since that prefix shifts all offsets by a fixed amount, slicing the original source at those offsets pulls the wrong span of text—same length, wrong content, or truncated to empty if it runs past the end of the string. Now I want to confirm this is actually reachable: it requires a list item using a dash marker followed by four or more spaces before content on the same line, where that content isn't a fenced code block and contains something that could be mistaken for a directive token. That leads me to double-check whether directive names can start with a digit, since I recall asserting that earlier and want to verify against howfactoryNamevalidates the first character in the directive extension's parsing logic. I realize this list-indentation path is actually exercised in real chat output, since that's why the plugin exists. Tracing through the offset math, if the prefixed offset exceeds the original document length, the slice returns an empty string — meaning the restored text vanishes entirely from the rendered message, which is a genuinely visible disappearing-text bug. I need to check whether parseRecoveredMarkdown only runs in specific conditions. Both issues trace back to the same root cause: enabling remarkDirective globally across the chat pipeline and then hand-rolling a source-slice-based un-parse step, so a safer fix might be reconstructing the directive text from the node itself, or simply not enabling remarkDirective globally at all. I'm weighing whether this second finding is too speculative to flag versus being a concrete regression worth calling out. I'll consolidate into a single comment at the plugin registration lines (291-292), since prior comments already covered the restore/traversal specifics at lines 418 and 446, so this new one focused on the grammar change from enabling remarkDirective won't be a duplicate. Let me verify those diff lines are actually addressable before finalizing placement. I want to double-check the container directive flattening claim against mdast-util-directive's behavior — the containerDirective node holds parsed block content as children, spanning from the opening:::noteto the closing:::, and the restore logic collapses this into a paragraph with the raw source text, so all inner formatting becomes literal. The PR's own test confirms this by expecting the HTML output to contain the raw fenced block wrapped in a paragraph tag. Now I want to check the unclosed directive case where there's no closing:::. For the unclosed case, the container extends to the end of the document, so the restored paragraph becomes the entire remaining text as a literal string, which seems consistent enough. I also note that::noteas a leaf directive only consumes its own line, which is straightforward. Before this PR, without remarkDirective active, the same fenced block would render as a plain paragraph with soft line breaks, matching the current output for the simple case — that's why those tests pass, with differences only showing up when there's inner markdown or nested blocks. I'm now wondering whether container directives require a blank line before them to be recognized as block constructs, since they need to interrupt or follow a paragraph correctly. Since container directives probably can't interrupt a paragraph the way fenced code can, that detail likely doesn't matter much here. Turning to the offset mismatch issue with list-indentation recovery, I want to confirm the recovered parse uses the same processor instance, meaning it inherits the directive extension from remarkDirective since all attachers register their micromark extensions at freeze time before any transforms run — so plugin order between remarkNormalizeListItemIndentation and remarkDirective doesn't prevent the recovered content from being parsed with directives active, producing directive nodes at the wrong source positions and causing remarkCodexFileCitations to slice the original text incorrectly. This gives me good confidence in the root cause, and I'm also noting the prefixed source is 25 characters longer, tracing where the recovered value originates from. I'm now working through a concrete example where the injected prefix ends in a colon immediately before real text, checking whether micromark would actually treat that boundary as a textDirective candidate given its rule about rejecting directives only when preceded by an unescaped colon, and tracing through how that plays out in parseRecoveredMarkdown. So this looks like a genuine regression: the prefix trick for inline GFM formatting breaks down whenever directive parsing intercepts it, causing lost formatting in that list item context. Checking the specific example confirms it: recovered text like "for (const step..." parses as a text directive named "for", falls back to a plain text node with no position, and gets inserted as a listItem child. That text node then renders unformatted by rehype, so inline markdown like code spans, links, and bold gets lost in the recovered over-indented list items — a real visual regression, though isolated to this fallback path. Existing tests won't catch it since the list indentation test doesn't enable remarkDirective, so I need to confirm this gap concretely before flagging it as a regression from turning directives on globally. Following the parse chain further, the check for the inline-prefix marker fails since the text doesn't actually start with the expected prefix string, so it falls back to treating the whole thing as a raw text block rather than parsed markdown blocks. That means the code line renders as literal text without inline markdown applied, whereas before the change it would have gone through paragraph parsing with proper inline handling. This confirms both bugs — the container flattening and this inline-code regression — trace back to the same root cause: adding remarkDirective into the shared chat pipeline. A cleaner fix would be to avoid that grammar change altogether and instead pre-transform citation text using the existing shared helper already used by mobile and the copy path, then feed the resulting markdown links into the current file-chip rendering. I'll post one comment spanning lines 291-292 describing both regressions and the fix. A prior review round already flagged the same registration lines for a different reason (unknown fallback nodes), but my new comment covers separate consequences—flattening inner markdown and breaking list-indentation—so it's not a duplicate, just related evidence after the code changed. With three rounds already in, I should stay concise and lead with the most verifiable issue: container flattening, since their own test asserts it directly. For the inline-directive breakage, I'm reasoning through whether micromark actually blocks a colon-directive when preceded by a word character, checking thepreviousfunction indirective-text.js— it seems to only reject when directly preceded by an unescaped colon, meaning a preceding letter like "x" should still allow the directive to parse. That supports my claim that over-indented list items lose inline formatting, which defeats the plugin's purpose, though I want to verify this more concretely before committing to it in the comment. Confirming the existing test case for "Error:ENOENT" solidifies that colon-prefixed text does get parsed as a directive, matching the pattern I flagged. I'll combine both findings into a single comment with two bullet points rather than splitting them, since they point to the same lines, leading with the functional regression as the primary issue. Now I'm pinning down the exact anchor for the review comment: apps/web/src/components/ChatMarkdown.tsx, line 292 with start_line 291 on the RIGHT side, since this spans a multi-line addition with no diff context needed beyond that. Time to write out the comment.
10. modify_pr — One finding: enabling remarkDirective for the whole chat pipeline changes the markdown grammar for all message text, which regresses two unrelated rendering paths. Details inline.
Agent Credits: 162 credits
Token usage: 38 uncached input · 782,055 cache-read input · 99,124 cache-write input · 21,063 output
Agent Credits may also include non-token charges from external tools such as web research.