Skip to content

Fix #2738: Prevent all-punctuation identifiers from collapsing in make_id - #2741

Open
shard-c6 wants to merge 1 commit into
Graphify-Labs:v8from
shard-c6:fix-2738-underscore-id-collapse
Open

Fix #2738: Prevent all-punctuation identifiers from collapsing in make_id#2741
shard-c6 wants to merge 1 commit into
Graphify-Labs:v8from
shard-c6:fix-2738-underscore-id-collapse

Conversation

@shard-c6

Copy link
Copy Markdown

Fixes #2738

Summary

This PR addresses the issue where private, all-punctuation identifiers (like Dart's const Foo._()) collapse during ID normalization and overwrite their parent file node's label, which subsequently breaks _is_file_node() detection and causes file nodes to be incorrectly classified as god nodes.

Changes

  1. Primary Fix: Updated make_id() in graphify/ids.py. When an entity is given but normalizes to an empty string (i.e. it only contains punctuation like _), make_id() now explicitly falls back to emitting "underscore" instead of silently collapsing. This prevents the symbol's ID from colliding with the file's ID.
  2. Secondary Fix: Updated node merging logic in build_from_json() inside graphify/build.py. When G.add_node() merges attributes for an existing node, it now preferentially keeps the label that matches the source filename instead of allowing arbitrary overlapping symbols to overwrite it.

@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.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.

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


Graphify review — findings

This PR modifies node-handling logic in two files. In graphify/build.py, the node-insertion path in build_from_json is changed so that when a node ID already exists in the graph with a conflicting label, it conditionally drops the incoming label attribute based on whether the existing/new labels are considered file-node labels for the associated source file. In graphify/ids.py, the make_id helper is reworked to normalize each part individually (substituting "underscore" for parts that normalize to empty) before joining and re-normalizing. The surface area is the graph-building node merge behavior and ID-generation logic, which are shared by many of the listed build/id symbols.

Worth a look

  • make_id changes normalization semantics: per-part normalize plus 'underscore' placeholdergraphify/ids.py:68 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
  • make_id no longer strips leading/trailing '_.' from partsgraphify/ids.py:68 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 2515 functions depend on the 87 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: build_from_json() — 151 callers, 19 callees

Verification — 2515 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: 1455 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify build\_from\_json.

The verifier did not have enough to check build\_from\_json, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 6 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly NameError — names the real obstacle, not a sampling gap)

No difference found (not proven): No behavior difference found in make\_id (not a proof).

The verifier ran both versions of make\_id 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.

Dart private named constructor (Foo._()) collapses to an empty entity in make_id, colliding with the file node and disabling _is_file_node

1 participant