Skip to content

Review 5385

Cindy Zhang edited this page Aug 27, 2026 · 6 revisions

Review 5385 — holding the typeahead search until the query is long enough

#5385 · freddymeta · collaborator (not in ENGOWNERS/DESIGNOWNERS, but has write access and has merged his own PR — so approve-with-nits tone, no Discord clause) Head reviewed eb704474d77 — unmoved across all three gates Verdict request-changes · one block, drafted not posted, with Cindy Prior review none — reviews and comments both empty RFC #5384, filed alongside the branch

Problem

Type one letter into a Typeahead over a remote directory and the menu opens saying No results found, which is false — one character matched too much, not too little. A screen-reader user hears it before finishing the first syllable, and the source has been fetched once per keystroke. A SearchSource can already refuse the query, which stops the request; it cannot stop the menu, because performSearch calls showLayer() whenever searchQuery.length > 0 regardless of what came back (BaseTypeahead.tsx:473-475) and there is no controlled isOpen, no imperative handle, and onOpenChange only reports.

Solution — 2 decisions · 67 runtime lines of 267

  1. minQueryLength gates the search, the menu open and the live-region announcement together (:310 predicate, :549-561 gate).
  2. ArrowDown no longer falls back to bootstrap entries below the threshold (:700) — leave it out and the feature leaks through the keyboard, so this is nearer one decision than two.

Both trace to a stated problem; nothing piggybacked. The 200 non-runtime lines are 32 of docs, 143 of tests with their own positive controls, a story and a changeset. The author asked in chat whether a bot should trim it. It should not — this PR is the right size for its idea.

Impact

Nobody, until someone opts in: the default is 1 and isBelowMinQueryLength(q, 1) is always false, verified in Chromium against warm main. Builders who opt in stop showing a false empty state at one and two characters — and hit the stuck loading indicator below, which is why that is the block: the only audience the prop documents is the only audience that hits it. Below the threshold a screen reader is told nothing at all, which is deliberate and is the open question the RFC escalates rather than answers.

API

+ BaseTypeahead.minQueryLength?: number = 1   (public, core barrel)
+ Typeahead.minQueryLength?: number           (public, forwards)
+ Tokenizer.minQueryLength?: number           (public, forwards)

No removals, no signature changes, no new exported types. Doc coverage is uneven: Typeahead.doc.mjs's docsZh and docsDense carry no props array at all, and Tokenizer.doc.mjs's zh entry is the English string.

Theme targets

None added; none re-pointed.

Ossification

The class holds, with a landed member the PR under-sells. PowerSearch renders <Tokenizer searchSource={…}> (PowerSearch.tsx:1034), so PowerSearch → Tokenizer → BaseTypeahead. usePowerSearchSource already refuses a short query in the source — which is exactly the half-measure the problem describes: the source returns [], the layer opens anyway, the user gets the empty panel. PowerSearch has the defect today and is one forwarded prop from being fixed.

But the precedent cited for the NAME is a different concept. PowerSearchField.typeaheadMinQueryLength is "Minimum query length before this field appears in typeahead" (PowerSearch/types.ts:333) — a per-field relevance filter on the contents of a list. minQueryLength is a per-component gate on whether a search runs at all. Two names one token apart, two meanings, one package; and the collision bites in the direction that matters, because someone wanting this behaviour in PowerSearch sets the field-level one everywhere and still gets the empty menu.

Breaking

  • API no — additive, optional, default reproduces today exactly.
  • Visual no when unset (driven on two stories against warm main); when set, the menu holding shut is the feature and the stuck spinner is the block.
  • Theme no.

Far side of the bound: 0 and 1 are no-ops by construction; at 3 both the 2→3 and 3→2 transitions were driven, and 3→2 is where the defect is.

Performance

No new state, effects, subscriptions or listeners. One dep change — the keydown useCallback swaps query.length for query (:759), so the handler's identity churns on same-length edits too; it is spread onto one <input> and nothing memoizes on it. Render count not measured.

Judgement

1. [BLOCKS] the below-threshold branch resets results and hasSearched but not
   isLoading, so an in-flight performSearch fails its own generation check in
   `finally` (:492-493) and the loading flag is never cleared
   → on a remote source — the only case the prop documents — backspacing from
     three characters to two leaves a clock spinning in the field and a
     role="status" "Loading" in the a11y tree, until a third character is
     typed                                        · BaseTypeahead.tsx:553-559
   confirmed three ways: reading the guarded `finally`; two-sided in Chromium
   (today's path re-searches and clears, the gated path does not); and by
   mutation — adding setIsLoading(false) beside the existing resets clears it.
   PRE-EXISTING MECHANISM — clearing the field mid-search strands it on main
   too — but debounceMs defaults to 150, so this PR routes an everyday
   type-pause-backspace into it. Said in the comment, per R13.
2. [not blocking] the name collides with PowerSearchField.typeaheadMinQueryLength
3. [not blocking] the zh doc entry is in English · Tokenizer.doc.mjs:388
4. [not blocking] conflicts with main, 4 files

Finding 1 carries the verdict, alone. The prop itself is fine and the review says so.

What moved underneath the conflict

#5237 added menuWidth into the same props block · #5400 added a case 'Tab': to the same keydown switch he edits, the one hunk that wants reading rather than accepting · #5267 swapped mergeRefs for useMergedRefs · #5235 rewrote the option list around groupItems · #5435 consumed every changeset in the v0.5.0 release.

Visual evidence

Real Chromium at the PR head, storybook dev :6620, source resolving at 600 ms. Frames on assets/pr-5385 of cixzhang/astryx. Probe banked as probe-kit/typeahead-loading-strand.cjs.

frame
today — backspace "Appl""Ap" mid-search, search re-runs and resolves ungated__backspace.png
minQueryLength={3} — same keys, no menu, clock never stops gated__stranded.png
control — three characters, spinner clears, menu opens gated__resolved.png

The review as drafted (146 words)

Thanks for the RFC first — the shape looks right.

One thing to fix. Below the threshold nothing clears isLoading, so the in-flight search fails its own generation check in finally. Backspacing from three characters to two — on a remote source, the case the prop is for — leaves a clock spinning in the field, and role="status" "Loading" in the a11y tree, until a third character is typed. Main strands the same way when you clear the field mid-search; your branch adds a second route in.

searchGenRef.current++;
searchSource.cancel?.();
setResults([]);
setHasSearched(false);
setIsLoading(false);
interrupted mid-search search allowed to finish
stuck clean

Both at "Ap", after backspacing from four characters mid-search.

Needs a merge with main too — the keydown switch has moved under you.

One naming note I've put to a maintainer rather than to you: typeaheadMinQueryLength on a PowerSearch field means "before this field appears in the list" — a different thing.

Can you add that line and re-push?

Inlines: BaseTypeahead.tsx:556Might need setIsLoading(false) here too — the in-flight search can no longer clear it. · Tokenizer.doc.mjs:388This one's still English in docsZh. BaseTypeahead's is translated.

The one question for the maintainer

Does minQueryLength keep that name, given typeaheadMinQueryLength already means something else one directory away? Everything else is settled.

What the gates changed

  • Gate 1 → 2. 175 words against the 150 cap. The pre-existing half of the defect was established in the private brief and dropped from the comment (R13) — the author would have found main doing the same in ten seconds and read the review as an accusation. One anchor (PowerSearch.tsx:1061) read off origin/main instead of the PR head (R14e). Merge archaeology in the comment (R11). Praise in an inline on a request-changes.

  • Gate 2 → 3. The comment passed — "post it as written" — and four hand-off claims failed: "doc'd EN + zh + dense on all three" is false for Typeahead.doc.mjs; the CI line named checks that do not exist on this head and called it green when the combined status is pending on review-required; an evidence claim broadened during the cut; and the naming clause compressed past usefulness with 17 words of headroom unspent. The i18n finding was found on this pass, having been called clean twice.

  • The pattern across both: the confidence gate was being run on the words that ship and not on the private brief she reads first.

  • Gate 3 — clean. "VIOLATIONS: none. WOULD SHE HAVE POSTED IT: yes — as written." Anchors re-opened at the head; head unmoved.

What Cindy changed before posting

(not yet posted — awaiting her read)


Re-review — 2026-08-25

Versions: Review Loop unversioned (pre-1.0) · Component Audit Rubric 1.10
Head: 57cde40f · Verdict: request changes

Problem. The prior loading-state strand is fixed, but minQueryLength still disables Tokenizer's independent hasCreate capability below the threshold.

Evidence. A 600 ms remote search clears loading immediately after backspace 3→2. With hasCreate, QA + Enter creates normally but silently does nothing with minQueryLength={3}.

Review as posted:

Thanks for fixing the stranded loading state — I re-ran the 600ms remote search, backspaced 3→2, and loading now clears immediately and stays clear after settlement.

One API interaction remains: with hasCreate, QA + Enter creates a token by default, but minQueryLength={3} makes Enter do nothing. minQueryLength is a search threshold, so it should not suppress the separate create capability.

Current main also fixes this head’s visual-gate failure; the merge added unrelated DateInput formatting. Could you keep creation available below the threshold, add that Tokenizer combination test, refresh main, and drop the DateInput noise?

[Reviewed by Robohands]


Round 3 — 2026-08-26

Versions: Review Loop 1.4.0 · Component Audit Rubric 1.12 Head: 94785a3a2d31d9bea0e596d2ee58f65f6d4a5d10 (fail-closed against the requested SHA before the worktree was cut; unmoved through the critic pass) Lane: full — two standing CHANGES_REQUESTED of ours, new public API, a changed visible endpoint, and more than one decisive check Verdict: request changes · 1 block · drafted, not posted

(Round 2's section above is headed 2026-08-25; the review itself was submitted 2026-08-26 04:47 UTC on 57cde40f. Left as written, noted here for the timeline.)

What the author pushed

94785a3"Keep Create available below minQueryLength, and refresh main". The Create entry moves out of the search source and into a new queryEntries callback on BaseTypeahead, so it is assembled from the typed text rather than fetched for it — and therefore is not held back by the threshold.

Every prior ask, re-checked at this head

round ask at 94785a3
1 clear isLoading on the below-threshold branch fixed (BaseTypeahead.tsx:601, and :666 in handleSelect)
1 should Create sit outside the gate? answered and implemented
1 merge with main done; main has not touched Typeahead/Tokenizer since the merge-base
1 inline Tokenizer.doc.mjs zh entry still English fixed
2 keep creation available below the threshold fixed
2 add the Tokenizer combination test doneTokenizer.test.tsx:640, :679, :708
2 refresh main · drop the DateInput noise done

Nothing stale is carried forward. Both findings below are about code that did not exist when either earlier review was written.

Problem

With the threshold set, a Tokenizer's hasCreate stopped working: typing QA and pressing Enter added a tag before, and did nothing with minQueryLength={3}. A person could no longer create a short tag at all, with nothing on screen saying why.

Solution — 3 decisions · 144 runtime lines of 483 added

  1. minQueryLength gates search, menu open and announcement together
  2. ArrowDown does not fall back to bootstrap entries below the threshold
  3. new: text-derived entries are assembled outside the search source and are not gated — a queryEntries seam on BaseTypeahead

Decision 3 is recorded in the changeset and not in the PR body, whose own table still reads 1–2 → no search, no live-region announcement, menu closed.

API

+ BaseTypeahead.minQueryLength?: number = 1              (public, core barrel)   ok
+ Typeahead.minQueryLength?: number                      (public, forwards)      ok
+ Tokenizer.minQueryLength?: number                      (public, forwards)      ok
+ BaseTypeahead.queryEntries?: (query, results) => T[]   (public, core barrel)   finding

Ossification

minQueryLength earns it — three components take it in this diff, and PowerSearch → Tokenizer → BaseTypeahead is the landed consumer.

queryEntries does not, yet. One member: Tokenizer. <BaseTypeahead on main appears only in Tokenizer.tsx and Typeahead.tsx, and Typeahead has no create affordance. Nothing else in packages/core/src takes a caller-supplied callback that returns entries — the render-function props on this family (renderItem, renderToken, renderOption, renderValue) all take data the component has and return a node. It also has no .doc.mjs entry anywhere.

This was not escalated as a maintainer decision, deliberately: R1g-surface's own table already decides a speculative one-member class ("not yet, wait for the second real case"), and the repo already ships the remedy — DefinedTheme.__inputTokens / __onDark / __onLight (packages/core/src/theme/defineTheme.ts:395, 402, 407) is a __ prefix plus @internal on a publicly exported interface. stripInternal is not set in any tsconfig, so the prefix is the part doing the work. Applying an existing rule is not setting a precedent, and the direction chosen is the reversible one.

Theme targets

None added, none re-pointed. Structural-only grep over the runtime diff: colour/style/stylex/themeProps hits 0, aria-/role=/catalog-string hits 0.

Breaking

  • API — no. Both props optional and additive; minQueryLength = 1 reproduces today exactly. tsc --noEmit clean.
  • Visualyes, one, on the default path. A Tokenizer with hasCreate over ≥ maxMenuItems matches gains an entry: 14 matches at the default maxMenuItems: 10 give 10 options ending Match 10 on main and 11 ending Create "Match" here. Growth, so this is not the low-risk class.
  • Theme — no.
  • Behaviour — empty state unchanged (length 0 is deliberately not "below the minimum"); loading improved and round 1's strand fixed; error, disabled and controlled/uncontrolled not reachable; both directions across the bound driven.

Performance

Zero effects added, zero listeners, zero observers, no forced-reflow call, no dependency. Two useCallbacks gain deps and one swaps query.length for query, so their identity churns per keystroke — both go straight to the <input> and nothing memoizes on either. Render passes not counted, and no degradation is claimed.

Visual evidence

VISUAL CHECK: manual frames required
WHY: pr-visual is green on this exact head (it was failing on 57cde40; the main
     refresh is what fixed it) — but no story covers the changed endpoint. The
     only story this PR adds is a Typeahead with no hasCreate, and no Tokenizer
     story sets minQueryLength at all. Green is not coverage.

One decisive pair, captured with sensor receipts. Every sensor matches across the arms except Build — story, globals, theme neutral, colour mode light, direction ltr, viewport 520×420@1, media, target count 1, semantic state {query:"QA", focused:true, searchCalls:0}, geometry, fonts, errors. The searchCalls: 0 assertion is what makes "no fetch happened" a fact rather than a race: the probe waits 1200 ms, past the 150 ms debounce plus a 600 ms source.

The before arm is the round-2 head's two runtime files checked out into the same worktree, and its diff was read hunk by hunk, not just SHA-checked: 99 lines across 2 files, every hunk the Create path, leaving minQueryLength, isBelowMinQueryLength, setIsLoading(false) and the ArrowDown gate intact.

before — menu closed, Enter inert after — Create "QA" offered and committable
before after

Both opened and looked at. The field's box is pixel-identical between them; the menu is the only difference. Published with both sensor receipts and their checksums under assets/pr-5385/; every URL above was checked anonymously, without credentials.

A11y

pr-a11y and pr-rtl green on this head, and .github/a11y-baseline.json is untouched — no silence bought. Driven in Chromium: the combobox pattern holds below the threshold (aria-expanded false → true, aria-activedescendant resolves to the option, role="option" inside role="listbox"); Enter commits the created token; ArrowDown opens the derived menu without falling back to bootstrap entries; and round 1's stranded role="status" "Loading" is gone — present in flight, clear 250 ms after the 4→2 backspace and still clear after the abandoned search settles. No new string, no direction change.

Judgement

GOAL: met. Two-sided in real Chromium at 600 ms: at this head QA offers Create "QA", Enter commits, and the source is never called; with the create path reverted in the same worktree the same keys leave the menu shut and Enter inert. The author's own new test is a genuine positive control — it fails on the previous head's runtime files and passes here.

AUTHOR CAN PROCEED: yes. Two outcome-shaped criteria, no decision withheld: queryEntries is not consumer surface, and the body says what a hasCreate Tokenizer does below the threshold and at the maxMenuItems boundary.

WORST OUTCOME: "the next person reading BaseTypeaheadProps finds an entry-injection hook that no doc explains, and once 0.5.x ships we cannot take it back." Compatible with request-changes; not with approve. No slot describes a user who is stuck, cannot reach something, hears nothing, or loses their place.

1. [BLOCKS] queryEntries lands on the publicly exported BaseTypeaheadProps
   → whoever reads that type next finds an entry-injection hook with one
     in-package caller and no doc entry, and after the next cut we can only
     remove it with a breaking change                    · BaseTypeahead.tsx:164
   confirmed two ways: grep of the whole tree for another member (0), and
   walking the export chain to the barrel.

2. [not blocking] the Create row is appended after the maxMenuItems slice
   → on the default path a Tokenizer with hasCreate over 14 matches offers 11
     options where main offered 10 and dropped Create — nobody opted in, and
     the body says every existing call site renders identically
                                                          · BaseTypeahead.tsx:504
   confirmed by mutated arm in one worktree.

Finding 1 carries the verdict alone. minQueryLength is fine and the create fix is right; the review says both.

Still open, carried not re-raised

minQueryLength versus PowerSearchField.typeaheadMinQueryLength, which means "before this field appears in the list" — a different concept one directory away. Raised in round 1 and put to a maintainer rather than to the author; round 2 dropped it. Re-blocking on it now would be a silent re-escalation. It is also not urgent: @astryxdesign/core published 0.5.0 and this PR is unmerged, so a rename before the next cut costs nothing.

The review as drafted (131 prose words)

Thanks — the create fix is right, and derived-not-fetched is the right shape. Drove it at 600ms: QA offers Create "QA", Enter commits, the source is never called, and the 3→2 backspace still clears loading.

before after
before after

"QA" in both, with hasCreate and minQueryLength={3}.

One thing before it lands. queryEntries sits on BaseTypeaheadProps, which the core barrel re-exports, so it ships as public API at the next cut with Tokenizer as its only caller and no BaseTypeahead.doc.mjs entry. Keep the mechanism, keep it off the contract — DefinedTheme.__inputTokens (defineTheme.ts:395) is how we carry a prop that has to live on an exported type.

Worth a body line too: with hasCreate and no threshold, the Create entry now survives the maxMenuItems cut — 11 options where main showed 10.

Can you re-push with that?

[Reviewed by Robohands]

Inlines: BaseTypeahead.tsx:164This ships on the public barrel. DefinedTheme.__inputTokens is how we carry an internal prop on an exported type. · BaseTypeahead.tsx:504Appending after the slice means maxMenuItems={10} now renders 11 with hasCreate. Worth saying in the body.

No [Full review] link and no Discord clause — collaborator bucket.

What the critic pass changed

  • Four anchors had drifted, one of them inside the posted text (defineTheme.ts:393:395). All re-opened at the head and the line's own text pasted into the draft.
  • A grep asserted before it was run — "no Tokenizer story sets minQueryLength". Run: 0 hits, so the claim survived, but it was unearned.
  • A visual sentence with no frame. Finding 2 read "14 matches now render 11 rows"; the run is fixed at one decisive pair and that pair belongs to the create endpoint. Reworded to the option count actually measured.
  • The frames were listed by path, not embedded. Added to the review block. The critic made publication a posting precondition; that precondition is now discharged — both frames and both receipts are published under assets/pr-5385/ on this wiki and every embed resolves anonymously, so the review text above is postable as it stands.
  • The runtime-line ratio was estimated; replaced with the measured +144/−30.
  • Three notes tracing to one mechanism collapsed into one finding.

What Cindy changed before posting

(not posted — this run was read-only on the PR; the wiki record is the only write it made)


Round 4 — 2026-08-26

#5385 · freddymeta · collaborator (write access; not in ENGOWNERS/DESIGNOWNERS) Head reviewed: 5067e1039408e561ad41416708345eab82119b74 Incremental from: 94785a3a2d31d9bea0e596d2ee58f65f6d4a5d10 Versions: Review Loop 1.5.0 · Component Audit Rubric 1.13 Lane: full — our Round 3 CHANGES_REQUESTED is still open, the PR adds public API and visible behavior, and both prior asks need independent verification. Verdict: approve (drafted; no public PR action)

We asked for two things in Round 3: keep the derived-entry seam off the consumer contract, and document that hasCreate can put one Create option above maxMenuItems. This round checks each ask at the current head rather than reopening settled design.

Round 3 ask Current-head result
Make queryEntries internal Satisfied. It is now __queryEntries, carries @internal, and follows the landed DefinedTheme.__inputTokens convention for internal wiring on an exported type (BaseTypeahead.tsx:164-171). No unprefixed queryEntries reference remains.
Document the maxMenuItems behavior Satisfied. The PR body, changeset, English/Chinese/dense Tokenizer docs, and a focused test now state that the cap counts search results and Create may add one more (Tokenizer.doc.mjs:133-146, :393-406, :530-532; Tokenizer.test.tsx:732-767).

PROBLEM

WHY 1: A remote typeahead searches every non-empty query and can announce “No results found” before the query is specific enough to search. WHY 2: A person hears or sees a false result while still typing, while the builder pays for requests they meant to suppress. WHY 3: Search controls over large directories become noisy and look broken at the exact point where users are trying to narrow them. USER-FACING PROBLEM: People searching a large source get premature requests and false empty feedback, and builders have no composition seam that closes both the request and menu paths. PROBLEM SEVERITY: harmful friction — the RFC demonstrates the missing control with external precedents and 36 migration call sites; the task remains possible, but the feedback is false and the request cost repeats per keystroke.

VERDICT: clear

SOLUTION

The search owner declines queries shorter than an opt-in threshold, closes the menu, clears abandoned loading state, and withholds result announcements. Tokenizer derives its Create option separately so a short value can still be created without fetching. The final push changes only the standing of that derived-entry seam and documents/tests the already-reviewed result-cap behavior.

SOLUTION (3 runtime decisions · ~144 runtime lines of 536 added)

  1. Gate search, menu opening, and announcements on one threshold.
  2. Preserve the threshold across ArrowDown and abandoned-search state transitions.
  3. Derive Create entries outside fetched results so creation remains available below the threshold.

BURDEN: medium — one public prop across three surfaces, existing debounce/generation state, one internal callback seam, and focused component/browser coverage; no new Effect, listener, observer, dependency, or global coupling. BURDEN MATCH: proportionate — the RFC demonstrates a repeated remote-search class, while the smaller existing source-level workaround cannot control the menu or announcement.

VERDICT: clear

ARCHITECTURE

OWNER: BaseTypeahead owns search, menu-open, result-announcement, and cancellation state; Typeahead and Tokenizer forward the public threshold. TIER 1: existing SearchSource, popover, announcement, and generation-control mechanisms are reused. TIER 2: none. SEAMS: Tokenizer’s Create path stays inside the package as __queryEntries; Typeahead has no create affordance and does not receive it. BEHAVIOR UNIT: inline plus pure predicate — the threshold extends BaseTypeahead’s existing search state machine, while isBelowMinQueryLength isolates the boundary. Extracting handlers would split ownership without a reusable contract; DOM-level tests are the correct boundary because the contract is the combined search, popover, and ARIA state.

seam driven result
Typeahead threshold crossing below 3 stays closed; at 3 searches and opens (prior round, unchanged here)
Tokenizer + hasCreate below threshold QA offers Create, Enter commits, input clears
Tokenizer + maxMenuItems={3} 3 fetched results plus one Create option
abandoned in-flight search prior 3→2 loading clear remains covered by the passing affected suites; the current delta does not touch that branch

VERDICT: clear

IMPACT

Existing callers keep the default threshold of 1. Opted-in builders avoid premature fetches and false empty announcements; Tokenizer users retain short-value creation. The current push changes no runtime outcome from Round 3: both browser frames are byte-identical across the reviewed and current heads.

VERDICT: clear

API

<Tokenizer
  label="Tags"
  searchSource={source}
  value={tags}
  onChange={setTags}
  minQueryLength={3}
  hasCreate
  maxMenuItems={3}
/>
change public? class doc’d? verdict
+ BaseTypeaheadProps.minQueryLength?: number = 1 yes remote Typeahead/Tokenizer searches Base docs ok
+ Typeahead.minQueryLength?: number yes forwards the same threshold en docs + story ok
+ Tokenizer.minQueryLength?: number yes forwards the same threshold en + zh + dense ok
~ queryEntries__queryEntries + @internal internal convention on exported type one in-package Tokenizer caller source-only by design ok

OSSIFICATION: minQueryLength serves the demonstrated remote-search class and matches the component-owned decision the source cannot make. The one-caller derived-entry seam is now explicitly internal using a landed convention, so it no longer asks consumers to depend on an unfinished class. Removing the accidental public spelling before release is the reversible choice.

VERDICT: clear

THEMING

No target, variable, token, StyleX rule, or painting element changes.

VERDICT: clear

BREAKING

BEHAVIOR: no lost path or changed default for the threshold; with hasCreate, a full result menu intentionally gains one Create option above the result cap. API: no breaking change — minQueryLength is additive/optional with default 1; the unreleased accidental seam is narrowed to the repo’s internal naming convention. VISUAL: one additional menu row is possible on the existing hasCreate full-results path; it is now explicit in body/docs/changeset and pinned by test. Old/current incremental frames are byte-identical. THEME: no.

VERDICT: note — hasCreate may render one more option than maxMenuItems; this is an explicit result-cap contract, not an undocumented regression.

PERFORMANCE & RESOURCES

EFFECTS: zero added or changed; the existing popover-anchor Effect is untouched. RENDER: the current push changes names/docs/tests only; identical browser states and pixels at old/current heads. The full PR adds one O(1) derived-entry callback invocation per search or below-threshold input change. LISTENERS/OBSERVERS: none added. LAYOUT: no reads, writes, or style changes. BUNDLE: no dependency; exact-head CI build is green.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: manual frames required. WHY: exact-head pr-visual is green, but its default stories do not type into the hasCreate × minQueryLength or full-results maxMenuItems endpoints. Two receipt-backed Chromium comparisons cover those states.

Below-threshold Create survives the internal rename

Reviewed head 94785a3 Current head 5067e103
Reviewed head: Create QA offered Current head: Create QA offered

maxMenuItems={3} means three results plus Create

Reviewed head 94785a3 Current head 5067e103
Reviewed head: three results plus Create Current head: three results plus Create
sensor reviewed head current head match?
Build 94785a3d… 5067e103… expected difference
Story/state 1 core-tokenizer--creatable; QA; focused; expanded; one Create "QA" option same yes
Story/state 2 core-tokenizer--creatable-with-search; i; focused; expanded; Alice/Bob/Charlie + Create "i" same yes
Theme/mode/direction neutral · light · LTR same yes
Viewport/media 520×420@1 · forced colors off · reduced motion off · fine pointer/hover same yes
Geometry/readiness/errors one visible 382×20 input · fonts loaded · no animation · no Storybook/page error same yes
PNG SHA-256 create aaaca0dd…; cap 531cf252… identical yes

Receipts: old Create · current Create · old cap · current cap.

Every pixel is identical across the incremental delta. The intended visible behavior is present at both heads; no new visual difference needs design judgment.

VERDICT: clear

A11Y & I18N

Exact-head pr-a11y and pr-rtl succeeded, and the PR does not edit the a11y baseline. The report still lists the existing Tokenizer contrast baseline; this diff changes no style or color. Chromium shows a focused combobox with aria-expanded="true", one real option below the threshold, and Enter commits it; the 3-plus-Create menu exposes four options. English, Chinese, and dense docs now carry the same threshold exception and result-cap meaning; no runtime string was added.

VERDICT: clear

JUDGEMENT

slot verdict
PROBLEM clear
SOLUTION clear
ARCHITECTURE clear
IMPACT clear
API clear
THEMING clear
BREAKING note — documented one-extra-Create behavior
PERFORMANCE clear
VISUAL clear
A11Y & I18N clear

GOAL: met — at the exact current head, both requested contract changes are present; 127 affected tests pass; Chromium preserves short Create and shows 3 results plus Create; old/current pixels are identical. DISPOSITION: prior queryEntries public-surface finding → fixed; prior undocumented maxMenuItems consequence → fixed; one-extra-Create row → accepted as the explicit documented/tested result-cap contract requested in Round 3. ADVICE: omitted — no remaining defect or decision. AUTHOR CAN PROCEED: yes — both Round 3 acceptance criteria are met and no human-owned API/design decision remains. WORST OUTCOME: none found.

JUDGEMENT NEEDED: none — the public threshold was settled in prior rounds; this push removes accidental surface and documents the already-reviewed behavior.

approve and merge

No findings.

REVIEW

Thanks — both asks are in: __queryEntries is internal, and the maxMenuItems exception is documented and tested. Typeahead/Tokenizer suites and both Chromium paths pass at this head.

[Reviewed by Robohands]

INLINE

None.

EVIDENCE I DID NOT SPEND

  • All substantive exact-head checks are green; only review-required remains pending behind our standing request-changes.
  • main changed only TypeaheadItem.tsx/its test in these component paths since the PR’s merge-base; it does not invalidate this mechanism.
  • tsc --noEmit for @astryxdesign/core passes at the exact head.

TIME

TIME total 13m setup 2m — exact detached worktree, APFS-cloned dependencies, Storybook dev; warm main not needed reading 4m — full PR/wiki history, prior rounds, incremental diff, source, current kit/rubric measuring 4m — two unit suites, core typecheck, two Chromium scenarios at two heads writing 3m — presentation, critic pass, wiki record waste 0m

WHAT I COULD NOT VERIFY

Nothing material. Chromium is the required and supported browser lane on this Mac; exact-head CI covers the repository-wide suite.

Critic pass

Pass 1 clean: verdict correct, 28 words, no rule violations, postable as written.

What changed before posting

No public PR action was taken. This is the exact draft for Cindy.

Clone this wiki locally