Skip to content

Commit f599991

Browse files
j15zclaude
andcommitted
docs(copilot): reattach an orphaned docstring and cut comment noise
One real defect. Adding hasSpecialTagMarker put it BETWEEN unclosedTagCannotResolve and the docstring written for it, so two doc blocks sat stacked and the function they described ended up with none. A reader scanning the file would have read the first block as preamble for the wrong function. Moved back, and while there: "14 substring scans" is SPECIAL_TAG_NAMES.length * 2, so adding an eighth tag would have quietly made it wrong — it now says a pass per tag name. The rest is noise removal: - Three comments narrating what the parser used to do. A comment is read by someone looking at the current code, not the diff, and this branch already writes that history at length in its commit messages. - Three millisecond measurements. Each one already stated the durable claim — quadratic, or a copy thrown away per opener per chunk — and then appended a number that will rot. The one measurement kept is the blind-spot paragraph on MAX_UNCLOSED_BODY_SCAN, where the number is what justifies the constant. - Two of the four restatements of "the renderer concatenates adjacent text segments". Kept where it is load-bearing, on pushText and on the test helper. - One claim gone stale in the last commit: the read-budget doc still described two helpers agreeing with each other, after they became one function. Left alone deliberately: the rules that look arbitrary and are not — marker blanking, the differing resume offsets, the accepted trades. Those are why this file reads as heavily commented, and they are the ones worth having. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2f57cf6 commit f599991

3 files changed

Lines changed: 31 additions & 49 deletions

File tree

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-sanitize.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ const HIDDEN_INLINE_REFERENCE_PATTERN =
1919
* - **No `<workspace_resource>` between opener and closer**, via the negative
2020
* lookahead. This is a cost bound, not a correctness rule: a lazy scan that may
2121
* cross an opener restarts from every opener, so a message repeating the tag
22-
* name is quadratic. Measured at 168KB of such prose: 154ms per call without
23-
* it, 0.34ms with — and this runs on the main thread for every streamed chunk.
22+
* name is quadratic — on the main thread, for every streamed chunk.
2423
*
2524
* Backticks must be DIRECTLY adjacent. Allowing whitespace between let the
2625
* pattern reach past the tag and take the delimiter off a neighbouring code

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,6 @@ describe('parseSpecialTags with <question>', () => {
223223
})
224224

225225
it('still parses a valid tag that follows a rejected one', () => {
226-
// Before the rewrite, rejecting an unclosed tag abandoned the rest of the
227-
// message, so this <options> tag was never parsed at all.
228226
const { segments } = parseSpecialTags(
229227
'I use <thinking> loosely here. Anyway: <options>[{"title":"A","description":"d"}]</options> done.',
230228
false
@@ -437,8 +435,6 @@ describe('parseSpecialTags with <question>', () => {
437435
})
438436

439437
it('shows prose immediately mid-stream instead of blanking the rest', () => {
440-
// The failure this replaces: everything after the marker stayed invisible
441-
// for the remainder of the stream, then reappeared when it ended.
442438
const content = 'The `<workspace_resource>` chip only renders for a real file.'
443439
const { segments, hasPendingTag } = parseSpecialTags(content, true)
444440
expect(hasPendingTag).toBe(false)
@@ -627,9 +623,6 @@ describe('parseSpecialTags with <question>', () => {
627623
'The `<workspace_resource>` file chip only renders when its path points to a real file.'
628624
const { segments, hasPendingTag } = parseSpecialTags(content, false)
629625
expect(hasPendingTag).toBe(false)
630-
// Asserted on the joined text, not segment boundaries: the renderer
631-
// concatenates adjacent text segments, so how the span is split is not
632-
// observable to a reader.
633626
expect(segments.every((segment) => segment.type === 'text')).toBe(true)
634627
expect(renderedText(segments)).toBe(content)
635628
})

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx

Lines changed: 30 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -516,10 +516,9 @@ const JSON_BODY_TAG_NAMES: ReadonlySet<(typeof SPECIAL_TAG_NAMES)[number]> = new
516516
* first character that breaks JSON viability — so a bounded window reaches the
517517
* same verdict as the full remainder for any payload a tag actually carries.
518518
* Unbounded, the check is O(body length) and runs once per opener inside a parse
519-
* that re-runs for every streamed chunk: a long reply repeatedly mentioning a tag
520-
* name cost seconds of main-thread time, and one 58KB reply whose early close was
521-
* misspelled — so a single body stretched most of the message — cost 242ms per
522-
* parse against 35ms bounded.
519+
* that re-runs for every streamed chunk. A long reply repeatedly mentioning a
520+
* tag name, or one whose misspelled early close stretches a single body across
521+
* most of the message, is then quadratic in the length of the reply.
523522
*
524523
* The window's one blind spot, and why it is accepted: a JSON body whose
525524
* top-level value closes BEYOND the window, followed by prose and no closing tag,
@@ -618,14 +617,24 @@ function isViableJsonPrefixOf(scannable: string): boolean {
618617
return true
619618
}
620619

620+
/**
621+
* Whether `text` contains a marker for one of the tags this parser knows.
622+
*
623+
* Deliberately the tag NAMES rather than anything tag-shaped. A prose body may
624+
* legitimately contain `<div>` or `Promise<void>`; only a marker the parser
625+
* would itself act on proves the enclosing opener was text. Shared so the
626+
* streaming and matched-pair paths cannot answer the same question differently
627+
* — them disagreeing is what let a late close swallow content already on screen.
628+
*/
629+
function hasSpecialTagMarker(text: string): boolean {
630+
return SPECIAL_TAG_NAMES.some((name) => text.includes(`</${name}>`) || text.includes(`<${name}>`))
631+
}
632+
621633
/**
622634
* True when an opening tag with no close yet can NEVER resolve, so the text
623635
* after it should be shown immediately instead of held back until the stream
624-
* ends.
625-
*
626-
* Without this, a message that merely mentions a tag in prose goes blank from
627-
* that point on for the rest of the stream — the text is only restored once
628-
* streaming stops.
636+
* ends. Without it, a message that merely mentions a tag in prose goes blank
637+
* from that point on until streaming stops.
629638
*
630639
* One rule decides it, chosen by body kind:
631640
*
@@ -635,32 +644,18 @@ function isViableJsonPrefixOf(scannable: string): boolean {
635644
* prose (no `{` at all), a misspelled close like `</workflow_resource>`, a
636645
* truncated `</workspac`, or no close whatsoever.
637646
* - **The prose-bodied tag** has no JSON to test, so the only evidence available
638-
* is that tags never nest: another special-tag marker in the body — opening or
639-
* closing, foreign or its own name — means this opener was literal text.
647+
* is that tags never nest: a marker for another special tag in the body means
648+
* this opener was literal text.
640649
*
641-
* Scanning for nested markers on a JSON body too was redundant: any marker
642-
* outside a string literal is itself content the JSON rule already rejects, and
643-
* markers inside one are legitimate quoted syntax that must NOT count as
644-
* evidence. It cost 14 substring scans per opener per streamed chunk to catch
645-
* nothing the viability rule misses.
650+
* Nested markers are NOT scanned for on a JSON body. A marker outside a string
651+
* literal is content the viability rule already rejects, and one inside is
652+
* legitimate quoted syntax that must not count as evidence — so the scan cost a
653+
* pass per tag name, open and close, to catch nothing.
646654
*
647-
* Both are conservative: they only fire on content that could not have parsed.
648-
* A false positive would merely show text early that a later chunk resolves
649-
* into a tag the end-of-stream parse still produces the correct final render.
655+
* Both rules are conservative: they fire only on content that could not have
656+
* parsed. A false positive merely shows text early that a later chunk resolves
657+
* into a tag, and the end-of-stream parse still renders correctly.
650658
*/
651-
/**
652-
* Whether `text` contains a marker for one of the tags this parser knows.
653-
*
654-
* Deliberately the tag NAMES rather than anything tag-shaped. A prose body may
655-
* legitimately contain `<div>` or `Promise<void>`; only a marker the parser
656-
* would itself act on proves the enclosing opener was text. Shared so the
657-
* streaming and matched-pair paths cannot answer the same question differently
658-
* — them disagreeing is what let a late close swallow content already on screen.
659-
*/
660-
function hasSpecialTagMarker(text: string): boolean {
661-
return SPECIAL_TAG_NAMES.some((name) => text.includes(`</${name}>`) || text.includes(`<${name}>`))
662-
}
663-
664659
function unclosedTagCannotResolve(
665660
tagName: (typeof SPECIAL_TAG_NAMES)[number],
666661
body: string
@@ -672,8 +667,7 @@ function unclosedTagCannotResolve(
672667
// Cheap rejection before the expensive one. isViableJsonPrefixOf decides on
673668
// the first non-whitespace character when it is not `{` or `[` — which is the
674669
// common case, a tag name mentioned in prose — so testing it here avoids
675-
// blanking up to a full window of text only to throw the copy away. Measured
676-
// at 86KB of such prose: 43ms per streaming parse before, 2.4ms after.
670+
// blanking up to a full window of text only to throw the copy away.
677671
const firstChar = pending.trimStart().charAt(0)
678672
if (firstChar !== '' && firstChar !== '{' && firstChar !== '[') return true
679673

@@ -804,9 +798,8 @@ export function memoizedIndexOf(
804798
* How much of a body may be inspected, and whether that is all of it.
805799
*
806800
* The read budget, isolated from what the body turns out to BE. Both the
807-
* unclosed and matched-pair paths spend it, and getting them to agree is the
808-
* point: they previously applied the same constant in two shapes, and the
809-
* difference between them was a bug.
801+
* unclosed and matched-pair paths spend it through this one function, so they
802+
* cannot drift out of agreement about how much of a body may be read.
810803
*/
811804
interface InspectedBody {
812805
/** The prefix actually examined. */
@@ -988,9 +981,6 @@ function resolveTagAt(
988981
* tags are suppressed and flagged via `hasPendingTag` so the caller can show a
989982
* loading indicator, and a trailing partial opening marker (`<opt`, `<usage_`)
990983
* is stripped during streaming so it never flashes as raw markup.
991-
*
992-
* Adjacent text segments are concatenated by the renderer, so emitting a span
993-
* as several pieces is display-neutral.
994984
*/
995985
export function parseSpecialTags(content: string, isStreaming: boolean): ParsedSpecialContent {
996986
const segments: ContentSegment[] = []

0 commit comments

Comments
 (0)