diff --git a/apps/backend/app/analysis/architecture.py b/apps/backend/app/analysis/architecture.py index 60402ddc..9dc391c4 100644 --- a/apps/backend/app/analysis/architecture.py +++ b/apps/backend/app/analysis/architecture.py @@ -1,4 +1,5 @@ import posixpath +import re from collections import Counter, defaultdict from app.extraction.lockfiles import SUPPORTED_LOCKFILE_FILENAMES @@ -59,6 +60,20 @@ } +_SYMBOL_DISAMBIGUATOR = re.compile(r"#\d+$") + + +def _display_symbol(qualified: str) -> str: + """Strip the uniqueness suffix a stable key carries for repeated names. + + Two `@overload`-style definitions of the same name in one file are distinct + nodes, so their keys are disambiguated (``group#5``). That suffix is an + identity detail, not part of what the code calls the symbol. + """ + + return _SYMBOL_DISAMBIGUATOR.sub("", qualified) + + class ArchitectureAnalyzer: """Builds the Architecture read model exclusively from sealed ri.v1 snapshots. @@ -135,8 +150,8 @@ def _nodes_for_modules(self, modules: list[RepositoryModule]) -> list[ArchNode]: id=module.id, name=module.name, type=node_type, # type: ignore[arg-type] - description=f"{module.name} derived from repository intelligence at {module.path_prefix}.", - responsibilities=[f"Owns {module.role} concerns"], + description=self._module_description(module), + responsibilities=self._module_responsibilities(module), files=module.files[:25], dependencies=[], dependents=[], @@ -151,6 +166,52 @@ def _nodes_for_modules(self, modules: list[RepositoryModule]) -> list[ArchNode]: ) return nodes + @staticmethod + def _module_description(module: RepositoryModule) -> str: + """State what the snapshot observed, rather than restating the path. + + Every clause here is an observed fact already sealed in the snapshot: + how many symbols the module defines, which of them a reader would + recognise it by, and where it lives. Nothing is inferred about what + the module is *for* -- that would be a guess, and an unsourced claim + is exactly what this product does not make. + """ + + if not module.symbols: + # No symbols observed is itself worth saying plainly, rather than + # dressing the path up as a description. + return f"{module.path_prefix} — no code symbols were extracted from this module." + count = len(module.symbols) + noun = "symbol" if count == 1 else "symbols" + notable = ArchitectureAnalyzer._notable_symbols(module.symbols) + if not notable: + return f"Defines {count} {noun}." + listed = ", ".join(notable) + if count == len(notable): + # Everything it defines is named, so "including" would understate it. + return f"Defines {count} {noun}: {listed}." + return f"Defines {count} {noun}, including {listed}." + + @staticmethod + def _module_responsibilities(module: RepositoryModule) -> list[str]: + """Observed properties of the module, not a guess at its purpose. + + The previous wording ("Owns unknown concerns") read as a statement + about the code when it was really a statement about the classifier + having no opinion. Where a role *was* classified it is reported as + one; where it was not, the entry is omitted rather than asserted. + """ + + entries: list[str] = [] + if module.role and module.role != "unknown": + entries.append(f"Classified as {module.role.replace('-', ' ')}") + file_count = len(module.files) + if file_count > 1: + entries.append(f"{file_count} files") + if module.symbols: + entries.append(f"{len(module.symbols)} observed symbols") + return entries + def _empty_module(self, files: list[str]) -> list[RepositoryModule]: return [ RepositoryModule( @@ -182,6 +243,84 @@ def _file_roles(self, facts: ArchitectureSnapshotFacts) -> dict[str, str]: roles[assertion.subject_key.removeprefix("file:")] = classification return roles + @staticmethod + def _paths_in_relationships(facts: ArchitectureSnapshotFacts) -> set[str]: + """Files that are one end of an observed architecture relationship. + + A file can be a real part of the system while defining nothing of its + own -- a package initialiser that only re-exports, a barrel module. + What makes it structural is that something resolved to it, or it + resolved to something, and both of those are sealed edges. + """ + + paths: set[str] = set() + for edge in facts.edges: + for key in (edge.subject_key, edge.object_key): + if key.startswith("file:"): + paths.add(key.removeprefix("file:")) + return paths + + @staticmethod + def _is_module_file( + path: str, + *, + role: str | None, + defines_symbols: bool, + related_paths: set[str], + ) -> bool: + """Whether this file is part of the system's own structure (#444). + + A module has to be something the extraction actually saw: symbols it + defines, a relationship it takes part in, or a role the snapshot + classified it into (``documentation``, ``test``, ``controller``). A + file that produced none of those is not being judged unimportant -- + nothing was observed about it, and inventing a module from a path is + how `.gitignore` ended up sitting in the Shared layer beside the + library itself. + """ + + if defines_symbols or path in related_paths: + return True + return role is not None and role != "unknown" + + def _symbols_by_file(self, facts: ArchitectureSnapshotFacts) -> dict[str, list[str]]: + """Map file path -> names of the symbols that file defines. + + Symbol stable keys are ``::`` (#217), so the + owning file is read off the key rather than inferred. These are + observed facts already sealed in the snapshot; nothing here computes + or estimates anything. + """ + + symbols: dict[str, list[str]] = defaultdict(list) + for stable_key in facts.symbol_keys: + path, separator, qualified = stable_key.partition("::") + if not separator or not path or not qualified: + continue + # Top-level definitions only. A method is defined by its class, not + # by the module, and counting every one of them turns "what does + # this module define" into a line-count proxy -- which is exactly + # the kind of synthesized measure #217 rules out. + if "." in qualified: + continue + symbols[path].append(_display_symbol(qualified)) + return {path: sorted(set(names)) for path, names in symbols.items()} + + @staticmethod + def _notable_symbols(qualified_names: list[str], limit: int = 4) -> list[str]: + """The symbols a reader would recognise the module by. + + The caller has already narrowed these to top-level definitions, so the + only judgement left is the oldest convention there is: a leading + underscore means the author did not mean it for the outside. Those are + dropped unless they are all there is. Ordering is deterministic, so the + same snapshot always renders the same description. + """ + + public = [name for name in qualified_names if not name.startswith("_")] + chosen = public or qualified_names + return sorted(chosen)[:limit] + def _modules_from_facts(self, facts: ArchitectureSnapshotFacts | None) -> list[RepositoryModule]: if facts is None: # Defensive only: build_architecture requires a sealed snapshot @@ -190,24 +329,39 @@ def _modules_from_facts(self, facts: ArchitectureSnapshotFacts | None) -> list[R # unresolved. It stays as an honest, empty module set rather than # ever reading `record.file_tree` (unsealed repository metadata). return self._empty_module([]) - # Dependency-manifest and lockfile paths already surface as - # dependency evidence (Dependency Graph) -- grouping them into an - # architecture module too misrepresents `package.json`/ - # `pyproject.toml` as a piece of the system's own structure. + role_by_path = self._file_roles(facts) + symbols_by_path = self._symbols_by_file(facts) + related_paths = self._paths_in_relationships(facts) + # Dependency-manifest and lockfile paths already surface as dependency + # evidence (Dependency Graph) -- grouping them into an architecture + # module too misrepresents `package.json`/`pyproject.toml` as a piece + # of the system's own structure. #396 stopped there, which was right + # but narrower than the defect: `.gitignore`, `.editorconfig`, + # `LICENSE.txt` and `uv.lock` are neither manifests nor lockfiles, so + # on `pallets/click` seven of the sixteen reported modules were not + # code. `_is_module_file` is what closes that hole. _non_module_filenames = SUPPORTED_MANIFEST_FILENAMES + SUPPORTED_LOCKFILE_FILENAMES file_paths = sorted( - node.stable_key.removeprefix("file:") - for node in facts.nodes - if node.node_kind == "file" - and node.stable_key.startswith("file:") - and posixpath.basename(node.stable_key.removeprefix("file:")) not in _non_module_filenames + path + for path in ( + node.stable_key.removeprefix("file:") + for node in facts.nodes + if node.node_kind == "file" and node.stable_key.startswith("file:") + ) + if posixpath.basename(path) not in _non_module_filenames + and self._is_module_file( + path, + role=role_by_path.get(path), + defines_symbols=bool(symbols_by_path.get(path)), + related_paths=related_paths, + ) ) if not file_paths: return self._empty_module([]) - role_by_path = self._file_roles(facts) grouped: dict[str, list[str]] = defaultdict(list) for path in file_paths: - grouped[self._module_id(path, role_by_path.get(path))].append(path) + module_id = self._module_id(path, role_by_path.get(path), defines_symbols=bool(symbols_by_path.get(path))) + grouped[module_id].append(path) modules: list[RepositoryModule] = [] for module_id, paths in grouped.items(): candidate_roles = [ @@ -228,14 +382,53 @@ def _modules_from_facts(self, facts: ArchitectureSnapshotFacts | None) -> list[R layer=layer_for_role(dominant), path_prefix=self._path_prefix(paths), files=sorted(paths), - symbols=[], + symbols=sorted({name for path in paths for name in symbols_by_path.get(path, [])}), dependencies=[], ) ) - return sorted(modules, key=lambda module: module.id) + return self._disambiguate_names(sorted(modules, key=lambda module: module.id)) + + @staticmethod + def _disambiguate_names(modules: list[RepositoryModule]) -> list[RepositoryModule]: + """Qualify names that would otherwise collide. + + A repository can hold several modules called ``utils`` -- FastAPI has + four. Rendering them all as "utils" tells the reader nothing about + which is which, so a colliding name takes on as much of its parent + path as it needs to become unique (``openapi/utils``, + ``security/utils``). Names that are already unique are left alone, so + the common case stays short. + """ + + by_name: dict[str, list[RepositoryModule]] = defaultdict(list) + for module in modules: + by_name[module.name].append(module) + for name, colliding in by_name.items(): + if len(colliding) < 2: + continue + for module in colliding: + qualifier = ArchitectureAnalyzer._qualifying_parent(module.id.removeprefix("module:"), name) + if qualifier: + module.name = f"{qualifier}/{name}" + return modules + + @staticmethod + def _qualifying_parent(path: str, name: str) -> str: + """The nearest ancestor directory that actually distinguishes ``path``. + + A directory named after the module it contains adds nothing: + `examples/termui/termui.py` qualified by its immediate parent reads + "termui/termui", which is noise where "examples/termui" is an answer. + Walk up until the ancestor says something the name does not. + """ + + for segment in reversed(posixpath.dirname(path).split("/")): + if segment and segment != name: + return segment + return "" @staticmethod - def _module_id(path: str, role: str | None) -> str: + def _module_id(path: str, role: str | None, *, defines_symbols: bool = False) -> str: parts = [part for part in path.strip("/").split("/") if part] if role in {"controller", "route"}: return "module:api" @@ -253,6 +446,13 @@ def _module_id(path: str, role: str | None) -> str: return "module:tests" if role == "documentation": return "module:documentation" + # A file that defines symbols is a module in its own right. Grouping + # by the directory below the source root instead collapses a whole + # package into one opaque node -- for a single-package repository that + # is the entire library reduced to a single box, which is what this + # branch used to do to every file under `src//`. + if defines_symbols and parts: + return f"module:{path.strip('/')}" if parts and parts[0] in {"app", "src", "backend", "frontend", "apps"} and len(parts) > 1: return f"module:{parts[1].lower()}" return f"module:{parts[0].lower() if parts else 'repository'}" @@ -260,6 +460,15 @@ def _module_id(path: str, role: str | None) -> str: @staticmethod def _module_display_name(module_id: str) -> str: raw = module_id.removeprefix("module:") + if "/" in raw: + # A per-file module id carries the path for uniqueness; the reader + # wants the module's own name. `src/click/core.py` reads as `core`, + # and a package initialiser reads as the package it opens. + stem = posixpath.splitext(posixpath.basename(raw))[0] + if stem in {"__init__", "index", "mod"}: + parent = posixpath.basename(posixpath.dirname(raw)) + return parent or stem + return stem if "." in raw: # `_module_id`'s fallback groups a top-level file with no # directory nesting by its own filename (e.g. "app.py") -- that diff --git a/apps/backend/app/extraction/lockfiles.py b/apps/backend/app/extraction/lockfiles.py index d7335910..20ec4272 100644 --- a/apps/backend/app/extraction/lockfiles.py +++ b/apps/backend/app/extraction/lockfiles.py @@ -64,10 +64,14 @@ json_object_member_lines, toml_array_of_tables_lines, ) -from app.extraction.support_matrix import supported_lockfile_filenames +from app.extraction.support_matrix import disclosed_lockfile_filenames, supported_lockfile_filenames from app.intelligence import canonical SUPPORTED_LOCKFILE_FILENAMES = supported_lockfile_filenames() +#: Lockfile names this extractor claims only in order to disclose that it does +#: not read them (#444). Skipping one silently reports the same empty result as +#: a repository that pins nothing, which is the opposite of an honest limit. +DISCLOSED_LOCKFILE_FILENAMES = disclosed_lockfile_filenames() #: ``package-lock.json`` revisions whose ``packages`` table this extractor reads. SUPPORTED_NPM_LOCKFILE_VERSIONS = (2, 3) @@ -116,7 +120,8 @@ def producer(self) -> str: return f"{self.name}@{self.version}" def supports(self, path: str) -> bool: - return posixpath.basename(path) in SUPPORTED_LOCKFILE_FILENAMES + basename = posixpath.basename(path) + return basename in SUPPORTED_LOCKFILE_FILENAMES or basename in DISCLOSED_LOCKFILE_FILENAMES def extract(self, path: str, source: bytes) -> ExtractionResult: text, source_diagnostic = decode_source(path, source, producer=self.producer) @@ -141,6 +146,8 @@ def extract(self, path: str, source: bytes) -> ExtractionResult: basename = posixpath.basename(normalized_path) file_subject = canonical.normalize_stable_key("file", f"file:{normalized_path}") try: + if basename in DISCLOSED_LOCKFILE_FILENAMES: + raise _UnsupportedLockfile(f"{basename} is recognised but not read, and no resolutions are claimed") if basename == "package-lock.json": resolutions = self._npm_resolutions(text) lockfile_format = "npm-package-lock" diff --git a/apps/backend/app/extraction/support_matrix.py b/apps/backend/app/extraction/support_matrix.py index 7985f22f..d40567bf 100644 --- a/apps/backend/app/extraction/support_matrix.py +++ b/apps/backend/app/extraction/support_matrix.py @@ -571,11 +571,24 @@ def _capability( "Resolved PyPI versions from poetry.lock with lock-version major 1 or 2.", ( "Only each [[package]] table's name and version are read. Lock-version 2 removed the per-package category " - "field, so the production/development split is reported as unknown rather than guessed. Pipfile.lock, " - "uv.lock, and pdm.lock are not read." + "field, so the production/development split is reported as unknown rather than guessed. Pipfile.lock and " + "pdm.lock are not read; uv.lock is recognised and disclosed separately." ), "src.poetry_lockfile", ), + _capability( + "lockfile.uv-lock", + "source", + "lockfile:uv.lock", + SupportStatus.UNSUPPORTED, + "uv.lock, Astral uv's resolved lockfile.", + ( + "The file is recognised and disclosed rather than read: no resolved version is claimed from it. It was " + "previously invisible, which left a uv-managed repository looking as though it pinned nothing." + ), + "src.uv_lockfile", + expected_diagnostic="RI-EXT-UNSUPPORTED", + ), ) @@ -772,8 +785,33 @@ def _supported_filenames(capabilities: tuple[Capability, ...], prefix: str) -> t ) +def _disclosed_filenames( + capabilities: tuple[Capability, ...], prefix: str, supported: tuple[str, ...] +) -> tuple[str, ...]: + """Derive the filenames the registry names as unsupported *formats*. + + A file the product cannot read is still worth recognising: silently + skipping it reports the same empty result as a repository that genuinely + pins nothing. An extractor claims these filenames only so it can disclose + them as ``RI-EXT-UNSUPPORTED``. Revision qualifiers (``@v1``) and anything + already supported are excluded — those are handled where the file is read. + """ + + return tuple( + sorted( + { + item.construct.removeprefix(prefix) + for item in capabilities + if item.status == SupportStatus.UNSUPPORTED and "@" not in item.construct + } + - set(supported) + ) + ) + + _SUPPORTED_MANIFEST_FILENAMES = _supported_filenames(MANIFEST_CAPABILITIES, "manifest:") _SUPPORTED_LOCKFILE_FILENAMES = _supported_filenames(LOCKFILE_CAPABILITIES, "lockfile:") +_DISCLOSED_LOCKFILE_FILENAMES = _disclosed_filenames(LOCKFILE_CAPABILITIES, "lockfile:", _SUPPORTED_LOCKFILE_FILENAMES) # Compose accepts four canonical filenames for one format, so the registry # carries the format id and the filename set is spelled out beside it. @@ -1059,6 +1097,10 @@ def supported_lockfile_filenames() -> tuple[str, ...]: return _SUPPORTED_LOCKFILE_FILENAMES +def disclosed_lockfile_filenames() -> tuple[str, ...]: + return _DISCLOSED_LOCKFILE_FILENAMES + + def supported_iac_filenames() -> tuple[str, ...]: return _SUPPORTED_IAC_FILENAMES diff --git a/apps/backend/app/intelligence/query_service.py b/apps/backend/app/intelligence/query_service.py index 13df5a60..daa52987 100644 --- a/apps/backend/app/intelligence/query_service.py +++ b/apps/backend/app/intelligence/query_service.py @@ -83,6 +83,13 @@ class ArchitectureSnapshotFacts: snapshot: RiSnapshot nodes: list[RiNode] + #: Stable keys of every ``symbol`` node in the snapshot, and nothing else. + #: The architecture consumer needs to know what each file defines, but + #: whole symbol rows are what dominate a large snapshot -- see the bound in + #: ``architecture_facts``. Keys alone are short strings and carry the file + #: and the qualified name already (``::``), which is all + #: the module inventory reads. + symbol_keys: list[str] edges: list[RiEdge] assertions: list[RiAssertion] node_evidence: dict[int, list[RiEvidence]] @@ -395,6 +402,13 @@ def architecture_facts(self, repository_id: str) -> ArchitectureSnapshotFacts | .order_by(RiNode.stable_key, RiNode.id) ).all() ) + symbol_keys = list( + self.db.scalars( + select(RiNode.stable_key) + .where(RiNode.snapshot_id == snapshot.snapshot_id, RiNode.node_kind == "symbol") + .order_by(RiNode.stable_key) + ).all() + ) diagnostics = list( self.db.scalars( select(RiDiagnostic) @@ -424,6 +438,7 @@ def architecture_facts(self, repository_id: str) -> ArchitectureSnapshotFacts | return ArchitectureSnapshotFacts( snapshot=snapshot, nodes=nodes, + symbol_keys=symbol_keys, edges=edges, assertions=assertions, node_evidence=self._evidence_for(snapshot, "node_ref", [node.id for node in nodes]), diff --git a/apps/backend/tests/benchmark/config/benchmark_support_matrix.json b/apps/backend/tests/benchmark/config/benchmark_support_matrix.json index 3c04b26c..3c89db36 100644 --- a/apps/backend/tests/benchmark/config/benchmark_support_matrix.json +++ b/apps/backend/tests/benchmark/config/benchmark_support_matrix.json @@ -56,6 +56,7 @@ "src.npm_lockfile_nested": {"description": "A second resolved version of one package in a nested npm tree."}, "src.npm_lockfile_v1": {"description": "A package-lock.json using the unsupported lockfileVersion 1."}, "src.poetry_lockfile": {"description": "A resolved PyPI version in poetry.lock."}, + "src.uv_lockfile": {"description": "A uv.lock, recognised and disclosed rather than read."}, "src.compose_service": {"description": "A declared Docker Compose service."}, "src.compose_volume": {"description": "A declared Docker Compose volume."}, "src.compose_network": {"description": "A declared Docker Compose network."}, @@ -120,6 +121,7 @@ "src.npm_lockfile_nested": "lockfile.npm-package-lock", "src.npm_lockfile_v1": "lockfile.npm-package-lock-v1", "src.poetry_lockfile": "lockfile.poetry-lock", + "src.uv_lockfile": "lockfile.uv-lock", "src.compose_service": "iac.docker-compose", "src.compose_volume": "iac.docker-compose", "src.compose_network": "iac.docker-compose", diff --git a/apps/backend/tests/benchmark/fixtures/adversarial/adv-src-lockfile-uv/manifest.json b/apps/backend/tests/benchmark/fixtures/adversarial/adv-src-lockfile-uv/manifest.json new file mode 100644 index 00000000..e9d29faf --- /dev/null +++ b/apps/backend/tests/benchmark/fixtures/adversarial/adv-src-lockfile-uv/manifest.json @@ -0,0 +1,13 @@ +{ + "schemaVersion":"ri-benchmark.v1","fixtureId":"adv-src-lockfile-uv","fixtureClass":"adversarial","language":"mixed", + "title":"Adversarial lockfile — recognised but unread uv.lock","description":"A uv.lock carries two real resolved versions and neither is claimed: the format is named and disclosed rather than skipped, so a uv-managed repository does not read as one that pins nothing.", + "sourceRoot":".","revisionIdentity":"upload-sha256","producerVersionSet":["dependency-lockfile@1.0.0","repository-inventory@1.1.0"], + "constructsCovered":["src.repository","src.uv_lockfile"],"deterministic":true, + "expected":{"nodes":[ + {"comment":"As with the unsupported npm revision, the repository entity comes from stored bytes before extractor dispatch, so a diagnostics-only revision is still sealable. No file node and no dependency node: the extractor claims nothing about a format it does not read.", + "nodeKind":"repository","stableKey":"repo:root","name":"repository","constructs":["src.repository"],"evidence":[{"path":"uv.lock","startLine":1,"endLine":1,"extractor":"repository-inventory","extractorVersion":"1.1.0"}]} + ],"edges":[],"observations":[],"assertions":[],"diagnostics":[ + {"comment":"Recognising the filename is the whole point (#444): before this, uv.lock reached no extractor at all, so the absence of pins was indistinguishable from a repository that declares none. The file is well-formed TOML, so RI-SRC-MALFORMED would be a false claim about the source.", + "code":"RI-EXT-UNSUPPORTED","category":"unsupported construct","severity":"info","message":"uv.lock is recognised but not read, and no resolutions are claimed","producer":"dependency-lockfile@1.0.0","path":"uv.lock","subject":"file:uv.lock","constructs":["src.uv_lockfile"]} + ]} +} diff --git a/apps/backend/tests/benchmark/fixtures/adversarial/adv-src-lockfile-uv/uv.lock b/apps/backend/tests/benchmark/fixtures/adversarial/adv-src-lockfile-uv/uv.lock new file mode 100644 index 00000000..8627f99c --- /dev/null +++ b/apps/backend/tests/benchmark/fixtures/adversarial/adv-src-lockfile-uv/uv.lock @@ -0,0 +1,12 @@ +version = 1 +requires-python = ">=3.11" + +[[package]] +name = "click" +version = "8.1.7" +source = { registry = "https://pypi.org/simple" } + +[[package]] +name = "pytest" +version = "8.3.4" +source = { registry = "https://pypi.org/simple" } diff --git a/apps/backend/tests/extraction/test_lockfiles.py b/apps/backend/tests/extraction/test_lockfiles.py index 8fcb9042..10f4e80a 100644 --- a/apps/backend/tests/extraction/test_lockfiles.py +++ b/apps/backend/tests/extraction/test_lockfiles.py @@ -20,7 +20,7 @@ SUPPORTED_POETRY_LOCK_MAJORS, LockfileExtractor, ) -from app.extraction.support_matrix import supported_lockfile_filenames +from app.extraction.support_matrix import disclosed_lockfile_filenames, supported_lockfile_filenames NPM_LOCK = b"""{ "name": "web", @@ -72,6 +72,15 @@ lock-version = "2.0" """ +UV_LOCK = b"""version = 1 +requires-python = ">=3.11" + +[[package]] +name = "click" +version = "8.1.7" +source = { registry = "https://pypi.org/simple" } +""" + def _nodes(result): """Index by stable key, keeping the first emission. @@ -105,7 +114,29 @@ def test_supported_filenames_come_from_the_capability_registry(): assert not extractor.supports("yarn.lock") assert not extractor.supports("pnpm-lock.yaml") assert not extractor.supports("Pipfile.lock") - assert not extractor.supports("uv.lock") + # uv.lock is the deliberate exception (#444): claimed so the extractor can + # say out loud that it does not read it. Supported filenames are unchanged, + # so nothing downstream mistakes the disclosure for extraction support. + assert set(disclosed_lockfile_filenames()) == {"uv.lock"} + assert extractor.supports("uv.lock") + assert "uv.lock" not in supported_lockfile_filenames() + + +def test_a_recognised_but_unread_lockfile_is_disclosed_rather_than_skipped(): + """#444: silence would report the same empty result as a repository that + genuinely pins nothing. The file is well-formed, so the disclosure is an + unsupported-construct info, never a malformed-source error.""" + + result = _extract("uv.lock", UV_LOCK) + + assert result.nodes == () + assert [diagnostic.code for diagnostic in result.diagnostics] == ["RI-EXT-UNSUPPORTED"] + diagnostic = result.diagnostics[0] + assert diagnostic.severity == "info" + assert diagnostic.path == "uv.lock" + assert diagnostic.subject == "file:uv.lock" + # No pin from the file is claimed, and none is quoted into the message. + assert "8.1.7" not in diagnostic.message # --- npm -------------------------------------------------------------------- diff --git a/apps/backend/tests/test_architecture_relationships.py b/apps/backend/tests/test_architecture_relationships.py index 80846333..f421eb7c 100644 --- a/apps/backend/tests/test_architecture_relationships.py +++ b/apps/backend/tests/test_architecture_relationships.py @@ -178,14 +178,16 @@ def test_architecture_edges_come_from_resolved_snapshot_evidence(auth_client): nodes = {node["id"]: node for node in architecture["nodes"]} # Both modules have the same persisted role (entrypoint); neither is collapsed. - assert "entrypoint" in nodes["module:alpha"]["tags"] - assert "entrypoint" in nodes["module:beta"]["tags"] + assert "entrypoint" in nodes["module:src/alpha/index.ts"]["tags"] + assert "entrypoint" in nodes["module:src/beta/index.ts"]["tags"] assert architecture["relationshipSnapshotId"] == snapshot_id import_edges = [ edge for edge in architecture["edges"] - if edge["source"] == "module:alpha" and edge["target"] == "module:beta" and edge["predicate"] == "imports" + if edge["source"] == "module:src/alpha/index.ts" + and edge["target"] == "module:src/beta/index.ts" + and edge["predicate"] == "imports" ] assert len(import_edges) == 1 edge = import_edges[0] @@ -199,19 +201,23 @@ def test_architecture_edges_come_from_resolved_snapshot_evidence(auth_client): "endLine": 1, } ] - assert "module:beta" in nodes["module:alpha"]["dependencies"] - assert "module:alpha" in nodes["module:beta"]["dependents"] - assert nodes["module:alpha"]["relationshipState"] == "connected" - assert nodes["module:beta"]["relationshipState"] == "connected" + assert "module:src/beta/index.ts" in nodes["module:src/alpha/index.ts"]["dependencies"] + assert "module:src/alpha/index.ts" in nodes["module:src/beta/index.ts"]["dependents"] + assert nodes["module:src/alpha/index.ts"]["relationshipState"] == "connected" + assert nodes["module:src/beta/index.ts"]["relationshipState"] == "connected" assert any( - item["source"] == "module:alpha" and item["target"] == "module:beta" and item["predicate"] == "calls" + item["source"] == "module:src/alpha/index.ts" + and item["target"] == "module:src/beta/index.ts" + and item["predicate"] == "calls" for item in architecture["edges"] ) assert not any(item["source"] == item["target"] for item in architecture["edges"]) assert not any(item["code"] == "ARCH-REL-ENDPOINT-UNMAPPED" for item in architecture["diagnostics"]) dependency_edges = [ - item for item in architecture["edges"] if item["source"] == "module:alpha" and item["target"] == "dep:npm:react" + item + for item in architecture["edges"] + if item["source"] == "module:src/alpha/index.ts" and item["target"] == "dep:npm:react" ] assert {item["predicate"] for item in dependency_edges} == {"imports", "depends_on"} assert not any(item["target"] == "dep:npm:lodash" for item in architecture["edges"]) @@ -223,7 +229,7 @@ def test_architecture_edges_come_from_resolved_snapshot_evidence(auth_client): if item["code"] == "ARCH-REL-REPO-SCOPED" and item["path"] == "package.json" and item["severity"] == "info" ) assert root_scope_diagnostic["nodeIds"] is None - assert nodes["module:lonely"]["relationshipState"] == "no-observed-relationships" + assert nodes["module:src/lonely/index.ts"]["relationshipState"] == "no-observed-relationships" assert nodes["module:documentation"]["relationshipState"] == "not-extracted" diagnostics = architecture["diagnostics"] @@ -231,10 +237,10 @@ def test_architecture_edges_come_from_resolved_snapshot_evidence(auth_client): assert any( item["code"] == "RI-RES-UNRESOLVED" and item["path"] == "src/unresolved/index.ts" for item in diagnostics ) - assert nodes["module:ambiguous"]["relationshipState"] == "unresolved" - assert nodes["module:unresolved"]["relationshipState"] == "unresolved" - assert not any(edge["source"] == "module:ambiguous" for edge in architecture["edges"]) - assert not any(edge["source"] == "module:unresolved" for edge in architecture["edges"]) + assert nodes["module:src/ambiguous/index.ts"]["relationshipState"] == "unresolved" + assert nodes["module:src/unresolved/index.ts"]["relationshipState"] == "unresolved" + assert not any(edge["source"] == "module:src/ambiguous/index.ts" for edge in architecture["edges"]) + assert not any(edge["source"] == "module:src/unresolved/index.ts" for edge in architecture["edges"]) evidence_response = auth_client.get(f"/intelligence/v1/snapshots/{snapshot_id}/evidence?limit=100") assert evidence_response.status_code == 200 @@ -276,7 +282,9 @@ def test_architecture_maps_every_snapshot_file_to_a_module(auth_client): import_edges = [ edge for edge in architecture["edges"] - if edge["source"] == "module:unmapped.ts" and edge["target"] == "module:beta" and edge["predicate"] == "imports" + if edge["source"] == "module:unmapped.ts" + and edge["target"] == "module:src/beta/index.ts" + and edge["predicate"] == "imports" ] assert len(import_edges) == 1 assert import_edges[0]["evidence"][0]["path"] == "unmapped.ts" @@ -319,7 +327,7 @@ def test_architecture_excludes_manifest_and_lockfile_paths_from_modules(auth_cli node_ids = {node["id"] for node in response.json()["nodes"]} assert not any("package.json" in node_id for node_id in node_ids) assert not any("package-lock.json" in node_id for node_id in node_ids) - assert "module:beta" in node_ids + assert "module:src/beta/index.ts" in node_ids def test_architecture_does_not_flag_a_module_for_external_or_platform_references(auth_client): @@ -341,10 +349,10 @@ def test_architecture_does_not_flag_a_module_for_external_or_platform_references diagnostics = architecture["diagnostics"] # 'fs' + readFileSync() are the language platform: not a coverage gap. - assert nodes["module:pure"]["relationshipState"] != "unresolved" + assert nodes["module:src/pure/index.ts"]["relationshipState"] != "unresolved" assert not any(item["code"] == "RI-RES-UNRESOLVED" and item["path"] == "src/pure/index.ts" for item in diagnostics) # '../nowhere' resolves to nothing in-repo: still a real gap. - assert nodes["module:broken"]["relationshipState"] == "unresolved" + assert nodes["module:src/broken/index.ts"]["relationshipState"] == "unresolved" assert any(item["code"] == "RI-RES-UNRESOLVED" and item["path"] == "src/broken/index.ts" for item in diagnostics) @@ -485,3 +493,35 @@ def test_architecture_without_a_sealed_snapshot_returns_404(auth_client): response = auth_client.get(f"/analysis/{repository['id']}/architecture") assert response.status_code == 404 + + +def test_repository_furniture_does_not_become_an_architecture_module(auth_client): + """#444: on `pallets/click`, seven of the sixteen reported modules were a + dotfile, a licence, a changelog or `uv.lock` -- the repository's furniture + given the same weight as the library. A file the extraction observed nothing + about is not a module of the system.""" + + sources = { + ".gitignore": b"dist/\n*.pyc\n", + ".editorconfig": b"root = true\n", + ".github/workflows/ci.yaml": b"name: ci\non: [push]\n", + ".pre-commit-config.yaml": b"repos: []\n", + "LICENSE.txt": b"BSD 3-Clause License\n", + "CHANGES.md": b"# Changes\n\n## 1.0\n", + "uv.lock": b'version = 1\n\n[[package]]\nname = "click"\nversion = "8.1.7"\n', + "src/alpha/index.ts": b"export const alpha = 1;\n", + } + repository = _upload(auth_client, sources) + _persist_snapshot(repository["id"], sources) + + architecture = auth_client.get(f"/analysis/{repository['id']}/architecture").json() + node_ids = {node["id"] for node in architecture["nodes"]} + + for furniture in (".gitignore", ".editorconfig", ".pre-commit-config", "license", "uv.lock", "github"): + assert not any(furniture in node_id.lower() for node_id in node_ids), f"{furniture} became a module: {node_ids}" + # The one file that defines something is still there, so the rule excludes + # furniture rather than everything that is not deeply nested. + assert "module:src/alpha/index.ts" in node_ids + # And nothing is left describing itself by its own path. + assert not any("derived from repository intelligence" in node["description"] for node in architecture["nodes"]) + assert not any("Owns unknown concerns" in node["responsibilities"] for node in architecture["nodes"]) diff --git a/apps/frontend/e2e/architecture-visual.spec.ts b/apps/frontend/e2e/architecture-visual.spec.ts index d62ec399..d79d3d82 100644 --- a/apps/frontend/e2e/architecture-visual.spec.ts +++ b/apps/frontend/e2e/architecture-visual.spec.ts @@ -188,10 +188,23 @@ test.describe('architecture graph visual acceptance', () => { await openArchitecture(page, byLabel('long-labels')); await waitForGraph(page); - const group = page.locator('.react-flow__node [role="group"]').first(); + // Found by name, not by position: node ids order the graph, so picking + // `.first()` silently re-pointed this assertion at the README module the + // moment module ids changed (#445). The long name is the subject here. + const groups = page.locator('.react-flow__node [role="group"]'); + const names = await groups.evaluateAll((nodes) => + nodes.map((node) => node.getAttribute('aria-label') ?? '') + ); + const longLabel = 'customer-subscription-entitlement-orchestration'; + const index = names.findIndex((name) => name.includes(longLabel)); + expect(index, `no module named for ${longLabel} among: ${names.join(' | ')}`).toBeGreaterThan(-1); + + const group = groups.nth(index); const accessibleName = await group.getAttribute('aria-label'); - expect(accessibleName).toContain('Customer Subscription Entitlement Orchestration'); + // Truncated on screen, whole in the accessible name -- the point of the test. expect(await group.getAttribute('title')).toBe(accessibleName); + const label = group.getByTestId('architecture-node-label'); + expect(await label.evaluate((node) => node.scrollWidth > node.clientWidth)).toBe(true); await capture(page, 'architecture-long-labels', testInfo); });