fix: handle interpolated empty rules and document Helix setupUpdate

[usercao]

Hi @quantizor, when you have some time, could you please review the changes below? Thanks!

Summary

• Add an empty merge commit that records the upstream repository's history, keeping this fork aligned without applying obsolete upstream changes.
• Suppress emptyRules diagnostics when a styled rule body contains a template interpolation that may produce declarations at runtime.
• Preserve diagnostics for genuinely empty rules, including rules with interpolations only in their selectors, and leave diagnostics from custom virtual-document providers unchanged.
• Cover nested and unclosed rules, multiline and Unicode-containing interpolations, CRLF input, real tsserver behavior, and scaling characteristics.
• Document editor-global and project-local Helix setup through typescript-language-server, including the required plugin location and host requirements.
• Keep the scaling guardrail reliable on coarse CPU clocks by selecting the minimum finite positive timing sample instead of treating a zero-length sample as the best result.

Motivation

Interpolated mixins can populate a styled rule at runtime, so reporting those rules as empty is a false positive. The plugin should remain silent when it cannot prove that a rule is empty while continuing to report rules that are demonstrably empty.

Helix users also need a complete setup path that does not require adding the plugin to every project. The new documentation explains both global registration and the existing project-local alternative.

Testing

• Added focused unit coverage for diagnostic filtering and source-position edge cases.
• Added an end-to-end tsserver scenario with a genuinely empty rule as a positive control.
• Added a scaling check for templates containing many interpolated and genuinely empty rules.
• Added deterministic unit coverage for zero, non-finite, and positive CPU timing samples.
• Ran corepack yarn verify, including formatting, lint, type checking, unit and end-to-end tests, scaling checks, package API checks, and packed-content validation.

Closes #4.
Closes #12.

[changeset-bot]

🦋 Changeset detected

Latest commit: 8570f92

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@styled/typescript-styled-plugin	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Copilot]

Copilot review overview

🔵 Needs a closer look

The diagnostic filter must distinguish code interpolations from comment-only interpolations.

Review effort: Lite
Findings: None

What changed in this PR

This PR makes emptyRules diagnostics interpolation-aware, adds coverage, and documents Helix setup and host requirements.

Changes:

• Suppresses eligible diagnostics for interpolated rule bodies.
• Adds unit, end-to-end, and scaling tests.
• Documents global and project-local Helix integration.
• Adds a patch changeset.

Review finding: comment-only interpolations can incorrectly suppress emptyRules; CSS-aware filtering and regression coverage are needed.

File	Description
test/​unit/​template-language-service.test.ts	Adds diagnostic edge-case tests.
test/​performance/​template-language-service-fixture.ts	Supports configured performance fixtures.
test/​performance/​scaling-check.ts	Adds interpolation scaling coverage.
test/​e2e/​scenarios/​plugin-lifecycle.test.ts	Verifies real tsserver behavior.
src/​features/​diagnostics.ts	Filters interpolated empty-rule diagnostics.
src/​features/​css-diagnostic-code.ts	Defines the empty-rule diagnostic code.
README.md	Documents Helix setup.
docs/​usage.md	References Helix integration.
docs/​tsserver-host.md	Documents language-server host requirements.
docs/​architecture.md	Documents diagnostic filtering behavior.
.changeset/​quiet-rules-interpolate.md	Records the patch release change.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[pullfrog]

Important

The emptyRules fix and its tests look right — I reverted src/features/ to the pre-fix commit and exactly the three new unit cases fail, and the new scaling check is linear at 3.35. The two README problems below are in the new Helix section: following the snippet as written silently turns off Helix's TypeScript inlay hints, and the location rule is stated backwards.

Reviewed changes

• Empty-rule filter — DiagnosticsFeature.validate drops an emptyRules diagnostic when the rule's own body holds a template interpolation, finding rule bodies with the existing boundary scanner and deriving the interpolation spans from the template node; a genuinely empty body, an interpolation that appears only in a selector, and a third-party VirtualDocumentProvider's diagnostics are all left as they were.
• Lint-rule id — EMPTY_RULESET_DIAGNOSTIC_CODE in src/features/css-diagnostic-code.ts, internal only, not on the ./api surface.
• Spec — a new "Empty rules" bullet in docs/architecture.md records the exemption, the body-scan rules (comments, strings, unquoted url(...), unclosed bodies), and the custom-provider carve-out.
• Tests — four unit cases (bodies with and without interpolations, CRLF and U+2028/U+2029 expressions, nested and unclosed rules, a custom provider), one tsserver e2e scenario with a genuinely empty rule as its positive control, and a test:scaling check for a template of many interpolated and many empty rules.
• Helix documentation — a README section for editor-global registration through typescript-language-server, a pinned Node floor for that server in docs/tsserver-host.md, and Helix added to the editor list in docs/usage.md.
• Changeset — .changeset/quiet-rules-interpolate.md, a patch whose sentence says what changed and for whom.

I also drove the real service over 29 hand-built shapes (keyframes frames, @media and @layer preludes, parent-selector blocks, &-joined and escaped selectors, interpolations in comments and strings, \} and \7b, url(x{y), unclosed bodies). Every outcome is either correct or errs silent, which is the direction AGENTS.md asks for.

ℹ️ The user-facing lint table does not mention the exemption

docs/usage.md's lint table still describes emptyRules as plain "Empty rulesets." A user who turns that rule on will now find a rule they deliberately emptied with a mixin is not reported, and nothing in the user-facing guide says so. The fix belongs in that table's cell or in one sentence under it; docs/architecture.md is the home to link, not to restate.

ℹ️ Three kinds of change, and a merge commit that only changes the DAG

The behavior fix, the Helix documentation, and 6a0049d are three independent things, and AGENTS.md asks for one kind per PR. 6a0049d is an empty merge whose only effect is making twelve microsoft/vscode-css-languageservice commits (dependabot bumps, 0.18.3 tag, a deprecation note) ancestors of main. Nothing in the repo's tooling breaks — scripts/changelog.cjs scopes attribution to the changeset file's path — but git log on main, the contributors graph, and any future git bisect all traverse that unrelated history from here on. Worth confirming that is the trade you want in main, and splitting the fix and the docs either way.

ℹ️ Nitpicks

• README:195 says "Version 6 of that language server"; docs/tsserver-host.md:29 pins 6.0.1, which the rest of the section's style would match, and a bare major will rot.
• The snippet needs [language-server.<name>].config, which arrived around Helix 23.05 and settled in 23.10; on 22.03 through 22.12 the equivalent lives in config.toml under [editor.lsp.typescript-language-server]. Every other editor section in this file names a host floor, and this one names none.
• Every other editor section ends by saying the path "requires validation against the actual editor's tsserver host and Node runtime". The Helix section makes verified-looking claims (the Node floor, hx --health) without that sentence, while making no claim about what it was actually run against.

_{Verified locally: test:unit (932 pass), test:e2e (160 pass, 12 files), test:scaling (all checks ok, new check 3.35), format:check, lint, typecheck. Reverting src/features/diagnostics.ts and src/features/css-diagnostic-code.ts to 6a0049d fails exactly the three new unit cases that name the behavior; the custom-provider case is a guard that cannot fail before the fix.}

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Space Bunny (free) ｜ 𝕏}

[pullfrog]

Important

The diagnosis in the new comment is right, and I reproduced both halves of it: the override does fix the case it names, and the class it names is still live on this commit. corepack yarn test:scaling fails at ef69124 with FAIL substitution (statements each opening an unterminated comment): N=6000 0.000ms, ratio=Infinity from the same coarse-tick cause. A per-case n bump is the wrong layer for that.

Reviewed changes

• Raised the N sample of the placeholders inside one block comment substitution check to 48,000, adding an optional per-case n to TextCheckCase and threading it through defineTextCheck for the substitution and escape case lists, to stop Math.min picking a zero-length N sample and reporting a phantom superlinear regression.

Technical details

# The zero N sample is a class, not one case

## Affected sites
- `test/performance/scaling-check.ts:441` — the per-case `n: 48_000` override this commit adds
- `test/performance/scaling-check.ts:1058` — `timing.bestMs = Math.min(timing.bestMs, measurement.elapsedMs)` in `timeInterleaved`; one zero sample sets a size's minimum to 0
- `test/performance/scaling-check.ts:1096` — `const ratio = at4N.bestMs / atN.bestMs`; a zero N side makes the ratio `Infinity`, reported through `isSuperlinear` as "grew superlinearly"
- `test/performance/scaling-check.ts:609` — `n = SUBSTITUTION_CHECK_N`, the shared default this commit now lets one case override
- `test/performance/scaling-check.ts:878` — the `escapeCases` map now destructures and forwards `n`, which no escape case sets

## What the evidence shows
`elapsedCpuMs` (line 167) divides `process.threadCpuUsage()` microseconds by 1000, so every reading is quantized to the kernel's `CLOCK_THREAD_CPUTIME_ID` tick. Sampling the two at-risk fixtures directly, 25 runs each, using the same `repeatUnit` build the checks use:

| case | N | min | median | samples reading 0.0000 ms |
|---|---|---|---|---|
| `placeholders inside one block comment` | 6,000 | 0.0000 | 1.0000 | 3 / 25 |
| `placeholders inside one block comment` | 48,000 | 7.9720 | 8.0450 | 0 / 25 |
| `statements each opening an unterminated comment` | 6,000 | 0.0000 | 1.0000 | 2 / 25 |
| `statements each opening an unterminated comment` | 24,000 | 3.9770 | 4.9990 | 0 / 25 |

A `0.0000 ms` reading is a one-tick sample whose accounted CPU time had not ticked yet, not a free run: the same fixture's 4N sample reads 3.98 to 4.00 ms. The override is the right answer for the case it names, and the `20d2ca2` CI log confirms the failure it removes: `FAIL substitution (placeholders inside one block comment): N=6000 0.000ms, 4N=24000 3.999ms, ratio=Infinity`.

The class is unfixed. On this head commit `corepack yarn test:scaling` fails:

```
FAIL substitution (statements each opening an unterminated comment): N=6000 0.000ms, 4N=24000 4.277ms,
ratio=Infinity (threshold 7, floor 1.5), stopped after 4 of 5 runs per size above 14, outside the range on all 3 attempts
```

A filtered rerun of that one check failed once in three runs. `JavaScript escape replacement (escape runs inside one name)` reads 0.998 ms at N=6,000, one tick from the same cliff, and `adjacent placeholders joined into one property name` plus the two padded-escape cases read two. This is pre-existing rather than introduced here, but it is the same defect this commit sets out to stabilize, so AGENTS.md's "search for every instance and fix all of them in the same change, or list the rest" applies.

## Required outcome
- No size reaches `timeInterleaved`'s minimum as 0 while a nonzero sample of that size exists, so a coarse tick can never be reported as a superlinear regression.

## Suggested approach
- In `timeInterleaved`, exclude a zero reading from a size's running minimum once that size has a nonzero reading. If every sample of a size reads 0, keep the 0, so a genuinely dead probe behaves exactly as it does today rather than gaining new handling here.
- That removes the need for `n: 48_000`, for `TextCheckCase.n`, for the `n` parameter on `defineTextCheck`, and for the `n` destructuring in the `escapeCases` map, and returns `substitution (placeholders inside one block comment)` to `SUBSTITUTION_CHECK_N`'s 0.5 s instead of 0.9 s.
- If the per-case override is kept instead, its comment should record the measurement that picked 48,000, namely the N sample's tick count and the margin over the coarsest tick worth supporting. `SUBSTITUTION_CHECK_N` at line 594 documents its own derivation the same way.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Space Bunny (free) ｜ 𝕏}

[pullfrog]

ℹ️ No blocking issues in the docs commit — one filing nit inline.

Reviewed changes

Everything since the last pullfrog review (ef69124) is documentation across four files.

• Helix snippet — rewrote the languages.toml block under [language-server.typescript-language-server.config], dropped the redundant command/args, and added hostInfo plus the seven TypeScript and seven JavaScript inlay-hint keys, so copying it no longer wipes Helix's built-in initialization options.
• location guidance — replaced the backwards "folder containing the scoped @styled package" rule with a pointer at the exact npm root -g output, and named the versions the setup was verified against (Helix 25.07.1, typescript-language-server 6.0.1).
• Escape clause — narrowed the new "Empty rules" bullet in docs/architecture.md from "escapes" to "CSS escapes" and recorded that a JavaScript escape is cooked into a real brace before the body scan.
• Host notes — added a Helix bullet to docs/tsserver-host.md, replaced the literal U+2028/U+2029 in the computeLineStarts example with \u2028/\u2029 escapes, refreshed the file's date, and pinned the language server's Node floor to the release it came from.
• User-facing lint table — docs/usage.md now describes emptyRules as provably empty rulesets with body interpolations exempt, and lists Helix among the setup paths.

I checked the load-bearing claims against primary sources rather than taking them on faith: Helix 25.07.1's languages.toml carries byte-identically to master and holds exactly the 15 leaf keys the snippet now repeats; merge_toml_values(a, b, 3) does hand back the right-hand config table whole at depth 0, so command and args genuinely survive and the "keep the complete table" instruction is right; typescript-language-server 6.0.1 declares engines.node >= 22.22.2 and does push each plugin location through --pluginProbeLocations; and docs/configuration.md for that release states both accepted location forms. corepack yarn format:check and corepack yarn test:scaling are clean at this head.

The docs/architecture.md thread stays open for the sibling clause at line 108, which the commit did not narrow, and the scaling-guardrail thread from the last review is untouched by a docs-only commit.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Space Bunny (free) ｜ 𝕏}

[pullfrog]

ℹ️ All three open threads are closed by this commit. Two leftovers, one inline and one here.

Reviewed changes

Everything since the prior pullfrog review (51e7e47) is one commit, 40fc6f6, across five files, and it lands the three open threads.

• Zero-CPU-sample fold — new selectMinimumPositiveSample in test/performance/scaling-check-cli.ts replaces the Math.min in timeInterleaved (test/performance/scaling-check.ts:1052), so a size whose samples include a 0.0000 ms reading keeps its smallest positive reading instead of collapsing to zero and reporting an Infinity ratio as a superlinear regression. A size where every sample is zero stays zero, and hasUnmeasurableTiming (test/performance/scaling-check.ts:1094) turns that into a distinct probe-broken outcome with its own retry message rather than over-limit.
• Override removed — TextCheckCase.n, the per-case n: 48_000, and the n destructuring in the escapeCases map all go, so the fix lands once in the fold rather than once per case. I grepped the repo for the leftovers: nothing still names 48_000 or the removed field. corepack yarn test:scaling at this head is 36 checks ok and 0 FAIL, with placeholders inside one block comment back at N=6000 0.999ms, ratio=5.00 in 0.5s wall and statements each opening an unterminated comment at N=6000 0.997ms, ratio=5.70, the two cases that were one tick from the cliff. That is a coarse-clock Linux run, which is exactly the class the fix targets.
• Early-exit guard — timeInterleaved now requires atN.bestMs > 0 before comparing against EARLY_EXIT_RATIO (test/performance/scaling-check.ts:1055), so an unmeasured N side runs all RUNS_PER_SIZE samples and reports why, rather than stopping after EARLY_EXIT_MIN_RUNS and appending a meaningless stopped after 4 of 5 runs note to a reading that was never going to be meaningful.
• Spec sibling bullet — docs/architecture.md:108 now reads "read after JavaScript escapes are cooked, so a JavaScript escape can contribute a structural brace" and narrows the scanner clause to CSS escapes, matching the "Empty rules" bullet. Verified against findStrayClosingBraceOffset(getTemplateCssText(context)) at src/features/diagnostics.ts:122 and getTemplateCssText returning replaceEscapes(...) output at src/virtual-document/javascript-escapes.ts:53, so the claim is true.
• Helix host note relocated — the docs/tsserver-host.md:13 bullet moved up into How tsserver loads a plugin and now closes with "its accepted location forms resolve under the classic Node10 rules above" instead of restating the --pluginProbeLocations resolution that line 8 owns.
• Unit coverage — three describe cases in test/unit/scaling-check-cli.test.ts pin selectMinimumPositiveSample with exact values across the not-yet-measured, smallest-positive, and all-unusable states. These can fail: with the old Math.min, the fold returns 0 for a (0, 4) pair and the first case fails. Extracting the fold into a pure exported helper is what makes that assertable at all, since timeInterleaved reads real CPU time.

One note on the thread I just closed on this commit: the scaling-guardrail fix is complete and verified, so I resolved it, but the one item from its suggested approach this commit left behind is now a dead parameter at test/performance/scaling-check.ts:607 and is filed inline rather than left implicit.

ℹ️ The command's reference doc still describes the old sampling rule

docs/maintenance.md is the home for what yarn test:scaling does and how it fails, and this commit changed both without touching it. Line 147 still says the check takes "the minimum of several runs per size" where it now takes the smallest positive one, and the failure-mode paragraph that follows still lists only a ratio above the threshold or below the floor, where a size with no positive sample is now a third way a timed check fails. The same stale enumeration survives in the CheckOutcome JSDoc at test/performance/scaling-check.ts:142, which still defines probe-broken as only the two pre-existing causes. Only main()'s runtime message was updated for the new case.

Technical details

# The positive-sample fold and its failure mode are undocumented

## Affected sites
- `docs/maintenance.md:147` — "taking the minimum of several runs per size"
- `docs/maintenance.md:153-163` — the failure-mode paragraph: only the ratio above the threshold and the ratio below the floor
- `docs/maintenance.md:258` — "(`--filter` parsing, per-check progress tracking, both covered by `test/unit/scaling-check-cli.test.ts`)"
- `test/performance/scaling-check.ts:142-147` — the `CheckOutcome` JSDoc, which enumerates `probe-broken`'s causes and does not include the new one
- `test/performance/scaling-check.ts:53-59` — the JSDoc this doc paraphrases, which now adds "coarse-clock noise" to the reason for taking the minimum
- `test/performance/scaling-check.ts:1094-1097` — `hasUnmeasurableTiming`, the third failure mode
- `test/performance/scaling-check.ts:1440-1448` — `main()`'s `probe-broken` message, already updated for it

## Required outcome
- The Scaling guardrail section and the `CheckOutcome` JSDoc state the current sampling rule and all three ways a timed check can fail, so a contributor reading them does not have to re-derive the coarse-clock cause from a `0.000ms` reading in a CI log.

## Suggested approach
- `docs/maintenance.md:147`: the smallest *positive* sample of several runs per size, and why, namely that a coarse `CLOCK_THREAD_CPUTIME_ID` clock can report zero for a sub-tick sample. The same paragraph already explains the neighbouring `process.threadCpuUsage` noise source, so the sentence has company.
- Failure modes: a size whose every sample reads zero, reported as a broken probe rather than as a ratio, retried like the other two. `probe-broken` is already the name `main()` prints, so the doc can use it directly.
- `test/performance/scaling-check.ts:144`: add the unmeasured timed size to the `probe-broken` enumeration, or point at `hasUnmeasurableTiming` as the single place that decides it.
- `docs/maintenance.md:258`: three exports and three `describe` blocks now, so "both" is one word stale.

## Open questions for the human
- `docs/architecture.md` needs no change here: its only mention of the check (`Caching`, the validation-cache byte budget) does not describe sampling.

Also clean locally at this head: corepack yarn format:check (136 files), lint (0 warnings, 0 errors), typecheck, test:unit (935 pass, 16 files).

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Space Bunny (free) ｜ 𝕏}

[quantizor]

@usercao after merge a prerelease will be automatically cut for you to test :) thanks for your help!

[usercao]

@quantizor Thanks! Just to make sure I understand correctly, would you prefer that I Squash and merge this now, or should I wait for you to merge it? Either works for me.

[quantizor]

You are free to merge when ready. Looks like there's some potential extra cleanup you can do if you want.

[usercao]

Thanks for confirming! I’ll go through the additional cleanup and address it before merging. Once everything is ready and all the checks are passing, I’ll squash and merge it. Thanks again!

[pullfrog]

✅ No new issues found. This delta closes the last open thread, and the reworded JSDoc now matches what the filter actually does.

Reviewed changes

Everything since the prior pullfrog review is one commit, 8570f92, across two files.

• JSDoc corrected to match the filter. filterEmptyRulesWithInterpolations (src/features/diagnostics.ts:195) now says "a template interpolation" rather than "a block-position interpolation." The code never tested block position: containsInterpolation asks only whether an interpolation span overlaps the rule body, so a block-position test would be false for a { /* ${mixin} */ }. This is also the wording docs/architecture.md:103 and the docs/usage.md lint table already used, so it closes a comment-versus-spec divergence rather than opening one.
• Dead n parameter removed. defineTextCheck (test/performance/scaling-check.ts:603) no longer takes an n argument; n: SUBSTITUTION_CHECK_N sits directly in the defineCheck call. Both call sites (lines 868 and 873) already passed no fourth argument, so every substitution and escape check keeps the size it had. The restored shape is byte-identical to the pre-ef69124 version at 20d2ca2.

I drove the real service over 37 hand-built shapes to confirm the filter the comment now describes: an interpolation in a selector (${sel} {}), in a comment (a { /* ${mixin} */ }), and in a string (a { content: "${mixin}"; }) each suppress the diagnostic, while a {}, a { } ${mixin}, a { b {} ${mixin} }, and /* ${mixin} */ a {} keep it. Every outcome is either correct or errs silent, the direction AGENTS.md asks for.

Clean locally at this head: corepack yarn format:check (136 files), lint (0 warnings, 0 errors), typecheck, test:unit (935 pass, 16 files), and test:scaling (36 checks ok, 0 FAIL, with placeholders inside one block comment back at N=6000 0.977ms, ratio=4.09 on a coarse-clock host).

  ^{｜ View workflow run ｜ Using Space Bunny (free) ｜ 𝕏}