Repository navigation
docs(spec): ChatMessageBubble records its shipped contract as current - #9136
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
There was a problem hiding this comment.
Promoting component:ChatMessageBubble to current waits on an engineering owner, who records the decision by commenting /approve-spec <full head SHA> on this pull request; the author may do this as an engineering owner. First, please correct the verification map so it credits only tests that fail when a claim breaks. That changes the spec text, so the approval should name the head that carries the fix.
- FR1 (no root for
children={null}), FR3 (the standalonebalancedfallback and the compact radius) and FR4 (ghost removing the fill and block padding) have no failing test, so they belong in the "Read from source" list unless focused tests are added. - AR4 is already covered by the Chromium visible-state test's 4.5:1 contrast assertion and can move into the map.
[Automated review]
| | AR1–AR4 | scoped axe report; Chromium receipts and pixels; narrow fixture | all visible states, light/dark, RTL, 320px | a semantic violation, low-contrast pair, or horizontal overflow fails its owning evidence layer | `audit:ChatMessageBubble/accessibility` | | ||
| | Contract | Verification | Representative states | Mutation or failure expectation | Audit section | | ||
| | -------- | --------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------- | --------------------------------------------------------------------------- | --------------------------------------- | | ||
| | FR1–FR3 | `ChatMessageBubble.test.tsx` children, sender, fallback, and compact density tests | standalone; user and assistant; compact | A missing root, fallback, or reflected axis fails. | `audit:ChatMessageBubble/behavior` | |
There was a problem hiding this comment.
No test fails if the bubble renders no root for children={null} (FR1), if a standalone bubble falls back to compact instead of balanced, or if the compact radius is dropped (FR3). With each of those changes applied, ChatMessageBubble.test.tsx, ChatMessage.test.tsx and themingTargets.test.ts all still pass. Move these partitions to the read-from-source list, or add focused tests.
| | Contract | Verification | Representative states | Mutation or failure expectation | Audit section | | ||
| | -------- | --------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------- | --------------------------------------------------------------------------- | --------------------------------------- | | ||
| | FR1–FR3 | `ChatMessageBubble.test.tsx` children, sender, fallback, and compact density tests | standalone; user and assistant; compact | A missing root, fallback, or reflected axis fails. | `audit:ChatMessageBubble/behavior` | | ||
| | FR4, FR9 | `ChatMessageBubble.test.tsx` variant and ghost tests; `themingTargets.test.ts` | filled; ghost at each density | Dropping reflection, the ghost alignment, or the target fails. | `audit:ChatMessageBubble/theming` | |
There was a problem hiding this comment.
The ghost tests compare inline-padding classes only, so a ghost bubble that keeps the sender fill and block padding passes every test. List FR4's fill and block-padding removal as read from source, or add a focused test.
| - FR3: the `spacious` inset. | ||
| - FR8: `className`, `style`, and `xstyle` composition. | ||
| - AR1: no role, tab stop, or name of the component's own. | ||
| - AR4: rendered text contrast in light and dark. |
There was a problem hiding this comment.
__tests__/ChatMessageBubble.a11y.chromium.spec.ts (lines 389–399) already fails below 4.5:1 bubble text contrast in light and dark, so AR4 can move into the map next to FR5 and AR3.
There was a problem hiding this comment.
Promoting component:ChatMessageBubble to current waits on an engineering owner, who records the decision by commenting /approve-spec <full head SHA> on this pull request; the author may do this as an engineering owner. The earlier verification-map corrections are in, but one row still overstates coverage, and fixing it changes the spec text, so the approval should name the head that carries the fix.
- The FR4/FR9 row credits ghost alignment at each density, but a ghost bubble that takes the wrong density's inline inset passes every test, because both ghost tests only check that the ghost and the name row share some class, and they always share
display: flex: narrow the row to removing the inset and list per-density ghost alignment as read from source, or tighten the tests. - The new AR4 row is missing its Audit section cell (
audit:ChatMessageBubble/accessibility).
[Automated review]
| | Contract | Verification | Representative states | Mutation or failure expectation | Audit section | | ||
| | -------- | --------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------- | --------------------------------------------------------------------------- | --------------------------------------- | | ||
| | FR1–FR3 | `ChatMessageBubble.test.tsx` children, sender, fallback, and compact density tests | standalone; user and assistant; compact | A missing root, fallback, or reflected axis fails. | `audit:ChatMessageBubble/behavior` | | ||
| | FR4, FR9 | `ChatMessageBubble.test.tsx` variant and ghost tests; `themingTargets.test.ts` | filled; ghost at each density | Dropping reflection, the ghost alignment, or the target fails. | `audit:ChatMessageBubble/theming` | |
There was a problem hiding this comment.
A ghost bubble that takes the wrong density's inline inset passes every test: both ghost tests only assert sharedClasses(ghost, nameSlot).length > 0, and the bubble root and name row always share the display: flex class. Only removing the inset entirely fails. Narrow this row to that, and list per-density ghost alignment as read from source (or tighten the tests).
| | FR7 | `ChatMessageBubble.test.tsx` slot tests; the Chromium visible-state test's numeric alignment | zero; true; false; empty string; nullish | A truthiness check loses zero's alignment or restores an empty wrapper. | `audit:ChatMessageBubble/behavior` | | ||
| | FR8, AR2 | `ChatMessageBubble.test.tsx` "forwards the ref and neutral DOM props to the bubble root" and `data-testid` test | ref; DOM, data, and ARIA props | Dropped forwarding fails. | `audit:ChatMessageBubble/public-api` | | ||
| | AR3 | `ChatMessageBubble.a11y.chromium.spec.ts` "keeps long content inside a 320px fixture" | 320 CSS px | Horizontal overflow fails. | `audit:ChatMessageBubble/accessibility` | | ||
| | AR4 | `ChatMessageBubble.a11y.chromium.spec.ts` visible-state test's contrast assertion | light and dark | Bubble text below 4.5:1 fails. | |
There was a problem hiding this comment.
This row has four cells under the five-column header; add the Audit section cell, audit:ChatMessageBubble/accessibility.
There was a problem hiding this comment.
Promoting component:ChatMessageBubble to current waits on an engineering owner, who records the decision by commenting /approve-spec <full head SHA> on this pull request; the author may do this as an engineering owner. The earlier verification-map fixes are in, but one gap remains, and because fixing it changes the spec text, the approval should name the head that carries the fix.
- The FR4/FR9 row lists
filledas a covered state, yet a filled bubble that loses its fill passes every test: add "FR4:filledpainting the sender fill" to the "Read from source, with no focused test yet" list, or add a test that fails when the fill is lost.
[Automated review]
| | Contract | Verification | Representative states | Mutation or failure expectation | Audit section | | ||
| | -------- | --------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------- | ----------------------------------------------------------------------------------- | --------------------------------------- | | ||
| | FR1–FR3 | `ChatMessageBubble.test.tsx` children, sender, fallback, and compact density tests | standalone; user and assistant; compact | A missing root, fallback, or reflected axis fails. | `audit:ChatMessageBubble/behavior` | | ||
| | FR4, FR9 | `ChatMessageBubble.test.tsx` variant and ghost tests; `themingTargets.test.ts` | filled; ghost | Dropping reflection, removing the ghost inset entirely, or losing the target fails. | `audit:ChatMessageBubble/theming` | |
There was a problem hiding this comment.
This row lists filled as a covered state, but a filled bubble painted with the ghost's transparent surface, or with no sender style, passes every ChatMessageBubble, ChatMessage and theming-target test: the variant test only checks data-variant, and the Chromium visible-state test asserts contrast and corners, not the fill. Add "FR4: filled painting the sender fill" to the read-from-source list below, or add a test that fails when the fill is lost.
There was a problem hiding this comment.
Promoting component:ChatMessageBubble to current now waits only on an engineering owner, who records the decision by commenting /approve-spec <full head SHA> on this pull request; the author may do this as an engineering owner. The earlier verification-map points are fixed: every credited test fails when its claim breaks, and the read-from-source list now names the filled sender fill. No other change is needed.
[Automated review]
2e62664 to
4b6c358
Compare
There was a problem hiding this comment.
Promoting component:ChatMessageBubble to current now waits only on an engineering owner, who records the decision by commenting /approve-spec <full head SHA> on this pull request; the author may do this as an engineering owner. The record matches the shipped source, and its verification map credits only tests that fail when their claim breaks. Optionally, add an ORD2 line (consumer styling applied after an explicit width) to the read-from-source list, since no test covers that order.
[Automated review]
|
/approve-spec 4b6c358 |
Exact decision needed
Promote
component:ChatMessageBubblefrom draft to current. No behavior, prop, default, target, or export changes.Existing authority searched
packages/core/src/Chat/ChatMessageBubble.spec.mdand the component's source, tests, and consumer docs on main.spec:AST-029owns the audit evidence inventory the verification map follows; the record's other cited owners are unchanged.Decision
component:ChatMessageBubble,authority: current, approved by cixzhang. Draft and audit wording is removed, basis cells read from current source and tests, and the verification map credits only tests that fail when a claim breaks, with a "Read from source, with no focused test yet" list for the rest. No implementation gap. OQ1–OQ3 stay open.Rejected alternatives
None.
Claim scope and non-goals
Claims are scoped to what main's source and tests show.
Evidence and implementation relationship
The component's colocated tests, as listed in the verification map.
Writing rubric results
Scope
Validation
pnpm check:knowledge: alignedgit diff --check: clean