From b87178d2619992738b1a8db9df8860a967f77a27 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 26 Oct 2026 20:05:09 -0400 Subject: [PATCH] fix(6495): a name defined in two packages counts only in the canon's own A by-name reference edge could not tell two definitions of one name apart: in a Go repo that defines `file` as importRun.file (internal/files, the canon) and as sftpRun.file (internal/replication), a mirror test calling `m.file(` was recorded as a use of the canon. names_canon now takes defined_at, every repo path that defines each name, in place of the shape file's own names. The own-file rule is unchanged. When the repo also defines the canon's name somewhere the canon is not recorded, a mention counts only from the canon's own package: the directory of a file location exactly, or under a directory location. A name defined once resolves as before. Proposer version 8 -> 9, so the next coverage refresh re-reads every row and replaces its reference edges. Co-Authored-By: Claude Opus 5.5 --- src/grimoire/services/shape_ledger.py | 70 +++++++++++++++++++-------- tests/test_shape_ledger.py | 29 +++++++++-- 2 files changed, 77 insertions(+), 22 deletions(-) diff --git a/src/grimoire/services/shape_ledger.py b/src/grimoire/services/shape_ledger.py index 7bdb78be..dc34576e 100644 --- a/src/grimoire/services/shape_ledger.py +++ b/src/grimoire/services/shape_ledger.py @@ -23,9 +23,10 @@ from __future__ import annotations import difflib import logging +import posixpath import re from datetime import datetime, timedelta, timezone -from typing import Iterable, NamedTuple +from typing import Iterable, Mapping, NamedTuple from sqlalchemy import select @@ -1798,7 +1799,7 @@ def _semantic_priority(row) -> tuple: # were examined under, so a tightened rule re-examines everything once. A # change that does not alter what is proposed needs no bump. A stale "miss" # basis on an old row is inert: a miss is not evidence (see _SEMANTIC_LIMIT). -_PROPOSER_VERSION = 8 +_PROPOSER_VERSION = 9 # Signature resemblance floor, name blanked (difflib ratio) — and a length # floor, because `def NAME():` resembles `def NAME(x):` at 0.95 while saying # nothing; a family shape has parameters to resemble. @@ -1925,18 +1926,42 @@ def text_contains(body: str, code: str) -> bool: return a in b or b in a +def _in_canon_home(canon: Canon, path: str) -> bool: + """Is ``path`` in the canon's own package? A location that names a file + makes its directory home, exactly (a Go package is one directory); a + location that names a directory is home with everything under it.""" + from grimoire.services.snippets import _path_touches + + here = posixpath.dirname((path or "").strip("/")) + for lp, _ls in canon.locations: + lp = (lp or "").strip("/") + if not lp: + continue + if "." in lp.rsplit("/", 1)[-1]: + if posixpath.dirname(lp) == here: + return True + elif _path_touches(lp, path): + return True + return False + + def names_canon( kind: str, path: str, body: str, canon: Canon, *, - project_id: int = 0, local_names: frozenset[str] = frozenset(), + project_id: int = 0, defined_at: Mapping[str, frozenset[str]] | None = None, ) -> bool: """Does this body name THIS canon, rather than merely contain its word? The by-name test behind the reference basis and every uses edge the - proposer writes. A mention counts only when it can resolve to the canon: + proposer writes. ``defined_at`` maps each name to the repo paths that + define it. A mention counts only when it can resolve to the canon: - never across projects: one repo does not call a function defined in another's, however alike the names are; - never a name the file defines itself (a module's own `_run` helper), unless the canon is recorded in this very file; + - when the repo also defines the name somewhere the canon is not + recorded (`sftpRun.file` beside the canon `importRun.file`), only from + the canon's own package: elsewhere the name says nothing about which + definition a call reaches; - never a generic verb, with or without a leading underscore; - a bare lowercase word (`file`, `seen`, `seal`) only where it is called as `word(`, because as a plain word it is everyone's variable. No space @@ -1950,10 +1975,17 @@ def names_canon( return False if sym.lstrip("_").lower() in _REFERENCE_STOPLIST: return False - if sym in local_names: + where = (defined_at or {}).get(sym, frozenset()) + if where: from grimoire.services.snippets import _path_touches - if not any(lp and _path_touches(lp, path) for lp, _ls in canon.locations): + def at_canon(p: str) -> bool: + return any(lp and _path_touches(lp, p) for lp, _ls in canon.locations) + + if path in where and not at_canon(path): + return False + if canon.locations and not all(at_canon(p) for p in where) \ + and not _in_canon_home(canon, path): return False if kind == "sym" and re.fullmatch(r"[a-z]+", sym): return re.search(rf"(? list[int]: """Every canon this body NAMES — the uses edges the proposer can write mechanically: same kind, same language family, not the shape's own name, and named in the sense `names_canon` gives.""" norm_sym = _norm_symbol(symbol) - local = frozenset(local_names) out: list[int] = [] for c in canons: if c.kind != kind or not c.symbol: @@ -1977,7 +2008,7 @@ def reference_canons( continue if _norm_symbol(c.symbol) == norm_sym: continue - if names_canon(kind, path, body, c, project_id=project_id, local_names=local): + if names_canon(kind, path, body, c, project_id=project_id, defined_at=defined_at): out.append(c.snippet_id) return out @@ -1985,7 +2016,7 @@ def reference_canons( def match_canon( kind: str, path: str, symbol: str, signature: str, body: str, canons: Iterable[Canon], *, project_id: int = 0, - local_names: frozenset[str] = frozenset(), + defined_at: Mapping[str, frozenset[str]] | None = None, ) -> tuple[int, str, float] | None: """The strongest (snippet_id, basis, score) a shape earns against the canon catalog, by the non-semantic bases — or None. Strongest by @@ -2010,7 +2041,7 @@ def match_canon( if not any(location_covers(lp, ls, path, symbol) for lp, ls in c.locations): offer("symbol", 1.0, c) continue # its own location is canonical territory, not a proposal - if names_canon(kind, path, body, c, project_id=project_id, local_names=local_names): + if names_canon(kind, path, body, c, project_id=project_id, defined_at=defined_at): offer("reference", 0.9, c) if c.code_norm and text_contains(body, c.code_norm): offer("text", 0.95, c) @@ -2146,12 +2177,14 @@ async def propose_for_repo( if canons is None: canons = await canon_catalog(user_id) by_key = {(d[0], d[1], d[2]): d for d in definitions} - # What each file defines itself: a name it defines resolves there. - names_by_path: dict[str, set[str]] = {} + # Where the repo defines each name: a mention resolves to the definition + # in its own file, and a name defined in several packages only from the + # canon's own (see names_canon). + paths_by_name: dict[str, set[str]] = {} for d in definitions: if d[1] == "sym": - names_by_path.setdefault(d[0], set()).add(_norm_symbol(d[2])) - defined_in = {p: frozenset(n) for p, n in names_by_path.items()} + paths_by_name.setdefault(_norm_symbol(d[2]), set()).add(d[0]) + defined_at = {n: frozenset(p) for n, p in paths_by_name.items()} # The semantic arm is the widest net and, across projects, is noise: it # is held to the shape's own project and language family. The precise # bases (symbol/text) still reach family canon in other projects. @@ -2190,14 +2223,13 @@ async def propose_for_repo( examined_as = f"{body_sha}@{_PROPOSER_VERSION}" if row.proposed_at is not None and row.proposed_sha == examined_as: continue - local = defined_in.get(row.path, frozenset()) if row.status not in _MECHANICAL_TODO: # A judged row gets no proposal — but its uses edges # are a fact about the body, judged or not: the consumer map # of a canon must include the call sites someone already # classified. Mark it examined so the scan runs once per body. used = reference_canons(row.kind, row.path, row.symbol, body, canons, - project_id=project_id, local_names=local) + project_id=project_id, defined_at=defined_at) await replace_reference_uses(session, row, used, evidence="proposer: body names the canon's symbol") row.proposed_at = now @@ -2207,7 +2239,7 @@ async def propose_for_repo( group = row.proposal_group # derive grouping is reassigned below hit = match_canon( row.kind, row.path, row.symbol, signature, body, canons, - project_id=project_id, local_names=local, + project_id=project_id, defined_at=defined_at, ) _clear_proposal(row) row.proposal_group = group @@ -2216,7 +2248,7 @@ async def propose_for_repo( # Consumption is recorded for every canon the body names, # whatever the row is then judged to be. used = reference_canons(row.kind, row.path, row.symbol, body, canons, - project_id=project_id, local_names=local) + project_id=project_id, defined_at=defined_at) await replace_reference_uses(session, row, used, evidence="proposer: body names the canon's symbol") if hit: diff --git a/tests/test_shape_ledger.py b/tests/test_shape_ledger.py index aa8f5b94..5f072975 100644 --- a/tests/test_shape_ledger.py +++ b/tests/test_shape_ledger.py @@ -590,12 +590,35 @@ def test_a_name_the_file_defines_itself_resolves_there(): canon = Canon(7, "sym", "run_helper", (("src/tools.py", "run_helper"),), "def run_helper(x):", _norm_text("x"), 2, "python") body = "def test_a():\n run_helper(1)\n" - local = frozenset({"run_helper", "test_a"}) - assert reference_canons("sym", "tests/test_a.py", "test_a", body, [canon], local_names=local) == [] - assert reference_canons("sym", "src/tools.py", "caller", body, [canon], local_names=local) == [7] + own = {"run_helper": frozenset({"tests/test_a.py", "src/tools.py"})} + assert reference_canons("sym", "tests/test_a.py", "test_a", body, [canon], defined_at=own) == [] + assert reference_canons("sym", "src/tools.py", "caller", body, [canon], defined_at=own) == [7] assert reference_canons("sym", "tests/test_b.py", "test_a", body, [canon]) == [7] +def test_a_name_defined_in_two_packages_counts_only_in_the_canons_own(): + """`m.file(` in the replication package is its own `sftpRun.file`, not the + files package's canon `importRun.file`; a name defined once is unaffected.""" + from grimoire.services.shape_ledger import Canon, _norm_text, reference_canons + canon = Canon(5, "sym", "file", (("internal/files/nextcloud.go", "file"),), + "func (r *importRun) file(", _norm_text("x"), 0, "go") + both = {"file": frozenset({"internal/files/nextcloud.go", "internal/replication/mirror.go"})} + once = {"file": frozenset({"internal/files/nextcloud.go"})} + other = "err := m.file(local, rpath, nil)" + assert reference_canons("sym", "internal/replication/mirror_test.go", "TestX", other, [canon], + defined_at=both) == [] + assert reference_canons("sym", "internal/replication/mirror_test.go", "TestX", other, [canon], + defined_at=once) == [5] + call = "return nil, r.file(ctx, src)" + assert reference_canons("sym", "internal/files/nextcloud.go", "entry", call, [canon], + defined_at=both) == [5] + assert reference_canons("sym", "internal/files/import.go", "run", call, [canon], + defined_at=both) == [5] + # A subdirectory is another Go package. + assert reference_canons("sym", "internal/files/sub/x.go", "run", call, [canon], + defined_at=both) == [] + + def test_a_plain_word_counts_only_where_it_is_called(): """`file`, `seen`: everyone's variable. `seal(...)` is still a call.""" from grimoire.services.shape_ledger import Canon, _norm_text, reference_canons -- 2.54.0