[azimafroozeh]

What & why

Two changes to what a plan tells you. First, every explain document carries pipelines, and the cut is the one the operators run: query and load explains used to clear the field, and the planner's cut could drift from the emission an operator declares. Second, a load csv overwrite reports what each of its operators did, the measurement the bulk-load work needs: after #1085 SF30 still takes 511 s and nothing recorded how that splits across parse, validate and write.

• pipelines on every route (explain_version 7), cut at each node that holds its whole input (Sort, Aggregate, AntiJoin, RankFuse, a load's Validate, a costed multi-hop Expand) and at a join's build side, the collected left side of a cross or contains join included.
• The printed cut is the executed cut: lowering refuses a plan whose operator declares an emission other than the planner's, and the whole GQT corpus runs under that guard. Pipelines describe the plan and carry no timing.
• A load that ran reports per operator, from the operators' own DataFusion metrics: rows, elapsed_compute_ns (its polls, busy time) and wall_ns (first poll to last batch, waits included), as executed.operators on the receipt; omnigraph mutate --json and POST /mutate answer it as ChangeOutput.executed, overwrite only.
• GQT load steps accept load pipeline <n>: … and load executed <Node>: rows <n> lines.

Backing issue / RFC

• The measurement step of docs/rfcs/2026-10-15-bulk-load-as-a-query-plan.md (feat(load): plan CSV overwrites and reuse validation reads #1026, perf(load): check edge endpoints in an unchained integer key set #1085 before it); D11 unchanged.

Checklist

• Focused: one logical change
• Tests: explain_pipelines_cut_at_breakers.gqt, explain_pipelines_rank_fuse_arms.gqt, explain_pipelines_collected_join_sides.gqt, load_csv_plan_overwrite_route_and_access_paths.gqt; Rust only where the format cannot reach: the planner's cross-join pipelines, the api-types round trip, the CLI's mutate --json wire shape (the served GQT host refuses expect plan on a load; no served load csv succeeds under test because a server-safe base may not use file://)
• Public docs: docs/user/queries/explain.md, docs/user/mutations/index.md, crates/omnigraph-gqt/README.md
• Release note: changelog.d/explain-pipelines-every-route.changed.md, changelog.d/load-executed-operators.added.md
• Invariants: reviewed against docs/dev/invariants.md, none weakened, no deny-list item hit

Local verification

• LDBC SNB SF1, 31 overwrite statements on a release build, phases derived from each answer's executed.operators (the three walls nest, source inside validate inside sink, so each phase is the difference of two walls): 13.72 s total; parse 5.47 s (40 %, equal to its compute: one thread), validate 3.36 s (24 %), write 3.22 s (23 %, compute 1.14 s, the rest waiting on fragment writes), other 1.57 s.

Notes for reviewers

• No storage format change.
• openapi.json: ChangeOutput gains the optional executed; additive.
• explain_version 6 to 7: a consumer pinning 6 refuses the new documents. Diff and merge documents print what they did before.
• The load still runs three serial phases (RFC D11); this PR measures that shape, it does not change it.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-20T09:50:22.806057Z	1616ecb	PR opened

ℹ️ 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" or "@codex security review".

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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1616ecb4cc

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

[cursor]

This change makes every explain document carry pipelines (explain_version 7), cut where an operator holds its whole input, and it records what each operator of a load csv overwrite did. The load still runs as three serial phases. The new receipt is how that time can be split. The PR merged during this review. The squash f092055da290658774553c2979e9b6b6f4b04aa9 has the same tree as the reviewed head 8bf8956adaadf18027b9ecc97502770a94bebde3.

What landed

• Every planned explain document has pipelines. Query and load explains used to clear the field. Diff and merge already printed it.
• The cut is each node whose operator emits only after its whole input (Sort, Aggregate, AntiJoin, RankFuse, Validate, a costed multi-hop Expand) and a join side the plan collects (hash-join build, cross-join left, contains-join left).
• Lowering compares EmissionType::Final with PhysicalNode::breaks_pipeline and refuses a mismatch. FragmentWrite is excluded.
• A load csv overwrite adds executed.operators (rows, elapsed_compute_ns, wall_ns). omnigraph mutate --json and POST /mutate return it as optional ChangeOutput.executed. Append, merge, and other changes omit it. OpenAPI does not require the field.

Codex finding on 1616ecb4

The Codex P2 on gate.rs still holds on this head. HashJoinExec reads the first probe batch before the build, and it reads no build when that batch is empty (crates/omnigraph/src/engine/operators/hash_join.rs). The new guard checks emission only, so a hash join passes. Commit 8bf8956a does not change that operator or the depends_on wording. It moves the load explain page. The review thread is marked resolved. The code the finding names is unchanged.

Blocking

depends_on is defined as the pipelines drained before this one starts. For a hash join that definition is false.

docs/dev/execution.md already says the operator reads the first probe batch before the build, and a probe with no rows never reads the destination. The build scan's report row is skipped. CrossJoinExec and ContainsJoinExec do collect the left side before they run the right side. HashJoinExec does not.

The same run shows both facts. I added a temporary assertion to engine_v2::proven_empty_ranked_source_skips_hash_builds_and_keeps_aggregate_row on this commit, then removed it. For query rows the explain document was:

• pipeline 0: Scan into HashJoin(build)
• pipeline 1: Scan into HashJoin(build)
• pipeline 2: Scan -> Expand -> HashJoin(probe) -> Expand -> HashJoin(probe) -> Projection into Sort, depends_on [0, 1]
• pipeline 3: Sort -> Page into the consumer, depends_on [2]

The same run left node_scan_projections empty, which is the test's existing proof that the destination build scans did not run. Command, from a detached worktree at 8bf8956a:

CARGO_TARGET_DIR=/workspace/target cargo test -p omnigraph-engine --test integration engine_v2::proven_empty_ranked_source_skips_hash_builds_and_keeps_aggregate_row -- --exact --nocapture

Result: ok. 1 passed; 0 failed; 0 ignored, 0.05 s. The assertion was not committed.

GQT cannot express "this scan did not run". The existing test says so. A .gqt line can pin depends_on, and explain_pipelines_cut_at_breakers.gqt pins consumer after 0 for the one-hop hash join. That case locks the wrong sentence. Extend the Rust test so a skipped build is not a depends_on predecessor, or so the document marks that side as conditional. Update the GQT line in the same change.

AntiJoin is the same class. The document lists the inner pipeline in depends_on. The operator drains the outer first and can skip the inner (empty outer, or the CSR degree path). The user guide already says the CSR path prints an inner pipeline it does not run. The field still lists that pipeline.

Do not repair this by draining the hash build first. That removes the empty-probe skip the execution guide and the existing test require. Make depends_on name a barrier the operator honors. A side the operator may skip, or may start after the other side, is not that barrier. The lowering guard cannot keep this promise: it compares final emission with incremental emission, and a hash join is incremental on both sides.

Inline locations for the same finding: crates/omnigraph-planner/src/physical.rs (the pipelines / depends_on contract, around the pull-chain wording) and crates/omnigraph-gqt/cases/v2/planner/explain_pipelines_cut_at_breakers.gqt (the after 0 line for the one-hop hash join, and the AntiJoin after 0, 1 line of the same class).

Optional

1. The emission guard skips FragmentWrite (crates/omnigraph/src/engine/lower.rs, the FragmentWrite exclusion in Walk::built). DataFusion 54 DataSinkExec copies the input's emission (datafusion-datasource-54.0.0/src/sink.rs, create_schema) and metrics() returns the sink metrics. The input is Validate, which is final, so the sink is final, while breaks_pipeline is false. The changelog says lowering refuses a different emission. The load sink is the exception, and the exemption has no comment. Make the sink's declared emission match the cut, or cut at the sink. A growing exemption list leaves the guard's promise false.

2. LoadOperatorExecuted::wall_ns is commented as the phase's wall time (crates/omnigraph/src/loader/mod.rs). The clock starts at the first poll of the output stream, or at the entry of write_all for the sink. CsvSource and Validate spawn their producers inside execute, before that poll. The three numbers are not a nested phase clock to subtract. Say "first poll to end, waits included" in the user guide, and drop "phase" from the struct comment. GQT pins rows only. One assertion on the existing overwrite load that the clocks are present, plus the reading rule, would lock the measurement. I did not re-run a load to time the clocks. The SF1 split in the PR body is the author's run.

3. The bulk-load RFC still records the explain decision as explain_version 6. This change is version 7 and prints pipelines on every route. Record that in the RFC decision log. Open PR feat(engine): expose ann diagnostics and budget native writer buffers #1086 edits the old explain page, including the sentence that a query has no pipelines, and adds ann_metrics. That page moved to split load explain out. The two texts will conflict. feat(server)!: say on every error body whether the request ran #1076, feat(cli)!: exit with what a failed command's outcome says #1078, and feat(cli): say who you are at a target and what you may do #1081 change loader error mapping and ChangeOutput construction. They do not add executed. I found no semantic clash on the mutate wire. feat(server,cli): report each branch's head, start and times in the branch listing #1012 and feat(server)!: assert the expected actor with Omnigraph-If-Actor #1074 also edit openapi.json. Expect a textual merge conflict.

Tradeoff and liability

One explain version, one cut function, and one guard at the lowering door are the right shape. The load measurement reads the operators' own metrics. It adds no side table and no second publication door. Overwrite-only matches the RFC rollout: append and merge still use the keyed adapters. There is no storage-format change and no Lance behavior change.

Version 7 is the liability. It tells every client that depends_on is an execution order. The push executor drains build sides in that order, and its callers are not wired. The query engine and the load engine do not. The next correction is another explain version. Fix the sentence in the document that clients will pin.

Load speed stays as it is. The three serial phases remain. This change measures that shape. That priority is right.

Validation

• Head reviewed: 8bf8956adaadf18027b9ecc97502770a94bebde3. Base: 85a4613831e5047a0c988ea0ad37768fa7dff4bb. Squash merge tree is identical (git rev-parse of both trees is 36b7ccffc7b7729de13d89951872f24d833889d2).
• CI on this head, from GitHub Actions: Test Workspace pass, GQT ordinary, dst, and served pass, clippy pass, rustfmt pass (runs 39404093696 and 39404093674). Those runs show the new cases pass. They do not show that depends_on matches the operator.
• Local proof is the single engine test above. I did not re-run the GQT corpus locally.
• No other human review covers this head. Codex covers 1616ecb4 only.

Recommendation: follow-up on the depends_on contract before clients pin version 7. I would not treat the merged text as the execution schedule.