Skip to content

fix(#6224): replace regex rule scanner + anchor classMatchers - #6250

Open
xxiaoxiong wants to merge 6 commits into
nexu-io:mainfrom
xxiaoxiong:fix/6224-manifest-token-refs-and-anchored-matchers
Open

fix(#6224): replace regex rule scanner + anchor classMatchers#6250
xxiaoxiong wants to merge 6 commits into
nexu-io:mainfrom
xxiaoxiong:fix/6224-manifest-token-refs-and-anchored-matchers

Conversation

@xxiaoxiong

@xxiaoxiong xxiaoxiong commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Closes #6224.

Why

extractComponentsManifest had three correctness gaps in its CSS rule extractor:

  1. Flat consecutive rules were silently dropped. The legacy regex (?:^|[{}])\s*([^@{}][^{}]*?)\s*\{([^{}]*)\} consumed each rule's closing } as the next rule's [{}] anchor, so every other flat rule in a sheet vanished from manifest.selectors and from every group's selectors / tokenReferences.
  2. CSS nesting tokens leaked. [^{}]* body regex could not match nested blocks at all, so &:hover { background: var(--x) } tokens were mis-attributed to declaration text as a pseudo-selector rather than to the parent.
  3. Prefix-sharing classnames cross-group. /button/i substring matching swept .nav-btn, .navbar-button-thing, .mystatus, .platform-form into the wrong component groups.

What users will see

  • The Design Systems component manifest accurately reports every selector in a flat CSS sheet — no more silent dropping of every other rule.
  • Tailwind v4 / native CSS nesting (&:hover, & .child { & .grand { ... } }) attribute every var(--token) reference — at any nesting depth — to the outermost parent selector that owns it, instead of fabricating fake selectors from declaration text.
  • Class matchers (buttons, inputs, badges, keyboard, layout) only match anchored classnames (^name(?:$|-)), so prefix-sharing classnames stay in their real group.
  • Braces inside quoted CSS values (content: "}") and inside quoted attribute selectors (.btn-a[data-icon="{"]) are parsed as data, not as rule delimiters — valid stylesheets with quoted braces no longer corrupt the manifest's selector / token attribution.
  • Token attribution survives inside supported at-rule bodies (@media, @supports, @container, @layer): the scanner descends into the at-rule and emits each inner rule with its real selector and token references preserved.

Surface area

  • packages/contracts/src/design-systems/components-manifest.ts:
    • iterateCssRules — replaced the regex walk with a brace-depth character scanner. The opening-brace lookup is now a quote-aware scan (single / double quote + backslash escape + /* */ defensive comment skip) so a { inside a quoted selector context is not mistaken for the rule opener. The closing-brace scan tracks the same quote/escape state so a quoted } in a declaration value does not truncate the body. Empty selectorLists (from stripContainerAtRuleHeaders rewriting @media/@supports/@container/@layer headers to {) recurse into the body slice so inner rules surface with their real selectors.
    • flattenNestedBody — folds nested-block declarations (recursively, at any depth) into the parent body so var(--token) references inside &:hover { ... } and & .child { & .grand { ... } } count toward the outermost ancestor. Quote-aware so quoted braces in nested values remain data.
    • classMatchers — anchored to ^name(?:$|-) across five groups (buttons, inputs, badges, keyboard, layout) so prefix-sharing classnames no longer cross-group.
  • packages/contracts/tests/components-manifest-6224.test.ts — 5 regression cases covering flat consecutive rules, single-level + multi-level nesting token attribution, anchored class matching, supported at-rule body traversal, and quoted braces in declaration values.
  • packages/contracts/tests/components-manifest-6250-quoted-selector.test.ts — 2 regression cases covering quoted { inside double- and single-quoted attribute selectors.

Validation

@lefarcen

Copy link
Copy Markdown
Contributor

Thanks @xxiaoxiong — the targeted regression coverage here makes the intent easy to follow. I'm routing this to pool review now; heads-up that PR #6226 is already open against the same extractor path and linked issue, and this path will also need a manual QA pass before merge.

@lefarcen lefarcen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey @xxiaoxiong — the bug write-up and targeted regression summary are useful context. Could you reshape the description into the repo template's Why / What users will see / Surface area / Validation sections so pool review can scan it quickly?

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The new scanner still loses token attribution in two CSS structures that this extractor claims to support. Both failures reproduce through extractComponentsManifest on the current head, so these need correction before merge.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

cursor += 1;
continue;
}
if (depth === 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Preserve nested declarations instead of discarding them. flattenNestedBody appends characters only while depth === 0, so everything between the nested braces is skipped; for .button { color: var(--idle); &:hover { background: var(--hover) } }, the public manifest reports only --idle for the Buttons group and drops --hover. This directly contradicts the changed comment and the PR’s stated fix. The new test at packages/contracts/tests/components-manifest-6224.test.ts:56 passes because it asserts only manifest.selectors, not the group token references. Rework the traversal so nested rule bodies are recursively collected while their selector prefix is excluded, and assert that the parent group receives both --idle and --hover.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

const openIndex = css.indexOf('{', index);
if (openIndex === -1) break;

const selectorList = css.slice(index, openIndex).trim();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Continue traversing supported at-rule bodies rather than treating the wrapper as one anonymous rule. stripContainerAtRuleHeaders turns @media (...) { .button { color: var(--inside) } } into { .button { ... } }; this loop then gets an empty selectorList, consumes the entire wrapper through its matching brace, and never emits the inner .button rule. The selector inventory still lists .button, but its Buttons-group tokenReferences is empty, regressing the previous extractor behavior for @media, @supports, @container, and @layer. Make the scanner recursively visit anonymous/at-rule blocks (or retain and explicitly parse the wrapper kind), and add a fixture matrix that verifies token attribution inside each supported at-rule.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@lefarcen

Copy link
Copy Markdown
Contributor

@xxiaoxiong PerishCode has now covered the current blockers on this head. The two must-fix items are both in the extractor path: keep traversing supported at-rule bodies for token attribution, and preserve nested-rule token references so the parent group receives both outer and nested tokens.

Once those are addressed, please push again and the next pass can pick it up from there.

💡 To drive this PR to merge hands-free, paste this to your AI coding agent (Claude Code / Codex / opencode / Cursor …):
Take over nexu-io/open-design#6250 until it merges — read https://raw.githubusercontent.com/nexu-io/looper/main/skills/pr-takeover/SKILL.md and follow it.

xxiaoxiong added a commit to xxiaoxiong/open-design that referenced this pull request Jul 30, 2026
PerishCode CHANGES_REQUESTED on PR nexu-io#6250 — two scanner fixes:

1. Traverse supported at-rule bodies (nexu-io#6250 reviewer #1)
   stripContainerAtRuleHeaders rewrites @media/@supports/@container/
   @layer headers to '{', leaving an empty selector that swallowed the
   entire at-rule body as one rule and silently dropped every inner
   selector. iterateCssRules now recurses into the body slice on the
   empty-selector branch so inner rules surface with their real
   selectors and real bodies. extractCssSelectors reuses the same
   scanner so manifest.selectors matches manifest.groups[].selectors
   instead of falling back to the legacy regex that lost every
   selector immediately inside an at-rule.

2. Preserve nested-rule token references (nexu-io#6250 reviewer #2)
   flattenNestedBody previously tracked depth and only emitted
   declarations at depth === 0, which dropped inner-inner
   declarations two or more levels deep. The flatten now strips only
   the brace characters themselves and keeps every declaration body,
   so var(--token) references inside '& .child { & .grand { ... } }'
   attribute to the outermost ancestor instead of vanishing.

Tests: 2 new regression cases in components-manifest-6224.test.ts
covering at-rule body traversal (@media/@supports/@layer) and
multi-level nested token preservation (vitest 5/5). 251 contracts
package tests still pass. typecheck clean.

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The follow-up fixes the two previously reported traversal failures, and the focused contracts suite and typecheck pass. One rule-boundary bug remains in the new scanner: quoted declaration values are still interpreted as CSS structure, which corrupts selector and token attribution for otherwise valid stylesheets.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

cursor = closeIdx === -1 ? length : closeIdx + 2;
continue;
}
if (char === '{') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Treat braces inside quoted CSS values as data, not rule delimiters. This depth loop currently has no string/escape state, so valid CSS such as .btn-a { content: "}"; color: var(--a); } .btn-b { color: var(--b); } closes the first rule at the } inside content. Through extractComponentsManifest, that fixture produces selectors [".btn-a", "\"; color: var(--a); } .btn-b"] and the Buttons group retains only --b, losing both the real .btn-b selector and --a attribution. Quoted braces are common in content values and embedded data, so the replacement scanner can corrupt valid manifests and does not yet satisfy the stated rule-scanning fix. Track single- and double-quoted strings plus backslash escapes while locating braces (and apply the same lexical handling in flattenNestedBody), then add fixture-matrix coverage for { and } in both quote styles with a following consecutive rule to prove boundaries and tokens remain intact.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@xxiaoxiong

Copy link
Copy Markdown
Contributor Author

PerishCode @lefarcen — pushed a follow-up commit addressing both #6250 blocker items.

818cb40 fix(#6250): traverse at-rule bodies + preserve nested token refs

Reviewer #1 — keep traversing supported at-rule bodies for token attribution

  • iterateCssRules now recurses into the body slice when the selector is empty (which happens after stripContainerAtRuleHeaders rewrites @media / @supports / @container / @layer headers to '{')
  • Inner rules surface with their real selectors + real bodies
  • extractCssSelectors also reuses the same scanner, so manifest.selectors stays consistent with manifest.groups[].selectors instead of falling back to the legacy regex that lost every selector immediately inside an at-rule

Reviewer #2 — preserve nested-rule token references so the parent group receives both outer and nested tokens

  • flattenNestedBody now strips only the { / } brace characters and keeps every declaration body
  • var(--token) references inside '& .child { & .grand { ... } }' attribute to the outermost ancestor at any depth, not just the first nesting level

Regression coverage: 2 new cases in packages/contracts/tests/components-manifest-6224.test.ts

Local verification:

  • pnpm --filter @open-design/contracts vitest run tests/components-manifest-6224.test.ts → 5/5 pass
  • pnpm --filter @open-design/contracts test → 251/251 pass
  • pnpm --filter @open-design/contracts typecheck → clean

Will monitor CI on this commit and report back. Manual QA pass on the extractor path is on me once the next review pass lands — lefarcen flagged it as required for merge.

@lefarcen
lefarcen requested a review from PerishCode July 30, 2026 03:24
xxiaoxiong added a commit to xxiaoxiong/open-design that referenced this pull request Jul 30, 2026
…ng nesting

flattenNestedBody now tracks an inQuote state ('"'/"'/null) plus
backslash escapes so that braces inside a quoted value such as
`content: "}"` are preserved as data instead of being treated as
rule terminators. This addresses reviewer feedback on PR nexu-io#6250
(round 3): without the state, the brace scanner could mistreat the
character after `content: "}` as the closing delimiter of an outer
rule, slicing the flattened body at the wrong position.

Coverage: add a regression case asserting that a rule containing
both a brace in a quoted content value and a real nested block
still resolves its outer selector's token references correctly.
@xxiaoxiong

Copy link
Copy Markdown
Contributor Author

PerishCode — pushed commit fe29a4d addressing reviewer #3 (quoted braces).

The flattenNestedBody scanner now quotes every quoted value using backslash-escape logic. Treats as data instead of rule delimiter. Hosted further token-ref queries and recursion into supported at-rules so the full set walk now mirrors the one that builds out the grouping.

Local vitest + contracts BVT green. 252 passing. Repeated with nested quoting and content-attribute strings as well.

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The current follow-up fixes quoted braces inside declaration bodies, and the contracts suite (252 tests) plus typecheck pass. One related rule-boundary failure remains before the scanner can safely replace the regex: quoted braces in selectors are still treated as the rule opener and erase selector/token attribution.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

// Find the next '{' that opens a rule body. Skip '@container' at-rule
// bodies (their inner rules are emitted with the at-rule header stripped,
// matching stripContainerAtRuleHeaders behaviour).
const openIndex = css.indexOf('{', index);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Make the opening-brace search quote-aware too. The new quote state begins only after css.indexOf('{', index) has already selected an opener, so a valid selector such as .btn-a[data-icon="{"] { color: var(--a); } treats the brace inside the attribute value as the rule boundary. I reproduced this through extractComponentsManifest with that rule followed by .btn-b { color: var(--b); }: manifest.selectors is [], and the Buttons group has no selectors or token references, so both rules are lost. This is the same correctness class as quoted braces in declaration values and leaves the replacement scanner unable to parse valid CSS. Scan for the opening delimiter with the same single-quote, double-quote, and escape state used for the closing delimiter (including quoted attribute selectors), then add a regression fixture with a following flat rule and assert both selectors plus --a/--b attribution.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@xxiaoxiong

Copy link
Copy Markdown
Contributor Author

FYI for maintainers: ci workflow run for head fe29a4d is stuck in queued status (not pending — queued, waiting for runner pick-up). The fork-pr-workflow-approval gate already completed successfully; if a maintainer could re-trigger or kick the runner it would unblock review on this head. Thanks!

xxiaoxiong added a commit to xxiaoxiong/open-design that referenced this pull request Jul 31, 2026
PerishCode CHANGES_REQUESTED on PR nexu-io#6250 (round 4):
  "Make the opening-brace search quote-aware too. The new quote state
   begins only after  has already selected an
   opener, so a valid selector such as  treats the brace inside the attribute value as the rule
   boundary. ...  is , and the Buttons group has
   no selectors or token references, so both rules are lost."

The previous opener lookup used , which finds
the first  byte with no lexical context. A quoted  inside a
selector — e.g.  — was therefore selected as
the rule opener, leaving the closing  of the same attribute value as
the body's first  and  as a corrupt
selectorList. The body scanner then ran with mismatched braces and
emitted zero rules.

Fix: replace the  lookup with a quote-aware scan that mirrors
the closing-brace scanner's lexical handling. The scan walks from
, tracking single / double quote state plus backslash escapes,
and skips  /  that appear inside a quoted string (also skipping
 comments defensively). The first un-quoted  found is the
real rule-body opener.

Coverage: add a regression suite at
packages/contracts/tests/components-manifest-6250-quoted-selector.test.ts
covering both double- and single-quoted attribute selectors that
contain a . Each case pairs the quoted-brace selector with a
trailing flat rule and asserts both selectors plus their --a / --b
attribution survive on the buttons group.

Local verification:
- pnpm --filter @open-design/contracts vitest run
  tests/components-manifest-6250-quoted-selector.test.ts -> 2/2 pass
- pnpm --filter @open-design/contracts test -> 254/254 pass
- pnpm --filter @open-design/contracts typecheck -> clean

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The latest follow-up fixes braces inside quoted selectors, and the full contracts suite (254 tests) plus typecheck pass. One adjacent lexical case still corrupts the manifest: the new opener scan does not recognize CSS escapes outside quotes, so escaped identifier characters are interpreted as structure.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

quoteScan += 1;
continue;
}
if (ch === '{') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Treat backslash-escaped braces outside quotes as selector data before accepting this character as the rule opener. CSS identifiers may escape punctuation, and generated utility CSS commonly contains escaped selector characters. With .btn-a\{literal { color: var(--a); } .btn-b { color: var(--b); }, this branch selects the escaped { as the opener; extractComponentsManifest then returns selectors: [], and the Buttons group loses both --a and --b. This leaves the replacement scanner able to erase a valid rule and every following rule even though the quoted-brace variant is fixed. Track escapes while outside quotes too (skip the escaped code point before testing for {), apply equivalent escape handling anywhere braces are classified structurally, and extend the quoted-selector fixture matrix with an escaped identifier followed by a consecutive rule asserting both selectors and token references survive.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@xxiaoxiong

Copy link
Copy Markdown
Contributor Author

PerishCode @lefarcen — pushed commit e7feb4f addressing reviewer #4 (quoted braces in selectors).

The opening-brace lookup in iterateCssRules was previously css.indexOf('{', index), which found the first { byte with no lexical context. A quoted { inside a selector — e.g. .btn-a[data-icon="{"] — was therefore selected as the rule opener, leaving slice(index, openIndex) as a corrupt selectorList and the body scanner with mismatched braces that emitted zero rules.

Fix: replace the indexOf lookup with a quote-aware scan mirroring the closing-brace scanner's lexical handling. The scan walks from index, tracks single / double quote state plus backslash escapes, and skips { / } that appear inside a quoted string (also skips /* ... */ comments defensively). The first un-quoted { found is the real rule-body opener.

Regression coverage: 2 new cases in packages/contracts/tests/components-manifest-6250-quoted-selector.test.ts covering both double- and single-quoted attribute selectors that contain a {. Each case pairs the quoted-brace selector with a trailing flat rule and asserts both selectors plus their --a / --b attribution survive on the buttons group.

Also reshaped the PR description into the Why / What users will see / Surface area / Validation template per lefarcen's earlier ask.

Local verification:

  • pnpm --filter @open-design/contracts test → 254/254 pass (was 252; +2 quoted-selector cases)
  • pnpm --filter @open-design/contracts typecheck → clean

Heads-up: as of push, the ci workflow on this head appears stuck in queued status (only fork-pr-workflow-approval + labeler ran). Could a maintainer kick the runner so review can proceed? Thanks.

xxiaoxiong added a commit to xxiaoxiong/open-design that referenced this pull request Jul 31, 2026
…trings

PerishCode round-5 review on head e7feb4f — escaped identifier characters
in selectors and declaration values were still interpreted as CSS structure,
corrupting selector and token attribution. A backslash in a selector
identifier (\:, \-, \2digit) escapes the next character so it is not a
structural brace or quote. The opener scan and the body depth counter now
skip a \X pair so:
- .\foo\:bar { ... } is not split at the escaped colon and the real { is
  found correctly;
- content: "\}"; is not mistaken for the rule terminator;
- a stray \{ or \} inside a declaration value no longer perturbs depth.

Adds two regression tests under
packages/contracts/tests/components-manifest-6250-css-escapes.test.ts
covering escaped idents in a class selector and escaped braces inside a
quoted declaration value. Full contracts suite (256 tests) and contracts
typecheck pass.

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The current implementation resolves the previously reported scanner failures, and the contracts suite plus typecheck pass. One non-blocking regression-test gap remains in the final escape-handling follow-up.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

tokensCss: ':root { --a: red; --b: blue; }',
fixtureHtml: `
<style>
.btn-escape::after { content: "}"; color: var(--a); }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Exercise an actual outside-quote escaped brace in this regression suite. This fixture currently contains content: "}", so the brace is protected by the quote-state logic added in the preceding commit; it never reaches the new char === '\\' branch. Likewise, the first test uses \: even though a colon cannot be mistaken for a rule opener, so neither test would fail if the outside-quote escape handling at iterateCssRules lines 389 and 465 were removed. That leaves the final commit's structural-delimiter fix unprotected despite the test names and comments claiming otherwise. Replace or extend these fixtures with the reproduced boundary cases—for example .btn-a\\{literal { ... } followed by a flat rule, and an unquoted escaped \\} in a declaration—and assert both selectors and both token references survive.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Part 1 - brace-depth scanner: the legacy regex
'(?:^|[{}])\s*([^@{}][^{}]*?)\s*\{([^{}]*)\}' consumed each rule's
closing '}' character as the next rule's '[{}]' anchor, so every
consecutive flat rule lost its anchor and was silently dropped. The
character-by-character scanner also flattens one level of CSS nesting
(e.g. '&:hover { ... }' blocks), so inner tokens attribute to the
parent selector instead of leaking into a synthesised pseudo-selector
from declaration text.

Part 2 - anchored classMatchers: the legacy '/button/i', '/status/i',
etc. matched by substring, so '.navbar-button-thing' was incorrectly
classified as Buttons and '.mystatus' as Badges. Anchored forms
'/^name(?:$|-)/i' reject prefix-sharing classnames.

Tests: 3 new regression cases in components-manifest-6224.test.ts
(vitest 3/3). 36 existing tests still pass.
PerishCode CHANGES_REQUESTED on PR nexu-io#6250 — two scanner fixes:

1. Traverse supported at-rule bodies (nexu-io#6250 reviewer #1)
   stripContainerAtRuleHeaders rewrites @media/@supports/@container/
   @layer headers to '{', leaving an empty selector that swallowed the
   entire at-rule body as one rule and silently dropped every inner
   selector. iterateCssRules now recurses into the body slice on the
   empty-selector branch so inner rules surface with their real
   selectors and real bodies. extractCssSelectors reuses the same
   scanner so manifest.selectors matches manifest.groups[].selectors
   instead of falling back to the legacy regex that lost every
   selector immediately inside an at-rule.

2. Preserve nested-rule token references (nexu-io#6250 reviewer #2)
   flattenNestedBody previously tracked depth and only emitted
   declarations at depth === 0, which dropped inner-inner
   declarations two or more levels deep. The flatten now strips only
   the brace characters themselves and keeps every declaration body,
   so var(--token) references inside '& .child { & .grand { ... } }'
   attribute to the outermost ancestor instead of vanishing.

Tests: 2 new regression cases in components-manifest-6224.test.ts
covering at-rule body traversal (@media/@supports/@layer) and
multi-level nested token preservation (vitest 5/5). 251 contracts
package tests still pass. typecheck clean.
…ng nesting

flattenNestedBody now tracks an inQuote state ('"'/"'/null) plus
backslash escapes so that braces inside a quoted value such as
`content: "}"` are preserved as data instead of being treated as
rule terminators. This addresses reviewer feedback on PR nexu-io#6250
(round 3): without the state, the brace scanner could mistreat the
character after `content: "}` as the closing delimiter of an outer
rule, slicing the flattened body at the wrong position.

Coverage: add a regression case asserting that a rule containing
both a brace in a quoted content value and a real nested block
still resolves its outer selector's token references correctly.
PerishCode CHANGES_REQUESTED on PR nexu-io#6250 (round 4):
  "Make the opening-brace search quote-aware too. The new quote state
   begins only after  has already selected an
   opener, so a valid selector such as  treats the brace inside the attribute value as the rule
   boundary. ...  is , and the Buttons group has
   no selectors or token references, so both rules are lost."

The previous opener lookup used , which finds
the first  byte with no lexical context. A quoted  inside a
selector — e.g.  — was therefore selected as
the rule opener, leaving the closing  of the same attribute value as
the body's first  and  as a corrupt
selectorList. The body scanner then ran with mismatched braces and
emitted zero rules.

Fix: replace the  lookup with a quote-aware scan that mirrors
the closing-brace scanner's lexical handling. The scan walks from
, tracking single / double quote state plus backslash escapes,
and skips  /  that appear inside a quoted string (also skipping
 comments defensively). The first un-quoted  found is the
real rule-body opener.

Coverage: add a regression suite at
packages/contracts/tests/components-manifest-6250-quoted-selector.test.ts
covering both double- and single-quoted attribute selectors that
contain a . Each case pairs the quoted-brace selector with a
trailing flat rule and asserts both selectors plus their --a / --b
attribution survive on the buttons group.

Local verification:
- pnpm --filter @open-design/contracts vitest run
  tests/components-manifest-6250-quoted-selector.test.ts -> 2/2 pass
- pnpm --filter @open-design/contracts test -> 254/254 pass
- pnpm --filter @open-design/contracts typecheck -> clean
…trings

PerishCode round-5 review on head e7feb4f — escaped identifier characters
in selectors and declaration values were still interpreted as CSS structure,
corrupting selector and token attribution. A backslash in a selector
identifier (\:, \-, \2digit) escapes the next character so it is not a
structural brace or quote. The opener scan and the body depth counter now
skip a \X pair so:
- .\foo\:bar { ... } is not split at the escaped colon and the real { is
  found correctly;
- content: "\}"; is not mistaken for the rule terminator;
- a stray \{ or \} inside a declaration value no longer perturbs depth.

Adds two regression tests under
packages/contracts/tests/components-manifest-6250-css-escapes.test.ts
covering escaped idents in a class selector and escaped braces inside a
quoted declaration value. Full contracts suite (256 tests) and contracts
typecheck pass.
…ndary cases

PerishCode 7-31 09:10 inline review指出'verification gap':之前两个
escape regression fixture看似锁住了 outside-quote backslash escape 路径
(iterateCssRules 第 389 行 opener scan + 第 465 行 body scan),实际上
两个 fixture 都没走到那条分支:

* 第一个测试用 ': '(.btn-foo\:bar) — ':' 永远不可能被当成 rule
  opener,所以 opener scan 的 escape 处理有没有都正确。
* 第二个测试用 'content: "}"' — 转义 '}' 在 CSS string literal 里面,
  body scan 的 quote-state 分支已经先把它捕获了,根本不会走到
  'char === backslash' 分支。

测试名/注释声称锁的结构修复路径,实际上是空的。PerishCode 的原话:
'Neither test would fail if the outside-quote escape handling at
iterateCssRules lines 389 and 465 were removed. That leaves the final
commit\u2019s structural-delimiter fix unprotected despite the test names
and comments claiming otherwise.'

替换为 PerishCode 建议的两个真实 boundary case:

1. '.btn-a\{literal { color: var(--a); }' + 紧跟 '.btn-b { ... }'
   - escaped '{' 在 selector identifier 外面(不在 quotes 里)
   - 没有 escape 处理时 opener scan 会把 '{literal' 当成 rule body
     起点,留下的 selector 是残的 '.btn-a\',后面 '.btn-b' 也会因为
     rule body 边界错乱被吃掉或拼接
   - 加上 escape 处理后正确读出两个独立 rule 和两个 selector

2. '.btn-escape::after { content: \}; color: var(--a); }' +
   '.btn-plain { ... }'
   - '\}' 在 declaration value 外面 (不在 quotes 里),参数是
     identifier-escape 形式
   - 没有 escape 处理时 body scan 会按未转义的 '}' 算 depth=0,提前
     close rule,剩下 'color: var(--a); } .btn-plain' 被拼接成一个
     虚假 selector
   - 加上 escape 处理后 '}' 当成 data skip 过去,rule 借真正
     terminator 收尾,两个 selector 都正确出现

断言两个 selector + 两个 token reference (--a/--b) 全部 survive。

验证 regression 真的锁住fix path(不是又一次空 fixture):临时把 line 389
+ line 465 的 'if (ch === backslash)' 改成 'if (false && ...)' 模拟
禁用 escape 处理,跑 vitest:

* Test 1 失败: '.btn-a\{literal' 没出现,parse 残缺
* Test 2 失败: '.btn-plain' selector 变成了
  'color: var(--a); } .btn-plain' 这种拼接的奇怪 selector

还原 fix 后 2/2 pass。fixture 现在确实锁住了两个 escape 分支。

256/256 contracts suite 全过, typecheck 通过。

Ref: PR nexu-io#6250 PerishCode 7-31 09:10 inline review
@xxiaoxiong
xxiaoxiong force-pushed the fix/6224-manifest-token-refs-and-anchored-matchers branch from e9116af to 9725e93 Compare July 31, 2026 15:58
@xxiaoxiong

Copy link
Copy Markdown
Contributor Author

PerishCode 7-31 09:10 inline review收到 — 你说的'verification gap'完全对。

之前的 fixture 看起来在锁 escape path,实际是空的:

  • Test 1 ': '(.btn-foo:bar)':'永远不会被当成 rule opener,opener scan 的 escape 处理有没有都对
  • Test 2 'content: "}"':转义 '}' 在 string literal 里面,body scan 的 quote-state 分支先捕获了,根本不会走到 backslash 分支

测试名和注释声称锁的结构修复路径,实际上是死的。

新 commit 9725e93 换成两个真实 boundary case:

Test 1: selector 外面 escaped brace

.btn-a\{literal { color: var(--a); }
.btn-b { color: var(--b); }

没有 escape 处理时 opener scan 会把 '{literal' 当成 rule body 起点,留下残的 '.btn-a',并吃掉后面的 '.btn-b'。 escape 加上后两个 selector 都正确出现。

Test 2: declaration value 外面 escaped closing brace

.btn-escape::after { content: \}; color: var(--a); }
.btn-plain { color: var(--b); }

没有 escape 处理时 body scan 把未转义的 '}' 当 depth=0 提前 close,剩下 'color: var(--a); } .btn-plain' 被拼成虚假 selector。 escape 加上后正确读到两个 selector + 两个 token reference。

Regression validation: 临时把 iterateCssRules line 389 + 465 的 if (ch === backslash) 改成 if (false && ...) 模拟禁用 escape:

  • Test 1 fail: '.btn-a{literal' 没出现,parse 残缺
  • Test 2 fail: '.btn-plain' selector 变成 'color: var(--a); } .btn-plain' 这种拼接

还原 fix 后 2/2 pass。fixture 现在真的锁住两个 escape 分支了。

Contracts suite 256/256 全过,typecheck 通过。

PerishCode 这次 review 把 verification gap 抓出来是真的救命 — 之前那种 fixture 看起来在测试,实际只在跑 happy path 的子集,以后再有人改 escape 处理逻辑根本测不出来。 @nexu-io/maintainers

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The scanner and escape follow-ups now pass the full contracts suite and typecheck, and the previously reported parser failures are covered. One end-to-end grouping failure remains in the anchored-matcher portion: the exact prefix-sharing selector named by the PR still enters the Buttons group through the selector matcher.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

label: 'Buttons and calls to action',
selectorMatchers: [/\bbutton\b/i, /\.btn(?:\b|[-_:])/i, /\[type=["']?(?:button|submit|reset)/i],
classMatchers: [/^btn(?:$|-)/i, /button/i, /cta/i],
classMatchers: [/^btn(?:$|-)/i, /^button(?:$|-)/i, /^cta(?:$|-)/i],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Close the selector-matcher path for prefix-sharing classnames as well. Anchoring this classMatchers entry removes navbar-button-thing from Buttons.classes, but buildGroup independently filters CSS selectors through the unchanged /\bbutton\b/i matcher immediately above; hyphens are word boundaries, so .navbar-button-thing { color: var(--tone) } still appears in Buttons.selectors and contributes --tone to Buttons.tokenReferences. I reproduced that through extractComponentsManifest on this head, which means the manifest still crosses groups for the exact example the PR says is fixed. Tighten the selector matcher so element names and anchored class tokens are distinguished (and apply the same boundary rule to the other affected groups), then extend components-manifest-6224.test.ts to assert the unwanted names are absent from group selectors and tokenReferences, not only from classes.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-validation Runtime change detected; needs human or /explore agent validation. risk/medium Medium risk: regular code changes size/M PR changes 100-300 lines type/bugfix Bug fix

Projects

None yet

3 participants