Skip to content

fix(export): stabilize graph.json field order across a read-rebuild round-trip - #2730

Open
C0KERNEL wants to merge 1 commit into
Graphify-Labs:v8from
C0KERNEL:fix-graph-json-field-order
Open

fix(export): stabilize graph.json field order across a read-rebuild round-trip#2730
C0KERNEL wants to merge 1 commit into
Graphify-Labs:v8from
C0KERNEL:fix-graph-json-field-order

Conversation

@C0KERNEL

Copy link
Copy Markdown
Contributor

Completes #2582.

#2582 made graph.json collection order byte-stable — the order of nodes, links, and hyperedges across each array — but its scope note deliberately stopped there: "intentionally limited to collection ordering; mapping-key ordering is unchanged." This PR closes that gap.

networkx.node_link_data() always appends the node key (id) at the end of each node dict. On a cold build id is still an inline attribute and lands mid-dict; after build_from_json consumes id as the pure node key, a re-serialize appends it last. The values are identical, but the field order churns — so a graph.json that is written, reloaded (e.g. by update/cluster-only), and re-written is not byte-stable, defeating #2582's guarantee on the mapping-key axis:

  • Cold build: ["label", "file_type", "source_file", "id", "community", "norm_label"]
  • After read → rebuild → write: ["label", "file_type", "source_file", "community", "norm_label", "id"]

The fix emits a canonical key order in to_json, just before the existing record sort: nodes lead with id, links with source/target/relation, then remaining keys sorted. The serialized form is now invariant regardless of how the attribute was stored in memory, so build → write → read → write is byte-identical. Endpoints are untouched — the _src/_tgt direction restore (#563/#1061) already guards source/target, and this change only reorders keys, never values.

Reproduction (deterministic, no API key needed)

import json, tempfile, os
from pathlib import Path
from graphify.export import to_json
from graphify.build import build_from_json

extraction = {
  "nodes": [
    {"id":"a_foo","label":"foo","file_type":"code","source_file":"a.py"},
    {"id":"b_bar","label":"bar","file_type":"code","source_file":"b.py"},
  ],
  "edges": [
    {"source":"a_foo","target":"b_bar","relation":"calls",
     "confidence":"EXTRACTED","confidence_score":1.0,"source_file":"a.py"},
  ],
  "hyperedges": [],
}
d = tempfile.mkdtemp()
p1 = os.path.join(d,"g1.json")
to_json(build_from_json(extraction), {0:["a_foo","b_bar"]}, p1, built_at_commit="x", force=True)
raw1 = json.loads(Path(p1).read_text())
p2 = os.path.join(d,"g2.json")
to_json(build_from_json(raw1), {0:["a_foo","b_bar"]}, p2, built_at_commit="x", force=True)
print("byte-identical:", Path(p1).read_bytes() == Path(p2).read_bytes())  # False before, True after

Test

tests/test_export.py::test_to_json_field_order_stable_across_read_rebuild — builds a graph, writes it, reads it back with build_from_json, writes again, and asserts the two files are byte-identical with id leading every node and source/target leading every link. Fails before the patch (the round-trip churns id's position), passes after. The existing test_to_json_sorts_graph_collections_across_insertion_order missed this because it only varied insertion order within a single build and never exercised a read-rebuild.

uv run pytest tests/ -q: 4335 passed, 45 skipped. The 5 failures (test_ollama_retry_cap.py ×4, test_labeling.py::test_label_communities_batches_when_over_batch_size) are pre-existing on the untouched v8 tip and unrelated — verified by reverting export.py/test_export.py to pristine origin/v8, where they still fail. They touch the Ollama retry cap and community-label batching, not export/serialization.

…ound-trip

node_link_data always appends the node key (`id`) at the end of each node
dict. On a cold build `id` is still an inline attribute and lands mid-dict;
after build_from_json consumes `id` as the pure node key, a re-serialize
appends it last. The values are identical, but the field order churns, so a
byte-diff of two equivalent graph.json files is noisy and any position-sensitive
consumer sees a spurious change on every round-trip.

Emit a canonical key order in to_json before the existing record sort: nodes
lead with `id`, links with `source`/`target`/`relation`, remaining keys sorted.
The serialized form is now invariant regardless of how the attribute was stored
in memory, so build -> write -> read -> write is byte-identical. Endpoints are
untouched (the _src/_tgt direction restore already guards those).

Adds test_to_json_field_order_stable_across_read_rebuild, which the existing
determinism test missed by only varying insertion order within a single build.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. 1 change(s) tested, no difference found (not proven).


Graphify review — findings

This PR modifies to_json in graphify/export.py to canonicalize the key order within each node and link dict before serialization, placing identity keys (id/label for nodes, source/target/relation for links) first and remaining keys sorted afterward. The stated intent is to keep graph.json output byte-stable across a build → write → read-back → write round-trip, since node_link_data otherwise places the node id key in a position-dependent spot. A new test (test_to_json_field_order_stable_across_read_rebuild) is added to assert byte-identity across the round-trip and to check the expected leading key order and endpoint preservation. The remaining changed test symbols appear to reflect line-number shifts from the insertion rather than behavioral changes.

No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 505 functions depend on the 156 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: to_json() — 45 callers, 7 callees

Verification — 505 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 365 function(s) in the blast radius were not formally verified this run

Formal verification

No difference found (not proven): No behavior difference found in to\_json (not a proof).

The verifier ran both versions of to\_json on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

· 1 more finding(s) on lines outside this diff (see the check run).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant