feat(engine): add edge keys with derived identity - #593
Open
azimafroozeh wants to merge 1 commit into
Open
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
5 tasks
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Implements RFC 0044, "Edge keys: derived edge identity", in full. Closes #583.
@key(src, dst, ...)becomes legal on edge types. A keyed edge derives its id from the key with the canonical encoding keyed nodes already use, so the same relationship inserted on both sides of a branch fork derives the same id and the merge converges to one row; the same key with different non-key properties surfaces a typeddivergent_insertconflict instead of a silent duplicate. Unkeyed edge types are bit-for-bit untouched: generated ids, parallel edges, and the keep-both merge outcome stay.The change set, per the RFC's six design changes plus contract closure:
@keygroup on an edge (both endpoints required, scalar members optional, repeats refused); property namesid,src,dst,from, andtoare now reserved on edge declarations.@uniquegroup over the key's column set is subsumed.ir_version(3); the supported set is {2, 3}, the v1 rejection is unchanged, andvalidate_schema_irrefuses a stamp that does not match the declared features.Backing issue / RFC
Checklist
Local verification
cargo test -p omnigraph-compiler: 330 passedcargo test -p omnigraph-engine --test writes --test validators --test branching --test merge_truth_table --test traversal: green; engine lib 373 passed, 1 failure is a pre-existing sandbox limitation (UnixListener bind denied in the dev harness), 1 ignoredomnigraph-dst: full suite green (78 passed), including the reclassified multiset-contract pin and its keyed twincargo clippy -p omnigraph-compiler -p omnigraph-engine --all-targets -- -D warnings -W clippy::dbg_macro: clean;cargo fmt --allcleanpython3 scripts/check-docs.py: OK (115 files)Notes for reviewers
ir_versionmints 3 for edge-key schemas. rfc: add RFC 0040, the system column namespace #546 also advancesir_versionand is open; renumbering follows if rfc: add RFC 0040, the system column namespace #546's implementation lands first (the coordination gate is stated in the RFC's Compatibility section).validate_schema_irnow refuses a stamp that does not match the declared features, one step beyond the RFC's letter: it closes the hand-authored low-stamp route that would let an old binary open a graph holding derived ids.id/src/dst/from/toon edge declarations is a behavior change for new schemas only; accepted graphs are untouched. Node declarations keep today's latitude.Greptile Summary
Implements derived identity for keyed edges across schema parsing, accepted IR, runtime catalogs, mutations, loaders, validation, and branch merging.
@key(src, dst, ...)on edge declarations and validates endpoint and scalar-member requirements.Confidence Score: 5/5
The PR appears safe to merge; no concrete blocking or independently actionable non-blocking issue remains.
The schema, mutation, loader, validation, and merge paths consistently derive keyed-edge identity from the accepted catalog, while tests preserve the existing behavior of unkeyed edges.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR S[Edge schema with @key] --> IR[Validated SchemaIR v3] IR --> C[Identity-bound catalog] C --> M[Mutation insert] C --> L[Loader input] M --> K[Canonical typed key tuple] L --> K K --> ID[Derived edge id] ID --> U[Upsert by identity] U --> V[Shared integrity validation] V --> B[Branch merge] B -->|equal keyed rows| O[One converged row] B -->|different non-key values| D[DivergentInsert conflict] C -->|no edge key| G[Generated ULID and multiset behavior]Reviews (1): Last reviewed commit: "feat(engine): add edge keys with derived..." | Re-trigger Greptile
Context used (4)