feat: add design-tree strip functions for ablation experiments#140
Merged
feat: add design-tree strip functions for ablation experiments#140
Conversation
12 strip functions that remove specific information types from design-tree text for targeted ablation: 1. layout-direction-spacing (flex, gap, padding, alignment) 2. size-constraints (fill sizes, min/max, flex-grow) 3. position-stacking (no-op stub — not in design-tree yet) 4. color-values (hex/rgba → [COLOR], including SVG fill/stroke) 5. typography (font-*, line-height, text-align, text-style comments) 6. shadows-effects (box-shadow, opacity) 7. component-references ([component:] annotations, component-properties) 8. component-descriptions (no-op stub — not in design-tree yet) 9. node-names-hierarchy (names → Node1, Node2, ...) 10. overflow-text-behavior (overflow: hidden) 11. hover-interaction-states ([hover]: lines) 12. design-token-references (/* var:... */ and /* text-style:... */ comments) Post-processes generated text — does NOT modify design-tree.ts. Flow: generateDesignTree() → stripDesignTree() → send to LLM 40 new tests covering all 12 types + idempotency + safety invariants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds a new design-tree post-processing module that strips or normalizes 12 information types from generated design-tree text, a comprehensive Jest test suite validating each transformer and edge cases, escaping fixes for TEXT serialization, and a re-export of the new module from core. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This was referenced Mar 27, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/engine/design-tree-strip.test.ts`:
- Around line 264-266: Replace the length-only assertion in the "exports all 12
info types" test with an exact contract check: locate the test that references
DESIGN_TREE_INFO_TYPES and change
expect(DESIGN_TREE_INFO_TYPES).toHaveLength(12) to
expect(DESIGN_TREE_INFO_TYPES).toEqual([...expected types in exact order...]);
copy the authoritative list of the 12 items from the DESIGN_TREE_INFO_TYPES
export (the source where DESIGN_TREE_INFO_TYPES is defined) and paste them as
the array literal so the test asserts exact contents and order rather than just
length.
In `@src/core/engine/design-tree-strip.ts`:
- Around line 172-173: The current use of .filter(Boolean) after mapping (e.g.
the chain ending with reassembleStyleLine(parsed) ?? "" }).filter(Boolean))
removes empty strings and thus drops original blank separator lines; change the
filter to only remove undefined/null results (for example use a predicate that
keeps empty strings such as .filter(s => s !== undefined && s !== null) or a
type-guard for defined strings) so that reassembleStyleLine(parsed) can return
"" and blank lines are preserved. Apply this replacement for each instance that
uses .filter(Boolean) in this file (the chains around
reassembleStyleLine(parsed) and the other occurrences noted).
- Around line 263-267: The header regex in design-tree-strip.ts (inside the
renderNode() output handling) only matches a fixed whitelist of types so other
headers like COMPONENT_SET or those containing '?x?' slip through; update the
regex used where match is assigned to accept any type token (e.g., \w+ or
[A-Z0-9_?]+) and any bounding-box token so it matches all generated headers,
then continue replacing with `${match[1]}Node${counter}${match[3]}` as before
(so keep using match[1] and match[3] for indent and suffix) to ensure all node
names are anonymized regardless of node.type or missing bbox.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c81b0bbc-a6e4-4506-abcf-780191d9fce2
📒 Files selected for processing (3)
src/core/engine/design-tree-strip.test.tssrc/core/engine/design-tree-strip.tssrc/core/index.ts
- Replace .filter(Boolean) with .filter(line !== null) to preserve blank separator lines (only remove null from empty style lines) - Use universal header regex [A-Z_]+ instead of type whitelist - Assert exact DESIGN_TREE_INFO_TYPES list in test (not just length) - Keep .filter(Boolean) for property-level filtering (empty strings from token removal should be dropped) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/engine/design-tree-strip.test.ts`:
- Around line 297-304: The test "never removes text content" should be hardened
by adding adversarial text strings to the FIXTURE and asserting stripDesignTree
still preserves them: modify the test fixture used by the spec (FIXTURE) to
include nodes whose text properties contain tokens that resemble design-tree
syntax (e.g., strings with ";", "svg: <", "[component:", "(FRAME, 1x1)", "/*
var:... */") and then in the test loop over DESIGN_TREE_INFO_TYPES call
stripDesignTree and assert the resulting string contains those exact adversarial
text values (use the existing test expectations pattern). Ensure you update only
the FIXTURE and the expectations in this test so stripDesignTree and
DESIGN_TREE_INFO_TYPES references remain unchanged.
In `@src/core/engine/design-tree-strip.ts`:
- Around line 131-149: The parseStyleLine function mishandles quoted values
because it finds "svg: <" and splits on "; " without regard to quotes; update
parseStyleLine to scan raw character-by-character, track quote state (single and
double, handling escapes) and only treat "svg: <" and semicolon separators as
delimiters when they occur outside of quotes; specifically change the svgIdx
detection to locate "svg: <" only when not inside a quoted region and build
propsRaw by splitting on semicolons found outside quotes (producing properties),
preserving any quoted text like the value for "text:" and keeping svgSegment
extraction similarly quote-aware.
- Around line 258-267: The regex in stripNodeNames is too broad and renames
occurrences inside non-header lines (e.g., style:, [hover]:,
component-properties:), so update stripNodeNames to only anonymize true header
lines: first check the line does not start with "style:", "[hover]:", or
"component-properties:" (or otherwise explicitly detect a header pattern such as
leading indent + Name + space + "(" type/dimensions ")") before applying the
/^(\s*)(.+?)(\s*\([A-Z_]+,\s*[\d?]+x[\d?]+\).*)$/ match; keep the counter logic
and return path the same but skip/return untouched any excluded
style/hover/component-properties lines.
- Around line 249-255: stripComponentReferences is currently removing
“[component: ...]” from any non-`component-properties:` line, which alters
inline text; change it so the replacement only runs for node header lines. In
the stripComponentReferences function, keep the existing filter but add a header
check (e.g. a regex that matches node header patterns such as lines that begin
with optional whitespace then a header token and a colon or a list-style header
like "- Header:"), and only call line.replace(/\s*\[component:[^\]]*\]/, "")
when that headerRegex.test(line); otherwise return the original line unchanged.
- Around line 293-295: removeTokenRefs is being applied to every entry in
parsed.properties and is stripping literal text values like text: "/* var:demo
*/"; update the mapping so properties whose key/name is "text" are not passed to
removeTokenRefs (i.e., return them unchanged) while still applying
removeTokenRefs to other properties and keeping the existing .filter(Boolean)
behavior; locate the parsed.properties mapping and add a guard that checks the
property identifier (e.g., p.key or p.name depending on the property shape) and
skips removeTokenRefs when it equals "text".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 475b19d1-a820-4509-870e-351dde33dd8b
📒 Files selected for processing (2)
src/core/engine/design-tree-strip.test.tssrc/core/engine/design-tree-strip.ts
Comment on lines
+293
to
+295
| parsed.properties = parsed.properties | ||
| .map((p) => removeTokenRefs(p)) | ||
| .filter(Boolean); |
Contributor
There was a problem hiding this comment.
Skip text: segments when removing token comments.
removeTokenRefs() is applied to every property, including text:. Literal copy such as text: "/* var:demo */" or text: "/* text-style: Heading */" gets erased, which turns this ablation into a text-content ablation too.
Suggested fix
parsed.properties = parsed.properties
- .map((p) => removeTokenRefs(p))
+ .map((p) => getPropertyName(p) === "text" ? p : removeTokenRefs(p))
.filter(Boolean);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/engine/design-tree-strip.ts` around lines 293 - 295, removeTokenRefs
is being applied to every entry in parsed.properties and is stripping literal
text values like text: "/* var:demo */"; update the mapping so properties whose
key/name is "text" are not passed to removeTokenRefs (i.e., return them
unchanged) while still applying removeTokenRefs to other properties and keeping
the existing .filter(Boolean) behavior; locate the parsed.properties mapping and
add a guard that checks the property identifier (e.g., p.key or p.name depending
on the property shape) and skips removeTokenRefs when it equals "text".
- Extract text: "..." as atomic segment before splitting by "; " (handles semicolons, SVG-like content, CSS-like content inside text) - Scope [component:] removal to header lines only (not text content) - Scope node name anonymization to header lines only - Add isHeaderLine() helper to distinguish headers from style/hover lines - Add 3 adversarial text tests with CSS-like, SVG-like, component-like text Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/engine/design-tree-strip.test.ts`:
- Around line 319-330: The test's adversarial input (ADVERSARIAL) doesn't
exercise color-value stripping because it uses the 3-digit hex "#fff" which
HEX_COLOR_RE ignores; update the ADVERSARIAL test data to include a 6-digit hex
(e.g. "#ffffff") or an rgba(...) token inside the quoted text so
stripDesignTree(stripColorValues) would have to preserve it, and adjust the
expectation in the it block (which calls stripDesignTree with
DESIGN_TREE_INFO_TYPES) to assert the new token (e.g. 'text: "A; background:
`#ffffff`; display: flex"' or an rgba form) so the spec actually guards against
accidental color-value mutation.
In `@src/core/engine/design-tree-strip.ts`:
- Around line 132-166: The parser currently assumes the style: line is
single-line and that extractTextSegment only matches a quoted text: "..."
without raw newlines, which breaks when generateDesignTree injects raw
node.characters or pretty-printed SVG (multiline and containing quotes). Update
the contract by either 1) changing generateDesignTree to serialize text and SVG
safely (escape newlines and quotes or encode/wrap them so style: remains
single-line) OR 2) make the parser robust: modify extractTextSegment to match a
text: "..." segment across newlines (use dotAll /s and properly handle escaped
quotes and escaped newlines) and update parseStyleLine to locate an SVG tail
starting at "svg: <" that may span multiple lines (consume until the matching
closing ">" or end-of-style), ensuring propsRaw splitting happens only outside
those protected spans; update functions extractTextSegment and parseStyleLine
(and add tests covering multiline quoted text and multiline SVG from
generateDesignTree) to restore the invariant that text content is never removed.
- Around line 41-63: The property "align-self" is in the wrong ablation set:
remove "align-self" from the SIZE_PROPS Set and add it to the LAYOUT_PROPS Set
so alignment is treated as layout-direction-spacing; specifically update the
constants LAYOUT_PROPS and SIZE_PROPS in design-tree-strip.ts by adding
"align-self" to LAYOUT_PROPS and deleting it from SIZE_PROPS.
- Around line 277-284: The current stripComponentReferences function (used with
isHeaderLine) removes the first instance of "[component: ...]" anywhere in the
header, which can corrupt node names; change the replacement to only strip the
trailing generated annotation by matching the bracketed token at the end of the
line (allowing trailing whitespace) so internal occurrences remain untouched.
Update the regex used in stripComponentReferences for header lines to target the
suffix form (i.e., the bracketed component token followed by optional whitespace
then end-of-line) and keep the rest of the function logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3903c097-e282-4eca-bb35-9eb575d908e0
📒 Files selected for processing (2)
src/core/engine/design-tree-strip.test.tssrc/core/engine/design-tree-strip.ts
…al hex - Move align-self from SIZE_PROPS to LAYOUT_PROPS (alignment = layout) - Anchor [component:] regex with $ to only match trailing annotation - Use 6-digit hex (#FFFFFF) in adversarial test to actually exercise color strip - Note: unescaped quotes in text content is a pre-existing design-tree.ts issue Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
design-tree.ts now escapes " and \ in node.characters before wrapping in quotes. Prevents broken parsing when text contains quote characters (e.g., 'She said "hello"' → 'text: "She said \"hello\""'). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/core/engine/design-tree-strip.ts (1)
133-166:⚠️ Potential issue | 🟠 Major
stripDesignTree()still breaks on multiline generator output.
src/core/engine/design-tree.ts:370-382appends raw SVG intostyle:lines, but Line 351 splits the tree on every\nbeforeparseStyleLine()runs. A pretty-printed SVG therefore gets fragmented into separate pseudo-lines, socolor-valuesmisses nested SVG colors and the other strip modes no longer operate on the original payload atomically. The PR notes the same serializer mismatch for raw quoted text, so the "never removes text content" invariant is still not guaranteed on valid generated trees. Please align the serializer/parser contract and add regressions for multiline SVG/raw-quote text before relying on these ablations.Also applies to: 350-353
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/engine/design-tree-strip.ts`:
- Around line 302-311: The stripOverflow function currently removes every
overflow declaration; change its filter so it only removes declarations where
the property is "overflow" and its value is "hidden". Locate stripOverflow and
its use of parseStyleLine, getPropertyName and reassembleStyleLine, and replace
the predicate inside parsed.properties.filter to additionally call the value
accessor (e.g. getPropertyValue or inspect the property token), trim and compare
case-insensitively to "hidden", returning false only for overflow:hidden and
preserving overflow:auto/scroll/visible and other styles.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9bdd4cc3-c57d-4637-803e-7ffd98f88d06
📒 Files selected for processing (2)
src/core/engine/design-tree-strip.test.tssrc/core/engine/design-tree-strip.ts
Future-proof: if design-tree ever emits overflow: auto/scroll/visible, those won't be incorrectly stripped by the overflow-text-behavior ablation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/core/engine/design-tree.ts (1)
364-366:⚠️ Potential issue | 🔴 CriticalEscape raw line breaks before emitting
text:.Line 365 still writes literal
\r/\n, so a multiline TEXT node is split into multiple physical lines andstripDesignTree(tree.split("\n"))can corrupt or drop the continuation. That breaks the text-preservation invariant on valid input.💡 One safe serialization approach
// Text content if (node.type === "TEXT" && node.characters) { - styles.push(`text: "${node.characters.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`); + const escapedText = node.characters + .replace(/\\/g, "\\\\") + .replace(/\r/g, "\\r") + .replace(/\n/g, "\\n") + .replace(/"/g, '\\"'); + styles.push(`text: "${escapedText}"`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/engine/design-tree.ts` around lines 364 - 366, The TEXT node serialization currently escapes backslashes and double quotes but leaves raw CR/LF characters, which breaks round-trip; in the block that checks node.type === "TEXT" and calls styles.push with `text: ...`, also escape newlines and carriage returns (e.g., replace \r and \n with literal "\\r" and "\\n" or normalize with \r?\n -> "\\n") after escaping backslashes and before escaping quotes so multiline TEXT nodes become single-line serialized tokens and preserve content during stripDesignTree/split("\n").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/engine/design-tree-strip.ts`:
- Around line 306-309: Remove the unused local variable by eliminating the
unused call to getPropertyName inside the parsed.properties filter predicate:
update the filter in the code that calls parsed.properties.filter(...) so it no
longer declares or assigns const prop = getPropertyName(p) and simply returns
the existing condition (e.g., keep p.trim() !== "overflow: hidden"); this
removes the unused symbol getPropertyName reference and fixes the TS6133 error
in the filter predicate.
In `@src/core/engine/design-tree.test.ts`:
- Around line 200-214: The test in design-tree.test.ts doesn't exercise
backslash escaping in generateDesignTree; update the TEXT node fixture (created
via makeNode/makeFile) so the characters string includes a literal backslash as
well as quotes (e.g., contains both " and \) to trigger the replace(/\\/g,
"\\\\") branch, then assert the output contains the properly escaped sequence
(via generateDesignTree) instead of only testing quote escaping.
---
Duplicate comments:
In `@src/core/engine/design-tree.ts`:
- Around line 364-366: The TEXT node serialization currently escapes backslashes
and double quotes but leaves raw CR/LF characters, which breaks round-trip; in
the block that checks node.type === "TEXT" and calls styles.push with `text:
...`, also escape newlines and carriage returns (e.g., replace \r and \n with
literal "\\r" and "\\n" or normalize with \r?\n -> "\\n") after escaping
backslashes and before escaping quotes so multiline TEXT nodes become
single-line serialized tokens and preserve content during
stripDesignTree/split("\n").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3ae5be7d-f5f3-43ab-9cb4-2887764bf196
📒 Files selected for processing (3)
src/core/engine/design-tree-strip.tssrc/core/engine/design-tree.test.tssrc/core/engine/design-tree.ts
Exercises the replace(/\\/g, "\\\\") branch that was untested. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/engine/design-tree.ts (1)
368-378:⚠️ Potential issue | 🟠 MajorSVG content may contain newlines, causing the same parsing issue.
SVG files read from disk often contain newlines, which would break
stripDesignTree()'s line-based parsing when the SVG spans multiple physical lines.🐛 Proposed fix to normalize SVG to single line
if (existsSync(svgPath)) { - const svg = readFileSync(svgPath, "utf-8").trim(); + const svg = readFileSync(svgPath, "utf-8").trim().replace(/\n/g, " "); styles.push(`svg: ${svg}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/engine/design-tree.ts` around lines 368 - 378, The SVG read into styles via the VECTOR handling (check node.type === "VECTOR", vectorDir, mappedFile, svgPath, and styles.push(`svg: ${svg}`)) can contain newlines which breaks stripDesignTree()'s line-based parser; normalize the SVG to a single-line string before pushing it into styles (e.g., collapse all newline sequences and excess whitespace into single spaces or otherwise escape line breaks) so the stored `svg` value is line-safe for downstream parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/engine/design-tree-strip.ts`:
- Around line 132-140: generateDesignTree's text serializer currently only
escapes backslashes and quotes (node.characters.replace(/\\/g,
"\\\\").replace(/"/g, '\\"')) but not newline characters, which causes style:
lines to be split by stripDesignTree/tree.split("\n") and breaks
parseStyleLine/extractTextSegment; fix by also escaping newline characters (and
carriage returns) in the serializer—e.g., add a .replace(/\r?\n/g, "\\n") step
to the node.characters replacement chain so that produced text segments remain
single-line and parseStyleLine/extractTextSegment can reliably match the full
text: "..." segment.
In `@src/core/engine/design-tree.ts`:
- Line 365: The TEXT node serialization in styles.push (the line that builds
text: "...") currently only escapes backslashes and quotes, so embedded newlines
break stripDesignTree's tree.split("\n") parsing; update the escaping sequence
for node.characters in the styles.push call (the TEXT node handling) to first
escape backslashes, then replace actual newlines with the two-character sequence
\n, and then escape double quotes—preserving the current order to avoid
double-escaping and ensuring multiline text remains a single line for
stripDesignTree parsing.
---
Outside diff comments:
In `@src/core/engine/design-tree.ts`:
- Around line 368-378: The SVG read into styles via the VECTOR handling (check
node.type === "VECTOR", vectorDir, mappedFile, svgPath, and styles.push(`svg:
${svg}`)) can contain newlines which breaks stripDesignTree()'s line-based
parser; normalize the SVG to a single-line string before pushing it into styles
(e.g., collapse all newline sequences and excess whitespace into single spaces
or otherwise escape line breaks) so the stored `svg` value is line-safe for
downstream parsing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d5ade230-ae2f-447e-a5b1-6dfb747c34fe
📒 Files selected for processing (3)
src/core/engine/design-tree-strip.tssrc/core/engine/design-tree.test.tssrc/core/engine/design-tree.ts
Comment on lines
+132
to
+140
| /** Extract `text: "..."` segment from raw style string, handling escaped quotes. */ | ||
| function extractTextSegment(raw: string): { textSegment: string | null; rest: string } { | ||
| // Match text: "..." allowing escaped quotes inside | ||
| const textMatch = raw.match(/(?:^|;\s*)text:\s*"(?:[^"\\]|\\.)*"/); | ||
| if (!textMatch) return { textSegment: null, rest: raw }; | ||
| const textSegment = textMatch[0].replace(/^;\s*/, "").trim(); | ||
| const rest = raw.replace(textMatch[0], "").replace(/;\s*$/, "").replace(/^;\s*/, ""); | ||
| return { textSegment, rest }; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does JavaScript regex character class [^"\\] match newline characters?
💡 Result:
No, the JavaScript regex character class [^\"\\] does not match newline characters. The character class [^\"\\] matches any single character except the double quote (") and the backslash (\). In JavaScript (ECMAScript), character classes match any Unicode code point not listed, including all line terminators such as \n (U+000A), \r (U+000D), \u2028, and \u2029. This contrasts with the standalone dot (.) metacharacter, which excludes line terminators unless the 's' (dotAll) flag is used. However, inside a character class, there is no such exclusion—negated classes match line terminators unless explicitly excluded (e.g., [^\r\n\"\\]). Authoritative sources like MDN confirm character classes treat characters literally and negated classes match anything not enumerated, with no special newline exemption.
Citations:
- 1: https://mdn2.netlify.app/en-US/docs/Web/JavaScript/Guide/Regular_Expressions/Character_Classes
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Regular_expressions/Character_class
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions/Cheatsheet
- 4: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_expressions/Character_classes
🏁 Script executed:
# First, let's examine the extractTextSegment function and its context in design-tree-strip.ts
cd src/core/engine && head -160 design-tree-strip.ts | tail -50Repository: let-sunny/canicode
Length of output: 1916
🏁 Script executed:
# Let's also check the stripDesignTree function to see how it splits and calls extractTextSegment
cd src/core/engine && rg -A 10 "stripDesignTree" design-tree-strip.ts | head -40Repository: let-sunny/canicode
Length of output: 578
🏁 Script executed:
# Check the design-tree.ts file to see generateDesignTree and how it handles text
cd src/core/engine && fd "design-tree.ts" --exec wc -l {}Repository: let-sunny/canicode
Length of output: 82
🏁 Script executed:
# Look for generateDesignTree implementation and text serialization
cd src/core/engine && rg -B 3 -A 15 "function generateDesignTree|export.*generateDesignTree" design-tree.ts | head -60Repository: let-sunny/canicode
Length of output: 1142
🏁 Script executed:
# Also search for where text content is encoded/written in design-tree.ts
cd src/core/engine && rg -B 2 -A 5 'text:.*"' design-tree.ts | head -80Repository: let-sunny/canicode
Length of output: 393
🏁 Script executed:
# Check the exact escaping in generateDesignTree - does it handle newlines?
cd src/core/engine && rg -B 5 -A 10 'node.characters.replace' design-tree.tsRepository: let-sunny/canicode
Length of output: 708
🏁 Script executed:
# Let's also check if there are any tests that verify text with newlines
cd . && fd -e test.ts -e spec.ts | xargs rg -l "generateDesignTree|stripDesignTree" 2>/dev/null | head -5Repository: let-sunny/canicode
Length of output: 139
🏁 Script executed:
# Check the tests for text with newlines
cd src/core/engine && cat -n design-tree.test.ts | grep -A 10 -B 5 "newline\|\\\\n" | head -40Repository: let-sunny/canicode
Length of output: 1445
🏁 Script executed:
# Check design-tree-strip.test.ts for newline tests
cd src/core/engine && cat -n design-tree-strip.test.ts | grep -A 10 -B 5 "newline\|\\\\n" | head -40Repository: let-sunny/canicode
Length of output: 1957
🏁 Script executed:
# Let's also see if there's any escape handling for newlines elsewhere
cd src/core/engine && rg "\\\\n" design-tree.ts design-tree-strip.tsRepository: let-sunny/canicode
Length of output: 240
generateDesignTree must escape newlines in text content for the parser to work correctly.
The text serializer at line node.characters.replace(/\\/g, "\\\\").replace(/"/g, '\\"') escapes backslashes and quotes, but does not escape newlines. If text content contains an actual newline character, it becomes a literal line break in the design tree output. When stripDesignTree calls tree.split("\n"), this breaks the style: line across multiple input lines, causing parseStyleLine to fail to match the complete text: "..." segment. The serializer must escape newlines as \n sequences so they survive the split operation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/engine/design-tree-strip.ts` around lines 132 - 140,
generateDesignTree's text serializer currently only escapes backslashes and
quotes (node.characters.replace(/\\/g, "\\\\").replace(/"/g, '\\"')) but not
newline characters, which causes style: lines to be split by
stripDesignTree/tree.split("\n") and breaks parseStyleLine/extractTextSegment;
fix by also escaping newline characters (and carriage returns) in the
serializer—e.g., add a .replace(/\r?\n/g, "\\n") step to the node.characters
replacement chain so that produced text segments remain single-line and
parseStyleLine/extractTextSegment can reliably match the full text: "..."
segment.
Real fixtures (e.g., mobile-ai-chat) have multiline text content. Unescaped newlines break stripDesignTree's line-based parsing. Escape order: backslash → newline → carriage return → quote Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SVG files from fixtures are 3+ lines. Without flattening, the style line splits across multiple lines, breaking stripDesignTree's line-based parsing. This is the last remaining unsafe content issue: - text: quotes/backslashes/newlines → already escaped - svg: multiline → now flattened to single line - node names → isHeaderLine() guard in strip functions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/engine/design-tree.test.ts`:
- Around line 248-250: The test currently calls lines.find(...) which can return
undefined and then immediately asserts on textLine; update the test in
design-tree.test.ts to guard the find result by asserting textLine is defined
(e.g., expect(textLine).toBeDefined() or an if-check) before calling
expect(textLine).toContain(...), ensuring the variable textLine (from
output.split and lines.find) cannot be undefined when you perform the content
assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7975ef3c-2576-4ff5-90e5-1c3339a656ef
📒 Files selected for processing (2)
src/core/engine/design-tree.test.tssrc/core/engine/design-tree.ts
Comment on lines
+248
to
+250
| const lines = output.split("\n"); | ||
| const textLine = lines.find((l) => l.includes("text:")); | ||
| expect(textLine).toContain("Line 1\\nLine 2\\nLine 3"); |
Contributor
There was a problem hiding this comment.
Guard the find() result before content assertion.
Line 249 can return undefined; add an explicit defined check before Line 250 to satisfy strict undefined-edge-case expectations in tests.
Proposed fix
const lines = output.split("\n");
const textLine = lines.find((l) => l.includes("text:"));
- expect(textLine).toContain("Line 1\\nLine 2\\nLine 3");
+ expect(textLine).toBeDefined();
+ expect(textLine!).toContain("Line 1\\nLine 2\\nLine 3");As per coding guidelines: "**/*.test.ts: Verify edge cases for noUncheckedIndexedAccess (undefined checks)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const lines = output.split("\n"); | |
| const textLine = lines.find((l) => l.includes("text:")); | |
| expect(textLine).toContain("Line 1\\nLine 2\\nLine 3"); | |
| const lines = output.split("\n"); | |
| const textLine = lines.find((l) => l.includes("text:")); | |
| expect(textLine).toBeDefined(); | |
| expect(textLine!).toContain("Line 1\\nLine 2\\nLine 3"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/engine/design-tree.test.ts` around lines 248 - 250, The test
currently calls lines.find(...) which can return undefined and then immediately
asserts on textLine; update the test in design-tree.test.ts to guard the find
result by asserting textLine is defined (e.g., expect(textLine).toBeDefined() or
an if-check) before calling expect(textLine).toContain(...), ensuring the
variable textLine (from output.split and lines.find) cannot be undefined when
you perform the content assertion.
This was referenced Mar 27, 2026
3 tasks
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
12 strip functions for targeted ablation — remove one information type at a time from design-tree text, then measure pixel accuracy impact (ΔV).
Flow
Post-processes generated text. Does NOT modify design-tree.ts.
12 strip types
[COLOR], including SVG fill/stroke[component:],component-properties:[hover]:lines/* var:... */,/* text-style:... */commentsSafety invariants (tested)
# Design Tree)text: "...")Closes #130
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests