fix(copilot): stop the special-tag parser discarding text it cannot resolve#5952
Open
j15z wants to merge 8 commits into
Open
fix(copilot): stop the special-tag parser discarding text it cannot resolve#5952j15z wants to merge 8 commits into
j15z wants to merge 8 commits into
Conversation
The special-tag parser suppressed everything after an opening tag with no close, even after streaming ended — so an assistant message that merely MENTIONED `<workspace_resource>` in prose lost its entire remainder in the UI. The stream itself completed fine; only the render truncated. Suppression now applies only while streaming. A completed message can never finish an unclosed tag, so the marker was literal text and the remainder is rendered as-is. Originally written alongside the docs/ VFS work and reverted at review time to keep that PR to one concern; this restores it on its own.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
6 tasks
…olve
Restoring the text only at end of stream left a real gap: once the model
mentions a tag in prose, everything after it stays invisible for the rest of
the stream and reappears in one jump when streaming stops. On a long reply
that is most of the message.
Two properties let us decide much earlier, both conservative — they only fire
on content that could not have parsed:
- Tags never nest, so any special-tag marker inside the body (opening OR
closing, not just a mismatched close) proves the opener was literal text.
- JSON-bodied tags must start with `{` or `[`, so the first non-space
character settles it — prose after the marker is caught on the very next
chunk rather than at the end.
The second is per-tag, not global: `thinking` bodies are prose by design
(parseTextTagBody), so the JSON rule cannot apply there and only the nesting
rule can rescue it. A test documents that remaining gap rather than leaving
it implicit.
A false positive is cheap by construction: text shows early and the
end-of-stream parse still produces the correct final render.
j15z
changed the title
The nesting rule treated any tag marker in the body as proof the opener was literal text. But a JSON body can legitimately quote tag syntax — a `<question>` asking which tag to use, a `<workspace_resource>` whose title mentions one — and that is exactly the "model explains its own tags" situation this whole fix exists for. Bailing there is the one expensive false positive in the design: the raw JSON renders and STAYS for the rest of the stream, then snaps into a card when the real close arrives. More jarring than the bug being fixed. For JSON-bodied tags the nesting scan now runs over a copy with string literals blanked (escape-aware, and tolerant of the unterminated trailing string that is normal mid-stream). Markers in real body position still count. `thinking` is unaffected — its body is prose, so there are no strings to confuse it. Tests cover both halves: the streaming case does not bail, and the same body resolves to a question card once it closes.
…ead JSON
Two more real cases from traces, both of which the earlier heuristics missed.
1. A MATCHED pair whose body fails to parse was silently dropped, cursor and
all. When the model explains tag syntax and ends with a backticked example
containing a real closing tag, that example closes an EARLIER opener and
three paragraphs become the "body" — which is not valid JSON, so the whole
span vanished and the render resumed mid-sentence (trace b095e080).
Now emitted verbatim, but only when the body contains a tag-shaped marker,
which is what shows the pairing was wrong. A marker-free body that merely
fails validation is a genuinely malformed agent payload and keeps being
dropped — an existing test asserts that deliberately, and showing the user
raw JSON there would be a regression.
2. The JSON heuristic only checked the FIRST character, so
`{"type":"file"}</workspac and then prose...` looked viable forever: it
opens with `{`, and the truncated close is not a marker any rule can see
(trace afbeefd0). Track depth instead — once the top-level value closes,
any non-whitespace after it is fatal, so the stray character decides it
immediately. String contents are blanked first so braces inside strings do
not skew the count.
Third real case from a trace (1206fd8a): `<workspace_resource>the gmail-agent
workflow</workspace_resource>` — a matched pair whose body is plain prose. The
sentence rendered as "...once I wired up to handle the welcome sequence" with
its subject silently removed.
The marker test could not fire (prose contains no tag markers), so it fell to
the deliberate drop-malformed-payload path. But that path exists for an agent
emitting BROKEN JSON, not for a tag wrapping prose.
The distinction is whether the body was ever an attempted payload, which
isViableJsonPrefix already answers: `{"type":"single_select"}` is a well-formed
JSON value failing its shape guard and keeps being dropped; `the gmail-agent
workflow` was never a payload and is emitted.
parseSpecialTags had grown five inline branches, each added for a specific malformation found in a trace. The shape made "drop it" the implicit fallback, which is how spans that were never malformed payloads ended up silently swallowed. Extracts resolveTagAt, returning one of four named outcomes — segment, literal, discard, pending — so each decision is explicit and the main loop just dispatches on it. Fixes a latent bug the old shape hid: rejecting an unclosed tag ran `break`, abandoning the rest of the message, so a genuinely valid tag after a literal mention was never parsed. Resolution now resumes just past the rejected opener and scanning continues. Test added. Two behavior notes: - Rejected spans are emitted in smaller pieces. The renderer concatenates adjacent text segments into one markdown string, so this is display-neutral; the two tests that asserted exact segment arrays now assert joined text. - Each opener is judged on its own evidence. Previously one verdict ended the whole parse, so a nested opener released everything; now the outer is released immediately and the inner is a fresh candidate that can still hold mid-stream. It resolves at end of stream either way.
Fourth malformation from a trace (220cc02d): the model wrote an opening tag with valid JSON and no closing tag of any kind. No marker rule can fire — there is no marker — but the JSON value completes and prose follows, which the depth rule settles at the first space after the `}`. Already handled by that rule; this pins it. Asserted as lossless rather than merely visible: mid-stream and complete, every character of the message survives, so nothing waits for the stream to end.
j15z
changed the title
j15z
marked this pull request as ready for review
July 25, 2026 06:04
PR SummaryMedium Risk Overview
Streaming heuristics decide when an unclosed tag cannot resolve: no nesting (foreign markers, with JSON string literals blanked so prompts can quote tag syntax), JSON depth so content after a closed Regression tests cover real transcript shapes (wrong close names, borrowed closes, prose-wrapped tags, lossless no-close cases) and assert joined text where segment boundaries are display-neutral. Reviewed by Cursor Bugbot for commit c61dd9d. Bugbot is set up for automated code reviews on this repo. Configure here. |
Contributor
Greptile SummaryThe PR makes special-tag parsing preserve unresolved prose while continuing to suppress malformed payloads.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx | Refactors special-tag resolution and fully addresses the previous JSON-string marker classification issue. |
| apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.ts | Adds focused regression coverage for complete and streaming special-tag parsing behavior. |
Reviews (2): Last reviewed commit: "fix(copilot): stop the literal path flas..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…the buffer Review round on the four-outcome rewrite. Three defects in how an opener is judged, plus the cost of judging it. A valid tag showed its raw payload as text while its closing marker streamed in. The JSON value closes at the `}`, so a half-arrived `</opt` read as stray trailing content and settled the tag as unresolvable. Every JSON-bodied tag hit this, and most replies carry a trailing <options> block. dropArrivingClose now ignores a trailing fragment that could still grow into this tag's own close. Evidence that a close is genuinely wrong still lands immediately: a misspelled `</workflow_resource>` is not a prefix of `</workspace_resource>`, and a truncated `</workspac` stops being one the moment prose follows it. bodyIsLiteralText scanned the raw body for tag markers while its streaming counterpart blanked JSON string literals first. A payload that failed its shape guard and legitimately quoted tag syntax was therefore called literal text and rendered as raw JSON, which is what discard exists to prevent. Both paths now judge the same blanked body. An opener whose own close was misspelled reached forward and matched the NEXT tag's close, swallowing a valid resource into one literal span and destroying its chip. When markers in the body prove the matched close belongs to a different opener, resolution resumes past the opener so the interior is rescanned and the inner tag still renders. Cost: resuming past a rejected opener instead of abandoning the message made each parse O(openers x length), and the parse re-runs for every streamed chunk. A 40KB reply repeatedly mentioning a tag name cost 7.3s of blocked main thread. The unclosed-body inspection is now bounded, since both rules decide on their first piece of evidence, and the opener and close lookups are memoized per parse rather than rescanning to the end of the buffer for every opener. Same message: 1.2s. A realistic 40KB reply with 8 mentions: 0.35ms per parse, 28ms across the entire stream. Also derives JSON_BODY_TAG_NAMES from SPECIAL_TAG_NAMES so a new tag cannot silently fall back to the weaker prose heuristics; deletes a docstring the rewrite orphaned above TAG_SHAPED_MARKER; corrects one still describing the first-character check that depth tracking replaced; drops a test duplicating one already in the file; and fixes a test that named the no-nesting rule but passed with that rule deleted, because its JSON closed before the marker.
Collaborator
Author
Collaborator
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c61dd9d. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The special-tag parser discards text it cannot resolve into a tag. A message that mentions a tag in prose, or wraps prose in one, loses everything from that point — sometimes several paragraphs — and resumes mid-sentence.
resolveTagAtnow resolves each opening tag to one of four outcomes:Three rules decide
literal, all conservative enough that a false positive only shows text early — the end-of-stream parse still produces the correct final render:<question>that legitimately quotes tag syntax is not mistaken for a broken one.Rejection resumes just past the opener rather than abandoning the message, so a genuinely valid tag later in the same reply still parses.
Type of Change
Testing
Tested manually against four real messages, each pinned as a regression test using its verbatim text: a matched pair whose prose was swallowed by a later backticked example, a truncated close, a matched pair wrapping prose, and no closing tag at all. The last is asserted lossless — every character survives mid-stream, so nothing waits for the stream to end.
133 message-content tests, tsc clean, biome clean.
Reviewers should focus on two behavior notes:
pendingMarkdown += s.contentacross adjacent text segments, so they concatenate into one markdown string. The tests that previously asserted exact segment arrays now assert joined text, which is what a reader sees.Checklist