feat(architecture): show a repository's real modules and what they define - #450
Closed
parthrohit22 wants to merge 2 commits into
Closed
parthrohit22 wants to merge 2 commits into
parthrohit22 wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
parthrohit22
force-pushed
the
feat/445-architecture-shows-real-modules
branch
from
September 11, 2026 17:17
231b141 to
a33cc71
Compare
This was referenced Sep 11, 2026
parthrohit22
added this pull request to stack #455
September 11, 2026 18:22
…fine The engine was already right and the view threw it away. On pallets/click, a 177-file library with 1,963 observed symbols, the Architecture view rendered one node -- "Click", "Owns utility concerns", "derived from repository intelligence at /src/click" -- which is a directory listing with extra steps, not an explanation of the system behind the code. Three things changed, none of them an inference: A file that defines symbols is now a module in its own right. The old grouping collapsed everything under `src/<package>/` into one box, so a single-package repository was reduced to a single node. Files that define nothing (documentation, tests, config) keep their existing role grouping. Descriptions state what the snapshot observed instead of restating the path: how many symbols the module defines and which ones a reader would recognise it by. Responsibilities report the classified role where there was one, and say nothing where there was not -- "Owns unknown concerns" was a statement about the classifier, dressed up as a statement about the code. Symbols reach the analyzer through a key-only query. The documented bound on architecture_facts deliberately excludes symbol rows because they dominate a large snapshot; stable keys are short strings and already carry the file and the qualified name, which is all the module inventory reads. Only top-level definitions count. A method is defined by its class, not by the module, and counting every one turns "what does this module define" into a line-count proxy -- the kind of synthesized measure #217 rules out. Names that would collide take on as much of their parent path as they need (FastAPI has four modules called `utils`); unique names are left short. On click: core 38 symbols, exceptions 22 including Abort and BadArgumentUsage, decorators 18 including command and group, parser 17. On fastapi: 3,139 files, 358 nodes, `applications` reads "Defines 1 symbol: FastAPI." Closes #445
`.first()` picked whichever node the graph happened to order first, so the assertion silently re-pointed at the README module as soon as module ids changed -- it was asserting on position while claiming to assert on a name. The fixture's three modules still carry long names (`customer-subscription-entitlement-orchestration/component`), so the case under test is unchanged; the test now locates that node by its accessible name and additionally checks the on-screen label really is overflowing, which is the half of "truncated visually and recoverable accessibly" that was never actually verified.
parthrohit22
force-pushed
the
feat/445-architecture-shows-real-modules
branch
from
September 11, 2026 18:35
3a43680 to
3b71edc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #445.
What was wrong
The engine was already right; the view discarded it. On
pallets/click— 177 files, 1,963 observed symbols,truthClass: observed— the Architecture view rendered the entire library as one node:{ "id": "module:Click", "name": "Click", "description": "Click derived from repository intelligence at /src/click.", "responsibilities": ["Owns utility concerns"] }What it does now
A file that defines symbols is a module. The old rule grouped everything under
src/<package>/into one id, so a single-package repository became a single box. Files that define nothing — documentation, tests, config — keep their existing role grouping, so nothing else in the view moves.Descriptions state observed facts. Symbol count plus the symbols a reader would recognise the module by. Every clause traces to a fact already sealed in the snapshot; nothing infers what a module is for.
Responsibilities stop asserting ignorance. "Owns unknown concerns" was a statement about the classifier wearing the costume of a statement about the code. Where a role was classified it is reported; where it was not, the entry is omitted.
Colliding names get qualified. FastAPI has four modules called
utils; they now readopenapi/utils,security/utils, and so on. Unique names stay short.Verified live
pallets/click:coreexceptionsdecoratorsparserThat ranking is the library as anyone who knows it would describe it.
fastapi: 3,139 files → 358 nodes (the documented bound holds), andapplicationsreads "Defines 1 symbol: FastAPI."Design notes
Symbols reach the analyzer through a key-only query.
architecture_factsdeliberately excludes symbol rows because they dominate a large snapshot. Stable keys are short strings and already carry<path>::<qualified>— all the module inventory reads — so the bound that matters is preserved.Only top-level definitions count. A method is defined by its class, not by the module. Counting every method turns "what does this module define" into a line-count proxy, which is the synthesized measure #217 rules out.
One acceptance criterion deliberately not met
I left
estimatedLines/estimatedComplexityas they are, and think they should stay:RiEvidence.logical_line_count, which is documented as internal validation metadata — producer-supplied, deliberately excluded from the canonical evidence record and the graph hash. Two equivalent graphs may carry different values. Promoting it to a displayed number would be exactly the unsourced claim this product refuses to make.not_computedis the same honest-limits posture asnot_assessedelsewhere: it distinguishes "nothing measures this" from "the field is missing." The UI already renders nothing fornot_computed(ArchitectureNode.tsx:128), so there is no user-visible noise — only JSON.Happy to strip them in a follow-up if you'd rather; it touches the schema, the report builders,
layout.tsand the generated contract, so it doesn't belong in this PR either way.Tests
Full backend suite green (
pytest -q, 13 skipped, 0 failed);ruff checkandruff format --checkclean.tests/test_architecture_relationships.pyfixture ids updated to the new per-file module ids. Response shape is unchanged, so the generated frontend contract needs no regeneration.