fix(suggest-packages): name a member by its path under the package (#446)

[zaebee]

Addresses the rendering half of #446.

The bug

Members were rendered as their last FQN segment, so p/sub/ and p/sub/sub.py both printed as sub — in two different communities. The report tells a maintainer to split a package, and they cannot tell which sub belongs where. The same collision made a bridge endpoint name the wrong file.

cgis's own tree has one: query/drift/ holding query/drift/drift.py. pkg/sub/sub.py is an ordinary Python layout.

Members are now named by their path under the analysed prefix — unique because full FQNs are, and no longer than the ambiguity requires:

before                          after
  6 │ drift                       6 │ drift
  7 │ drift, drift_service…       7 │ drift.drift, drift.drift_service…

A correction to the issue as I filed it

I wrote that the partition was non-disjoint and modularity therefore meaningless. That was wrong. build_file_graph keys on full FQNs — 27 files, 27 distinct on cgis.query — so Q = 0.492 was computed over a proper partition all along. The duplication lived one layer away, in a display function.

I inferred the computation from its rendering without reading it. Corrected on the issue, and recorded here because the PR would otherwise carry the same false claim.

What the fix made visible

Readable names expose the other half of #446 rather than hiding it. analysis now plainly sits alone in one community while analysis.analyzer, analysis.anomaly and analysis.health sit in another — a sub-package rendered as a peer of its own contents. That is a modelling question (should membership be the package's direct children?), it moves Q legitimately, and it stays open for a decision taken with numbers on the #179 corpus.

Verification

• Two tests written first and watched failing. The second was rewritten twice: it first contained an or True that could not fail, and then passed under the old rendering for the wrong reason — sub happened to be a valid member name. It now asserts that the bridge touching p.sub.sub names it sub.sub, and was checked red against the old rendering and green against the new.
• make format && make lint && make type-check && make doc-coverage — pass.
• Full suite: 2371 passed, 2 skipped, 1 xfailed.

🤖 Generated with Claude Code

https://claude.ai/code/session_018Z1HtHgNsrKkQoc7wN6xeG

[gemini-code-assist]

Code Review

This pull request replaces the _leaf function with _member_name to prevent name collisions in package reports when multiple files or sub-packages share the same trailing segment, and adds corresponding unit tests. The reviewer identified a potential name collision when the analyzed package itself is a node in the graph (i.e., fqn == prefix), which would cause _member_name to return the same name as a sub-module of the same name (e.g., p and p.p). A code suggestion was provided to explicitly handle fqn == prefix by returning "init".

[gemini-code-assist]

When the analyzed package itself is a node in the graph (i.e., fqn == prefix, representing the package's __init__.py file), fqn.startswith(f"{prefix}.") is False. As a result, _member_name falls back to returning the last segment of the prefix (e.g., "drift" for "cgis.query.drift").

However, if there is also a sub-module with the same name as the package (e.g., "cgis.query.drift.drift", which is extremely common in Python layouts), _member_name will return "drift" for it as well. This creates a name collision between the package root and the sub-module, which defeats the purpose of this PR.

To prevent this collision, we should explicitly handle the case where fqn == prefix and return "__init__" (or another distinct placeholder) to represent the package root.

Here is a test case that demonstrates this collision:

def test_package_root_and_submodule_collision(tmp_path: Path) -> None:
    # p is the package root (e.g., representing p/__init__.py)
    # p.p is a module inside it (e.g., representing p/p.py)
    files = [make_file_node("p"), make_file_node("p.p")]
    edges = [make_import_edge("p", "p.p")]
    db = _store_with(tmp_path, files, edges)
    report = suggest_packages(db, prefix="p", with_calls=False)
    
    rendered = [f for community in report.communities for f in community.files]
    assert len(rendered) == len(set(rendered)), f"Colliding names found: {rendered}"
    assert "__init__" in rendered
    assert "p" in rendered

Suggested change

	if not prefix or not fqn.startswith(f"{prefix}."):
	return fqn.rsplit(".", 1)[-1]
	return fqn[len(prefix) + 1 :]
	if fqn == prefix:
	return "__init__"
	if not prefix or not fqn.startswith(f"{prefix}."):
	return fqn.rsplit(".", 1)[-1]
	return fqn[len(prefix) + 1 :]

[zaebee]

Taken, and it is a fair comment on my own fix: the same bug, one level down.

Naming members relative to the prefix left the analysed package's own node — where fqn == prefix — falling through to the last-segment fallback. Confirmed on real data rather than the constructed case:

cgis suggest-packages cgis.query.drift
  members: ['_scc', 'drift', 'drift', 'drift_service', 'fingerprint', ...]
  duplicated: ['drift']

drift/__init__.py and drift/drift.py, exactly as you describe. So the first fix closed the collision when you analyse the parent and reopened it when you analyse the package itself.

__init__ is the right name for it, and for a reason worth writing down: it cannot clash. A nested __init__.py has its node id folded into its own package's name (query/analysis/__init__.py → cgis.query.analysis → renders analysis), so only the root ever produces __init__. After the fix:

  members: ['__init__', '_scc', 'drift', 'drift_service', 'fingerprint', 'fractal',
            'ontology_init', 'quotient', 'triads']
  duplicated: []

Suite 2372 passed, 2 skipped, 1 xfailed.

[zaebee]

Both taken. The first one made it clear that my previous two attempts had each closed one shape and left another — so the fix now decides uniqueness over the whole set instead of arguing it per name.

__init__ was wrong twice. You are right on both halves, and the second is the one that matters: this tool is MCP-facing, so prefix + "." + name == node id is the contract an agent depends on to read a community, pick a member and ask about it. prefix + ".__init__" names nothing, leaving one unresolvable member in every report. And the Python-only filename is a real problem for a tool that reads FILE nodes in any language — the TypeScript extractor folds /index exactly as the Python one folds /__init__.

Rendering the root as its own FQN — my first replacement — was also wrong, and I caught it only because I ran it: with prefix p, the root renders p and a module p.p renders p too. Third shape of the same collision.

That is what changed the approach. _member_names now takes the whole file set: relative paths under the prefix, the package's own node by its FQN, and if those still clash — possible only when a module is named exactly after its package — the entire report falls back to full FQNs. Degrading the whole table keeps one rule visible in the output, rather than one row spelled unlike its neighbours for a reason the reader cannot see. The property is structural now, so it is testable directly instead of case by case.

The stale contract — Community and Bridge docstrings plus docs/specs/2026-06-13-suggest-packages-design.md:226 — is corrected. Worth flagging that those had been false since my first commit on this branch, not before it: I changed the emitted values and left the three places that describe them.

Dogfooded at HEAD: cgis.query 27/27 distinct, cgis.query.drift 9/9, cgis.guardian 26/26, no duplicates. Suite 2373 passed, 2 skipped, 1 xfailed.

[zaebee]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates the package suggestion service to use unique display names (paths under the analyzed package or full FQNs) instead of just the last FQN segment, preventing name collisions in communities and bridges. It introduces the _member_names helper function to resolve these names uniquely and adds comprehensive unit tests to verify the behavior. The review feedback suggests simplifying the nested ternary conditional expression in _member_names by handling the empty prefix case upfront, which would significantly improve readability.

[gemini-code-assist]

The dictionary comprehension in _member_names uses a nested ternary conditional expression (fid if fid == prefix else fid[len(prefix) + 1 :] if prefix else fid) which is difficult to read and contains a redundant check on prefix for every element. Since prefix is constant for the function call, we can handle the empty/falsy prefix case upfront. This simplifies the comprehension significantly and improves readability.

    if not prefix:
        return {fid: fid for fid in file_ids}

    relative = {
        fid: fid if fid == prefix else fid[len(prefix) + 1 :]
        for fid in file_ids
    }
    if len(set(relative.values())) == len(relative):
        return relative
    return {fid: fid for fid in file_ids}

[zaebee]

Taken — and this round is readability only, no correctness findings, which after a round that turned up two real ones is the convergence signal.

The nested ternary did re-test prefix for every element though it is constant for the call. Lifted out, and the early return lets the empty-prefix case and the collision fallback share one absolute map instead of building it twice.

Behaviour checked rather than asserted: 2373 passed, and the dogfood numbers are unchanged — cgis.query 27/27 distinct, cgis.query.drift 9/9, cgis.guardian 26/26, no duplicates.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
85.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud