docs(rfc): RFC-013 node identity: surrogate + mutable natural key

[ragnorc]

What & why

Adds RFC-013, a proposed design that decomposes node identity into an immutable derived-then-frozen surrogate id (the sole edge-resolution and merge-matching key) and a mutable natural key (@key/@unique) resolved to the surrogate at write time, fixing the "@key is useless because I can't rename it" problem. The surrogate is derived-then-frozen rather than random, which lets three-way merge unify same-key inserts across branches and requires zero data migration of existing graphs.

Backing issue / RFC

• Implements / is an accepted RFC: this PR is the RFC — docs/dev/rfc-013-node-identity-surrogate-key.md (Status: Proposed; maintainer internal process applies). Threads existing tickets iss-gq-expose-system-id-column, iss-714, dec-918.

Checklist

• Change is focused (one logical change: add RFC-013 + its dev-index link)
• Tests added/updated for behavior changes (N/A — proposal doc, no code)
• Public docs updated if user-facing surface changed (N/A — dev-track RFC, no public surface change yet)
• Reviewed against docs/dev/invariants.md — the RFC includes an Invariants & deny-list check (aligns with invariant 15 logical-over-physical; strengthens merge correctness)

Notes for reviewers

Design was validated against Lance/LanceDB, HelixDB, Dolt, Datomic, Neo4j, Dgraph, ArangoDB before writing. The load-bearing decision is derive-then-freeze over random-per-write (Dolt's random-UUID path silently duplicates entities on merge); the breaking-change analysis (RFC §Breaking change) concludes zero on-disk migration under that choice. Open questions left for maintainer ruling: annotation naming (keep @key vs explicit @id/@natural split), id name collision, and whether iss-lint-edge-endpoint-requires-key downgrades to advisory. scripts/check-agents-md.sh passes.

---

Note

Low Risk
Docs-only proposal with no code or storage format changes in this PR.

Overview
Adds RFC-014 (docs/dev/rfc-014-node-identity-surrogate-key.md), a proposed design to split node identity into an immutable derive-then-freeze surrogate id (edges + merge) and a mutable @key/@unique natural key resolved at write time, with zero on-disk migration under that choice.

The doc threads GQ $node.id, cross-version uniqueness, merge behavior, invariants check, and a phased rollout (in-scope now: expose/reserve id and cross-version uniqueness; deferred: mutable @key, resolve-on-write edges).

docs/dev/index.md gains one Active Implementation Plans row pointing at the new RFC.

^{Reviewed by Cursor Bugbot for commit 30bb648. Bugbot is set up for automated code reviews on this repo. Configure here.}

Greptile Summary

This PR adds RFC-014 (titled RFC-013 in the PR) proposing the surrogate+natural-key identity decomposition for OmniGraph nodes, and links it from docs/dev/index.md. There are no runtime or schema code changes.

• RFC design: Decomposes today's fused @key/id into an immutable derive-then-freeze surrogate id (sole edge-resolution and merge-matching key) and a mutable natural key (@key/@unique), with a phased six-PR rollout where PRs 1–2 are in-scope and PRs 3–6 are deferred.
• Numbering inconsistency: The PR title and description say "RFC-013" but the file is named rfc-014-... and the document header carries no RFC number; rfc-013-* does not exist in the tree, leaving a gap in the sequence.
• Design gap — recycled natural keys: The "resolve-or-seed" specification (look up by key (or id), reuse if found) leaves undefined what happens when a new node's key matches the frozen surrogate of a previously-renamed node; this can silently merge unrelated entities or produce a duplicate-id write.

Confidence Score: 4/5

Docs-only PR; no code or on-disk format changes land here, so merging carries no immediate runtime risk. The design gap around recycled natural keys should be resolved in the RFC before the implementation PRs begin.

The RFC omits a concrete resolution path for the case where a new node's natural key matches the frozen surrogate of a previously-renamed node — an implementor following this document could introduce a silent entity-merge or a duplicate-id corruption. Because no code ships in this PR the gap can still be closed at the document level before PRs 1–6 begin, but the design is incomplete as written.

docs/dev/rfc-014-node-identity-surrogate-key.md — the resolve-or-seed section and the RFC number in the document header both need attention before implementation starts.

Important Files Changed

Filename	Overview
docs/dev/rfc-014-node-identity-surrogate-key.md	New RFC proposing surrogate+natural-key identity decomposition; design is well-researched but contains an unaddressed surrogate-collision scenario when a natural key is recycled after a rename, and carries an RFC number inconsistent with the PR title/description.
docs/dev/index.md	Adds one table row linking to rfc-014; the linked filename is rfc-014 while the PR title and description call this RFC-013, a discrepancy the index entry inherits.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Node insert with key='foo'] --> B{Resolve-or-seed lookup}
    B -->|key='foo' found in current nodes| C[Reuse existing frozen id]
    B -->|key='foo' NOT found| D{Check id='foo' across all nodes}
    D -->|id='foo' NOT found anywhere| E[Seed id = 'foo' ✅ fresh node]
    D -->|id='foo' found on renamed node\ne.g. Node A now has key='bar' but id='foo'| F[❓ Undefined in RFC]
    F --> G[Option A: Reuse id='foo'\n→ silently merges new entity into Node A]
    F --> H[Option B: Seed id='foo'\n→ duplicate-id write, storage crash or silent corruption]
    F --> I[Option C: Reject insert\n→ 'foo' is a reserved surrogate, non-recyclable]

    style F fill:#ffcc00,stroke:#cc9900
    style G fill:#ff6666,stroke:#cc0000
    style H fill:#ff6666,stroke:#cc0000
    style I fill:#99ff99,stroke:#009900

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Node insert with key='foo'] --> B{Resolve-or-seed lookup}
    B -->|key='foo' found in current nodes| C[Reuse existing frozen id]
    B -->|key='foo' NOT found| D{Check id='foo' across all nodes}
    D -->|id='foo' NOT found anywhere| E[Seed id = 'foo' ✅ fresh node]
    D -->|id='foo' found on renamed node\ne.g. Node A now has key='bar' but id='foo'| F[❓ Undefined in RFC]
    F --> G[Option A: Reuse id='foo'\n→ silently merges new entity into Node A]
    F --> H[Option B: Seed id='foo'\n→ duplicate-id write, storage crash or silent corruption]
    F --> I[Option C: Reject insert\n→ 'foo' is a reserved surrogate, non-recyclable]

    style F fill:#ffcc00,stroke:#cc9900
    style G fill:#ff6666,stroke:#cc0000
    style H fill:#ff6666,stroke:#cc0000
    style I fill:#99ff99,stroke:#009900

Loading

_{Reviews (2): Last reviewed commit: "docs(rfc): renumber node-identity RFC to..." | Re-trigger Greptile}

Context used:

• Context used - AGENTS.md (source)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7a8815467c

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

[chatgpt-codex-connector]

Define ambiguity rules for endpoint references

With mutable natural keys, this bare-string endpoint contract can become ambiguous within the endpoint node type: e.g. node B is created with key bar so its frozen surrogate is bar, B is later renamed to baz, and node A is then renamed to natural key bar. A subsequent edge endpoint value bar now matches both B's surrogate id and A's current natural key, but the RFC does not specify whether to reject, prioritize one namespace, reserve historical ids, or require property-qualified lookup refs; implementing this as written risks attaching edges to the wrong node after renames.

Useful? React with 👍 / 👎.

[aaltshuler]

Review: the case against exposing id (PR 1) — "pure addition" is the wrong mental model

First, credit where due: the second commit materially strengthened this — the zero-migration claim is now code-verified (id is already a separate physical column; merge already keys on it), the renumber to 014 resolves the collision with the write-path RFC, and re-sequencing tightening-before-loosening (PR 2 cross-version uniqueness ahead of PR 4 mutable-@key) resolves the atomicity concern from my earlier review. Those are real.

This comment is the bear case on PR 1 specifically (expose id + reserve the name), which the RFC frames as a "pure addition." It isn't — it promotes an internal physical value into a permanent public contract and opens a query surface that needs completeness work. Arguments tagged [inherent] (true of exposing id at all), [design] (specific to derive-from-@key), [rollout] (specific to PR 1 before PR 4).

The reframe: the value is narrow and conditional

For a keyed node with an immutable key — the dominant shape — @key already is the stable, queryable, referenceable id; exposing id adds nothing. The benefit exists only downstream of deciding to make @key mutable (PR 4), or for keyless node types (no natural key to address). And a zero-engine-change workaround already exists for the mutable-field need (immutable @key + @unique/@index). So the decision isn't really "expose id?" — it's "do we want mutable natural keys badly enough to take on a permanent contract + query-surface work, given the workaround?"

A. Permanent-contract / reversibility — the least-escapable cost

1. Irreversible exposure [inherent]. Once id is in results and depended on (Hyrum), you can never un-expose it, change its format, or change how it's minted. "Reversible — on-disk format unchanged" is true for storage, false for the contract.
2. Forecloses identity flexibility [inherent]. Permanently commits to "logical id = this exact string" — you can never converge it with the Lance stable-row-id (dec-918) or change the keyless ULID scheme.
3. Promotes physical/derived state into the logical contract [inherent]. Cuts against "logical contract over physical state."

B. The sequencing / Hyrum trap

4. PR 1 exposes id == @key while PR 4 (divergence) is deferred [rollout]. Every adopter in the gap learns the equality, stores it, joins on it — then PR 4, whose purpose is to break that equality, breaks them. You ship an observable invariant a later PR is designed to violate. This is the one I'd most weight.
5. The name id amplifies over-trust [design]. It's the conventional "stable identity" field, so it's the value most likely to be over-relied on; a less conventional name resists the wrong mental model.

C. Semantic footguns

6. Per-type, not global, uniqueness [inherent]. Person.id="alice" and Company.id="alice" coexist. Exposed as "node id," it invites the false "same id ⇒ same node" across types. The RFC doesn't flag this.
7. Two competing reference namespaces [design]. Exposing both the slug (@key) and the surrogate (id) splits external integrators on which to store and forces coherence between them — worse for integration than the one canonical id you already had.
8. Derive-from-key kills the only unique integration benefit [design]. A surrogate could be an opaque, non-PII handle — but id == @key (required for merge convergence) makes it a copy of the slug. Merge-convergence and opaque-handle are mutually exclusive here.
9. Heterogeneous format [inherent]. slug (keyed) vs ULID (keyless) — ORDER BY id / min(id) / cross-type compare are well-defined but odd, and a new surface to document.

D. Engine risk — it's not a pure addition

10. Typecheck-vs-execution completeness gap [inherent]. One special-case (typecheck.rs:950) green-lights id in positions execution may not carry it. Likely failure: "works in RETURN, null/empty in WHERE / ORDER BY / after Expand." The RFC notes projection resolves id but is silent on filter/order/traversal-output.
11. Scattered magic-column special-casing [inherent]. id is a queryable-but-not-a-property; every site enumerating NodeType.properties (project/filter/order/lint/migration) must now know it. One special-case is insufficient; sprinkling it is the deny-list "per-feature special-casing" anti-pattern. Fix shape: one "resolvable columns = properties + system columns" mechanism, not a magic-string check.
12. Filter pushdown on id [inherent]. The same col-vs-ident/coercion path #283/#285 just fixed must treat id correctly — needs targeted tests, not assumption.
13. Traversal/Expand output threading [inherent]. Expand row-assembly must carry id for the target across variable-length hops and anti-join (not { }) patterns.

E. Security / disclosure

14. ULID timestamp leak [design]. Keyless ids are ULIDs whose first 48 bits encode creation time — exposing them leaks each node's creation timestamp to anyone with read. Sensitive under policy/multi-tenant. Unmentioned.
15. Enumerable stable handle [inherent]. Combined with per-type collisions, a new probing surface to reason about under policy.

F. Surface-area / maintenance

16. Wire/OpenAPI/SDK ripple [inherent]. Result rows gain id; SDK ergonomics expand (omnigraph-ts already lags). id is the conventional JSON identity field, reinforcing #5.
17. Diff/change-feed exposure [inherent]. id becomes visible in diffs — a new observable contract reconciliation logic will bind to, and that PR 4's divergence then shifts.
18. Reserved-name fix is piecemeal [design]. PR 1 reserves only id; the consistent move is to reserve the whole system-column namespace (src, dst, _row_id, _row_*_at_version, …) at once.

Not a veto — what would de-risk it

Several of these are mitigable, and a few dissolve under a different sequence/design:

• Don't expose id until PR 4 can ship close behind (or expose it only once divergence is possible), so nobody learns the id == @key equality — kills #4, the strongest objection.
• Make id a real "system column" category resolved through one mechanism, and audit the full PropAccess execution surface (filter/order/Expand-output/aggregation) with targeted tests — kills #10/#11/#13.
• Decide #6 (per-type uniqueness), #14 (ULID leak), and #18 (full namespace reservation) explicitly in the doc.

The two questions I'd gate the decision on: (a) is the underlying mutable-key need real enough to not just use the existing @unique+@index workaround? and (b) if yes, can expose (PR 1) and diverge (PR 4) ship together rather than a long gap that ossifies id == @key?