From 2320f2adc1cdb629c2706ee9f908d11df427382c Mon Sep 17 00:00:00 2001 From: safishamsi Date: Thu, 6 Aug 2026 17:29:18 +0100 Subject: [PATCH] fix(update,java): prune newly-ignored files; stop external-annotation/local-class conflation (#2495, #2504) #2495 (thanks @alisson-acioli): graphify update now evicts a file newly added to .graphifyignore/--exclude even though it still exists on disk (positive ignore-rule evidence), with .gitignore-driven eviction gated on an explicit full update; unchanged in-corpus files and fail-closed #1795 cases are preserved. #2504 (thanks @te7ina-honey): the Java type resolver runs before the unique-label stub rewire and parks imported-but-external types on their FQN, and cross-file import resolution checks the package, so a local class and a same-named external annotation (e.g. Spring @Component) no longer collapse into one node. In-corpus annotation resolution unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- graphify/detect.py | 64 ++++++++ graphify/extract.py | 22 ++- graphify/extractors/engine.py | 29 +++- graphify/extractors/resolution.py | 133 ++++++++++++++-- graphify/watch.py | 113 ++++++++++--- tests/test_java_type_resolution.py | 168 ++++++++++++++++++++ tests/test_watch.py | 245 ++++++++++++++++++++++++++++- 7 files changed, 716 insertions(+), 58 deletions(-) diff --git a/graphify/detect.py b/graphify/detect.py index f8e1854..b0ea762 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -9,6 +9,7 @@ from concurrent.futures import ThreadPoolExecutor from enum import Enum from functools import lru_cache from pathlib import Path +from typing import Callable from graphify.google_workspace import ( GOOGLE_WORKSPACE_EXTENSIONS, @@ -1196,6 +1197,69 @@ def _is_ignored( return _eval(path) +def ignored_predicate( + root: Path, + *, + extra_excludes: list[str] | None = None, + gitignore: bool = True, +) -> Callable[[Path], bool]: + """Build a per-path predicate answering "would detect() exclude this path?". + + Mirrors detect()'s ignore decisions for a single existing path WITHOUT + re-walking the corpus, from the same machinery detect() uses: the ancestor + .graphifyignore/.gitignore chain (_load_graphifyignore), CLI/persisted + ``--exclude`` patterns appended last at the root anchor (#947), nested + per-directory ignore files along the path's own lineage (#1206), the + _is_noise_dir directory pruning, and _SKIP_FILES. The sensitive-file + heuristic (_is_sensitive) is deliberately NOT included: callers use this + predicate as positive evidence of a live ignore RULE (#2495), and a + heuristic match is not user intent. + + Nested patterns are loaded lazily, once per directory, into one shared + pattern list. That accumulation cannot cross-contaminate results — a + pattern only ever matches paths under its anchor directory, so patterns + from a sibling subtree are inert — which is the same invariant detect()'s + live os.walk relies on, and it keeps the shared _is_ignored cache valid. + """ + root = root.resolve() + patterns = _load_graphifyignore(root, gitignore=gitignore) + if extra_excludes: + for pat in extra_excludes: + line = _parse_gitignore_line(pat) + if line: + patterns.append((root, line)) + cache: dict[Path, bool] = {} + # root's own ignore file is the last entry of _load_graphifyignore's chain. + loaded_dirs: set[Path] = {root} + + def _ignored(path: Path) -> bool: + path = Path(os.path.abspath(path)) + try: + rel_parts = path.relative_to(root).parts + except ValueError: + return False # outside the scan root: detect() never considered it + if path.name in _SKIP_FILES: + return True + # Noise-dir pruning: os.walk never descends these, so anything beneath + # one is excluded from the corpus regardless of ignore patterns. + parent = root + for part in rel_parts[:-1]: + if _is_noise_dir(part, parent): + return True + parent = parent / part + # Load ignore files along this path's own lineage — detect()'s walk + # would have loaded exactly these before reaching the file (#1206). + ancestor = root + for part in rel_parts[:-1]: + ancestor = ancestor / part + if ancestor not in loaded_dirs: + loaded_dirs.add(ancestor) + patterns.extend(_load_dir_own_ignore(ancestor, gitignore=gitignore)) + return _is_ignored(path, root, patterns, _cache=cache) + + return _ignored + + def _auto_follow_symlinks(root: Path) -> bool: """Return whether ``root`` has any direct symlinked child. diff --git a/graphify/extract.py b/graphify/extract.py index dc7540d..8b1d3da 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -5327,6 +5327,21 @@ def extract( except Exception as exc: import logging logging.getLogger(__name__).warning("PHP type-reference resolution failed, skipping: %s", exc) + # Java package/import disambiguation must likewise run BEFORE the rewire + # (#2504): an EXTERNAL import (`org.springframework.stereotype.Component`) + # leaves a bare `Component` stub that the rewire would collapse onto the only + # internal class with that simple name, manufacturing a false hub. Parking + # such references on an FQN-labeled stub first prevents the merge, and + # import-exact resolution of internal references (#1318/#1744) still applies. + _java_sel = [(r, p) for r, p in zip(per_file, paths) if p.suffix == ".java"] + if _java_sel: + try: + _resolve_java_type_references( + [r for r, _ in _java_sel], [p for _, p in _java_sel], all_nodes, all_edges + ) + except Exception as exc: + import logging + logging.getLogger(__name__).warning("Java type-reference resolution failed, skipping: %s", exc) _rewire_unique_stub_nodes(all_nodes, all_edges) # Add cross-file class-level edges (Python only - uses Python parser internally) @@ -5349,13 +5364,6 @@ def extract( except Exception as exc: import logging logging.getLogger(__name__).warning("Java cross-file import resolution failed, skipping: %s", exc) - # Re-point dangling implements/inherits edges that bare-name resolution - # left on shadow stubs, using imports for exact-package disambiguation (#1318). - try: - _resolve_java_type_references(java_results, java_paths, all_nodes, all_edges) - except Exception as exc: - import logging - logging.getLogger(__name__).warning("Java type-reference resolution failed, skipping: %s", exc) # Cross-file C# type-reference resolution: re-point dangling inherits/implements/ # references edges left on shadow stubs, disambiguating same-named types by the diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index c1466ef..e19f8fe 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -548,9 +548,11 @@ def _java_method_receiver_types( return table -def _java_annotation_names(declaration_node, source: bytes) -> list[str]: - """Collect annotation names from a Java declaration's `modifiers` child.""" - names: list[str] = [] +def _java_annotation_names(declaration_node, source: bytes) -> list[tuple[str, str]]: + """Collect ``(simple, raw)`` annotation names from a Java declaration's + `modifiers` child. ``raw`` keeps the dotted qualifier of an inline-qualified + annotation (``@org.pkg.Foo``); it equals ``simple`` when unqualified.""" + names: list[tuple[str, str]] = [] modifiers = None for child in declaration_node.children: if child.type == "modifiers": @@ -568,9 +570,10 @@ def _java_annotation_names(declaration_node, source: bytes) -> list[str]: name_node = sub break if name_node is not None: - text = _read_text(name_node, source).rsplit(".", 1)[-1] + raw = _read_text(name_node, source) + text = raw.rsplit(".", 1)[-1] if text: - names.append(text) + names.append((text, raw)) return names def _php_name_text(node, source: bytes) -> str | None: @@ -2946,7 +2949,14 @@ def _extract_generic( if tid.is_named: _emit_java_parent_type(tid, "inherits", line) - for anno_name in _java_annotation_names(node, source): + for anno_name, anno_raw in _java_annotation_names(node, source): + # An inline-qualified annotation (`@org.pkg.Foo`) keeps its + # full dotted name so a bare same-named local class can't + # absorb it; _resolve_java_type_references maps internal + # FQNs back to their real nodes (#2504). Groovy has no such + # resolver pass, so it keeps the legacy bare-name stub. + if "." in anno_raw and config.ts_module == "tree_sitter_java": + anno_name = anno_raw target_nid = ensure_named_node(anno_name, line) if target_nid != class_nid: add_edge(class_nid, target_nid, "references", line, @@ -3568,8 +3578,11 @@ def _extract_generic( target_nid = ensure_named_node(ref_name, line) if target_nid != func_nid: add_edge(func_nid, target_nid, "references", line, context=ctx) - for anno_name in _java_annotation_names(node, source): - target_nid = ensure_named_node(anno_name, line) + for anno_name, anno_raw in _java_annotation_names(node, source): + # Inline-qualified: keep the dotted name (#2504); see the + # class-level annotation handling above. + target_nid = ensure_named_node( + anno_raw if "." in anno_raw else anno_name, line) if target_nid != func_nid: add_edge(func_nid, target_nid, "references", line, context="attribute") diff --git a/graphify/extractors/resolution.py b/graphify/extractors/resolution.py index 097c32b..b5edd26 100644 --- a/graphify/extractors/resolution.py +++ b/graphify/extractors/resolution.py @@ -2174,9 +2174,14 @@ def _resolve_cross_file_java_imports( ) -> list[dict]: """Two-pass Java import resolution. - Pass 1: build a global index {ClassName: [node_id, ...]} across all Java nodes. + Pass 1: build a global index {ClassName: [(node_id, package), ...]} across + all Java nodes (packages come from a re-parse; node metadata doesn't carry + them). Pass 2: re-parse each Java file; for every `import a.b.C;`, resolve C against - the index. Wildcard and stdlib imports produce no edge. + the index, skipping candidates whose defining file declares a different + package — an external `org.springframework.stereotype.Component` must not + link to a local `com.example.model.Component` (#2504). Wildcard and stdlib + imports produce no edge. """ try: import tree_sitter_java as tsjava @@ -2187,8 +2192,45 @@ def _resolve_cross_file_java_imports( language = Language(tsjava.language()) parser = Parser(language) - # Pass 1: class-name → node_id index (only internal, uppercase-starting names) - name_to_ids: dict[str, list[str]] = {} + # Pre-pass: declared package per source_file string (and parsed trees for + # pass 2, so each file is only parsed once). + parsed: dict[str, tuple[bytes, object]] = {} + pkg_by_src: dict[str, str] = {} + for path, file_result in zip(paths, per_file): + try: + source = path.read_bytes() + tree = parser.parse(source) + except Exception: + continue + parsed[str(path)] = (source, tree) + pkg = "" + for child in tree.root_node.children: + if child.type == "package_declaration": + pkg = _read_text(child, source).strip()[len("package"):].strip().rstrip(";").strip() + break + pkg_by_src[str(path)] = pkg + for node in file_result.get("nodes", []): + src = node.get("source_file") + if src: + pkg_by_src.setdefault(src, pkg) + + def _pkg_matches(imp_pkg: str, tgt_pkg: str) -> bool: + if imp_pkg == tgt_pkg: + return True + # `import p.Outer.Inner` against a nested type defined in package p: + # the leftover segments must all be type-like (uppercase-first), which + # conventional lowercase external packages can never satisfy. + if tgt_pkg: + if not imp_pkg.startswith(tgt_pkg + "."): + return False + rest = imp_pkg[len(tgt_pkg) + 1:] + else: + rest = imp_pkg + return bool(rest) and all(seg[:1].isupper() for seg in rest.split(".")) + + # Pass 1: class-name → (node_id, package) index (only internal, + # uppercase-starting names) + name_to_ids: dict[str, list[tuple[str, str]]] = {} for file_result in per_file: for node in file_result.get("nodes", []): label = node.get("label", "") @@ -2200,18 +2242,17 @@ def _resolve_cross_file_java_imports( continue if not label[0].isalpha() or not label[0].isupper(): continue - name_to_ids.setdefault(label, []).append(nid) + name_to_ids.setdefault(label, []).append((nid, pkg_by_src.get(src, ""))) # Pass 2: resolve imports to real node IDs new_edges: list[dict] = [] seen_pairs: set[tuple[str, str]] = set() for path in paths: file_nid = _make_id(str(path)) - try: - source = path.read_bytes() - tree = parser.parse(source) - except Exception: + entry = parsed.get(str(path)) + if entry is None: continue + source, tree = entry def walk(n) -> None: if n.type == "import_declaration": @@ -2225,12 +2266,16 @@ def _resolve_cross_file_java_imports( if not parts: return last = parts[-1] + imp_pkg = ".".join(parts[:-1]) if last and last[0].islower() and len(parts) >= 2: last = parts[-2] + imp_pkg = ".".join(parts[:-2]) at_line = n.start_point[0] + 1 - for tgt_nid in name_to_ids.get(last, []): + for tgt_nid, tgt_pkg in name_to_ids.get(last, []): if tgt_nid == file_nid: continue + if not _pkg_matches(imp_pkg, tgt_pkg): + continue key = (file_nid, tgt_nid) if key in seen_pairs: continue @@ -2270,8 +2315,14 @@ def _resolve_java_type_references( names the exact package, so it disambiguates where bare-name matching cannot. Mutates ``all_nodes``/``all_edges`` in place. Runs after id-disambiguation so - target ids are final, and after ``_rewire_unique_stub_nodes`` so it only has - to handle the ambiguous remainder. + target ids are final, but BEFORE ``_rewire_unique_stub_nodes`` (#2504): the + rewire itself manufactures a false merge when the bare stub for an EXTERNAL + import (``org.springframework.stereotype.Component``) collapses onto the + only internal class with that simple name. References proven external by an + explicit import are re-pointed to an FQN-labeled sourceless stub the + bare-label rewire cannot collapse; references with no import/package facts + are left untouched so the legacy unique-label rewire keeps handling plain + same-package/default-package corpora (mirrors the PHP #1923 fix). """ try: import tree_sitter_java as tsjava @@ -2332,11 +2383,15 @@ def _resolve_java_type_references( pkg = pkg_by_file[src] fqn_to_id.setdefault(f"{pkg}.{label}" if pkg else label, nid) - # Bare shadow stubs: no source_file, type-like label. + # Shadow stubs: no source_file, type-like label. Dotted labels are included + # for qualified inline annotations (`@com.example.anno.Loggable`), which the + # engine mints with their full dotted name so a same-named local class can't + # absorb them (#2504). stub_label: dict[str, str] = { node["id"]: node.get("label", "") for node in all_nodes - if node.get("id") and not node.get("source_file") and node.get("label", "")[:1].isupper() + if node.get("id") and not node.get("source_file") + and (node.get("label", "")[:1].isupper() or "." in node.get("label", "")) } if not stub_label: return @@ -2351,6 +2406,28 @@ def _resolve_java_type_references( # the reference must point at the RIGHT one (#1744). Mirrors the C# resolver, # whose REPOINT set already covers `references`. REPOINT_RELATIONS = {"implements", "inherits", "extends", "imports", "references"} + + node_ids = {n.get("id") for n in all_nodes if n.get("id")} + external_stub_ids: dict[str, str] = {} + new_nodes: list[dict] = [] + + def _external_stub(fqn: str) -> str: + nid = external_stub_ids.get(fqn) + if nid: + return nid + nid = _make_id(fqn) + if nid not in node_ids: + new_nodes.append({ + "id": nid, + "label": fqn, + "file_type": "code", + "source_file": "", + "source_location": "", + }) + node_ids.add(nid) + external_stub_ids[fqn] = nid + return nid + repointed_from: set[str] = set() for edge in all_edges: if edge.get("relation") not in REPOINT_RELATIONS: @@ -2360,17 +2437,41 @@ def _resolve_java_type_references( if not label: continue ref_file = edge.get("source_file", "") - resolved = None + if "." in label: + # FQN-labeled stub (qualified inline annotation): resolve it against + # the internal definitions; an external FQN stays parked as-is. + resolved = fqn_to_id.get(label) + if resolved and resolved != tgt: + edge["target"] = resolved + repointed_from.add(tgt) + continue fqn = imports_by_file.get(ref_file, {}).get(label) if fqn: resolved = fqn_to_id.get(fqn) - if resolved is None: # same-package reference (no explicit import) + if resolved is None: + # `import p.Outer.Inner`: strip trailing type-like segments to + # find the defining package of an internal nested type. + head = fqn.split(".")[:-1] + while resolved is None and head and head[-1][:1].isupper(): + head.pop() + resolved = fqn_to_id.get(".".join(head + [label])) + if resolved is None: + # Explicit import with no internal definition: proven EXTERNAL. + # Park the edge on an FQN-labeled stub the bare-name rewire + # cannot collapse onto a same-named local class (#2504 — this + # is the Java counterpart of the PHP #1923 fix). + edge["target"] = _external_stub(fqn) + repointed_from.add(tgt) + continue + else: # same-package reference (no explicit import) pkg = pkg_by_file.get(ref_file, "") resolved = fqn_to_id.get(f"{pkg}.{label}" if pkg else label) if resolved and resolved != tgt: edge["target"] = resolved repointed_from.add(tgt) + if new_nodes: + all_nodes.extend(new_nodes) if not repointed_from: return diff --git a/graphify/watch.py b/graphify/watch.py index 862997a..933bdf9 100644 --- a/graphify/watch.py +++ b/graphify/watch.py @@ -8,6 +8,7 @@ import re import sys import time from pathlib import Path +from typing import Callable # Single source of truth in graphify.paths (#1423); re-exported as _GRAPHIFY_OUT. from graphify.paths import GRAPHIFY_OUT as _GRAPHIFY_OUT @@ -467,8 +468,20 @@ def _reconcile_existing_graph( full_rebuild: bool, deleted_paths: set[str], deleted_source_identities: set[str], + is_ignored_always: Callable[[Path], bool] | None = None, + is_ignored_full: Callable[[Path], bool] | None = None, ) -> tuple[dict, dict]: - """Merge fresh extraction with preserved graph entries and evict stale sources.""" + """Merge fresh extraction with preserved graph entries and evict stale sources. + + ``is_ignored_always``/``is_ignored_full`` (#2495) report whether a LIVE + ignore rule matches a path, mirroring the detect() call that produced + ``code_files`` (detect.ignored_predicate). ``is_ignored_always`` covers + .graphifyignore + persisted ``--exclude`` patterns — unambiguous + graph-level intent, honored on every rebuild. ``is_ignored_full`` adds + .gitignore-driven rules, honored only on a full rebuild (an explicit + ``graphify update``) so the incremental/hook path keeps preserving a + deliberately-graphed .gitignore'd tree (#1795). + """ existing_graph_data: dict = {} if not existing_graph.exists(): return result, existing_graph_data @@ -533,14 +546,38 @@ def _reconcile_existing_graph( # # Fail-closed eviction: a source identity missing from the corpus is only # DELETION evidence when the file is actually gone from disk. A file that - # still exists but stopped being collected was *excluded* (ignore rules or - # filters changed — e.g. a .gitignore the scanner newly honors), and - # treating that as deletion silently mass-evicts good nodes. Preserve - # instead and say so; a full re-extraction still purges deliberately - # excluded sources via the AST ownership rule below. + # still exists but stopped being collected was *excluded*, and treating + # that as deletion silently mass-evicts good nodes. Evicting an alive + # source therefore needs POSITIVE evidence — a live ignore rule matching + # the path (#2495) — not mere corpus absence: a filter regression, an + # extractor loss, or the sensitive-file heuristic still preserves, with + # a loud message (#1795). excluded_alive_files: set[str] = set() excluded_alive_nodes = 0 + newly_ignored_files: set[str] = set() + newly_ignored_nodes = 0 _alive_cache: dict[str, bool] = {} + _ignored_cache: dict[str, bool] = {} + + def _ignored_now(identity: str) -> bool: + """True when a live ignore rule matches this alive, corpus-absent source. + + .graphifyignore/--exclude matches evict on every rebuild; + .gitignore-driven matches only on a full rebuild (see docstring). + """ + ignored = _ignored_cache.get(identity) + if ignored is None: + target = Path(identity) + ignored = bool( + (is_ignored_always is not None and is_ignored_always(target)) + or ( + full_rebuild + and is_ignored_full is not None + and is_ignored_full(target) + ) + ) + _ignored_cache[identity] = ignored + return ignored for node in existing.get("nodes", []): source_file = node.get("source_file") if not source_file or _is_remote_source(source_file): @@ -551,17 +588,23 @@ def _reconcile_existing_graph( if _get_extractor(Path(source_file)) is None: # Non-AST source (semantic doc/paper/image — .txt/.pdf/.png/...): # never present in current_sources (built from AST-extractable - # code_files), so corpus absence is meaningless. Disk absence is - # the ONLY deletion evidence here — otherwise its semantic nodes + # code_files), so corpus absence is meaningless. Deletion + # evidence here is disk absence — otherwise its semantic nodes # are preserved forever and returned as authoritative even after - # the file is deleted (#2051). A present-but-unextractable file - # stays preserved (alive -> skip). + # the file is deleted (#2051) — or a live ignore rule matching + # the alive file (#2495), the same positive evidence the AST + # branch below requires. A present-but-unexcluded-but- + # unextractable file stays preserved (alive -> skip). if identity: alive = _alive_cache.get(identity) if alive is None: alive = Path(identity).exists() _alive_cache[identity] = alive - if not alive: + ignored = alive and _ignored_now(identity) + if ignored: + newly_ignored_files.add(identity) + newly_ignored_nodes += 1 + if not alive or ignored: normalized = source_paths.normalize(source_file) if normalized: deleted_paths.add(normalized) @@ -576,9 +619,16 @@ def _reconcile_existing_graph( alive = Path(identity).exists() _alive_cache[identity] = alive if alive: - excluded_alive_files.add(identity) - excluded_alive_nodes += 1 - continue + if _ignored_now(identity): + # Intentionally excluded by a live ignore rule: + # deliberate graph-level intent, so treat it exactly + # like a deletion (#2495) — fall through to evict. + newly_ignored_files.add(identity) + newly_ignored_nodes += 1 + else: + excluded_alive_files.add(identity) + excluded_alive_nodes += 1 + continue normalized = source_paths.normalize(source_file) if normalized: deleted_paths.add(normalized) @@ -586,12 +636,18 @@ def _reconcile_existing_graph( node_evicted_source_identities.add(identity) edge_evicted_source_identities.add(identity) hyperedge_evicted_source_identities.add(identity) + if newly_ignored_files: + print( + f"[graphify watch] pruned {newly_ignored_nodes} node(s) from " + f"{len(newly_ignored_files)} newly-ignored file(s) " + "(matched by a live ignore rule while absent from the scan corpus)." + ) if excluded_alive_files: print( f"[graphify watch] fail-closed: kept {excluded_alive_nodes} node(s) " f"from {len(excluded_alive_files)} file(s) that left the scan corpus " - "but still exist on disk (ignore rules or filters changed?). " - "Run a full re-extraction to purge them if the exclusion is intentional." + "but still exist on disk and match no current ignore rule (filters " + "changed?). Add them to .graphifyignore if the exclusion is intentional." ) # A full re-extraction owns the AST nodes of every source it actually @@ -999,13 +1055,30 @@ def _rebuild_code( # hook rebuild does not silently re-include deliberately excluded paths # (#1886). _persisted_excludes = _read_build_excludes(out) + _gitignore_enabled = _read_build_gitignore(out) detected = detect( watch_path, follow_symlinks=follow_symlinks, extra_excludes=_persisted_excludes or None, - gitignore=_read_build_gitignore(out), + gitignore=_gitignore_enabled, ) code_files = [Path(f) for f in detected['files']['code']] + # #2495: hand reconcile the same ignore decisions the detect() call + # above made, so a newly-ignored file that still exists on disk is + # purged from the graph instead of preserved forever by the fail-closed + # keep. Two tiers: .graphifyignore + persisted --exclude patterns are + # unambiguous graph-level intent (evict on every rebuild), while + # .gitignore-driven exclusion is honored only on a full rebuild — the + # incremental/hook path keeps preserving a deliberately-graphed + # .gitignore'd tree (#1795). + from graphify.detect import ignored_predicate + _ignored_always = ignored_predicate( + watch_root, extra_excludes=_persisted_excludes or None, gitignore=False, + ) + _ignored_full = ignored_predicate( + watch_root, extra_excludes=_persisted_excludes or None, gitignore=True, + ) if _gitignore_enabled else _ignored_always + # Include document files that have AST extractors (e.g. .md, .mdx, .qmd) ast_doc_files: list[Path] = [] for doc_file in detected['files'].get('document', []): @@ -1132,8 +1205,8 @@ def _rebuild_code( # disables the shrink guard that would otherwise catch the # loss. Preserve it — a genuine deletion still evicts via the # branch below, the corpus sweep evicts a truly-gone non-AST - # source, and a deliberate exclusion is purged by a full - # re-extraction. + # source, and a deliberate exclusion is purged by the + # corpus sweep's ignore-rule check (#2495). continue deleted_in_root = next( @@ -1272,6 +1345,8 @@ def _rebuild_code( full_rebuild=changed_paths is None, deleted_paths=deleted_paths, deleted_source_identities=deleted_source_identities, + is_ignored_always=_ignored_always, + is_ignored_full=_ignored_full, ) except (RuntimeError, ValueError) as exc: # Existing graph present but unreadable — over the size cap diff --git a/tests/test_java_type_resolution.py b/tests/test_java_type_resolution.py index 7729749..fcc72cb 100644 --- a/tests/test_java_type_resolution.py +++ b/tests/test_java_type_resolution.py @@ -253,6 +253,174 @@ def test_java_user_types_still_emit_references(tmp_path: Path): assert "OrderDto" in ref_targets, "user type OrderDto must still emit references" +def test_java_external_annotation_does_not_conflate_with_local_class(tmp_path: Path): + # #2504: Spring's @Component is EXTERNAL. A local `class Component` with the + # same simple name must not absorb the services' annotation/import edges — + # the bare-name stub used to collapse onto it and manufacture a false hub. + local = _write( + tmp_path / "src/com/example/model/Component.java", + "package com.example.model;\npublic class Component {}\n", + ) + user = _write( + tmp_path / "src/com/example/service/UserService.java", + "package com.example.service;\n" + "import org.springframework.stereotype.Component;\n" + "@Component\npublic class UserService {}\n", + ) + order = _write( + tmp_path / "src/com/example/service/OrderService.java", + "package com.example.service;\n" + "import org.springframework.stereotype.Component;\n" + "@Component\npublic class OrderService {}\n", + ) + inventory = _write( + tmp_path / "src/com/example/app/Inventory.java", + "package com.example.app;\n" + "import com.example.model.Component;\n" + "public class Inventory {\n" + " private Component part;\n" + "}\n", + ) + result = extract([local, user, order, inventory], cache_root=tmp_path) + by_id = {n["id"]: n for n in result["nodes"]} + + local_id = next( + n["id"] for n in result["nodes"] + if n.get("label") == "Component" and "model" in n.get("source_file", "") + ) + + # No references/imports edge from the service files may land on the local class. + conflated = [ + e for e in result["edges"] + if e.get("target") == local_id and "service" in e.get("source_file", "") + ] + assert not conflated, f"external @Component conflated with local class: {conflated}" + + # The @Component attribute edges park on an external FQN-labeled stub. + attr = [ + e for e in result["edges"] + if e.get("relation") == "references" and e.get("context") == "attribute" + and "service" in e.get("source_file", "") + ] + assert len(attr) == 2, "expected one @Component attribute edge per service" + for e in attr: + tgt = by_id.get(e["target"]) or {} + assert tgt.get("label") == "org.springframework.stereotype.Component" + assert not tgt.get("source_file") + + # The genuine internal use still resolves to the local class. + inv_edges = [ + e for e in result["edges"] + if e.get("source_file", "").endswith("Inventory.java") + and e.get("target") == local_id + ] + assert inv_edges, "Inventory's use of the local Component must still resolve" + + +def test_java_in_corpus_annotation_still_resolves(tmp_path: Path): + # #2504 guard: a locally-defined @interface used via an explicit import must + # still resolve to the real source-backed node, not get parked externally. + anno = _write( + tmp_path / "src/com/example/anno/Loggable.java", + "package com.example.anno;\npublic @interface Loggable {}\n", + ) + svc = _write( + tmp_path / "src/com/example/service/AuditService.java", + "package com.example.service;\n" + "import com.example.anno.Loggable;\n" + "@Loggable\npublic class AuditService {}\n", + ) + result = extract([anno, svc], cache_root=tmp_path) + by_id = {n["id"]: n for n in result["nodes"]} + attr = [ + e for e in result["edges"] + if e.get("relation") == "references" and e.get("context") == "attribute" + ] + assert attr, "expected a @Loggable attribute edge" + for e in attr: + tgt = by_id.get(e["target"]) or {} + assert tgt.get("label") == "Loggable" + assert tgt.get("source_file"), "annotation must land on the real @interface node" + assert "anno" in tgt["source_file"] + + +def test_java_same_package_annotation_resolves_without_import(tmp_path: Path): + # #2504 guard: same-package use (no import, no facts) keeps resolving via + # the legacy unique-label rewire. + anno = _write( + tmp_path / "src/com/example/anno/Loggable.java", + "package com.example.anno;\npublic @interface Loggable {}\n", + ) + svc = _write( + tmp_path / "src/com/example/anno/AuditService.java", + "package com.example.anno;\n" + "@Loggable\npublic class AuditService {}\n", + ) + result = extract([anno, svc], cache_root=tmp_path) + by_id = {n["id"]: n for n in result["nodes"]} + attr = [ + e for e in result["edges"] + if e.get("relation") == "references" and e.get("context") == "attribute" + ] + assert attr, "expected a @Loggable attribute edge" + for e in attr: + tgt = by_id.get(e["target"]) or {} + assert tgt.get("label") == "Loggable" + assert tgt.get("source_file"), "annotation must land on the real @interface node" + + +def test_java_qualified_inline_annotation_resolves_internally(tmp_path: Path): + # #2504 (qualified inline form): `@com.example.anno.Loggable` with no import + # must resolve to the real @interface node via its FQN. + anno = _write( + tmp_path / "src/com/example/anno/Loggable.java", + "package com.example.anno;\npublic @interface Loggable {}\n", + ) + svc = _write( + tmp_path / "src/com/example/service/AuditService.java", + "package com.example.service;\n" + "@com.example.anno.Loggable\npublic class AuditService {}\n", + ) + result = extract([anno, svc], cache_root=tmp_path) + by_id = {n["id"]: n for n in result["nodes"]} + attr = [ + e for e in result["edges"] + if e.get("relation") == "references" and e.get("context") == "attribute" + ] + assert attr, "expected a qualified @Loggable attribute edge" + for e in attr: + tgt = by_id.get(e["target"]) or {} + assert tgt.get("label") == "Loggable" + assert tgt.get("source_file"), "qualified annotation must land on the real node" + + +def test_java_qualified_inline_external_annotation_does_not_bind_local_class(tmp_path: Path): + # #2504 (qualified inline form): an external `@io.micrometer.core.annotation.Timed` + # must not bind to a same-named local class. + local = _write( + tmp_path / "src/com/example/model/Timed.java", + "package com.example.model;\npublic class Timed {}\n", + ) + svc = _write( + tmp_path / "src/com/example/service/MetricsService.java", + "package com.example.service;\n" + "public class MetricsService {\n" + " @io.micrometer.core.annotation.Timed\n" + " public void record() {}\n" + "}\n", + ) + result = extract([local, svc], cache_root=tmp_path) + local_id = next( + n["id"] for n in result["nodes"] + if n.get("label") == "Timed" and n.get("source_file") + ) + conflated = [ + e for e in result["edges"] + if e.get("target") == local_id and "service" in e.get("source_file", "") + ] + assert not conflated, f"external qualified annotation bound to local class: {conflated}" + + def test_java_cross_file_constructor_call_resolves(tmp_path: Path): # #1373: `new Foo(...)` in a method body must produce a cross-file edge to the # Foo definition. Foo is NOT used as a return type here, so the edge can only diff --git a/tests/test_watch.py b/tests/test_watch.py index 4854a67..d06140c 100644 --- a/tests/test_watch.py +++ b/tests/test_watch.py @@ -1748,11 +1748,13 @@ def test_merge_changed_paths_dedupes_in_order(): def test_rebuild_code_preserves_nodes_from_excluded_but_alive_file(tmp_path, capsys): - """Fail-closed eviction: a file that leaves the scan corpus (newly ignored) - but still exists on disk was EXCLUDED, not deleted — its nodes must survive - an incremental rebuild, with a loud message, instead of being silently - mass-evicted as stale sources (the docs/brainstorms incident: an upgrade - started honoring .gitignore and evicted 655 nodes whose files were present). + """Fail-closed eviction: a file that leaves the scan corpus but still exists + on disk was EXCLUDED, not deleted — on an INCREMENTAL rebuild its nodes must + survive a newly-honored .gitignore, with a loud message, instead of being + silently mass-evicted as stale sources (the docs/brainstorms incident: an + upgrade started honoring .gitignore and evicted 655 nodes whose files were + present). .gitignore-driven eviction is deliberately gated on a full rebuild + (#2495): only .graphifyignore/--exclude matches evict on the hook path. """ import json from graphify.watch import _rebuild_code @@ -1769,14 +1771,14 @@ def test_rebuild_code_preserves_nodes_from_excluded_but_alive_file(tmp_path, cap labels = {n["label"] for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]} assert "brainstorm.md" in labels - # The file becomes ignored (leaves the corpus) but stays on disk. - (corpus / ".graphifyignore").write_text("notes/\n", encoding="utf-8") + # The file becomes gitignored (leaves the corpus) but stays on disk. + (corpus / ".gitignore").write_text("notes/\n", encoding="utf-8") capsys.readouterr() assert _rebuild_code(corpus, changed_paths=[Path("auth.py")], acquire_lock=False) is True labels = {n["label"] for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]} assert "brainstorm.md" in labels, ( - "nodes from an excluded-but-alive file must be preserved, not evicted" + "nodes from a gitignored-but-alive file must be preserved on an incremental rebuild" ) assert "fail-closed: kept" in capsys.readouterr().out @@ -1803,6 +1805,233 @@ def test_rebuild_code_still_evicts_when_excluded_file_is_also_deleted(tmp_path): assert "login()" in labels +# ── #2495: `graphify update` purges newly-ignored files that still exist ─────── + + +def test_update_prunes_newly_graphifyignored_file(tmp_path, monkeypatch, capsys): + """#2495: a file added to .graphifyignore while still on disk must be purged + from the graph by a full `graphify update` — nodes, edges, and hyperedges — + instead of being preserved forever by the fail-closed keep. The legitimate + shrink passes without --force/GRAPHIFY_FORCE, unchanged in-corpus files stay + byte-identical, and the prune is reported distinctly from the fail-closed + keep line.""" + from graphify.watch import _rebuild_code + + monkeypatch.delenv("GRAPHIFY_FORCE", raising=False) + corpus = tmp_path / "corpus" + corpus.mkdir() + (corpus / "a.py").write_text("def alpha():\n return 1\n", encoding="utf-8") + (corpus / "b.py").write_text("def helper():\n return 2\n", encoding="utf-8") + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + graph_path = corpus / "graphify-out" / "graph.json" + data = json.loads(graph_path.read_text(encoding="utf-8")) + assert any(n.get("source_file") == "b.py" for n in data["nodes"]) + # Inject a hyperedge OWNED by b.py whose members all live in a.py, so only + # the source-identity eviction (not member loss) can remove it. + a_ids = [n["id"] for n in data["nodes"] if n.get("source_file") == "a.py"] + assert a_ids + data.setdefault("hyperedges", []).append( + {"id": "h_b", "nodes": a_ids, "source_file": "b.py"} + ) + graph_path.write_text(json.dumps(data), encoding="utf-8") + + def _a_items(payload): + return sorted( + ( + json.dumps(item, sort_keys=True) + for bucket in ("nodes", "links") + for item in payload.get(bucket, []) + if item.get("source_file") == "a.py" + ), + ) + + a_before = _a_items(data) + (corpus / ".graphifyignore").write_text("b.py\n", encoding="utf-8") + capsys.readouterr() + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + + after = json.loads(graph_path.read_text(encoding="utf-8")) + for bucket in ("nodes", "links", "hyperedges"): + assert not any( + item.get("source_file") == "b.py" for item in after.get(bucket, []) + ), f"{bucket} still reference newly-ignored b.py (#2495)" + assert "h_b" not in {h.get("id") for h in after.get("hyperedges", [])} + assert _a_items(after) == a_before, ( + "unchanged in-corpus file's items must survive the prune byte-identical" + ) + out = capsys.readouterr().out + assert "newly-ignored file(s)" in out + assert "fail-closed: kept" not in out + + +def test_update_still_prunes_deleted_file_with_ignore_rules_present(tmp_path): + """Regression guard for #2495: routing ignore-rule evidence through the + corpus sweep must not weaken plain deletion eviction while unrelated ignore + rules exist.""" + from graphify.watch import _rebuild_code + + corpus = tmp_path / "corpus" + corpus.mkdir() + (corpus / ".graphifyignore").write_text("unrelated/\n", encoding="utf-8") + (corpus / "a.py").write_text("def alpha():\n return 1\n", encoding="utf-8") + (corpus / "b.py").write_text("def helper():\n return 2\n", encoding="utf-8") + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + graph_path = corpus / "graphify-out" / "graph.json" + + (corpus / "b.py").unlink() + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + sources = {n.get("source_file") for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]} + assert "b.py" not in sources, "deleted file's nodes must still be evicted" + assert "a.py" in sources + + +def test_update_keeps_alive_file_dropped_from_corpus_without_ignore_rule( + tmp_path, monkeypatch, capsys +): + """#1795 retained under #2495: evicting an alive file now requires POSITIVE + evidence — a live ignore rule matching it. A file that leaves the corpus for + any other reason (filter regression, extractor loss) stays preserved, with + the fail-closed message.""" + from graphify import detect as detect_mod + from graphify.watch import _rebuild_code + + corpus = tmp_path / "corpus" + corpus.mkdir() + (corpus / "a.py").write_text("def alpha():\n return 1\n", encoding="utf-8") + (corpus / "b.py").write_text("def helper():\n return 2\n", encoding="utf-8") + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + graph_path = corpus / "graphify-out" / "graph.json" + + real_detect = detect_mod.detect + + def narrowed(root, **kwargs): + res = real_detect(root, **kwargs) + res["files"]["code"] = [ + f for f in res["files"]["code"] if not str(f).endswith("b.py") + ] + return res + + monkeypatch.setattr(detect_mod, "detect", narrowed) + capsys.readouterr() + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + sources = {n.get("source_file") for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]} + assert "b.py" in sources, ( + "alive file dropped from the corpus with NO matching ignore rule must be kept" + ) + out = capsys.readouterr().out + assert "fail-closed: kept" in out + assert "newly-ignored" not in out + + +def test_update_evicts_semantic_nodes_from_newly_ignored_non_ast_source(tmp_path, capsys): + """#2495 extends #2051: a non-AST source (.txt) whose semantic nodes evict on + disk absence must also evict when the file is alive but newly matched by a + .graphifyignore rule.""" + from graphify.watch import _rebuild_code + + corpus = tmp_path / "corpus" + corpus.mkdir() + (corpus / "app.py").write_text("def handle():\n return 1\n", encoding="utf-8") + (corpus / "notes.txt").write_text("Rationale to be ignored.\n", encoding="utf-8") + (corpus / "kept.txt").write_text("Rationale that stays.\n", encoding="utf-8") + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + graph_path = corpus / "graphify-out" / "graph.json" + data = json.loads(graph_path.read_text(encoding="utf-8")) + # No LLM in tests, so inject the semantic layer these .txt files would carry. + data["nodes"].extend([ + {"id": "notes_concept", "label": "Notes Concept", "file_type": "concept", + "source_file": "notes.txt"}, + {"id": "kept_concept", "label": "Kept Concept", "file_type": "concept", + "source_file": "kept.txt"}, + ]) + graph_path.write_text(json.dumps(data), encoding="utf-8") + + (corpus / ".graphifyignore").write_text("notes.txt\n", encoding="utf-8") + capsys.readouterr() + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + after_ids = {n["id"] for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]} + assert "notes_concept" not in after_ids, ( + "semantic node from a newly-ignored alive non-AST source must be evicted (#2495)" + ) + assert "kept_concept" in after_ids + assert "newly-ignored file(s)" in capsys.readouterr().out + + +def test_gitignore_eviction_gated_on_full_rebuild(tmp_path, capsys): + """#2495 policy split: a .gitignore-only match is preserved fail-closed on an + incremental rebuild (#1795's motivating case — a deliberately-graphed + .gitignore'd tree survives the hook path), but an explicit full + `graphify update` honors it and purges the file.""" + from graphify.watch import _rebuild_code + + corpus = tmp_path / "corpus" + generated = corpus / "generated" + generated.mkdir(parents=True) + (corpus / "app.py").write_text("def handle():\n return 1\n", encoding="utf-8") + (generated / "gen.py").write_text("def generated():\n return 2\n", encoding="utf-8") + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + graph_path = corpus / "graphify-out" / "graph.json" + sources = {n.get("source_file") for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]} + assert "generated/gen.py" in sources + + (corpus / ".gitignore").write_text("generated/\n", encoding="utf-8") + capsys.readouterr() + + # Incremental (hook) rebuild: .gitignore-driven eviction is NOT honored. + assert _rebuild_code( + corpus, changed_paths=[Path("app.py")], no_cluster=True, acquire_lock=False + ) is True + sources = {n.get("source_file") for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]} + assert "generated/gen.py" in sources, ( + ".gitignore-only match must be preserved on the incremental path" + ) + assert "fail-closed: kept" in capsys.readouterr().out + + # Explicit full update: the .gitignore match is honored and purged. + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + sources = {n.get("source_file") for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]} + assert "generated/gen.py" not in sources, ( + ".gitignore match must be purged by an explicit full update (#2495)" + ) + assert "newly-ignored file(s)" in capsys.readouterr().out + + +def test_graphifyignore_eviction_applies_to_incremental_rebuilds(tmp_path, capsys): + """#2495 policy split, other half: a .graphifyignore match is unambiguous + graph-level intent and evicts on the incremental (hook) path too.""" + from graphify.watch import _rebuild_code + + corpus = tmp_path / "corpus" + corpus.mkdir() + (corpus / "app.py").write_text("def handle():\n return 1\n", encoding="utf-8") + (corpus / "vendor.py").write_text("def vendored():\n return 2\n", encoding="utf-8") + + assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True + graph_path = corpus / "graphify-out" / "graph.json" + + (corpus / ".graphifyignore").write_text("vendor.py\n", encoding="utf-8") + capsys.readouterr() + + assert _rebuild_code( + corpus, changed_paths=[Path("app.py")], no_cluster=True, acquire_lock=False + ) is True + sources = {n.get("source_file") for n in json.loads(graph_path.read_text(encoding="utf-8"))["nodes"]} + assert "vendor.py" not in sources, ( + ".graphifyignore match must evict on an incremental rebuild too (#2495)" + ) + assert "app.py" in sources + assert "newly-ignored file(s)" in capsys.readouterr().out + + # --- #1915: semantic-backed docs must not be double-represented by the AST quick-scan ---