Skip to content

Night Watch Component Auditor

Ernest Tien edited this page Aug 6, 2026 · 24 revisions

Night Watch — Component Auditor

Assigned to: Cindy's Navi (cixzhang)

Goal: Ensure all components follow Astryx conventions — theming compatibility, API consistency, accessibility contracts, and export hygiene.

Source of truth documents:

⚠️ Keep in sync. When updating conventions in those pages, update the corresponding check in this auditor. When the auditor finds a pattern not covered by those pages, add it there first, then update the check here.


Why This Role Exists

Component quality regresses silently. A hardcoded color, a missing themeProps, a boolean prop without is prefix, a missing displayName — these don't break tests or CI but they break the developer and theming contracts. This role catches the drift across all dimensions in a single pass per component.


Scope

1. CSS Variable Usage (Theming)

All visual properties must use CSS variables (design tokens), not hardcoded values.

Check for:

  • Colors: no hardcoded hex, rgb, hsl values in StyleX styles — must use colorVars
  • Shadows: no raw boxShadow strings — must use elevation token variables (e.g. shadowVars['--shadow-menu']), not partially-tokenized strings like `0 4px 12px ${colorVars['--color-shadow']}`
  • Spacing: no raw px values for margins, paddings, gaps — must use spacingVars
  • Radii: no hardcoded border-radius — must use radiusVars
  • Typography: font sizes, weights, line heights should come from token variables
  • Font sizes must use semantic type scale tokens (typeScaleVars) instead of raw text size tokens (textSizeVars). Components should express intent ("this is a label") not implementation ("this is 14px").

Semantic type scale mapping:

Raw token (textSizeVars) Semantic token (typeScaleVars) Used for
--text-base --text-label-size Interactive controls: buttons, tabs, selectors, menu items, form labels, nav items
--text-base --text-body-size Content: text inputs, links, descriptions, table cells
--text-sm, --text-xsm --text-supporting-size Secondary: descriptions, breadcrumbs, captions, placeholders, badges, helper text
--text-lg --text-large-size Emphasis: empty state titles, nav headings
--text-2xs --text-supporting-size Micro: compact density descriptions

Neutral gray token semantics: Three tokens serve "gray" backgrounds. Components must use the correct one based on what the element is:

Token Role Use for
--color-secondary Neutral fill for self-contained elements Buttons (default), badges, tokens, kbd, avatar fallback, pagination dots, status dots, selected nav items, icon containers
--color-muted Background for containers with content inside Sections, code blocks, table zebra stripes, progress/slider tracks, disabled input fills, featured cards
--color-overlay-hover Translucent hover/active feedback Hover on ghost/surface elements, list/menu item highlights, table row hover

Detection: search for --color-wash, --color-muted, --color-secondary, and --color-overlay-hover in component StyleX styles and verify:

  • --color-wash should only appear in page-level backgrounds (AppShell). Flag if used on individual components.
  • --color-muted used on a small self-contained element (dot, badge, icon circle) → should be --color-secondary
  • --color-secondary used on a container with content inside → should be --color-muted
  • --color-overlay-hover used as a resting (non-hover) background → should be --color-secondary or --color-muted
  • --color-muted used for a hover/active state → should be --color-overlay-hover

Decision tree: (1) hover/active state → overlay-hover, (2) container with content → muted, (3) self-contained element on surface → secondary.

Internal variable access in theme files (packages/themes/*):

  • Theme files must not reference internal layout/container padding variables: --layout-padding-inner-x, --layout-padding-inner-y, --layout-padding-outer-x, --layout-padding-outer-y, --container-padding-inline, --container-padding-block-start, --container-padding-block-end
  • These are implementation details of the container/layout system. Use padding, paddingBlock, or paddingInline on the component override instead — the theme pipeline maps them to container tokens automatically (PR #1235)
  • Detection: grep for --layout-padding-* and --container-padding-* in any file under packages/themes/
  • Fix: replace with padding: '16px 20px' (or paddingBlock/paddingInline) in the component's base slot
  • Deprecated: --astryx-card-padding, --astryx-section-padding, --astryx-dialog-padding still work via fallback but should be migrated to padding shorthand

Exceptions:

  • 0, 'none', 'transparent', 'inherit', 'currentColor' are fine
  • Layout values that aren't semantically spacing (e.g. width: '100%', flex: 1) are fine
  • Large layout constraints (maxHeight, maxWidth, minWidth) are acceptable as hardcoded values. These are component-specific design decisions, not theme tokens. Examples: a dropdown's maxHeight: '300px' to cap scroll height, an empty state's maxWidth: '360px' to constrain text width. These rarely need to change per-theme and tokenizing them adds indirection without meaningful benefit. Flag only if a constraint seems wrong, not because it's hardcoded.
  • Animation values (durations, easing) have their own token story — flag but don't block
  • Internal layout padding variables (--layout-padding-*) are fine in packages/core/ — they're internal there. Only flag them in packages/themes/.

2. Component CSS Variable Naming (Theming)

Component-specific CSS custom properties must follow the naming convention:

--[_]<component>[-<part>]-<property>
  • <component> — lowercase component name without Astryx prefix, matching themeProps output (e.g. button, dropdown-menu, segmented-control)
  • <part> (optional) — sub-element within the component (e.g. composer in chat-composer)
  • <property> — what the var controls (e.g. radius, padding, opacity)
  • --_ prefix — private vars (managed by the derived expansion pipeline); public vars omit it

Check for:

  • Component CSS vars follow the naming pattern. Flag any that don't match (e.g. --composer-radius should be --_chat-composer-radius).
  • Private vars (those with a derived entry mapping a standard CSS property to them) must:
    • Use the --_ prefix in their name
    • Have private: true in their doc entry
    • Be listed in derivedVarRegistry.ts
  • Public vars (no standard CSS property equivalent — e.g. --button-press-scale) must:
    • NOT use the --_ prefix
    • NOT have private: true
  • Var names must use the component key from themeProps, not abbreviations or different names. For sub-elements, include the parent component name (e.g. --_chat-composer-radius, not --_composer-radius).

Auto-fix: Rename vars that don't match the convention. Update source files, doc files, and derivedVarRegistry.ts together.

3. themeProps Application (Theming)

Every component and visual variant must apply themeProps on the correct DOM element so themes can target them.

Check for:

  • The element receiving visual styles has the correct themePropsnot always the root element. The main visual styles (background, border-radius, shadow, typography) determine which element gets the className.
  • Visual variants that need distinct theming get variant-specific classNames (e.g. astryx-button-ghost, astryx-button-secondary)
  • Props that drive visual styles must be included in themeProps. If a prop selects between different StyleX style objects (e.g. statusStyles[status]), that prop must appear in the themeProps call. Look for fooStyles[prop] in stylex.props() and verify prop is in the corresponding themeProps() call.
  • Visual variation props must use the variant map pattern for extensibility. When a prop drives visual styles (e.g. status, color, variant), the style lookup should use a variant map registered via defineTheme's variants field — not a hardcoded Record<KnownValue, StyleXStyles>. This lets theme packages add new values (e.g. a custom Banner status) without modifying the component. The pattern:
    • Component declares a default variant map: const statusStyles = { info: {...}, warning: {...}, error: {...} }
    • Theme extends it via defineTheme({ variants: { bannerStatus: { custom: {...} } } })
    • Component reads the merged map at runtime, so unknown values from themes just work
    • If a visual prop uses a hardcoded lookup that would break on unknown values, flag it for migration to the variant map pattern (#759)
  • Composed sub-components get their own component's className, not the parent's

Sub-element targeting: Visually distinct sub-elements need their own themeProps when they have their own color, background, or border distinct from the parent (e.g. switch thumb, radio dot, progress bar fill). Structural-only wrappers do not.

Common mistakes from past audits:

  • Placing themeProps on positioning wrapper instead of visual container (#749, #744)
  • Hardcoded shadow strings with only color tokenized (#746, #670)
  • Adding themeProps to composition wrappers that wrap other themed Astryx components (#749, #672)
  • Visual props missing from themeProps call (#808)
  • Hardcoded variant style maps that reject unknown theme values — use extensible variant map pattern (#759)

4. Component Reuse (Theming + API)

Components should compose from core primitives so theming cascades correctly and the API contract is consistent.

Check for:

  • Close buttons should use Button (ghost variant)
  • Dividers should use Divider for standalone structural elements. For repeated siblings, prefer borderBlockEnd with colorVars['--color-border'] + :last-child suppression
  • Icons should use Icon with the registry — not inline SVGs
  • Skip compositions that wrap other themed components — adding themeProps to the wrapper creates specificity conflicts. Flag for human review instead.

5. Prop Naming (API)

All props must follow naming conventions.

Check for:

  • Booleans missing is/has prefix (e.g. disabledisDisabled)
  • Callbacks not following on{Verb}{Scope?} pattern
  • Primary change callback named something other than onChange
  • Visibility callbacks not using onOpenChange pattern
  • Directional props using left/right instead of start/end
  • Uncontrolled defaults not using default prefix (e.g. initialValuedefaultValue)
  • Boolean defaults not preserving is/has after default (e.g. defaultOpendefaultIsOpen)

Exceptions: HTML attribute passthroughs (onClick, onBlur) and html-prefixed collision avoidance (htmlName, htmlFor) are fine.

6. Type & Context Naming (API)

Check for:

  • Props types not named <Component>Props
  • Variant/status types not named <Component>Variant / <Component>Status
  • Contexts not named <Component>Context
  • Public hooks: component-scoped hooks use the use<Component> form (e.g. useDialog, useTheme). Generic utility hooks (useFocusTrap, useListFocus, useMediaQuery) follow the same unprefixed convention.

7. Component Structure (API)

Check for:

  • Missing displayName on exported components (prevents minification from losing names in prod)
  • Missing file header with @file, @input, @output, @position
  • Components not extending BaseProps
  • Missing xstyle, className, style escape hatches — these must come from BaseProps (or Pick<BaseProps, ...>), never re-declared manually. When adding escape hatches to a component that doesn't have them, extend the base type — don't copy-paste the three prop declarations.
  • Overlay/layer components (HoverCard, Popover, Dialog, Tooltip) pass style props through to the layer's render function (ContextRenderProps), NOT via a wrapper <div>. The layer system already handles style application on the correct DOM element. Never wrap content in a new <div> just to apply consumer styles — it breaks layout context and flex/grid inheritance.
  • Not using mergeProps() for style merging
  • Missing 'use client' directive on files using React client APIs. Run node scripts/check-use-client.mjs to verify.
  • Public hooks: must have file header, 'use client' if using client APIs, and follow standard hook patterns (return type, options object for complex args)
  • Components that extend BaseProps must capture ...rest, forward it, and resolve prop collisions deliberately. Extending BaseProps only makes pass-through props (data-testid, aria-*, id, role, tabIndex, event handlers) type-check — it does not wire them to the DOM. Two failure modes, both invisible to tests and CI:
    • Dropped entirely. The component destructures a fixed set of named props with no ...rest (or captures ...rest but never spreads it) → every pass-through attribute vanishes. Detection: props extend BaseProps (or Pick/Omit of it) but the destructure has no rest element, or ...rest is never spread onto the primary rendered element (the one carrying themeProps / the interactive role, not a wrapper). Category: structure-issue.

    • Clobbered by collision. {...rest} is spread but a prop the consumer passed collides with one the component sets. The fix is not one rule — it is per prop category. Verify each collision is handled by the right policy:

      Prop category Correct policy What to check
      className / style / xstyle combine merged via mergeProps(), not overwritten by a bare spread
      contract props (role, owned aria-*, computed id, type) component wins set after {...rest} on the element (a {...rest} placed after role="radiogroup" lets a consumer clobber the computed role → flag)
      handlers the component also implements (onClick that selects, onKeyDown that navigates) compose consumer handler destructured out of ...rest and combined via composeEventHandlers(); a component that spreads {...rest} then sets its own onClick after it drops the consumer's (bug #3852 fixed for SegmentedControlItem) → flag
      neutral pass-throughs (data-testid, other data-*, aria/handlers the component does not implement) forward carried through by {...rest}
    • Do not flag a {...rest} placed before contract props as wrong — that is the correct placement for the component to win. Only flag when an owned handler or contract prop can be clobbered by the consumer, or when a handler the component implements is not composed with the consumer's.

    • See API Conventions#capture-and-forward-rest and API Conventions#precedence-combine-compose-or-clobber. Fixed/added in #3738 (CheckboxInput), #3852 (SegmentedControl), #3863 (composeEventHandlers + SegmentedControlItem onClick).

  • Composed components must not let {...rest} overwrite explicit props. When a component delegates to another Astryx component, every prop set explicitly on the inner component must be destructured from the rest spread and properly merged (className concatenated, xstyle appended to array, event handlers composed via composeEventHandlers, style forwarded). See API Conventions#composed-component-prop-forwarding.

8. Input Component Consistency (API)

For input components only:

Check for:

  • Missing required field props: label, value, onChange/onChangeAction
  • Missing standard optional props: isLabelHidden, description, isOptional, isRequired, isDisabled
  • status prop not following {type, message?} shape
  • size variants not using 'sm' | 'md'
  • Missing isLoading / isBusy for async-capable components
  • hasAutoFocus without data-autofocus: Components that accept a hasAutoFocus prop and set autoFocus={hasAutoFocus} on a native element must also set data-autofocus={hasAutoFocus || undefined} on that same element. This allows Dialog to re-focus the element after showModal() (React's autoFocus fires before the dialog is visible, so it silently fails). Detection: find autoFocus={hasAutoFocus} without a corresponding data-autofocus on the same element. See PR #1044.

9. Accessibility Contract (API)

Check for:

  • Interactive components missing label prop
  • Missing aria-required, aria-invalid, aria-describedby wiring on inputs
  • isDisabled not mapping to native disabled attribute
  • Busy state using native disabled (should use aria-busy + visual treatment only)
  • Icon-only buttons missing aria-label
  • Focus not preserved during state transitions

10. Export Hygiene (API)

Check for:

  • Components missing from src/index.ts re-exports
  • Missing type exports (props, variant, status types)
  • Component not registered as separate entry point in tsup.config.ts
  • Public hooks missing from src/hooks/index.ts and src/index.ts

11. Design Conventions (Design — objective checks only)

The Design Conventions "smells" that can be verified from source without visual judgment. These map to the 🟢 (automatable) rows of that page's evaluatable table. Do not audit anything that needs a human eye (proportions, state treatment, density, composition) — that stays in Hardening L3.

Check for:

  • Off-grid spacing — padding/margin/gap values not on the 4px scale (--spacing-*, 2px half-steps); no raw px where a spacing token exists
  • Inverted nesting — a child element's gap wider than its parent's gap (grouping reads backwards)
  • Non-concentric radius — a nested element where r_inner ≠ r_outer − gap; radius values off the documented role scale (--radius-inner/element/container/page/full)
  • Contrast + overlay tokens — no #000/#fff literals (use tinted neutrals); interaction/selection tints use alpha overlay tokens (--color-overlay-*), not opaque fills; status never conveyed by color alone (must pair an icon)
  • Type hierarchy — role tokens over raw sizes; no body text below 12px; line-height ≥ 1.3 on multi-line body; no letter-spacing above 0.05em on body text
  • Motion safety:hover guarded by @media (hover: hover); prefers-reduced-motion honored; transitions/animations target transform/opacity, not layout properties (width/height/margin)
  • Reorder state contract — ordered collections start from a dedicated handle; pointer previews move freely while the collection stays stable; placement uses a tokenized insertion cue; the mutation commits once on drop; keyboard reordering exposes the same cue and announcements; reduced motion skips settling animation; ghost emphasis does not lower text/control contrast below AA; no raised-card shadow or magic z-index
  • Stacking — no overflow: hidden/clip on an ancestor of a popover/menu/tooltip that must escape; z-index follows the semantic layer order, not magic numbers

Nightly Checklist

This role runs once per night and produces one PR covering all findings.

Step 1: Check if Already Run Today

Check memory/xds-night-watch-state.json for componentAuditor.lastRunAt. If it's from today (PST), skip and NO_REPLY.

Step 2: Two-Pass Component Selection

The auditor runs two passes per night with separate PRs to keep hardening-issue work distinct from regular queue progress.

Pass 1 — Hardening Issues (priority)

  1. List open hardening issues:
    gh issue list --repo facebook/astryx --state open --label hardening \
      --json number,title,body --limit 50
  2. Extract component names from single-component issues (e.g. "Hardening: Toolbar" → Toolbar)
  3. Already audited? If the component is in completedComponents (state file), close the issue immediately — no re-audit needed:
    gh issue close <number> --repo facebook/astryx --reason completed \
      --comment "Component previously audited by Night Watch. See audit PRs in component history."
  4. For remaining components: audit them (Step 3) and create a separate PR (branch: navi/hardening-issues/YYYY-MM-DD)
  5. Add all audited components to completedComponents in state.
  6. Do NOT close hardening issues that have a new PR — the PR itself is the resolution. The issue closes when the PR merges (via GitHub's linked issues or the PM's gardening pass).
  7. For batch hardening issues (e.g. "Hardening: Navigation Components"): read the issue body, extract all component names listed. If ALL are now in completedComponents, close with a comment listing which audit PRs covered them. Otherwise skip — it'll close once the last component is done.

Pass 2 — Queue (regular rounds)

  1. Check memory/xds-night-watch-state.json for componentAuditor.auditQueue
  2. If the queue is empty, scan packages/core/src/ for all component directories and build a fresh queue
  3. Pick the next 5 components from the queue
  4. Dedupe: skip any that were already audited in Pass 1 this run
  5. Audit them (Step 3) and create a separate PR (branch: navi/component-audit/YYYY-MM-DD)
  6. Update state: move completed to completedComponents, advance queue

Step 3: Audit the Components

For each component, run all checks (sections 1-10 above) in a single pass:

  1. Read the source files — component, sub-components, types, styles, index.ts
  2. Check theming: CSS variables, themeProps, component reuse
  3. Check API: prop naming, type naming, structure, composition, input consistency, a11y, exports

Step 4: Report Findings

For each issue found, log to memory/xds-night-watch/{date}.md under a ## Component Audit section:

  • Categorize as: hardcoded-value, missing-classname, classname-wrong-element, naming-violation, missing-type-prefix, structure-issue, composition-violation, input-inconsistency, a11y-gap, export-gap
  • Include file path, line number, current code, and what the fix should be

Step 5: Fix Findings — Two PRs Per Night (max)

Create separate PRs for each pass:

  • Pass 1 branch: navi/hardening-issues/YYYY-MM-DD
  • Pass 2 branch: navi/component-audit/YYYY-MM-DD

For each PR:

  • Run pnpm test and pnpm build before creating the PR. Fix any failures.
  • Publish PRs ready for review (not draft) — label with hardening
  • When a finding needs human judgment, include it under a Needs Review section in the PR description
  • If no issues found for a pass, do not create a PR — just update state and close issues

Step 5b: Refinement Loop — Drive PR to Green

After creating the PR, don't wait for the daytime PR Review job. Run the refinement loop immediately:

  1. Record the HEAD sha you just pushed
  2. Schedule a one-shot recheck 5 minutes out (to let CI start)
  3. On recheck: a. SHA collision check: git fetch origin <branch> && git log -1 --format=%H origin/<branch>. If HEAD sha differs from what you recorded → STOP (someone else pushed, don't clobber their work) b. Check CI: gh pr checks <number>. If all green → done, log success c. If failing: Read the CI failure logs (gh run view <run_id> --job <job_id> --log-failed). Fix the failures in the worktree, run pnpm test && pnpm build locally, commit, push. Record the new HEAD sha. d. Schedule another recheck 5 minutes out
  4. Iteration limit: max 3 fix attempts per PR. If still failing after 3 → log it and move on. The daytime PR Review job will pick it up.

Key rules:

  • Always check SHA before acting — if it changed, someone else is working on it
  • Only fix CI failures (test, lint, build) — don't re-audit or expand scope during refinement
  • Each recheck is a one-shot scheduled job, not a busy-wait
  • Log each iteration to memory/xds-night-watch/{date}.md

Step 6: Update State

Update memory/xds-night-watch-state.json:

{
  "componentAuditor": {
    "lastAuditedComponent": "Button",
    "auditQueue": ["Card", "Dialog", ...],
    "prsFiled": ["#650", "#800"],
    "lastRunAt": "2026-03-22T04:00:00Z",
    "completedComponents": ["Button", "TextInput"]
  }
}

When the queue is empty, clear completedComponents and rebuild from scratch.


Does NOT Do

  • Subjective design decisions — only objective checks (the objectively-checkable Design Conventions in dimension 11 are in scope; visual judgment — proportions, state treatment, density, composition — stays in Hardening L3)
  • Review PRs (that's Reviewer)
  • Fix CI beyond the refinement loop (that's QA)
  • General issue triage (that's PM) — only closes hardening-labeled issues after auditing

State Schema

{
  "componentAuditor": {
    "lastAuditedComponent": "string",
    "auditQueue": ["string[]"],
    "prsFiled": ["string[]"],
    "lastRunAt": "ISO timestamp",
    "completedComponents": ["string[]"]
  }
}

Clone this wiki locally