feat: WEIGHTED milestone (v7.3.0) — edge properties #9
+3,276
−57
Conversation
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
Add encodeEdgePropKey/decodeEdgePropKey/isEdgePropKey utilities using \x01 prefix to guarantee collision-freedom with node property keys. 35 tests including fuzz and edge cases.
Encodes edge identity as \x01from\0to\0label in the PropSet op's node field, so JoinReducer's existing LWW pipeline handles edge properties transparently. 14 tests.
Add SCHEMA_V3 constant and detectSchemaVersion() helper. Schema v3 signals edge property PropSet ops; format is identical to v2. Codec handles both v2 and v3 transparently. 24 tests.
No prod code changes — existing LWW path handles edge prop keys transparently. 29 tests covering tiebreaks, fuzz, mixed props, overwrites, and joinStates merging.
getEdges() now returns a props field on each edge. New getEdgeProps() method for direct edge property lookup. 13 tests.
Add assertOpsCompatible() guard that rejects patches containing edge property ops when the reader only supports schema v2. v3 patches with only classic node/edge ops pass through — the schema number alone is not a rejection criterion. Fail fast, never silently drop data.
Track edgeBirthLamport in state to enforce clean-slate semantics on edge re-add. Props with lamport < birth lamport are filtered out at query time. Checkpoint serialization updated. 10 tests.
📝 WalkthroughWalkthroughAdds edge-property support and schema-v3 awareness across state, codec, patch builder, graph queries, checkpointing, error handling, tests, docs, examples, and roadmap/changelog updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PatchBuilder
participant WarpGraph
participant Persist
participant JoinReducer
Client->>PatchBuilder: setEdgeProperty(from,to,label,key,value)
PatchBuilder->>WarpGraph: submitPatch(patch with PropSet(edge-namespace))
WarpGraph->>Persist: persistPatch(patch)
Persist-->>WarpGraph: ack
WarpGraph->>JoinReducer: applyOpV2 / joinStates (EdgeAdd/PropSet)
JoinReducer-->JoinReducer: update edgeBirthEvent & LWW props
WarpGraph->>Persist: materialize/stateUpdate
Client->>WarpGraph: getEdgeProps(from,to,label) or getEdges()
WarpGraph->>JoinReducer: read materialized state (filter by edgeBirthEvent)
JoinReducer-->>WarpGraph: filtered props
WarpGraph-->>Client: props / edges with props
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…GHTED - CHANGELOG: add GROUNDSKEEPER and WEIGHTED sections, update test count - README: add setEdgeProperty to Quick Start and Patch Operations, add getEdgeProps to Simple Queries, update getEdges return shape - index.d.ts: add getEdgeProps method, update getEdges return type with props field, add encodeEdgePropKey/decodeEdgePropKey/isEdgePropKey exports - docs/GUIDE.md: add Edge Properties section with setting, reading, visibility, conflict resolution, and schema compatibility docs - examples/edge-properties.js: new runnable demo covering all edge property APIs including multi-writer LWW and clean-slate re-add
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@ROADMAP.md`:
- Line 227: A fenced code block in ROADMAP.md is missing a language/info string
(MD040); update the empty triple-backtick fence shown in the diff to include a
language tag (e.g., change ``` to ```text or another appropriate language) so
the code block has an info string and satisfies the MD040 lint rule.
In `@src/domain/services/JoinReducer.js`:
- Around line 141-152: The EdgeAdd handler currently stores only eventId.lamport
in state.edgeBirthLamport (in the 'EdgeAdd' case using encodeEdgeKey and
eventId), which allows later adds from other writers with equal/lower lamports
to fail to advance birth and leak old props; instead store the full EventId
(lamport + writerId + patchSha + opIndex) for each edgeKey in
state.edgeBirthLamport and use the same LWW ordering logic you use for
properties to compare EventIds when updating birth (replace if newEvent >
prevEvent). Then update WarpGraph.getEdgeProps() / getEdges() to filter property
incarnations by comparing their EventId against state.edgeBirthLamport[edgeKey]
using that same LWW comparator so re-adds never resurrect older props.
In `@src/domain/services/PatchBuilderV2.js`:
- Around line 166-194: PatchBuilderV2 is adding edge-prop ops via
setEdgeProperty (uses EDGE_PROP_PREFIX and createPropSetV2) but build() and
commit() still hardcode schema "2"; update build() to inspect this._ops for any
edge-prop ops (e.g., ops created by createPropSetV2 with node starting with
EDGE_PROP_PREFIX) and set schema = "3" when present (otherwise keep "2"), then
ensure that same derived schema value is placed into the returned patch object
and included in the commit() message payload so the patch and commit both
advertise the correct schema.
In `@src/domain/types/WarpTypesV2.js`:
- Around line 170-174: Update the PatchV2 JSDoc typedef so its schema property
matches createPatchV2's documentation: change the `@property` annotation for
PatchV2.schema from {2} to {2|3} so IDE/type hints reflect both schema versions;
locate the PatchV2 typedef (symbol PatchV2) and edit the `@property` {2} schema
line to `@property` {2|3} schema to align with createPatchV2's documented options.
In `@src/domain/WarpGraph.js`:
- Around line 1844-1895: getEdgeProps currently only checks edgeAlive but must
also verify both endpoint nodes are live like getEdges does: compute the node
keys for from/to (use encodeNodeKey) and use
orsetContains(this._cachedState.nodeAlive, nodeKey) and the corresponding birth
lamports (this._cachedState.nodeBirthLamport.get(nodeKey)) to ensure each node's
latest birth lamport is >= edge birthLamport (or otherwise not tombstoned before
the edge's incarnation); if either endpoint is not considered alive by those
checks, return null. Update getEdgeProps to perform these node-liveness checks
(referencing getEdgeProps, encodeNodeKey, this._cachedState.nodeAlive,
this._cachedState.nodeBirthLamport, orsetContains) before collecting and
returning properties.
In `@test/unit/domain/WarpGraph.edgeProps.test.js`:
- Around line 23-26: The test helper addEdgeProp currently sets props as {
value, lamport, writerId } which bypasses the stale-filtering logic in
getEdgeProps/getEdges that expects a nested eventId; update addEdgeProp (and its
use of encodeEdgePropKey) to set the property as { eventId: { lamport: 1,
writerId: 'w1', patchSha: 'aabbccdd', opIndex: 0 }, value } so
register.eventId.lamport checks work correctly and stale props are filtered.
In `@test/unit/domain/WarpGraph.edgePropVisibility.test.js`:
- Around line 245-253: The test currently passes an arrow function into expect,
which always evaluates truthy; change it to pass the actual boolean result
instead by calling graph._cachedState.prop.has(propKey) directly (use
encodeEdgePropKey to build propKey as already done) so the assertion becomes
expect(graph._cachedState.prop.has(propKey)).toBeTruthy(), leaving the
getEdgeProps assertion unchanged.
🧹 Nitpick comments (2)
src/domain/services/CheckpointService.js (1)
4-8: File header comment is outdated.The header comment on lines 4-5 states "Provides functionality for creating and loading schema:2 checkpoints" and line 7-8 says "This service only supports schema:2 (V5) checkpoints." However,
loadCheckpointnow accepts both schema:2 and schema:3 (line 178).Consider updating the comments to reflect schema:3 support.
test/unit/domain/services/EdgePropKey.test.js (1)
328-341: Good documentation of codec limitation.This test explicitly documents that fields containing
\0won't round-trip correctly. This is valuable documentation. Consider adding a note in the production code's JSDoc forencodeEdgePropKeywarning that fields must not contain the null character.
- Refactor edgeBirthLamport → edgeBirthEvent storing full EventId with
compareEventIds for total ordering (prevents same-lamport birth leak)
- PatchBuilderV2 auto-detects schema 3 from edge prop ops in build/commit
- getEdgeProps checks both endpoint node liveness before returning props
- Fix addEdgeProp test helper to use proper LWW register format
- Fix always-truthy arrow function in edgePropVisibility expect assertion
- Add MD040 language tag to ROADMAP.md fenced code block
- Update PatchV2 typedef schema to {2|3}
- Update CheckpointService header for schema:3 support
- Add @warning JSDoc on encodeEdgePropKey about null character fields
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@ROADMAP.md`:
- Around line 248-255: The milestone summary currently lists WEIGHTED as
“Planned” which conflicts with the DAG showing WEIGHTED (v7.3.0) as 100% CLOSED;
update the milestone summary row for WEIGHTED to mark it as Completed/Closed (or
the repository's canonical "Closed" status) and, if present, adjust the
percent/completion count to 100% (7/7) so the summary matches the DAG entry for
WEIGHTED (v7.3.0) and related node names like WT/OPS/1, WT/SCHEMA/1, and
WT/VIS/1.
In `@src/domain/services/PatchBuilderV2.js`:
- Around line 185-193: setEdgeProperty currently appends an edge prop op without
ensuring the edge exists; update PatchBuilderV2 to validate existence by
tracking edges created in this patch and refusing (or throwing) when the edge
was never added: add a private Set (e.g., this._edgesAdded) that is updated in
the edge-creation method(s) (e.g., addEdge/createEdge/createEdgeV2) and then,
inside setEdgeProperty(from, to, label, key, value), compute the same edge
identity used elsewhere (the `${EDGE_PROP_PREFIX}${from}\0${to}\0${label}` form
or the canonical edge id) and check membership in this._edgesAdded before
calling createPropSetV2; if missing, throw or return an error to prevent
dangling edge props.
In `@src/domain/WarpGraph.js`:
- Around line 2002-2004: Update the comment above the edgePropsByKey Map to
reflect that stale props are filtered using full EventId ordering via
compareEventIds rather than lamport-only comparison; mention compareEventIds and
EventId (or eventId) explicitly and remove references to "lamport-only" so
readers know the code compares complete EventId tuples against the edge's birth
EventId when filtering stale properties.
In `@test/unit/domain/WarpGraph.edgePropVisibility.test.js`:
- Around line 27-35: The test helper addEdge currently updates
state.edgeBirthEvent by only comparing lamport, which diverges from production's
full EventId ordering; modify addEdge to store a full EventId object (including
lamport, writerId and opIndex or the same shape used in production) in
state.edgeBirthEvent and, instead of comparing lamport directly, import and use
the compareEventIds helper to decide whether to replace the stored birth event;
update the import list to bring in compareEventIds and ensure the stored event
object matches the reducer's EventId shape so concurrent adds are ordered
identically to runtime.
🧹 Nitpick comments (1)
src/domain/services/JoinReducer.js (1)
80-101: Prefer EDGE_PROP_PREFIX constant over magic values.
encodeEdgePropKeyandisEdgePropKeyhardcode the prefix. UsingEDGE_PROP_PREFIXkeeps the namespace definition single-sourced and reduces drift risk.♻️ Suggested refactor
-export function encodeEdgePropKey(from, to, label, propKey) { - return `\x01${from}\0${to}\0${label}\0${propKey}`; -} +export function encodeEdgePropKey(from, to, label, propKey) { + return `${EDGE_PROP_PREFIX}${from}\0${to}\0${label}\0${propKey}`; +} -export function isEdgePropKey(key) { - return key.charCodeAt(0) === 1; -} +export function isEdgePropKey(key) { + return key[0] === EDGE_PROP_PREFIX; +}
- ROADMAP: mark GROUNDSKEEPER and WEIGHTED as complete - PatchBuilderV2: validate edge existence before setEdgeProperty (tracks edges added in patch via _edgesAdded Set, checks state for pre-existing) - JoinReducer: use EDGE_PROP_PREFIX constant in encodeEdgePropKey and isEdgePropKey instead of hardcoded magic values - WarpGraph: update getEdges comment to reflect full EventId ordering - edgePropVisibility test: use compareEventIds in addEdge helper, fix concurrent two-writer test expectation for full EventId ordering - edgeProps tests: add addEdge calls before setEdgeProperty per validation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Implements the complete WEIGHTED milestone (v7.3.0): first-class edge properties with CRDT semantics, schema v3, sync safety, and visibility gating.
encodeEdgePropKey/decodeEdgePropKeywith\x01prefix for collision-free namespacing against node property keysPatchBuilderV2.setEdgeProperty(from, to, label, key, value)generating PropSet ops in the edge namespacegetEdges()returnspropsfield; newgetEdgeProps(from, to, label)convenience methodassertOpsCompatible()guard +E_SCHEMA_UNSUPPORTEDerror — fail fast on v3→v2 sync with edge prop ops; node-only v3 patches accepted by v2 readers+2,969 lines across 21 files (13 prod, 8 test). 159 new tests, 1764 total passing.
Test plan
Summary by CodeRabbit
New Features
Documentation
Improvements
Tests