Skip to content

fix(graph): F5 audit cleanup — CRITICAL+HIGH+MEDIUM fixes + tests#28

Merged
explosivebit merged 3 commits into
developfrom
feature/audit-cleanup-f5
May 5, 2026
Merged

fix(graph): F5 audit cleanup — CRITICAL+HIGH+MEDIUM fixes + tests#28
explosivebit merged 3 commits into
developfrom
feature/audit-cleanup-f5

Conversation

@explosivebit
Copy link
Copy Markdown
Contributor

@explosivebit explosivebit commented May 5, 2026

Summary

4-expert audit on develop (post-PR #26 merge bf842a7) flagged 2 CRITICAL, 5 HIGH, 3 MEDIUM, 1 LOW. This PR closes them all and adds vitest unit-test coverage for the cluster geometry.

CRITICAL (2)

  • ForceView link key by object identity — every rebuild() reassigned simLinks so Svelte destroyed and recreated every <line>. Now keyed on ${source.id}>${target.id}:${relation} stable string.
  • detectClusters double-work + radii cache out of order — Map.keys() insertion order from ringCounts was non-monotonic. computeRingRadius's cache then resolved ring 2 before ring 1, producing wrong radii (visible regression: RFC + ADR sharing one orbit position in RadialView). Fix: sorted iteration in both passes; lib also exposes cluster.orbits / cluster.radii / nodeAdjacency so views don't recompute them.

HIGH (5)

  • force-cluster-repel — typed Object.assign construction, drop unsafe cast, constrain NodeT extends SimulationNodeDatum.
  • ForceView signature effect — only re-bind clusterRepel + explicit .initialize?.(simNodes) for fresh cluster ids.
  • Score memo + didFit reactive — gate Map rebuild on content signature; reset fitToView on layout-shape changes.
  • Zoom-floor 0.45 (was 0.2) — labels stay legible, fitToView floor matches.
  • noUncheckedIndexedAccess: true — fallout resolved in cluster.svelte.ts orphans-fill block.
  • forceClusterRepel short-circuits when n < 2 or all nodes share one cluster.

MEDIUM (3)

  • Drop _adjacency parameter from computeOrbitRing; rename _tick → _invalidationTick; demote misleading FIXME comment.
  • a11y: verified role="img" + role="button" setup intact in both views.

LOW (1)

Tests

  • cluster.test.ts (16): chord rule, radial gap, N=1, ring-0 pin, monotonic, anchored angles, multi-cluster.
  • regression.test.ts (2): ring N − ring N-1 ≥ RING_GAP for the workspace shape that triggered the bug; RFC/ADR cannot share radius. Would FAIL against the pre-fix code.

Verify

  • npx svelte-check — 0 errors / 0 warnings / 405 files.
  • npm test — 18/18 passed.
  • npm run smoke (root) — PASS.
  • DOM check on live workspace (23 nodes / 5 clusters): 0 bbox overlaps, 23/23 cards exact-on-orbit (was 21/22 in EVID-012; also 2 overlaps before this fix).

Refs: PRD-005 RFC-004

Test plan

  • CI smoke matrix (3-OS × Node 22) green.
  • svelte-check clean on PR (verified locally).
  • vitest suite passes on PR.
  • Manual visual: switch to Radial → confirm no overlap; switch to Force → simulation settles.

🤖 Generated with Claude Code

@explosivebit
fix(graph): F5 audit cleanup — CRITICAL+HIGH+MEDIUM fixes
8fd3f53 
CRITICAL:
- ForceView: stable each-block link key built from source/target/relation
  string. Was keyed by object identity; every rebuild recreated all
  <line> elements (post-merge audit on PR #26).
- detectClusters: extend ClusterInfo with `orbits` + `radii` (computed
  in pass-2) and ClusterDetectionResult with `nodeAdjacency`. Both
  views read these instead of recomputing computeOrbitRing /
  computeRingRadius / buildHierarchyAdjacency per render — the work
  is now done once.
- detectClusters: iterate `counts.keys()` in SORTED order in both the
  actualMaxR pass and the radii[] population. Map.keys() returns
  insertion order, which depended on orbit assignment order; without
  sorted iteration computeRingRadius's cache resolved ring 2 before
  ring 1 and produced ring 2 = 126 instead of 252. Visible regression:
  RFC + ADR sharing a single orbit position in RadialView.
- Filter memo (RadialView + ForceView): wrap filterArtifacts /
  filterEdges / scoreById in $derived.by guarded by content
  signature so a 10s poll with identical payload doesn't invalidate
  the layout.

HIGH:
- force-cluster-repel: drop unsafe `as ForceClusterRepel<NodeT>` cast.
  Constrain `ForceClusterRepelOptions<NodeT extends SimulationNodeDatum>`.
  Use typed Object.assign so the returned value has all members at
  construction time.
- ForceView layout-signature effect: drop redundant re-bind of
  forceX/forceY/orbital (their accessors already read `layout` fresh
  per tick). Re-bind clusterRepel only when nodeToCluster changes,
  then explicitly call `.initialize?.(simNodes)` on the new instance
  to refresh cached cluster ids.
- RadialView: didFit is `$state` with a layout-shape signature effect
  that resets it on substantial transitions; fitToView re-runs after
  filter clears / dataset reloads.
- RadialView + ForceView: `scaleExtent([0.45, 4])` (was 0.2). Same
  floor in RadialView fitToView. Labels stay legible at min zoom.
- force-cluster-repel: early-exit when n < 2 or all nodes share one
  cluster id. Skip pair when both nodes are fixed (a.fx/fy and
  b.fx/fy non-null).
- tsconfig: enable `noUncheckedIndexedAccess`. Resolve fallout in
  cluster.svelte.ts (Array index narrowing in the orphans-fill block).

MEDIUM:
- cluster.svelte.ts: drop unused `_adjacency` parameter from
  computeOrbitRing (call sites updated).
- ForceView: rename `_tick` → `_invalidationTick` with comment;
  demote misleading FIXME(reactivity-loop) to a plain explanatory
  comment per .claude/rules/10-comments-policy.md.
- a11y: verified RadialView + ForceView keep `role="img"` on the SVG
  (per PRD-003 FR-002) AND `role="button" tabindex="0"` on each
  `<g.node>`. No code change needed.

Refs: PRD-005 RFC-004
@explosivebit
fix(deps): F5 LOW — bump cookie ≥ 0.7.0 (CVE-2024-47764)
06574d3 
Add overrides → cookie ">=0.7.0" in template/package.json. npm install
refreshes lockfile to cookie@1.1.1. Closes GHSA-pxg6-pf52-xh8x (LOW).
Practical impact ≈ 0 (we don't serialize user-controlled cookie values),
but eliminates the open dependabot alert on develop.

Refs: PRD-005 RFC-004
@explosivebit
test(graph): F5 — vitest unit-tests for cluster geometry + radii regr…
f062b38 
…ession

Add vitest as devDep (--ignore-scripts) and a "test" npm script to
template/. Two test files:

cluster.test.ts (16 tests):
- computeRingRadius chord rule for N=2..12, radial-gap rule, N=1
  special case, ring-0 pin, monotonicity.
- computeOrbitRing root pin, same-kind same-ring, missing-types
  collapse inward.
- computeAnchoredAngles even spread when no anchors, root angle 0.
- ringCounts grouping; detectClusters K=0/1/2/≥3, hierarchy routing.

regression.test.ts (2 tests):
- ring N radius ≥ ring N-1 radius + RING_GAP across all populated
  rings of a real workspace shape (PRD + 3 RFC + 2 ADR + 8 EVID).
- RFC (ring 1) and ADR (ring 2) cannot share radius.

These two regression tests would FAIL against the pre-fix
detectClusters (Map.keys() insertion order was non-monotonic, cache
resolved out-of-order).

Verify: npm test → 18 passed (2 files); svelte-check 0/0/405.

Refs: PRD-005 RFC-004
@explosivebit explosivebit merged commit 6dac7b4 into develop May 5, 2026
3 checks passed
@explosivebit explosivebit deleted the feature/audit-cleanup-f5 branch May 5, 2026 19:41
@explosivebit explosivebit mentioned this pull request May 5, 2026
Merged
4 tasks
explosivebit added a commit that referenced this pull request May 5, 2026
@explosivebit
feat(graph): F6 UX follow-ups — cluster collapse + ArrowKey nav + EVI…
1f5cdad 
…D-013 (#31)

## Summary

Two UX follow-ups from the F5 audit, plus the F5 acceptance evidence
pack.

### F6-T1 — RadialView cluster collapse

Toggle ("−/+") near each cluster's centroid. Visible only for clusters
with ≥ 3 rings. Click/Enter collapses that cluster to root + ring 1;
click again to expand. State per-view; fitToView re-runs on toggle.

### F6-T2 — ArrowKey navigation between nodes

New shared lib `keyboard-nav.ts` with `pickNextNode(current, candidates,
direction)` — cone-based selection (±60° around axis), cost = `distance
· (1 + 2·angle/π)`, falls back to nearest. Wired into both RadialView
and ForceView. Adds `data-id` on each `<g class="node">`. 6 unit tests.

### F6-T3 — EVID-013

Forgeplan acceptance pack closing the F5 audit-cleanup work that landed
in PR #28. Linked `informs` to PRD-005, RFC-004, EVID-012. R_eff 0.80;
PRD-005 quality bumped to A (0.82).

## Verify

- `npx svelte-check` — 0 errors / 0 warnings / 406 files.
- `npm test` — 24 / 24 passed (16 cluster + 2 regression + 6
keyboard-nav).
- `npm run smoke` — PASS.
- DOM check on live workspace: collapse hides ring ≥ 2 (rings 8 → 6,
nodes 23 → 13 when 1 cluster collapsed). ArrowRight from PRD-001 →
RFC-002 (correctly picks rightward neighbour).

Refs: `PRD-005` `RFC-004` `EVID-013`

## Test plan

- [ ] CI smoke matrix (3-OS × Node 22) green.
- [ ] svelte-check clean on PR.
- [ ] vitest 24/24 pass.
- [ ] Manual: tab into a node, press arrows — focus moves directionally.
Click "−" on a centre cluster — outer rings hide.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@explosivebit