Skip to content

Measure the themes' contrast in CI - #49

Open
koppor wants to merge 4 commits into
tests-pr47-findingsfrom
check-contrast
Open

Measure the themes' contrast in CI#49
koppor wants to merge 4 commits into
tests-pr47-findingsfrom
check-contrast

Conversation

@koppor

@koppor koppor commented Sep 10, 2026

Copy link
Copy Markdown
Member

Every contrast finding on the recent theme pull requests was found by hand or by a review bot, one pair at a time. A check now computes the ratios of each palette — layered on the JabRef theme the way JabRef layers it, following token references and derive() — against the WCAG floor the style guide states, and a per-theme baseline holds the shortfalls that exist today so the gate stops new ones. The CI log lists every pair, so lowering a baseline number is mechanical.

Stacked on github.com//pull/48; triggered by #46 (comment)

🤖 Generated with Claude Code

https://claude.ai/code/session_01X4mvBHaWcrBN9QDm5tzRdo

koppor and others added 2 commits September 10, 2026 13:03
Every contrast finding on the recent theme pull requests was found by hand
or by a review bot, one pair at a time. A check now computes the ratios of
each palette, layered on the JabRef theme the way JabRef layers it, against
the WCAG floor; a per-theme baseline holds the existing shortfalls so the
gate stops new ones.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4mvBHaWcrBN9QDm5tzRdo
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4mvBHaWcrBN9QDm5tzRdo
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Enforce theme contrast baselines in CI

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Measure layered theme palettes against WCAG floors, resolving token references and JavaFX
 derive().
• Baseline existing shortfalls so CI rejects regressions while allowing incremental improvements.
• Document contrast requirements and integrate the audit into theme validation.
Diagram

graph TD
  A["Theme CSS"] --> B["Palette layering"] --> C["Token resolution"] --> D["WCAG checks"] --> E{"Baseline met?"}
  F["Baseline file"] --> E
  E -->|yes| G["CI passes"]
  E -->|no| H["CI fails"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Baseline individual contrast failures
  • ➕ Prevents a newly introduced failure from being hidden by fixing a different failure
  • ➕ Makes accepted debt explicit by theme, scheme, foreground, and background pair
  • ➕ Supports precise review of baseline additions and removals
  • ➖ Produces a larger and more frequently changing baseline file
  • ➖ Requires normalization when the checked token-pair matrix changes
2. Use JavaFX-backed CSS evaluation
  • ➕ Matches JavaFX parsing and color derivation semantics more closely
  • ➕ Avoids relying on regular expressions for nested CSS and media blocks
  • ➖ Adds runtime and implementation complexity
  • ➖ May require constructing scenes or exposing computed token values
  • ➖ The lightweight script is easier to run locally and in existing CI

Recommendation: Incremental baseline gating is the right rollout strategy, but baselining normalized failure identities would be safer than storing only counts. Count-only comparison can allow a new regression when another failure is fixed in the same change. The lightweight Python resolver remains appropriate for the repository's constrained token syntax, provided its parsing and derive() assumptions are covered by focused fixtures.

Files changed (4) +196 / -0

Tests (1) +158 / -0
check-contrast.pyAdd layered WCAG contrast validator +158/-0

Add layered WCAG contrast validator

• Introduces a Python audit that parses light and dark palettes, layers theme overrides onto JabRef defaults, and resolves hexadecimal colors, token references, and 'derive()' expressions. It measures applicable foreground/background pairs and fails when a theme exceeds its baselined shortfall count.

.github/check-contrast.py

Documentation (1) +11 / -0
styleguide.mdDocument WCAG contrast floors and baseline policy +11/-0

Document WCAG contrast floors and baseline policy

• Adds the repository's 4.5:1 and 3:1 contrast requirements and explains palette layering. It also documents how the CI baseline permits incremental remediation while requiring new themes to start clean.

styleguide.md

Other (2) +27 / -0
contrast-baseline.txtRecord existing per-theme contrast shortfalls +25/-0

Record existing per-theme contrast shortfalls

• Defines the currently accepted number of contrast failures for each existing theme. Unlisted and newly added themes receive a zero-shortfall allowance, while improved themes trigger guidance to lower their baseline.

.github/contrast-baseline.txt

check-themes.ymlRun contrast validation in theme CI +2/-0

Run contrast validation in theme CI

• Adds the contrast audit to the existing theme parsing and validation job, causing pull requests and main-branch pushes to fail when shortfall counts regress.

.github/workflows/check-themes.yml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 10, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Contrast regressions can trade places ✓ Resolved 🐞 Bug ≡ Correctness
Description
The baseline comparison checks only len(found) against a stored per-theme count rather than
preserving the identities of failing foreground/background pairs. When one existing shortfall is
fixed while a different pair falls below its target, the total remains unchanged and CI accepts the
new regression.
Code

.github/check-contrast.py[R142-143]

+    if len(found) > allowed.get(theme, 0):
+        worse.append(f"{theme}: {len(found)} shortfall(s), {allowed.get(theme, 0)} allowed")
Evidence
The script reduces all failures to len(found) before comparison, while every baseline entry stores
only a numeric total; therefore equal-sized but different failure sets are indistinguishable.

.github/check-contrast.py[139-146]
.github/contrast-baseline.txt[1-25]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The contrast baseline stores and compares only failure counts, allowing a newly failing pair to replace a resolved pair without failing CI.
## Fix Focus Areas
- .github/check-contrast.py[124-146]
- .github/contrast-baseline.txt[1-25]
## Recommended Fix
Store canonical theme, scheme, foreground, and background identities for each accepted shortfall. Compare the current failure set with that baseline, fail for every newly introduced pair, and report baseline entries whose pairs have been resolved.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Named colors disappear from checks ✓ Resolved 🐞 Bug ≡ Correctness
Description
color() accepts only three- or six-digit hexadecimal literals before returning None for
otherwise valid named colors such as white and black, and failures() silently excludes pairs
containing that result. Included DinoGirls themes use those names for default, muted, and emphasis
foregrounds, so numerous surface and fill combinations are absent from both their measurements and
their baselines.
Code

.github/check-contrast.py[63]

+    hex_color = re.fullmatch(r"#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})", value)
Evidence
The parser's only literal-color branch recognizes #RGB and #RRGGBB, and line 116 requires both
resolutions to be truthy. A theme included by the script's one-level glob assigns white and
black directly to several foreground tokens, proving those checks are currently skipped.

.github/check-contrast.py[56-75]
.github/check-contrast.py[108-119]
themes/DinoGirls Themes/jabrefdark-jabreflight-contrasttext.css[15-17]
themes/DinoGirls Themes/jabrefdark-jabreflight-contrasttext.css[53-55]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Valid named foreground colors currently resolve to `None`, causing required contrast pairs to be silently omitted.
## Fix Focus Areas
- .github/check-contrast.py[56-75]
- .github/check-contrast.py[108-119]
- themes/DinoGirls Themes/jabrefdark-jabreflight-contrasttext.css[15-17]
- themes/DinoGirls Themes/jabrefdark-jabreflight-contrasttext.css[53-55]
## Recommended Fix
Add resolution for the JavaFX named colors used by themes, including `white` and `black`. Treat any unresolved token participating in a required pair as a CI error rather than skipping it, so unsupported values cannot reduce the measured failure count.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Some filled controls escape checks ✓ Resolved 🐞 Bug ≡ Correctness
Description
ON_FILL omits both emphasis text on the hover overlay and selected-badge text on
derive(-color-success, 70%). Papers documents these as actual rendering combinations in both
schemes, so changes to either foreground or fill can violate that theme's stated contrast contract
without entering found.
Code

.github/check-contrast.py[R39-40]

+ON_FILL = {"-color-fg-emphasis": ["-color-selection", "-color-badge-bg", "-color-button-default"],
+           "-color-badge-selected-fg": ["-color-success"]}
Evidence
The pair matrix checks emphasis only against selection, badge, and default-button fills, and checks
selected-badge foreground only against the base success color. Papers explicitly states that
emphasis is rendered on the hover overlay and that selected-badge text is rendered on both the base
and 70%-derived success fills.

.github/check-contrast.py[39-40]
themes/Papers/papers.css[12-18]
themes/Papers/papers.css[81-85]
themes/Papers/papers.css[158-160]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The fill-pair matrix omits two documented foreground/background combinations used by actual controls.
## Fix Focus Areas
- .github/check-contrast.py[39-40]
- .github/check-contrast.py[108-119]
- themes/Papers/papers.css[12-18]
- themes/Papers/papers.css[81-85]
- themes/Papers/papers.css[158-160]
## Recommended Fix
Add checks for emphasis text on the composited hover overlay and selected-badge text on `derive(-color-success, 70%)`. Extend pair evaluation to resolve derived expressions and composite translucent overlays over their underlying surfaces before calculating contrast.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/check-contrast.py Outdated
Comment thread .github/check-contrast.py Outdated
Comment thread .github/check-contrast.py Outdated
A count alone lets a change fix one pair and break another unnoticed, so
the baseline carries a fingerprint of the pairs as well.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4mvBHaWcrBN9QDm5tzRdo
@koppor

koppor commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

🤖 Generated with Claude Code

Qodo's point about count-only baselining is right — fixing one pair while breaking another keeps the number and slips through. c153089 adds a fingerprint of the pair set to each baseline line, so a swap fails with the line to write if it was intended. Verified by pointing a baseline line at a wrong fingerprint: the check reports it and exits 1.

The full pair list stays out of the baseline on purpose: it is roughly 700 lines today, most of them inherited by the fourteen Dino Girl's themes from the JabRef theme, so every improvement there would rewrite a large part of the file. The CI log prints every pair.

Named colors were read as unmeasurable, so the themes that write white and
black dropped out of half their own checks. Translucent fills are now
measured over each surface they can lie on, and the badge text is measured
on the paler green JabRef derives for a partly selected group -- the pair
that started this in the first place.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4mvBHaWcrBN9QDm5tzRdo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant