Repository navigation
docs(spec): ChatMessageMetadata records its shipped contract as current - #9293
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 backfill waits on an engineering owner's decision: making component:ChatMessageMetadata current turns FR1–FR6, AR1–AR3, ORD1–ORD2 and PR1 into binding requirements, and no owner has approved this head yet. Please fix three things in the record first, so the approval covers the corrected text:
- The "Read from source, with no focused test yet" list includes FR3, the FR4 captions and FR6.
ChatMessageMetadata.a11y.chromium.spec.tsand theChatMessageBubble.test.tsxstatus tests already check these, so credit them in the map. - The FR5 row credits ARIA passthrough to a test that passes no
aria-*prop. Its quoted test name also lost its asterisks to Markdown (data-_). - FR4 says five registry icons, but
deliveredandreadsharecheckDouble.
After that, an engineering owner records the decision by commenting /approve-spec <full head SHA>, and the author may do this as an engineering owner.
[Automated review]
| | FR1 | The root appears when at least one slot passes the shared scalar-presence predicate or `status` is present. Non-scalar values are treated as present without inspecting their inner content. | Current source and tests | | ||
| | FR2 | Accepted slot values render in timestamp, footer, status order with dots between values considered present; numeric `0` is visible in either slot, and non-scalar nodes remain opaque. | Current source and tests | | ||
| | FR3 | A `user` message reverses row direction; `assistant`, `system`, and absent context use the forward path. | Current source | | ||
| | FR4 | Status selects one of five registry icons and localized labels; failure uses the error text token, and sending has a CSS pulse suppressed under reduced motion. | Current source and locale catalog | |
There was a problem hiding this comment.
STATUS_CONFIG maps five statuses to four icons: delivered and read both use checkDouble (ChatMessageMetadata.tsx:40-41). Suggest: "Status selects one of four registry icons (delivered and read share checkDouble) and localized labels; …"
| | Contract | Verification | Representative states | Mutation or failure expectation | Audit section | | ||
| | -------- | ------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------- | ------------------------------------------------------- | -------------------------------------- | | ||
| | FR1–FR2 | `ChatMessageMetadata.test.tsx` separator, empty-row, and numeric-zero tests; the metadata tests in `ChatMessageBubble.test.tsx` | empty scalar; zero; timestamp, footer, status | A stray separator, a blank row, or a hidden zero fails. | `audit:ChatMessageMetadata/behavior` | | ||
| | FR5 | `ChatMessageMetadata.test.tsx` "forwards rest props (data-_, aria-_, id) to the root element" | data, ARIA, and id props | A dropped prop fails. | `audit:ChatMessageMetadata/public-api` | |
There was a problem hiding this comment.
This test passes no aria-* prop: a version that drops aria-* from rest still passes it. Also, the name lost its asterisks to Markdown; the test is "forwards rest props (data-*, aria-*, id) to the root element". Put it in backticks and drop ARIA from this row, or move ARIA passthrough to the source-only list.
| | FR1–FR2 | `ChatMessageMetadata.test.tsx` separator, empty-row, and numeric-zero tests; the metadata tests in `ChatMessageBubble.test.tsx` | empty scalar; zero; timestamp, footer, status | A stray separator, a blank row, or a hidden zero fails. | `audit:ChatMessageMetadata/behavior` | | ||
| | FR5 | `ChatMessageMetadata.test.tsx` "forwards rest props (data-_, aria-_, id) to the root element" | data, ARIA, and id props | A dropped prop fails. | `audit:ChatMessageMetadata/public-api` | | ||
|
|
||
| Read from source, with no focused test yet: |
There was a problem hiding this comment.
Several items below are already tested. ChatMessageMetadata.a11y.chromium.spec.ts (still in verified_by, run by the release check) asserts sender-relative order in LTR and RTL, each status caption, and the translated title. The ChatMessageBubble.test.tsx tests "renders status" and "renders timestamp and status on one row" fail without the status aria-label. Credit them in the map and keep this list to what no test catches.
…ts and counts four icons
There was a problem hiding this comment.
Approval of this backfill still waits on an engineering owner's decision: making component:ChatMessageMetadata current turns FR1–FR6, AR1–AR3, ORD1–ORD2 and PR1 into binding requirements, and no owner has approved this head. The latest commit fixes the FR4 icon count, the FR5 row and most of the source-only list; two verification-map details remain:
- The FR1–FR2 row credits the
ChatMessageMetadatatests inChatMessageBubble.test.tsx, but none of them fails when a blank row, a hidden zero or a stray separator is introduced; drop that credit or point it at the browser spec'sempty-footer,empty-rowandnumeric-slotscases. - The "Read from source" list leaves out FR3's forward row for standalone and
systemuse, which no test checks.
Then an engineering owner records the decision by commenting /approve-spec <full head SHA> on the head that carries the final text; the author may do this as an engineering owner.
[Automated review]
| | FR6, AR1–AR3 | Axe and browser semantic receipts | static status and caller button | Missing text/icon or falsely claimed AT timing remains detectable | `audit:ChatMessageMetadata/testing` | | ||
| | Contract | Verification | Representative states | Mutation or failure expectation | Audit section | | ||
| | ------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------- | ---------------------------------------------------------------------------------- | -------------------------------------- | | ||
| | FR1–FR2 | `ChatMessageMetadata.test.tsx` separator, empty-row, and numeric-zero tests; the metadata tests in `ChatMessageBubble.test.tsx` | empty scalar; zero; timestamp, footer, status | A stray separator, a blank row, or a hidden zero fails. | `audit:ChatMessageMetadata/behavior` | |
There was a problem hiding this comment.
None of the ChatMessageMetadata tests in ChatMessageBubble.test.tsx fails when a blank row, a hidden zero or a stray separator is introduced; only ChatMessageMetadata.test.tsx does. Their empty case (renders nothing when all props are empty) counts article elements, so it passes while an empty metadata root renders. Suggest dropping "; the metadata tests in ChatMessageBubble.test.tsx" here, or crediting the browser spec's empty-footer, empty-row and numeric-slots cases instead.
| - FR5: ARIA passthrough, `ref`, `className`, `style`, and `xstyle` | ||
| composition, and the target. | ||
| - FR6, AR1: no live region of the row's own. | ||
| - PR1: no state, Effect, timer, observer, or listener. |
There was a problem hiding this comment.
Suggest adding FR3, AV3: the forward row for system and standalone use. The browser spec checks sender order only for the user status-read and assistant assistant-footer cases, so nothing catches a change to the fallback outside ChatMessage.
…nd lists the forward fallback
There was a problem hiding this comment.
Approval of this backfill waits on an engineering owner's decision: making component:ChatMessageMetadata current turns FR1–FR6, AR1–AR3, ORD1–ORD2 and PR1 into binding requirements, and no owner has approved this head. The latest commit fixes both earlier verification-map notes; one detail remains:
- The FR1–FR2 row lists a "timestamp, footer, status" state, but
ChatMessageMetadata.test.tsxnever renders a status, so dropping the dot before the status or moving the status ahead of the footer passes every credited Vitest test; credit the browser spec'snarrow-overflowandstatus-*cases there, and list the separator before a status after an empty scalar slot as read from source.
Then an engineering owner records the decision by commenting /approve-spec <full head SHA> on the head that carries the final text; the author may do this as an engineering owner.
[Automated review]
| | FR6, AR1–AR3 | Axe and browser semantic receipts | static status and caller button | Missing text/icon or falsely claimed AT timing remains detectable | `audit:ChatMessageMetadata/testing` | | ||
| | Contract | Verification | Representative states | Mutation or failure expectation | Audit section | | ||
| | ------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------- | ---------------------------------------------------------------------------------- | -------------------------------------- | | ||
| | FR1–FR2 | `ChatMessageMetadata.test.tsx` separator, empty-row, and numeric-zero tests | empty scalar; zero; timestamp, footer, status | A stray separator, a blank row, or a hidden zero fails. | `audit:ChatMessageMetadata/behavior` | |
There was a problem hiding this comment.
This row lists a "timestamp, footer, status" state, but none of the credited ChatMessageMetadata.test.tsx tests passes a status. Dropping the dot between footer and status (ChatMessageMetadata.tsx:152), or rendering the status before the footer, leaves every credited Vitest test green; only the browser spec's narrow-overflow text check catches those, and the map credits that spec under FR3, FR4, FR6. Suggest crediting the browser spec's narrow-overflow and status-* cases here, and adding "FR2: the separator before status after an empty scalar slot" to the source-only list, since no test covers that case.
There was a problem hiding this comment.
Approval of this backfill waits on an engineering owner's decision: making component:ChatMessageMetadata current turns FR1–FR6, AR1–AR3, ORD1–ORD2 and PR1 into binding requirements, and no owner has approved this head yet. The latest commit fixes the last verification-map note, and no other changes are needed. Before approving, consider FR6: it would make an aria-label on the role-less status span a requirement, and ARIA does not let that role be named. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request; the author may do this as an engineering owner, and any later edit to the record's body resets the approval.
[Automated review]
There was a problem hiding this comment.
Approval of this backfill waits on an engineering owner's decision: making component:ChatMessageMetadata current turns FR1–FR6, AR1–AR3, ORD1–ORD2 and PR1 into binding requirements, and no owner has approved this head yet. The latest commit resolves the earlier FR6 concern by recording the role-less status aria-label as an implementation gap, and no other changes are required. Optionally, align the Review checks row ("omitted translated status label") with the Current-state impact wording so the summary table does not read as rejecting a fix for that gap. An engineering owner records the decision by commenting /approve-spec <full head SHA> on the head that carries the final text; the author may do this as an engineering owner, and any later edit to the record's body resets the approval.
[Automated review]
| | End-user impact | Readers see a compact line of time, extra details, and delivery status in reading order, with a dot only between items that show something and a caption beside each status icon. | | ||
| | Builder impact | None. | | ||
| | Compatibility/readiness | Records shipped behavior; no public change. One implementation gap, listed under Current-state impact; two owner questions stay open. | | ||
| | Review checks | Reject a lost visible zero, stranded separator, omitted translated status label, dropped root passthrough, or claim that a static DOM attribute proves an AT announcement. | |
There was a problem hiding this comment.
Optional: Current-state impact now calls the role-less translated aria-label "the status label" and a gap, so "omitted translated status label" here can read as rejecting the fix for that gap. Saying "omitted translated status caption or title" here, or "status aria-label" in the Current-state impact heading, removes that reading.
There was a problem hiding this comment.
Approval of this backfill waits on an engineering owner's decision: making component:ChatMessageMetadata current turns FR1–FR6, AV1–AV3, AR1–AR3, ORD1–ORD2, PR1 and the representative states into binding requirements, and no owner has approved this head yet. The latest commit aligns the Review checks row with the recorded status aria-label gap, and no other changes are required. An engineering owner records the decision by commenting /approve-spec <full head SHA> on the head that carries the final text; the author may do this as an engineering owner, and any later edit to the record's body resets the approval.
[Automated review]
|
/approve-spec 10c982a |
github-actions
Bot
removed
the
needs:spec-owner-review
Exact decision needed
Promote
component:ChatMessageMetadatafrom draft to current. No behavior, prop, default, target, or export changes.Existing authority searched
packages/core/src/Chat/ChatMessageMetadata.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:ChatMessageMetadata,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. One implementation gap: the status span carries anaria-labelwithout a role, which WAI-ARIA 1.2 prohibits on a generic element (FR6, Current-state impact). OQ1–OQ2 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