Repository navigation
docs(spec): ChatMessage records its shipped contract as current - #9131
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:spec-owner-review
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval of this promotion waits on an engineering owner's decision at this head: an engineering owner records it by commenting /approve-spec <full head SHA> on the pull request, and the author may do so if they are an engineering owner. Two corrections to the record are needed either way:
- FR7 says caller ARIA props reach the article, but
ChatMessage.tsxreplaces a calleraria-labeland drops a calleraria-labelledby; FR7 should name those as component-owned naming under FR6 and AR1. - The verification map lists FR3's scalar skip and
:emptycollapse and FR5's context as untested, thoughChatMessage.test.tsxandChatMessageBubble.test.tsxfail when they break, and it credits a Chromium accessibility check that no committed test runs.
[Automated review]
| | FR4 | Density MUST resolve explicit `density` → nearest `ChatMessageList` density → `balanced`, and the resolved value MUST reflect on the root target. | shipped implementation; consumer docs; focused tests; theming metadata | | ||
| | FR5 | Descendants MUST receive the resolved `{sender, density}` value through `ChatMessageContext`. | shipped implementation; descendant composition | | ||
| | FR6 | The article MUST carry the translated sender fallback as `aria-label`, and a non-system `name` that is not a skipped scalar MUST also label it through `aria-labelledby`. A name that renders text names the article; one that renders none leaves the fallback (accessible name computation 1.2, step 2B). | owner direction; accessible name computation 1.2; focused tests | | ||
| | FR7 | Accepted `ref`, DOM/data/ARIA props, `className`, `style`, and `xstyle` MUST reach and compose on the same article that carries the `chat-message` target. | `architecture:public-component-api/INV5–INV6, INV8`; focused tests | |
There was a problem hiding this comment.
FR7 says accepted ARIA props reach the article, but ChatMessage.tsx spreads caller props before its own aria-label and aria-labelledby: a caller aria-label="Order 42 update" renders "Message from assistant", and a caller aria-labelledby is dropped. Name aria-label and aria-labelledby here as component-owned naming under FR6/AR1 (public-component-api INV5 allows that), or state the precedence in ORD2.
| | ------------ | --------------------------------------------------------------- | ------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------- | ---------------------- | | ||
| | FR1–FR3 | `ChatMessage.test.tsx`; `ChatMessage.stories.tsx` | all senders; skipped scalar, renders-nothing, numeric, content, and suppressed slot states | Removing the scalar skip or the CSS collapse leaves an empty slot or a gap | `audit:ChatMessage/§4` | | ||
| | FR4–FR5 | `ChatMessage.test.tsx`; `themingTargets.test.ts` | standalone; inherited density; explicit override | Changing precedence or reflection gives descendants and themes a different density | `audit:ChatMessage/§2` | | ||
| | FR6, AR1–AR2 | `ChatMessage.test.tsx`; Chromium accessibility tree | visible, omitted, skipped-scalar, renders-nothing, and system names | Dropping the fallback `aria-label` or the name reference leaves an article unnamed | `audit:ChatMessage/§1` | |
There was a problem hiding this comment.
No committed Chromium spec covers ChatMessage (packages/core/src/Chat/__tests__/ has specs for ChatMessageBubble, ChatMessageList, ChatMessageMetadata and others, not ChatMessage). Drop "Chromium accessibility tree" or name the spec; AR2 is also listed as untested at line 233.
| - FR3: the visual slot order, skipping boolean and empty-string values, and a | ||
| wrapper collapsing when its content renders nothing. | ||
| - FR5: the context value descendants receive. |
There was a problem hiding this comment.
These entries understate coverage: removing the boolean/empty-string skip fails 9 tests and removing the :empty collapse fails 12 in ChatMessage.test.tsx (the evidence row FR1–FR3 already credits), and breaking the context's sender or density fails ChatMessageBubble.test.tsx:31, :49 and :187. Keep only the slot order under FR3, and credit ChatMessageBubble.test.tsx for FR5 (and in verified_by).
There was a problem hiding this comment.
Approval of this promotion waits on an engineering owner's decision at this head: an engineering owner records it by commenting /approve-spec <full head SHA> on the pull request, and the author may do so if they are an engineering owner. FR7 now names the article's own naming attributes, and the verification map credits the tests that protect FR3 and FR5. One small correction remains: the "Read from source, with no focused test yet" list still names AR2, although the FR6, AR1–AR2 row credits ChatMessage.test.tsx and its empty-array and empty-Fragment cases fail when slot content is inspected, so narrowing that entry to AR3 resolves it.
[Automated review]
| - FR3: the visual slot order. | ||
| - FR7: a caller `aria-label` or `aria-labelledby` not overriding the naming. | ||
| - FR6: a name that renders no text leaving the fallback name. | ||
| - AR2–AR3: no slot inspection, and no keyboard model of the component's own. |
There was a problem hiding this comment.
AR2 is credited to ChatMessage.test.tsx in the FR6, AR1–AR2 row (line 224), and it is tested: deciding slot presence by inspecting arrays and Fragments fails the empty-array, non-rendering-array and empty-Fragment collapse cases. Narrowing this entry to "AR3: no keyboard model of the component's own" keeps the list accurate.
There was a problem hiding this comment.
Approval of this promotion waits on an engineering owner's decision at this head: an engineering owner records it by commenting /approve-spec <full head SHA> on the pull request, and the author may do so if they are an engineering owner. The AR2 entry is now correct. One correction remains: the "Read from source, with no focused test yet" list should also name FR2's system message still rendering its children and PR1–PR2, because a system message that drops its children, a context value rebuilt on every render, and an Effect that adds a listener each pass every test that renders ChatMessage.
[Automated review]
| - FR1: start, end, and centre alignment by sender. | ||
| - FR3: the visual slot order. | ||
| - FR7: a caller `aria-label` or `aria-labelledby` not overriding the naming. | ||
| - FR6: a name that renders no text leaving the fallback name. | ||
| - AR3: no keyboard model of the component's own. |
There was a problem hiding this comment.
This list misses three claims no committed test protects: FR2's system message still rendering its children, PR1's no-Effect rule and PR2's stable context value. A system message that drops its children, a context value rebuilt every render, and an Effect adding a window listener each pass every test that renders ChatMessage. Adding "FR2: a system message still rendering its children" and "PR1–PR2: no listener or Effect, and a context value that changes only with sender or density" keeps the list accurate.
There was a problem hiding this comment.
Approval of this promotion waits on an engineering owner's decision at this head: an engineering owner records it by commenting /approve-spec <full head SHA> on the pull request, and the author may do so if they are an engineering owner. The FR2 and PR1–PR2 entries are now correct. One correction remains: the "Read from source, with no focused test yet" list should also name FR7's xstyle composition and ORD1's density-driven gaps, because dropping xstyle from the article or holding every gap at balanced passes every core UI test, while every other claim the map credits has a test that fails when it breaks.
[Automated review]
| - FR1: start, end, and centre alignment by sender. | ||
| - FR2: a system message still rendering its children. | ||
| - FR3: the visual slot order. | ||
| - FR7: a caller `aria-label` or `aria-labelledby` not overriding the naming. | ||
| - FR6: a name that renders no text leaving the fallback name. | ||
| - PR1–PR2: no listener or Effect, and a context value that changes only with | ||
| sender or density. | ||
| - AR3: no keyboard model of the component's own. |
There was a problem hiding this comment.
Two claims are still missing from this list: FR7's xstyle (line 122) and ORD1's density-driven gaps (lines 152–154). Dropping xstyle from the article's stylex.props call and holding the root and children gaps at balanced together pass all 15,364 core UI tests. Adding "FR7: xstyle composing on the article" and "ORD1: root and children gaps following the resolved density" completes the list; every other claim the map credits fails a test when broken.
There was a problem hiding this comment.
Approval of this promotion waits on an engineering owner's decision at this head: promoting FR1–FR7, ORD1–ORD2, PR1–PR2, AR1–AR3 and DEC-1 to current, including the rows the draft marked proposed and the FR7/ORD2 naming rule. An engineering owner records it by commenting /approve-spec <full head SHA> on the pull request, and the author may do so if they are an engineering owner. The xstyle and ORD1 entries are now in the "Read from source, with no focused test yet" list, and every claim the verification map credits fails a test when broken, so no record correction remains.
[Automated review]
|
/approve-spec f60332c |
github-actions
Bot
removed
the
needs:spec-owner-review
Exact decision needed
Promote
component:ChatMessagefrom draft to current. No behavior, prop, default, target, or export changes.Existing authority searched
packages/core/src/Chat/ChatMessage.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:ChatMessage,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 (a separately themeable name part) stays 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