diff --git a/apps/backend/app/analysis/architecture.py b/apps/backend/app/analysis/architecture.py index 3252722..3279006 100644 --- a/apps/backend/app/analysis/architecture.py +++ b/apps/backend/app/analysis/architecture.py @@ -57,18 +57,21 @@ class ArchitectureAnalyzer: """Builds the Architecture read model exclusively from sealed ri.v1 snapshots. No filesystem read, no working-tree fallback, and no legacy - ``repo_metadata['intelligence']`` read: every field is derived from - :class:`SnapshotQueryService` facts, or from ``record.file_tree`` (already - persisted repository metadata, not a filesystem walk) when no sealed - snapshot exists yet. + ``repo_metadata['intelligence']`` or ``record.file_tree`` read: every field + is derived from :class:`SnapshotQueryService` facts. A repository with no + sealed snapshot for its current revision raises ``NotFoundError`` (#217), + the same 404 contract Dependencies, Review and Insights already use — + never a fallback graph built from unsealed repository metadata. """ def __init__(self, snapshots: SnapshotQueryService | None = None) -> None: self.snapshots = snapshots def build_architecture(self, record: RepositoryRecord) -> ArchitectureResponse: + if self.snapshots is not None: + self.snapshots.require_sealed_snapshot_for_current_revision(record.id) facts = self.snapshots.architecture_facts(record.id) if self.snapshots is not None else None - modules = self._modules_from_facts(facts, record) + modules = self._modules_from_facts(facts) frameworks = self._frameworks_from_facts(facts) primary_language = self._primary_language_from_facts(facts) entry_points = self._entry_points_from_facts(facts) @@ -129,24 +132,17 @@ def _nodes_for_modules(self, modules: list[RepositoryModule]) -> list[ArchNode]: files=module.files[:25], dependencies=[], dependents=[], - estimated_complexity="high" if len(module.files) > 30 else "medium" if len(module.files) > 10 else "low", - estimated_lines=max(len(module.files) * 80, 20), + # No producer measures size/complexity (#217): file count is + # not a line count or a complexity metric, so it is never + # used to synthesize one. + estimated_complexity="not_computed", + estimated_lines="not_computed", tags=[module.layer, module.role, module.id.replace("module:", "")], layer=module.layer, ) ) return nodes - def _persisted_file_paths(self, tree: list[dict]) -> list[str]: - paths: list[str] = [] - for item in tree: - if item.get("type") == "file" and isinstance(item.get("path"), str): - paths.append(item["path"]) - children = item.get("children") - if isinstance(children, list): - paths.extend(self._persisted_file_paths(children)) - return sorted(set(paths)) - def _empty_module(self, files: list[str]) -> list[RepositoryModule]: return [ RepositoryModule( @@ -178,11 +174,14 @@ def _file_roles(self, facts: ArchitectureSnapshotFacts) -> dict[str, str]: roles[assertion.subject_key.removeprefix("file:")] = classification return roles - def _modules_from_facts( - self, facts: ArchitectureSnapshotFacts | None, record: RepositoryRecord - ) -> list[RepositoryModule]: + def _modules_from_facts(self, facts: ArchitectureSnapshotFacts | None) -> list[RepositoryModule]: if facts is None: - return self._empty_module(self._persisted_file_paths(record.file_tree or [])) + # Defensive only: build_architecture requires a sealed snapshot + # before calling this whenever self.snapshots is configured, so a + # production caller never reaches this branch with real facts + # unresolved. It stays as an honest, empty module set rather than + # ever reading `record.file_tree` (unsealed repository metadata). + return self._empty_module([]) file_paths = sorted( node.stable_key.removeprefix("file:") for node in facts.nodes @@ -311,8 +310,8 @@ def _dependency_nodes(self, facts: ArchitectureSnapshotFacts | None) -> list[Arc files=sorted({entry.path for entry in evidence}), dependencies=[], dependents=[], - estimated_complexity="low", - estimated_lines=0, + estimated_complexity="not_computed", + estimated_lines="not_computed", tags=["external", "dependency"], layer="external", ) diff --git a/apps/backend/app/api/routes/analysis.py b/apps/backend/app/api/routes/analysis.py index 9da879a..65d58ab 100644 --- a/apps/backend/app/api/routes/analysis.py +++ b/apps/backend/app/api/routes/analysis.py @@ -137,7 +137,9 @@ def cancel_analysis( response_model=ArchitectureResponse, responses=documented_responses( 200, - "Architecture model derived from repository intelligence.", + "Architecture model derived from the sealed repository-intelligence snapshot " + "bound to the repository's current revision. A repository with no sealed " + "snapshot yet returns 404, never a fallback graph (#217).", { "repositoryId": _REPOSITORY_ID, "repositoryName": "example-service", @@ -147,15 +149,8 @@ def cancel_analysis( "edges": [], "modules": [], "requestFlow": [], - "relationshipSnapshotId": None, - "diagnostics": [ - { - "code": "ARCH-REL-NOT-EXTRACTED", - "category": "relationship extraction", - "severity": "info", - "message": "No sealed repository-intelligence snapshot is available for relationship analysis.", - } - ], + "relationshipSnapshotId": "snap_example", + "diagnostics": [], "summary": { "language": "Python", "framework": "FastAPI", diff --git a/apps/backend/app/reports/builders.py b/apps/backend/app/reports/builders.py index 9394d90..52b92d1 100644 --- a/apps/backend/app/reports/builders.py +++ b/apps/backend/app/reports/builders.py @@ -147,7 +147,6 @@ def build_architecture_document(architecture: ArchitectureResponse) -> ReportDoc fields=[ ("Type", node.type.replace("-", " ")), ("Layer", node.layer.replace("-", " ")), - ("Size Class", node.estimated_complexity), ], bullets=[f"File: {path}" for path in node.files[:10]], ) diff --git a/apps/backend/app/schemas/architecture.py b/apps/backend/app/schemas/architecture.py index 03717e3..d65db57 100644 --- a/apps/backend/app/schemas/architecture.py +++ b/apps/backend/app/schemas/architecture.py @@ -59,8 +59,12 @@ class ArchNode(CamelModel): files: list[str] dependencies: list[str] dependents: list[str] - estimated_complexity: Literal["low", "medium", "high"] - estimated_lines: int + # No repository-intelligence producer measures complexity or line counts + # today (#217): these are never a synthesized guess (e.g. file count * 80). + # A real value can only appear once a named heuristic with a truth class + # backs it; until then every node reports "not_computed" explicitly. + estimated_complexity: Literal["low", "medium", "high", "not_computed"] + estimated_lines: int | Literal["not_computed"] tags: list[str] layer: str parent_module: str | None = None diff --git a/apps/backend/tests/test_architecture_relationships.py b/apps/backend/tests/test_architecture_relationships.py index c7c61d2..9eb7e78 100644 --- a/apps/backend/tests/test_architecture_relationships.py +++ b/apps/backend/tests/test_architecture_relationships.py @@ -413,33 +413,16 @@ def record_evidence_batch(_conn, _cursor, statement, parameters, _context, _exec assert max(evidence_query_parameter_counts) <= ARCHITECTURE_EVIDENCE_BATCH_SIZE + 1 -def test_architecture_without_snapshot_does_not_claim_isolation(auth_client): +def test_architecture_without_a_sealed_snapshot_returns_404(auth_client): # Leave the analysis job queued (worker not drained) so no snapshot is sealed: - # this exercises the genuine "no sealed snapshot" architecture response, which - # durable analysis otherwise reaches only in the window before the worker runs. + # this exercises the genuine "no sealed snapshot" case, which durable analysis + # otherwise reaches only in the window before the worker runs. Architecture must + # 404 the same way Dependencies/Review/Insights already do (#217) rather than + # falling back to a graph built from unsealed repository metadata. repository = _upload( auth_client, {"src/lonely/index.ts": b"export const lonely = 1;\n"}, analyse=False ) response = auth_client.get(f"/analysis/{repository['id']}/architecture") - assert response.status_code == 200 - architecture = response.json() - assert architecture["relationshipSnapshotId"] is None - assert architecture["edges"] == [] - assert architecture["nodes"][0]["relationshipState"] == "not-extracted" - assert architecture["diagnostics"] == [ - { - "code": "ARCH-REL-NOT-EXTRACTED", - "category": "relationship extraction", - "severity": "info", - "message": "No sealed repository-intelligence snapshot is available for relationship analysis.", - "path": None, - "startLine": None, - "endLine": None, - "subjectKey": None, - "objectKey": None, - "details": None, - "nodeIds": None, - } - ] + assert response.status_code == 404 diff --git a/apps/backend/tests/test_ingestion_pipeline.py b/apps/backend/tests/test_ingestion_pipeline.py index 1175e1b..12a2640 100644 --- a/apps/backend/tests/test_ingestion_pipeline.py +++ b/apps/backend/tests/test_ingestion_pipeline.py @@ -137,12 +137,11 @@ def test_analysis_read_endpoints_return_typed_defaults_before_worker_runs(auth_c record = session.get(RepositoryRecord, repository_id) assert "intelligence" not in (record.repo_metadata or {}) + # Architecture is sealed-snapshot-bound (#217), same as Dependencies/Review/ + # Insights: no snapshot yet means 404, never a fallback graph built from + # unsealed repository metadata. architecture_response = auth_client.get(f"/analysis/{repository_id}/architecture") - assert architecture_response.status_code == 200 - architecture = architecture_response.json() - assert architecture["edges"] == [] - assert architecture["relationshipSnapshotId"] is None - assert architecture["diagnostics"][0]["code"] == "ARCH-REL-NOT-EXTRACTED" + assert architecture_response.status_code == 404 # Dependency Graph is sealed-snapshot-bound (#158), same as Review/Insights: # no snapshot yet means 404, not a typed-empty 200. diff --git a/apps/backend/tests/test_legacy_read_model_guard.py b/apps/backend/tests/test_legacy_read_model_guard.py index 4b7df8c..1184928 100644 --- a/apps/backend/tests/test_legacy_read_model_guard.py +++ b/apps/backend/tests/test_legacy_read_model_guard.py @@ -50,3 +50,26 @@ def test_product_code_has_no_legacy_intelligence_read_path(): f"{path.relative_to(PRODUCT_ROOT)}:{node.lineno}: reads repo_metadata.get('intelligence')" ) assert violations == [] + + +#: Analytical consumers per AGENTS.md's canonical-boundary rule: architecture, +#: dependencies, review, insights, AI and exports. `app/services` and +#: `app/workers` are deliberately excluded -- they legitimately read/write +#: `record.file_tree` as part of ingestion (populating the field the sealed +#: snapshot is built from), not as a canonical-source bypass. +ANALYTICAL_CONSUMER_DIRS = ("analysis", "graph", "review", "insights", "ai", "reports") + + +def test_analytical_consumers_never_read_repository_file_tree(): + """Regression guard for Issue #217: no analytical consumer may derive its + output from ``RepositoryRecord.file_tree`` -- a sealed ``ri.v1`` snapshot, + or an explicit missing-snapshot state, is the only allowed source.""" + + violations: list[str] = [] + for consumer_dir in ANALYTICAL_CONSUMER_DIRS: + for path in sorted((PRODUCT_ROOT / consumer_dir).rglob("*.py")): + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + for node in ast.walk(tree): + if isinstance(node, ast.Attribute) and node.attr == "file_tree": + violations.append(f"{path.relative_to(PRODUCT_ROOT)}:{node.lineno}: reads .file_tree") + assert violations == [] diff --git a/apps/backend/tests/test_prototype_journey.py b/apps/backend/tests/test_prototype_journey.py index eaae661..16a1561 100644 --- a/apps/backend/tests/test_prototype_journey.py +++ b/apps/backend/tests/test_prototype_journey.py @@ -204,12 +204,9 @@ def test_journey_reports_honest_state_before_a_snapshot_exists(auth_client): # Enqueue but do not drain the worker: no snapshot is sealed. assert auth_client.post(f"/analysis/{repository_id}/start").status_code == 200 - architecture = auth_client.get(f"/analysis/{repository_id}/architecture").json() - assert architecture["relationshipSnapshotId"] is None - assert any( - diagnostic["code"] == "ARCH-REL-NOT-EXTRACTED" - for diagnostic in architecture["diagnostics"] - ) + # Architecture 404s rather than falling back to a graph built from unsealed + # repository metadata (#217) -- the same honest contract as the manifest below. + assert auth_client.get(f"/analysis/{repository_id}/architecture").status_code == 404 explanation = auth_client.get( f"/analysis/{repository_id}/architecture/authentication" diff --git a/apps/backend/tests/test_rate_limit.py b/apps/backend/tests/test_rate_limit.py index ff35341..798fa29 100644 --- a/apps/backend/tests/test_rate_limit.py +++ b/apps/backend/tests/test_rate_limit.py @@ -269,12 +269,13 @@ def test_export_endpoint_is_charged_against_the_heavy_budget(limited_client): to the (looser) default class this would never hit 429.""" repository_id = _import_sample(limited_client) # 1st heavy hit - # "architecture" is used here (not "dependencies") because this repository - # was only imported, never analysed, and Dependency Graph is now - # sealed-snapshot-bound (#158) — it would 404 rather than exercise the - # rate-limit budget this test actually targets. + # Every export target is sealed-snapshot-bound (Dependencies since #158, + # Architecture since #217), and this repository was only imported, never + # analysed — so the export 404s. That still proves the request reached the + # route handler (past the rate limiter), which is all this regression needs; + # only a 429 would mean the budget wasn't charged. payload = {"repositoryId": repository_id, "target": "architecture", "format": "json"} - assert limited_client.post("/export", json=payload).status_code == 200 # 2nd heavy hit + assert limited_client.post("/export", json=payload).status_code == 404 # 2nd heavy hit blocked = limited_client.post("/export", json=payload) # 3rd heavy hit assert blocked.status_code == 429 diff --git a/apps/frontend/src/app/pages/ArchitecturePage.tsx b/apps/frontend/src/app/pages/ArchitecturePage.tsx index 16e5a51..a4cbcd2 100644 --- a/apps/frontend/src/app/pages/ArchitecturePage.tsx +++ b/apps/frontend/src/app/pages/ArchitecturePage.tsx @@ -41,32 +41,54 @@ export function ArchitecturePage() { ); } - if (!architecture.model) { + if (architecture.loading) { return (
{architecture.error}
- -{architecture.error}
+{node.description}
+ {/* Complexity/line-count stats were removed (#217): no repository- + intelligence producer measures either, so there was no real value + to show. */}