Copilot's seven review comments on #52 all hold, and they share a cause nothing would have caught

[Gnuk]

Copilot left seven review comments on #52. All seven are right — each was checked against the code rather than taken on trust. Two are defects with visible effects, five are missing accessible names or states.

But they share a cause that none of them names, and two of them undercount the problem. That cause is worth fixing alongside the symptoms.

The two functional defects

The dropdown ion never applies. styles/src/organism/combobox/combobox.mixin.pug:22 calls +ippon-dropdown({ id, alternative: 'options', … }), while dropdown.mixin.pug:2 destructures ion. So ion is undefined, ionClass falls back to ippon-dropdown---buttons, and the Pattern Library combobox keeps the action-panel gap and padding while the option-list width selector never matches. The word alternative appears nowhere in the dropdown mixin — this is a rename that did not propagate.

message={null} opens an empty popover. Both comboboxes compute:

const hasContent =
  props.options.length > 0 ||
  (props.placeholderRows ?? 0) > 0 ||
  props.message !== undefined ||
  props.footer !== undefined;

message and footer are declared ReactNode, and null is the ordinary way to say "render nothing" in React — {condition && <p/>} yields false, {condition ? <p/> : null} yields null. Testing against undefined alone makes both count as content, so an empty popover opens when options is empty. Affects IpponSingleCombobox.tsx:60-61 and :101, IpponMultiCombobox.tsx:69 and :109.

The five accessibility gaps, and what Copilot missed

Four unnamed controls, one unnamed listbox, one missing invalid state. Two things to add to Copilot's reading:

The library already documents the rule it breaks. styles/src/molecule/option-list/option-list.md:29 states: "Name the list through aria-labelledby, pointing at the same Label atom that names the field." The mixin implements it — option-list.mixin.pug:2 accepts labelledBy and renders aria-labelledby. But combobox.code.pug:24 omits it. The Pattern Library's own example contradicts the Pattern Library's own documentation, which is worse than an oversight: the example is what people copy.

There are four unnamed clickable ions, not two. Copilot flagged combobox.mixin.pug:12 and input-search.code.pug:8. A sweep of clickable: true finds two more, both in styles/src/atom/badge/badge.code.pug:10-11. And here too the option already exists: ion.mixin.pug takes label and renders aria-label. Nothing needed inventing — the callers simply do not pass it.

The checkbox disagrees with itself across implementations. react/src/IpponCheckbox.tsx:31 renders aria-invalid={variant === 'error' || undefined}. styles/src/atom/input-text/input-text.mixin.pug:4,13 does the equivalent. checkbox.mixin.pug changes the class only. Same component, same design system, two answers.

What actually lets this happen

None of these would have been caught, because nothing in the pipeline looks at rendered markup. pnpm lint:ci runs stylelint "src/**/*.scss" — SCSS only. styles/test/ holds Sass function tests and nothing else. No axe, no jsx-a11y, no snapshot of the Pug output; a grep for any of them comes back empty. The React side has 387 unit tests, the Pug side has none of its markup under test.

So the Pug mixins and the React components can drift apart indefinitely, and the only thing standing between a missing aria-label and a release is whether a reviewer notices. On #52, the reviewer that noticed was a bot.

What to decide

Whether the fix stops at the seven, or goes after the cause. Fixing the symptoms is an afternoon. A guard is a design decision, and the options differ a lot in cost: jsx-a11y on the React side only, an axe pass over rendered Pug in CI, or a set of markup assertions on the mixins. The last one is the only one that would have caught the alternative/ion rename.

Whether React and Pug should be checked against each other at all. They implement the same components and are allowed to differ today. If they are meant to agree on ARIA, that is a rule that can be written down and tested. If they are not, aria-invalid on the checkbox is a choice rather than a bug, and it should be documented as such.

Whether the empty-popover fix changes public behaviour. Treating null as absent is what the documentation implies, but a caller currently passing message={null} to force the panel open would lose that. Worth a look before it lands.

Scope note

The seven comments are on code already merged in #52. Nothing here is urgent, and nothing blocks anyone — this is cleanup plus a decision about what should have caught it.