[azimafroozeh]

An edge load csv probes its endpoints on one thread after the drain while the parse partitions, which already sweep every other check, sit idle: 56 s of SF30's 210 s load after #1141. The bulk-load RFC (docs/rfcs/2026-10-15-bulk-load-as-a-query-plan.md, #1026) specifies the endpoint check; this PR decides where the plan probes it.

The planner now places the endpoint checks per edge: swept, on the parse partitions as each batch arrives, when both checks read their committed key set whole over node types keyed by one I32, I64 or U32 @key; judged, after the drain, otherwise. The merge keeps the earliest miss, so the refusal is still the first missing endpoint in row order.

On SF30, 31 overwrite statements on a fresh graph, release builds of 484e3345 and this branch, one round each: load 210.0 → 168.5 s, the edges' validate 55.6 → 11.8 s; parse 55.2 → 58.3 s as the probe's CPU moves into it, write and capture unchanged.

For review: the decision to challenge is the admission order. The key sets are admitted before any partition parses and their scan buffers are released after the drain, so a memory refusal reports one actual in every DST replay, and a set the budget cannot admit refuses before a malformed row does, where the judged route reports the parse error; load_csv_edge_endpoints_set_admission_refuses_before_parse.gqt pins it. Least sure: U64 keys stay judged because the set is i64-keyed; load_csv_edge_endpoints_key_types.gqt pins it. Explain JSON gains probe on each edge_ri check and the statistic endpoint_probe, additive, same explain_version. The DST cost golden gains two lines, EndpointProbeSwept and EndpointProbeJudged, none moved; release note changelog.d/load-endpoint-checks-swept.performance.md.

[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	⚠️ Failed 2026-10-24T00:13:09.668675Z We couldn’t complete this review. Comment @codex review to try again.	cec63a6	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.

[ragnorc]

Review

An edge load csv whose endpoint checks scan whole node-id sets (access scan) over one I32, I64, or U32 @key now builds those sets while the CSV parses. Each parse partition keeps its earliest missing endpoint. The merge keeps the earliest miss. After a successful parse, the refusal is still the first missing endpoint in row order, from before to inside one row. Value and uniqueness still win, because they are judged before edge_ri. Every other key shape stays on the post-drain probe (judged).

Explain output gains probe (swept or judged) on each edge_ri check, and the statistic endpoint_probe. explain_version stays 7. Old plan lines that omit probe still decode, and a missing probe takes the judged path.

Recommendation: keep the placement. Correct the contract text in a follow-up. This pull request merged at a509c564 during the review. The comments below are on that commit.

U64 stays judged. That is the right call. The integer set is keyed by i64, and canonical_i64 returns no value above i64::MAX. A swept U64 would report a live node as missing. The text path compares the decimal spelling. load_csv_edge_endpoints_key_types.gqt pins both the plan bit and the same missing-id message.

Blocking

1. The release note and the load guide state a refusal and a set lifetime the swept route does not keep. The note says the refusal is unchanged. explain-load.md says that either route refuses the first missing endpoint. execution.md still says an empty edge input skips the integer sets, and that each set stays charged only while the other set is built and both are probed. On swept, admit_sets reserves every distinct endpoint table before any partition parses. A set the budget cannot hold returns query_memory_bytes and does not parse the file. The judged route still reports the parse error. The same early admission also runs for an empty source. The bulk-load RFC puts resource failures outside the row-order promise, so the early admission can stay: it is what makes a budget refusal's actual one value in every DST replay. The texts have to say that.

Optional

1. A key-set read error hides a parse error when both fail. futures::join! waits for the parse and the key-set read. The code then applies set_guards? before drained?. The judged route never starts the key-set read after a parse error, so the caller sees the parse error. Check drained first if a finished parse error should win. Leave admit_sets before the join.

Tradeoff and liability

The design keeps one judgment. judge_load_swept still walks checks in refusal order and still builds the orphan message with orphan_violation. The swept path only moves the fetch. A self-edge loads one set, where the judged path built the same set twice. That is less machinery, not a second validator.

What this spends: the scan input and the build scratch stay reserved until after the drain, including after those buffers are freed, so a parallel parse cannot change actual. Peak reservation during the parse is the held batches plus every swept set. Two large endpoint types therefore hold both scan inputs at once. The error path also waits for the key-set read before it returns a parse error. Those costs are the right ones for large integer node sets and edge files that parse. They are the wrong thing to leave out of the note.

Five more moves of the same kind (uniqueness, cardinality, retained edges) can reuse EndpointProbe and the same admit-before-parse rule. Each one needs the same sentence in the note. A polymorphic endpoint (open RFC on interfaces and unions) fails the one-integer-key test and stays judged. No new Lance API is involved. The scan is the existing count_rows(None) plus scan_bounded path.

This matches the bulk-load RFC: partitions may compute together, and the refusal for a parsed input stays in check order and row order. It sits beside #1141. removal is still its own plan fact, and this branch merges main after that change. #1130 adds read retrieval descriptors on a different document and does not move explain_version. #1139 adds MCP tools and does not change load planning. A later load tool returns these same errors.

Validation

Reviewed head: a509c5644f23fd1cd60e9046977dc884086927ab. Base: 3602c308. No earlier substantive review. The Codex attempt on cec63a6 failed and has no findings. Devin skipped the review (trial expired).

Local, on a509c564:

• cargo test -p omnigraph-planner --offline --test integration load_plan::endpoint_probe — 1 passed.
• cargo test -p omnigraph-engine --offline --lib held_batches_are_probed — 1 passed. The earliest miss is the same when the sets arrive before the batches and when they arrive after both partitions have the batches.
• cargo test -p omnigraph-engine --offline --lib the_sets_are_admitted — 1 passed. A budget one byte under the set refuses before input_rows moves. The scan and build reservations stay until the operator drops them.
• cargo test -p omnigraph-engine --offline --lib validate::load::tests — 4 passed.
• cargo test -p omnigraph-gqt-core --offline --lib load_plan:: — 3 passed.
• cargo test -p omnigraph-gqt --offline --test gq_logic_tests load_csv_edge_endpoints — 3 passed (key_types, on_integer_keys, set_admission_refuses_before_parse).
• cargo test -p omnigraph-gqt --offline --test gq_logic_tests load_csv_integer_endpoint_memory — 1 passed, including the DST replay.
• cargo test -p omnigraph-gqt --offline --test gq_logic_tests load_csv_plan_overwrite_route_and_access_paths — 1 passed, including the DST replay.

Before and after, same case file, load_csv_edge_endpoints_set_admission_refuses_before_parse.gqt:

• 3602c308 (main, case added only in a throwaway worktree, then removed): failed for the predicted reason. The integer-key load returned incorrect number of fields for line 2, expected 2 got more than 2. The existing load_csv_edge_endpoints_on_integer_keys.gqt on that commit passed.
• a509c564: passed. The same load returned resource limit exceeded for query_memory_bytes.

That is the precedence change. It is not a compile failure or a zero-test run. The temporary case was not pushed.

CI on a509c564 (pull_request runs 40043933689, 40043933618, 40043933694): Test Workspace, GQT ordinary and dst, DST pinned suite, clippy, rustfmt, and the storage jobs passed. I did not re-run the DST cost golden locally. The pinned suite on this commit did. The golden adds EndpointProbeSwept (15 gets) and EndpointProbeJudged (13 gets). No old line moves. The pull request does not say why the swept label has two more gets.

I did not measure the SF30 times in the pull request body (210.0 s to 168.5 s, validate 55.6 s to 11.8 s). Treat those numbers as the author's, unverified.

Inline comments

changelog.d/load-endpoint-checks-swept.performance.md:7

This sentence is too wide. After a successful parse, the first missing endpoint is unchanged. On swept, the key sets are admitted before any partition parses. A set the budget cannot hold returns query_memory_bytes and never reports the malformed row. The judged route still reports the parse error. load_csv_edge_endpoints_set_admission_refuses_before_parse.gqt pins that split. On 3602c308 the same case fails with incorrect number of fields for line 2. Say that exception here. The same admission also runs for an empty source, so the "empty edge input skips these sets" sentence in execution.md is no longer true for this route.

docs/user/queries/explain-load.md:55

This sentence holds for a missing endpoint after the file has parsed. Add the budget case beside it. On swept, a key set the budget cannot admit is refused before a malformed row and before an empty source is reported. On judged, those source errors still come first. Check order after a successful parse is unchanged.

docs/dev/execution.md:1033

admit_sets reserves these sets before drain_and_sweep. The paragraph below still says each set stays charged only while the other is built and both are probed, and that an empty edge input skips the sets. That is the judged lifetime. On swept, one set is admitted per node type before the parse, the scan and build reservations stay until after the drain, and an empty or malformed source does not skip them. Update that paragraph so the next change does not restore the old lifetime by mistake.

crates/omnigraph/src/engine/operators/validate.rs:180

Optional. When the key-set read and the parse both fail, this ? returns the read error and drops the parse error. The judged route never starts the key-set read after a parse error, so the caller sees the parse error. Check drained first if a finished parse error should win. Leave admit_sets above the join. That order is what makes a budget refusal report one actual.