Graph: make index exhaustion atomic

.changeset/tidy-graphs-stop.md

@@ -0,0 +1,5 @@
+---
+"effect": patch
+---
+
+Make Graph node and edge allocation fail atomically after allocating `Number.MAX_SAFE_INTEGER`.

packages/effect/src/Graph.ts

@@ -1460,8 +1460,13 @@ export const addNode = <N, E, T extends Kind = "directed">(
   mutable: MutableGraph<N, E, T>,
   data: N
 ): NodeIndex => {
-  const impl = getMutableImplForMutation(mutable)
+  assertMutable(mutable)
+  const impl = internal.toImpl(mutable)
   const nodeIndex = impl.nextNodeIndex
+  if (nodeIndex === internal.exhausted) {
+    throw new GraphError({ message: "Node index allocation exhausted" })
+  }
+  csr.invalidate(mutable)
 
   // Add node data
   impl.nodes.set(nodeIndex, data)
@@ -1471,7 +1476,7 @@ export const addNode = <N, E, T extends Kind = "directed">(
   impl.reverseAdjacency.set(nodeIndex, [])
 
   // Update graph allocators
-  impl.nextNodeIndex = impl.nextNodeIndex + 1
+  impl.nextNodeIndex = nodeIndex === Number.MAX_SAFE_INTEGER ? internal.exhausted : nodeIndex + 1
 
   return nodeIndex
 }
@@ -2244,7 +2249,8 @@ export const addEdge = <N, E, T extends Kind = "directed">(
   target: NodeIndex,
   data: E
 ): EdgeIndex => {
-  const impl = getMutableImplForMutation(mutable)
+  assertMutable(mutable)
+  const impl = internal.toImpl(mutable)
 
   // Validate that both nodes exist
   if (!impl.nodes.has(source)) {
@@ -2255,6 +2261,10 @@ export const addEdge = <N, E, T extends Kind = "directed">(
   }
 
   const edgeIndex = impl.nextEdgeIndex
+  if (edgeIndex === internal.exhausted) {
+    throw new GraphError({ message: "Edge index allocation exhausted" })
+  }
+  csr.invalidate(mutable)
 
   // Create edge data
   const edgeData: Edge<E> = { source, target, data }
@@ -2285,7 +2295,7 @@ export const addEdge = <N, E, T extends Kind = "directed">(
   }
 
   // Update allocators
-  impl.nextEdgeIndex = impl.nextEdgeIndex + 1
+  impl.nextEdgeIndex = edgeIndex === Number.MAX_SAFE_INTEGER ? internal.exhausted : edgeIndex + 1
 
   // Only invalidate cycle flag if the graph was acyclic
   // Adding edges cannot remove cycles from cyclic graphs

packages/effect/src/internal/graph.ts

@@ -9,6 +9,12 @@ import { hasProperty } from "../Predicate.ts"
 /** @internal */
 export const TypeId = "~effect/collections/Graph"
 
+/** @internal */
+export const exhausted = Symbol("effect/Graph/exhausted")
+
+/** @internal */
+export type Allocator = Graph.NodeIndex | typeof exhausted
+
 /** @internal */
 export interface GraphImpl<in out N, in out E, T extends Graph.Kind = "directed">
   extends Iterable<readonly [Graph.NodeIndex, N]>, Equal.Equal
@@ -20,8 +26,8 @@ export interface GraphImpl<in out N, in out E, T extends Graph.Kind = "directed"
   edges: Map<Graph.EdgeIndex, Graph.Edge<E>>
   adjacency: Map<Graph.NodeIndex, Array<Graph.EdgeIndex>>
   reverseAdjacency: Map<Graph.NodeIndex, Array<Graph.EdgeIndex>>
-  nextNodeIndex: Graph.NodeIndex
-  nextEdgeIndex: Graph.EdgeIndex
+  nextNodeIndex: Allocator
+  nextEdgeIndex: Allocator
   acyclic: Option.Option<boolean>
   toJSON(): unknown
 }
@@ -202,8 +208,18 @@ export const hydrate = <N, E, T extends Graph.Kind>(
       graph.reverseAdjacency.get(edge.source)!.push(edge.index)
     }
   }
-  graph.nextNodeIndex = snapshot.nodes.length === 0 ? 0 : snapshot.nodes[snapshot.nodes.length - 1].index + 1
-  graph.nextEdgeIndex = snapshot.edges.length === 0 ? 0 : snapshot.edges[snapshot.edges.length - 1].index + 1
+  const lastNodeIndex = snapshot.nodes.length === 0 ? undefined : snapshot.nodes[snapshot.nodes.length - 1].index
+  graph.nextNodeIndex = lastNodeIndex === undefined
+    ? 0
+    : lastNodeIndex === Number.MAX_SAFE_INTEGER
+    ? exhausted
+    : lastNodeIndex + 1
+  const lastEdgeIndex = snapshot.edges.length === 0 ? undefined : snapshot.edges[snapshot.edges.length - 1].index
+  graph.nextEdgeIndex = lastEdgeIndex === undefined
+    ? 0
+    : lastEdgeIndex === Number.MAX_SAFE_INTEGER
+    ? exhausted
+    : lastEdgeIndex + 1
   graph.acyclic = Option.none()
   return graph as unknown as Graph.Graph<N, E, T>
 }

packages/effect/test/Graph.test.ts

@@ -148,6 +148,71 @@ describe("Graph", () => {
       })
     })
 
+    it("allocates the maximum safe node index once and then fails atomically", () => {
+      const graph = Graph.fromSnapshot({
+        type: "directed",
+        nodes: [{ index: Number.MAX_SAFE_INTEGER - 1, data: "A" }],
+        edges: []
+      })
+
+      const mutable = Graph.beginMutation(graph)
+      assert.strictEqual(Graph.addNode(mutable, "B"), Number.MAX_SAFE_INTEGER)
+      for (let i = 0; i < 2; i++) {
+        assertGraphError(() => Graph.addNode(mutable, "unreachable"), "Node index allocation exhausted")
+      }
+      assert.deepStrictEqual(Array.from(mutable), [
+        [Number.MAX_SAFE_INTEGER - 1, "A"],
+        [Number.MAX_SAFE_INTEGER, "B"]
+      ])
+      assert.strictEqual(Graph.nodeCount(mutable), 2)
+    })
+
+    it("allocates the maximum safe edge index once and then fails atomically", () => {
+      const graph = Graph.fromSnapshot({
+        type: "directed",
+        nodes: [{ index: 0, data: "A" }, { index: 2, data: "B" }],
+        edges: [{ index: Number.MAX_SAFE_INTEGER - 1, source: 0, target: 2, data: "first" }]
+      })
+
+      const mutable = Graph.beginMutation(graph)
+      assert.strictEqual(Graph.addEdge(mutable, 0, 2, "last"), Number.MAX_SAFE_INTEGER)
+      assert.strictEqual(Graph.isAcyclic(mutable), true)
+      for (let i = 0; i < 2; i++) {
+        assertGraphError(() => Graph.addEdge(mutable, 0, 2, "unreachable"), "Edge index allocation exhausted")
+      }
+      assert.deepStrictEqual(Array.from(Graph.edges(mutable)), [
+        [Number.MAX_SAFE_INTEGER - 1, { source: 0, target: 2, data: "first" }],
+        [Number.MAX_SAFE_INTEGER, { source: 0, target: 2, data: "last" }]
+      ])
+      assert.strictEqual(Graph.isAcyclic(mutable), true)
+      assert.strictEqual(Graph.edgeCount(mutable), 2)
+    })
+
+    it("hydrates maximum safe indexes as exhausted and validates edge endpoints first", () => {
+      const mutable = Graph.beginMutation(Graph.fromSnapshot({
+        type: "directed",
+        nodes: [{ index: 0, data: "A" }, { index: Number.MAX_SAFE_INTEGER, data: "B" }],
+        edges: [{ index: Number.MAX_SAFE_INTEGER, source: 0, target: Number.MAX_SAFE_INTEGER, data: "edge" }]
+      }))
+
+      assertGraphError(() => Graph.addNode(mutable, "unreachable"), "Node index allocation exhausted")
+      assertGraphError(
+        () => Graph.addEdge(mutable, 1, Number.MAX_SAFE_INTEGER, "unreachable"),
+        "Node 1 does not exist"
+      )
+      assertGraphError(
+        () => Graph.addEdge(mutable, 0, 1, "unreachable"),
+        "Node 1 does not exist"
+      )
+      assertGraphError(
+        () => Graph.addEdge(mutable, 0, Number.MAX_SAFE_INTEGER, "unreachable"),
+        "Edge index allocation exhausted"
+      )
+      assert.deepStrictEqual(Graph.successors(mutable, 0), [Number.MAX_SAFE_INTEGER])
+      assert.strictEqual(Graph.nodeCount(mutable), 2)
+      assert.strictEqual(Graph.edgeCount(mutable), 1)
+    })
+
     it("rejects invalid snapshot indexes", () => {
       assertGraphError(
         () =>

packages/effect/test/schema/Graph.test.ts

@@ -136,6 +136,33 @@ describe("Schema.Graph", () => {
     assert.strictEqual(edgeIndex, 5)
   })
 
+  it("decodes maximum safe indexes into exhausted allocators", () => {
+    const decoded = decodeDirected({
+      type: "directed",
+      nodes: [{ index: 0, data: "A" }, { index: Number.MAX_SAFE_INTEGER, data: "B" }],
+      edges: [{ index: Number.MAX_SAFE_INTEGER, source: 0, target: Number.MAX_SAFE_INTEGER, data: 1 }]
+    })
+
+    const mutable = Graph.beginMutation(decoded)
+    throws(() => Graph.addNode(mutable, "unreachable"), (error) => {
+      assert.strictEqual(error instanceof Graph.GraphError, true)
+      if (error instanceof Graph.GraphError) {
+        assert.strictEqual(error.message, "Node index allocation exhausted")
+      }
+    })
+    throws(() => Graph.addEdge(mutable, 0, Number.MAX_SAFE_INTEGER, 2), (error) => {
+      assert.strictEqual(error instanceof Graph.GraphError, true)
+      if (error instanceof Graph.GraphError) {
+        assert.strictEqual(error.message, "Edge index allocation exhausted")
+      }
+    })
+    assert.deepStrictEqual(encodeDirected(Graph.endMutation(mutable)), {
+      type: "directed",
+      nodes: [{ index: 0, data: "A" }, { index: Number.MAX_SAFE_INTEGER, data: "B" }],
+      edges: [{ index: Number.MAX_SAFE_INTEGER, source: 0, target: Number.MAX_SAFE_INTEGER, data: 1 }]
+    })
+  })
+
   it("does not serialize removed trailing allocation history", () => {
     const graph = Graph.directed<string, number>((mutable) => {
       const a = Graph.addNode(mutable, "A")