refactor(cli): collapse export + graphs-list onto GraphClient (RFC-009 Phase 3c)

[aaltshuler]

The last two embedded-vs-remote forks move onto the GraphClient enum, so every such if in the CLI now lives in client.rs — the point of RFC-009 Phase 3. Builds on 3a (#210) and 3b (#211).

What moved

• export<W: Write> — the streaming verb 3b deferred (writes to a writer, chunks the HTTP response body, rather than returning a DTO). Embedded calls db.export_jsonl_to_writer; Remote streams the chunked body through. Opens WITHOUT policy (like reads), so it routes via resolve().
• list_graphs — remote-only by design (no local enumeration endpoint), so the Embedded arm keeps the loud "requires a remote multi-graph server" bail verbatim. Routing it through the enum still buys the shared resolve() addressing/token preamble the arm hand-rolled.

Cleanup

Retire the now-orphaned execute_export_to_writer / execute_export_remote_to_writer pair, and sweep two pre-existing dead fns: inferred_config_path (helpers.rs) and yaml_string (output.rs, shadowed by test-local copies).

Parity

parity_matrix gains one row, parity_export — the single intended matrix change in this phase. Export is a JSONL stream, not a single --json doc, so it compares the two arms line-wise (sorted; twin graphs are byte-copies, so rows need no scrubbing). graphs list gets no row — its remote-only behavior is a documented exclusion, not an equality case.

Verification

• All 12 parity rows green (incl. parity_export).
• cli_data green (export e2e), warning-clean build.
• Full cargo test --workspace --locked → exit 0, zero failures (run locally — Test Workspace no longer runs on PRs per ci: run Test Workspace only on main, not on pull requests #212).

Out of scope: Phase 4 (plane capability advertisement) and Phase 5 (route alignment).

🤖 Generated with Claude Code

Greptile Summary

This PR completes RFC-009 Phase 3 by moving the last two embedded-vs-remote forks — export and graphs list — onto the GraphClient enum, so every CLI is_remote branch now lives in client.rs. It also retires four orphaned/dead helpers (execute_export_to_writer, execute_export_remote_to_writer, inferred_config_path, yaml_string) and adds the parity_export row to the parity matrix.

• export method (client.rs): streams JSONL into a W: Write; Remote arm chunks the HTTP response body without buffering; Embedded arm opens without policy (consistent with reads), calls Omnigraph::open directly — both arms are direct transliterations of the removed helpers with no behavioral change.
• list_graphs method (client.rs): Remote-only by design; Embedded arm preserves the verbatim error message from the old main.rs guard, now buying the shared resolve() addressing/token preamble for free.
• parity_export test: Uses a sorted line-wise comparison instead of the usual single-doc scrub, with a non-empty guard to prevent the check from being vacuous; only a minor diagnostic gap when the remote arm returns an empty body (caught by the final assert_eq!, but with a less specific message).

Confidence Score: 4/5

Safe to merge; the refactor is a mechanical transliteration with no behavioral changes to either the export stream or the graphs-list error path.

The core logic changes are direct port-overs of removed helpers into GraphClient match arms, with the old and new code paths identical line-for-line. The only observation is a minor diagnostic gap in parity_export: the non-empty guard only covers the local output, so a silent empty-body response from the remote arm would surface as a generic assert_eq! failure rather than a targeted message.

crates/omnigraph-cli/tests/parity_matrix.rs — the parity_export test would benefit from a symmetric non-empty guard on remote_lines.

Important Files Changed

Filename	Overview
crates/omnigraph-cli/src/client.rs	Adds export and list_graphs methods to GraphClient, completing RFC-009 Phase 3c; logic is a direct transliteration of the removed helpers, behavior preserved.
crates/omnigraph-cli/src/helpers.rs	Removes three dead functions (execute_export_to_writer, execute_export_remote_to_writer, inferred_config_path) now that their logic lives in client.rs.
crates/omnigraph-cli/src/main.rs	Export and graphs list handlers replaced with GraphClient::resolve() + method call; unused ExportRequest/GraphListResponse imports removed; http_client still required for operator-alias path.
crates/omnigraph-cli/src/output.rs	Removes dead yaml_string helper; no functional changes.
crates/omnigraph-cli/tests/parity_matrix.rs	Adds parity_export row; uses line-wise sorted comparison instead of the usual single-doc scrub; only the local arm gets an explicit non-empty guard, leaving a gap in diagnostic clarity if the remote arm silently returns an empty body.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    CLI["main.rs: Command::Export / Command::Graphs::List"]
    resolve["GraphClient::resolve(config, server, graph, uri, target)"]
    isRemote{is_remote_uri?}
    remoteVariant["GraphClient::Remote { http, base_url, token }"]
    embeddedVariant["GraphClient::Embedded { uri, graph: None, actor: None }"]

    exportRemote["export() Remote arm\nPOST /export → stream chunks → writer"]
    exportEmbedded["export() Embedded arm\nOmnigraph::open(uri)\n→ export_jsonl_to_writer → writer"]

    listRemote["list_graphs() Remote arm\nGET /graphs → GraphListResponse"]
    listEmbedded["list_graphs() Embedded arm\nbail! (remote-only)"]

    CLI --> resolve
    resolve --> isRemote
    isRemote -- yes --> remoteVariant
    isRemote -- no --> embeddedVariant
    remoteVariant --> exportRemote
    embeddedVariant --> exportEmbedded
    remoteVariant --> listRemote
    embeddedVariant --> listEmbedded

Loading

_{Reviews (1): Last reviewed commit: "refactor(cli): collapse export + graphs-..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[greptile-apps]

aaltshuler has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

[greptile-apps]

Missing non-empty guard on the remote output

assert!(!local_lines.is_empty()) guards against the fixture producing no rows on the local arm, but the remote arm has no equivalent check. If the remote arm silently returns a 200 with an empty body (e.g., a chunked-transfer bug where the first chunk() is None), remote_lines will be empty while local_lines is non-empty, and the test will still fail — but with the generic "JSONL streams diverge" message rather than a diagnostic one that points directly at the remote arm being empty. Adding a parallel assert for remote_lines mirrors the stated intent and makes the failure mode self-documenting.