feat: Galaxy v6 physics engine, graph scene overhaul, and ledger UI improvements - #138
feat: Galaxy v6 physics engine, graph scene overhaul, and ledger UI improvements#138Coding-Dev-Tools wants to merge 192 commits into
Conversation
…mprovements - Galaxy physics: orbital mechanics, leapfrog integration, black hole gravity, solar system hierarchy, evidence-mass sizing, drag velocity preservation - Graph scene: ghost edge sort fix, connected_only+include_history interaction, set-wise visibility refactor, code overlay fallback correction - Ledger UI: graph load caching fix, repo filter in cache key, accessibility improvements, Galaxy controls integration - Store: schema migration, logical digest header zeroing fix, verification scope correction, context_savings workspace coercion - Service: entity visibility filter, workspace N+1 query fix, history mode ghost flags, migration locking - API: graph scene fallback narrowing, whitespace workspace validation - E2E tests: Galaxy slider/half-step/orbital separation coverage - Unit tests: graph engine assets, explorer v2, scene contracts, benchmarks - Scripts: dashboard readiness probe, port conflict differentiation - Docs: changelog, MCP tools reference, skill package update Co-authored-by: review agents (16 parallel reviewers, 4 fix agents)
![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: 3f3b1f4993
ℹ️ 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".
- Restore schema cookie (offset 40) normalization in _logical_digest: SQLite backup can reset this field, causing digest mismatch and aborting all v3-v16 migrations. Codex P1. - Reserve edge capacity for historical ghost relations: when include_history=True, ghost edges covering historical nodes are selected first before applying the final edge_cap, preventing the time-travel view from losing all historical edges. Codex P2. - Update test_store_class_integrity to validate the corrected digest offsets (24, 40, 92).
- Replace U+2014 em dash with double hyphen in CHANGELOG.md line 73 - Fixes test_public_facing_docs_do_not_use_em_dashes assertion
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3e2fb6890
ℹ️ 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".
Prevents unbounded URL growth on persistent asset load failures. The retry counter appends to asset URLs as a cache-buster; without a cap, repeated failures produce increasingly long URLs and complicate debugging. Ten retries provides ample recovery attempts before giving up gracefully.
static/index.html (Ledger primary) incorrectly marked the Classic link as active with aria-current=page. Swap so Ledger shows as the active choice on the primary dashboard, matching the classic_assets variant which correctly marks Classic as active. Prevents a brief flash of incorrect state before JS init and fixes the HTML-only fallback.\n\nFrom Review13 medium finding.
- ledger.js: reject scene payloads carrying an error field before the object-fallback, preventing error response objects from being treated as graph data and causing downstream undefined property access. - dashboard_assets/index.html: add aria-labelledby to graph tuning range inputs referencing their label spans so screen readers announce dynamic label text changes in Galaxy mode. From Review08 low findings.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5dbcdbfcf
ℹ️ 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".
5 fixes for issues flagged by the Codex reviewer: - P1 service.py: historical supports query now filters by memory.workspace_id so a cross-workspace support cannot leak into the include_history scene. - P2 service.py: evidence facets (memory_types, time_from, time_to) are applied in history mode instead of being skipped by the live-only evidence_filter short-circuit. - P2 service.py: entity candidate cap applies after session-scope pruning so private evidence cannot crowd out public entities. - P1 engine.py: secure_erase re-checks successors after the potentially long index.delete and cleans up any new target IDs before calling store.secure_erase_memory. - P2 graph_scene.py: ghost canonical nodes that collide with a live canonical_id are keyed as :ghost so the live node keeps its mass, community, and relations. Co-authored-by: Codex review bot (addressed findings from d5dbcdb)
There was a problem hiding this comment.
💡 Codex Review
engraphis/engraphis/service.py
Lines 7817 to 7819 in 3160ada
Use historical validity when filtering ghost evidence
When include_history=true is combined with memory_types, time_from, or time_to, a support for a genuinely historical edge has valid_to <= t, so this newly added live-validity predicate rejects it and the ghost relation disappears. Fresh evidence beyond the prior facet comment is that the replacement branch still requires supports to be live at the selected world-time; apply the history-mode validity predicate here instead.
AGENTS.md reference: AGENTS.md:L162-L164
ℹ️ 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".
Move prune_entities assignment before its first reference in the evidence_filter computation. Ruff F821 caught the UnboundLocalError that broke all graph scene operations across every Python version.
9555936 to
0b2f070
Compare
The previous commit introduced a dependency on prune_entities in the evidence_filter assignment that broke live-only evidence filtering when memory_types/time facets were requested without include_history. Restore the original 'not include_history' logic; the prune_entities override for history mode is handled separately below. Fixes test_graph_scene_filters_supporting_memory_type_and_time_window.
Move the entity visibility check from a per-chunk correlated SQL query to a single upfront workspace-wide edge visibility scan. Entity rows are then filtered in Python using set membership, avoiding the extra SQL query that broke test_graph_scene_entity_visibility_single_query. This ensures private entities cannot consume the candidate budget when a workspace has more than 3*MAX_GRAPH_ANALYSIS_ENTITIES rows.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46a1d3c220
ℹ️ 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".
savingsQuery() returned empty string for the 'all' preset, producing URLs like /context-savings? with a dangling query separator. Move the ? prefix into the returned string so the 'all' case produces a clean /context-savings path with no query component. Fixes one of the remaining low-severity review findings.
The 'Ledger deadline includes stalled graph assets' E2E test was flaky on CI runners under load. The 5s default Playwright timeout for the reload+render cycle was too tight. Increase to 15s to match other graph-dependent assertions in the suite.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad1a5498e4
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5524fbd798
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
engraphis/engraphis/service.py
Lines 10448 to 10449 in 107492b
Apply bi-temporal visibility to health metrics
When sync has stored a row whose ingested_at is in the future, /analytics/health immediately includes that not-yet-known memory in both the decay distribution and orphan count because this live predicate never checks ingestion time. It also excludes a row prematurely whenever expired_at is non-null but still in the future. Use the same current world/system-time visibility predicate as ordinary reads so these health metrics describe memories visible now.
AGENTS.md reference: AGENTS.md:L204-L206
ℹ️ 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".
| "AND (ingested_at IS NULL OR ingested_at<=?) " | ||
| "AND (expired_at IS NULL OR ?<expired_at)" | ||
| ) | ||
| memory_params.extend([t, t, known_t, known_t]) |
There was a problem hiding this comment.
When valid_at is later than known_at, the second parameter here compares valid_to_recorded_at with world time instead of system time. For example, a support closed in 2025 but not recorded until 2028 should remain visible for valid_at=2030&known_at=2020; the support query retains it correctly, but this metadata lookup drops it, causing the supported relation to disappear from the scene. Pass known_t for the second placeholder.
AGENTS.md reference: AGENTS.md:L204-L206
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fba58717b7
ℹ️ 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".
| if len(combined) > chunk_size: | ||
| content_budget = max(chunk_size - len(prev_tail) - 1, 0) | ||
| combined = prev_tail + ( | ||
| " " + chunks[i][:content_budget] if content_budget else "" | ||
| ) |
There was a problem hiding this comment.
When a non-first chunk is close to chunk_size, especially when it is the final chunk, slicing chunks[i][:content_budget] discards its suffix rather than moving it into another chunk. For example, chunking w00 … w23 with chunk_size=20 and overlap=8 omits w23 entirely because no following overlap can recover the truncated tail, so callers embed incomplete documents. Fresh evidence beyond the prior overlap report is that this new reserve-overlap branch now causes source-data loss; rechunk the content instead of truncating it.
Useful? React with 👍 / 👎.
| workspace: str | ||
| repo: str | ||
| changed_files: list[str] | ||
| changed_files: list[str] = Field(default_factory=list, max_length=2_000) |
There was a problem hiding this comment.
When POST /api/v2/code/impact omits changed_files, this default supplies an empty list; _clean_string_list() accepts it and analyze_impact() still loads and analyzes the entire bounded code graph before returning a misleading successful low-risk result. This also allows a malformed request to consume the default 10,000-node traversal capacity, whereas the read-only counterpart requires at least one changed file; keep this field required and enforce a minimum list length.
Useful? React with 👍 / 👎.
…ysics tolerances - Ledger cache-buster: stable-orbit-lanes-6 → orbital-speed-1 - Ledger repel label: 'Orbital separation' → 'Orbital speed' - Graph-engine radius tolerance: 1.3x → 1.5x (orbital speed scales local radius) - Graph-engine maxSpeed: 48 → 52 (orbital speed control adds modest velocity)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0d2e06c8a
ℹ️ 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".
| uri_path = unquote(parsed.path) | ||
| if parsed.netloc and parsed.netloc != "localhost": | ||
| uri_path = f"//{parsed.netloc}{uri_path}" | ||
| return str(Path(url2pathname(uri_path)).expanduser()) |
There was a problem hiding this comment.
When ENGRAPHIS_DB_PATH is a regular SQLite URI with options such as file:/data/engraphis.db?mode=ro, ?mode=rw, or ?immutable=1, this conversion discards the entire query before Store opens the database. Consequently, a read-only database can be opened writable, and mode=rw can silently create a missing database instead of failing; preserve the URI options while deriving a separate physical path only for filesystem operations such as migration locking.
Useful? React with 👍 / 👎.
…ions for connection 6a112d4 correctly preserves SQLite URI options (mode=ro/rw, immutable) for the connection, but left self.path as the raw file: URI string. test_memory_service_create_normalizes_file_uri expected store.path to be a plain filesystem path. Normalize self.path through _physical_sqlite_path when the input is a file: URI, so store.path is always a filesystem path. The connection still receives the original path parameter with all URI options intact.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da9ed45c60
ℹ️ 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".
| if is_local_mode() and control_url is None and compute_url is None: | ||
| # A missing manifest endpoint should retain the normal configuration error; | ||
| # otherwise pure-local mode reports the wrong problem before preflight runs. | ||
| if default_control_url(): |
There was a problem hiding this comment.
On a fresh installation with no hosted environment variables—the exact state targeted by the documented engraphis connect --token engr_ct_... command—default_control_url() returns the shipped manifest endpoint, so this condition always raises 409 before preflight or token redemption. This also contradicts the CLI help, which says the shipped endpoint is the default; either treat invoking connect as explicit cloud opt-in or update every onboarding command to require --control-url.
Useful? React with 👍 / 👎.
| for var in _HOSTED_MODE_ENV_VARS: | ||
| if os.environ.get(var, "").strip(): | ||
| return "hosted" | ||
| return "local" |
There was a problem hiding this comment.
When a user connects with an explicit --control-url, the credential and endpoints are persisted in ~/.engraphis/cloud_session.json, and subsequent processes are intentionally able to use that session without environment secrets via cloud_session.configured(). Because this detector checks only environment variables, a later dashboard started without those variables reports local, causing /api/auth/state to expose hosted_team=false, an empty cloud URL, and a misleading LOCAL indicator even while the saved cloud session is active; include the validated persisted-session state in mode detection.
AGENTS.md reference: AGENTS.md:L67-L71
Useful? React with 👍 / 👎.
1 participant
Summary
Galaxy v6 physics, graph-scene projection, and Ledger dashboard overhaul. The PR contains 58 changed files (+17,766 / -1,342) on one branch targeting
main.What changed
0..400; the live solver uses a0.032fixed timestep, physical stellar-surface pressure, and bounded 18-unit global orbit seeding, while oversized static scenes map the full slider range.Review hardening
0..400range.Validation — final head
805cd72ruff check .— passedpyright— passedpython scripts/externalize_dashboard_assets.py— passedpython scripts/check_commercial_manifest.py— passedpython -m pytest -o addopts="" tests -q -rs— 4,026 passed, 35 platform/optional-extra skips, 1 expected duplicate-ZIP warning from the malformed-container security testnpx playwright test --reporter=list— 63 passed