Repository navigation
fix(validate,schema)!: enforce @card minimums for every written source node - #1212
Conversation
A non-zero @card minimum was counted only for sources whose edges a write added, moved or removed, so a node written without its edges published below the minimum, and an edge-table overwrite never recounted the sources it emptied (the overwritten table's committed view is empty). The unified evaluator now also counts every source node row a mutation, JSONL load or merge writes, and reads the old sources of removed or moved edges from the pre-write image. A `load csv` node plan carries the new typed `source_cardinality` check, and the overwrite judge recounts the sources an edge overwrite removes. Schema apply refuses a new edge type with a non-zero minimum whose source node type already exists (OG-MF-107): its existing nodes have none of the new edges.
…a-c2c6f3 # Conflicts: # crates/omnigraph-gqt-core/src/load_plan.rs # crates/omnigraph-gqt/README.md # crates/omnigraph/src/validate/load.rs # docs/user/queries/explain.md # docs/user/schema/index.md # skills/omnigraph/references/schema.md
Keep the mutations page within its 350-line cap, spell the product Omnigraph, fix a stale evaluate_cardinality doc line, and trim the mutation case to two environments and one result query.
WhyA non-zero WhatThe change counts every source node a write inserts or updates when the minimum is greater than 0. An edge overwrite reads the replaced source column once, in bounded batches, and keeps the smallest source the delta leaves with no edge. Schema apply refuses a new edge with a minimum when that source type is already accepted ( HowMutation, JSONL load, merge validation, and Blob writes share ImpactA writer that loads nodes before edges now fails closed. A writer that overwrites an edge table and leaves a source empty also fails closed. Rows written before this change stay in place until a later write inserts or updates the node or changes its edges. Schema plan and schema apply stay data-independent. A required property already uses that rule ( RecommendationRevise. One blocking finding. Two optional findings. Blocking
This change puts every inserted or updated source id on that probe. Admission for Chunk Optional
Tradeoffs and liabilityThe change removes a path-dependent contract. The same nodes were valid or invalid according to whether the writer touched an edge. It adds a probe sized to the written node set, a second node-count site in the CSV judge, and a documented break for writers that load nodes before edges. Efficiency of a large node reload is the quality this change serves less. Correctness of the published minimum stays first. Agreement between schema plan and schema apply stays with it. That order matches this contract. The overwrite path shows a bound is possible. The node probe should use a bound of the same kind. These choices are sound.
ValidationHead Head GQT,
Result: 2 passed, 427 filtered out. Merge base, fresh target Same GQT command. The full CSV case on the merge base failed as
Result: failed at step 1. Head Rust, same target and checkout:
CI on this head is green. GQ Logic Tests passed ordinary, served, dst, and dst-clippy. CI passed Test Workspace, clippy, rustfmt, S3, and Azure. DST passed the pinned suite. I did not rerun clippy locally. Lance 13.0.0 scanner default LimitationsI did not measure a large load. The blocking finding is from the code. Uniqueness chunks for this reason, and this change widens the key set to every written source. I did not execute schema apply on the merge base. The merge-base Inline comments
Uniqueness chunks at Chunk
This arm sends every sweep id through the same unchunked Admission in Optional: this arm repeats the node rule that |
…ery route The committed `@card` lookup built one `in_list` of every key and kept every matching `(id, src)`, and the written-source rule widened its keys to every node a write inserts or updates, so a node-table overwrite or a merge planned one filter of every id. Sources are now counted in sorted filtered scans of at most 8,192 keys each, streamed and kept as one count per source; by-id lookups of removed and moved edges chunk the same way. The `load csv` judge's cardinality and source-cardinality arms call the evaluator's `evaluate_cardinality`, so which rows count has one edit site. A fast-forward's pure node inserts skip validation only when no edge type with a non-zero `@card` minimum starts at the type: such inserts are below the minimum, which only a branch written before inserted nodes were counted can hold.
|
Addressed in 805aec1. Blocking, unchunked probe. Agreed. The written-source rule widened Optional 1, fast-forward skip. Taken as the code change rather than a note. Optional 2, second rule site. Both CSV arms now call Local runs pass for the engine integration suite (validators, merge fast-forward), schema apply, writes, the engine and compiler lib tests and the planner load-plan tests. The two |
WhyThe previous review found one blocking problem. The written-source rule sent every inserted or updated source id through one WhatCommit This delta is one commit. Its parent is How
Schema apply does not probe written rows.
Status of the earlier findingsBlocking, unchunked probe: fixed. The fix is in the committed lookup that owns the probe, not in one caller. The affected class is every write whose source set can exceed one chunk: a node overwrite (JSONL or CSV), and a branch merge. A keyed append or merge refuses a table above 8,192 rows, so that path stays in one chunk. The code still chunks it. An edge overwrite does not use this probe. Its committed view is empty, and the replaced-image scan stays bounded. Schema apply does not use it. The count is correct across a chunk boundary. Sources are sorted, and the chunks do not overlap. A source with no returned edge is absent from the map, and the check treats that as zero. The reload of 9,000 sources that keep an edge is the proof that the second chunk is counted. If that chunk were skipped, those sources would look empty and the reload would be refused. Optional 1, fast-forward skip: taken in code. The predicate returns false for a minimum source. It stays true for a plain node, a minimum target, and a max-only source. Optional 2, second rule site: taken. Both CSV arms call FindingsNo new blocking finding. No new optional finding. Tradeoffs and liabilityThe change removes the uncapped filter and the retained edge pairs on the written-source path. A source with many edges now adds one to a counter. Peak retained state is one count per affected source, plus one chunk of literals. That matches invariant 11: hot-path memory follows the working set. Invariant 8 still holds. A source under the minimum fails with the same The by-id lookup still keeps one pair per changed edge id. That set is the delta the validator already holds. It is not the edge table. The new cost is one filtered scan per 8,192 sources. Edge tables carry a BTREE on the source column ( The fast-forward change adds a catalog check and, when it hits, a full validation of pure node inserts. Same-binary merges cannot hold the bad rows. The cost appears for a schema that already has a minimum, and only when every candidate is a pure node insert. A pre-change branch that fast-forwards such inserts is now refused. That is the intended break, applied at the merge door that used to skip the check.
Five more checks of this shape should add a caller of Efficiency of a very large source set is the quality served less here. Each chunk is a separate scan. Correctness of the published minimum stays first. That order matches this contract. ValidationHead Local head, checkout
Local merge base, checkout
CI on this head is green. GQ Logic Tests, CI, and DST passed. I did not rerun clippy, S3, or Azure locally. LimitationsI did not measure a source with millions of edges. The bound is from the code: the scan streams, and the map stores one count. I did not run a deletion-vector fixture. The scanner default cited above is the basis for that claim. I did not execute schema apply on the merge base. The head test still refuses the new edge, and this delta does not edit that path. RecommendationReady. The blocking finding is fixed at the probe that owns it. Both optional findings are taken. This delta adds no new defect that I can show. |
…a-c2c6f3 # Conflicts: # skills/omnigraph/SKILL.md # skills/omnigraph/references/schema-design.md
A non-zero
@cardminimum was enforced only for sources whose edges a write added, moved or removed. A node written without any edge of the type was never counted, so underedge WorksAt: Person -> Company @card(1..)aPersonwith noWorksAtloaded, alone or beside people loaded with theirs, while deleting a person's last edge was refused: the same graph state was valid or not depending on how it was reached. Two more doors did the same. An edge-table overwrite never recounted the sources it emptied, because the replaced table's committed view is empty. Schema apply could add an edge type with a minimum over existing source nodes, all of which then sat below it. The user docs said cardinality is checked on every write path; the skill docs described the gap and told agents to keep orphan queries.The unified evaluator (mutation, JSONL load, merge, Blob cell writes) now counts every source node row a write inserts or updates when the minimum is non-zero, the same rule uniqueness and endpoint checks apply to every row a write touches, so a node and its required edges arrive in one mutation query or one load. Under an overwrite, every source of the replaced image holds only its delta edges: one bounded scan of the replaced table's source column keeps the smallest source the delta leaves without an edge, so memory stays with the delta and no key list grows with the input. A
load csvplan declares that scan as thecardinalitycheck'sscanaccess, and the overwrite judge refuses a plan with a minimum that lacks it instead of counting nothing; a node plan carries a new typedsource_cardinalitycheck. The schema planner refuses an added edge type with a minimum from an existing node type asOG-MF-107, data-independently, as it refuses a required property (OG-MF-103). Committed edges are counted per source in sorted filtered scans of at most 8,192 sources each, streamed and kept as one count per source, because a node-table overwrite and a merge can write any number of sources; theload csvjudge calls the evaluator'sevaluate_cardinality, so the rule has one edit site; and a fast-forward of pure node inserts keeps the evaluator when the type is the source of an edge with a minimum, since only a branch written before this change can hold such nodes.The regression tests fail on the code before this change at the predicted step and pass after it: in
validators.rs, a JSONL load of an edgeless person beside one loaded with an edge (append and merge) and edge overwrites that remove or move a person's only edge; two.gqtcases covering mutations (one with two inserted people and one edge), a generated node-only load, everyload csvmode, the CSV edge overwrite and the plan'ssource_cardinalityline; the planner tests forOG-MF-107and for the declared accesses; avalidators.rsoverwrite of 9,000 people that crosses the 8,192-source chunk boundary; a unit test of the fast-forward rule; andschema_apply.rs, where the refused apply publishes nothing and a new type with its required edge applies and is enforced from its first write. Two existing tests whose seeds relied on the gap now seed valid graphs. Locally the engine, planner, compiler and gqt-core suites pass; the GQT corpus passed except for wall-clock budget timeouts on unrelated cases while other builds saturated the machine, and workspace Clippy was not rerun after merging main because the disk was full, so CI's run is the check for that.For review: the decision to challenge is that an update counts, not only an insert. A node an earlier version wrote below the minimum is refused the next time a write updates it, a property update or a Blob cell write included, until its edges are added, and a rebuild by export and load refuses it too; counting only new nodes would need an existence probe and leave such nodes writable forever. A writer that loads nodes before their edges breaks, so the note is breaking.
OG-MF-107also refuses an empty source type, because a data-dependent check at apply time would letschema planandschema applydisagree. Explain JSON gains thesource_cardinalitycheck kind andscanaccess for an overwrite's cardinality under a minimum, additive under the sameexplain_version; the store format is unchanged, and no DST or cost test declares a@cardminimum, so no golden line moves. There is no backing issue; the gap was found while reviewing the schema docs.