fix(ci): stop compose-preview firing on non-UI PRs - #1742
Merged
Merged
Conversation
Two independent reasons the preview + a11y jobs ran and commented on PRs that cannot affect a single @Preview (e.g. a scrimage bump used only by backend/service-graphql). 1. The workflow had no path filter, so both 45-minute jobs ran on every PR. Add paths-ignore for backend/iosApp/landing-page/docs/markdown. Deliberately a denylist: previews render against shared KMP code and the version catalog, so an allowlist would silently stop rendering the first time a preview picked up a new transitive dependency. Dependency bumps that touch gradle/libs.versions.toml still render everything. 2. The a11y report is bimodal, so roughly half of all runs disagreed with the baseline and posted the full 74-preview standing report. Diffing the two variants, exactly one preview flaps: wearApp / SpeakerDetailsScreen_Devices - Large Round DuplicateSpeakableTextCheck (INFO), present in one variant and absent in the other: ImageView desc="John O'Reilly" duplicates 1 other item The speaker photo was labelled with the speaker's name while the SectionHeader immediately below announces the same string, so a screen reader read it twice. Which of SubcomposeAsyncImage's loading / error / success slots has settled when the a11y tree is scanned is a race, and only some of them carried the label - hence the finding appearing and disappearing between runs on identical code. The photo is decorative next to that header, so drop the label from all three slots. That removes a real finding and makes the flap structurally impossible rather than merely less likely. The comment logic itself was already correct: a11y.sh fetches compose-preview/a11y/main and stays silent on no change. It simply had nothing to stay silent about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014XYVcWMboPQeAHgw5Y7qCM
Contributor
Contributor
Accessibility Report5 error(s) · 0 warning(s) · 7 info across 74 preview(s). wearApp
|
| # | Level | Rule | Element | Message |
|---|---|---|---|---|
| 1 | ERROR | SpeakableTextPresentCheck | android.view.View | This item may not have a label readable by screen readers. |
| 2 | ERROR | SpeakableTextPresentCheck | android.view.View | This item may not have a label readable by screen readers. |
ColorSchemeSpecimenPreview
| # | Level | Rule | Element | Message |
|---|---|---|---|---|
| 1 | INFO | DuplicateSpeakableTextCheck | android.widget.TextView | This non-clickable item's speakable text: "Aa" is identical to that of 5 other item(s). |
Color_0x000080Swatches
| # | Level | Rule | Element | Message |
|---|---|---|---|---|
| 1 | INFO | DuplicateSpeakableTextCheck | android.widget.TextView | This non-clickable item's speakable text: "onSurfaceVariant / surface" is identical to that of 1 other item(s). |
Color_0x008000Swatches
| # | Level | Rule | Element | Message |
|---|---|---|---|---|
| 1 | INFO | DuplicateSpeakableTextCheck | android.widget.TextView | This non-clickable item's speakable text: "onSurfaceVariant / surface" is identical to that of 1 other item(s). |
Color_0x800000Swatches
| # | Level | Rule | Element | Message |
|---|---|---|---|---|
| 1 | INFO | DuplicateSpeakableTextCheck | android.widget.TextView | This non-clickable item's speakable text: "onSurfaceVariant / surface" is identical to that of 1 other item(s). |
ConferenceHomeScreenLoading · wearos_large_round
| # | Level | Rule | Element | Message |
|---|---|---|---|---|
| 1 | ERROR | SpeakableTextPresentCheck | android.view.View | This item may not have a label readable by screen readers. |
DefaultsSwatches
| # | Level | Rule | Element | Message |
|---|---|---|---|---|
| 1 | INFO | DuplicateSpeakableTextCheck | android.widget.TextView | This non-clickable item's speakable text: "onSurfaceVariant / surface" is identical to that of 1 other item(s). |
NotSetSwatches
| # | Level | Rule | Element | Message |
|---|---|---|---|---|
| 1 | INFO | DuplicateSpeakableTextCheck | android.widget.TextView | This non-clickable item's speakable text: "onSurfaceVariant / surface" is identical to that of 1 other item(s). |
SessionCardLoadingPreview
| # | Level | Rule | Element | Message |
|---|---|---|---|---|
| 1 | ERROR | SpeakableTextPresentCheck | android.view.View | This item may not have a label readable by screen readers. |
SocialIconPreview
| # | Level | Rule | Element | Message |
|---|---|---|---|---|
| 1 | ERROR | TouchTargetSizeCheck | android.widget.ImageView desc="Github" | This item's size is 24dp x 24dp. Consider making this touch target 32dp wide and 32dp high or larger. |
ThemeFoundationConfettiPreview
| # | Level | Rule | Element | Message |
|---|---|---|---|---|
| 1 | INFO | DuplicateSpeakableTextCheck | android.widget.TextView | This non-clickable item's speakable text: "Confetti" is identical to that of 1 other item(s). |
Unchanged (63 preview(s))
AndroidSessionDetailPreviewAndroidSpeakerDetailsPreviewBackgroundAndroidBackgroundDefaultBackgroundDynamicBookmarkAddPreviewBookmarkOnPreviewColorSchemeSpecimenPreviewConfettiHeaderPreviewScheduleScreenPreviewSessionDetailsPreviewSessionDetailsScreenPreviewSessionItemBreakPreviewSessionItemLightningPreviewSessionItemPopulatedPreviewSpeakerDetailsScreenPreviewSpeakerItemPreviewThemeFoundationAndroidPreviewThemeFoundationConferenceSeedPreviewThemeFoundationDefaultPreviewThemeFoundationDynamicPreviewTypographySpecimenPreviewBookmarksPreviewBookmarksPreviewEmptyBookmarksPreviewErrorBookmarksPreviewErrorLongBookmarksPreviewLongColor_0x000080Color_0x008000Color_0x800000ConferenceHomeScreenConferenceHomeScreenWithBookmarksConferencesViewCuratedPreviewConferencesViewErrorPreviewConferencesViewLongPreviewConferencesViewPreviewDayChipPreviewDefaultsHomeListViewAndroidMakersHomeListViewDevFestHomeListViewDroidconHomeListViewKotlinConfHomeListViewPreviewNotSetPlaceholderButtonPreviewScreenHeaderPreviewSectionHeaderPreviewSessionCardBookmarkedPreviewSessionCardPopulatedPreviewSessionDetailViewPreviewSessionDetailsScreenSessionListViewLongPreviewSessionListViewPreviewSessionSpeakerChipPreviewSettingsListViewLongPreviewSettingsListViewPreviewSpeakerDetailsScreenSpeakerDetailsViewPreviewThemeFoundationAndroidMakersPreviewThemeFoundationDevFestPreviewThemeFoundationDroidconPreviewThemeFoundationKotlinConfPreviewTypographySpecimenPreview
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.












Summary
Two independent reasons the preview + a11y jobs ran and commented on PRs that cannot affect a single
@Preview— e.g. #1738, ascrimage-filtersbump used only bybackend/service-graphql.1. No path filter
The workflow ran both 45-minute jobs on every PR. Added
paths-ignoreforbackend/,iosApp/,landing-page/,docs/,**.md.Deliberately a denylist rather than an allowlist of UI paths: previews render against shared KMP code and the version catalog, so an allowlist would silently stop rendering the first time a preview picked up a new transitive dependency. Note this does not help dependency bumps that touch
gradle/libs.versions.toml, even backend-only ones — those still render everything.The list is duplicated across the
pushandpull_requesttriggers because GitHub Actions supports neither YAML anchors nor!negation in path filters.2. One nondeterministic a11y finding
The a11y comment logic is already correct —
a11y.shfetchescompose-preview/a11y/main, diffs, and stays silent when nothing changed. It simply had something to report.The report is bimodal. Two stable
findings.jsonblobs alternate, and whichever one a PR lands on decides whether the bot posts:1f5bdd25compose-preview/a11y/main(the baseline), PRs #1737, #1733 → silent23550c30Diffing the two: 74 entries, 73 byte-identical, exactly one flaps.
SubcomposeAsyncImagelabelled the speaker photo with the speaker's name, while theSectionHeaderimmediately below announces the same string — so a screen reader read it twice. Which of theloading/error/successslots has settled when the a11y tree is scanned is a race, and only some of them carried the label, hence the finding appearing and disappearing between runs on identical code.The photo is decorative next to that header, so the label is dropped from all three slots. That removes a real finding and makes the flap structurally impossible rather than merely less likely.
Test plan
contentDescriptionis semantics-only, so the preview PNGs are unaffected.SpeakerDetailsScreenINFO as resolved, and subsequent unrelated PRs should stop getting the standing report.paths-ignorelists; this PR itself toucheswearApp/so it is not skipped by its own filter.Not addressed here
The underlying renderer nondeterminism that also makes ~6
wearApppreview PNGs flap on unrelated PRs is a separate fix incompose-ai-tools(stitcher anchor selection + downloadable-font resolution), not something this repo can fix.Two
MainActivitypreviews are still masked bymissing-renders: warn— a persistent render failure, not a transient one. Left alone here; it needs the CI render report to diagnose.Generated by Claude Code