Skip to content

Review 5109

Cindy Zhang edited this page Aug 27, 2026 · 1 revision

Review #5109 — CLI hook index completeness

cixzhang · the maintainer · MERGED · view on GitHub

PR

#5109 Document the seven core hooks missing from the CLI hook index, and fail the build when it goes stale by cixzhang (bucket: the maintainer)

HEAD REVIEWED

961c92cdca9baf2e537a68e36fdb331c1ac95abc

VERSIONS

LOOP VERSION: 1.5.0 AUDIT RUBRIC: 1.13

LANE

LANE: fast WHY: Docs plus one test-only derivation; zero runtime lines, no public API/default/state/effect/dependency/visual change, no conflict or competing PR, and one decisive export-to-discovery check settles the goal.

PROBLEM

WHY 1: Seven shipped hooks are exported from the public hooks barrel but absent from CLI discovery. WHY 2: A builder following the documented discovery workflow is told those primitives do not exist. WHY 3: The builder repeats solved work and can replace shared accessibility or focus behavior with an unreliable local version. USER-FACING PROBLEM: A builder searching Astryx for an existing primitive gets no result, does extra work, and risks shipping a weaker replacement. PROBLEM SEVERITY: harmful friction — current main returns NOT FOUND for all seven even though every hook is exported.

VERDICT: clear

SOLUTION

The CLI discovers a hook only when a documentation record sits beside it. This change adds the seven missing records and makes the discovery suite compare those records with the actual built export list. The check lives with CLI discovery, while each record stays with the hook it describes, so a future export without documentation fails immediately.

SOLUTION (2 decisions · 0 runtime lines of 997 changed lines)

  1. Add caller-facing records for the seven already-exported hooks — fixes the current discovery gap.
  2. Derive the completeness check from the built barrel, including export * forms — prevents the same drift.

BURDEN: low — documentation plus a test-only module import; no shipped state, effects, listeners, timers, dependencies, or public runtime surface. BURDEN MATCH: proportionate — seven records repair seven live misses, and one contract test prevents recurrence.

VERDICT: clear

ARCHITECTURE

OWNER: CLI hook discovery owns the searchable index; each hook owns its colocated documentation. TIER 1: existing hook barrel and discovery APIs reused. TIER 2: none. SEAMS: named re-exports and export * re-exports from the public hooks barrel. BEHAVIOR UNIT: pure utility — exportedHookNames imports one built ESM entry and is focused-tested against both export forms.

seam driven result
named barrel exports all seven current hooks are discovered
export * fixture useStarred is included alongside useNamed
component-owned hook docs remain outside the standalone barrel gate by construction

The implementation derives from the source of truth rather than maintaining a second inventory.

VERDICT: clear

IMPACT

Builders using astryx hook or astryx search can now find seven existing primitives; installed component behavior is unchanged. No sibling gap becomes newly visible: the current built barrel and discovery index are complete at this head.

VERDICT: clear

API

No public runtime API or signature changes. The CLI now returns documentation records for seven hooks that were already public through @astryxdesign/core/hooks.

OSSIFICATION: none — this restores discovery for existing surface and adds no new concept, export, default, or accepted value.

VERDICT: clear

THEMING

No targets, tokens, variables, StyleX, or rendered style contracts change; the changed paths are hook docs, a discovery test, and a changeset.

VERDICT: clear

BREAKING

BEHAVIOR: no — no component or hook implementation changes. API: no — existing exports and signatures are unchanged. VISUAL: no — no rendered output, DOM, styles, or stories change. THEME: no — no theme surface changes.

VERDICT: clear

PERFORMANCE & RESOURCES

EFFECTS: zero; the diff adds no runtime code.

RENDER: no reachable change. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no runtime dependency or code delta; the built-barrel import runs only in the discovery test.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: every changed path is a .doc.mjs, a Node discovery test, or a changeset; no runtime render, DOM, CSS, story, or visible endpoint changes.

VERDICT: clear

A11Y & I18N

The useAnnounce guidance matches the source's persistent empty live regions, polite/assertive modes, and automatic clearing. No runtime accessibility semantics, translated strings, locale behavior, or direction-sensitive layout change; typecheck:docs validates all seven records.

VERDICT: clear

JUDGEMENT

slot verdict
PROBLEM clear
SOLUTION clear
ARCHITECTURE clear
IMPACT clear
API clear
THEMING clear
BREAKING clear
PERFORMANCE clear
VISUAL clear
A11Y & I18N clear

GOAL: met — current main returns NOT FOUND for all seven; this head resolves all seven, the focused suite passes 11/11, and removing the records makes the gate fail with the exact seven-name inventory. DISPOSITION: no negative findings. ADVICE: omitted — the implementation already uses the built barrel as the source of truth. AUTHOR CAN PROCEED: yes — focused tests and typechecks are green; merge after required CI passes. WORST OUTCOME: none found → approve and merge.

JUDGEMENT NEEDED: none — documentation and a contract test for already-public hooks.

approve and merge

REVIEW

Thanks, this is good. The seven docs match their hooks, and the export-derived check fails on the exact current gap and passes when restored.

[Reviewed by Robohands]

INLINE (0–3, only when genuinely line-anchored)

None.

EVIDENCE I DID NOT SPEND

  • No formal prior review exists; the maintainer's direction comment confirms the problem, solution, and bounded scope.
  • useMergedRefs landed after this branch started and already has its own hook doc; the missing inventory remains exactly seven.
  • Critic pass: verdict correct; public review is 24 words before attribution, has no unsupported claim, and is ready to post.

TIME

TIME total 11m setup 2m dedicated worktree + fast install (warm main reused: no; no browser work needed) reading 5m current kit, rubric, PR, sources, docs, discovery code, and history measuring 3m current-main reproduction, positive/negative gate, CLI probes, typechecks writing 1m presentation + one critic pass waste 1m corrected one shell-list mistake and rebuilt a required typecheck dependency

WHAT I COULD NOT VERIFY

Nothing.

EVIDENCE AND TESTS

  • Current main: all seven exported hooks return NOT FOUND from discovery.
  • Exact head 961c92cdca9baf2e537a68e36fdb331c1ac95abc: all seven resolve; focused discovery test 11/11 passed.
  • Negative gate: removing the seven docs fails with the exact seven-name inventory.
  • Core source/docs and CLI authoring/strict typechecks passed; repository structural checks passed.
  • Required GitHub CI passed; non-applicable visual, RTL, and accessibility jobs skipped by scope.
  • Squash merge: d88306a553122f1bab1400c1f90585560be8a21f.

CRITIC

DRAFT: approve and merge. CRITIQUE 1: passes the fast-lane eligibility, current-head, current-main, caller-facing docs, derived-inventory, word-count, evidence, and public-language gates. The 24-word review has no unsupported finding or private control field. No rewrite required.

What changed before posting

Posted as drafted.

Clone this wiki locally