Repository navigation
docs(spec): ChatToolCalls records its shipped contract as current - #9339
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
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.
This spec promotion needs an engineering owner's decision before it can be approved: on main the ChatToolCalls record is still a draft, so its claims, including FR5 recording the signed +N/-N colours as a known contrast gap while OQ2 stays open, are not yet current spec. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Because a new commit resets that approval, please fix these first:
- The verification map: no test covers FR2's translated-count fallback, the FR6 test already checks
className,styleand the target, the FR1 "latest-call surface" test cannot catch a wrong surface, and the axe scan and two story play guards that AR4 relies on are missing. - The Public contract line leaves out the shipped
datacall field.
[Automated review]
| | Governing rules | `architecture:public-component-api/INV5–INV6, INV8–INV9`; `architecture:interaction-modality`; `spec:AST-009/FR7`; `spec:AST-029/FR3–FR7`; WCAG 2.2 SC 1.4.3 and 2.4.7. | | ||
| | Area | Contract | | ||
| | ----------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | Public contract | `ChatToolCalls` with required `calls` (each with `name`, optional `status`, `target`, `node`, `duration`, `additions`, `deletions`, `stats`, `errorMessage`, `resultDetail`, and `key`), optional `label`, `isExpanded`, `defaultIsExpanded`, `onExpandedChange`, and root div props and ref. | |
There was a problem hiding this comment.
ChatToolCallItem also ships data?: unknown (ChatToolCalls.tsx:75-76, packages/core/api-report.md, and the calls prop docs). Add it so the current contract lists every released call field.
| - **AR2 — Collapsed ownership.** Collapsed group content is inert; hidden detail content is unmounted. | ||
| - **AR3 — Status meaning.** Every status exposes translated visually hidden text. Complete and error also use distinct semantic marks; pending and running currently share the same subtle spinner (OQ1). Error messages are exposed to assistive technology and echoed in the status icon's hover tooltip. | ||
| - **AR4 — Static evidence.** Focus appearance, keyboard behavior, inertness, color contrast, and DOM relationships use unit, axe, and real-browser evidence under `spec:AST-009/FR7`; this audit makes no AT-announcement claim. | ||
| - **AR3 — Status meaning.** Every status exposes translated visually hidden text. Complete and error also use distinct semantic marks; pending and running share the same subtle spinner (OQ1). Error messages are exposed to assistive technology and echoed in the status icon's hover tooltip. |
There was a problem hiding this comment.
Only call rows set the tooltip (ChatToolCalls.tsx:448-450); the status icon on the collapsed group header has no title (:648-649). Limit the tooltip clause to call rows.
| | Docs / surface | docs, declarations, exports, knowledge check, docs typecheck | public props, Chat subpath, draft record | Missing or stale docs, export, target, or knowledge shape fails. | `audit:ChatToolCalls/docs` | | ||
| | Contract | Verification | Representative states | Mutation or failure expectation | Audit section | | ||
| | ------------ | ------------------------------------------------------------------------------------------------- | ------------------------------------------ | -------------------------------------------------------------------------- | ----------------------------------- | | ||
| | FR1 | `ChatToolCalls.test.tsx` empty, single, latest-call surface, collapsed default, and toggle tests | empty; single; grouped | A rendered empty root, group chrome on one call, or a wrong surface fails. | `audit:ChatToolCalls/behavior` | |
There was a problem hiding this comment.
"renders latest call as surface" still passes when the collapsed surface shows the first call, because getAllByText('editFile') also matches the inert full list. The test that fails on a wrong surface is "includes the latest status in a collapsed group accessible name"; credit that one here.
| | Contract | Verification | Representative states | Mutation or failure expectation | Audit section | | ||
| | ------------ | ------------------------------------------------------------------------------------------------- | ------------------------------------------ | -------------------------------------------------------------------------- | ----------------------------------- | | ||
| | FR1 | `ChatToolCalls.test.tsx` empty, single, latest-call surface, collapsed default, and toggle tests | empty; single; grouped | A rendered empty root, group chrome on one call, or a wrong surface fails. | `audit:ChatToolCalls/behavior` | | ||
| | FR2 | `ChatToolCalls.test.tsx` "renders the public custom group label when expanded" | supplied label | An ignored label fails. | `audit:ChatToolCalls/behavior` | |
There was a problem hiding this comment.
This test covers only a supplied label. Replacing the translated-count fallback with an empty string passes all 27 tests, so FR2's omission half belongs on the untested list below (or needs a test).
| - FR1: caller order in the expanded list. | ||
| - FR4: Enter and Space activation. | ||
| - FR5: the secondary foreground on supporting text. | ||
| - FR6: `className`, `style`, and `xstyle` composition, and the target. |
There was a problem hiding this comment.
"forwards the root ref and supported DOM props" already checks className, style and the astryx-chat-tool-calls target: dropping either input or renaming the target fails it. Only xstyle is untested.
… and scopes the tooltip
There was a problem hiding this comment.
This spec promotion is ready for an engineering owner's decision: on main the ChatToolCalls record is still a draft, so making its claims current, including FR5 recording the signed +N/-N colours as a known contrast gap while OQ2 stays open, needs owner approval. The earlier corrections are in: the public contract lists data, the tooltip clause is scoped to call rows, and the verification map's credits and untested list hold up when each behaviour is broken in turn. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; a new commit resets that approval, so optionally fix the draft path in the description (docs/specs/components/Chat/ChatToolCalls.spec.md) first.
[Automated review]
|
/approve-spec 6d367f3 |
github-actions
Bot
removed
the
needs:spec-owner-review
Exact decision needed
Promote
component:ChatToolCallsfrom draft to current. No behavior, prop, default, target, or export changes.Existing authority searched
docs/specs/components/Chat/ChatToolCalls.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:ChatToolCalls,authority: current, approved by cixzhang. Draft and audit wording is removed, basis cells read from current source and tests, and the verification map credits tests for what they catch, with a "Read from source, with no focused test yet" list for the rest. One implementation gap: signed+N/-Nstatistics can miss WCAG 2.2 SC 1.4.3 text contrast (Current-state impact, OQ2). OQ1 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