You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
svelte-kit-vice/audit/components/tag-group.md

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 (lift selectedSet = $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: open

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(); ... }) and getItems() (lines 98-104) does root.querySelectorAll<HTMLElement>(...) + .filter(el.closest(...) === root).
  • impact: Although rovingTargetEl is 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 on this.opts.value.current and 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 rovingTargetEl is ALREADY a single provider-level $derived (one querySelectorAll per invalidation, NOT per item) and the per-item isRovingTarget is an O(1) pointer ===. That is the CORRECT lifted A31 pattern, not a violation. The only real kernel is the A18 recommendation (a live querySelectorAll on 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).

Powered by TurnKey Linux.