feat(ui): show expected good failures as a green ✗ - #1140
Merged
Conversation
A GOOD_FAILURE trial rendered as a red X with a nearly invisible 4px green corner dot, and the analyzer legend labeled that dot "Pass" — an expected failure was indistinguishable from a bad one, or read as a success. The mirror case (BAD_SUCCESS) showed an unqualified green check. Make the cell semiotics uniform: glyph shape encodes the execution outcome (check/cross), fill color encodes the QA verdict once the trial is classified (green = good, red = bad). A good failure is now a green X, a bad success a red check; good-success and bad-failure look exactly as before. - status-config: getQaGlyphMatrixClass(status, classification) holds the 2x2 rule; shared ANALYSIS_CLASSIFICATION_LABELS. Only binary outcomes take the tone — partials keep the warm ramp, transitional states stay neutral. - trials matrix: apply the tone fill, show the corner dot only when it still carries information (QA running/failed, classified partial or error), and use the classification label in aria-label/tooltip. - legend: rename tone chips Pass/Fail to Good/Bad rendered as a check+X glyph pair, and rewrite the anatomy demo to teach shape=result, color=QA verdict. - trial-detail navigator: same override via the shared helper. Verified with tsc, eslint, and a browser-rendered harness covering all classification x outcome combinations (harness removed after).
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Oddish previewCommit:
Vercel deployment URL: https://oddish-p12qq7cgv.oddish.app Plan:
This comment is updated by the PR Preview workflow. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: medium. Left a non-blocking comment — Cursor Bugbot completed with 1 unresolved finding (QA glyph tone not gated by showAnalysis on share/public paths), so this is not auto-approved. Human review is needed; reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The QA-tone glyph change evaluated classification visibility ad hoc per render site, and two of the new consumers forgot the showAnalysis gate: the matrix aria-label announced "Good failure" on public share views, and the trial-detail navigator applied the QA fill and classification labels unconditionally. showAnalysis=false is the share-view contract for hiding QA entirely; every other surface (verdict badge, analysis card, live tab, repro command) honors it. Root fix instead of per-callsite patches: getVisibleAnalysisClassification (showAnalysis, trial) in status-config is now the single place the visibility gate lives. Matrix cell fill/label and the navigator glyph all derive from it, so a future consumer cannot forget the gate. Verified: tsc + eslint clean; browser harness rendering the matrix with showAnalysis on/off over classified trials — QA tones and labels show when enabled, plain Pass/Fail with default fills when hidden.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2135235. Configure here.
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
1 participant
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
A trial classified as GOOD_FAILURE rendered as a big red ✗ with a nearly invisible 4px green corner dot, and the analyzer legend labeled that dot "Pass" — so an expected failure either looked like a bad failure or read as a success. The mirror case (BAD_SUCCESS) showed an unqualified green ✓ with an invisible red dot.
Fix: shape = outcome, color = QA verdict
One uniform semiotic rule for result cells:
Good-success and bad-failure look exactly as before; only the two "cross" cases change color, so there's nothing new to learn for the common cases.
status-config.ts:getQaGlyphMatrixClass(status, classification)holds the 2×2 rule in one place; sharedANALYSIS_CLASSIFICATION_LABELS. Only binary outcomes take the tone — partials keep the warm ramp, transitional states stay neutral.aria-label/tooltip use the classification label ("Good failure", …).Verification
tsc --noEmitandeslintclean on all three files; diff is 147 insertions across 3 files, no unrelated churn.ExperimentTrialsTablein a temporary harness page with 10 synthetic cases (all classification × outcome combinations, analyzing, QA-failed, partial, unclassified, harness error), drove it in a browser, screenshot-confirmed the green ✗, and DOM-asserted every row's fill class, dot presence, aria-label, and tooltip. Harness removed afterward.Note
Low Risk
Presentation-only changes to experiment/trial UI with no backend or data-model impact; behavior is gated consistently via
showAnalysisfor public shares.Overview
Trial matrix cells and the trial-detail navigator now use a single rule: ✓/✗ shape is the harness outcome, and fill color is the QA verdict once analysis succeeds (green = good, red = bad). That makes good failures a green ✗ and bad successes a red ✓ instead of relying on a tiny corner dot.
Shared logic in
status-config.tsaddsgetVisibleAnalysisClassification(centralshowAnalysisgate),getQaGlyphMatrixClass, classification labels, and QA tone classes. Matrix cells use those for fill,aria-label/tooltips, and hide the corner dot when the glyph color already carries the verdict.The experiment table legend renames Pass/Fail chips to Good/Bad, shows ✓✗ pairs for those tones, and refreshes the anatomy tooltip to teach shape vs color.
Reviewed by Cursor Bugbot for commit 2135235. Bugbot is set up for automated code reviews on this repo. Configure here.