test(viewer): tokens SSOT proptest surface (WBS-6.2 #450) - #471
test(viewer): tokens SSOT proptest surface (WBS-6.2 #450)#471KooshaPari wants to merge 1 commit into
Conversation
Adds crates/sl-viewer/tests/properties_viewer_tokens.rs with 10
proptest properties pinning the design-token SSOT invariants:
* lab_coat::*:
* Every hex is a well-formed #RRGGBB (7-char lowercase ASCII hex).
* Every hex is non-empty.
* All 16 documented hex constants are pairwise distinct.
* Every hex appears in TOKENS_CSS so the Rust mirror and the
CSS SSOT stay in sync.
* REQUIRED_CSS_VARS:
* Every entry starts with --.
* Every entry is non-empty.
* The set is duplicate-free.
* Every entry appears in TOKENS_CSS.
* VIEWER_COLOR_SCHEME:
* Declares both :root and :root[data-theme dark] selectors.
* Uses color-scheme exactly twice.
Updates WBS-6.2 evidence list, TRACEABILITY.json, and CHANGELOG.
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Review limit reached
Next review available in: 57 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| /// The full list of `lab_coat::*` hex constants in stable declaration | ||
| /// order. We compute this once via a small reflection-on-source approach: | ||
| /// every `pub const` in `lab_coat::*` whose value is a `&'static str` | ||
| /// starting with `#`. Since we can't introspect Rust modules at runtime, | ||
| /// we hard-code the list (mirroring `tokens.rs`). The constants are | ||
| /// public — any new addition requires also extending this list, which | ||
| /// the `proptest` exhaustiveness check below will catch. | ||
| fn lab_coat_hex_list() -> &'static [&'static str] { | ||
| &[ | ||
| lab_coat::LAB_WHITE, | ||
| lab_coat::SLATE, | ||
| lab_coat::COBALT, | ||
| lab_coat::COBALT_ON_DARK, | ||
| lab_coat::ORANGE, | ||
| lab_coat::TEAL, | ||
| lab_coat::TEAL_ON_DARK, | ||
| lab_coat::BG_DARK, | ||
| lab_coat::SURFACE_LIGHT, | ||
| lab_coat::BORDER_LIGHT, | ||
| lab_coat::BORDER_DARK, | ||
| lab_coat::TEXT_DARK, | ||
| lab_coat::TEXT_MUTED_LIGHT, | ||
| lab_coat::TEXT_MUTED_DARK, | ||
| lab_coat::DANGER_LIGHT, | ||
| lab_coat::DANGER_DARK, | ||
| ] |
There was a problem hiding this comment.
Suggestion: The list is manually maintained, so adding a new lab_coat constant to tokens.rs without adding it here will not cause any property to fail. The comment claims an exhaustiveness check, but there is no independent count or enumeration against the production module. This can silently leave a new Rust token unvalidated; use a genuinely exhaustive source or remove the exhaustiveness claim. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ New Lab-Coat constants can bypass token properties.
- ⚠️ Rust/CSS drift may reach the viewer undetected.
- ⚠️ The documented exhaustiveness claim is false.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/tests/properties_viewer_tokens.rs
**Line:** 44:69
**Comment:**
*Incomplete Implementation: The list is manually maintained, so adding a new `lab_coat` constant to `tokens.rs` without adding it here will not cause any property to fail. The comment claims an exhaustiveness check, but there is no independent count or enumeration against the production module. This can silently leave a new Rust token unvalidated; use a genuinely exhaustive source or remove the exhaustiveness claim.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| let hex = lab_coat_hex_list()[i]; | ||
| prop_assert!( | ||
| TOKENS_CSS.contains(hex), | ||
| "TOKENS_CSS missing lab_coat hex {:?}", | ||
| hex, |
There was a problem hiding this comment.
Suggestion: contains(hex) only verifies that the color appears somewhere in the shared stylesheet, not that the corresponding Lab-Coat token is assigned that color. For example, changing BG_DARK to the unrelated skeleton color #e5e7eb would still pass because that value exists elsewhere in TOKENS_CSS, while the --sl-bg mapping would be out of sync. Validate the token name and value on the same declaration line, as the existing mirror test does. [api mismatch]
Severity Level: Major ⚠️
- ❌ Viewer colors can diverge from Rust token meanings.
- ⚠️ Dark and light surfaces may render incorrectly.
- ⚠️ Existing checks omit viewer-specific CSS mappings.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/tests/properties_viewer_tokens.rs
**Line:** 114:118
**Comment:**
*Api Mismatch: `contains(hex)` only verifies that the color appears somewhere in the shared stylesheet, not that the corresponding Lab-Coat token is assigned that color. For example, changing `BG_DARK` to the unrelated skeleton color `#e5e7eb` would still pass because that value exists elsewhere in `TOKENS_CSS`, while the `--sl-bg` mapping would be out of sync. Validate the token name and value on the same declaration line, as the existing mirror test does.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| fn required_css_var_in_tokens_css(i in required_var_index_strategy()) { | ||
| let var = REQUIRED_CSS_VARS[i]; | ||
| prop_assert!( | ||
| TOKENS_CSS.contains(var), | ||
| "TOKENS_CSS missing required CSS var {:?}", | ||
| var, | ||
| ); |
There was a problem hiding this comment.
Suggestion: TOKENS_CSS.contains(var) does not prove that the required variable is declared. For instance, if the exact --sl-bg declaration is removed while --sl-bg-muted remains, the test still passes because the shorter name is a substring of the remaining variable. Match a complete custom-property declaration rather than an arbitrary substring. [api mismatch]
Severity Level: Major ⚠️
- ⚠️ Missing required CSS variables can evade CI.
- ❌ Viewer styles may lose intended token values.
- ⚠️ Prefix collisions weaken both token checks.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/tests/properties_viewer_tokens.rs
**Line:** 154:160
**Comment:**
*Api Mismatch: `TOKENS_CSS.contains(var)` does not prove that the required variable is declared. For instance, if the exact `--sl-bg` declaration is removed while `--sl-bg-muted` remains, the test still passes because the shorter name is a substring of the remaining variable. Match a complete custom-property declaration rather than an arbitrary substring.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| fn viewer_color_scheme_declares_both_selectors(_i in 0u8..4) { | ||
| prop_assert!(VIEWER_COLOR_SCHEME.contains(":root")); | ||
| prop_assert!(VIEWER_COLOR_SCHEME.contains("[data-theme=\"dark\"]")); |
There was a problem hiding this comment.
Suggestion: The two independent substring checks do not require the dark attribute selector to be attached to :root. A stylesheet containing a standalone :root rule and a separate .other[data-theme="dark"] rule would pass even though the injected dark color-scheme rule does not apply to the document root. Check for the exact :root[data-theme="dark"] selector. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Dark-mode browser color handling can fail.
- ⚠️ Scrollbars and form controls may retain light styling.
- ⚠️ Viewer root theme toggling becomes incorrectly wired.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/tests/properties_viewer_tokens.rs
**Line:** 171:173
**Comment:**
*Incorrect Condition Logic: The two independent substring checks do not require the dark attribute selector to be attached to `:root`. A stylesheet containing a standalone `:root` rule and a separate `.other[data-theme="dark"]` rule would pass even though the injected dark color-scheme rule does not apply to the document root. Check for the exact `:root[data-theme="dark"]` selector.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
Closing due to merge conflicts. |
User description
Summary
Adds
crates/sl-viewer/tests/properties_viewer_tokens.rswith 10 proptest properties pinning thetokensSSOT (WBS-6.2 #450).lab_coat::*(5 properties)#RRGGBB(7-char lowercase ASCII).TOKENS_CSS.REQUIRED_CSS_VARS(3 properties)--, is non-empty, and is unique across the documented set.TOKENS_CSS.VIEWER_COLOR_SCHEME(2 properties):rootand:root[data-theme="dark"]selectors.color-schemeproperty exactly twice.Validation
cargo test -p sl-viewer --test properties_viewer_tokens --features "desktop parquet" --locked— 10 passedcargo fmt --all --check— cleanWBS / TRACEABILITY
WBS-6.2 evidence list and
TRACEABILITY.jsongaincrates/sl-viewer/tests/properties_viewer_tokens.rs. CHANGELOG Unreleased documents the new surface.CodeAnt-AI Description
Add property coverage for viewer design tokens and color-scheme wiring
What Changed
Impact
✅ Earlier detection of viewer theme regressions✅ Fewer Rust/CSS design-token mismatches✅ Reliable light and dark browser color handling💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.