fix(dashboard): refine Galaxy layout and physics response - #205
fix(dashboard): refine Galaxy layout and physics response#205Coding-Dev-Tools wants to merge 13 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 544aaa6616
ℹ️ 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".
| .forEach(key => { | ||
| const target = Number(item.carrier[key]); | ||
| if (Number.isFinite(target) && target > 0) { | ||
| item.carrier[key] = target * ratio; |
There was a problem hiding this comment.
When a gravity or central-field slider changes after carrier lanes have been established, item.nodes already contains item.carrier, so the loop above scales the carrier's __galaxy*Lane*Radius fields once and this second loop scales the same fields again. The carrier position moves by ratio, but its authoritative cached lane becomes ratio²; on the next physics slice, carrier support projects the whole system onto that incorrect radius, causing an extra jump and breaking the intended path-independent response. Remove the duplicate carrier scaling or exclude the carrier from the first cache update.
Useful? React with 👍 / 👎.
| { id: 'graph-repel', key: 'repel', fallback: 100 }, | ||
| { id: 'graph-link', key: 'link', fallback: 8 }, | ||
| { id: 'graph-gravity', key: 'gravity', fallback: 96 }, | ||
| { id: 'graph-gravity', key: 'gravity', fallback: 120 }, |
There was a problem hiding this comment.
For an existing user whose saved graph preferences contain the former default tuning.gravity: 96 and physicsVersion: 4, restoration spreads that saved value over this new preset, so the new 120 default never takes effect. Preference snapshots persist every tuning value even if the user only changed an unrelated graph option, making this affect users who never customized gravity. Bump the physics version and narrowly migrate the exact retired Galaxy default, as the existing 48/60 orbital-speed migration does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c231747ea
ℹ️ 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".
| ['orbit_radius', '__galaxyOrbitBaseRadius'].forEach(key => { | ||
| const val = Number(node[key]); | ||
| if (Number.isFinite(val) && val > 0) { | ||
| node[key] = val * localRatio; | ||
| } |
There was a problem hiding this comment.
When Galaxy is collapsed or exceeds the live-solver limit, satellites already carry a __galaxyKinematicLocalOrbit.baseRadius; this loop scales only orbit_radius and __galaxyOrbitBaseRadius. On the next kinematic frame, advanceGalaxyKinematicLocalMembers() reads the unchanged active cache and restores the old radius, so the new local-gravity feedback immediately snaps back in precisely the large/collapsed views that use the fallback. Scale or invalidate the kinematic cache as part of this response.
Useful? React with 👍 / 👎.
| const localGChanged = (next.localGravitationalConstant !== undefined || next.G_star !== undefined) | ||
| && Number.isFinite(previousLocalG) && Number.isFinite(nextLocalG) | ||
| && previousLocalG > 0 && nextLocalG > 0 | ||
| && Math.abs(nextLocalG - previousLocalG) > 1e-12 |
There was a problem hiding this comment.
When the valid Local solar gravity slider reaches 0 or moves away from 0, these positivity checks suppress the immediate radius response entirely. This also makes the response path-dependent: 2 → 1 expands satellites, while 2 → 0 → 1 leaves them unchanged because both transitions are rejected. Use a bounded endpoint mapping so zero participates without producing an infinite ratio.
Useful? React with 👍 / 👎.
| cachedPhysicsSnapshot = snapshot; | ||
| cachedPhysicsSnapshotStep = galaxySteps; | ||
| return snapshot; |
There was a problem hiding this comment.
These cache assignments are unreachable because the object literal is returned at line 10691, and snapshot is never defined. Consequently the cache-hit branch can never run, so while the spacetime overlay is active every animation frame still rebuilds community groups and allocates a full node snapshot—an avoidable O(n) cost on large graph views. Build a snapshot value, cache it, and then return it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfef126caf
ℹ️ 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 targetX = parent.x + unitX * targetRadius; | ||
| const targetY = parent.y + unitY * targetRadius; |
There was a problem hiding this comment.
When a nested satellite's authored lane crosses the black-hole horizon, applyGalaxyOrbitalSpeedControl() first clamps it to the horizon and applyGalaxyBlackHoleExclusion() confirms that boundary, but this post-clock guard then reconstructs targetX/targetY solely from the parent and authored radius. Because runGalaxyFrame() invokes this guard after its final black-hole exclusion, the satellite is moved back inside the painted horizon for the rendered frame. Constrain this target against the global anchor or run the horizon projection again after the lane repair.
Useful? React with 👍 / 👎.
| report.speedCapped = finalSpeed.maximumAfter > MAX_NODE_SPEED | ||
| + SPEED_LIMIT_DIAGNOSTIC_EPSILON; |
There was a problem hiding this comment.
Whenever the integrator or the new post-clock guard actually caps an excessive velocity, maximumAfter is guaranteed to be below MAX_NODE_SPEED, so this assignment overwrites the earlier report.speedCapped with false. Consequently the subsequent galaxySpeedCaps++ never records these events and physicsDiagnostics().speedCapActivations under-reports unstable frames; retain the integrator flag and/or use finalSpeed.applied rather than testing the already-projected speed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cfd3b3a6b
ℹ️ 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".
| ['galactic_target_radius', 'galactic_radius', 'galactic_preferred_radius', | ||
| '__galaxyCarrierLaneRadius', '__galaxyCarrierLaneBaseRadius', | ||
| '__galaxyCoreLaneRadius', '__galaxyCoreLaneBaseRadius'] |
There was a problem hiding this comment.
When a central-gravity or black-hole-mass slider changes after a collapsed or oversized Galaxy has run, this scales the carrier lane fields but omits the carrier's active __galaxyKinematicGlobalOrbit.baseRadius and .radius. The next advanceGalaxyKinematicOrbits() frame reconstructs the carrier radius from that stale cache and snaps the whole system back to its former lane. The new local-cache update addresses satellite caches but provides fresh evidence that the corresponding global carrier cache remains uncovered.
Useful? React with 👍 / 👎.
| if (!(Number.isFinite(Number(phase.localSpeed)) && Number(phase.localSpeed) > 1e-5) | ||
| || phaseMultiplierChanged) { | ||
| const seededSpeed = Math.abs(currentTangent); | ||
| phase.localSpeed = nestedCarrier ? requestedRelativeSpeed : seededSpeed > 1e-5 | ||
| ? Math.min(requestedRelativeSpeed, seededSpeed) : requestedRelativeSpeed; |
There was a problem hiding this comment.
After a planet's __galaxySpeedControlPhase.localSpeed has been initialized, changing localGravitationalConstant recomputes requestedRelativeSpeed but this condition refreshes the cache only when the orbital-speed multiplier changes. Consequently the post-integrator clock keeps forcing the old tangent indefinitely—even at G_star = 0—and overwrites the velocity produced by the newly configured stellar field. Include the local-gravity setting in the cache key or invalidate localSpeed when that control changes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc9155a96f
ℹ️ 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".
| second time against its planet leaves no tangent whenever that planet is near the | ||
| emergency ceiling, which makes only the deepest authored orbit appear frozen. The | ||
| carrier frame is capped below; preserve the differential moon velocity here. */ | ||
| const localSpeedLimit = nestedParent ? Number.POSITIVE_INFINITY : strictSpeedLimit; |
There was a problem hiding this comment.
When a collapsed or oversized Galaxy contains a nested moon and the local-gravity/orbital-speed controls are high, this Infinity branch allows the moon's full relative speed to be added to its already-moving planet. I checked the runGalaxyFrame() kinematic-fallback path: unlike the live-solver branch, it never calls enforceGalaxyGlobalSpeedLimit(), so valid maximum controls can produce moon velocities far above MAX_NODE_SPEED (for example, over 120 with a limit of 48), making fallback orbits run excessively fast. Apply a hierarchical speed budget here or cap the completed kinematic frame.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d29a7bc8d
ℹ️ 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 seededSpeed = Math.abs(currentTangent); | ||
| phase.localSpeed = nestedCarrier ? requestedRelativeSpeed : seededSpeed > 1e-5 | ||
| ? Math.min(requestedRelativeSpeed, seededSpeed) : requestedRelativeSpeed; |
There was a problem hiding this comment.
When a top-level planet already has a nonzero tangent and the user increases Local solar gravity, this refresh caps the new cache at seededSpeed, which is the pre-change tangent, so the stronger field can never select its higher requestedRelativeSpeed and the post-integrator clock keeps overriding the new physics. In the new 1→4 gravity regression scenario, the cache can even fall from about 52.6 to 25.8 rather than increase. Fresh evidence beyond the earlier cache-key issue is this newly added refresh branch using Math.min(requestedRelativeSpeed, seededSpeed); on an explicit gravity change, initialize from the new requested target and let the subsequent world-speed budget perform the cap.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3076de07c8
ℹ️ 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 nestedParentSpeedLimit = Math.max(1, absoluteSpeedLimit * 0.05); | ||
| const requestedParentSpeed = ownsNestedOrbit | ||
| ? Math.min(localTargetSpeed, nestedParentSpeedLimit) : localTargetSpeed; |
There was a problem hiding this comment.
When a planet has any visible child, such as a moon, this unconditionally limits its star-relative speed to 5% of the world cap—2.4 with the dashboard's 48-unit limit—even when the combined parent and child velocities would remain safely below 48. With the default gravity fixture, the same planet moves at about 20.7 without a moon but only 2.4 when the moon is present, so merely adding or revealing a descendant makes the parent orbit roughly eight times slower; reserve only the actual headroom needed for descendants instead of imposing this fixed ceiling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a37b8824ca
ℹ️ 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 fCentral = Math.max(0.05, gNorm * Math.sqrt(Math.max(0, mNorm))); | ||
| const rMod = Math.pow(fCentral, -0.65); |
There was a problem hiding this comment.
When the Galactic-gravity multiplier moves from its default 2 to the valid zero endpoint, the 0.05 floor makes rMod about 7, so carriers and their cached base radii are expanded far beyond the fixed far-field envelope. On the next collapsed/oversized kinematic frame, boundedRadius() clamps only the active radius while leaving the oversized base radius intact; moving the slider back then applies the reciprocal ratio to inconsistent values, so the carrier does not return to its original lane. Use an endpoint mapping compatible with the envelope or reconcile the base cache whenever the active radius is clamped.
Useful? React with 👍 / 👎.
| non_global = [c for c in ordered if str(c["id"]) != global_community_id] | ||
| non_global_count = len(non_global) |
There was a problem hiding this comment.
This replaces the carrier-placement algorithm and changes its spacing constants, but ALGORITHM_VERSION remains galaxy-v12-responsive-compact-orbits. Because coordinates are deliberately excluded from scene_hash and both hashes include the unchanged version string, identical graph inputs now produce different coordinates under the same scene_hash, layout_seed, and algorithm_version; consumers using those fields as the public scene/layout identity cannot distinguish the old and new layouts. Bump the algorithm version whenever changing this deterministic placement contract.
Useful? React with 👍 / 👎.
| return { bodies: bodies.length, systems, satellites, systemPacking, | ||
| blackHoleSpinAngle, ghostOrbit: integrateGalaxyGhostOrbits(nodes, opts) }; | ||
| blackHoleSpinAngle, ghostOrbit: integrateGalaxyGhostOrbits(nodes, opts), | ||
| maximumSpeed: finalSpeed.maximumAfter, speedCapped: finalSpeed.applied }; |
There was a problem hiding this comment.
When a collapsed or oversized Galaxy exceeds the world-speed limit, this new final cap correctly returns speedCapped: true, but runGalaxyFrame() increments galaxySpeedCaps only in the live-integrator branch and ignores the flag in the kinematicFallback branch. Consequently physicsDiagnostics().speedCapActivations remains zero precisely when this new fallback safety cap is firing, hiding instability in large/collapsed scenes; consume this flag in the kinematic branch as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc1e517cfd
ℹ️ 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 managedCarrierLane = carrier.__galaxyCarrierLaneManaged === true; | ||
| const desiredTangent = (managedCarrierLane | ||
| ? galaxyManagedCarrierTargetSpeed(field, radius, opts.orbitalSpeed, true) | ||
| : galaxyCarrierTargetSpeed(field, radius, opts.orbitalSpeed)) * sign; |
There was a problem hiding this comment.
When a managed carrier occupies a far lane and Orbital speed moves away from exactly 100, neutralPhase becomes false and this branch applies galaxyManagedCarrierTargetSpeed() to the carrier's actual velocity. That helper's radius * 0.039 minimum was introduced as a painted-phase visibility floor; using it here creates a discontinuous physical acceleration for tiny slider changes—for example, a radius-500 carrier is forced to at least 19.5 units at 101 even when its calibrated target is lower—and can trigger the global cap and perturb every local orbit. Use the calibrated physical carrier target here and reserve the managed minimum for advancing the cached painted phase.
Useful? React with 👍 / 👎.
| || galaxyLastLocalOrbitBoundary; | ||
| galaxyLastOrbitalCorrection = 0; | ||
| galaxyLastLocalVelocityLimits = 0; | ||
| if (report.speedCapped) galaxySpeedCaps++; |
There was a problem hiding this comment.
The new increment is confined to the kinematicFallback branch, while the corresponding increment was removed from the live-integrator branch. Consequently, whenever integrateGalaxyLeapfrog() or the new post-clock enforceGalaxyGlobalSpeedLimit() sets report.speedCapped during a normal live scene, physicsDiagnostics().speedCapActivations remains unchanged, hiding exactly the instability this diagnostic is meant to report. Increment the counter after the branch or retain an equivalent increment in both paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2f0acb65f
ℹ️ 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 center = galaxyGlobalAnchor(nodes); | ||
| const isPaused = state.settings.orbitPaused === true || state.settings.frozen === true | ||
| || !running || pageHidden(); | ||
| if (cachedPhysicsSnapshot && cachedPhysicsSnapshotStep === galaxySteps && cachedPhysicsSnapshotStep >= 0) { |
There was a problem hiding this comment.
When the spacetime overlay is active and a node is released between physics steps, finishNodeDrag() updates the node velocity and lastSlingshotRelease, then synchronously invokes the ledger callback, which calls getPhysicsSnapshot(). Because galaxySteps has not changed and this path does not invalidate the new cache, the branch returns the pre-release node velocities and stale slingshot value, so the overlay misses the release until a later solver step (and indefinitely if physics is paused immediately afterward). Invalidate the cache when drag/release state mutates, or include that state in the cache identity.
Useful? React with 👍 / 👎.
1 participant
Summary\n- distribute Galaxy systems across concentric tiers to reduce outer-ring crowding\n- refine orbital gravity/radius response and immediate local-gravity feedback\n- synchronize dashboard defaults and regression coverage\n\n## Validation\n- python -m pytest tests/test_graph_engine_asset.py tests/test_ledger_sliders_and_physics.py -q\n- ruff check engraphis/core/graph_scene.py tests/test_graph_engine_asset.py tests/test_ledger_sliders_and_physics.py\n- git diff --check