Activate served graph firehose ingest - #441
Conversation
| StatusCode::SERVICE_UNAVAILABLE => { | ||
| "graph recovery must complete before stream ingest can begin" | ||
| } |
There was a problem hiding this comment.
🟡 Firehose clients are told recovery is pending when the real problem is an unconfirmed write
An ingest request whose durability acknowledgement came back unknown is reported to the caller as "graph recovery must complete before stream ingest can begin" (StatusCode::SERVICE_UNAVAILABLE arm at crates/omnigraph-server/src/lib.rs:1100-1102), so operators chase a recovery problem that does not exist.
Impact: Operators debugging a failed stream start are pointed at the wrong subsystem and may wait for or trigger recovery work that is unrelated to the actual failure.
How the ambiguous-acknowledgement branch collapses into the recovery message
ApiError::from_graph_stream_start (crates/omnigraph-server/src/lib.rs:1079-1113) deliberately rewrites AckUnknown from 500 to 503 (crates/omnigraph-server/src/lib.rs:1084-1093) so the ambiguity stays retryable/unavailable. The subsequent message table then keys purely off the HTTP status, so both OmniError::RecoveryRequired and OmniError::AckUnknown land on the same 503 string. The new test graph_stream_start_errors_redact_private_engine_evidence only asserts that private evidence is absent, so the mismatch is not caught. The acknowledgement_unknown flag is already in scope and can select a distinct, still-redacted message.
| StatusCode::SERVICE_UNAVAILABLE => { | |
| "graph recovery must complete before stream ingest can begin" | |
| } | |
| StatusCode::SERVICE_UNAVAILABLE if acknowledgement_unknown => { | |
| "graph stream ingest durability acknowledgement is unknown; retry the request" | |
| } | |
| StatusCode::SERVICE_UNAVAILABLE => { | |
| "graph recovery must complete before stream ingest can begin" | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
Stack
Depends on #438. This PR is intentionally based on
codex/graph-native-stream-ingestso the review contains only F7a activation.Summary
POST /graphs/{graph_id}/stream/ingestplus served-onlyomnigraph stream ingestValidation
cargo check --workspace --lockedscripts/check-agents-md.shgit diff --checkThe hour-long full workspace test gate was intentionally not rerun locally.
Greptile Summary
The PR activates graph-native served streaming ingest across the engine, HTTP API, CLI, shared wire types, OpenAPI contract, and documentation.
Confidence Score: 5/5
The PR appears safe to merge because no blocking failure remains within the eligible follow-up-review scope.
No blocking failure remains.
Important Files Changed
Sequence Diagram
sequenceDiagram participant CLI as CLI / HTTP client participant Server as Omnigraph server participant Policy as Cedar policy participant Engine as Graph stream ingest participant WAL as Lance MemWAL participant Driver as Resident fold driver participant Manifest as Graph manifest CLI->>Server: "POST /graphs/{id}/stream/ingest (no If-Match)" Server->>Policy: Authorize stream_ingest Policy-->>Server: Allow Server->>Engine: Effect-free authority preflight Engine-->>Server: Current graph authority token Server-->>CLI: 428 + strong ETag, no body polling CLI->>Server: Retry with If-Match + NDJSON stream Server->>Engine: Start checked graph ingest Engine->>WAL: Lazily enroll and durably append rows WAL-->>Engine: Per-row durable outcome Engine-->>CLI: Ordered redacted NDJSON outcomes Driver->>WAL: Coalesced fold Driver->>Manifest: Atomically publish visible graph stateReviews (4): Last reviewed commit: "fix(ci): scope AWS stack floor to tests" | Re-trigger Greptile
Context used (5)