Skip to content

_coalesce_edges raises on any duplicate edge: constructs PyCallEdge with source=/target= instead of src=/dst= #133

Description

@rahlk

Problem

PyCG._coalesce_edges (codeanalyzer/semantic_analysis/pycg/pycg_analysis.py:468) raises
whenever it actually has something to coalesce. It builds the merged edge with the wrong field
names:

merged[key] =PyCallEdge(
source=existing.source, # PyCallEdge has no `source`target=existing.target, # ... and no `target`weight=existing.weight+edge.weight,
prov=existing.prov,
)

PyCallEdge (schema/py_schema.py) declares src / dst with no aliases. Verified on main
(6f02581):

>>> PyCallEdge(source='a', target='b', weight=2, prov=['pycg'])
ValidationError: 2 validation errors for PyCallEdge
src Field required
dst Field required
>>> e.source
AttributeError: 'PyCallEdge' object has no attribute 'source'

The existing.source read raises first, so the failure is an AttributeError, not the
ValidationError.

The bug is latent because it sits behind the duplicate branch:

foredgeinedges:
key= (edge.src, edge.dst)
ifkeyinmerged: # <-- only this path is broken
...
else:
merged[key] =edge# <-- the common path, fine

So it only fires when two shards report the same (src, dst) pair. It is reachable — line 710,
on the sharded analysis path (self._coalesce_edges(all_edges)), which is the path large
projects take.

Scope boundary

Fixes the field names and the attribute read in _coalesce_edges, and adds the test that would
have caught it. Does not change coalescing semantics, does not touch merge_edges in
semantic_analysis/call_graph.py (a separate, correct implementation), and does not change
sharding.

Goals

  • _coalesce_edges constructs with src= / dst= and reads existing.src / existing.dst
  • A unit test feeds it two edges with the same (src, dst) and asserts one edge out with
    summed weight — the case that currently raises
  • Check whether prov should union rather than take existing.prov: merge_edges unions,
    this does not. Both inputs are prov=["pycg"] here so it is not observable today, but the
    divergence should be deliberate rather than accidental

Caveats and known risks

  • This has probably never executed successfully. Either shards never produced a duplicate
    edge in any tested project, or the exception was swallowed upstream. Worth checking whether a
    try/except around the shard collection has been hiding it — if so, that swallow is the more
    serious defect and this issue should say so.
  • Fixing it changes output on sharded runs: edge counts drop and weights rise where
    duplicates existed. That is the intended behaviour, but any test pinning sharded edge counts
    will move, and the change should not be mistaken for a regression.
  • Sharded runs are the load-sensitive path (PyCG shard timeouts under load produce spurious
    results), so the verification run needs a quiet machine to be trustworthy.
  • Related: Builtins get two can:// identities: PyCG spells the module <builtin>, Jedi spells it builtins #132 canonicalizes PyCG builtin spellings, which will create duplicate (src, dst)
    pairs where two spellings collapse — so it makes this latent bug reachable on more paths. Builtins get two can:// identities: PyCG spells the module <builtin>, Jedi spells it builtins #132
    deliberately avoids routing through _coalesce_edges for that reason; this fix removes the
    need for that avoidance.

Definition of done

  • A test that fails before and passes after, feeding duplicate (src, dst) edges through
    _coalesce_edges
  • A sharded run over test/fixtures/whole_applications/xarray completes with no AttributeError
  • The prov question above is answered explicitly in the PR, either way

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions