Review 5199

#5199 — OverflowList: report the collapsed set with onOverflowChange

cixzhang · merged as 7e6b73b · view PR

Verdict: comment — semantic approve

PR

#5199 OverflowList: report the collapsed set with onOverflowChange by cixzhang (bucket: the maintainer)

HEAD REVIEWED

ce5f832881a021fb958eef6796f1566e08eff4b3 <- every claim below was verified at this commit

VERSIONS

LOOP VERSION: 1.5.0 AUDIT RUBRIC: 1.13

LANE: full WHY: new public API + a changed shared hook + a new permanently-observed element. Three independent fast-lane disqualifiers, so fast was never eligible and no promotion was needed.

THE HEADLINE OF THIS ROUND — the five commits net to zero runtime change

The brief for this gate said five observer-fix commits since 888a724 addressed gate 7's width-only and duplicate-measurement findings. They did not. The two runtime files at this head are byte-identical to the head gate 7 reviewed:

                useOverflow.ts blob            OverflowList.tsx blob
888a724 (g7)    8391c5faad699f04a50e414c…      c709ac8530cd26d6cbc768ae…
ce5f832 (g8)    8391c5faad699f04a50e414c…      c709ac8530cd26d6cbc768ae…
git diff 888a724..ce5f832 -- <those two files>   →   empty

What happened is a rebase plus an exploration that was backed out. The branch was rebased onto 1d94f85 (main moved: #5466, #5558, #5214, #5594 landed), which reset both files to main's version at f208b97. The five commits then re-derived the fix, going out through per-child tracking and back:

commit	useOverflow.ts	OverflowList.tsx	what it did
05a72ed observe measured child sizes	+16	−44	re-applied the measurement-container observer
c30a5af track the measured child tree	+45	+44	added per-child mutation/size tracking
0bfedf4 keep remeasurement flat in list size	+43/−32	—	reworked that tracking to be O(1)
37bf9b7 narrow child mutation tracking	−1	—	narrowed it
ce5f832 avoid duplicate child remeasurement	−15	—	removed the remaining per-child machinery

Net across all five, against f208b97: useOverflow.ts +20/−8 (the docblock and the measureRef observer) and OverflowList.tsx unchanged — i.e. exactly the state gate 7 reviewed. The genuine net gain of the round is in the tests: OverflowList.test.tsx +60 lines, including a new unequal-width reorder test and a real sharpening of the same-count test (see BREAKING).

Consequence for this review: gate 7's two nits are not fixed, they are unchanged, and I re-measured both at this head rather than carrying its numbers. Everything else gate 7 established about this runtime still holds by construction, and I say below which claims I re-drove and which I carried on the blob identity.

PRIOR REVIEW (R1e, R31b): gh api repos/facebook/astryx/pulls/5199/reviews and .../comments are both empty — no GitHub review of any kind exists, so nothing of ours is blocking this PR. Three issue comments are ours or the author's:

1. cixzhang, 27 Aug 04:03Z — "Direction: keep onOverflowChange. OverflowList should own measurement while exposing the collapsed set so an existing standing menu can own rendering without a second anchor." The capability question, ruled by the author (R33).
2. Robohands, 27 Aug 08:58Z (gate 7) — "Thanks — this works: one 'More' at every width, and the collapsed set tracks. Approve once you undraft. Two nits: the new observer only sees width, and mount measures once more."
3. Robohands, 27 Aug 10:28Z — "Thanks — re-checked at ce5f832: one 'More' at every width, and the collapsed set tracks and empties. Still an approve once you undraft. The two earlier nits remain non-blocking." No gate artifact was written for it (review-artifacts/pr-5199/ held gates 1–7 only when this gate started).

I am extending, not contradicting, all three. Comment 3 reached the same verdict at the same head and said the nits remain; it did not say why they remain, which is the fact above. Nothing here reverses a colleague or the author.

R31c — did main invalidate the PR? No. Warm main is at 143c519, ahead of the PR's merge-base 1d94f85, and git diff 1d94f85..143c519 -- useOverflow.ts OverflowList.tsx is empty: nothing has moved under the files this PR changes. mergeable: MERGEABLE, mergeStateStatus: BLOCKED — the block is the draft flag, not a conflict.

PROBLEM

WHY 1: OverflowList will only hand its collapsed items to a node it mounts itself (overflowRenderer), and only while something is overflowing. A row that already carries a "…" menu therefore grows a second one beside it. WHY 2: the person using that toolbar now has two identical-looking menus in the same corner, and which one holds "Export" depends on how wide their window is — so they open both. WHY 3: a toolbar exists so the actions for the thing in front of you sit in one predictable place. Two anchors that swap contents on resize is the single failure that defeats it.

USER-FACING PROBLEM: someone narrowing a session header watches a second "…" appear next to the one that was already there, taking the action they were reaching for with it. PROBLEM SEVERITY: missing capability — a demonstrated job (a row with a standing menu collecting its own collapsed actions) has no supported path today short of dropping OverflowList and re-implementing the hidden measurement container, the gap-token-to-pixel mapping, the slicing and collapseFrom against useOverflow.

VERDICT: clear

SOLUTION

First: explain it like I'm five

The list already works out which items do not fit. It now tells you that set whenever it changes, instead of only being able to draw its own "…" for you. The list keeps doing the measuring; you keep owning whatever anchor is already on the row. So a header that already has a menu simply gains entries as the row narrows, and loses them again as it widens — and no second button ever appears.

Then enumerate the decisions

SOLUTION  (2 decisions · ~122 runtime lines of 876)
  1. the list reports its collapsed set through a callback; the caller owns the anchor   [the fix]
  2. the hidden measurement container is watched for size changes, so a child that
     changes size re-measures even when available width did not

Both trace to a written problem: 1 to the whole "Why" section, 2 to "There is a second correctness gap on that path: useOverflow only re-measures when the available container, options, or item count changes." Nothing is piggybacked, so no split is owed (R29). The decision count is unchanged from gate 7 because the runtime is unchanged.

The 876 lines are 122 runtime, 554 tests, 198 docs, 43 story, 7 changeset — bulk that is genuinely evidence.

BURDEN: medium — one optional public prop; one layout effect; one useState set from a ref callback during commit; one extra permanently-observed element per list; two JSON keys recomputed per render when the callback is set. BURDEN MATCH: proportionate — the problem class is missing capability, not polish, so the budget is not the polish budget (R42). The measuredKey gate is what stops the pre-measurement set being reported, and it is three lines.

7d hypothesis, restated for this round: everything I find will be a child of "the measurement container's box is being used as a proxy for 'the measured children changed'". Confirmed by ARCHITECTURE; nothing refuted it. A second hypothesis — the new observer is pinned by no test — was refuted by measurement (EVIDENCE I DID NOT SPEND).

VERDICT: note — the second decision's sensor still covers one of the two dimensions it stands in for (ARCHITECTURE).

ARCHITECTURE

OWNER: useOverflow owns measurement; OverflowList owns the report
TIER 1: sharedResizeObserver (reused) · useIsomorphicLayoutEffect (reused) — none new
TIER 2: none
SEAMS: overflowRenderer · collapseFrom start/end · direct useOverflow consumers
       (public hook, zero in-repo direct call sites)
BEHAVIOR UNIT: useOverflow (named hook, its own focused suite at
       packages/core/src/hooks/useOverflow.test.ts, 504 lines) + one layout effect
       in OverflowList. R35's mechanical threshold — two or more Effects coordinating
       refs/timers/observers — is not met: OverflowList adds exactly one Effect, and
       useOverflow's single pre-existing layout effect is untouched. Ref callbacks
       are not Effects.

Driven at this head, not read, and not carried:

SEAM / SENSOR                          DRIVEN RESULT AT ce5f832
caller-owned sibling anchor,           works — 1 anchor at 520/428/328/240/228px and
  full sweep incl. widen-back            back out to 520; hidden 0→1→2→4→4→2→0;
                                         sameAnchorNode=true at every step
menu contents at 240px                 ["Branch","Share","Export","Duplicate",
                                        "Rename session","Delete session"]
a measured child gets WIDER            observed box 661,0 → 731.9,0   moved=true
a measured child gets TALLER,          observed box 661,0 → 661,0     moved=FALSE
  not wider

Carried from gate 7 on blob identity, not re-driven: overflowRenderer + callback together, and the positioned-ancestor clamp case. Both were driven at 888a724 against byte-identical files; I say so rather than re-presenting them as fresh measurements.

The finding, unchanged and re-measured. useOverflow.ts:251 (observeResize(el, () => {) observes the measurement container as a stand-in for "a measured child changed size", and calculate() reads both offsetWidth and offsetHeight off those children (useOverflow.ts:182, :183-186). The container is height: 0 (OverflowList.tsx:61), so a content-box observer can only ever move on width. Measured above: 49→120px wide moved the box; 20→120px tall left it at 661,0 exactly.

rowHeight feeds the bounded row area in maxRows mode, so a child that gets taller without getting wider — a font swap, a badge gaining a line — leaves that area at the old height. Pre-existing on main, where nothing observed the container at all (R13), so this is a partial fix, not a regression. What is new is the claim, and this round it is in three places rather than one:

• useOverflow.ts:12-13 — "changes to available width or measured child sizes recalculate the result"
• the changeset — "child-size changes refresh every OverflowList and direct useOverflow consumer even when the available container width does not change" (this one ships to consumers as release notes)
• useOverflow.doc.mjs usage.description and docsDense.usage.description — "react to container and measured-child size changes"

Mitigating, and worth stating because it bounds the damage: the same public hook doc still carries the caveat {guidance: false, description: 'Use for vertical overflow; this hook measures horizontal width only.'} — so a builder reading the hook's own doc page gets the correct answer alongside the over-broad one. The changeset and the source docblock do not carry it.

VERDICT: note — width-only sensor; three places now claim both axes.

IMPACT

End user: nobody, until a builder opts in. No existing list renders differently — the callback adds no element to the row (driven: exactly one anchor at seven widths across the sweep), and the exact-head visual job agrees at scale (686 shots, zero OverflowList changes — BREAKING). Once a builder opts in, the experience is the one the PR set out to buy: you narrow a session header and the actions that no longer fit turn up inside the "More" menu that was already sitting there, in the same spot, instead of a second "…" arriving beside it.

Builder: everyone on the next release gets one more optional prop, and — opted in or not — one more observed element and one more measurement pass per list mount (PERFORMANCE).

What landing this newly exposes: nothing becomes visibly wrong that was not. useOverflow is public and now re-measures on child-size changes it previously ignored; there are no in-repo direct consumers, so the only people who see the change are external ones whose lists start correcting themselves. That is the fix, not a break.

VERDICT: clear

API

The call site, copied from apps/storybook/stories/OverflowList.stories.tsx:441:

const [hidden, setHidden] = useState<OverflowItem[]>([]);

<OverflowList gap={2} onOverflowChange={setHidden}>
  {actions.map(action => <Button key={action} label={action} size="sm" />)}
</OverflowList>
<DropdownMenu
  button={{label: 'More', variant: 'ghost', size: 'sm'}}
  items={[
    ...hidden.map(({index}) => ({label: actions[index]})),
    {label: 'Rename session'},
    {label: 'Delete session'},
  ]}
/>

	change	public?	class	doc'd?	verdict
+	OverflowList.onOverflowChange?: (overflowItems: OverflowItem[]) => void	yes — OverflowList/index.ts barrel	onOpenChange (85 files in core), onCollapsedChange/onWidthChange (11)	en + zh + dense	ok

OverflowItem was already exported (OverflowList/index.ts:10), so the payload type is importable — no public callback naming a type consumers cannot reach.

Rubric §3: P1 OverflowListProps extends BaseProps<HTMLDivElement> (unchanged) · P2 {...props} still lands on the visible container, not the measurement one (OverflowList.tsx:398) · P3 no new collisions · P4 no new boolean · P5 onChange would be wrong — this is a report, not a controlled value, and there is no overflowItems prop to pair with it.

Does it earn its permanence?

1. Class, not a single case. ComplexSelector.onOpenChange landed at this PR's original merge-base (#5211) doing the identical thing: expose state the component already computes so the consumer can own rendering without reading the shell's DOM. Convention grep at origin/main (R3k): 85 files carry onOpenChange, 11 carry onCollapsedChange/onWidthChange. A merged member is precedent (R22).
2. What exists and does not reach. overflowRenderer renders into the list and only while overflowing, and its measurement copy always receives every item, so a reporter placed inside it cannot tell which set is real. The two coexist rather than compete.
3. Cost of being wrong: a deprecation cycle on one optional callback. No default moves; no existing meaning changes.

VERDICT: clear

THEMING

no new theme targets.

grep -nE "#[0-9a-fA-F]{3,8}|rgba?\(|hsla?\(|boxShadow|light-dark\(" OverflowList.tsx useOverflow.ts  → 0

themeProps('overflow-list') is untouched and still points at the visible container, so astryx-overflow-list selects the same element. T2: nothing themeable is removed — the diff pins no value on xstyle or style.

VERDICT: clear

BREAKING

A consumer who can be broken exists: @astryxdesign/core is published and not private.

• API — no. Additive optional prop; no default moved, no type narrowed, no export removed.
• Visual — no, and this round it is measured rather than argued. The exact-head visual job ran (Stable visual regression: success, run 33058576861 at ce5f832): 6 of 686 shots changed, and none of them is OverflowList — SVGIconRegistry ×2, TransferList ×2, useChartRange ×2. All three live in packages/lab/src, and git grep -ln "useOverflow\|OverflowList" -- packages/lab/src packages/charts/src is empty, so none of them can reach this diff; they are baseline drift from the rebase, exactly the noise the brief flagged, and I neither accept them nor attribute them here. apps/storybook/stories/OverflowList.stories.tsx is in the shot set, and it did not move. Gate 7 could only argue this from the diff; at this head the gate settles it.
• Theme — no. No target added, removed or repointed.
• Behaviour — yes, deliberately. Walked the states: empty/zero unchanged (calculate() still early-returns at children.length === 0); loading, error, disabled and controlled/uncontrolled are not reachable — the diff adds no state, no default and no prop that changes one. The boundary state moves: a list whose measured children change size at the same count now re-measures where it previously kept the old fit. That reaches every existing consumer, not just opt-ins, and the changeset states it.

The test that pins it got genuinely better this round, and it is the only substantive net change of the five commits. re-measures a same-count keyed set before reporting it became re-measures same-count content changes with stable keys: the rerender's third key changed from 'wide' to 'c', which makes the keys identical across the two renders. Before, the test passed through measurementKey (the JSON of child keys moved), so it was exercising the keyed-membership path while claiming the stable-key one; now the only route is the measurement container's observer, and the test drives it explicitly with triggerResize(measureContainer()) at OverflowList.test.tsx:843.

VERDICT: note — behaviour changes for every existing list, deliberately and as stated.

PERFORMANCE & RESOURCES

+ useIsomorphicLayoutEffect [measuredKey, measurementKey, overflowKey, onOverflowChange]
                                        OverflowList.tsx:352   reports; no DOM read, no DOM write
~ ref callback reportMeasureRef [measureRef, measurementKey]
                                        OverflowList.tsx:331   sets state during commit
~ measureRef [calculate]                useOverflow.ts:239     +1 permanently observed element
  useIsomorphicLayoutEffect [calculate]  useOverflow.ts:260    unchanged

Effect + deps	external system	why not a handler	state/render cost	listener lifetime + cleanup	focused test
layout effect [measuredKey, measurementKey, overflowKey, onOverflowChange] OverflowList.tsx:352	none — notifies the consumer after measurement commits	no handler owns "the browser finished laying this out"	0 extra passes (gate 7's counter arm; runtime unchanged)	n/a	15 tests, incl. settles when the callback stores the collapsed set in state
measureRef observing the measure container useOverflow.ts:251	DOM — element box	a resize has no handler	+1 measurement pass at mount — re-measured below	attached on ref attach, unobserveResize on detach :241-244	re-measures same-count content changes with stable keys — proved decisive below; plus does not report after unmount

Mount cost, re-counted at this head (forced layout reads on measured children; calculate() reads 2 per child). BEFORE is warm main at 143c519, and I checked it is a valid baseline: git diff 1d94f85..143c519 over the two runtime files is empty, so main's copy of this code is the merge-base's copy.

story	measured children	warm main 143c519 (:6100)	head ce5f832 (:6412)	Δ
core-overflowlist--default	6	18 offsetWidth / 15 offsetHeight — 3 passes	24 / 20 — 4 passes	+1 pass, +6 reads
core-overflowlist--dynamic-items	6	18 / 15 — 3	24 / 20 — 4	+1 pass, +6
core-overflowlist--multi-row	11	33 / 30 — 3	44 / 40 — 4	+1 pass, +11

Identical to gate 7's numbers, as the byte-identical runtime predicts. Flat in interactions, linear in N.

The mechanism, which is the ask. observeResize() fires its callback synchronously (sharedResizeObserver.ts:77, "Fire once immediately so callers get an initial measurement"), and ResizeObserver.observe() then delivers its own initial notification for the same element at the same size. Every newly observed element measures twice. That was already true of the visible container; this diff adds a second observed element, so each OverflowList now pays it twice.

Why this is a note and not a block. The cost is spread, not introduced — sharedResizeObserver has always double-fired on observe(), the visible container has always paid it, and the diff adds a second payer rather than the behaviour. It is bounded at one pass per mount and does not grow with interaction. And no user is on the other end of it — the reads batch into one clean-layout pass. Who pays is the library, which is a cost we carry forever, so the accept is not mine to make (R18g): the number is above, the invariant is one measurement pass per newly observed element, and probe-kit/overflowlist-calculate-count.cjs is the acceptance test — head should read 3 passes, as main does.

Worst realistic N, from the repo: git grep -c "<OverflowList" origin/main → 11 call sites in apps/storybook/stories/OverflowList.stories.tsx, 2 in packages/core/src/Tokenizer/Tokenizer.tsx, 1 everywhere else. Eleven lists of eleven children is ~121 extra reads at mount.

Listeners: +1 element on the shared observer, torn down in measureRef(null). Layout: no forced read during render. Bundle: no new dependency. Perf test: not required — O(N), but on mount and resize, not on a flow a person hits constantly.

Reproduction, banked (this head's Storybook ran on port 6412; warm main on 6100):

probe-kit/overflowlist-calculate-count.cjs        SB_PORT=6412 (head) / 6100 (warm main)
probe-kit/overflowlist-measure-sensor-axes.cjs    SB_PORT=6412
probe-kit/overflowlist-standing-anchor-frames.cjs SB_PORT=6412 OUT=~/astryx/shots-5199-g8 \
                                                  BUILD_ROOT=~/astryx/worktrees/pr5199-g8 SHA=ce5f832…
probe-kit/overflow-standing-anchor.cjs            SB_PORT=6412 <sha> <worktree>   (full sweep + widen-back)

No new probe was needed this round — gate 7's five are all reusable as written, which is the harness working.

VERDICT: note — +1 measurement pass per list mount, on every consumer, opted in or not. The library pays; accepting it is Cindy's, not mine.

VISUAL EVIDENCE

VISUAL CHECK: manual frames required
WHY: the acceptance evidence for this PR is visible behaviour (one anchor, contents
     moving into it), and `core-overflowlist--standing-anchor` is a story added by
     this PR, so the visual gate has no baseline for it and cannot cover the
     endpoint. The gate IS green at this head and does cover the pre-existing
     OverflowList stories — that is the BREAKING · Visual evidence, not this slot's.

The frames are handed over by path, not embedded: the attachment upload is denied from this host (xfb_metamate_nest_bulk_file_upload → "Access denied from corp-net"). Whoever publishes takes them from ~/astryx/shots-5199-g8/. Under R38 this slot's durable-embed requirement is unmet for that reason and no other; the frames themselves were captured with receipts and opened.

Sensor receipt — written by captureWithSensors(), expectations authored from the contract before the page was read, all four armed frames passing every sensor, expected never loosened:

sensor	value (identical across all four frames; only the row's own width differs)
Build	ce5f832881a021fb958eef6796f1566e08eff4b3 (~/astryx/worktrees/pr5199-g8), expected == observed
Story	core-overflowlist--standing-anchor, route /iframe.html
Globals	{}
Theme / mode	neutral / light
Direction	ltr
Viewport	760×220 @1
Media	forcedColors false · reducedMotion false · coarsePointer false · hover true
Target count	1 ([data-testid="standing-anchor"])
State	{rowWidth, visibleActions, hiddenCount, anchorsOnScreen, anchorX} — the row table below
Geometry	box 16,16 520×46 (wide) / 300×46 (narrow), insideViewport true
Readiness	fonts loaded, runningAnimations 0
Errors	storybookError: null, no pageerror, no console error

VISUAL EVIDENCE
- 5 actions fit, the menu holds only its own entries   after · OPENED   shots-5199-g8/standing-anchor__wide.png
- 3 collapsed into that same menu, still one anchor    after · OPENED   shots-5199-g8/standing-anchor__narrow.png
- the two steps between them                           captured with receipts, numbers only — NOT opened
                                                                        shots-5199-g8/standing-anchor__{mid,tight}.png

frame	row	actions in the row	hidden	anchors on screen	anchor x
wide	520px	Rerun Branch Share Export Duplicate	0	1	445
mid	380px	Rerun Branch Share	2	1	305
narrow	300px	Rerun Branch	3	1	225
tight	230px	Rerun	4	1	155

Every difference between the two frames I opened, labelled:

• wide → narrow, three actions leave the row. Intentional — the PR states it: "as the row narrows, the actions that no longer fit should join the menu that is already on screen."
• one anchor at every width, never two. Intentional — "The caller owns the anchor; the list adds nothing to the row."
• the anchor's x moves 445 → 225. Intentional — the row's own reflow; the element is never remounted (sameAnchorNode=true at all seven sweep widths).
• No other difference. Nothing shifted that the PR did not claim.

VERDICT: clear

A11Y & I18N (rubric §1 and §9)

grep -nE "aria-|role=|useTranslator|t\('@astryx" OverflowList.tsx useOverflow.ts
  → 1 hit, pre-existing: aria-hidden="true" on the measurement container (OverflowList.tsx:369)
git diff 1d94f85..HEAD -- <those two files> | grep -cE "^\+.*(aria-|role=|useTranslator|t\('@astryx)"  → 0

• A18 axe — auto-covered and GREEN at this exact head. pr-a11y: success on ce5f832 (22 check-runs, not a path-filtered silence: test, lint, build, build-storybook, check-components, pr-rtl and theme-layers all ran and succeeded). This is the one thing gate 7 could not have: it reviewed a head where CI was still in progress. No baseline entry was added — the diff touches no .github/a11y-baseline.json.
• A1 accessible name — checked in the frames: the same single "More" button, named, at every width.
• A5 focus never dropped — checked by me: the diff removes no focusable element and adds none, and the measurement container is inert + aria-hidden exactly as before. No consumer-supplied DOM is involved, so R34c's trap does not apply.
• A2, A4, A8/A9, A10, A13, A14, A15 — not reachable: no interaction, no reveal, no animation, no painted state added.
• A11, A12, A16, A17 — not reachable: no disabled state, no input ARIA, no AT string, no hand-rolled keyboard handler.
• I1 / I4 / I5 / I6 / I7 — no user-visible or AT-facing string added; the new doc entries land in docs, docsZh and docsDense.
• I8–I12 direction — not reachable: the diff writes no CSS and no physical property. pr-rtl: success at this head as well. RTL not driven by me, because nothing in the diff can flip.

VERDICT: clear — nothing here is touched, and that is what I checked.

JUDGEMENT

PROBLEM        clear
SOLUTION       note — sensor covers one of the two dimensions it stands in for
ARCHITECTURE   note — width-only sensor; three places now claim both axes
IMPACT         clear
API            clear
THEMING        clear
BREAKING       note — behaviour changes for every list, deliberately and as stated
PERFORMANCE    note — +1 measurement pass per mount, all consumers
VISUAL         clear
A11Y & I18N    clear

Reading the bodies, not the labels (R34d): no slot's prose contains a sentence a user would call a bug. The strongest is ARCHITECTURE's maxRows height case, and it is main's behaviour too — the diff narrows the hole rather than opening it. Two notes do compound in one place worth naming: ARCHITECTURE's over-broad claim now lives in the changeset, which is release notes rather than a source comment, so the builder most likely to be misled is the one who never opens useOverflow.ts. That raises the value of fixing the wording; it does not make anyone stuck, so it does not raise the verdict.

GOAL: met — driven at the exact head, full sweep including the widen-back the previous rounds did not close: 520 → 428 → 328 → 240 → 228 → 328 → 520px, hidden 0→1→2→4→4→2→0, one "More" on screen at every step and sameAnchorNode=true throughout, its x moving only with the row's own reflow. At 240px the menu reads ["Branch","Share","Export","Duplicate","Rename session","Delete session"] — the collapsed actions inside the menu that was already there. The stated problem was a second anchor appearing; a second anchor never appears.

DISPOSITION

1. Width-only sensor, with useOverflow.ts:12-13, the changeset and useOverflow.doc.mjs all claiming both axes → nit in the approve, non-blocking. One word fixes the claim; the height half is pre-existing on main and independently landable.
2. Every newly observed element measures twice at mount, 4 passes where main does 3 → nit in the approve, non-blocking, stated with the number at this head. The accept/reject is Cindy's, not mine (R18g).
3. The five commits net to zero runtime change → not a finding against the PR; the code they land on is the code gate 7 approved, and the round's net gain (a genuinely sharper same-count test) is real. Recorded because the brief for this gate asserted the opposite and a reader would otherwise believe two nits had been closed.
4. BREAKING · Behaviour is not "no" → not worth raising as a finding (R16d): it is decision 2 working as written, and the changeset says so. No other negative finding exists, so nothing is orphaned.

ADVICE: bounded direction — for the wording, cited: the container is height: 0 at OverflowList.tsx:61, so "widths" is the true word in all three places, and useOverflow.doc.mjs's existing guidance: false line is the phrasing to match. For the double initial measurement I have not built a fix, so I give the invariant rather than the code: one newly observed element should produce one measurement pass, and probe-kit/overflowlist-calculate-count.cjs is the acceptance test — head should read 3 passes, as main does.

AUTHOR CAN PROCEED: yes — nothing is withheld. Both nits carry acceptance criteria, and the author merges.

WORST OUTCOME: "in maxRows mode a child that gets taller without getting wider leaves the bounded row area at the old height" → note. Compatible: it is main's behaviour too, it is reachable only in maxRows mode, and nobody is stuck, unreachable, unannounced, or losing data.

JUDGEMENT NEEDED: none — the capability question was ruled on this PR by its author on 27 Aug, and the naming rides a landed class (85 onOpenChange files in core; ComplexSelector.onOpenChange merged at this PR's original merge-base).

Verdict: comment — and it is an approve. GitHub will not take an approval on a draft; nothing here needs to change before it undrafts, and CI is green at this head.

comment (approve once undrafted)

1. [not blocking] the measurement container's observer only moves on width
   → a builder reads the changeset's "child-size changes refresh every OverflowList"
     and ships a maxRows tag cloud whose bounded height goes stale on a font swap
                                     · useOverflow.ts:12-13, :251 · the changeset

2. [not blocking] every newly observed element measures twice at mount
   → nobody sees it; the library carries it — 4 measurement passes per list where
     main does 3, on every consumer whether or not they opt in   · useOverflow.ts:251

REVIEW

Re-checked at ce5f832: the five observer commits net to zero runtime change — useOverflow.ts and OverflowList.tsx are byte-identical to this morning's head. Both nits stand; approve once you undraft.

INLINE (0-3, only if genuinely line-anchored)

none this round — the two line-anchored nits are unchanged from gate 7 and are already stated in its posted comment; repeating them at the line would be a third statement of the same two sentences.

EVIDENCE I DID NOT SPEND

• A finding I built and then killed by measuring it. The new observer is added to useOverflow, which has its own 504-line focused suite (useOverflow.test.ts) that the PR never touches, and OverflowList.test.tsx's StubResizeObserver.observe() is a no-op — so "delete the observeResize line and every test still passes" looked certain. It is false. I applied the revert arm (measureRef back to a bare calculate()) and ran all three suites that reference the hook: 1 failed / 159 passed — re-measures same-count content changes with stable keys at OverflowList.test.tsx:845, because triggerResize dispatches through the shared observer's registration and there is none to dispatch to. The line is pinned. Restored the file; worktree clean.
• The 6 changed visual shots at this head are SVGIconRegistry ×2, TransferList ×2, useChartRange ×2 — all packages/lab/src, none reaching useOverflow or OverflowList (grep empty). Baseline drift from the rebase; not accepted, not attributed here.
• Visual acceptance shows cancelled at this head (its initialize job cancelled, authorize/accept skipped). That is the accept-a-frame workflow, not the regression check; Stable visual regression is the one that ran and succeeded.

TIME

TIME  total 10m
  setup        3m   worktree + clone-install (1.8s, lockfile-identical donor) +
                    one @astryxdesign/build build + storybook dev on 6412
                    (warm main reused: yes, port 6100)
  reading      3m   brief, critic, harness, RUNBOOK gates, the full diff, the five
                    commits' blob chain, useOverflow, the test stub, both doc files
  measuring    3m   4 probes re-run at this head (all banked by gate 7, none rewritten),
                    1 revert arm, 4 frames captured, 2 opened; 0 re-measures
  writing      1m   draft + self-critic pass
  waste        0m   nothing I would not do again. The one dead end — the
                    untested-observer finding — was killed by a 20-second revert arm
                    before it reached the draft, which is the check working, not waste.

Gate 7 took 30m; this one took 10 because every probe already existed and the runtime turned out to be identical, which the blob check established in the first two minutes.

WHAT I COULD NOT VERIFY

• overflowRenderer + callback coexistence, and the positioned-ancestor clamp case, were driven at 888a724, not re-driven here. The two runtime files are byte-identical (blob hashes above), so the results carry — but they are carried, not fresh.
• Safari/WebKit: this Mac runs Chromium only, so the ResizeObserver initial-notification counts are Chromium's.
• The durable Review page cannot embed the frames from this host (upload denied from corp-net); they exist with receipts at ~/astryx/shots-5199-g8/.

What changed before posting

The exact-head result was posted in the maintainer's voice with the required attribution:

Thanks — re-checked at ce5f832: one "More" at every width, and the collapsed set tracks and empties. Still an approve once you undraft. The two earlier nits remain non-blocking.

[Reviewed by Robohands]

Posted review.