client: adopt official @modernrelay/omnigraph SDK + parallelize ego reads by ragnorc · Pull Request #2 · ModernRelay/notebook

ragnorc · 2026-06-09T16:00:26Z

What

Replaces colombo's hand-rolled omnigraph HTTP client with the official @modernrelay/omnigraph SDK, plus supporting fixes surfaced while wiring it against a live 0.6.x server.

Changes

@omnigraph/client → SDK transport.Client is now a thin facade over the SDK's Omnigraph class (canonical POST /query + POST /mutate). It keeps the existing Client / OmnigraphHttpError surface, so ServerSource and its tests are untouched, and reshapes the SDK's camelCase responses back to the existing snake_case ReadOutput / ChangeOutput. Removes the bespoke fetch/json() plumbing.
ServerSource.readEgo parallelized. The center + per-edge-type incident reads are mutually independent, so they now fire concurrently (Promise.all) instead of a sequential loop — collapses (k+1) round-trips into ~1 (measured ~4.6× faster over HTTPS).
Token fallback. TUI + client now fall back to OMNIGRAPH_BEARER_TOKEN when OMNIGRAPH_TOKEN is unset (the conventional omnigraph env var).
Web relative-server fix.web/config resolves a relative ?server=/og to an absolute origin URL — the SDK builds requests via new URL(base + path), which throws on a relative base (this is why the web path silently failed before).
Dev ergonomics. A dev-only /og Vite proxy (same-origin, avoids CORS); server-demo.sh doc updated to /query.
Tests. New http.test.ts covers the facade reshape and the OmnigraphHttpError message contract (401/network/conflict) that the web error-classifier matches on.

Verification

pnpm --filter @omnigraph/client typecheck && test — green (28 tests, incl. the unchanged source.test.ts).
pnpm -r typecheck && pnpm -r build — green; tui/web compile untouched.
Live: POST /query + POST /mutate against a 0.6 server (200 + correct shapes; mutation→/query → 400); TUI renders end-to-end; web vite build bundles the SDK cleanly.

Notes / follow-ups

React.StrictMode removed in web/main.tsx as a workaround: its dev double-invoke runs the effect cleanup, which calls runtime.dispose(), wedging the runtime on the loading skeleton. The proper fix is the App.tsx lifecycle (don't dispose on the StrictMode cleanup) — left as a follow-up.
Web loading is all-or-nothing (renders only when every cell settles) and the Select control's $bindState re-emits on each render → re-run loop. Both pre-date this PR; flagged for a separate fix.
ReadOutput.target type is string but the server returns {branch, snapshot}; coerced in the facade, proper typing deferred (ripples into runtime).

Note

Medium Risk
Core server I/O path changes (SDK + endpoint rename) with preserved error contracts; parallel ego reads increase concurrent load; StrictMode removal masks a lifecycle bug in dev only.

Overview
Replaces the hand-rolled omnigraph HTTP stack in @omnigraph/client with @modernrelay/omnigraph, while keeping the existing Client / OmnigraphHttpError surface so ServerSource, tests, and the web error-classifier stay on the same snake_case shapes and error message patterns. Transport now uses POST /query and POST /mutate via the SDK; the facade maps camelCase SDK responses back and re-wraps NetworkError / OmnigraphError (including AbortError passthrough). Public input types rename read/change to query/mutate with query/name instead of query_source/query_name.

ServerSource.readEgo runs center and incident reads concurrently with Promise.all instead of a sequential loop, reducing round-trips for ego graph cells.

Web/TUI/dev: relative ?server=/og bases are resolved to an absolute URL before constructing Client; TUI and Client accept OMNIGRAPH_BEARER_TOKEN when OMNIGRAPH_TOKEN is unset; Vite adds a dev /og proxy; React.StrictMode is temporarily removed in main.tsx; server-demo.sh documents /query and the new JSON body field query.

Tests: new http.test.ts covers response reshaping and classifier-aligned error messages.

^{Reviewed by Cursor Bugbot for commit ebc65c3. Bugbot is set up for automated code reviews on this repo. Configure here.}

Greptile Summary

Replaces the hand-rolled HTTP client in @omnigraph/client with the official @modernrelay/omnigraph SDK as the transport layer, while preserving the existing Client / OmnigraphHttpError surface for ServerSource and the web error-classifier. Accompanying fixes address relative-server URL resolution in the web, token env-var fallback, and a dev Vite proxy.

SDK facade (http.ts):Client now wraps the SDK's Omnigraph class; toHttpError re-wraps SDK error classes (NetworkError, OmnigraphError) back into OmnigraphHttpError to keep the message-contract regexes the web classifier depends on. ReadInput/ChangeInput are renamed QueryInput/MutateInput to match the new endpoints (/query, /mutate).
Parallelised ego reads (source.ts):readEgo fires the center query and all incident queries concurrently via Promise.all instead of a sequential loop, collapsing k+1 round-trips into ~1.
React.StrictMode removed (web/main.tsx): Documented as a temporary workaround — StrictMode's dev double-invoke triggers runtime.dispose(), keeping the page on the loading skeleton; the proper fix is deferred to App.tsx.

Confidence Score: 4/5

Safe to merge — the SDK swap is well-contained behind the existing Client facade, the error-contract tests confirm the web classifier regexes are preserved, and the ego-read parallelisation is behaviourally equivalent to the old sequential loop.

The changes are thorough and the test coverage for the new facade is solid. The two items worth a second look are the target coercion (acknowledged in the PR notes — falls back to main when server returns a null target, which could show the wrong branch in the UI if the server default is not main) and the CLAUDE.md description still pointing at the old /read and /change endpoints. Neither blocks merging, but the target fallback could confuse users on non-main default-branch setups.

CLAUDE.md (stale endpoint references); packages/client/src/http.ts line 97 (target fallback); packages/web/src/main.tsx (StrictMode removal, deferred fix noted).

Important Files Changed

Filename	Overview
packages/client/src/http.ts	Core facade replacing hand-rolled HTTP with SDK. Error re-wrapping, signal passthrough, and response reshaping are correct; target fallback to "main" is a known coercion.
packages/client/src/http.test.ts	New tests cover reshape, error wrapping (401/network/conflict), and the message-contract regexes the web error-classifier depends on.
packages/client/src/source.ts	readEgo parallelised with Promise.all — center and all incident queries fire concurrently; early-return for zero-edge case still works correctly since plan.incident is empty when query.out/in are empty.
packages/web/src/config.ts	Relative-server path now resolved to absolute URL via window.location.origin; correctly handles /path and /path?query forms.
packages/web/src/main.tsx	StrictMode removed as a documented workaround for runtime.dispose() being triggered by cleanup double-invoke; regression risk until App.tsx lifecycle is fixed.
CLAUDE.md	Not modified in this PR, but the package-map entry and ServerSource description still reference /read and /change — stale after the endpoint rename to /query and /mutate.

Sequence Diagram

sequenceDiagram
participant SS as ServerSource
participant C as Client (facade)
participant SDK as Omnigraph SDK
participant Server as omnigraph-server
Note over SS,Server: readEgo — parallel fan-out (new)
SS->>C: query(center, signal)
SS->>C: query(incident[0], signal)
SS->>C: query(incident[n], signal)
par Promise.all
C->>SDK: og.query(centerInput, signal)
SDK->>Server: POST /query (center)
Server-->>SDK: queryName, target, rows
SDK-->>C: camelCase response
C-->>SS: ReadOutput snake_case
and
C->>SDK: og.query(incidentInput, signal)
SDK->>Server: POST /query (incident)
Server-->>SDK: queryName, target, rows
SDK-->>C: camelCase response
C-->>SS: ReadOutput snake_case
end
SS->>SS: merge center + incident rows
Note over SS,Server: mutate
SS->>C: mutate(body, signal)
C->>SDK: og.mutate(input, signal)
SDK->>Server: POST /mutate
Server-->>SDK: branch, queryName, affectedNodes
SDK-->>C: camelCase response
C-->>SS: ChangeOutput snake_case
Note over C,SDK: Error path
SDK--xC: NetworkError / OmnigraphError / AbortError
C->>C: toHttpError() wraps to OmnigraphHttpError
C--xSS: OmnigraphHttpError message contract preserved

_{Reviews (1): Last reviewed commit: "client: adopt @modernrelay/omnigraph SDK..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used:

Context used - CLAUDE.md (source)

…web fixes - @omnigraph/client: replace the hand-rolled fetch client with a thin facade over the official @modernrelay/omnigraph SDK (canonical POST /query + /mutate). Keeps the Client / OmnigraphHttpError surface so ServerSource and its tests are unchanged; reshapes the SDK's camelCase responses back to the existing snake_case ReadOutput / ChangeOutput. - ServerSource.readEgo: fire the center + per-edge incident reads concurrently (Promise.all) instead of a sequential loop — collapses (k+1) round-trips to ~1. - tui + client: fall back to OMNIGRAPH_BEARER_TOKEN when OMNIGRAPH_TOKEN is unset. - web/config: resolve a relative `?server=/og` to an absolute origin URL — the SDK builds requests via new URL(base+path), which requires an absolute base. - web: dev-only `/og` Vite proxy; drop React.StrictMode (workaround, see PR). - http.test.ts: cover the facade reshape + the error-message contract.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:ebc65c3c1b

ℹ️ 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".

chatgpt-codex-connector · 2026-06-09T16:03:13Z

 "test": "vitest run"
 },
 "dependencies": {
+ "@modernrelay/omnigraph": "^0.6.0",


Restore Node 20-compatible client installs
This new SDK dependency raises the effective runtime floor for @omnigraph/client: the root package still advertises engines.node: >=20, but the lockfile records @modernrelay/omnigraph@0.6.0 with engines: {node: '>=22'}. In Node 20/21 environments that follow the repo's advertised support, engine-strict installs/deploys will now fail before the TUI/client can run; please either bump the workspace/package engine to Node 22+ or depend on an SDK build that supports Node 20.
Useful? React with 👍 / 👎.

greptile-apps · 2026-06-09T16:06:26Z

+ );
+ return {
+ query_name: r.queryName,
+ target: r.target?.branch ?? r.target?.snapshot ?? "main",


Silent "main" fallback when server returns no target
r.target?.branch ?? r.target?.snapshot ?? "main" silently coerces a {branch: null, snapshot: null} response (or a missing target field) to the string "main". If the server is operating on a non-main default branch, the UI will display the wrong branch name for the query result. The PR notes defer the proper fix, but it's worth tracking that the fallback can produce actively wrong output, not just a missing label.

devin-ai-integration

Devin Review found 2 potential issues.

View 2 additional findings in Devin Review.

devin-ai-integration · 2026-06-09T16:07:44Z

 "test": "vitest run"
 },
 "dependencies": {
+ "@modernrelay/omnigraph": "^0.6.0",


🚩 SDK requires Node ≥22 but repo declares Node ≥20
The new dependency @modernrelay/omnigraph@0.6.0 declares engines: { node: '>=22' } in the lockfile (pnpm-lock.yaml:239), while the root package.json:9 declares "node": ">=20". Someone running Node 20 or 21 (which satisfies the repo's engine constraint) would have an officially unsupported SDK. In practice engines is advisory unless engine-strict=true is set, and the SDK may well work on Node 20, but this is worth reconciling.
Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration · 2026-06-09T16:07:45Z

+// NOTE (dev workaround): StrictMode's double-invoke runs the effect cleanup,
+// which calls runtime.dispose() — disposing the runtime mid initial-run so it
+// never notifies and the page stays on the loading skeleton. Disabled here
+// while pointing at a real server. Real fix belongs in App.tsx lifecycle.
+createRoot(root).render(<App />);


🚩 StrictMode removal is a workaround, not a fix
The PR removes React.StrictMode wrapping with a comment explaining that double-invoke cleanup disposes the runtime prematurely. The comment itself acknowledges this is a dev workaround and that the real fix belongs in App.tsx lifecycle (e.g., using a ref to track whether dispose should actually run, or re-creating the runtime on re-mount). This isn't a bug in the PR per se, but losing StrictMode means double-render bugs won't be caught during development.
Was this helpful? React with 👍 or 👎 to provide feedback.

- Text/Quote `text_column` is now required (z.string().min(1)) — a Text/Quote cell with rows but no text column was silently rendering the empty fallback instead of failing validation. (#3) - validate: warn when a *required* (non-nullable) catalog param is bound to $state without a default — it resolves at runtime, which validate can't see, so flag it rather than passing silently. (#2) - validate: document that the server-bound checks are intentional — structural parse already runs offline above; the meaningful ref/param validation needs a resolvable source by design. (#1, accepted-by-design) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

chatgpt-codex-connectorBot reviewed Jun 9, 2026
View reviewed changes

ragnorc merged commit dd20883 into mainJun 9, 2026
1 of 2 checks passed

greptile-appsBot reviewed Jun 9, 2026
View reviewed changes

devin-ai-integrationBot reviewed Jun 9, 2026
View reviewed changes

ragnorc mentioned this pull request Jun 9, 2026
web: StrictMode-safe runtime lifecycle + progressive rendering + state dedupe #3
Open

Navigation Menu

Uh oh!

client: adopt official @modernrelay/omnigraph SDK + parallelize ego reads - #2

client: adopt official @modernrelay/omnigraph SDK + parallelize ego reads#2
ragnorc merged 1 commit into
mainfrom
ragnorc/dac-to-colombo-port

ragnorc commented Jun 9, 2026•
edited by greptile-apps Bot
Loading

Uh oh!

chatgpt-codex-connectorBot left a comment

Uh oh!

chatgpt-codex-connectorBotJun 9, 2026

Uh oh!

Uh oh!

greptile-appsBotJun 9, 2026

Uh oh!

devin-ai-integrationBot left a comment

Uh oh!

devin-ai-integrationBotJun 9, 2026

Uh oh!

devin-ai-integrationBotJun 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

ragnorc commented Jun 9, 2026• edited by greptile-apps BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Changes

Verification

Notes / follow-ups

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

chatgpt-codex-connectorBot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connectorBotJun 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-appsBotJun 9, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integrationBot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integrationBotJun 9, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integrationBotJun 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ragnorc commented Jun 9, 2026•
edited by greptile-apps Bot
Loading