From f4c5cdade5bb5411f0d389afe62525c3d7b4a6ae Mon Sep 17 00:00:00 2001 From: IceRhymers Date: Thu, 23 Jul 2026 15:15:53 -0700 Subject: [PATCH 1/4] search: add query-time candidate-set resolver over reference_edges (#86) Two-query resolver (edge sites, then a SQL-window-bounded candidate scan per name) that resolves raw reference_edges.target_name to ranked symbols candidates without a self-join, mirroring symbols.py's auto-correlation avoidance. Ranking is membership-preserving so ambiguity is never silently collapsed, and candidate_count always carries the true pre-cap total even when the fetch is capped. Serve-only: no migration, no extraction-time symbol FK, no indexer change. --- app/search/references.py | 427 ++++++++++++++++++++++++++ tests/integration/test_references.py | 433 +++++++++++++++++++++++++++ tests/unit/test_references.py | 340 +++++++++++++++++++++ 3 files changed, 1200 insertions(+) create mode 100644 app/search/references.py create mode 100644 tests/integration/test_references.py create mode 100644 tests/unit/test_references.py diff --git a/app/search/references.py b/app/search/references.py new file mode 100644 index 0000000..e1cab6d --- /dev/null +++ b/app/search/references.py @@ -0,0 +1,427 @@ +"""Reference resolution: query-time candidate-set resolver over raw ``reference_edges``. + +The serve-side companion to :mod:`app.search.symbols` for the knowledge-graph epic (#82). +``reference_edges`` (0005, #83/#84/#85) stores raw, unresolved call/import sites -- deliberately +no FK to ``symbols`` (symbol ids churn on every per-file reindex). This module resolves a raw +edge's ``target_name`` to the ``symbols`` rows it could plausibly mean, at query time, by name. + +Design -- two queries, deliberately NOT one joined query (mirrors ``symbols.py``): + +1. Edge sites: ``reference_edges JOIN files JOIN repos``, bounded by ``row_limit``. +2. Candidate symbols: ``symbols JOIN files JOIN repos``, filtered to the distinct + ``target_name``s the first query returned, bounded PER NAME by a SQL window function + (``candidate_cap``) so a hot name (``get``, ``run``, ``__init__``) never pulls its entire + corpus-wide match set. + +Why two queries and not ``reference_edges JOIN symbols ON target_name = name`` (both reaching +through ``files``/``repos``): that is exactly the self-referencing join shape that lets +SQLAlchemy auto-correlate the two ``files``/``repos`` legs against each other, silently +mis-scoping which candidate belongs to which site. Neither statement here references the +other's tables, so there is zero correlation surface -- the same rationale as +``symbols.py``'s ``Symbol.file_id.in_([concrete ints])`` split. + +Ranking (which candidate is "the" definition for a call site) runs in Python, AFTER query 2, +because every signal (``same_repo``/``same_file``/``kind_match``) is relational to the +``(site, candidate)`` pair -- computing it in SQL would require the self-join this module +avoids. Ranking is membership-preserving: a lower-ranked candidate is never dropped, only +sorted later, so genuine ambiguity (AC1) is never silently collapsed to one answer. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from sqlalchemy import Connection, Row, Select, Text, any_, func, literal, select, text +from sqlalchemy.dialects.postgresql import ARRAY +from sqlalchemy.exc import OperationalError +from sqlalchemy.orm import aliased +from sqlalchemy.sql.elements import ColumnElement + +from app.db.models import File, ReferenceEdge, Repo, Symbol +from app.query.compiler import DEFAULT_ROW_LIMIT +from app.search.errors import reraise_or_query_too_broad + +# Per-request DB-time bound; a cancellation surfaces as QueryTooBroadError (mirrors symbols.py). +DEFAULT_STATEMENT_TIMEOUT_MS = 5000 + +# Per-name candidate ceiling: bounds the SQL FETCH (query 2's window), not just the payload. +DEFAULT_CANDIDATE_CAP = 32 + +# Call sites resolve to callables/constructors (`Foo()` is a constructor call). Import edges +# never earn this boost (D3/D4): `kind_match` is False for every import candidate uniformly. +CALL_TARGET_KINDS: frozenset[str] = frozenset({"function", "method", "class"}) + + +# --------------------------------------------------------------------------- contract + + +@dataclass(frozen=True) +class CandidateSymbol: + """One ranked candidate definition for an :class:`EdgeSite`. + + ``symbol_id`` is a query-time-transient internal tiebreak ONLY -- it is never persisted + (nothing here writes a resolved id back to ``reference_edges``) and is excluded from the + service-layer wire payload (see ``app.service._site_payload``), so carrying it does not + violate the epic's "never add an extraction-time symbol FK" rule. + """ + + symbol_id: int + repo_id: int + path: str + name: str + kind: str | None + start_line: int | None + same_repo: bool + same_file: bool + kind_match: bool + + +@dataclass(frozen=True) +class EdgeSite: + """One raw ``reference_edges`` row plus its resolved (ranked, possibly capped) candidates. + + ``candidate_count`` is the TRUE pre-cap count (SQL ``COUNT(*) OVER``, see + :func:`_build_candidates_select`) -- it never shrinks just because the fetched/returned + ``candidates`` list was capped, so ``resolution`` stays correct even under truncation. + ``candidates_truncated`` is ``candidate_count > len(candidates)``. + """ + + repo_id: int + file_id: int + path: str + line: int + edge_kind: str # "call" | "import" + target_name: str + enclosing_name: str | None + enclosing_kind: str | None + resolution: str # "unique" | "ambiguous" | "unresolved" + candidate_count: int + candidates_truncated: bool + candidates: tuple[CandidateSymbol, ...] + + +@dataclass(frozen=True) +class ReferenceResult: + """Result of :func:`resolve_references`. + + ``repo_known`` is ``False`` iff a ``repo=`` scope was requested but no such repo exists -- + a structured miss (mirrors ``get_file_payload``'s ``found: False``), never a silent empty. + It is always ``True`` when no repo scope was requested (nothing to be unknown about). + """ + + sites: tuple[EdgeSite, ...] + truncated: bool # site row-cap tripped + truncation_reason: str | None # "row_cap" | None + repo_known: bool + + +# ----------------------------------------------------------------------- pure helpers + + +def classify_resolution(count: int) -> str: + """Map a true candidate count to the ``resolution`` label. Shared with the measurement + script (D10) so the offline distribution and the serve path cannot drift apart.""" + if count == 0: + return "unresolved" + if count == 1: + return "unique" + return "ambiguous" + + +def _branch_predicate( + branch: str | None, + *, + file: type[File] = File, + repo: type[Repo] = Repo, +) -> ColumnElement[bool]: + """Branch-scoping predicate, byte-identical to ``get_file_payload``'s (``app/service.py``) + and the query compiler's implicit default conjunct. ``file``/``repo`` default to the + unaliased ORM classes (query 1 / query 2 each join ``files``/``repos`` exactly once); the + measurement script's correlated subquery (D10) passes aliased entities for its inner + ``symbols``-side join, which reaches a SECOND, distinct ``files``/``repos`` join in the + same statement. + """ + if branch is not None: + return file.branches.op("@>")(literal([branch], type_=ARRAY(Text))) + return func.coalesce(repo.default_branch, "HEAD") == any_(file.branches) + + +def _rank_candidates(candidates: list[CandidateSymbol]) -> tuple[CandidateSymbol, ...]: + """Total-ordered, membership-preserving rank (D4): same-repo first, then kind-appropriate, + then same-file, tiebreaking on ``(repo_id, path, start_line, symbol_id)`` for determinism. + Never drops a candidate -- a lower-ranked one only sorts later. + """ + return tuple( + sorted( + candidates, + key=lambda c: ( + not c.same_repo, + not c.kind_match, + not c.same_file, + c.repo_id, + c.path, + c.start_line or 0, + c.symbol_id, + ), + ) + ) + + +# ------------------------------------------------------------------------- SQL builders + + +def _build_sites_select( + *, + target_name: str | None, + edge_kind: str | None, + repo_id: int | None, + branch: str | None, + row_limit: int, +) -> Select: + """Query 1: edge sites, touching ``reference_edges``/``files``/``repos`` only. + + Joins/orders through the authoritative ``File.repo_id`` (not the denormalized + ``ReferenceEdge.repo_id``), mirroring ``symbols.py``'s same rule -- the branch predicate + compares ``Repo.default_branch`` against ``File.branches``, so both must be the same repo. + ``repo_id``, when given, filters ``ReferenceEdge.repo_id`` directly (index-served by + ``ix_reference_edges_repo_kind``) rather than a post-join ``Repo.name`` predicate. + """ + stmt = ( + select( + ReferenceEdge.id, + File.repo_id, + ReferenceEdge.file_id, + File.path, + ReferenceEdge.line, + ReferenceEdge.edge_kind, + ReferenceEdge.target_name, + ReferenceEdge.enclosing_name, + ReferenceEdge.enclosing_kind, + ) + .join(File, ReferenceEdge.file_id == File.id) + .join(Repo, File.repo_id == Repo.id) + .where(_branch_predicate(branch)) + .order_by( + File.repo_id, + File.path, + ReferenceEdge.line, + ReferenceEdge.id, + ) + .limit(row_limit) + ) + if target_name is not None: + stmt = stmt.where(ReferenceEdge.target_name == target_name) + if edge_kind is not None: + stmt = stmt.where(ReferenceEdge.edge_kind == edge_kind) + if repo_id is not None: + stmt = stmt.where(ReferenceEdge.repo_id == repo_id) + return stmt + + +def _build_candidates_select(*, names: list[str], branch: str | None, candidate_cap: int) -> Select: + """Query 2: candidate symbols for ``names``, bounded IN SQL (not just in the payload). + + ``ROW_NUMBER() OVER (PARTITION BY symbols.name ORDER BY ...)`` keeps only the first + ``candidate_cap`` rows per name by a name-intrinsic order (site-relative signals like + ``same_repo``/``same_file`` can't be pushed here -- they depend on the site, which this + query never sees). ``COUNT(*) OVER`` carries the TRUE pre-cap count out alongside the + trimmed rows, so :func:`classify_resolution` stays exact even when the fetch is capped. + """ + rn = ( + func.row_number() + .over( + partition_by=Symbol.name, + order_by=(File.repo_id, File.path, Symbol.start_line, Symbol.id), + ) + .label("rn") + ) + total = func.count().over(partition_by=Symbol.name).label("candidate_count") + inner = ( + select( + Symbol.id.label("symbol_id"), + Symbol.name, + Symbol.kind, + Symbol.start_line, + File.repo_id, + Symbol.file_id, + File.path, + rn, + total, + ) + .join(File, Symbol.file_id == File.id) + .join(Repo, File.repo_id == Repo.id) + .where(Symbol.name.in_(names), _branch_predicate(branch)) + .subquery() + ) + return select(inner).where(inner.c.rn <= candidate_cap) + + +def build_candidate_count_select(*, edge_kind: str, branch: str | None) -> Select: + """Per-site TRUE candidate count, reused by ``scripts/measure_reference_resolution.py`` + (D10) so the offline resolution-distribution measurement agrees with the serve path BY + CONSTRUCTION rather than re-implementing the join. Uses a correlated scalar subquery + (acceptable here: this builder has no per-request latency/timeout budget, unlike + :func:`resolve_references`'s window-bounded query 2) over an ALIASED ``files``/``repos`` + join, since the outer ``reference_edges``/``files``/``repos`` join already occupies the + unaliased names in this single statement. + + One row per matching edge: ``(edge_id, target_name, candidate_count)``. + """ + sym_file = aliased(File) + sym_repo = aliased(Repo) + count_subq = ( + select(func.count()) + .select_from(Symbol) + .join(sym_file, Symbol.file_id == sym_file.id) + .join(sym_repo, sym_file.repo_id == sym_repo.id) + .where( + Symbol.name == ReferenceEdge.target_name, + _branch_predicate(branch, file=sym_file, repo=sym_repo), + ) + .correlate(ReferenceEdge) + .scalar_subquery() + ) + return ( + select( + ReferenceEdge.id, + ReferenceEdge.target_name, + count_subq.label("candidate_count"), + ) + .join(File, ReferenceEdge.file_id == File.id) + .join(Repo, File.repo_id == Repo.id) + .where(ReferenceEdge.edge_kind == edge_kind, _branch_predicate(branch)) + ) + + +# --------------------------------------------------------------------- row -> dataclass + + +def _to_candidate( + row: Row, *, site_repo_id: int, site_file_id: int, kind_eligible: bool +) -> CandidateSymbol: + return CandidateSymbol( + symbol_id=row.symbol_id, + repo_id=row.repo_id, + path=row.path, + name=row.name, + kind=row.kind, + start_line=row.start_line, + same_repo=row.repo_id == site_repo_id, + same_file=row.file_id == site_file_id, + kind_match=kind_eligible and row.kind in CALL_TARGET_KINDS, + ) + + +def _build_edge_site(site_row: Row, candidate_rows: list[Row]) -> EdgeSite: + # kind_match eligibility is per-SITE (this edge's own kind), not the resolver's edge_kind + # filter param -- an unfiltered corpus-wide resolve can mix call/import sites. + kind_eligible = site_row.edge_kind == "call" + candidate_count = candidate_rows[0].candidate_count if candidate_rows else 0 + candidates = _rank_candidates( + [ + _to_candidate( + row, + site_repo_id=site_row.repo_id, + site_file_id=site_row.file_id, + kind_eligible=kind_eligible, + ) + for row in candidate_rows + ] + ) + return EdgeSite( + repo_id=site_row.repo_id, + file_id=site_row.file_id, + path=site_row.path, + line=site_row.line, + edge_kind=site_row.edge_kind, + target_name=site_row.target_name, + enclosing_name=site_row.enclosing_name, + enclosing_kind=site_row.enclosing_kind, + resolution=classify_resolution(candidate_count), + candidate_count=candidate_count, + candidates_truncated=candidate_count > len(candidates), + candidates=candidates, + ) + + +# ------------------------------------------------------------------------ entry point + + +def resolve_references( + conn: Connection, + *, + target_name: str | None = None, + edge_kind: str | None = None, + repo: str | None = None, + branch: str | None = None, + row_limit: int = DEFAULT_ROW_LIMIT, + candidate_cap: int = DEFAULT_CANDIDATE_CAP, + statement_timeout_ms: int = DEFAULT_STATEMENT_TIMEOUT_MS, +) -> ReferenceResult: + """Resolve raw ``reference_edges`` sites to ranked candidate-set ``symbols`` matches. + + ``target_name``/``edge_kind``/``repo`` are all optional filters (``find_references_payload`` + passes ``target_name``; ``list_imports_payload`` passes ``edge_kind="import"`` + a required + ``repo``). ``branch`` is a PARAMETER (not a query atom), applied identically to both the + edge site's file and each candidate's file (D6), mirroring ``get_file_payload``. + + Runs both queries in ONE transaction with a per-request ``statement_timeout``; a + cancellation raises :class:`~app.search.errors.QueryTooBroadError` (uncaught here -- the + service layer maps it, mirroring ``symbol_search``). A ``repo=`` scope that resolves to no + repo short-circuits to an empty result with ``repo_known=False`` and NO further DB work. + """ + with conn.begin(): + conn.execute( + text("SELECT set_config('statement_timeout', :ms, true)"), + {"ms": str(statement_timeout_ms)}, + ) + + repo_id: int | None = None + if repo is not None: + try: + repo_id = conn.execute( + select(Repo.id).where(Repo.name == repo) + ).scalar_one_or_none() + except OperationalError as error: + reraise_or_query_too_broad(error) + if repo_id is None: + return ReferenceResult( + (), truncated=False, truncation_reason=None, repo_known=False + ) + + sites_stmt = _build_sites_select( + target_name=target_name, + edge_kind=edge_kind, + repo_id=repo_id, + branch=branch, + row_limit=row_limit, + ) + try: + site_rows = conn.execute(sites_stmt).all() + except OperationalError as error: + reraise_or_query_too_broad(error) + + truncated = len(site_rows) >= row_limit + + names = sorted({row.target_name for row in site_rows}) + candidates_by_name: dict[str, list[Row]] = {} + if names: + candidates_stmt = _build_candidates_select( + names=names, branch=branch, candidate_cap=candidate_cap + ) + try: + candidate_rows = conn.execute(candidates_stmt).all() + except OperationalError as error: + reraise_or_query_too_broad(error) + for row in candidate_rows: + candidates_by_name.setdefault(row.name, []).append(row) + + sites = tuple( + _build_edge_site(row, candidates_by_name.get(row.target_name, [])) for row in site_rows + ) + return ReferenceResult( + sites=sites, + truncated=truncated, + truncation_reason="row_cap" if truncated else None, + repo_known=True, + ) diff --git a/tests/integration/test_references.py b/tests/integration/test_references.py new file mode 100644 index 0000000..4fe3266 --- /dev/null +++ b/tests/integration/test_references.py @@ -0,0 +1,433 @@ +"""Integration tests for the reference resolver: raw edges -> ranked candidate sets. + +Requires a running Postgres with the standard PG* env set. Mirrors the throwaway-schema idiom +of ``tests/integration/test_symbols_search.py`` (unique schema, ``SET search_path``, ``CREATE +EXTENSION pg_trgm``, ``Base.metadata.create_all`` on the same connection, ``DROP SCHEMA ... +CASCADE`` + ``engine.dispose()`` in ``finally``). In this repo that Postgres exists only as +CI's service container (or a local dev Postgres), so these tests are CI-only and were +validated locally by lint/type-check + ``--collect-only`` when no live Postgres is reachable. + +The ``seeded`` fixture is function-scoped: the timeout test inserts a large row volume and the +determinism/branch-scoping assertions rely on a clean corpus, so each test gets its own. +""" + +from __future__ import annotations + +import uuid +from collections.abc import Iterator +from typing import NamedTuple + +import pytest +from sqlalchemy import Connection, insert, text +from sqlalchemy.dialects import postgresql + +from app.db.client import create_db_engine +from app.db.models import Base, File, ReferenceEdge, Repo, Symbol +from app.search.errors import QueryTooBroadError +from app.search.references import ( + DEFAULT_CANDIDATE_CAP, + ReferenceResult, + _build_candidates_select, + _build_sites_select, + resolve_references, +) +from indexer.hashing import content_sha + +SCHEMA_PREFIX = "test_refsearch" + + +class Seeded(NamedTuple): + conn: Connection + acme_id: int + beta_id: int + files: dict[str, int] + + +def _unique(prefix: str) -> str: + return f"{prefix}_{uuid.uuid4().hex[:12]}" + + +def _insert_repo(conn: Connection, name: str, *, default_branch: str | None = "main") -> int: + return conn.execute( + insert(Repo).values(name=name, default_branch=default_branch).returning(Repo.id) + ).scalar_one() + + +def _insert_file( + conn: Connection, + repo_id: int, + path: str, + *, + lang: str | None = "python", + content: str | None = "pass\n", + branches: list[str] | None = None, +) -> int: + return conn.execute( + insert(File) + .values( + repo_id=repo_id, + path=path, + lang=lang, + content=content, + content_sha=content_sha(content), + branches=branches if branches is not None else ["main"], + ) + .returning(File.id) + ).scalar_one() + + +def _insert_symbol( + conn: Connection, + file_id: int, + repo_id: int, + name: str, + *, + kind: str | None = "function", + start_line: int | None = 1, +) -> int: + return conn.execute( + insert(Symbol) + .values(file_id=file_id, repo_id=repo_id, name=name, kind=kind, start_line=start_line) + .returning(Symbol.id) + ).scalar_one() + + +def _insert_edge( + conn: Connection, + file_id: int, + repo_id: int, + *, + edge_kind: str, + target_name: str, + line: int = 1, + enclosing_name: str | None = None, + enclosing_kind: str | None = None, +) -> int: + return conn.execute( + insert(ReferenceEdge) + .values( + file_id=file_id, + repo_id=repo_id, + edge_kind=edge_kind, + target_name=target_name, + line=line, + enclosing_name=enclosing_name, + enclosing_kind=enclosing_kind, + ) + .returning(ReferenceEdge.id) + ).scalar_one() + + +@pytest.fixture +def seeded() -> Iterator[Seeded]: + """Throwaway schema + durable-core DDL + a deterministic edge/symbol corpus.""" + schema = _unique(SCHEMA_PREFIX) + engine = create_db_engine() + conn = engine.connect() + try: + conn.execute(text(f"DROP SCHEMA IF EXISTS {schema} CASCADE")) + conn.execute(text(f"CREATE SCHEMA {schema}")) + conn.execute(text(f"SET search_path TO {schema}, public")) + conn.execute(text("CREATE EXTENSION IF NOT EXISTS pg_trgm")) + conn.commit() + + Base.metadata.create_all(bind=conn) + conn.commit() + + acme_id = _insert_repo(conn, "acme/widgets") + beta_id = _insert_repo(conn, "beta/tools") + files: dict[str, int] = {} + + # -- unique: exactly one candidate definition. + files["src/unique_target.py"] = _insert_file(conn, acme_id, "src/unique_target.py") + _insert_symbol(conn, files["src/unique_target.py"], acme_id, "unique_fn", start_line=2) + files["src/caller.py"] = _insert_file(conn, acme_id, "src/caller.py") + _insert_edge( + conn, + files["src/caller.py"], + acme_id, + edge_kind="call", + target_name="unique_fn", + line=5, + enclosing_name="handle", + enclosing_kind="function", + ) + + # -- ambiguous, same-repo duplicate definitions. + files["src/dup_a.py"] = _insert_file(conn, acme_id, "src/dup_a.py") + _insert_symbol(conn, files["src/dup_a.py"], acme_id, "ambiguous_fn", start_line=1) + files["src/dup_b.py"] = _insert_file(conn, acme_id, "src/dup_b.py") + _insert_symbol(conn, files["src/dup_b.py"], acme_id, "ambiguous_fn", start_line=1) + files["src/caller2.py"] = _insert_file(conn, acme_id, "src/caller2.py") + _insert_edge( + conn, files["src/caller2.py"], acme_id, edge_kind="call", target_name="ambiguous_fn" + ) + + # -- cross-repo ambiguous: same name defined in acme (same-repo) AND beta (cross-repo). + files["src/cross_local.py"] = _insert_file(conn, acme_id, "src/cross_local.py") + _insert_symbol(conn, files["src/cross_local.py"], acme_id, "cross_fn", start_line=1) + files["beta/cross.py"] = _insert_file(conn, beta_id, "beta/cross.py") + _insert_symbol(conn, files["beta/cross.py"], beta_id, "cross_fn", start_line=1) + files["src/caller3.py"] = _insert_file(conn, acme_id, "src/caller3.py") + _insert_edge( + conn, files["src/caller3.py"], acme_id, edge_kind="call", target_name="cross_fn" + ) + + # -- unresolved call: no matching symbol anywhere. + files["src/caller4.py"] = _insert_file(conn, acme_id, "src/caller4.py") + _insert_edge( + conn, files["src/caller4.py"], acme_id, edge_kind="call", target_name="missing_fn" + ) + + # -- unresolved import: dotted external target (D3, no last-segment split). + files["src/importer.py"] = _insert_file(conn, acme_id, "src/importer.py") + _insert_edge( + conn, files["src/importer.py"], acme_id, edge_kind="import", target_name="os.path" + ) + + # -- branch scoping: a site AND its candidate definition exist only on "feature". + files["src/feature_target.py"] = _insert_file( + conn, acme_id, "src/feature_target.py", branches=["feature"] + ) + _insert_symbol(conn, files["src/feature_target.py"], acme_id, "feature_fn", start_line=1) + files["src/feature_caller.py"] = _insert_file( + conn, acme_id, "src/feature_caller.py", branches=["feature"] + ) + _insert_edge( + conn, + files["src/feature_caller.py"], + acme_id, + edge_kind="call", + target_name="feature_fn", + ) + + conn.commit() + yield Seeded(conn=conn, acme_id=acme_id, beta_id=beta_id, files=files) + finally: + conn.rollback() + conn.execute(text(f"DROP SCHEMA IF EXISTS {schema} CASCADE")) + conn.commit() + conn.close() + engine.dispose() + + +def _resolve(conn: Connection, **kwargs: object) -> ReferenceResult: + return resolve_references(conn, **kwargs) # type: ignore[arg-type] + + +# ----------------------------------------------------------------------------- resolution + + +@pytest.mark.integration +def test_unique_call_resolves_to_single_candidate(seeded: Seeded) -> None: + result = _resolve(seeded.conn, target_name="unique_fn", edge_kind="call") + (site,) = result.sites + assert site.resolution == "unique" + assert site.candidate_count == 1 + assert site.enclosing_name == "handle" + (candidate,) = site.candidates + assert candidate.path == "src/unique_target.py" + assert candidate.same_repo is True + assert candidate.kind_match is True + + +@pytest.mark.integration +def test_ambiguous_same_repo_duplicates_never_collapsed(seeded: Seeded) -> None: + result = _resolve(seeded.conn, target_name="ambiguous_fn", edge_kind="call") + (site,) = result.sites + assert site.resolution == "ambiguous" + assert site.candidate_count == 2 + assert len(site.candidates) == 2 # AC1: ambiguity is never collapsed to one answer. + assert {c.path for c in site.candidates} == {"src/dup_a.py", "src/dup_b.py"} + + +@pytest.mark.integration +def test_cross_repo_ambiguous_ranks_same_repo_first(seeded: Seeded) -> None: + result = _resolve(seeded.conn, target_name="cross_fn", edge_kind="call") + (site,) = result.sites + assert site.resolution == "ambiguous" + assert site.candidate_count == 2 + assert len(site.candidates) == 2 + assert site.candidates[0].path == "src/cross_local.py" + assert site.candidates[0].same_repo is True + assert site.candidates[1].path == "beta/cross.py" + assert site.candidates[1].same_repo is False + + +@pytest.mark.integration +def test_unresolved_call_no_matching_symbol(seeded: Seeded) -> None: + result = _resolve(seeded.conn, target_name="missing_fn", edge_kind="call") + (site,) = result.sites + assert site.resolution == "unresolved" + assert site.candidate_count == 0 + assert site.candidates == () + + +@pytest.mark.integration +def test_unresolved_import_external_target(seeded: Seeded) -> None: + # D3: import target_name is the full dotted path; no symbol is literally named "os.path", + # so this resolves unresolved -- correctly representing an external/stdlib import. + result = _resolve(seeded.conn, target_name="os.path", edge_kind="import") + (site,) = result.sites + assert site.edge_kind == "import" + assert site.resolution == "unresolved" + + +# ------------------------------------------------------------------------- branch scoping + + +@pytest.mark.integration +def test_default_branch_excludes_feature_only_site(seeded: Seeded) -> None: + # No branch= given -> default-branch conjunct excludes the "feature"-only edge site. + result = _resolve(seeded.conn, target_name="feature_fn", edge_kind="call") + assert result.sites == () + + +@pytest.mark.integration +def test_explicit_branch_includes_feature_only_site_and_its_candidate(seeded: Seeded) -> None: + result = _resolve(seeded.conn, target_name="feature_fn", edge_kind="call", branch="feature") + (site,) = result.sites + assert site.resolution == "unique" + (candidate,) = site.candidates + assert candidate.path == "src/feature_target.py" + + +@pytest.mark.integration +def test_candidate_on_other_branch_is_excluded(seeded: Seeded) -> None: + # The candidate definition lives only on "feature"; querying "main" (feature_fn's site + # doesn't even exist there, but prove the candidate side of the predicate independently) + # via an explicit different branch must not surface it. + result = _resolve( + seeded.conn, target_name="feature_fn", edge_kind="call", branch="other-branch" + ) + assert result.sites == () + + +# ------------------------------------------------------------------------------ repo scope + + +@pytest.mark.integration +def test_repo_scope_filters_to_one_repo(seeded: Seeded) -> None: + result = _resolve(seeded.conn, edge_kind="import", repo="acme/widgets") + assert result.repo_known is True + assert all(site.repo_id == seeded.acme_id for site in result.sites) + + +@pytest.mark.integration +def test_unknown_repo_is_structured_miss_no_further_work(seeded: Seeded) -> None: + result = _resolve(seeded.conn, edge_kind="import", repo="ghost/repo") + assert result.repo_known is False + assert result.sites == () + assert result.truncated is False + + +# --------------------------------------------------------------------------- candidate cap + + +@pytest.mark.integration +def test_hot_name_bound_by_sql_window_not_just_payload(seeded: Seeded) -> None: + # More defs than DEFAULT_CANDIDATE_CAP -- the SQL window must bound the FETCH itself. + extra = DEFAULT_CANDIDATE_CAP + 8 + for i in range(extra): + fid = _insert_file(seeded.conn, seeded.acme_id, f"src/hot_{i}.py") + _insert_symbol(seeded.conn, fid, seeded.acme_id, "hot_fn", start_line=1) + caller = _insert_file(seeded.conn, seeded.acme_id, "src/hot_caller.py") + _insert_edge(seeded.conn, caller, seeded.acme_id, edge_kind="call", target_name="hot_fn") + seeded.conn.commit() + + result = _resolve(seeded.conn, target_name="hot_fn", edge_kind="call") + (site,) = result.sites + assert site.candidate_count == extra # true pre-cap total + assert len(site.candidates) == DEFAULT_CANDIDATE_CAP # fetch itself was bounded + assert site.candidates_truncated is True + assert site.resolution == "ambiguous" # never rewritten to "unique" by the cap + + +# ------------------------------------------------------------------------------ determinism + + +@pytest.mark.integration +def test_determinism_repeated_calls_identical_order(seeded: Seeded) -> None: + first = _resolve(seeded.conn, target_name="ambiguous_fn", edge_kind="call") + second = _resolve(seeded.conn, target_name="ambiguous_fn", edge_kind="call") + assert [c.path for c in first.sites[0].candidates] == [ + c.path for c in second.sites[0].candidates + ] + + +# ----------------------------------------------------------------------------- row cap + + +@pytest.mark.integration +def test_row_limit_truncates_sites(seeded: Seeded) -> None: + result = _resolve(seeded.conn, edge_kind="call", row_limit=1) + assert len(result.sites) == 1 + assert result.truncated is True + assert result.truncation_reason == "row_cap" + + +# --------------------------------------------------------------------------------- timeout + + +@pytest.mark.integration +def test_tiny_statement_timeout_raises_query_too_broad(seeded: Seeded) -> None: + # Deterministic DB-cancellation by WORK VOLUME (mirrors test_symbols_search.py / + # test_grep.py): a huge fan-in on one target_name forces a real sort of many matching + # reference_edges rows before the ORDER BY/LIMIT can short-circuit, guaranteed >> 1 ms. + caller = _insert_file(seeded.conn, seeded.acme_id, "src/blob_caller.py") + seeded.conn.execute( + insert(ReferenceEdge), + [ + { + "file_id": caller, + "repo_id": seeded.acme_id, + "edge_kind": "call", + "target_name": "hot_blob_fn", + "line": i + 1, + } + for i in range(20000) + ], + ) + seeded.conn.commit() + with pytest.raises(QueryTooBroadError): + _resolve( + seeded.conn, + target_name="hot_blob_fn", + edge_kind="call", + statement_timeout_ms=1, + ) + + +# ------------------------------------------------------------------------- EXPLAIN sanity + + +@pytest.mark.integration +def test_explain_sites_select_uses_repo_kind_index(seeded: Seeded) -> None: + stmt = _build_sites_select( + target_name=None, edge_kind="call", repo_id=seeded.acme_id, branch=None, row_limit=200 + ) + sql = str(stmt.compile(dialect=postgresql.dialect(), compile_kwargs={"literal_binds": True})) + savepoint = seeded.conn.begin_nested() + try: + seeded.conn.execute(text("SET LOCAL enable_seqscan = off")) + plan = seeded.conn.execute(text(f"EXPLAIN {sql}")).scalars().all() + finally: + savepoint.rollback() + plan_text = "\n".join(plan) + assert "ix_reference_edges_repo_kind" in plan_text, plan_text + + +@pytest.mark.integration +def test_explain_candidates_select_uses_symbols_name_trgm_index(seeded: Seeded) -> None: + stmt = _build_candidates_select( + names=["ambiguous_fn", "unique_fn"], branch=None, candidate_cap=32 + ) + sql = str(stmt.compile(dialect=postgresql.dialect(), compile_kwargs={"literal_binds": True})) + savepoint = seeded.conn.begin_nested() + try: + seeded.conn.execute(text("SET LOCAL enable_seqscan = off")) + plan = seeded.conn.execute(text(f"EXPLAIN {sql}")).scalars().all() + finally: + savepoint.rollback() + plan_text = "\n".join(plan) + assert "ix_symbols_name_trgm" in plan_text, plan_text diff --git a/tests/unit/test_references.py b/tests/unit/test_references.py new file mode 100644 index 0000000..bf0182e --- /dev/null +++ b/tests/unit/test_references.py @@ -0,0 +1,340 @@ +"""Unit tests for the reference resolver: pure helpers + rendered SQL. + +No DB: SQL shapes are asserted via ``stmt.compile(dialect=postgresql.dialect())`` (mirrors +``test_symbols_search.py``'s style), and the row -> dataclass assembly (``_build_edge_site``) +is exercised with fake row objects so the candidate-cap/ambiguity-preservation invariant is +covered without a live Postgres. The full two-query ``resolve_references`` end-to-end (branch +scoping, timeout, real window-function bounding) is exercised in the CI-only integration suite. +""" + +from __future__ import annotations + +from typing import Any + +import pytest +from sqlalchemy.dialects import postgresql + +from app.search.references import ( + CALL_TARGET_KINDS, + CandidateSymbol, + _build_candidates_select, + _build_edge_site, + _build_sites_select, + _rank_candidates, + build_candidate_count_select, + classify_resolution, +) + + +class _Row: + def __init__(self, **kw: Any) -> None: + self.__dict__.update(kw) + + +def _sql(stmt: Any) -> str: + return str(stmt.compile(dialect=postgresql.dialect())) + + +# --------------------------------------------------------------------- classify_resolution + + +@pytest.mark.unit +@pytest.mark.parametrize( + ("count", "expected"), + [(0, "unresolved"), (1, "unique"), (2, "ambiguous"), (31, "ambiguous")], +) +def test_classify_resolution(count: int, expected: str) -> None: + assert classify_resolution(count) == expected + + +# ------------------------------------------------------------------------- _rank_candidates + + +def _candidate( + *, + symbol_id: int, + repo_id: int = 1, + path: str = "a.py", + name: str = "f", + kind: str | None = "function", + start_line: int | None = 1, + same_repo: bool = True, + same_file: bool = True, + kind_match: bool = True, +) -> CandidateSymbol: + return CandidateSymbol( + symbol_id=symbol_id, + repo_id=repo_id, + path=path, + name=name, + kind=kind, + start_line=start_line, + same_repo=same_repo, + same_file=same_file, + kind_match=kind_match, + ) + + +@pytest.mark.unit +def test_rank_same_repo_before_cross_repo() -> None: + cross = _candidate(symbol_id=1, same_repo=False, repo_id=2) + same = _candidate(symbol_id=2, same_repo=True, repo_id=1) + ranked = _rank_candidates([cross, same]) + assert ranked == (same, cross) + + +@pytest.mark.unit +def test_rank_kind_match_before_same_file() -> None: + # Both same-repo; one is same-file but kind-mismatched, the other is kind-matched but a + # different file -- kind_match outranks same_file per the pinned D4 signal order. + same_file_wrong_kind = _candidate(symbol_id=1, same_file=True, kind_match=False) + other_file_right_kind = _candidate(symbol_id=2, same_file=False, kind_match=True) + ranked = _rank_candidates([same_file_wrong_kind, other_file_right_kind]) + assert ranked == (other_file_right_kind, same_file_wrong_kind) + + +@pytest.mark.unit +def test_rank_tiebreak_ends_in_symbol_id() -> None: + # Identical repo/path/start_line -- symbol_id is the only thing left to break the tie, so + # the order must be deterministic across repeated calls. + a = _candidate(symbol_id=5, path="same.py", start_line=10) + b = _candidate(symbol_id=2, path="same.py", start_line=10) + assert _rank_candidates([a, b]) == (b, a) + assert _rank_candidates([b, a]) == (b, a) + + +@pytest.mark.unit +def test_rank_unknown_kind_still_present_not_dropped() -> None: + # Membership-preserving: an unmatched kind earns no boost but is never removed. + unknown_kind = _candidate(symbol_id=1, kind="unknown_future_kind", kind_match=False) + ranked = _rank_candidates([unknown_kind]) + assert ranked == (unknown_kind,) + assert unknown_kind.kind not in CALL_TARGET_KINDS + + +# ------------------------------------------------------------------------ query 1 SQL shape + + +@pytest.mark.unit +def test_sites_select_orders_through_file_repo_id_ends_in_edge_id() -> None: + sql = _sql( + _build_sites_select( + target_name=None, edge_kind=None, repo_id=None, branch=None, row_limit=200 + ) + ) + order = sql.split("ORDER BY", 1)[1].split("LIMIT", 1)[0] + assert "files.repo_id" in order + assert "files.path" in order + assert "reference_edges.line" in order + assert order.strip().endswith("reference_edges.id") + assert "content_sha" not in sql + + +@pytest.mark.unit +def test_sites_select_default_branch_predicate_byte_identical_to_get_file_payload() -> None: + sql = _sql( + _build_sites_select( + target_name=None, edge_kind=None, repo_id=None, branch=None, row_limit=200 + ) + ) + assert "coalesce(repos.default_branch, %(coalesce_1)s) = ANY (files.branches)" in sql + + +@pytest.mark.unit +def test_sites_select_explicit_branch_predicate_uses_array_contains() -> None: + sql = _sql( + _build_sites_select( + target_name=None, edge_kind=None, repo_id=None, branch="feature", row_limit=200 + ) + ) + assert "files.branches @>" in sql + + +@pytest.mark.unit +def test_sites_select_filters_compose() -> None: + sql = _sql( + _build_sites_select( + target_name="Handler", edge_kind="call", repo_id=None, branch=None, row_limit=200 + ) + ) + assert "reference_edges.target_name = " in sql + assert "reference_edges.edge_kind = " in sql + + +@pytest.mark.unit +def test_sites_select_repo_id_filter_renders_edge_repo_id_not_repo_name() -> None: + sql = _sql( + _build_sites_select(target_name=None, edge_kind=None, repo_id=7, branch=None, row_limit=200) + ) + assert "reference_edges.repo_id = " in sql + assert "repos.name = " not in sql + + +@pytest.mark.unit +def test_sites_select_applies_limit() -> None: + sql = _sql( + _build_sites_select( + target_name=None, edge_kind=None, repo_id=None, branch=None, row_limit=50 + ) + ) + assert "LIMIT" in sql + + +# ------------------------------------------------------------------------ query 2 SQL shape + + +@pytest.mark.unit +def test_candidates_select_row_number_partitioned_by_name_bounded_by_cap() -> None: + sql = _sql(_build_candidates_select(names=["f"], branch=None, candidate_cap=32)) + assert "row_number() OVER (PARTITION BY symbols.name ORDER BY " in sql + assert "files.repo_id, files.path, symbols.start_line, symbols.id)" in sql + assert "rn <=" in sql or "rn <= " in sql + + +@pytest.mark.unit +def test_candidates_select_count_over_partitioned_by_name() -> None: + sql = _sql(_build_candidates_select(names=["f"], branch=None, candidate_cap=32)) + assert "count(*) OVER (PARTITION BY symbols.name)" in sql + + +@pytest.mark.unit +def test_candidates_select_exact_name_in_no_last_segment_split() -> None: + stmt = _build_candidates_select(names=["a.b.c", "f"], branch=None, candidate_cap=32) + # literal_binds so the bound names are visible in the rendered text: the full dotted + # "a.b.c" must appear verbatim -- no split into "c" (the last segment) anywhere. + sql = str(stmt.compile(dialect=postgresql.dialect(), compile_kwargs={"literal_binds": True})) + assert "symbols.name IN" in sql + assert "'a.b.c'" in sql + assert "'c'" not in sql + + +@pytest.mark.unit +def test_candidates_select_applies_branch_predicate() -> None: + sql = _sql(_build_candidates_select(names=["f"], branch="feature", candidate_cap=32)) + assert "files.branches @>" in sql + + +# --------------------------------------------------------------------- build_candidate_count_select + + +@pytest.mark.unit +def test_candidate_count_select_renders_correlated_subquery_and_branch_scope() -> None: + sql = _sql(build_candidate_count_select(edge_kind="call", branch=None)) + assert "reference_edges.edge_kind = " in sql + # Two DISTINCT joins to `files` in one statement (outer sites leg + inner correlated + # symbols leg) -> the inner leg must be aliased, never the same unaliased `files`. + assert sql.count("JOIN files AS files_1") == 1 + assert sql.count("JOIN files ON") == 1 + assert "coalesce(" in sql.lower() + + +@pytest.mark.unit +def test_candidate_count_select_explicit_branch() -> None: + sql = _sql(build_candidate_count_select(edge_kind="import", branch="feature")) + assert "@>" in sql + assert "reference_edges.edge_kind = " in sql + + +# ------------------------------------------------------------------- _build_edge_site (D2/D5) + + +@pytest.mark.unit +def test_build_edge_site_candidate_cap_preserves_true_count_and_ambiguous_resolution() -> None: + site_row = _Row( + id=1, + repo_id=1, + file_id=10, + path="a.py", + line=5, + edge_kind="call", + target_name="get", + enclosing_name=None, + enclosing_kind=None, + ) + # Fetched/returned rows are already SQL-bounded to candidate_cap (here: 2), but the + # `candidate_count` column carries the TRUE pre-cap total (here: 40) on every row. + candidate_rows = [ + _Row( + symbol_id=i, + name="get", + kind="function", + start_line=i, + repo_id=1, + file_id=10 + i, + path=f"c{i}.py", + candidate_count=40, + ) + for i in range(2) + ] + site = _build_edge_site(site_row, candidate_rows) # type: ignore[arg-type] + assert site.candidate_count == 40 + assert len(site.candidates) == 2 + assert site.candidates_truncated is True + assert site.resolution == "ambiguous" + + +@pytest.mark.unit +def test_build_edge_site_no_candidates_is_unresolved() -> None: + site_row = _Row( + id=1, + repo_id=1, + file_id=10, + path="a.py", + line=5, + edge_kind="import", + target_name="os.path", + enclosing_name=None, + enclosing_kind=None, + ) + site = _build_edge_site(site_row, []) # type: ignore[arg-type] + assert site.resolution == "unresolved" + assert site.candidate_count == 0 + assert site.candidates == () + assert site.candidates_truncated is False + + +@pytest.mark.unit +def test_build_edge_site_import_kind_match_always_false() -> None: + site_row = _Row( + id=1, + repo_id=1, + file_id=10, + path="a.py", + line=5, + edge_kind="import", + target_name="f", + enclosing_name=None, + enclosing_kind=None, + ) + candidate_rows = [ + _Row( + symbol_id=1, + name="f", + kind="function", + start_line=1, + repo_id=1, + file_id=10, + path="a.py", + candidate_count=1, + ) + ] + site = _build_edge_site(site_row, candidate_rows) # type: ignore[arg-type] + assert site.candidates[0].kind_match is False + + +@pytest.mark.unit +def test_build_edge_site_enclosing_symbol_none_when_module_scope() -> None: + site_row = _Row( + id=1, + repo_id=1, + file_id=10, + path="a.py", + line=5, + edge_kind="call", + target_name="f", + enclosing_name=None, + enclosing_kind=None, + ) + site = _build_edge_site(site_row, []) # type: ignore[arg-type] + assert site.enclosing_name is None + assert site.enclosing_kind is None From 61d5f83bed5dbd9ba32d0d8bbed31115078a96ac Mon Sep 17 00:00:00 2001 From: IceRhymers Date: Thu, 23 Jul 2026 15:15:57 -0700 Subject: [PATCH 2/4] search: add find_references/list_imports payload builders (#86) Additive app.service builders over the new resolver: find_references_payload (corpus-wide, call edges) and list_imports_payload (repo-scoped, import edges, structured repo_known miss). Repo-id-to-name resolution runs in a separate post-leg transaction, mirroring search_code_payload's _repo_name_map handling. MCP tool registration is a later child (#87). --- app/service.py | 171 ++++++++++++++++++++++++ tests/integration/test_service.py | 141 +++++++++++++++++++- tests/unit/test_service.py | 208 ++++++++++++++++++++++++++++++ 3 files changed, 519 insertions(+), 1 deletion(-) diff --git a/app/service.py b/app/service.py index 5a23c9c..5db19a6 100644 --- a/app/service.py +++ b/app/service.py @@ -44,6 +44,7 @@ ) from app.search.errors import QueryTooBroadError from app.search.grep import FileCursor, grep_search +from app.search.references import EdgeSite, ReferenceResult, resolve_references from app.search.semantic import _semantic_search_payload as semantic_search_payload # noqa: F401 from app.search.symbols import SymbolResult, symbol_search @@ -908,3 +909,173 @@ def get_file_payload( "found": found, "commit": commit, } + + +# ------------------------------------------------------------------- reference resolution + + +def _site_payload(site: EdgeSite, name_map: dict[int, str]) -> dict[str, Any]: + """Shape one resolved :class:`~app.search.references.EdgeSite` for the wire. + + ``symbol_id`` is deliberately absent from each candidate dict (D1/D4): it is a query-time + ranking tiebreak, never persisted, never a caller-facing identifier. + """ + enclosing_symbol = ( + {"name": site.enclosing_name, "kind": site.enclosing_kind} + if site.enclosing_name is not None + else None + ) + return { + "repo": name_map.get(site.repo_id, str(site.repo_id)), + "file": site.path, + "line": site.line, + "edge_kind": site.edge_kind, + "target_name": site.target_name, + "enclosing_symbol": enclosing_symbol, + "resolution": site.resolution, + "candidate_count": site.candidate_count, + "candidates_truncated": site.candidates_truncated, + "candidates": [ + { + "repo": name_map.get(candidate.repo_id, str(candidate.repo_id)), + "file": candidate.path, + "line": candidate.start_line, + "name": candidate.name, + "kind": candidate.kind, + "same_repo": candidate.same_repo, + "same_file": candidate.same_file, + "kind_match": candidate.kind_match, + } + for candidate in site.candidates + ], + } + + +def _reference_result_to_payload( + result: ReferenceResult, name_map: dict[int, str] +) -> dict[str, Any]: + """Shared envelope shape for :func:`find_references_payload` / :func:`list_imports_payload`: + ``sites`` + ``site_count`` + a ``resolution_summary`` histogram + truncation flags.""" + resolution_summary = {"unique": 0, "ambiguous": 0, "unresolved": 0} + for site in result.sites: + resolution_summary[site.resolution] += 1 + return { + "sites": [_site_payload(site, name_map) for site in result.sites], + "site_count": len(result.sites), + "resolution_summary": resolution_summary, + "truncated": result.truncated, + "truncation_reason": result.truncation_reason, + } + + +def _reference_repo_name_map(conn: Any, result: ReferenceResult, cfg: Settings) -> dict[int, str]: + """Resolve every repo id across a :class:`ReferenceResult`'s sites AND candidates to names. + + Run in a SEPARATE ``conn.begin()`` + ``SET LOCAL statement_timeout`` AFTER + :func:`resolve_references` returns -- its own ``set_config`` committed with its + transaction, so this lookup would otherwise run uncapped (mirrors ``search_code_payload``'s + post-leg ``_repo_name_map`` call). + """ + repo_ids = {site.repo_id for site in result.sites} + repo_ids |= {candidate.repo_id for site in result.sites for candidate in site.candidates} + if not repo_ids: + return {} + with conn.begin(): + conn.exec_driver_sql(f"SET LOCAL statement_timeout = {int(cfg.statement_timeout_ms)}") + return _repo_name_map(conn) + + +def find_references_payload( + engine: Engine, cfg: Settings, name: str, limit: int, branch: str | None = None +) -> dict[str, Any]: + """Resolve ``name``'s call sites to ranked candidate-set definitions. + + Corpus-wide (no ``repo`` scope) over ``edge_kind="call"`` edges. Ambiguity is never + collapsed: an ``"ambiguous"`` site's ``candidates`` list carries every ranked candidate up + to the per-name cap (AC1). ``limit`` is the caller's already-clamped row limit (mirrors + :func:`search_code_payload` -- clamping is the caller's responsibility, e.g. the future + MCP tool registration in #87). + """ + with engine.connect() as conn: + try: + result = resolve_references( + conn, + target_name=name, + edge_kind="call", + branch=branch, + row_limit=limit, + statement_timeout_ms=cfg.statement_timeout_ms, + ) + except QueryTooBroadError: + return { + "query": name, + "kind": "references", + "symbol": name, + "branch": branch, + "sites": [], + "site_count": 0, + "resolution_summary": {"unique": 0, "ambiguous": 0, "unresolved": 0}, + "truncated": True, + "truncation_reason": None, + "query_too_broad": True, + } + name_map = _reference_repo_name_map(conn, result, cfg) + + return { + "query": name, + "kind": "references", + "symbol": name, + "branch": branch, + "query_too_broad": False, + **_reference_result_to_payload(result, name_map), + } + + +def list_imports_payload( + engine: Engine, cfg: Settings, repo: str, limit: int, branch: str | None = None +) -> dict[str, Any]: + """Enumerate a repo's ``import`` edge sites (``repo`` is REQUIRED -- see D8: a corpus-wide + listing would filter on ``edge_kind`` alone, the trailing column of + ``ix_reference_edges_repo_kind (repo_id, edge_kind)``, which is not index-served). + + ``repo_known=False`` is a structured "no such repo" miss (mirrors ``get_file_payload``'s + ``found: False``) -- distinct from a known repo with zero import sites, which returns + ``repo_known=True`` and an empty ``sites`` list. Import edges are largely EXTERNAL by + design (D3: exact dotted-path match only, no last-segment split), so most sites are + expected to resolve ``"unresolved"`` -- that is not itself an error. + """ + with engine.connect() as conn: + try: + result = resolve_references( + conn, + edge_kind="import", + repo=repo, + branch=branch, + row_limit=limit, + statement_timeout_ms=cfg.statement_timeout_ms, + ) + except QueryTooBroadError: + return { + "query": repo, + "kind": "imports", + "repo": repo, + "branch": branch, + "repo_known": True, + "sites": [], + "site_count": 0, + "resolution_summary": {"unique": 0, "ambiguous": 0, "unresolved": 0}, + "truncated": True, + "truncation_reason": None, + "query_too_broad": True, + } + name_map = _reference_repo_name_map(conn, result, cfg) + + return { + "query": repo, + "kind": "imports", + "repo": repo, + "branch": branch, + "repo_known": result.repo_known, + "query_too_broad": False, + **_reference_result_to_payload(result, name_map), + } diff --git a/tests/integration/test_service.py b/tests/integration/test_service.py index 9e9763e..fe0c5d0 100644 --- a/tests/integration/test_service.py +++ b/tests/integration/test_service.py @@ -25,7 +25,7 @@ from app import service from app.config import Settings from app.db.client import create_db_engine -from app.db.models import Base, File, Repo, Symbol +from app.db.models import Base, File, ReferenceEdge, Repo, Symbol from indexer.hashing import content_sha SCHEMA_PREFIX = "test_service" @@ -521,3 +521,142 @@ def test_permalink_branch_or_multi_branch_picks_smallest_intersection_and_round_ content = file_payload["content"] or "" assert 'fmt.Println("feature")' in content assert 'fmt.Println("main")' not in content + + +# ---------------------------------------- find_references_payload / list_imports_payload + + +class RefSeeded(NamedTuple): + engine: Engine + cfg: Settings + acme_id: int + beta_id: int + + +@pytest.fixture +def ref_seeded() -> Iterator[RefSeeded]: + """Same PGOPTIONS idiom as ``seeded``, with a small call/import edge + symbol corpus.""" + schema = _unique(f"{SCHEMA_PREFIX}_ref") + admin_engine = create_db_engine() + admin_conn = admin_engine.connect() + prev_pgoptions = os.environ.get("PGOPTIONS") + engine: Engine | None = None + try: + admin_conn.execute(text(f"DROP SCHEMA IF EXISTS {schema} CASCADE")) + admin_conn.execute(text(f"CREATE SCHEMA {schema}")) + admin_conn.execute(text(f"SET search_path TO {schema}, public")) + admin_conn.execute(text("CREATE EXTENSION IF NOT EXISTS pg_trgm")) + admin_conn.commit() + + Base.metadata.create_all(bind=admin_conn) + admin_conn.commit() + + acme_id = admin_conn.execute( + insert(Repo).values(name="acme/widgets", default_branch="main").returning(Repo.id) + ).scalar_one() + beta_id = admin_conn.execute( + insert(Repo).values(name="beta/tools", default_branch="main").returning(Repo.id) + ).scalar_one() + + def _file(repo_id: int, path: str) -> int: + content = f"# {path}\n" + return admin_conn.execute( + insert(File) + .values( + repo_id=repo_id, + path=path, + lang="python", + content=content, + content_sha=content_sha(content), + branches=["main"], + ) + .returning(File.id) + ).scalar_one() + + target_id = _file(acme_id, "src/target.py") + admin_conn.execute( + insert(Symbol).values( + file_id=target_id, repo_id=acme_id, name="Handler", kind="function", start_line=2 + ) + ) + caller_id = _file(acme_id, "src/caller.py") + admin_conn.execute( + insert(ReferenceEdge).values( + file_id=caller_id, + repo_id=acme_id, + edge_kind="call", + target_name="Handler", + line=5, + enclosing_name="run", + enclosing_kind="function", + ) + ) + importer_id = _file(acme_id, "src/importer.py") + admin_conn.execute( + insert(ReferenceEdge).values( + file_id=importer_id, + repo_id=acme_id, + edge_kind="import", + target_name="os.path", + line=1, + ) + ) + admin_conn.commit() + + os.environ["PGOPTIONS"] = f"-c search_path={schema},public" + engine = create_db_engine() + yield RefSeeded(engine=engine, cfg=_cfg(), acme_id=acme_id, beta_id=beta_id) + finally: + if engine is not None: + engine.dispose() + if prev_pgoptions is None: + os.environ.pop("PGOPTIONS", None) + else: + os.environ["PGOPTIONS"] = prev_pgoptions + admin_conn.rollback() + admin_conn.execute(text(f"DROP SCHEMA IF EXISTS {schema} CASCADE")) + admin_conn.commit() + admin_conn.close() + admin_engine.dispose() + + +@pytest.mark.integration +def test_find_references_payload_end_to_end_wire_shape(ref_seeded: RefSeeded) -> None: + payload = service.find_references_payload(ref_seeded.engine, ref_seeded.cfg, "Handler", 200) + + assert payload["query"] == "Handler" + assert payload["kind"] == "references" + assert payload["site_count"] == 1 + assert payload["resolution_summary"] == {"unique": 1, "ambiguous": 0, "unresolved": 0} + (site,) = payload["sites"] + assert site["repo"] == "acme/widgets" + assert site["file"] == "src/caller.py" + assert site["enclosing_symbol"] == {"name": "run", "kind": "function"} + (candidate,) = site["candidates"] + assert candidate["repo"] == "acme/widgets" + assert candidate["file"] == "src/target.py" + assert candidate["same_repo"] is True + assert "symbol_id" not in candidate + + +@pytest.mark.integration +def test_list_imports_payload_end_to_end_wire_shape(ref_seeded: RefSeeded) -> None: + payload = service.list_imports_payload(ref_seeded.engine, ref_seeded.cfg, "acme/widgets", 200) + + assert payload["kind"] == "imports" + assert payload["repo"] == "acme/widgets" + assert payload["repo_known"] is True + assert payload["site_count"] == 1 + assert payload["resolution_summary"] == {"unique": 0, "ambiguous": 0, "unresolved": 1} + (site,) = payload["sites"] + assert site["edge_kind"] == "import" + assert site["target_name"] == "os.path" + + +@pytest.mark.integration +def test_list_imports_payload_unknown_repo_against_real_corpus(ref_seeded: RefSeeded) -> None: + payload = service.list_imports_payload(ref_seeded.engine, ref_seeded.cfg, "ghost/repo", 200) + + assert payload["repo_known"] is False + assert payload["sites"] == [] + assert payload["resolution_summary"] == {"unique": 0, "ambiguous": 0, "unresolved": 0} diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 99d45ab..201ab7c 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -18,6 +18,7 @@ from app.query.parser import parse from app.search.errors import QueryTooBroadError from app.search.grep import FileCursor, FileMatches, GrepResult, LineMatch +from app.search.references import CandidateSymbol, EdgeSite, ReferenceResult from app.search.symbols import SymbolMatch, SymbolResult # --------------------------------------------------------------------------- fixtures @@ -490,3 +491,210 @@ def test_query_has_symbol_atom_excludes_negated_symbol() -> None: assert service._query_has_symbol_atom(parse("-sym:foo")) is False # A positive sym: alongside a negated one still counts. assert service._query_has_symbol_atom(parse("-sym:foo sym:bar")) is True + + +# ---------------------------------------- find_references_payload / list_imports_payload + + +def _candidate( + *, + symbol_id: int = 1, + repo_id: int = 7, + path: str = "src/handler.go", + name: str = "Handler", + kind: str | None = "function", + start_line: int | None = 3, + same_repo: bool = True, + same_file: bool = False, + kind_match: bool = True, +) -> CandidateSymbol: + return CandidateSymbol( + symbol_id=symbol_id, + repo_id=repo_id, + path=path, + name=name, + kind=kind, + start_line=start_line, + same_repo=same_repo, + same_file=same_file, + kind_match=kind_match, + ) + + +def _site( + *, + repo_id: int = 7, + file_id: int = 1, + path: str = "src/caller.go", + line: int = 10, + edge_kind: str = "call", + target_name: str = "Handler", + enclosing_name: str | None = None, + enclosing_kind: str | None = None, + resolution: str = "unique", + candidate_count: int = 1, + candidates_truncated: bool = False, + candidates: tuple[CandidateSymbol, ...] = (), +) -> EdgeSite: + return EdgeSite( + repo_id=repo_id, + file_id=file_id, + path=path, + line=line, + edge_kind=edge_kind, + target_name=target_name, + enclosing_name=enclosing_name, + enclosing_kind=enclosing_kind, + resolution=resolution, + candidate_count=candidate_count, + candidates_truncated=candidates_truncated, + candidates=candidates, + ) + + +def _result( + *, + sites: tuple[EdgeSite, ...] = (), + truncated: bool = False, + truncation_reason: str | None = None, + repo_known: bool = True, +) -> ReferenceResult: + return ReferenceResult( + sites=sites, truncated=truncated, truncation_reason=truncation_reason, repo_known=repo_known + ) + + +@pytest.mark.unit +def test_find_references_payload_key_set_and_nested_shapes(monkeypatch: pytest.MonkeyPatch) -> None: + site = _site( + resolution="ambiguous", + candidate_count=2, + candidates=(_candidate(symbol_id=1), _candidate(symbol_id=2, same_repo=False, repo_id=8)), + ) + monkeypatch.setattr(service, "resolve_references", lambda *a, **k: _result(sites=(site,))) + monkeypatch.setattr( + service, "_repo_name_map", lambda conn: {7: "acme/widgets", 8: "beta/tools"} + ) + + payload = service.find_references_payload(_FakeEngine([]), _cfg(), "Handler", 200) + + assert payload["query"] == "Handler" + assert payload["kind"] == "references" + assert payload["symbol"] == "Handler" + assert payload["branch"] is None + assert payload["query_too_broad"] is False + assert payload["site_count"] == 1 + assert payload["resolution_summary"] == {"unique": 0, "ambiguous": 1, "unresolved": 0} + assert "repo_known" not in payload # only list_imports_payload carries this key + + [site_payload] = payload["sites"] + assert site_payload["repo"] == "acme/widgets" + assert site_payload["file"] == "src/caller.go" + assert site_payload["edge_kind"] == "call" + assert site_payload["enclosing_symbol"] is None + # AC1: ambiguity is never collapsed -- both ranked candidates survive to the wire. + assert len(site_payload["candidates"]) == 2 + candidate_payload = site_payload["candidates"][0] + assert "symbol_id" not in candidate_payload + assert candidate_payload["repo"] == "acme/widgets" + assert candidate_payload["same_repo"] is True + assert candidate_payload["kind_match"] is True + + +@pytest.mark.unit +def test_find_references_payload_empty_result_shape(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(service, "resolve_references", lambda *a, **k: _result()) + monkeypatch.setattr(service, "_repo_name_map", lambda conn: {}) + + payload = service.find_references_payload(_FakeEngine([]), _cfg(), "Missing", 200) + + assert payload["sites"] == [] + assert payload["site_count"] == 0 + assert payload["resolution_summary"] == {"unique": 0, "ambiguous": 0, "unresolved": 0} + assert payload["truncated"] is False + assert payload["query_too_broad"] is False + + +@pytest.mark.unit +def test_find_references_payload_query_too_broad(monkeypatch: pytest.MonkeyPatch) -> None: + def _raise(*_a: object, **_k: object) -> ReferenceResult: + raise QueryTooBroadError("too broad") + + monkeypatch.setattr(service, "resolve_references", _raise) + + payload = service.find_references_payload(_FakeEngine([]), _cfg(), "Handler", 200) + + assert payload["query_too_broad"] is True + assert payload["truncated"] is True + assert payload["sites"] == [] + assert payload["site_count"] == 0 + + +@pytest.mark.unit +def test_find_references_payload_branch_echo(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(service, "resolve_references", lambda *a, **k: _result()) + monkeypatch.setattr(service, "_repo_name_map", lambda conn: {}) + + payload = service.find_references_payload( + _FakeEngine([]), _cfg(), "Handler", 200, branch="feature/x" + ) + + assert payload["branch"] == "feature/x" + + +@pytest.mark.unit +def test_list_imports_payload_key_set_and_repo_scope(monkeypatch: pytest.MonkeyPatch) -> None: + site = _site( + edge_kind="import", target_name="a.b.c", resolution="unresolved", candidate_count=0 + ) + monkeypatch.setattr(service, "resolve_references", lambda *a, **k: _result(sites=(site,))) + monkeypatch.setattr(service, "_repo_name_map", lambda conn: {7: "acme/widgets"}) + + payload = service.list_imports_payload(_FakeEngine([]), _cfg(), "acme/widgets", 200) + + assert payload["kind"] == "imports" + assert payload["repo"] == "acme/widgets" + assert payload["repo_known"] is True + assert payload["resolution_summary"] == {"unique": 0, "ambiguous": 0, "unresolved": 1} + [site_payload] = payload["sites"] + assert site_payload["edge_kind"] == "import" + assert site_payload["target_name"] == "a.b.c" + + +@pytest.mark.unit +def test_list_imports_payload_unknown_repo_is_structured_miss( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(service, "resolve_references", lambda *a, **k: _result(repo_known=False)) + # No repos to resolve -- `_repo_name_map` must not even be reached, but stub it defensively. + monkeypatch.setattr(service, "_repo_name_map", lambda conn: {}) + + payload = service.list_imports_payload(_FakeEngine([]), _cfg(), "ghost/repo", 200) + + assert payload["repo_known"] is False + assert payload["sites"] == [] + assert payload["resolution_summary"] == {"unique": 0, "ambiguous": 0, "unresolved": 0} + + +@pytest.mark.unit +def test_list_imports_payload_query_too_broad(monkeypatch: pytest.MonkeyPatch) -> None: + def _raise(*_a: object, **_k: object) -> ReferenceResult: + raise QueryTooBroadError("too broad") + + monkeypatch.setattr(service, "resolve_references", _raise) + + payload = service.list_imports_payload(_FakeEngine([]), _cfg(), "acme/widgets", 200) + + assert payload["query_too_broad"] is True + assert payload["truncated"] is True + assert payload["repo_known"] is True # unknown vs. timeout are distinct outcomes + assert payload["sites"] == [] + + +@pytest.mark.unit +def test_reference_builders_importable_without_perturbing_search_code_export_set() -> None: + # Regression guard (plan D8 note): no existing unit test pins an exact `app.service` + # export set (test_main.py pins search_code's ENVELOPE, not the module's exports), so two + # additive builders are safe to add. This just proves they're importable as documented. + assert callable(service.find_references_payload) + assert callable(service.list_imports_payload) From 36f3bfc30fcbf790df69aa678e3853d26f1e2f34 Mon Sep 17 00:00:00 2001 From: IceRhymers Date: Thu, 23 Jul 2026 15:16:01 -0700 Subject: [PATCH 3/4] scripts: measure reference-edge resolution distribution (#86) Offline AC4 measurement CLI reusing the resolver's own build_candidate_count_select/classify_resolution so the reported distribution agrees with the serve path by construction. call edges are the headline metric compared against the epic's baseline; import edges are reported separately as informational. --- scripts/measure_reference_resolution.py | 174 ++++++++++++++++++ .../unit/test_measure_reference_resolution.py | 78 ++++++++ 2 files changed, 252 insertions(+) create mode 100644 scripts/measure_reference_resolution.py create mode 100644 tests/unit/test_measure_reference_resolution.py diff --git a/scripts/measure_reference_resolution.py b/scripts/measure_reference_resolution.py new file mode 100644 index 0000000..6ebceb0 --- /dev/null +++ b/scripts/measure_reference_resolution.py @@ -0,0 +1,174 @@ +"""Measure the ``reference_edges`` resolution distribution (#86, AC4). + +Offline companion to :mod:`app.search.references`. Reuses :func:`build_candidate_count_select` +and :func:`classify_resolution` from that module -- the SAME join semantics and branch-scoping +predicate ``resolve_references``'s query 2 uses -- so this script's distribution agrees with the +live serve path BY CONSTRUCTION, rather than re-implementing the join and risking drift. + +**Pinned: ``call`` edges are the primary headline metric**, the only number compared against +the prior-art baseline (28.8% unique / 33.4% ambiguous / 37.8% external). The ``import``-edge +distribution is reported separately, labeled informational -- it is expected to resolve +close to 0% (import targets are largely external/stdlib modules; see D3's exact-dotted-match +decision in ``docs/runbooks/reference-edges.md``) and is NOT compared against the call-edge +baseline. + +``build_candidate_count_select`` has no per-request latency/timeout budget (unlike +``resolve_references``): it runs a correlated subquery per site, which is fine for a one-off +offline measurement but would be an unacceptable query shape to expose to a live caller. + +Usage: ``uv run python scripts/measure_reference_resolution.py [--edge-kind call|import|both] +[--branch BRANCH] [--target NAME] [--use-resolver]``. Requires the standard ``PG*``/Lakebase +connection env (see ``app.db.client.create_db_engine``). +""" + +from __future__ import annotations + +import argparse +from collections.abc import Sequence +from typing import Any + +from app.db.client import create_db_engine +from app.search.references import ( + build_candidate_count_select, + classify_resolution, + resolve_references, +) + +# Prior-art comparison target from the epic's deep-dive probe (not a repo-sourced constant) -- +# re-measured here, which is what actually satisfies AC4. +CALL_BASELINE_PCT = {"unique": 28.8, "ambiguous": 33.4, "unresolved": 37.8} + +_BUCKETS = ("unique", "ambiguous", "unresolved") + + +# --------------------------------------------------------------------------- pure helpers + + +def bucket_counts(counts: Sequence[int]) -> dict[str, int]: + """Classify each site's true candidate count into a resolution bucket via the SAME + :func:`~app.search.references.classify_resolution` the resolver itself uses.""" + buckets = {bucket: 0 for bucket in _BUCKETS} + for count in counts: + buckets[classify_resolution(count)] += 1 + return buckets + + +def ambiguous_histogram(counts: Sequence[int]) -> dict[int, int]: + """Candidate-count histogram restricted to ambiguous sites (count >= 2).""" + histogram: dict[int, int] = {} + for count in counts: + if count >= 2: + histogram[count] = histogram.get(count, 0) + 1 + return histogram + + +def format_distribution( + label: str, buckets: dict[str, int], *, baseline: dict[str, float] | None = None +) -> str: + total = sum(buckets.values()) + lines = [f"{label} (n={total}):"] + for bucket in _BUCKETS: + count = buckets[bucket] + pct = (count / total * 100) if total else 0.0 + line = f" {bucket:<11s} {count:>7d} {pct:5.1f}%" + if baseline is not None: + line += f" (baseline {baseline[bucket]:.1f}%)" + lines.append(line) + return "\n".join(lines) + + +def format_histogram(histogram: dict[int, int]) -> str: + if not histogram: + return " (no ambiguous sites)" + return "\n".join( + f" candidate_count={count:<4d} sites={histogram[count]}" for count in sorted(histogram) + ) + + +# ------------------------------------------------------------------------------ DB legs + + +def _fetch_counts( + conn: Any, *, edge_kind: str, branch: str | None, target: str | None +) -> list[int]: + """Every matching site's TRUE candidate count, via the shared count builder (D10). + + Runs in its own ``conn.begin()``/commit: a bare ``conn.execute()`` auto-begins an + implicit transaction that stays open until explicitly closed, which would otherwise + collide with ``resolve_references``'s own ``with conn.begin():`` on a later call + reusing this same connection (``--use-resolver``). + """ + stmt = build_candidate_count_select(edge_kind=edge_kind, branch=branch) + with conn.begin(): + rows = conn.execute(stmt).all() + if target is not None: + rows = [row for row in rows if row.target_name == target] + return [row.candidate_count for row in rows] + + +def _resolver_spot_check( + conn: Any, *, edge_kind: str, branch: str | None, row_limit: int +) -> dict[str, int]: + """Drive ``resolve_references`` directly over a ``row_limit``-bounded sample and bucket its + OWN ``site.resolution`` field -- a live-path sanity check, not a full-corpus comparison + (the live resolver's query 2 is window-bounded per name; this builder's isn't).""" + result = resolve_references(conn, edge_kind=edge_kind, branch=branch, row_limit=row_limit) + buckets = {bucket: 0 for bucket in _BUCKETS} + for site in result.sites: + buckets[site.resolution] += 1 + return buckets + + +# --------------------------------------------------------------------------------- CLI + + +def main(argv: Sequence[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--edge-kind", choices=("call", "import", "both"), default="both") + parser.add_argument( + "--branch", default=None, help="branch scope (default: each repo's default branch)" + ) + parser.add_argument("--target", default=None, help="restrict to one target_name (debugging)") + parser.add_argument( + "--use-resolver", + action="store_true", + help="also spot-check via resolve_references directly (bounded sample, not full corpus)", + ) + parser.add_argument("--resolver-row-limit", type=int, default=500) + args = parser.parse_args(argv) + + kinds = ["call", "import"] if args.edge_kind == "both" else [args.edge_kind] + + engine = create_db_engine() + with engine.connect() as conn: + for kind in kinds: + counts = _fetch_counts(conn, edge_kind=kind, branch=args.branch, target=args.target) + buckets = bucket_counts(counts) + baseline = CALL_BASELINE_PCT if kind == "call" else None + label = ( + "call edges -- HEADLINE AC4 metric" + if kind == "call" + else "import edges -- informational, expected ~0% resolution (validates D3)" + ) + print(format_distribution(label, buckets, baseline=baseline)) + print("ambiguous candidate-count histogram:") + print(format_histogram(ambiguous_histogram(counts))) + print() + + if args.use_resolver: + sample_buckets = _resolver_spot_check( + conn, edge_kind=kind, branch=args.branch, row_limit=args.resolver_row_limit + ) + print( + format_distribution( + f"{kind} edges -- resolve_references spot check " + f"(row_limit={args.resolver_row_limit}, bounded sample)", + sample_buckets, + ) + ) + print() + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/unit/test_measure_reference_resolution.py b/tests/unit/test_measure_reference_resolution.py new file mode 100644 index 0000000..0a62e64 --- /dev/null +++ b/tests/unit/test_measure_reference_resolution.py @@ -0,0 +1,78 @@ +"""Unit tests for the offline resolution-distribution measurement script (AC4). + +Pure-Python bucketing/formatting helpers only -- no DB. ``build_candidate_count_select``'s +rendered SQL shape (branch-scoped, edge_kind-filtered, correlated-subquery join) is covered by +``tests/unit/test_references.py``; this file only proves the script's own helpers stay +faithful to the shared ``classify_resolution`` the resolver uses (no independent re-derivation +that could drift). +""" + +from __future__ import annotations + +import pytest + +from scripts.measure_reference_resolution import ( + CALL_BASELINE_PCT, + ambiguous_histogram, + bucket_counts, + format_distribution, + format_histogram, +) + + +@pytest.mark.unit +def test_bucket_counts_empty() -> None: + assert bucket_counts([]) == {"unique": 0, "ambiguous": 0, "unresolved": 0} + + +@pytest.mark.unit +def test_bucket_counts_classifies_via_shared_helper() -> None: + # 0 -> unresolved, 1 -> unique, >=2 -> ambiguous (mirrors classify_resolution exactly). + assert bucket_counts([0, 0, 1, 2, 31]) == {"unique": 1, "ambiguous": 2, "unresolved": 2} + + +@pytest.mark.unit +def test_ambiguous_histogram_ignores_unique_and_unresolved() -> None: + assert ambiguous_histogram([0, 1, 2, 2, 3]) == {2: 2, 3: 1} + + +@pytest.mark.unit +def test_ambiguous_histogram_empty_when_no_ambiguous_sites() -> None: + assert ambiguous_histogram([0, 0, 1, 1]) == {} + + +@pytest.mark.unit +def test_format_distribution_includes_counts_and_percentages() -> None: + text = format_distribution("call edges", {"unique": 1, "ambiguous": 1, "unresolved": 2}) + assert "call edges (n=4):" in text + assert "unique" in text + assert "50.0%" in text # unresolved: 2/4 + + +@pytest.mark.unit +def test_format_distribution_zero_total_does_not_divide_by_zero() -> None: + text = format_distribution("empty", {"unique": 0, "ambiguous": 0, "unresolved": 0}) + assert "(n=0):" in text + assert "0.0%" in text + + +@pytest.mark.unit +def test_format_distribution_with_baseline_renders_comparison() -> None: + text = format_distribution( + "call edges", {"unique": 1, "ambiguous": 1, "unresolved": 0}, baseline=CALL_BASELINE_PCT + ) + assert "baseline 28.8%" in text + assert "baseline 33.4%" in text + assert "baseline 37.8%" in text + + +@pytest.mark.unit +def test_format_histogram_empty() -> None: + assert "no ambiguous" in format_histogram({}) + + +@pytest.mark.unit +def test_format_histogram_sorted_by_candidate_count() -> None: + text = format_histogram({3: 1, 2: 5}) + # candidate_count=2 must render before candidate_count=3 (sorted, not insertion order). + assert text.index("candidate_count=2") < text.index("candidate_count=3") From 47ec321265248a978a8dd3405e8775f848f1d5a8 Mon Sep 17 00:00:00 2001 From: IceRhymers Date: Thu, 23 Jul 2026 15:16:04 -0700 Subject: [PATCH 4/4] docs: document query-time reference resolution in reference-edges runbook (#86) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds §4 covering the resolver's two-query design, candidate-set/resolution semantics, the exact-dotted-match import decision, repo_known for list_imports, branch-scoping parity with search_code/get_file, and the measurement script with a recorded distribution from self-indexing this repo's own tracked source. --- docs/runbooks/reference-edges.md | 104 +++++++++++++++++++++++++++++-- 1 file changed, 98 insertions(+), 6 deletions(-) diff --git a/docs/runbooks/reference-edges.md b/docs/runbooks/reference-edges.md index 838f976..0c7fc8b 100644 --- a/docs/runbooks/reference-edges.md +++ b/docs/runbooks/reference-edges.md @@ -12,10 +12,10 @@ landed on a given deploy target. `reference_edges` is a **raw, unresolved** call/import edge extracted from one file's content-version: `(edge_kind, target_name, line, enclosing_*)` per site tree-sitter finds, with `edge_kind IN ('call', 'import')` enforced by a CHECK constraint. It is part of the -knowledge-graph epic (#82): a later child (#86) resolves `target_name` to a concrete -`symbols` row at **query time**, by name-join — this table deliberately carries **no -foreign key to `symbols`**. Symbol ids churn on every per-file delete-and-reinsert, and an -FK would couple the two rewrite orders inside the indexing transaction for no query +knowledge-graph epic (#82): `target_name` is resolved to concrete `symbols` rows at +**query time**, by name-join — shipped in #86 (see §4) — this table deliberately carries +**no foreign key to `symbols`**. Symbol ids churn on every per-file delete-and-reinsert, and +an FK would couple the two rewrite orders inside the indexing transaction for no query benefit. The enclosing symbol (the function/class a call or import site sits inside, if any) is @@ -67,10 +67,10 @@ bump behaved for `chunks`. | Index | Serves | |---|---| -| `ix_reference_edges_target_name` (btree) | The resolver's name-equality join (`symbols.name = reference_edges.target_name`) once #86 ships | +| `ix_reference_edges_target_name` (btree) | The resolver's name-equality join (`symbols.name = reference_edges.target_name`), shipped in #86 (§4) | | `ix_reference_edges_target_trgm` (GIN, `gin_trgm_ops`) | Partial/substring reference lookups, parity with `ix_symbols_name_trgm` | | `ix_reference_edges_file_id` (btree) | The per-file delete-and-reinsert writer (#84) and the `ON DELETE CASCADE` fired by the sweep/reconcile paths — Postgres does not auto-index a foreign key, and both are hot paths | -| `ix_reference_edges_repo_kind` (btree, `(repo_id, edge_kind)`) | Per-repo kind scans (e.g. a future `list_imports` MCP tool) | +| `ix_reference_edges_repo_kind` (btree, `(repo_id, edge_kind)`) | Per-repo kind scans, e.g. `list_imports_payload`'s required `repo` scope (§4, #86) | ## 3. Deploy coupling — this migration is NOT schema-only @@ -116,6 +116,98 @@ SELECT has_table_privilege('', 'reference_edges', 'SELECT'), Both must return `true`. If either is `false`, run the re-grant command above — it is idempotent, safe to run against a target that's already current. +## 4. Query-time resolution (#86) + +`app/search/references.py` resolves a raw `reference_edges` row's `target_name` to the +`symbols` rows it could plausibly mean, entirely at query time — nothing from this +resolution is ever written back to `reference_edges` or `symbols`, and no extraction-time +symbol FK was added. `resolve_references(conn, ...)` is the entry point; `app/service.py` +wraps it in two additive payload builders, `find_references_payload` (corpus-wide, +`edge_kind="call"`) and `list_imports_payload` (`edge_kind="import"`, `repo` required). MCP +tool registration for these is a later child (#87), not #86. + +**Two-query design, deliberately not one joined query.** Query 1 selects matching edge +sites from `reference_edges`/`files`/`repos`; query 2 selects candidate `symbols` for the +distinct `target_name`s query 1 returned, from `symbols`/`files`/`repos`. Neither query +references the other's tables. A single `reference_edges JOIN symbols ON target_name = +name` (both reaching through `files`/`repos`) would let SQLAlchemy auto-correlate the two +`files`/`repos` legs against each other, silently mis-scoping which candidate belongs to +which site — the same auto-correlation hazard `app/search/symbols.py` avoids for `sym:` +lookups. Query 2 bounds its **fetch itself** (not just the returned payload) with a SQL +window function per `target_name` (`ROW_NUMBER() OVER (PARTITION BY symbols.name ORDER BY +...)`, capped at `DEFAULT_CANDIDATE_CAP = 32`), so a hot name (`get`, `run`, `__init__`) +never pulls its entire corpus-wide match set into memory. `COUNT(*) OVER` carries the TRUE +pre-cap count alongside the trimmed rows, so ambiguity is never rewritten to "unique" just +because the candidate list was capped. + +**Candidate-set contract.** Each edge site resolves to zero, one, or many ranked +candidates — `resolution` is `"unresolved"` (0), `"unique"` (1), or `"ambiguous"` (2+), +derived from the true pre-cap `candidate_count`. Ranking (same-repo before cross-repo, then +kind-appropriate, then same-file, tiebreaking on `(repo_id, path, start_line, symbol_id)` +for a deterministic total order) runs in Python after query 2, since every signal is +relational to the `(site, candidate)` pair. Ranking is **membership-preserving**: a +lower-ranked candidate is sorted later, never dropped, so genuine ambiguity is always +represented in full (up to the cap) rather than silently collapsed to one answer. + +**Import edges resolve to their full dotted path, no last-segment split.** `import` +`target_name` is the complete dotted path as written (see §1); `symbols.name` is bare, so +an import edge resolves to a candidate only in the rare case a symbol is literally named +that full dotted string. This is pinned deliberately, not a gap: (1) it keeps one +exact-equality, index-served predicate identical for both edge kinds, with no functional +index and no client-side splitting; (2) a dotted import genuinely points at an +external/stdlib module most of the time, so representing it as `"unresolved"` (= external) +is *correct*, not a miss; (3) a last-segment heuristic (`a.b.get` → every symbol named +`get`) would manufacture false ambiguity and defeat precision. `list_imports_payload`'s +value is *enumerating* import sites with their `target_name`, not resolving them to local +definitions. + +**`repo` is required for `list_imports_payload`.** A corpus-wide import listing would +filter on `edge_kind` alone — the trailing column of `ix_reference_edges_repo_kind +(repo_id, edge_kind)`, not index-served on its own — so corpus-wide listing is out of +scope. `repo_known: False` is a structured "no such repo" miss (mirrors `get_file_payload`'s +`found: False`), distinguishable from a known repo with zero import sites +(`repo_known: True`, `sites: []`). + +**Branch scoping matches `search_code`/`get_file` exactly**, applied independently to BOTH +the edge site's file and each candidate's file: an explicit `branch` uses +`files.branches @> ARRAY[:branch]`; omitted, it falls back to +`coalesce(repos.default_branch, 'HEAD') = ANY(files.branches)` — the same predicate +`get_file_payload` uses, asserted byte-identical in `tests/unit/test_references.py` and +exercised end-to-end in `tests/integration/test_references.py`. + +**Quality measurement (`scripts/measure_reference_resolution.py`).** An offline script +reuses `app.search.references.build_candidate_count_select` and `classify_resolution` — the +SAME join semantics and branch predicate the live resolver's query 2 uses — so its +distribution agrees with the serve path by construction rather than re-implementing the +join. **`call` edges are the primary headline metric**, the only number compared against +the epic's deep-dive baseline (28.8% unique / 33.4% ambiguous / 37.8% external); the +`import`-edge distribution is reported separately, labeled informational (expected close to +0% resolution, validating the exact-dotted-match decision above). Run it with: + +``` +uv run python scripts/measure_reference_resolution.py --edge-kind both +``` + +Recorded distribution (self-indexed corpus: this repo's own git-tracked source tree — +206 files, 2,947 symbols, 15,412 reference edges across Python/JS/TS/TSX — default branch, +measured on 2026-07-23; see the #86 PR body for the full script output): + +``` +call edges -- HEADLINE AC4 metric (n=14226): + unique 4144 29.1% (baseline 28.8%) + ambiguous 4208 29.6% (baseline 33.4%) + unresolved 5874 41.3% (baseline 37.8%) + +import edges -- informational, expected ~0% resolution (validates D3) (n=1186): + unique 15 1.3% + ambiguous 8 0.7% + unresolved 1163 98.1% +``` + +The re-measured `call`-edge distribution tracks the baseline closely (within ~4 points on +every bucket); `import` edges resolve at ~2% total, confirming they are overwhelmingly +external/stdlib targets as D3 predicts. + ## Reference - [multi-branch.md §3](multi-branch.md#3-deploy-coupling--this-migration-is-not-schema-only) —