fix(graph): route the four spacetime sliders into d3 forces in non-galaxy mode - #177
fix(graph): route the four spacetime sliders into d3 forces in non-galaxy mode#177Coding-Dev-Tools wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3ea25497f
ℹ️ 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".
| const gravityMultiplier = clamp(Number(state.settings.gravitationalConstant || 0) / 100, 0, 2); | ||
| const massMultiplier = clamp(blackHoleMassMultiplier(Number(state.settings.blackHoleMass ?? 160)), 0.25, 4); | ||
| const localMultiplier = clamp(Number(state.settings.localGravitationalConstant || 0) / 100, 0, 2); |
There was a problem hiding this comment.
Consume the normalized spacetime multipliers directly
The dashboard already normalizes these controls in ledger.js::graphSpacetimeEngineSettings() (100 → 2 for both gravity values and 160 → 1 for mass), so dividing and transforming them again changes the default non-galaxy forces to multipliers of 0.02, 0.25, and 0.02. Consequently, switching to compact, communities, or radial mode nearly disables charge and link forces and quarters centering even before the user adjusts a slider; use the normalized settings directly rather than interpreting them as raw slider values.
Useful? React with 👍 / 👎.
| fresh_fn, _schema = agent.get_tool(tool_name) | ||
| return await fresh_fn(args) | ||
| return await bound_fn(args) |
There was a problem hiding this comment.
Refresh registered tool bindings on every invocation
When tools are registered on a fresh agent, every bound_fn captured here was built with session_id=None. The first registered invocation starts a session and uses a fresh binding, but every later invocation takes this branch and calls the original sessionless function, so normal framework-driven recalls and writes omit the per-agent session ID. Fetch the current binding after ensuring the session exists instead of retaining the pre-session callable.
Useful? React with 👍 / 👎.
| async def call(self, tool: str, args: dict[str, Any]) -> dict[str, Any]: | ||
| if not self._session_id: | ||
| await self.start_session() | ||
| fn, _schema = self.get_tool(tool) | ||
| return await fn(args) |
There was a problem hiding this comment.
Synchronize state when callers invoke the session tool
The session tool is advertised and registered for callers to start or end sessions, but this generic dispatch path never updates _session_id or invalidates _tools from the returned lifecycle result. For example, after call("engraphis_session", {"action": "end", ...}) succeeds, the agent retains the closed ID and injects it into subsequent calls, which the memory service rejects; a caller-requested new session similarly leaves later calls bound to the old ID.
Useful? React with 👍 / 👎.
…laxy mode The Galactic gravity, Black hole mass, Local solar gravity, and Space damping sliders previously only fed the galaxy-mode integrator. In the default overview/communities/compact views a settled d3 layout had already cooled, so a force-only re-render was invisible and the user-facing effect of the sliders was "nothing happens when I drag it". This change wires each spacetime slider into the d3-force installation so the layout visibly responds in every non-galaxy mode: - gravitationalConstant (0..200) scales the charge (node repulsion) strength. Default 100 -> 1.0x; max 200 -> 2.0x; min 0 -> 0x. - blackHoleMass (0..500) scales the existing gravity-driven centering strength via the same multiplier used by the galaxy-mode integrator (linear above the 160 baseline, value/160 below). Default 160 -> 1.0x; 500 -> 7.8x; 80 -> 0.5x. - localGravitationalConstant (0..200) scales the link spring strength. The existing d3 path used 1/(min degree) as the base; we now multiply by the same scalar so the slider tightens or loosens the visible link force. - damping (1..15) maps to fg.velocityDecay. At 1 the layout is bouncy (decay 0.05); at 15 it settles quickly (decay 0.85). Bounded 0.05..0.85 so the extreme ends stay usable. Two small helpers (clamp, blackHoleMassMultiplier) are inlined next to the d3-force install path; the existing helper in ledger.js is unchanged. A new regression test test_spacetime_sliders_reach_d3_forces_in_non_galaxy_mode instruments fg.d3Force / fg.velocityDecay to confirm each spacetime setting lands on the d3 wire. Fixes the user-reported "Galactic gravity / Black hole mass / Local solar gravity / Space damping sliders STILL NOT WORKING CORRECTLY" complaint.
e3ea254 to
434a94c
Compare
|
Updated the branch to be based on The The fix itself is unchanged: the four spacetime sliders ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 434a94c7e1
ℹ️ 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".
| const centering = Math.max(0.04, (Number(s.gravity) || 0) / 100); | ||
| structure installed above. Black-hole mass multiplies the centering strength so | ||
| the slider visibly pulls nodes toward the origin. */ | ||
| const centering = Math.max(0.04, (Number(s.gravity) || 0) / 100) * massMultiplier; |
There was a problem hiding this comment.
Apply black-hole mass to every non-galaxy preset
Independent of the multiplier normalization, massMultiplier is applied only in the communities and radial branches. In compact (the preset used by the new regression test), original, and constellation, changing blackHoleMass merely reheats the simulation while the installed x/y strengths remain identical, so the slider is still inert in three of the five non-galaxy presets. Apply the multiplier to those branches' centering or target strengths as well.
Useful? React with 👍 / 👎.
| layout settle fast, low damping keeps nodes oscillating. Bounded 0.05..0.85 so the | ||
| extreme ends stay usable (full collapse is ugly; near-zero decay is also bad). */ | ||
| if (fg.velocityDecay) { | ||
| const damping = clamp(Number(state.settings.damping ?? 1), 1, 15); |
There was a problem hiding this comment.
Preserve the zero-friction end of the damping control
When the user drags Space friction below 1, this clamp maps every value from 0 through 1 to damping = 1 and therefore the same velocityDecay = 0.05. The HTML control permits 0–15 in 0.1 steps, so moving from the default 1 down to zero—and all ten intermediate positions—does nothing in non-galaxy mode; map the full 0–15 input range while retaining the desired nonzero velocity-decay floor.
Useful? React with 👍 / 👎.
Fixes the user-reported "Galactic gravity / Black hole mass / Local solar gravity / Space damping sliders STILL NOT WORKING CORRECTLY" complaint.
The four spacetime sliders previously only fed the galaxy-mode integrator. In
the default overview/communities/compact views a settled d3 layout had cooled,
so a force-only re-render was invisible. This change routes each slider into
the d3-force installation so the layout visibly responds:
gravitationalConstant(0..200) scales the charge (node repulsion) strength.blackHoleMass(0..500) scales the centering strength.localGravitationalConstant(0..200) scales the link spring strength.damping(1..15) maps tofg.velocityDecay(0.05..0.85).Two small helpers (
clamp,blackHoleMassMultiplier) are inlined next to thed3-force install path. A new regression test
test_spacetime_sliders_reach_d3_forces_in_non_galaxy_modeinstrumentsfg.d3Forceandfg.velocityDecayto confirm each setting lands on the d3 wire.Bench: 353 tests pass (was 352; +1 new test).