feat(apollo-vertex): add ConfidenceSignal component - #1001
feat(apollo-vertex): add ConfidenceSignal component#1001ChloeDalyUiPath wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new ConfidenceSignal UI component to the Apollo Vertex registry and documentation. The component is intended to communicate AI confidence levels (high/medium/low/unknown) via a signal-bar chip, optionally showing a hover/click popover with explanations, factor breakdowns, and CTAs.
Changes:
- Introduces
ConfidenceSignal+SignalBars, including an optional “acquire” animation and hover-open / 150ms-close popover behavior. - Registers the component in
apps/apollo-vertex/registry.jsonfor the Vertex registry build pipeline. - Adds a new docs page at
/components/confidence-signaland adds it to the components nav.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| apps/apollo-vertex/registry/confidence-signal/confidence-signal.tsx | Implements the ConfidenceSignal chip, popover content, and signal-bar SVG/animation. |
| apps/apollo-vertex/registry.json | Registers the new registry:ui entry for confidence-signal. |
| apps/apollo-vertex/app/components/confidence-signal/page.mdx | Adds component documentation and usage examples. |
| apps/apollo-vertex/app/components/_meta.ts | Adds “Confidence Signal” to the components navigation. |
Suppressed comments (5)
apps/apollo-vertex/registry/confidence-signal/confidence-signal.tsx:328
- Same issue for
nextStep:hrefis part of the prop type but isn’t used, so link-style next steps can’t be implemented without custom wrappers.
onClick={nextStep.onClick}
>
{nextStep.label}
<ArrowUpRight className="size-3" />
</Button>
apps/apollo-vertex/app/components/confidence-signal/page.mdx:33
- Same as above:
nextStepis shown withouthref/onClick, producing a no-op CTA in the docs example.
<div className="p-4 border rounded-lg mt-4 flex flex-wrap gap-4 items-center">
<ConfidenceSignal level="high" variant="med" />
<ConfidenceSignal level="medium" variant="med" nextStep={{ label: 'Review' }} />
<ConfidenceSignal level="low" variant="med" nextStep={{ label: 'Review' }} />
<ConfidenceSignal level="unknown" variant="med" />
</div>
apps/apollo-vertex/app/components/confidence-signal/page.mdx:42
- Same as above:
nextStepis shown withouthref/onClick, producing a no-op CTA in the docs example.
<div className="p-4 border rounded-lg mt-4 flex flex-wrap gap-4 items-center">
<ConfidenceSignal level="high" variant="max" />
<ConfidenceSignal level="medium" variant="max" nextStep={{ label: 'Review' }} />
<ConfidenceSignal level="low" variant="max" nextStep={{ label: 'Review' }} />
<ConfidenceSignal level="unknown" variant="max" />
</div>
apps/apollo-vertex/app/components/confidence-signal/page.mdx:74
- The popover example provides
nextStepwithouthref/onClick, which renders a no-op CTA in the docs.
{ label: 'Document match', value: '2 / 5', status: 'error' },
{ label: 'Historical accuracy', value: '61%' },
]}
nextStep={{ label: 'Review manually' }}
/>
apps/apollo-vertex/app/components/confidence-signal/page.mdx:84
- The acquire-animation example passes
nextStepwithouthref/onClick, producing a no-op CTA in the docs.
<div className="p-4 border rounded-lg mt-4 flex flex-wrap gap-4 items-center">
<ConfidenceSignal level="high" variant="max" animateIn />
<ConfidenceSignal level="medium" variant="max" animateIn nextStep={{ label: 'Review' }} />
</div>
| const ACQUIRE_KEYFRAMES = ` | ||
| @keyframes confidence-signal-acquire { | ||
| 0% { transform: scaleY(1); } | ||
| 30% { transform: scaleY(0.05); } | ||
| 100% { transform: scaleY(1); } | ||
| } | ||
| `; |
There was a problem hiding this comment.
Fixed. The SVG now carries a local @media (prefers-reduced-motion: reduce) rule that overrides the inline animation, so the bars render at their final state with no motion.
| const LEVEL_TEXT_CLASS: Record<ConfidenceLevel, string> = { | ||
| high: "text-success", | ||
| medium: "text-amber-700", | ||
| low: "text-destructive", | ||
| unknown: "text-foreground", | ||
| }; |
There was a problem hiding this comment.
Fixed. Medium now uses text-warning-foreground dark:text-warning, matching badge.tsx/alert.tsx. The split is needed because --warning is a light amber that fails contrast on a light background while --warning-foreground is near-black in both themes. Note the bar fills themselves stay literal hues on purpose: the signal metaphor relies on a fixed green/amber/red ramp reading identically in both themes, the way a battery or wifi icon does.
| {explainCta && ( | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| className="w-full" | ||
| onClick={explainCta.onClick} | ||
| > | ||
| {explainCta.label} | ||
| <ArrowUpRight className="size-3" /> | ||
| </Button> | ||
| )} |
There was a problem hiding this comment.
Fixed. ConfidenceSignalCta now renders an anchor via Button asChild when href is present, so the target is navigable, middle-clickable, and copyable. onClick still fires. Also tightened the type so at least one of href/onClick is now required.
| <div className="p-4 border rounded-lg mt-4 flex flex-wrap gap-4 items-center"> | ||
| <ConfidenceSignal level="high" variant="min" /> | ||
| <ConfidenceSignal level="medium" variant="min" nextStep={{ label: 'Review' }} /> | ||
| <ConfidenceSignal level="low" variant="min" nextStep={{ label: 'Review' }} /> | ||
| <ConfidenceSignal level="unknown" variant="min" /> | ||
| </div> |
There was a problem hiding this comment.
Fixed. Every example CTA now passes a real href. This is also enforced at the type level now, so a CTA with neither href nor onClick fails to compile rather than silently rendering a no-op.
| { label: 'Source quality', value: 'High', status: 'success' }, | ||
| { label: 'Data recency', value: '< 30 days', status: 'success' }, | ||
| ]} | ||
| explainCta={{ label: 'View audit trail' }} |
There was a problem hiding this comment.
Fixed, same as above. explainCta in the popover examples now points at a real target.
2303d26 to
bce2dca
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (5)
apps/apollo-vertex/registry/confidence-signal/confidence-signal.tsx:113
tooltipOpenis preserved while the popover is open. If the tooltip was open before opening the popover, closing the popover (e.g. by clicking outside) will immediately reopen the tooltip even when the pointer/focus is no longer on the trigger. ResettingtooltipOpenas part of the popoveronOpenChangeavoids this stale-state reopen/flash.
<Popover open={detailsOpen} onOpenChange={setDetailsOpen}>
apps/apollo-vertex/registry/confidence-signal/confidence-signal-factors.tsx:32
key={factor.label}is not guaranteed to be unique (labels can repeat), which can cause React key collisions and unstable row reconciliation. Use a stable unique key (e.g. include the index, or introduce anidonConfidenceFactor).
{factors.map((factor) => (
<ConfidenceSignalFactorRow key={factor.label} factor={factor} />
))}
apps/apollo-vertex/registry/confidence-signal/confidence-signal-levels.ts:17
ConfidenceCtacurrently allows providing neitherhrefnoronClick, which renders a CTA that looks interactive but does nothing (and the rest of this component set assumes at least one action). Consider enforcing “at least one of href/onClick” at the type level to prevent invalid CTAs.
label: string;
/** Renders the CTA as a link. Takes precedence over `onClick` alone. */
href?: string;
onClick?: () => void;
}
apps/apollo-vertex/registry/confidence-signal/confidence-signal-cta.tsx:28
if (cta.href)treats an empty-string href as “no href”, which would render a<button>instead of an<a>and drop link affordances. Checkinghref !== undefinedmatches the intended optionality and is robust against empty strings.
if (cta.href) {
apps/apollo-vertex/registry/confidence-signal/confidence-signal-chip.tsx:46
aria-labelandtype="button"are currently set before{...props}, so callers can accidentally override them via props spread (which contradicts the “always announced”accessibleLabelcontract and can reintroduce default submit-button behavior in forms). Spread props first, then settype/aria-labelso the component guarantees these attributes.
<button
type="button"
className={cn(
"inline-flex items-center gap-1.5 text-xs font-semibold focus-visible:outline-none focus-visible:ring-[3px] focus-visible:ring-ring/50",
LEVEL_CONFIG[level].textClass,
interactive && "cursor-pointer",
className,
)}
aria-label={accessibleLabel}
{...props}
// After the spread: as a Tooltip/Popover trigger this chip is cloned with
// the trigger's own `data-slot`, which would otherwise mask its identity.
data-slot="confidence-signal"
data-level={level}
>
bce2dca to
b6e36a1
Compare
|
Pushed an update addressing all review feedback. Rebased into the single commit rather than stacking fixups, per the repo's git workflow. @frankkluijtmans — your four points:
Copilot's earlier pass: Copilot's latest pass (3 suppressed comments), also fixed:
Checks: Two notes for reviewers:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (3)
apps/apollo-vertex/registry/confidence-signal/confidence-signal.tsx:123
- The popover currently relies on nesting
PopoverTriggerinsideTooltipTrigger(via thetooltipvariable). After making the tooltip trigger directly on the chip, the popover needs its own trigger wrapper. Wrapping the tooltip+chip in aPopoverTrigger asChildon a simple DOM element avoids nested Radix triggers while still allowing clicks/Enter on the inner button to bubble and open the popover.
<Popover open={detailsOpen} onOpenChange={handleDetailsOpenChange}>
{tooltip}
<PopoverContent align="start" className="flex w-64 flex-col gap-3">
apps/apollo-vertex/registry/confidence-signal/confidence-signal-chip.tsx:26
ConfidenceSignalChipis used as the child of RadixTooltipTrigger asChild/PopoverTrigger asChild. For RadixasChildto work correctly (positioning, focus management), the child must accept aref. This component is a plain function component, so it doesn’t forward refs and can cause runtime warnings or broken tooltip/popover behavior. Convert it toReact.forwardRef<HTMLButtonElement, ConfidenceSignalChipProps>and pass the ref to the<button>.
function ConfidenceSignalChip({
level,
label,
accessibleLabel,
animateIn = false,
apps/apollo-vertex/registry/confidence-signal/confidence-signal.tsx:103
TooltipTrigger asChildis currently given aPopoverTriggerelement whenhasDetailsis true. RadixasChildrequires the child to be a DOM element or aforwardRefcomponent;PopoverTriggerhere is a wrapper component (notforwardRef), so the tooltip trigger ref/handlers won’t attach reliably. Consider making the tooltip always trigger directly on the chip, and move the popover trigger wrapper to the popover render path instead.
This issue also appears on line 121 of the same file.
<Tooltip open={tooltipOpen && !detailsOpen} onOpenChange={setTooltipOpen}>
<TooltipTrigger asChild>
{hasDetails ? <PopoverTrigger asChild>{chip}</PopoverTrigger> : chip}
</TooltipTrigger>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
apps/apollo-vertex/registry/confidence-signal/confidence-signal.tsx:108
- Tooltips won’t reliably show on a disabled
<button>(disabled elements don’t receive pointer/focus events). SinceConfidenceSignalPropsinheritsbuttonprops (includingdisabled) and you’re guaranteeing “every chip carries a tooltip”, this should follow the existing pattern used elsewhere in apollo-vertex: wrap the disabled trigger in a non-disabled<span>and make the tooltip trigger target that wrapper.
const tooltip = (
<Tooltip open={tooltipOpen && !detailsOpen} onOpenChange={setTooltipOpen}>
<TooltipTrigger asChild>
{hasDetails ? <PopoverTrigger asChild>{chip}</PopoverTrigger> : chip}
</TooltipTrigger>
apps/apollo-vertex/registry/confidence-signal/confidence-signal.tsx:126
text-muted-foregroundon the<span>wrapping<SignalBars />has no effect because the SVG uses explicitfillhex values (it doesn’t inheritcurrentColor). This makes the styling misleading and harder to maintain. Either remove the unused text color class or updateSignalBarsto usecurrentColorif you actually want it tinted via text classes.
<span className="shrink-0 text-muted-foreground">
<SignalBars level={level} />
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (4)
apps/apollo-vertex/registry/confidence-signal/confidence-signal-chip.tsx:41
- In
ConfidenceSignalChip,{...props}comes aftertype="button"andaria-label={accessibleLabel}, so callers can override them (e.g., accidentally turning the chip into a submit button inside a form). Spread props first and settype/aria-labelafter to enforce the intended behavior.
<button
type="button"
className={cn(
"inline-flex items-center gap-1.5 text-xs font-semibold focus-visible:outline-none focus-visible:ring-[3px] focus-visible:ring-ring/50",
LEVEL_CONFIG[level].textClass,
interactive && "cursor-pointer",
className,
)}
aria-label={accessibleLabel}
{...props}
apps/apollo-vertex/registry/confidence-signal/confidence-signal-factors.tsx:37
ConfidenceSignalFactorsuses a composite key of${label}:${value}. If the same factor appears twice (same label and value), React keys collide and rows can be reused incorrectly. Include the index (or add an explicit id field) to guarantee uniqueness.
{factors.map((factor) => (
// Labels are not guaranteed unique, so pair label with value: two rows
// that match on both are indistinguishable to the reader anyway.
<ConfidenceSignalFactorRow
key={`${factor.label}:${factor.value}`}
factor={factor}
/>
apps/apollo-vertex/registry/confidence-signal/confidence-signal-cta.tsx:28
- The CTA rendering checks
if (cta.href), which treats an empty string as “no href” and falls back to the button path (whereonClickmay be undefined). Since the type distinguishes by presence (hrefvs nohref), check forundefinedinstead of truthiness.
if (cta.href) {
apps/apollo-vertex/registry/confidence-signal/confidence-signal-chip.tsx:11
ConfidenceSignalChipPropscurrently allows passingtypeandaria-labelvia the button props, but the component already manages these internally (type="button"andaccessibleLabel). Omitting them from the public props avoids accidental overrides and clarifies the API contract.
This issue also appears on line 32 of the same file.
export interface ConfidenceSignalChipProps
extends Omit<React.ComponentProps<"button">, "children"> {
level: ConfidenceLevel;
3a4fe40 to
d117b5c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
apps/apollo-vertex/registry/confidence-signal/confidence-signal-chip.tsx:46
{...props}is spread aftertypeandaria-label, so callers can overridetype="button"(risking accidental form submits) and replace the computed accessible label. Spread props first, then set the non-overridable attributes after.
aria-label={accessibleLabel}
{...props}
apps/apollo-vertex/registry/confidence-signal/confidence-signal-cta.tsx:24
- The ArrowUpRight icon is decorative and should be hidden from assistive tech to avoid being announced alongside the CTA label.
<ArrowUpRight className="size-3" />
d117b5c to
de83d52
Compare
|
Pushed 1. Prop spread order in
2. Decorative Verified in rendered output: all 16 chips emit Checks all pass ( |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
apps/apollo-vertex/registry/confidence-signal/confidence-signal-chip.tsx:33
- When converting
ConfidenceSignalChiptoforwardRef, ensure the forwardedrefis attached to the underlying<button>so Radix triggers can measure and focus it.
<button
className={cn(
apps/apollo-vertex/registry/confidence-signal/confidence-signal-chip.tsx:55
- After wrapping the component in
forwardRef(...), the closing brace needs to close theforwardRefcall (and optionally setdisplayName). Otherwise this file will not compile.
}
apps/apollo-vertex/registry/confidence-signal/confidence-signal-chip.tsx:26
TooltipTrigger/PopoverTriggerare used withasChildand receive<ConfidenceSignalChip />as the child. SinceConfidenceSignalChipis a function component that doesn’t forward refs, the Radix trigger ref will not reach the underlying<button>, which can break popover positioning and focus handling. Convert the chip toforwardRefand accept therefparameter.
function ConfidenceSignalChip({
level,
label,
accessibleLabel,
animateIn = false,
| @@ -0,0 +1,57 @@ | |||
| "use client"; | |||
|
|
|||
| import type * as React from "react"; | |||
There was a problem hiding this comment.
Not acting on this one — I believe it's a false positive for React 19, but flagging my reasoning in case a reviewer disagrees.
forwardRef isn't needed here. This app is on React 19.2.3, where ref is passed to function components as an ordinary prop and forwardRef is no longer required. ConfidenceSignalChipProps extends React.ComponentProps<"button">, which in @types/react 19 includes ref, and the component spreads {...props} onto the <button> — so the Radix trigger's ref does reach the element.
Verified three ways:
- Type test —
<ConfidenceSignalChip ref={useRef<HTMLButtonElement>(null)} ... />typechecks cleanly, sorefis part of the props contract and correctly typed. - House pattern —
grep -rl forwardRef registry/returns zero files. The repo's ownButton, used withasChildthroughout, is a plain function component spreading{...props}. AddingforwardRefhere would make this the only component doing so. - Behaviour — the popover opens, positions against the trigger, and returns focus correctly.
Worth noting the suggestion is also self-inconsistent: the accompanying suppressed comments warn that the forwardRef conversion would break compilation unless the closing brace and displayName are also fixed — which is a hazard introduced by the change, not by the current code.
Happy to convert it if anyone prefers the explicit form, but on React 19 it'd be redundant.
|
Since checks are gated on To get real CI signal in the meantime, I mirrored this commit onto #1034 (same commit, same authorship) directly in this repo rather than a fork. This PR stays open and untouched — nothing here is being closed or merged, #1034 is just to unblock CI while the write-access question gets sorted out. |
|
Update: with real CI now running on #1034, it caught a genuine bug — the registry check (which never ran here due to the fork gate) failed on: ```
```json Same value in both themes (already true for this token everywhere else in |
frankkluijtmans
left a comment
There was a problem hiding this comment.
Check @petervachon 's comments. Do we need some format/lint fixups?
Adds a signal-bar AI confidence chip (high/medium/low/unknown) with min/med/max density variants, a tooltip on every chip, and a popover for factor breakdowns and next-step CTAs, plus an opt-in one-time acquire animation that respects prefers-reduced-motion. The action CTA (nextStep) is required for medium/low confidence per team decision, and optional for high/unknown, enforced via a discriminated union type. Level labels and default explanations resolve through react-i18next under the confidence_signal_* prefix. Registered in registry.json with docs at /components/confidence-signal.
de83d52 to
21e7b1e
Compare
|
Thanks @petervachon — pulled your fix onto this branch ( First: you were right that the registry check never actually ran here. I'd been reporting "Apollo Vertex Registry Check ✓" from the check-run list, but that was the fork-gated job reporting a skip, not a real pass. My mistake, and it's exactly why the missing I reproduced the failure locally to confirm before fixing: Applied the @frankkluijtmans — on your question about format/lint fixups: no, nothing outstanding. Ran them just now against this branch:
So the only real defect was the |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
apps/apollo-vertex/registry/confidence-signal/confidence-signal-cta.tsx:29
- Branching on
if (cta.href)is a truthiness check, so an empty string (href: \"\") will incorrectly fall through to the button path (and can yieldonClickundefined). Prefer checking defined-ness (cta.href !== undefined) or discriminating via'href' in ctato match the TypeScript intent of the union.
if (cta.href) {
return (
apps/apollo-vertex/registry/confidence-signal/confidence-signal-factors.tsx:34
- React keys must be unique among siblings for reconciliation to work correctly. If two factors share the same
labelandvalue, this key collides and can cause incorrect row updates. Use a stable unique id if available, or include the array index as a last resort (e.g.,${label}:${value}:${index}) to guarantee uniqueness.
{factors.map((factor) => (
// Labels are not guaranteed unique, so pair label with value: two rows
// that match on both are indistinguishable to the reader anyway.
<ConfidenceSignalFactorRow
key={`${factor.label}:${factor.value}`}
factor={factor}
/>
apps/apollo-vertex/registry/confidence-signal/confidence-signal-chip.tsx:42
- The comment is misleading: placing
aria-labelbefore{...props}means a caller-providedaria-labelwill overrideaccessibleLabel(the default is lost). Either update the comment to reflect override behavior, or change the code to preserve the default when the caller doesn’t provide one (e.g., derivearia-labelfromprops['aria-label'] ?? accessibleLabel).
// Before the spread, so a caller can supply a more contextual label
// ("Confidence for Acme Health Plan: high") without losing the default.
aria-label={accessibleLabel}
{...props}
| // Suppressed while the popover is open so the two never stack. | ||
| const tooltip = ( | ||
| <Tooltip open={tooltipOpen && !detailsOpen} onOpenChange={setTooltipOpen}> | ||
| <TooltipTrigger asChild> | ||
| {hasDetails ? <PopoverTrigger asChild>{chip}</PopoverTrigger> : chip} | ||
| </TooltipTrigger> |
Adds
ConfidenceSignal, a signal-bar AI confidence chip (high/medium/low/unknown) with threedensity variants (
min/med/max), a tooltip on every chip, and a popover for factor breakdownsand next-step CTAs, plus an opt-in one-time "acquire" animation. Registered in
registry.jsonwithdocs at
/components/confidence-signal.Visuals, popover structure/behaviour, and animation timing were matched against the team demo at
ai-confidence-demo-kappa.vercel.app: rounded signal-bar pills, faded same-hue tracks for unfilledbars, per-level default explanations, and per-factor status tints.
Note on the
nextSteprequirementAn earlier internal demo site documented the action CTA as required for
low/unknown. Per thelive team decision (Peter + Haidy), this PR requires it for
medium/lowinstead (optional forhigh/unknown), enforced at compile time via a discriminated union onConfidenceSignalProps.Both interpretations were considered; this follows the call, not the demo site.
Accessibility and conventions
variant="min"(icon only) is never unlabelled.nextStepstays reachable by keyboardand on touch. The tooltip is suppressed while the popover is open.
href(rendered as a link, so navigable/middle-clickable) and/oronClick.prefers-reduced-motion: reduce.react-i18nextunder theconfidence_signal_*prefix in
locales/en.json, per AGENTS.md. Onlyen.jsonwas touched.-levels,-bars,-chip,-factors,-cta) withtargetfields onevery
registry.jsonfile entry, per the multi-file registry guidance in AGENTS.md.Scope
Component only, per the agreed two-PR split. A follow-up PR adds usage guidance to
app/guidelines/ai-toolkitand the components overview entry.Checks
pnpm install,pnpm registry:build,pnpm format,pnpm lint,pnpm lint:deps, andpnpm typecheckall pass (lint:depsreports only the 10 pre-existing repo-wide warnings, 0errors). Server-rendered output verified at
/components/confidence-signal: all variants and levelsrender, i18n labels resolve, and the SVG markup matches the demo. No runtime errors in the dev
server log.