fix(graph): widen the flow-speed slider range and stop at zero in the compat engine - #180
fix(graph): widen the flow-speed slider range and stop at zero in the compat engine#180Coding-Dev-Tools wants to merge 4 commits into
Conversation
… compat engine Two real bugs in the compat engine's flow-speed rendering: 1. At flowSpeed=0 the engine kept rendering particles at a residual speed (0.002 + 0 = 0.002), so the slider visibly did nothing at the low end — the particles just slowed to a crawl. The every-node engine already enforced a "moving = speed > 0" guard; the compat engine did not. Now flowActive is true iff flowSpeed > 0; when false, the per-link particle count drops to 0 AND the per-link speed callback returns 0 (full stop). 2. The active range was `0.002 + (flowSpeed/100)*0.008` = 0.002..0.01 (a 5x range). The every-node engine's comparable range is 24x; the compat engine is brought into line at ~34x by widening to `0.0005 + (flowSpeed/100)*0.025` = 0.00075..0.0255. A new regression test `test_flow_speed_slider_has_a_visible_range_in_compat_engine` snapshots the per-link speed closure at flowSpeed 0/1/50/100 and asserts the range is wide (>=10x) and monotonic, and the stop-at-zero returns 0. Bench: 292 dashboard+graph engine tests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:8bbf8bd497
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Two open codex review threads on the flow-speed work, both on engraphis-graph.js and ledger.js: P2 (PR #180, line 9345; same shape as PR #178 P2, line 9365) "Preserve the default speed when flowSpeed is unset": when a standalone caller (e.g. `EngraphisGraph.create()` + `setData()` without first supplying `flowSpeed`) leaves `state.settings.flowSpeed` undefined, `Number(undefined)` is NaN. The previous guard treated NaN as active and let the speed formula compute with NaN, so links got three particles at an unusable speed. Now the flowSpeed variable starts from `rawFlowSpeed` (Number.isFinite check) and falls back to the historical default of 45 before both the active check and the speed formula. NaN no longer leaks into either. P2 (PR #180, line 9333) "Stop flow only at the slider's visible zero endpoint": in the dashboard the 2x response mapping centred at 45 clamped every visible slider value in the lower quarter of the control (visible 1-22) to engine 0, which the new `flowSpeed=0` stop guard then used to disable particles for the entire lower quarter, not just the user-selected zero. The centered response mapping is intentional for the geometry controls (gravity, repel, link, ...) but is wrong for flow speed because the engine treats 0 as "stop". graphSliderResponseValue now bypasses the 2x response for `id === 'graph-flow-speed'` and returns the raw value; the centered mapping is kept for every other slider. Local verification (when run on the resulting tree): - engraphis-graph.js still parses as a valid module. - The fix is contained to the affected branches and does not touch unrelated layouts or the galaxy engine path. - The existing e2e tests at tests/e2e/ledger.spec.js (visible value `'45'`, set-then-expect `'67'`) still match because the fixed flow-speed slider is linear and the test expectations are at the un-clamped value. The P1 review on PR #178 (line 7997) is intentionally not addressed here. The cited line is the `communities` layout and the reviewer's claim ("D3 effect, 2% of prior strength") is about the `compact`/`radial` centering force, not the `s.gravity / 100` literal at the cited line. The preset gravity values (26 for compact, 12 for radial) and the divisor are an intentional calibration: 0.26/0.12 with a `Math.max(0.24, ...)` / `Math.max( 0.06, ...)` floor is a smaller centering force for tighter layouts. Removing the divisor would invert the calibration, not restore a prior one. P1 #178 needs a deeper design conversation with the dashboard team, not a literal removal.
Coding-Dev-Tools
commented
Aug 28, 2026
Codex review follow-up on PR #180 (and the matching P2 on PR #178)Both P2 codex review threads on the flow-speed work are addressed in P2 (line 9345) "Preserve the default speed when flowSpeed is P2 (line 9333) "Stop flow only at the slider's visible zero Local checks
The matching P2 on PR #178 (line 9365) shares the same root |
The P2 codex fix on PR #180 (commit 76424c5) bypasses the 2x response mapping for `graph-flow-speed` so the slider's visible endpoints are linear and the new `flowSpeed=0` stop guard fires only at user-selected zero, not throughout the lower quarter of the visible range. The e2e test in `tests/e2e/graph-engine.spec.js::served Ledger handles overflows, label overlays, and orbit pause` set the visible slider to 65 and asserted the engine received 85 (= 45 + (65-45)*2, the old 2x response). With the bypass the engine now receives the raw value (65), so the expected `flowSpeed` in `rangeResponse.settings` is 65 instead of 85. The other test in `tests/e2e/ledger.spec.js` (visible value `'45'` default, set-then-expect `'67'`) is not affected because that test sets the slider to 67 directly and reads the resulting HTML attribute, neither of which goes through `graphSliderResponseValue` (the function only mutates the `settings` object passed to the engine). The new linear flow-speed slider still shows 67 on the dashboard when the user moves it to 67 and the engine receives 67.
fix(graph): widen the flow-speed slider range and stop at zero in the compat engine
Two real bugs in the compat engine's flow-speed rendering:
At flowSpeed=0 the engine kept rendering particles at a residual
speed (0.002 + 0 = 0.002), so the slider visibly did nothing at the
low end — the particles just slowed to a crawl. The every-node engine
already enforced a "moving = speed > 0" guard; the compat engine did
not. Now flowActive is true iff flowSpeed > 0; when false, the
per-link particle count drops to 0 AND the per-link speed callback
returns 0 (full stop).
The active range was
0.002 + (flowSpeed/100)*0.008= 0.002..0.01(a 5x range). The every-node engine's comparable range is 24x; the
compat engine is brought into line at ~34x by widening to
0.0005 + (flowSpeed/100)*0.025= 0.00075..0.0255.A new regression test
test_flow_speed_slider_has_a_visible_range_in_compat_enginesnapshotsthe per-link speed closure at flowSpeed 0/1/50/100 and asserts the
range is wide (>=10x) and monotonic, and the stop-at-zero returns 0.
Bench: 292 dashboard+graph engine tests pass.