Skip to content

resolver: a first-party root whose imports are all two segments classifies EXTERNAL #438

Description

@zaebee

Found while reviewing #437 (the #435 guard). Pre-existing on main; the guard is what makes it visible.

The gap

_build_external_roots decides a root is first-party when any import value under it strips to a real node (_strips_to_a_node, indices.py:198). That function requires two segments to remain after stripping, for a documented reason: allowing one would let from pydantic import config strip to a project's own config.py and classify the whole of pydantic as INTERNAL.

The cost is that a root whose every first-party import is exactly two segments never qualifies. from app import models has value app.models; range(1, len(parts) - 1) is empty, so nothing is tested at all.

Reproduced at 7118807:

nodes:  models.py, svc.py            # ingested at app/
svc.py: from app import models
        def go(): models.get_user()

external_roots = ['app']             # a first-party root, called external

What it costs today, and what it will cost

On main the misclassification is latent — the strip loop resolves the call anyway, so only classify_fqn consumers see it: namespace stats, unresolved_ratio, anything that filters on EXTERNAL.

With #437's guard the strip loop refuses to discard a foreign head, so the call stops resolving. That is the correct behaviour given the classification; the classification is what is wrong.

How narrow it is, measured

One deeper first-party import anywhere in the repository fixes the whole root:

+ other.py: from app.models import get_user      # a single 3-segment value

external_roots = []                              # 'app' now first-party

So it takes a codebase where no import under the root has three or more segments — a consistently module-style (from pkg import module) first-party layout with no exceptions. Real, but not common. Ownima/owner-api is not affected: it imports from app.models import X in many places.

Shape of a fix

The #424 discriminator was chosen because it separated cleanly on measured data — app had 3346 of 4885 values reach a node, while pydantic, sqlalchemy, grpc, fastapi and httpx had zero each. That is a ratio, and the implementation collapsed it to any(...) plus a two-segment floor. Replacing the floor with the ratio would admit app.models (1 of 1 resolves) while still rejecting pydantic (1 of many).

That changes classification for every repository, so it needs the corpus measurement #319 asks for rather than a patch: baseline external_roots, unresolved_ratio per domain, and the tolerances.lock ratchet on the #179 dogfooding set before and after.

Marker in the tree

tests/unit/test_resolver.py::test_module_style_first_party_import_at_subdirectory_ingest is xfail(strict=True), so the suite fails the day this is fixed and the marker is left behind.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions