Repository navigation
fix(python): the try/except-ImportError shim emits BOTH resolutions' edges — the #441 deferred refinement (#705) - #868
Conversation
…edges — the #441 deferred refinement (#705) A module that both imports a name (under try/except ImportError) and defines a fallback class of that name gave the two resolution paths DIFFERENT targets: the typed-receiver path (t = Thing(); t.go()) bound the local fallback; the class-name path (Thing.go(None)) bound the import. Each caller reached exactly one target and they disagreed — at parse time the branch that runs is unknowable, so one of the two edges was always a false negative. The fix: after the primary resolution of an obj.method call — INCLUDING the case where the primary ABSTAINED (the dominant real-world shim's optional dependency is EXTERNAL, nothing in-repo to resolve; gating on the primary starved exactly that shape) — when the receiver name carries BOTH a same-file class record AND an import record in the caller's file, the other branch's edge is emitted too: branch A through _resolve_class_method (the local class's own method, else its #441 same-file base chain), branch B through _resolve_import. Never a duplicate (the primary filtered), never a fabricated edge (each branch abstains when it finds nothing). The BOTH-records condition is the guard, carrying #318's family exclusions: a name with ONE record never fans out (the negative controls); a FUNCTION-LOCAL namesake is not a shim (the non-defining caller sees only the import edge); a receiver the caller's own scope BINDS is a variable, not a class reference (the bound_names abstention). Disclosed sibling limits (named, not fixed): the CONSTRUCTOR edge on a dual name stays one-answer; the FUNCTION shim (an imported function + a local def) stays one-answer.
R3/R4 — PR 868, at head
|
| shape | master | this head |
|---|---|---|
M1 typed receiver on a dual name (t = Thing(); t.go()) |
1 edge → the local fallback | 2 — the import edge added |
M2 class-name form on a dual name (Thing.go(None)) |
1 edge → the import | 2 — the fallback edge added |
| external-import shim, M2 form — the primary abstains | 0 edges | 1 → the fallback (the F1 catch, measured) |
fallback inherits go() from a same-file base |
1 → Base.go |
2 — the import edge added |
| fallback lacks the method, a vendor class has it | 0 edges | 1 → the import only |
| local-only name (no import record) | 1 | identical — no fan-out |
| import-only name (no class record) | 1 | identical — no fan-out |
| function-local namesake + a plain import (the F2 guard) | 1 | identical — no fan-out |
bound-name shadow (def caller(Thing): Thing.go(), the F5 guard) |
1 | identical — no fan-out |
| the constructor on a dual name (disclosed limit) | 1 → local __init__ |
identical |
the function shim (from x import go + local def go, disclosed limit) |
1 → local go |
identical |
| the property read on a dual name (disclosed limit) | 1 → local val |
identical |
| both branches already answered from two call sites | 2 | identical — no duplicate |
| both branches resolve to the same id (dedup) | 1 | identical |
receiver is an attribute chain (self.t.go()) |
0 | identical |
| 200 ordinary calls, no shim anywhere (stress) | 200 | identical |
| the regression floor — an ordinary repo, no shim at all | 6 edges | byte-identical edge set |
The PR's own disclosed MISS was reproduced and shown NOT to be deepened: with a fallback lacking the method and exactly one unrelated same-named class elsewhere, the primary's step-3 cross-file unique match fabricates unrelated.py:Thing.go — identically on both legs. The PR names this in its limits list; it is pre-existing #440-family behaviour, not introduced here.
THE REAL-CORPUS LEG — 15,960 functions, not a synthetic argument
The python parser's own pipeline (FunctionExtractor → CallGraphBuilder) was run over the export's whole libs/openant-core on both legs:
master dbb79937 |
this head | |
|---|---|---|
| functions | 15,960 | 15,975 |
| classes | 2,065 | 2,065 |
| forward call edges | 25,736 | 25,770 |
Zero functions, zero classes and zero call edges were lost. The single apparent "loss" is an id relocation inside the changed file itself: the nested def _visit sits at line 419 at master and line 435 at this head, because the call-site hunk inserts exactly 16 lines above it, and this parser keys a nested unit by its line (_visit#L419 → _visit#L435). Normalising that one relocated id, the base edge set and function set are strict subsets of the head's. Of the 34 gained edges, 31 are the new test file's own calls and 3 are the new helper method's (_extract_calls_from_code → _resolve_shim_dual_branches, and that helper's two calls to _resolve_import / _same_file_base_walk).
A census that bounds the blast radius exactly: across the whole tree — 917 files carrying imports and 2,065 classes — there are ZERO dual-record names. The hook therefore fires zero times on OpenAnt's own source; the only behavioural change on the self-scan is the three new edges of the helper itself. The fix is real (proven on the shapes above) and its effect on this repository's own analysis is nil.
THE CONSUMERS THE DIFF DOES NOT NAME, ENUMERATED FROM THE BASE AND THEN EXECUTED
tests/conformance/test_F1_receiver_type_contract.py— the RECEIVER-TYPE CONFORMANCE CONTRACT. This is the strict consumer of exactly the surface the hook extends (it loads the pythonCallGraphBuilderby path and pins receiver-type resolution), the PR does not modify it, and the PR's evidence bundle does not name it. Run on both legs: green on both.tests/test_python_call_graph_builder.py— the seven-defect regression file for this very class, not modified, green on both legs.tests/parsers/python/test_issue318_typed_receiver_bases.pyandtest_issue440_mro_import_bases.py— the two families whose guards this hook mirrors (function_local, bound-name, the MRO chain). Not modified; green on both legs; and mutant I below shows the hook is wired tightly enough that breaking its receiver guard breaks Inherited methods resolve throughself/super()but not through a typed receiver or the class name, so same-file inherited calls emit no edge #318's pins, not only A try/except-ImportError shim gives one name two resolutions — a typed receiver binds the fallback class while the class-name form binds the import, and each loses the other's edge #705's.- The reachability consumer selection —
test_reachability_synthetic_harness_net.py,test_entry_point_detector.py,test_llm_reachability.py,test_reporter_reachable_units.py,test_reachability_prune_telemetry.py,test_call_graph_output.py— run on both legs: 298 passed / 5 skipped at master, 308 passed / 5 skipped at this head, zero failures either leg, the +10 being exactly this PR's new suite and the skip set identical (4 node/go-binary environment skips plus one out-of-tree harness skip). - The call site is purely additive into a
set.calls.add(extra)cannot remove an edge the primary produced, and_resolve_shim_dual_branchesperforms no mutation of builder state — so edge-set monotonicity is a structural property here, and the table above is its executed confirmation rather than its only evidence.
RULE 40 — ten mutants, nine killed, one predicted survivor, hash-bound, the instrument checked first
Control call_graph_builder.py 0600ffd58b92c05a → 271 passed, 1 skipped across tests/parsers/python/ plus three reachability consumers. The one skip is named, not hidden: test_F4_entry_root_additive_v3.py's pristine-vs-patched harness needs an out-of-tree pre-#165 checkout and an uncommitted patch file; no receipt below rests on it. Each mutant reverts a promise this PR makes; every anchor was asserted present before the edit; the control was restored byte-exact after each one (10/10) and re-confirmed.
| mutant | sha16 | result | caught by |
|---|---|---|---|
A — the hook re-gated behind if resolved: (the F1 false-negative direction) |
f780f1d60c5506d1 |
1 FAILED | test_external_import_shim_class_name_form_gets_the_fallback_edge |
B — the BOTH-records guard weakened to has_import alone (over-match) |
922dea441e975bc6 |
1 FAILED | test_function_local_namesake_is_not_a_shim |
| C — the F5 bound-name guard removed | b1877bdb3040d0a4 |
1 FAILED | test_bound_name_shadowing_a_dual_name_does_not_fan_out |
D — the F2 function-local exclusion removed from has_local |
5455801cb9e1776e |
1 FAILED | test_function_local_namesake_is_not_a_shim |
| E — the F7 catch reverted: branch A back through the cross-file unique match | 4e4ecfae4fd4086a |
1 FAILED | test_fallback_lacking_the_method_never_fabricates_cross_file |
| F — branch B (the import side) deleted — under-seed | 893a3ba307909b01 |
1 FAILED | test_shim_typed_receiver_reaches_both_branches |
| G — branch A (the local fallback) deleted — under-seed | 14431fc500099636 |
3 FAILED | the M2, external-import and base-chain pins |
H — the local_types typed-receiver remap removed |
a50cec3c171f5cdc |
1 FAILED | test_shim_typed_receiver_reaches_both_branches |
I — the receiver ast.Name guard removed (seeded expecting a possible survivor) |
f7d8e23c73ed192c |
4 FAILED | test_issue318_typed_receiver_bases.py ×4 — see the honesty note below |
| J — the duplicate filter removed (seeded expecting a survivor) | e8dd1faa0a607cfc |
SURVIVED — as predicted | nothing; see below |
Two of these are reported against the PR, not for it.
- J survived, and the prediction was stated before the run. The "never a duplicate" promise in the helper's docstring is not pinned by any test, because the call site adds into a
set— a duplicate is absorbed by the consumer, not prevented by the producer. The promise is therefore a property of the consumer, and the filter is defence-in-depth rather than a contract. Harmless, but the docstring states it as if the helper guaranteed it. - I was killed by an existence cascade, not by a behavioural pin. Removing
if not isinstance(recv, ast.Name)makesrecv.idraiseAttributeErroron any attribute-chain receiver, which crashes the builder and takes down four#318tests. That is a weaker pin than a behavioural one: the A try/except-ImportError shim gives one name two resolutions — a typed receiver binds the fallback class while the class-name form binds the import, and each loses the other's edge #705 suite itself does not test a non-ast.Namereceiver. The executed consequence is benign (shape L above: an attribute-chain receiver yields no edge on either leg), so this is a test-coverage note, not a defect.
WHAT THIS PR DOES NOT SAY — enumerated at this head and handed up
- The guard is broader than the word "shim". The condition is "a local class record and an import record for the same name", which also matches shapes that are not try/except-ImportError shims at all. Two were executed:
import vendor+class vendor:(master 1 edge → head 2), andimport vendor as Thing+ a localclass Thing(master 1 → head 2). In both, the head's added edge is to the binding that Python would actually use at runtime (the later module-levelclassstatement shadows the import), so the new edge is arguably the more correct one and the old is retained alongside it — over-seed, the safe direction. But the PR's prose frames the trigger as the shim, and the trigger is in fact the dual record. Not a regression (no edge is lost in either shape) and not blocking; a human should read the framing against#705's intent. - Branch A's locality claim is imprecise in the code's own docstring. It says branch A answers with "the local class's own or inherited method — the same-file base chain", but
#440made_same_file_base_walkimport-aware (_mro_first_definer), so branch A can legitimately reach a cross-file ancestor. Executed (shape K): a fallback inheriting from an imported base resolves through tobasemod.py:LocalBase.go. That edge is a real inheritance relation and it was already present at master via the primary, so nothing changed behaviourally — but "same-file base chain" is no longer what that helper does, and the F7 reasoning in the PR body leans on the word same-file. - A fifth copy of an existing predicate. The helper re-implements
_method_in_classinline (for func_id in self.methods_by_class.get(class_key, []): if ...get('name') == method_name) rather than calling it; that exact method exists 60 lines above in the same class. Cosmetic, zero behavioural effect, but it is the one-predicate-many-copies shape this repo has been repeatedly bitten by. - The three one-answer limits the PR discloses were each verified, not taken on trust — constructor, function shim and property read all read identical on both legs (table above). The disclosure is accurate.
- The pre-existing over-seed on
def caller(Thing): Thing.go()is master's, not this PR's. Master already emits an edge to the importedThing.godespite the parameter shadow; the new hook correctly declines to add a second. Recorded so the F5 guard is not credited with fixing something it only avoids making worse.
Receipts for every number above: runs/pr868-2026-10-26.md and runs/drain-2026-10-26/.
HALT at stage 3 — PR 868, head
|
… Exception (#705) The #705 hook wrapped its two resolver calls in `except Exception:` → `<hit> = None`. That is a BROAD swallow in the python resolver core, which had zero before it, and it diverges from this very file's convention: the same two helpers (`_same_file_base_walk`, `_resolve_import`) are called at five other sites here (852, 1112, 1232, 1262, 1385) and every one of them propagates. A broad catch there can hide a real failure as "no edge" — the under-seed direction the repo's reachability rule calls worst. Deleting the two handlers is NOT the repair, and that was measured rather than assumed. On an external-import shim whose local fallback carries a base chain thousands deep, reached by the class-name form only, the hook's branch A is the FIRST walker to touch that chain (on that shape the primary abstains and never walks it). Executed on four trees built from this head: master dbb7993 → OK, no edge this branch, authored → OK, no edge (swallowed) both handlers DELETED → RecursionError out of _resolve_shim_dual_branches → main() sys.exit(1) → the WHOLE repo's call graph is lost this commit → OK, no edge So the handlers are a live fail-safe, not dead code, and the honest repair is to NARROW them to the exception that is actually raised: `except RecursionError:`. That is this file's OWN idiom — `_module_containers` already catches `(SyntaxError, ValueError, RecursionError)` with the comment "never let a bad one abort the whole build_call_graph() — degrade … over-seed floor, not a crash". Recursion depth is the only live raise on either path (`_base_chain` carries a `_stack` cycle guard, `_resolve_import` a `seen` guard), and every other exception now propagates, as at the five unguarded sites. The degrade stays SILENT with a naming comment because that is what both of this file's in-class handlers do (181 and 595); the only handler that prints to stderr is `main()`'s CLI top level. No behaviour change on anything measured, re-derived on these bytes: tests/parsers/python/ + test_python_call_graph_builder.py + test_F1_receiver_type_contract.py = 247 passed, 1 skipped (identical to the authored head), and a real-corpus run of the parser's own pipeline over libs/openant-core = 15975 functions / 2065 classes / 25770 edges with a BYTE-IDENTICAL edge-set digest to the authored head. NOT fixed here, named instead: the TYPED-RECEIVER form of the same pathological input (`t = Thing(); t.go()`) raises RecursionError through the PRIMARY path (`_resolve_class_method` → `_same_file_base_walk`, cgb:1112) on MASTER ITSELF, with this hook nowhere on the stack. Widening the catch to cover it would mean wrapping the whole per-function `_extract_calls_from_code` call, degrading EVERY edge of a function instead of one speculative extra — a master-side defect and a scope widening #705 does not ask for.
R3/R4 — PR 868, at head
|
| tree | result |
|---|---|
master dbb79937 |
OK, no edge |
ba21b348 as authored |
OK, no edge (swallowed) |
| both handlers DELETED | RecursionError out of _resolve_shim_dual_branches → main() sys.exit(1) — the whole repo's call graph is lost |
this head 19c94441 |
OK, no edge |
So the handlers are a live fail-safe, not dead code, and the deletion would have turned a master-surviving input into a whole-analysis crash — the maximal under-seed.
The repair that landed (19c94441, +16/−2, one file): both handlers narrowed from except Exception: to except RecursionError:. That is this file's own idiom — _module_containers already catches (SyntaxError, ValueError, RecursionError) with the comment "never let a bad one abort the whole build_call_graph() — degrade … over-seed floor, not a crash". Recursion depth is the only live raise on either path (_base_chain carries a _stack cycle guard, _resolve_import a seen guard); every other exception now propagates, matching this file's five unguarded calls of the same two helpers (852, 1112, 1232, 1262, 1385) — the divergence the halt named is what the narrowing removes.
Two refinements, resolved against this file's own precedent rather than by preference:
- Placement — kept at the two call sites, not moved one level up. Both of this file's in-class handlers (181, 595) sit at the fragile operation and degrade locally; neither wraps a callee tree. A catch around the per-function
_extract_calls_from_codecall would degrade every edge of a function instead of one speculative extra — strictly more loss, the wrong direction for an over-seed rule. - Logging — silent, with a naming comment. Measured in this file: both in-class handlers degrade silently (595 with exactly such a comment); the only handler that prints to stderr is
main()'s CLI top level. The hook fires inside the loop over every function in the repo, so a print would be unbounded noise. Named rather than hidden: this is a deliberate silent-named degrade, and the comment is the record.
NOT fixed here, named instead: the typed-receiver form of the same pathological input (t = Thing(); t.go()) raises RecursionError through the primary path (_resolve_class_method → _same_file_base_walk, cgb:1112) on master itself, with this hook nowhere on the stack. That is a master-side defect and covering it is a scope widening #705 does not ask for.
Receipts for the repair, each executed on these bytes: the arm's own predicate returns to the baseline 2 (except RecursionError is not a type it flags — read from d3_degraded.scan_bare_except_empty, not assumed); tests/parsers/python/ + test_python_call_graph_builder.py + test_F1_receiver_type_contract.py = 247 passed, 1 skipped, identical to the authored head; a real-corpus run over the whole libs/openant-core = 15,975 functions / 2,065 classes / 25,770 edges with a byte-identical edge-set digest to the authored head; and two must-still-fail controls — re-seeding a bare except: or except Exception: at the same two sites takes the arm back to 4, so its teeth are intact in both directions.
R2 — THE NEVER-WAIVE CLASS, NAMED TWICE OVER (unchanged by the repair)
Re-derived from the whole diff at this head: 2 paths, +416 / −0 against master dbb79937 — libs/openant-core/parsers/python/call_graph_builder.py (+118 / −0) plus the 298-line #705 suite. Zero deletions anywhere in the diff, the narrowing included (it rewrites lines that are themselves additions against master).
parsers/* and a graph writer — this file is the python call-graph writer, and the change alters which call edges are emitted, i.e. which units are reachable — are both named in §6 R2's never-waive list (PROCESS-PR-MERGE-V1.md:371-373). So R2 = REACHABILITY / parsers/* + graph-writer, the never-AGENT-waive class. No PR-specific maintainer R2 comment exists and none is claimed here — the override rides the committed class grant under the authority ruling Q2 (13dbd03c: R2 is never-AGENT-waive, not never-maintainer-waive), on the 782/817/818/867 and 788/815/845 precedents.
What it is NOT, each checked by execution at this head: zero .gohtml, types.go, schemas.py or CODEOWNERS paths; zero .github/workflows paths; to_dict occurs zero times in the diff; config/models.json untouched; zero entry_point_detector paths. The six sibling-language call_graph_builder.py files (c, php, ruby, rust, swift, zig) are separate files and none is touched.
THE AUTHORITY, STATED PLAINLY
The repair commit rides the maintainer's committed fix-round grant for this PR (ADDENDUM 39, "the fix round on the sibling's branch is GRANTED under this delegation"). It is a prose grant under amendment 51, not the literal [PUSH-GRANT] string §7 C4 asks for — that deviation is disclosed here and on the receipt rather than papered over, as the 661 arc disclosed the same one. The push was lease-pinned (--force-with-lease at ba21b348), fast-forward, master never touched, and the pushed content was read back byte-equal to the local tree.
What
Closes #705 — PR #441's disclosed-but-never-filed limit, verbatim: "The try/except-ImportError shim: a module-level fallback class + an import gives M1 the fallback and M2 the import (two answers for one name; both-hit-emit-both is the deferred refinement)."
The two resolution paths picked different targets for the same name and each emitted only its own edge: the typed-receiver path (
t = Thing(); t.go()) bound the local fallback class; the class-name path (Thing.go(None)) bound the import. At parse time the branch of the shim that runs is unknowable, so one of the two edges was always a false negative.The fix
For an
obj.method(...)call whose receiver name carries both a same-file class record and an import record in the caller's file, both edges are emitted — and the hook fires even when the primary resolution abstained, because the dominant real-world shim has an external optional dependency (nothing in-repo to resolve on the import side; the fallback's edge must still be emitted).The guard carries #318's family exclusions so a single record never fans out:
Disclosed sibling limits (named, not fixed)
The constructor edge on a dual name stays one-answer (
Thing()binds the local__init__); the function shim (from x import go+ a localdef go) stays one-answer; the property read on a dual name stays one-answer (the hook sees only calls). The primary path's own cross-file unique-match (a fallback lacking the method + an unrelated same-named class) still fabricates through the typed-receiver primary — identical at base, a #440-family follow-up, recorded as the probed MISS in the evidence bundle.Evidence (runs/705)
7d64bf37..ce0bf611: 10 nodes / 4 witnesses, all assertion-red at base (the M1-both, the M2-both, the external-import M2, the fallback-base-chain variant); the reviewer re-derived the witnesses' behavioral red at base with the symbol pins stripped.if resolved:gate starving the external-import shim — the false-negative direction), F2/F5 (the function-local and bound-name guards), F7 (branch A's cross-file fabrication → the same-file restriction), with executed three-way controls (base / pre-fix / head) on every probe; terminal round TERMINAL DRY, three non-blocking notes in reconciliation.3 failed, 13202 passed, 38 skipped, 1 xfailed, 1 warning in 1694.24s— the standing trio (test_interrupt_then_resume+ the twollm_sdk_contract_floorpins), identity-matched in both legs;3 failed, 13213 passed, 38 skipped, 1 xfailed, 1 warning in 1624.21s— the +11 passing = this suite's 10 tests + the 1 parametrized no-hardcoded-paths item; the skips 38=38; the trio identity-matched;EXIT 0;head-parity-full-CONTAMINATED-npm-race.log, no receipt drawn from it; the settled leg is the parity evidence).