Skip to content

chore(post-8f): cleanup cluster — setState fix + ProcessHubId brand + Canvas decomposition + tsc hygiene#168

Merged
jukka-matti merged 15 commits into
mainfrom
cleanup/setstate-appmain
May 14, 2026
Merged

chore(post-8f): cleanup cluster — setState fix + ProcessHubId brand + Canvas decomposition + tsc hygiene#168
jukka-matti merged 15 commits into
mainfrom
cleanup/setstate-appmain

Conversation

@jukka-matti
Copy link
Copy Markdown
Owner

@jukka-matti jukka-matti commented May 14, 2026

Summary

Bundled cleanup PR closing the three carry-forward items from the 8f canvas-viewport retrospective (per feedback_single_pr_at_end_cleanup pattern):

  1. setState-in-render warning in AppMain — fixed
  2. LOW feat: yamazumi reporting with mode-specific KPIs and activity breakdown #19 brand ProcessHubId opaque type + remove __wall-canvas-unbound__ sentinel — fixed
  3. LOW docs: migrate to Starlight + LikeC4, fix Storybook v10 #16 Canvas/index.tsx 1135 → 845 lines via 3 extractions — fixed

Plus opportunistic cleanup of trivial pre-existing tsc errors discovered during the work (categories: Vite ImportMeta types, DataRow fixture casts, missing vitest imports).

What's in the bundle (15 commits)

PR1 — setState-in-render fix (3 commits)

  • 6c5bc1a7 Replace bare usePanelsStore() whole-store sub with 24 individual selectors in useAppPanels.ts. Closes docs/investigations.md:72-82.
  • b662e26d Doc resolution paragraph + corrected rule attribution.
  • 7a55a8b2 Extract openDataTableAtRow compound action; drop dead _vi import.

PR2 — Brand ProcessHubId + sentinel cleanup (5 commits)

  • a7a1ad3f Define ProcessHubId = string & { readonly __brand: 'ProcessHubId' } in packages/core/src/processHub.ts; add asProcessHubId() (throws on empty per feedback_strict_assert_over_silent_migration) + isProcessHubId(); 25-file sweep; replace WallCanvas sentinel with null short-circuit.
  • 9dfb8c68 Prettier-formatting fix.
  • 125004ce Sweep 7 consumers' import paths from @variscout/stores@variscout/core/processHub.
  • d5e6cd28 Finish sweep (15 more consumers exposed when barrel shim removed).
  • 9602b132 Test coverage: factory + predicate + null short-circuit.

PR3 — Canvas/index.tsx decomposition (3 commits)

  • 710eb725 Extract useCanvasHypothesisDrawing hook (~120 LOC + 12 tests).
  • 7cacf356 Extract useCanvasHypothesisArrows hook (~100 LOC + 8 tests).
  • 4c0bf00d Extract <CanvasLevelRouter> sub-component (~100 LOC + 10 tests). Canvas/index.tsx: 1135 → 845.

Pre-existing tsc hygiene (4 commits)

  • afd8c8b7 Add vite-env.d.ts in core + hooks for ImportMeta.env/.glob; fix small test-globals issues.
  • 091f9d3b Cast DataRow[] fixtures in 2 hook tests + log deferred items.
  • e73fca64 Import beforeEach in responsesApi.test.ts.
  • f2468f96 Update investigations.md: close item chore(deps): bump actions/checkout from 4.2.2 to 6.0.2 #3, log new core fixture-shape mismatches.

Reviewer chain (per feedback_subagent_driven_default)

  • 4 Sonnet implementer dispatches
  • 3 Sonnet spec reviewers
  • 1 Sonnet quality reviewer (PR1) + 1 Sonnet quality reviewer (PR2)
  • 1 Opus final reviewer (PR1)
  • Pending: Opus final reviewer for full bundle

Test plan

  • PWA tests green (337 → 339 + regression tests for new hooks)
  • All package builds green
  • PWA + Azure tsc 0 errors
  • Pre-existing tsc errors reduced: core 14 → 11 (3 fixed), hooks 17 → 4 (13 fixed)
  • --chrome walk: console clean on Frame-tab activation, LOD transitions, Lock-canvas toggle (regression-tested via App.test.tsx)
  • Owner manual --chrome smoke walk after merge: paste → L1 → wheel-zoom L2 → step click → L3 → all interaction paths work

Deferred follow-ups (logged in docs/investigations.md)

  • d3 type-package resolution in useCanvasViewportInput.ts (needs @types/d3-* dep verify)
  • Tuple-mock typing in useHubCommentStream.test.ts
  • 9 entity-fixture-shape mismatches in core tests (SustainmentRecord + ProcessMap fixtures need missing required fields added)
  • Vitest worker timeouts under turbo concurrent load (infra tuning — separate diagnostic)
  • LOW docs: migrate to Starlight + LikeC4, fix Storybook v10 #16 partial: 4 remaining Canvas/index.tsx seams (viewport fit / step card grid / content L2 / mobile picker) deferred to next viewport feature

pr-ready-check

PWA + tests green; pre-existing azure-app Dexie IndexedDB API missing failures in useCanvasViewportLifecycle.test.ts unchanged from main.

🤖 Generated with ruflo

jukka-matti and others added 3 commits May 14, 2026 10:44
…anelsStore() with individual field selectors

The bare `const store = usePanelsStore()` whole-store subscription in
`useAppPanels` violated the CLAUDE.md/ADR-078 "Never bare useStore()" rule.
In React 19 Strict Mode + Zustand 5 (useSyncExternalStore), the whole-store
snapshot causes tearing detection to fire on every panelsStore update
(panel-state transitions co-incident with LOD switches, frame-tab activation,
Lock canvas toggle), producing 8+ "Cannot update a component (AppMain) while
rendering" warnings per interaction.

Fix: rewrite useAppPanels.ts to use 24 individual usePanelsStore(s => s.field)
selectors — one per state field and action. useEffect dependency arrays cleaned
accordingly (no more `store` object in deps). App.tsx return interface unchanged.

Regression test added in apps/pwa/src/__tests__/App.test.tsx: mounts App,
exercises showFrame() panel transition, asserts zero setState-in-render warnings.
All 40 PWA test files green; pwa build green.

Co-Authored-By: ruflo <ruv@ruv.net>
Spec-review followup: add commit hash 6c5bc1a to the setState-in-render
resolution paragraph, and correct rule attribution from "CLAUDE.md / ADR-078"
to "packages/stores/CLAUDE.md:18 (cites ADR-041)" — ADR-078 covers the
6-stores-across-3-layers model, not selector discipline.

Co-Authored-By: ruflo <ruv@ruv.net>
… _vi import

Code-review followup: per feedback_fix_absorbed_violations_at_seam, lift the
two bare usePanelsStore.setState() calls in useAppPanels.openDataTableAtRow
into a single store-action openDataTableAtRow(index, isDesktop). Drops the
last bare-setState seam violation in the hook. Also removes a dead `vi as _vi`
import from App.test.tsx.

Co-Authored-By: ruflo <ruv@ruv.net>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
mean-beoynd-lite-pwa Ready Ready Preview, Comment May 14, 2026 0:40am
variscout_website Ready Ready Preview, Comment May 14, 2026 0:40am

jukka-matti and others added 2 commits May 14, 2026 12:20
…emoval

Define `ProcessHubId = string & { readonly __brand: 'ProcessHubId' }` in
`packages/core/src/processHub.ts` (the only definition point). Add
`asProcessHubId()` (throws on empty) and `isProcessHubId()` predicate;
update `DEFAULT_PROCESS_HUB_ID` and `normalizeProcessHubId` to return
`ProcessHubId`. Re-export from stores for back-compat consumption.

Replace `hubId ?? '__wall-canvas-unbound__'` sentinel in WallCanvas with
`hubId ?? null`; update `useCanvasViewportInput` to accept `ProcessHubId | null`
with a null short-circuit (no store writes when hub is absent).

25-file sweep across packages/hooks/ui/apps: all `string`-typed hub slots
at store boundaries cast/narrowed to `ProcessHubId`; test fixtures use
`const h = (id: string) => id as ProcessHubId` helper. Both app lifecycle
hooks (PWA + Azure) use `normalizeProcessHubId(hubId)` → `boundHubId` so
the raw `string | null | undefined` param never reaches store calls.

Closes LOW #19 from the 8f retrospective (docs/investigations.md updated).

Co-Authored-By: ruflo <ruv@ruv.net>
Prettier placed the const between two import statements during the
pre-commit format pass. Move it after all imports to keep ES module
import grouping clean.

Co-Authored-By: ruflo <ruv@ruv.net>
jukka-matti and others added 10 commits May 14, 2026 12:36
Removes the back-compat re-export shim at canvasViewportStore.ts:16 per
feedback_no_backcompat_clean_architecture; 7 consumers now import the
branded type directly from @variscout/core/processHub. Barrel index.ts
updated to re-export ProcessHubId from @variscout/core/processHub directly
(not via the store file) so existing code that happens to pull from the
barrel continues to compile.

Co-Authored-By: ruflo <ruv@ruv.net>
Sweep the remaining ~15 consumers missed by the earlier pass; barrel
shim removal exposed the gaps via TS error. All ProcessHubId imports
now go directly to @variscout/core/processHub.

Co-Authored-By: ruflo <ruv@ruv.net>
…wportInput null short-circuit

Two Important test gaps from PR2 code review: factory throw-on-empty
(asProcessHubId, isProcessHubId) and the new hubId:null no-op path in
useCanvasViewportInput. Both verify load-bearing contracts that were
shipped without exercise.

Co-Authored-By: ruflo <ruv@ruv.net>
Moves all drawing-mode pointer/keyboard event handlers and the
reset effect out of Canvas/index.tsx into a dedicated hook. The
hook returns stable handler refs plus endpointLabel and
parseEndpointElement helpers — 12 vitest tests cover all branches.

Co-Authored-By: ruflo <ruv@ruv.net>
Moves arrowSegments state, cardElements ref, and both useLayoutEffects
(ResizeObserver setup + DOM measurement) out of Canvas/index.tsx into a
dedicated hook. Measurement is stable equality-guarded to avoid needless
re-renders. 8 vitest tests cover empty, ResizeObserver attach/detach, and
remeasure-on-viewport-change paths.

Co-Authored-By: ruflo <ruv@ruv.net>
Moves the lens-validity gate and all l1/l2/l3 content composition
out of Canvas/index.tsx into an internal sub-component. Canvas drops
from 1135 to 848 lines. CanvasAuthoringMode and CanvasL3Archetype are
defined in CanvasLevelRouter and re-exported from Canvas to avoid a
circular import. 10 vitest tests cover the routing, lens-invalid empty
state, archetype switching, and mode-toggle visibility. Barrel updates
and CanvasWorkspace.test.tsx mock additions for the two new hooks land
in this commit.

Co-Authored-By: ruflo <ruv@ruv.net>
…e-existing)

Category A — add minimal ImportMeta.env / ImportMeta.glob augmentation .d.ts
files to packages/core/src/ and packages/hooks/src/ so that import.meta.env.DEV
and import.meta.glob(...) resolve without requiring vite as a direct dep.
Overloaded signatures distinguish eager (→ T) from lazy (→ () => Promise<T>)
so calls into registerLocaleLoaders (which expects LocaleLoaderMap) type-check.

Category C — two trivial fixes:
  - packages/hooks/src/__tests__/setup.ts: cast (window as unknown as Record...)
    to satisfy TS2352 (neither type sufficiently overlaps).
  - packages/hooks/src/__tests__/findingSourceLensCapture.test.ts: add _filters
    param to the vi.fn mock that takes one argument; was inferred as 0-arg.
  - packages/hooks/src/__tests__/setup.ts: /// <reference types="vitest/globals" />
    so beforeEach is recognised by standalone tsc (globals:true is vitest-only).

All pre-existing; unrelated to PR1/PR2/PR3 logic.

Co-Authored-By: ruflo <ruv@ruv.net>
…sting)

Category B — cast Record<string,unknown>[] test fixtures to DataRow[] at the
constant definition site so useParetoChartData and useYamazumiChartData calls
type-check without changing test behaviour:
  - packages/hooks/src/__tests__/timeLensWiring.test.ts: add DataRow import +
    cast PARETO_100 at buildParetoRows(100) assignment.
  - packages/hooks/src/__tests__/useYamazumiChartData.test.ts: add DataRow
    import + change EMPTY_DATA type annotation from Record<string,unknown>[] to
    DataRow[].

docs/investigations.md — new entry "Pre-existing tsc errors deferred from
PR #168" listing the 3 remaining items that are out of scope for a trivial-cast
pass: d3 type deps (useCanvasViewportInput.ts:2-4), tuple-mock fetch typing
(useHubCommentStream.test.ts:274-277), and beforeEach globals in
responsesApi.test.ts:862.

All pre-existing; unrelated to PR1/PR2/PR3 logic.

Co-Authored-By: ruflo <ruv@ruv.net>
One-line addendum to the pre-existing-tsc-error cleanup commit: the
responsesApi.test.ts file uses beforeEach at line 862 but the vitest
import on line 1 omits it. Same category as the other ImportMeta /
test-globals fixes in afd8c8b.

Co-Authored-By: ruflo <ruv@ruv.net>
… mismatches)

The beforeEach import fix in e73fca6 closes the responsesApi.test.ts
item from the deferred-tsc-errors entry. Adds item #4 capturing the
9 newly-surfaced SustainmentRecord + ProcessMap fixture-shape mismatches
that became visible once responsesApi.test.ts was unblocked.

Co-Authored-By: ruflo <ruv@ruv.net>
@jukka-matti jukka-matti changed the title fix(pwa): eliminate setState-in-render warning in AppMain chore(post-8f): cleanup cluster — setState fix + ProcessHubId brand + Canvas decomposition + tsc hygiene May 14, 2026
@jukka-matti jukka-matti merged commit 7c7dfd6 into main May 14, 2026
3 checks passed
jukka-matti added a commit that referenced this pull request May 14, 2026
Adds Amendment 2026-05-14 to the pinned "8f canvas viewport SHIPPED"
entry capturing PR #168 (7c7dfd6): setState-in-render fix +
ProcessHubId brand + Canvas decomposition + tsc hygiene. All
canvas-viewport carry-forward work closed.

Co-Authored-By: ruflo <ruv@ruv.net>
jukka-matti added a commit that referenced this pull request May 14, 2026
…re catch-up (#169)

* fix(hooks): resolve d3 module type imports in useCanvasViewportInput

@types/d3-transition was accidentally placed in dependencies instead of
devDependencies in commit 07add8a; d3 type errors do not appear once
pnpm install correctly wires the devDependency symlinks.

Move @types/d3-transition to devDependencies alongside @types/d3-selection
and @types/d3-zoom; all four @types/d3-* packages now sit in the correct
field and resolve cleanly under Bundler moduleResolution.

Refs investigations.md item 1 (post-#168 tsc hygiene).

Co-Authored-By: ruflo <ruv@ruv.net>

* fix(hooks): type vi.fn fetch mock so .mock.calls[] tuple resolves

vi.fn(() => ...) inferred a 0-arg call signature, making .mock.calls[0]
an empty tuple []. Changed to vi.fn<typeof fetch>(() => ...) (vitest 4.x
single-type-param form) so MockParameters<typeof fetch> flows through
and [url, init] destructuring type-checks correctly. Applied to all
three fetchMock declarations in the file for consistency.

Refs investigations.md item 2 (post-#168 tsc hygiene).

Co-Authored-By: ruflo <ruv@ruv.net>

* test(core): catch up SustainmentRecord fixtures to current schema

Add status, title, consecutiveOnTargetTicks, hasOverride, and
lastEvaluatedSnapshotId to all inline SustainmentRecord literals in
processHub.test.ts (3 sites), processState.test.ts (1 site), and the
recordFor() builder in sustainment.test.ts (1 site); semantically
meaningful defaults throughout — no as-casts or ts-ignore.

Refs investigations.md item 3 (post-#168 tsc hygiene).

Co-Authored-By: ruflo <ruv@ruv.net>

* test(core): catch up ProcessMap fixtures + fix null/undefined value mismatches

Add version, tributaries, createdAt, updatedAt to ProcessMap fixtures in
stampStepCapabilities.test.ts via a local makeMap builder; fix two ctqColumn
values from null to undefined (omitted) to match ProcessMapNode's string|undefined
type.

Refs investigations.md item 3 (post-#168 tsc hygiene).

Co-Authored-By: ruflo <ruv@ruv.net>

* docs(investigations): close items 1, 2, and fixture-shape half of 4 (PR A post-168)

T1 (e2c584ec) — d3 types placement fix; original hypothesis amended.
T2 (35c34d83) — vi.fn<typeof fetch>(...) tuple-mock typing.
T3 (7685734d) — SustainmentRecord fixture catch-up.
T4 (27027162) — ProcessMap fixture catch-up.

Item 4's vitest-timeout sub-item remains open for PR B.

Co-Authored-By: ruflo <ruv@ruv.net>

---------

Co-authored-by: ruflo <ruv@ruv.net>
jukka-matti added a commit that referenced this pull request May 14, 2026
…egex + structural-absence-guard docs (#171)

* refactor(core): read-once + alternation in architecture-grep test

Architecture test now reads all ~190 files once into a Map and runs each
per-name regex against the cached strings, dropping 3040 reads/run to
190. Preserves 16 it() blocks for per-name granular failure reporting.

Adds docstring section framing the test as a tripwire (denylist; narrow
scope) not a wall — references docs/investigations.md branded-Cpk-type
follow-up entry to be added in T5.

Refs investigations.md "Pre-existing tsc errors deferred from PR #168"
(vitest worker-timeout sub-item).

Co-Authored-By: ruflo <ruv@ruv.net>

* docs(testing): add architecture-test (structural-absence guard) section

Documents the denylist/CI-grep pattern used by
architecture.noCrossInvestigationAggregation.test.ts (ADR-073) and
scripts/check-level-boundaries.sh (ADR-074). Covers when to use, when
not to use, implementation pattern (read-once cache + per-name regex),
honest framing of denylist limits, and pointer to the branded-type
durable-answer follow-up in investigations.md (added in T5).

Refs investigations.md "Pre-existing tsc errors deferred from PR #168"
(vitest worker-timeout sub-item).

Co-Authored-By: ruflo <ruv@ruv.net>

* docs(testing+investigations): add architecture-test cross-ref + branded-Cpk follow-up entry

- .claude/rules/testing.md: add line pointing to the new architecture-test
  docs section.
- docs/investigations.md: new entry "Branded Cpk type as durable replacement
  for forbidden-name guard". Resolves forward references from T2's test
  docstring and T4's docs section. Proposes opaque-type pattern mirroring
  ProcessHubId from PR #168; estimated scope and promotion path included.

Refs investigations.md "Pre-existing tsc errors deferred from PR #168"
(vitest worker-timeout sub-item).

Co-Authored-By: ruflo <ruv@ruv.net>

* chore(testing): remove retry-once workaround for architecture-grep flake

T2 refactored the test to read-once + per-name regex (commit 06d2638),
eliminating the file-IO pattern that caused the flake under turbo
concurrent load. The retry-once workaround is now redundant and removed:

- docs/05-technical/implementation/testing.md: anti-pattern note no
  longer references the deleted feedback_pr_ready_check_retry_on_grep_test
  memory; updated to credit the fix commit and note no retry is needed.
- Out-of-repo system cleanup (not in this commit):
  - Deleted ~/.claude/projects/.../memory/feedback_pr_ready_check_retry_on_grep_test.md
  - Removed corresponding MEMORY.md index entry (line 75)

Keeps the hooks/index.test.ts known-flake line in .claude/rules/testing.md
intact (different root cause — import/env time, not file IO).

Co-Authored-By: ruflo <ruv@ruv.net>

---------

Co-authored-by: ruflo <ruv@ruv.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant