suggest-packages: two files can render under the same name, and members mix package with module level

[zaebee]

Found by pointing cgis suggest-packages at cgis's own query/ package, which is the tool's strongest structural opinion about its own code — and which turns out to be computed over a broken partition.

What it reports

cgis suggest-packages cgis.query
  ✂️  SPLIT  cgis.query  Q=0.492  divergence=0.319  direction=under_split

with 14 communities over 27 members, 26 of them distinct.

Two defects, one cause

Members are keyed by basename over the whole subtree, not by the package's direct children. cgis/query/ actually contains four sub-packages (analysis/, context/, drift/, render/) and two modules (engine.py, fqn.py).

1. A sub-package is listed as a peer of its own contents. analysis sits in community 0 while analyzer, anomaly, health (community 1) and cohesion, suggest_service (community 2) — every one of them a file inside analysis/ — sit in others. Same for render against its graph_json, mermaid, metrics. The graph being partitioned therefore mixes two levels of the tree.

2. Basenames collide, so the partition is not disjoint. drift appears in two communities:

  6 │ drift
  7 │ drift, drift_service, ontology_init, quotient

because cgis/query/drift/ is a directory and cgis/query/drift/drift.py is a module inside it. Modularity is defined over a partition; computed over a multiset it does not mean what the verdict reads it as.

Scope of the collision in this repository: exactly one occurrence —

find src -type d | while read d; do b=$(basename "$d"); [ -f "$d/$b.py" ] && echo "$d/$b.py"; done
  src/cgis/query/drift/drift.py

— but pkg/sub/sub.py is an ordinary Python layout, and the keying makes any repository using it produce a non-partition. Checked the other packages at 529b2e2: cgis.guardian (26/26), cgis.extractors (10/10), cgis.core (4/4) are clean, so only the nested-package case is affected.

Why it matters

This is the one verdict in the tool that tells a maintainer to restructure their code, and on the only package here that is already subdivided it says under_split — a conclusion drawn from counting a package and its own children as siblings. Acting on it would be acting on a number that does not describe the thing it names.

Shape of a fix

The question "should this package be split" is about its direct children: four sub-packages and two modules for cgis.query, not twenty-seven basenames. Keying members by the path segment directly under the analysed prefix — and treating a sub-package as one node whose edges are the union of its contents' — would make the partition disjoint by construction and put the modularity back on the level the verdict claims to be about.

Acceptance should be that no member appears twice, on this repository and on the #179 dogfooding set, plus a re-read of cgis.query's verdict once the members are its actual children.

Related

The feature shipped in #242 (closed). This is the first time its output was checked against a package containing sub-packages.

[zaebee]

Correction — the central claim of this issue is wrong, and I should not have made it.

I wrote that the partition is not disjoint and that "modularity is computed over a multiset, so it does not mean what the verdict reads it as". That is false. Checked by running the code rather than reading its output:

build_file_graph(nodes, edges, "cgis.query", with_calls=False)
  files in the graph: 27   distinct: 27
  the two that display as 'drift': ['cgis.query.drift', 'cgis.query.drift.drift']

The graph is keyed on full FQNs, so cgis.query.drift (the sub-package's __init__) and cgis.query.drift.drift (the module inside it) are two distinct nodes. Q = 0.492 is computed correctly. The duplication exists only in the rendering: suggest_service.py:68 reduces each member to fqn.rsplit(".", 1)[-1] for display.

I inferred the mechanism from the rendered table without reading build_file_graph. Recording that plainly because it is the fourth time in one session I have done it.

What is actually wrong

1. The display is ambiguous, and on this package actively misleading. Two different files render as drift, in two different communities. A reader is being told to split a package and cannot tell which drift belongs where. Any pkg/sub/sub.py layout hits it. This one is a straightforward fix: render enough of the path to disambiguate — drift/ vs drift/drift, or the segment relative to the analysed prefix.

2. The member set mixes levels — a design question, not a bug. _under(n.id, prefix) admits every descendant, so analysis (a sub-package) is a node alongside analyzer, anomaly, health, which live inside it. That is a real modelling choice and it does move Q, but legitimately: it is answering "how do all these files cluster" rather than "should this package's direct children be regrouped".

Which of those two questions suggest-packages should answer is worth deciding explicitly. For an already-subdivided package like cgis.query, the second reads more useful — a under_split verdict on a package with four sub-packages is confusing precisely because it is answering the first. But that is a decision to take with numbers on the #179 corpus, not a defect to patch.

Revised scope

• The rendering fix is worth doing on its own and is small.
• The granularity question should be measured before it is changed: what each package's verdict becomes under direct-children membership, across the dogfooding set.

Retitling accordingly.

[zaebee]

The rendering half shipped in #447 (08af353); this issue now tracks the granularity question only.

Member names are decided over the whole file set rather than per name: a path under the analysed package, the package's own node by its FQN, and a whole-report fallback to full FQNs in the one case those still clash — a module named exactly after its package. Every name maps back to a node id, which matters because this tool is MCP-facing.

Three shapes of the same collision were closed in sequence, each found only after the previous fix: p/sub/ against p/sub/sub.py; the package's own node against a module named after it; and __init__ as a Python-only name that resolved to nothing in any language. That is the reason the final form makes uniqueness a property of the set instead of an argument about each name.

Correction carried over

This issue first claimed the partition was non-disjoint and the modularity therefore meaningless. That was wrong — build_file_graph keys on full FQNs, 27 files and 27 distinct on cgis.query, so Q = 0.492 was computed correctly all along. I had inferred the computation from its rendering. Corrected in the thread above, and the PR carries the same correction.

What remains open here

Should membership be the package's direct children? Now visible in the output rather than hidden behind collapsed names:

cgis suggest-packages cgis.query
  0 │ analysis
  1 │ analysis.analyzer, analysis.anomaly, analysis.health, drift._scc, drift.fingerprint
  2 │ analysis.cohesion, analysis.suggest_service

analysis — a sub-package — sits as a peer of files inside it. _under(n.id, prefix) admits every descendant, so the graph mixes two levels of the tree. That is a modelling choice, not a defect: it answers "how do all these files cluster" rather than "should this package's direct children be regrouped". For an already-subdivided package the second reads more useful, and a under_split verdict on a package with four sub-packages is confusing precisely because it answers the first.

Deciding it needs numbers, not an argument: each package's verdict under direct-children membership, across the #179 dogfooding set, against what it reports today.