Network.plot(): a conflict with segment ID 0 is silently ignored (truthiness test on an ID)

[awickert]

Network.plot() resolves planform overlaps between segments. Its nested helper check_for_segment_conflicts returns the ID of a conflicting segment, or False when there is none:

• returns other_ID — grlp.py#L2088 and #L2096
• returns False — grlp.py#L2109

Both call sites test that return value for truthiness:

• if conflicting_id: — L2197
• while check_for_segment_conflicts(...): — L2212

Segment IDs start at zero — i = 0 then seg.set_ID(i) in Network.initialize(), L1495-L1498. So return 0, meaning "conflicts with segment 0", is indistinguishable from return False, meaning "no conflict".

Consequence

When the conflicting segment is ID 0, both the if and the while see a falsy value, so the overlap is never resolved. Because both sites fail the same way there is no exception and no warning — the planform is simply laid out as though there were no conflict. Only Network.plot() is affected; no computed result depends on it.

Network.plot() is exercised by tests/test_build_synthetic_network.py::test_network_plot_returns_planform, but that test asserts only that the returned x/y arrays have matching lengths, so it would not detect an unresolved overlap.

Suggested fix

Distinguish the sentinel from a valid ID. Either have the helper return None and test is not None, or keep False and test against it explicitly:

conflicting_id = check_for_segment_conflicts(seg.ID, segs_by_topo_length, self, ys)
if conflicting_id is not False:
    ...
while check_for_segment_conflicts(seg.ID, segs_by_topo_length, self, ys) is not False:
    ...

Related, in the same block

seg_to_adjust is bound only inside if conflicting_id: (L2198) but read in the following while body (L2215-L2216). That read is currently unreachable when the binding is skipped, because the while re-evaluates the same predicate on an unchanged ys — so it is safe only by that coupling. Note also that seg_to_adjust persists across iterations of the enclosing for, so if the coupling were ever broken the loop would silently reuse a stale segment ID rather than raising UnboundLocalError. Binding it before the if would remove the hazard.

How this was found

Running pyright over a vendored copy of this engine, which flags the seg_to_adjust binding as possibly-unbound; the ID-0 problem turned up while checking whether that path was reachable. Line numbers are against master @ 97240523. I have not produced a failing figure — the analysis is from the code and the ID assignment.

🤖 Generated with Claude Code