Skip to content

docs: rewrite ARCHITECTURE.md's module table from the real surface, with a test that keeps it honest - #2646

Open
rajarshidattapy wants to merge 1 commit into
Graphify-Labs:v8from
rajarshidattapy:fix/architecture-md-module-table
Open

docs: rewrite ARCHITECTURE.md's module table from the real surface, with a test that keeps it honest#2646
rajarshidattapy wants to merge 1 commit into
Graphify-Labs:v8from
rajarshidattapy:fix/architecture-md-module-table

Conversation

@rajarshidattapy

Copy link
Copy Markdown
Contributor

Fixes #2640.

The bug

The "Module responsibilities" table is where library users and agents start (AGENTS.md points
agents at the docs before the code). Six of its sixteen documented functions did not exist:

MISSING graphify.detect.collect_files
MISSING graphify.build.build_graph
MISSING graphify.analyze.analyze
MISSING graphify.report.render_report
MISSING graphify.export.export
MISSING graphify.serve.start_server

and extract() was given as extract(path) — "file path → dict" — for a function whose first
parameter is a list, so following the doc raises TypeError: 'WindowsPath' object is not iterable.

Fix

Rewrote the table from the actual public surface, introspected rather than guessed
(inspect.signature over each module's public functions). All six mappings applied as the
issue specifies, with two amendments — both verified:

  • to_wiki lives in graphify.wiki, not graphify.export. It gets its own row.
  • serve.py's real entry points are serve(graph_path) and
    serve_http(graph_path, *, host, port, ...).

Four more errors in the same table

hasattr only proves a name exists, so the issue's repro couldn't catch these. Each row's
described behaviour was wrong too:

row claimed actual
cluster "graph with community attr on each node" returns {community_id: [node_id]}; the graph is not mutated
validate "extraction dict → raises on schema errors" validate_extraction returns list[str] of errors; assert_valid is the one that raises
watch watch(root, flag_path) watch(watch_path, debounce=3.0) — there is no flag_path parameter
detect "directory → [Path] filtered list" detect(root) returns a scan-summary dict: files by category, total_files, total_words, warning, scan_root

Also corrected build_graph() in the pipeline diagram and the schema section, and softened
"Each stage is a single function in its own module" — no longer true of analyze.py or
export.py.

On the root= parameter

The issue is right that this is the load-bearing part: a caller following the old doc writes
extract(paths) with no root and silently gets non-canonical source_file values and
absolute path segments in node ids. A parameter listed in a table is easy to skim past, so
rather than only fixing the signature there is now a short "Calling extract() from your
own code"
subsection with a worked example and an explicit "Always pass root", spelling
out that the inferred root is the common parent of the list you passed, not your project root.

Tests

tests/test_architecture_doc.py implements the issue's suggestion, parsing the table rather
than restating it — so a new row extends coverage for free, and renaming a function in the
code fails here until the doc is updated:

@pytest.mark.parametrize("module,func", _documented_symbols())
def test_architecture_table_symbols_exist(module, func):
    assert hasattr(importlib.import_module(module), func)

Plus test_architecture_documents_extract_as_taking_a_list and
test_architecture_tells_library_callers_to_pass_root (asserting against the live signature,
not just the prose), and test_the_table_was_actually_parsed — a guard on the parser itself,
since a regex that silently matches nothing would make every parametrized case vacuously pass.

Verified against the pre-fix doc: exactly the six reported symbols fail, plus both extract()
signature tests. 38 tests pass on the new doc. Full suite shows the same 42 pre-existing
failures as baseline (Windows/env), none new.

One change beyond the reported scope

Step 1 of "Adding a new language extractor" says to add the function in extract.py, but
graphify/extractors/MIGRATION.md documents an in-progress split to one module per language.
Repointed step 1 at graphify/extractors/ and noted that extract.py re-exports during the
migration. Adjacent to the reported defect rather than part of it — happy to drop this hunk if
the split isn't meant to be advertised yet.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).


Graphify review — findings

This PR updates ARCHITECTURE.md to align the module responsibilities documentation with the actual codebase, correcting function names/signatures (e.g., renaming stages like build_graphbuild, report()report.generate()), replacing several documented-but-nonexistent entry points, and expanding the table with modules like wiki.py and additional per-format export functions. It also adds a new "Calling extract() from your own code" section documenting the list-of-paths signature and the root keyword argument. It adds a new test file, tests/test_architecture_doc.py, which parses the module responsibilities table and asserts that every documented symbol actually exists, that extract() takes a list parameter, and that the doc instructs callers to pass root. The surface area is limited to documentation and a new test file; no library source code is changed.

No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 21 functions depend on the 21 functions this change touches.

Health — grade A; no new coupling hotspots.

Verification — 21 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 21 function(s) in the blast radius were not formally verified this run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ARCHITECTURE.md documents six functions that do not exist and the wrong signature for extract()

1 participant