Repository navigation
fix(rtl-audit): resolve component filters through the Storybook index - #9319
Conversation
A touched-components run for a change inside packages/core/src/Resizable/ filtered the RTL audit to core/ResizeHandle, the folder's public export, but the folder's stories are titled Core/Resizable, so no route matched, the audit exited FATAL with no report, and the step failed. A filter that no route names now selects the stories whose componentPath lies in its source folder, and the report records the alias (scope.filterAliases) so the touched check reads ResizeHandle's result from Core/Resizable's stories. A filter with no story at all is recorded in scope.noStories with a visible note; when every filter has none, the audit writes that report and exits 0. The touched check shows such an owner as not run (no stories found), which the comment renders as —.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Package | Where | Signature | Usage | |
|---|---|---|---|---|
| + | core |
. · Types · interface FieldStatusProps |
message: string |
import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {message: …} |
| + | core |
. · Types · interface FieldStatusProps |
ref?: React.Ref<HTMLDivElement> |
import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {ref: …} |
| + | core |
. · Types · interface FieldStatusProps |
type: InputStatusType |
import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {type: …} |
| + | core |
. · Types · interface FieldStatusProps |
variant?: FieldStatusVariant = 'attached' |
import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {variant: …} |
| + | core |
. · Types · interface FieldStatusProps |
xstyle?: StyleXStyles |
import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {xstyle: …} |
| + | core |
. · Types · interface FieldStatusProps |
inherits AriaAttributes, DOMAttributes, HTMLAttributes |
import type {FieldStatusProps} from '@astryxdesign/core' |
| + | core |
. · Types · interface SearchContext |
interface SearchContext<T extends SearchableItem = SearchableItem> |
import type {SearchContext} from '@astryxdesign/core' |
| + | core |
./Typeahead · Also exported from an earlier entry point · SearchContext |
SearchContext (.) |
import {SearchContext} from '@astryxdesign/core/Typeahead' |
| ~ | core |
. · Components · component ComplexSelector |
prop statusVariant?: FieldStatusVariant = 'attached' for input selectors; 'detached' for secondary and ghost selectors → prop statusVariant?: FieldStatusPlacement = 'attached' for input selectors; 'detached' for secondary and ghost selectors |
<ComplexSelector statusVariant="attached" /> |
| ~ | core |
. · Components · component DateInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<DateInput statusVariant="attached" /> |
| ~ | core |
. · Components · component DateRangeInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<DateRangeInput statusVariant="attached" /> |
| ~ | core |
. · Components · component Field |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<Field statusVariant="attached" /> |
| ~ | core |
. · Components · component FieldStatus |
component FieldStatus(props: FieldStatusProps) → component FieldStatus @deprecated |
<FieldStatus /> |
| ~ | core |
. · Components · component FieldStatus |
prop variant?: FieldStatusVariant = 'attached' → prop variant: FieldStatusVariant = 'attached' |
<FieldStatus variant="attached" /> |
| ~ | core |
. · Components · component FileInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<FileInput statusVariant="attached" /> |
| ~ | core |
. · Components · component MultiSelector |
prop statusVariant?: FieldStatusVariant = 'attached' for input selectors; 'detached' for secondary and ghost selectors → prop statusVariant?: FieldStatusPlacement = 'attached' for input selectors; 'detached' for secondary and ghost selectors |
<MultiSelector statusVariant="attached" /> |
| ~ | core |
. · Components · component NumberInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<NumberInput statusVariant="attached" /> |
| ~ | core |
. · Components · component PowerSearch |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<PowerSearch statusVariant="attached" /> |
| ~ | core |
. · Components · component Selector |
prop statusVariant?: FieldStatusVariant = 'attached' for input selectors; 'detached' for secondary/ghost selectors → prop statusVariant?: FieldStatusPlacement = 'attached' for input selectors; 'detached' for secondary/ghost selectors |
<Selector statusVariant="attached" /> |
| ~ | core |
. · Components · component TextArea |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<TextArea statusVariant="attached" /> |
| ~ | core |
. · Components · component TextInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<TextInput statusVariant="attached" /> |
| ~ | core |
. · Components · component TimeInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<TimeInput statusVariant="attached" /> |
| ~ | core |
. · Components · component Tokenizer |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<Tokenizer statusVariant="attached" /> |
| ~ | core |
. · Components · component Typeahead |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<Typeahead statusVariant="attached" /> |
| ~ | core |
. · Types · interface FieldStatusProps |
interface FieldStatusProps extends BaseProps<HTMLDivElement> (props of FieldStatus) → interface FieldStatusProps extends BaseProps<HTMLDivElement> |
import type {FieldStatusProps} from '@astryxdesign/core' |
| ~ | core |
. · Types · interface FieldStatusVariantMap |
tooltip: true → tooltip: true @deprecated |
import type {FieldStatusVariantMap} from '@astryxdesign/core'; const x: FieldStatusVariantMap = {tooltip: …} |
| ~ | core |
. · Types · interface SearchSource |
search: (query: string) => Promise<T[]> | T[] → search: (query: string, context?: SearchContext<T>) => Promise<T[]> | T[] |
import type {SearchSource} from '@astryxdesign/core'; const x: SearchSource = {search: …} |
| ~ | core |
. · Types · interface UseInputStatusIconOptions |
statusVariant?: FieldStatusVariant → statusVariant?: FieldStatusPlacement |
import type {UseInputStatusIconOptions} from '@astryxdesign/core'; const x: UseInputStatusIconOptions = {statusVariant: …} |
| ~ | richtext |
. · Components · component RichTextEditor |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusVariant | 'tooltip' = 'attached' |
<RichTextEditor statusVariant="tooltip" /> |
Not run on this PR: add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.
CI impact: 0 job-min per update · pool 0% · standard runners · 1 CI action or script file changed (not measured)
| Before | After | Change | |
|---|---|---|---|
| Job-minutes per PR update | 78.7 | 78.7 | 0 |
| Runner pool, runner-minutes a day | 36,690 | 36,690 | 0 (0%) |
| Average concurrent jobs | 25.5 | 25.5 | 0 |
| Time to green (median → this run) | — | 16.6 min | — |
| Runs a day | 466 | 466 | 0 |
Not measured (the jobs that use these keep their recent minutes above): .github/scripts/rtl-audit-coverage.test.mjs.
Assumptions: before is each job's average over the last 20 pull-request runs; after keeps that for unchanged jobs and uses this run's minutes for added or changed ones, weighted by how often each changed job ran in those runs; runs a day averages the seven whole days before today; time to green compares this run with the median of those runs that ran the same jobs.
Generated by PR Enrichment workflow | 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.
Changes requested: resolving core/ResizeHandle through Core/Resizable's stories fixes the crash, but the new no-stories path reports an in-scope component without stories as — instead of the coverage gap that architecture:rtl-audit-evidence INV5 requires (HStack, VStack, StackItem and LayerProvider now read "no stories found" even though 53 to 494 audited stories use them).
- Keep the crash fix and keep storyless owners in the coverage roster so they report a coverage gap; if storyless components should count as N/A instead, an owner needs to amend INV5 first.
- Resolve the owner's source folder with
componentExportsFromBarrelfromscripts/component-packages.cjsinstead of a new<Name>.tsxdirectory scan, which already missesContextMenuItem,ChatComposerTokenElementand every Charts, Rich Text and Vega component. - Keep each aliased owner's own roster entry, since
ChatLayoutScrollButtoncurrently takes the combined verdict of 16 Chat components and loses its verified-N/A entry.
[Automated review]
| ) | ||
| : []; | ||
| if (mine.length === 0) { | ||
| noStories.push(filter); |
There was a problem hiding this comment.
A storyless owner leaves rosterFilters here, so it never reaches buildComponentCoverage. architecture:rtl-audit-evidence INV5 says "Missing renderability is itself a coverage gap when the component is in scope". Keeping the filter in the roster lets the coverage step report the gap (or a verified-N/A declaration) instead of —.
| * A filter's source folder: the package folder holding `<Name>.tsx`, for a | ||
| * package-qualified filter (`core/resizehandle` → core, Resizable). | ||
| */ | ||
| function sourceFolderOf(filter) { |
There was a problem hiding this comment.
scripts/component-packages.cjs already resolves each public component to its implementing file with componentExportsFromBarrel, and changedFolderMembers uses it for this exact lookup. This filename scan finds no folder for ContextMenuItem, ChatComposerTokenElement or any Charts, Rich Text or Vega component. Taking the folder from the registry entry removes the second resolver.
| const noStories = reports.some(report => | ||
| (report?.scope?.noStories ?? []).includes(filterKey), | ||
| ); | ||
| const names = value => |
There was a problem hiding this comment.
Every alias counts as this owner, so core/ChatLayoutScrollButton takes the results of 16 Chat components (100 stories). Because the owner has also left the roster, its own verified-N/A entry no longer applies. Keeping the owner in the roster and naming the alias behind a failure would keep the cell honest.
| ) { | ||
| return {result: 'fail', detail: `RTL ${coverage.status}`}; | ||
| } | ||
| if (mine.length === 0 && noStories) { |
There was a problem hiding this comment.
This returns not-run / "no stories found" for an in-scope owner with no story. INV5 treats that as a coverage gap (a failure). For HStack, VStack, StackItem and LayerProvider the label is also untrue: 53 to 494 audited stories use them.
…; keep storyless owners as coverage gaps Review: a component with no story must still read as a coverage gap (architecture:rtl-audit-evidence INV5), not N/A; a Chat member must not take its siblings' verdict; and the owner's folder comes from its package barrel. A filter no route names now adopts only its folder's orphan stories (index componentPath in the folder, title naming no public component) and routes them to the filter's own component, so it keeps its own coverage entry and verified-N/A record. The folder comes from componentExportsFromBarrel. A filtered run with no story writes a coverage-only report (each component a gap or its verified N/A) instead of the fatal error. The touched check needs no change: results now carry the owner's own name.
There was a problem hiding this comment.
Changes requested: resolving filters through the Storybook index removes the crash, but core/ResizeHandle still reads an RTL coverage gap, because the D4 check of its own pill (declared under Resizable in targets.json) never joins its adopted plan, even though that check passes.
- Route curated targets on adopted stories to the owner (or name
ResizeHandleas that target's component), and say in the description what a Resizable-only pull request now shows. ChatLayoutScrollButtonadopts all tenCore/Chatstories, which never render it, so adopt only stories that render the owner and replace thecore-chat--layouttest fixture, which the real index does not contain.
[Automated review]
| ownerOf: filter => sourceOwnerOf(filter, publicComponentsByPackage), | ||
| }); | ||
| const scopedStoryRoutes = resolvedFilters.routes; | ||
| for (const [component, storyIds] of Object.entries(resolvedFilters.adopted)) { |
There was a problem hiding this comment.
The adopted owner gets these stories, but the curated loop below (matchesFilter(component) at line 1390) still selects targets by filter name, so the D4 target on core-resizable--horizontal-split (component Resizable, overlay .astryx-resize-handle) never joins ResizeHandle's plan. On this head --filter core/resizehandle ends in an RTL coverage gap while that D4 card passes under --filter core/resizable. Admitting curated targets on adopted story ids under the owner's name would fix it.
| if (direct.length > 0) continue; | ||
| const owner = ownerOf(filter); | ||
| if (owner == null) continue; | ||
| const orphans = routes.filter( |
There was a problem hiding this comment.
On the real index this also adopts all 10 Core/Chat stories for core/ChatLayoutScrollButton (they route to the non-component core/Chat with componentPath in Chat/), and none of them renders it. Requiring the adopted story's file to render the owner would keep Resizable, Indicator and NavMenu adoption and drop the Chat case.
| }); | ||
| }); | ||
|
|
||
| it("never takes a sibling component's stories: a Chat member keeps its own entry", () => { |
There was a problem hiding this comment.
This passes only because the fixture routes core-chat--layout to core/ChatLayout. The real index has no such story; its Core/Chat stories (core-chat--default and nine more) route to core/Chat, so the owner adopts them. Using the real shape here would catch it.
… curated checks to it Review: ResizeHandle still read a coverage gap because the D4 check of its pill, declared under Resizable in targets.json, never joined its adopted plan; and ChatLayoutScrollButton adopted Core/Chat's stories, which never render it. Adoption now also requires the story file to use the component, and a curated target on an adopted story checks the adopting component. ChatLayoutScrollButton keeps its verified N/A; the test fixture uses real Core/Chat story ids.
There was a problem hiding this comment.
Recommended for approval: component filters now resolve through the Storybook index, ResizeHandle gets its own RTL result from the D4 check of its pill, and ChatLayoutScrollButton no longer adopts Core/Chat stories that never render it. Both earlier findings are addressed, and no further changes are needed.
[Automated review]
…n does (#9391) fix(rtl-audit): the full audit adopts orphan stories as a filtered run does A component whose stories carry its folder's non-component title (a sub-component such as CheckIndicator under Core/Indicator) adopted those stories in a filtered pull-request run (#9319) but not in the release check's unfiltered audit, so the two scored it differently. resolveFilterRoutes now applies the same adoption, through one code path, to every known component no route names when no filter is given, and keeps every existing route. Each package barrel is read once. Tests check that a full run and a filtered run agree on ResizeHandle and ChatLayoutScrollButton.
Maintainer impact
A pull request that changes only
packages/core/src/Resizable/, such as #9197 or #9297, got a red, misleading touched-components check. The RTL audit was filtered tocore/ResizeHandle, the folder's public export, but the folder's stories are titledCore/Resizable, which names no public component. No story matched, so the audit stopped withFATAL: no runnable stories resolved for core scope, wrote no report, and the step failed. Now ResizeHandle is audited through those stories under its own name, including the curated D4 check of its pill, so a Resizable-only pull request shows ResizeHandle's own RTL result instead of a red step. A filtered component that truly has no story, such asHStack, gets a report where it reads as a coverage gap, as the RTL evidence rules require, instead of a crash.CI impact:
Touched components (visual, accessibility, RTL)andpr-rtlrun the same steps.Intended invariant
architecture:rtl-audit-evidenceINV5: a component in scope with no applicable story is a coverage gap unless verified N/A. This change keeps that, and adds no N/A path.spec:AST-030FR12: the touched check records each touched component's result. A sub-component whose stories carry its folder's non-component title is audited through them, under its own name.Change
apps/storybook/rtl-audit/rtl-audit-coverage.mjs:resolveFilterRoutes.componentPathin the folder, a title naming no public component, and a story file that uses the component. It routes them to the filter's own component, so the component keeps its own coverage entry and verified-N/A record and never takes a sibling's verdict.apps/storybook/rtl-audit/rtl-audit.mjs:componentExportsFromBarrel, and prints which stories an owner adopted.targets.json) checks the adopting component..github/scripts/touched-components.mjs: unchanged. Results now carry the owner's own name, and a coverage-gap report is an existing soft RTL fail.Evidence
rtl-audit-coverage.test.mjsresolveFilterRoutes:core/ResizeHandle;ChatLayoutScrollButtonadopts none of Core/Chat's real stories, which never render it;index.json:--filter core/HStackwrites a coverage-only report (core/HStack: coverage-gap), with no fatal error.--filter core/ChatLayoutScrollButtonadopts nothing and keeps its verified N/A.--filter core/ResizeHandleadopts Core/Resizable's stories (the run prints them) and reaches the browser launch. This machine cannot launch Chromium, so the rendered result comes from CI on the next Resizable pull request.pnpm vitest run --project node .github/scripts/: 102 files, all tests pass.Scope