Refactor graph ids toward append-only allocation#45
Draft
rollrat wants to merge 10 commits into
Draft
Conversation
Keep graph node identifiers stable across world-to-logic transforms by removing compact id rebuilds and making petgraph conversion sparse-id safe. This prepares the graph model for provenance metadata without relying on node vector ordering.
Introduce a graph-owned node id counter and add coverage that newly allocated ids do not reuse removed node ids. This is the first step toward moving node identity out of GraphNode payloads.
Move Graph.nodes behind an id-keyed container so node ordering is structural instead of maintained by ad hoc sorts or binary-search insertion. This keeps synthetic ids append-only across removals during world-to-logic transforms and removes the transform-level manual sort.
Add Graph::from_nodes so imported graphs initialize next_node_id from existing node ids instead of patching allocator state during deletion. Update graph construction sites to use that API and remove the preserve_next_node_id_high_watermark deletion hook.
Collapse overlapping sparse-id, id-keyed storage, and append-only allocator tests into one focused graph regression while keeping the world-to-logic sparse synthetic-node test. This reduces test fixture bulk without dropping the core invariants.
Switch the graph node storage wrapper from BTreeMap to IndexMap while preserving deterministic id-ordered iteration at construction and insertion. This keeps the storage ready for graph-owned node ids without changing traversal output.
Make Graph own node id storage by moving ids out of GraphNode and into GraphNodes entries. Preserve sparse and append-only ids through explicit keyed construction while keeping existing graph transforms and placer code on node references that carry storage ids.
Fix comments that were mojibaked during the GraphNode id refactor. This keeps the code behavior unchanged and restores the original Korean explanatory comments around graph merge and rebuild helpers.
Keep GraphNodeId production behind Graph::add_node and keyed graph construction. Remove direct keyed insertion and local synthetic id counters from graph builders, transforms, and clustered graph construction so transforms no longer mint graph ids outside Graph.
Move graph merging to allocator-backed imports so imported nodes receive fresh ids and internal edges are remapped through explicit old-to-new mappings. This keeps GraphNodeId production inside GraphNodes and removes rebuild-style id shifting from graph merges and logic binop replacement.
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
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
Testing