9.7 KiB
Audit: tag-group
audit-version: 1 audited-at: 2026-06-26 scope: ['soma', 'sema'] (SCOPE-DRIFT → SYS-1) method: adversarially-verified workflow (analyze → refute); HIGH lead-verified by direct read of the cited code. B5 ground-truth: the A31 O(N²) isSelected/isExpanded (.includes from a per-item $derived) is confirmed across listbox/grid-list/tree-view/tree-grid/tag-group (SYS-7); rovingTargetEl is correctly LIFTED everywhere (not A31); virtual-* use SvelteMap (A33-clean). provider: src/uix/soma/components/tag-group/tag-group-provider.svelte.ts reactivity (A31/A33/A35): A31 HAZARD FOUND (HIGH). Line 321 (TagGroupItemProvider.isSelected): readonly isSelected = $derived.by(() => this.provider.isSelected(this.opts.value.current)); calls provider.isSelected(value: string): boolean { return this.opts.value.current.includes(value); } (line 115-117). Each item's derived tracks this.opts.value.current (global selection array). On any selection change, all N items' derive
Summary
Counts (post-verification): CRITICAL 0 · HIGH 1 · MEDIUM 2 · LOW 1.
Findings
HIGH: A31: per-item derived reading global provider state causes O(N²) — tag-group-001
- dimension: B: Behavior
- rule: A31: per-item derived reading global provider state causes O(N²)
- location: src/uix/soma/components/tag-group/tag-group-provider.svelte.ts:321
- evidence: readonly isSelected = $derived.by(() => this.provider.isSelected(this.opts.value.current));
- impact: Each of N TagGroupItemProvider instances has a $derived that calls provider.isSelected(value), which reads this.opts.value.current (global selection array). When value changes, all N deriveds re-run, iterating an array to find membership. On a selection with 30+ items, this causes noticeable frame drops.
- repro: Render 50+ tags; select/deselect tags rapidly and observe all item deriveds (which feed aria-selected, data-state, data-highlighted, tabindex) re-computing in O(N²) time.
- proposed-fix: Lift a Set-based selection cache onto TagGroupProvider (e.g., readonly selectedSet = $derived.by(() => new Set(this.opts.value.current))). Each item's isSelected becomes this.selectedSet.has(value) (O(1)).
- verify: [confirmed] CONFIRMED. tag-group-provider.svelte.ts:321
readonly isSelected = $derived.by(() => this.provider.isSelected(this.opts.value.current));is a per-item $derived calling provider.isSelected (lines 115-117:return this.opts.value.current.includes(value);) which reads the GLOBAL selection array and scans it with .includes() (O(N)). With N items each re-deriving an O(N) scan on every selection change → O(N²). This is NOT a lifted Set — it's a raw .includes() over the shared array, matching the A31/SYS-7 pattern exactly. The derived feeds aria-selected, data-state, data-highlighted and tabindex (props derived lines 346-366), so the whole item prop set re-computes quadratically. Identical hazard duplicated in TagGroupLinkProvider.isSelected (line 410). Fix as proposed (liftselectedSet = $derived.by(() => new Set(this.opts.value.current)), item does .has()). - fix-status: fixed (
92f988e7)
MEDIUM: SYS-1: scope-drift — eidos directory exists but morfo scope omits 'eidos' — tag-group-002
- dimension: A: Contract
- rule: SYS-1: scope-drift — eidos directory exists but morfo scope omits 'eidos'
- location: src/uix/morfo/components/tag-group.ts:7
- evidence: morfo declares scope: ['soma', 'sema'], but src/uix/eidos/components/tag-group/ directory exists with CSS and component files.
- impact: Eidos layer (visual recipe) is undeclared in the contract. Tooling (validators, docs, imports) may miss this layer or incorrectly assume it doesn't exist.
- repro: Check scope field in morfo; observe eidos directory structure; verify scope is ['soma', 'sema'] not ['soma', 'sema', 'eidos'].
- proposed-fix: Update morfo scope to ['soma', 'sema', 'eidos'].
- verify: [confirmed] CONFIRMED. morfo tag-group.ts:7 declares
scope: ['soma', 'sema']yet src/uix/eidos/components/tag-group/ exists with tag-group.css, types.ts, README.md and 5 .svelte wrappers (tag-group, -item, -label, -link, -remove-button). The visual layer is real but undeclared in the contract scope. SYS-1 scope-drift, MEDIUM. Fix: scope: ['soma', 'sema', 'eidos']. - fix-status: fixed (
212624e0)
MEDIUM: Test coverage gap for A31-hazard paths (large N selection scenario) — tag-group-003
- dimension: F: Tests
- rule: Test coverage gap for A31-hazard paths (large N selection scenario)
- location: src/uix/soma/components/tag-group/tag-group-provider.svelte.test.ts
- evidence: Tests create 3 items (alpha, beta, gamma) and perform selection/navigation. No test with N=30+, N=100+, or stress-test for derived re-run scaling.
- impact: O(N²) regressions (or other scaling issues) would not be caught until users hit them in production at 30+ items. Missing test for high-risk behavior dimension (A31).
- repro: Add test: create 50 items, select/deselect items in a loop, measure frame times or derived invocation count.
- proposed-fix: Add test case: 'handles large item counts without O(N²) derived re-runs' — verify isSelected derived is O(1) per item or add a performance assertion.
- verify: [confirmed] CONFIRMED (kept MEDIUM, not HIGH — it's a missing-test, not a defect). tag-group-provider.svelte.test.ts exercises only 3 items (alpha/beta/gamma, opts items line 78) and never asserts on scaling. The A31 O(N²) path at line 321 (and 410) has no large-N regression guard. The file is also
// @vitest-environment jsdom(line 1, SYS-3) but interaction is driven at provider-method level (onclick/onkeydown invoked directly) so the jsdom limitation is secondary here; the real gap is the absence of a 30+/100+ item scaling assertion covering the confirmed A31 hazard. - fix-status: open
LOW: A31/A18: per-item derived reads provider.rovingTargetEl, which runs a live querySelectorAl — tag-group-004
- dimension: B: Behavior
- rule: A31/A18: per-item derived reads provider.rovingTargetEl, which runs a live querySelectorAll over the whole tree on every read → O(N²)
- location: src/uix/soma/components/tag-group/tag-group-provider.svelte.ts:325
- evidence: Item:
readonly isRovingTarget = $derived.by(() => this.opts.ref.current === this.provider.rovingTargetEl);(line 325, duplicated for Link at line 414).rovingTargetEl(lines 106-111) is$derived.by(() => { const items = this.getItems(); ... })andgetItems()(lines 98-104) doesroot.querySelectorAll<HTMLElement>(...)+.filter(el.closest(...) === root). - impact: Although
rovingTargetElis itself a single provider-level $derived (so the querySelectorAll runs once per invalidation, not once per item), it has no reactive dependency on a registry — it depends only onthis.opts.value.currentand whatever is read inside getItems(). When selection changes, rovingTargetEl re-runs a full-tree querySelectorAll (O(N)) once, then every item's isRovingTarget re-compares (O(N)) → O(N) overall for the roving target itself, but combined with the confirmed isSelected O(N²) the whole item prop graph is quadratic on every selection mutation. Separately, A18: roving/keyboard nav uses a live querySelectorAll (getItems) on every keydown (handleItemKeydown line 175) and every focusAt (line 161) rather than a registry Map of registered parts. - proposed-fix: A18 part: maintain a registry Map of mounted item/link parts (register on mount, unregister on dispose) and derive the ordered item list + roving target from it instead of
querySelectorAll, eliminating repeated DOM walks on each keydown. (The A31 part is already covered by tag-group-001.) - verify: [verifier-added → LEAD-DOWNGRADED to LOW] The verifier raised this as HIGH "A31", but lead read of lines 106-111 + 325-326 shows
rovingTargetElis ALREADY a single provider-level$derived(onequerySelectorAllper invalidation, NOT per item) and the per-itemisRovingTargetis an O(1) pointer===. That is the CORRECT lifted A31 pattern, not a violation. The only real kernel is the A18 recommendation (a livequerySelectorAllon every keydown could be a registry Map) — a LOW nit, not a HIGH. The finding's own impact text concedes "the querySelectorAll runs once per invalidation, not once per item". Downgraded to LOW. - fix-status: open
No-findings dimensions
C: DOM-selector, D: Frontier, E: TSC / Theming, A: Contract (beyond scope-drift) - morfo has 'as const satisfies Morfo', parts registered, no undeclared aria/data, B: Behavior (A30, A33, A35, A6, A18, A10, A14, A12) - clean
Theming facts (E-bis)
- magic z-index: none
- magic literals: none
- undeclared parts: none
- roles clean: true · variants clean: true
Tests (F)
- exists: true · env: jsdom (@vitest-environment jsdom)
- covers: selection (single/multiple); keyboard navigation (arrow, Home, End, Enter, Space, Delete, Backspace); click selection; item removal; label registration; link + remove-button nested behavior; semantic event emission (commit-select, commit-unselect, commit-remove, commit-reset)
- untested: Large N (30+, 100+) item scaling / O(N²) behavior; A31 derived re-run performance regression; RTL keyboard navigation (getDirectionalKeys used but RTL flow not tested)
Style observations (non-blocking)
- CSS references component-scoped tokens (--tag-group-gap-md, --tag-group-item-height-md, etc.) rather than direct canonical spacing/sizing tokens, which is acceptable (recipe tokens are component-local); no magic z-index or raw hex values observed; uses proper CSS variables for colors (--tag-group-item-bg, etc.) which resolve to role-based colors in theme.
- Focus ring styling delegated to --focus-ring-* tokens (proper).
- Transition durations use --duration-* tokens (proper).
- Disabled state uses --tag-group-disabled-opacity (proper opacity token reference).