refactor(app): extract useForceGraphSimulation from MapPage#8581
Merged
Conversation
- Move spec loading, similarity weights/minSim state, the KNN graph derivation (node cache + cluster seeding), the settling gate (settled/tickProgress), search matching, and the tuned d3-force configuration into src/hooks/useForceGraphSimulation.ts - Hook stays decoupled from ForceGraph2D's imperative ref API: it returns plain state + handlers (markSettled, handleEngineTick, resetWeights) and a static forceConfig; the only inversion is an onRepaint callback fired when thumbnails finish loading - MapPage keeps the canvas element, paint callbacks, UI overlays, and all ref-coupled interaction handlers (camera fit, force wiring, fly-to); behavior, analytics events, aria attributes and keyboard interactions are unchanged - outlierSquashForce + cluster color constants move to the hook module; MapPage.helpers stays a pure math module (header comment updated); hook exported from the src/hooks barrel - New useForceGraphSimulation.test.ts (17 tests) covers graph derivation, weight/minSim handling, the settling gate, search matching, and the relocated outlierSquashForce suite; MapPage.test.tsx keeps the rendering/interaction tests (frontend suite 552 -> 562) Part 10 of the frontend modernization roadmap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the /map page by extracting force-graph simulation/data orchestration (spec loading, KNN graph derivation, settling gate, search matching, and force tuning) out of the large MapPage.tsx component into a dedicated useForceGraphSimulation hook, while keeping MapPage focused on the ForceGraph2D canvas, paint callbacks, and ref-coupled handlers.
Changes:
- Added
useForceGraphSimulationhook encapsulating map spec fetch + graph/simulation state and returning ref-decoupled state/handlers + force config. - Simplified
MapPage.tsxby wiring the new hook outputs into the existing UI/canvas logic (includingonRepaint→fgRef.refresh()). - Moved/added tests to cover hook behavior (graph derivation, weights/minSim, settling gate, search, and outlier-squash force), and adjusted MapPage tests accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/pages/MapPage.tsx | Removes in-component simulation/data orchestration and wires the new hook outputs into ForceGraph2D + UI handlers. |
| app/src/pages/MapPage.test.tsx | Drops direct unit tests for the extracted force and retains MapPage integration assertions (force wiring, overlay behaviors, etc.). |
| app/src/pages/MapPage.helpers.ts | Updates module header to reflect shared usage between MapPage paint logic and the new hook. |
| app/src/hooks/useForceGraphSimulation.ts | New hook implementing spec fetch, graph derivation + caching, settling/tick state, search matching, and force configuration/factories. |
| app/src/hooks/useForceGraphSimulation.test.ts | New unit tests for the hook and the extracted outlier-squash force behavior. |
| app/src/hooks/index.ts | Exports the new hook and its public types from the hooks barrel. |
Comment on lines
+207
to
+208
| // Static by construction — a single frozen object keeps the identity stable | ||
| // across renders so consumers can safely list it in dependency arrays. |
Owner
Author
There was a problem hiding this comment.
Applied — FORCE_CONFIG is now Object.freeze()d, so the comment is literally true.
Comment on lines
+522
to
+527
| const handleEngineTick = () => { | ||
| tickCountRef.current += 1; | ||
| const PROGRESS_TICK_BATCH = 6; | ||
| if (tickCountRef.current % PROGRESS_TICK_BATCH === 0) { | ||
| setTickProgress(Math.min(1, tickCountRef.current / COOLDOWN_TICKS)); | ||
| } |
Owner
Author
There was a problem hiding this comment.
Applied — tick progress is denominated in FORCE_CONFIG.cooldownTicks now.
Copilot review findings: the 'frozen' comment is now literally true via Object.freeze, and handleEngineTick derives the progress denominator from FORCE_CONFIG.cooldownTicks instead of the raw constant so the progress bar can't drift from the configured simulation lifetime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MarkusNeusinger
added a commit
that referenced
this pull request
Jun 10, 2026
…tion (#8596) ## Summary Part 11 (final) of the frontend modernization roadmap. - **`app/ARCHITECTURE.md`**: directory layout (routes/layouts/pages/sections/components/hooks/lib/theme), conventions (`src/` alias imports, `paths.*` URL registry, `lib/api` client, theme tokens, feature folders), data flow, testing pattern, quality gates, and how-to-add recipes for pages/sections/hooks/endpoints - **`agentic/docs/project-guide.md`**: frontend section now lists the full gate (`lint`, `fm:check`, `type-check` incl. tests, `test`), mentions the dev-time checker overlay, and links the new architecture doc With this, the roadmap is complete: tooling baseline (#8519), src/ aliases (#8520), theme split (#8525), global-config (#8529), API client (#8531), routes registry (#8546), structure split (#8547), FilterBar (#8559), SpecTabs (#8564), MapPage hook (#8581). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part 10 of the frontend modernization roadmap. Extracts the force-graph simulation orchestration from the 1689-line
MapPage.tsxinto a dedicated hook — no behavior change.src/hooks/useForceGraphSimulation.ts(556 LOC): specs loading, weights/minSim state, settled/tick gating, node cache, search haystacks/matches, graph derivation (viaMapPage.helpers), and a staticforceConfig(engine params, link distance/strength, collide + outlier-squash force factories). Ref-decoupled — returns plain state/handlers; the single inversion is anonRepaintcallback (thumbnail loads) that MapPage wires tofgRef.current.refresh().MapPage.tsx: 1689 → 1292 LOC; keeps ForceGraph2D, canvas paint callbacks, UI overlays (search, legend, weights panel), and the ref-coupled handlers (camera fit, flyTo). The remaining bulk is JSX overlay markup — further extraction would be UI decomposition, out of scope here.hooks/barrel.Tests: 598 → 608.
Verification
yarn lint✓ (one pre-existing react-refresh warning in MapPage resolved by the move) ·yarn fm:check✓ ·yarn type-check✓ ·yarn test608/608 ✓ ·yarn build✓🤖 Generated with Claude Code