Repository navigation
test(rtl): route Charts package audits - #6252
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
cixzhang
requested review from
imdreamrunner and
josephfarina
as code owners
September 11, 2026 16:52
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
Contributor
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR No new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: 36 accessibility violation(s) found — 2 critical, 34 serious. Button pattern - 1 issue(s)
ChatComposer - 1 issue(s)
ChatComposerInput - 1 issue(s)
ChatReasoning - 1 issue(s)
ChatToolCalls - 1 issue(s)
Checkbox pattern - 1 issue(s)
CheckboxList - 1 issue(s)
ClickableCard - 1 issue(s)
CodeEditor - 1 issue(s)
CodeEditorPerf - 1 issue(s)
CodeEditorTheme - 1 issue(s)
CodeTheme - 1 issue(s)
DateRangeInput - 1 issue(s)
FileInput - 1 issue(s)
GridMasonry - 1 issue(s)
Heading - 1 issue(s)
Icon - 1 issue(s)
LogStream - 1 issue(s)
MediaTheme Auto - 1 issue(s)
PowerSearch - 1 issue(s)
ProgressBar - 1 issue(s)
Radio group pattern - 1 issue(s)
RadioList - 1 issue(s)
RichTextEditor - 2 issue(s)
SelectableCard - 1 issue(s)
Stepper - 1 issue(s)
TableGroupedRows - 1 issue(s)
TableTree - 1 issue(s)
Text - 1 issue(s)
Theme - 1 issue(s)
PopArt - 1 issue(s)
Thumbnail - 1 issue(s)
Timestamp - 1 issue(s)
Token - 1 issue(s)
Tokenizer - 1 issue(s)
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Component authors can still lose both browser audits when a registry change and component source change land together. The new Rich Text/Vega routes and the prior Charts fixes pass, but the routing owner needs one fail-closed correction; see the inline finding.
| fi | ||
| FILES=$(git diff --name-only origin/${{ github.base_ref }}...HEAD) | ||
| CHANGED=$(printf '%s\n' "$FILES" | grep -E '^packages/(core|lab)/src/' | grep -v -E '(hooks|theme|utils|i18n|__tests__)/' | head -1 || true) | ||
| COMPONENT_ROOTS=$(node -e 'const {COMPONENT_PACKAGES}=require("./scripts/component-packages.cjs"); process.stdout.write(COMPONENT_PACKAGES.map(pkg => pkg.src).join("|"))') |
There was a problem hiding this comment.
COMPONENT_ROOTS is read from the PR checkout. A change that removes or renames a root while modifying that package makes CHANGED empty: pr-a11y skips, and pr-rtl only runs the Chart/ChartLegend harness smoke instead of the changed component. Please load these roots from trusted base policy, or force full component scope whenever the registry changes, and add a mutation test for a registry-plus-source diff.
There was a problem hiding this comment.
Thanks, the latest routing patch closes the named target, route, classifier, and unknown-path gaps. CI can still report complete coverage when a selected accessibility story fails, and a verified-N/A policy change can still skip the owner it reclassifies. Please close both paths; see inlines.
| const component = (story.title || '').split('/').pop() || storyId; | ||
| const auditedStoryKey = storyKey(component, story.name || storyId); | ||
| storyKeyById.set(storyId, auditedStoryKey); | ||
| auditedStoryKeys.push(auditedStoryKey); |
There was a problem hiding this comment.
This key is recorded before the scan succeeds, so a timeout resolves a live baseline and exits green.
| /(^|\/)src\/.*\.a11y\./.test(file) || | ||
| file === 'playwright.config.ts'; | ||
| const isRtlHarness = file => | ||
| file.startsWith('apps/storybook/rtl-audit/') || |
There was a problem hiding this comment.
verified-not-applicable.json can reclassify any owner, but this path only runs Chart/ChartLegend; make it full-scope and test the mutation.
There was a problem hiding this comment.
Contributors still cannot rely on this gate: required test-node is red, and a story that renders after the fixed pause is rejected before axe. Please wait for rendered story content within a bounded timeout and make every new fail-closed branch behavior-tested.
| .waitFor({state: 'visible', timeout: 5000}); | ||
| const storyRoot = page.locator('#storybook-root'); | ||
| await storyRoot.waitFor({state: 'attached', timeout: 5000}); | ||
| const rendered = await storyRoot.evaluate( |
There was a problem hiding this comment.
A fixed pause can reject a valid slow render. Wait for story content within a bounded timeout.
| expect(throwError).toBeGreaterThan(catchStart); | ||
| expect(throwError).toBeLessThan(finallyStart); | ||
| expect(source).toContain("waitUntil: 'domcontentloaded'"); | ||
| expect(source).toContain("locator('body.sb-show-main')"); |
There was a problem hiding this comment.
These source checks miss three new guards; mutation tests still pass when they are removed.
cixzhang
force-pushed
the
fix/rtl-chart-package-routing-2026-09-11
branch
from
September 12, 2026 05:56
c6e77e6 to
3e6f34b
Compare
cixzhang
force-pushed
the
fix/rtl-chart-package-routing-2026-09-11
branch
from
September 12, 2026 06:17
3e6f34b to
6d55f31
Compare
cixzhang
force-pushed
the
fix/rtl-chart-package-routing-2026-09-11
branch
from
September 12, 2026 06:20
6d55f31 to
110b15d
Compare
korkt-kim
pushed a commit
to korkt-kim/astryx
that referenced
this pull request
Sep 13, 2026
* test(rtl): route Charts package audits * ci(rtl): exercise harness-only changes * fix(lab-readiness): follow RTL package roster * fix(rtl): route complete Charts roster * fix(rtl): project all component packages * fix(ci): trust base component audit scope * fix(a11y): route package-qualified owners * fix(ci): fail closed for unmatched audit scope * fix(a11y): scope baseline by audited stories * fix(ci): fail closed for route policy changes * fix(a11y): key baseline by owner and story * fix(ci): protect shared audit routing policy * fix(a11y): avoid network-idle scan timeouts * fix(ci): shard full RTL audit by package * fix(a11y): preserve missing-build CLI contract * fix(ci): close audit readiness races * fix(ci): fail closed on missing audit inputs * fix(ci): require completed RTL scans * fix(a11y): reject docs-only story indexes * fix(a11y): migrate legacy story baseline keys * fix(ci): close audit completion trust gaps
This branch was successfully deployed
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
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.
Problem
The landed Chart audit in #6247 exposed a shared RTL harness routing defect.
pr-rtlcorrectly passedChart,ChartLegend, but the runner only admittedcore-*andlab-*stories and only discovered source components in those two packages. The result measuredlab/Chart, excluded everycharts-*story, and reportedChartLegendas an unexplained coverage gap.This is a forward-fix to the reusable coverage contract established in #5364 / #5365. It does not change Chart runtime behavior or its audit score.
Changes
scripts/component-packages.cjsregistry.Lab/RichTextEditormaps to Rich Text, andVega/VegaChartmaps to Vega.Chart,ChartLegendsmoke scope instead of skipping their own check.analysis.json; pr-a11y and pr-rtl consume the same canonical owner-to-story routes, and pr-a11y fails when a selected owner resolves to zero stories.pr-rtljoin that rejects classifier failure before reading scope outputs.The open RTL evidence/design records in #6046 and #6047 remain draft and non-authoritative. This PR only repairs executable routing and evidence. #5858 changes representative-story applicability on the same harness but does not own package routing.
Regression proof
Before the Review Loop fixes, exact-head probes showed four broken seams:
packages/charts/src/Chart.tsxwas outsidecheck-components, so a source-only Charts change skippedpr-rtl.--filter ChartAxis,ChartGrid,ChartSwatch,ChartTooltipran 0 automatic stories and returned 4 unexplained gaps.lab/RichTextEditorinstead of their canonical package.After the fix:
Card,RichTextEditor, andVegaChartto Core, Rich Text, and Vega from the canonical registry.VegaChart.tsxresolves toforce_full_component_audits=true; both browser audits omit their filters and inspect the full Storybook.true; no unmatched input can grant a browser-audit skip.charts/MissingOwnerscope exits 1 before launching a browser instead of returning a zero-story success.RichTextEditor::With Toolbarresolved; the other 12 Rich Text baseline entries remain unchecked, and baseline regeneration preserves them.--filter Chart,ChartLegend:lab/Chart: D2 pass, measured;charts/Chart: D2 pass, measured;charts/ChartLegend: D2 pass, measured;--filter ChartAxis,ChartGrid,ChartSwatch,ChartTooltip:richtext/RichTextEditorandrichtext/RichTextEditorToolbar: curated pass, measured;vega/VegaChart: verified N-A after owned stories ran where available;Test plan
Follow-up checks and trusted Review Loop outcome will be recorded on this exact head before merge.