Skip to content

feat(query): cgis suggest-packages — graph-driven sub-package suggestions (#242 slice 1)#245

Merged
zaebee merged 16 commits into
mainfrom
feat/issue-242-suggest-packages
Jun 14, 2026
Merged

feat(query): cgis suggest-packages — graph-driven sub-package suggestions (#242 slice 1)#245
zaebee merged 16 commits into
mainfrom
feat/issue-242-suggest-packages

Conversation

@zaebee

@zaebee zaebee commented Jun 13, 2026

Copy link
Copy Markdown
Owner

Summary

Slice 1 of RFC #242: a new analysis cgis suggest-packages <prefix> that turns "this package feels messy" into a data-backed verdict. For a package, it builds the intra-package file graph, detects dependency communities (greedy Clauset-Newman-Moore modularity Q, no NetworkX), measures how far the directory layout diverges from those communities (1 − NMI), and emits a Q∧D∧direction-gated verdict. Surfaced via CLI and MCP. Report-only — no --apply, no naming map (those are out of scope / slice 2).

This is the domain-quotient machinery (quotient.py) one scale down: community detection over a package's own import graph instead of the cross-domain graph.

What's in it

  • src/cgis/query/cohesion.py (pure, no I/O): build_file_graph (root-agnostic — reconciles import targets to internal files by FQN suffix), greedy_modularity (deterministic CNM), partition_divergence (1 − NMI), layout_direction, classify_verdict.
  • src/cgis/query/suggest_service.py: suggest_packages(db, prefix, with_calls, min_q) → SuggestReport; mirror of drift_service.py.
  • CLI cgis suggest-packages <prefix> [--db --with-calls --format text|json --min-q]; MCP cgis_suggest_packages.

Verdicts (gated on BOTH signals)

Q = "the structure is real"; D = "the layout disagrees". direction (|P_dir| vs |P_comm|) disambiguates:

meaning
split strong Q, divergent, flatter layout (under-split)
consolidate strong Q, divergent, finer layout (over-split)
aligned structure exists, dirs already express it
leave no real structure — flat is fine
borderline weak-Q band, or matched layout
no_signal nothing to measure (with a diagnostic note)

Two load-bearing design points (from spec review)

  • Root-agnostic (the 🔴 fix). Verified empirically: ingesting src/cgis yields file nodes query.* but import targets cgis.query.* (EXTERNAL placeholders) → a naive file_path collapse finds zero internal edges and silently reports no_signal on the exact package the tool exists for. build_file_graph reconciles by dot-boundary FQN suffix (reusing feat(mcp): expose drift/validate as MCP tools + fuzzy FQN lookup #145's machinery). A genuinely mis-rooted graph gets a distinct diagnostic note, never a silent clean verdict (Drift metric can be gamed by cosmetic re-exports (IMPORTS-layer laundering) #182 anti-silent-failure). Regression test runs both ingest roots → same verdict.
  • Flat-package degeneracy. For a flat package D = 1.0 always (NMI with a trivial partition is 0), so the verdict is driven by Q; the divergence=0.2 threshold only does work on nested packages and is marked provisional, calibrated here on cgis.guardian.

Dogfood (self-parsing)

  • cgis.querysplit (Q=0.475, D=1.0, 21 files, 8 communities) — the tool flags the very package the RFC is about.
  • cgis.guardian (has a real providers/ subpackage) → borderline (Q=0.333, D=0.587) — non-flat layout keeps D below 1.0 and Q below the split threshold.

Self-drift held — the two new files in query/ did not push cgis.query past tolerance, so no re-baseline was needed (RFC risk #2 did not materialize).

Test Plan

  • make format && make lint && make type-check && make pytest && make doc-coverage — all green (1053 passed, mypy strict 60 files, interrogate 99.6%)
  • Unit: cohesion.py (graph build incl. root-agnostic reconciliation, CNM determinism, NMI edge cases, verdict table), suggest_service (verdicts, mis-rooted diagnostic), root-agnostic two-root regression
  • Self-parsing dogfood + nested-package D calibration
  • CLI (text + json) and MCP tool

Built SDD: spec (docs/specs/2026-06-13-suggest-packages-design.md) → plan (docs/specs/plans/2026-06-13-suggest-packages.md) → 10 TDD tasks, each spec + quality reviewed.

Closes part of #242 (slice 1; slices 2 = query/ restructure remains).

🤖 Generated with Claude Code

zaebee and others added 12 commits June 13, 2026 13:10
…ce (#242 slice 1)

Slice 1 of RFC #242: cgis suggest-packages <prefix> — hand-rolled CNM modularity
over the intra-package import graph (--with-calls for combined), 1-NMI divergence
vs current directory layout, Q∧D double-gated verdict (split/borderline/aligned/
leave/no_signal). CLI + MCP. Report-only; --apply and naming map out of scope.

Refs #242

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ration (#242 review)

Colleague review of the design (verified the 🔴 empirically):
- 🔴 build_file_graph reconciles import targets to internal files by FQN suffix
  (#145 resolve_fqn/find_nodes_by_suffix) → root-agnostic; src/ and src/cgis/ ingests
  now yield the same verdict. Mis-rooted/unresolvable case emits a distinct diagnostic
  note, never a silent no_signal (#182 silent-failure theme).
- 🟡 isolated files = singleton communities (counted, Q-neutral); flat-package D=1.0
  degeneracy made explicit; divergence=0.2 marked PROVISIONAL + nested-package
  calibration task (measure-before-fix, like tangle).
- 🟡 NMI is symmetric, verdict is directional → add direction (under/over/matched),
  gate split on under_split, new consolidate verdict for over_split, matched→borderline.
- CNM local-optimum stated as a known approximation, not silent.

Refs #242

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Build order per drift precedent: pure cohesion.py (file graph w/ root-agnostic
suffix reconciliation → CNM modularity → 1-NMI divergence → direction → verdict)
→ suggest_service orchestration + SuggestReport → root-agnostic regression test
→ CLI + MCP → self-parsing dogfood (cgis.query split, guardian nested calibration)
→ self-drift check + conscious re-baseline. All reused APIs verified against code
(save_graph, get_all_*, store.db_path, @mcp.tool, json).

Refs #242

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@zaebee

zaebee commented Jun 13, 2026

Copy link
Copy Markdown
Owner Author

/guardian review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary: Reviewed diff for logic correctness, type safety, and ontology compliance. Found 4 issues: (1) greedy modularity may terminate early due to _MIN_GAIN threshold, (2) isolated files handling is correct but docstring could be clarified, (3) mis-rooted diagnostic may miss unresolved IMPORTS edges, (4) diagnostic message may be misleading for non-IMPORTS edges. No type safety or ontology violations detected.


🤖 mistral-medium-latest · 37,225 prompt + 1,205 completion = 38,430 tokens · graph 4/9 files (44%)

Comment thread src/cgis/query/cohesion.py
Comment thread src/cgis/query/suggest_service.py
Comment thread src/cgis/query/suggest_service.py
github-actions Bot added a commit that referenced this pull request Jun 13, 2026
@zaebee zaebee self-assigned this Jun 13, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the cgis suggest-packages feature, which analyzes package cohesion by detecting dependency communities using a greedy Clauset-Newman-Moore modularity algorithm and measuring directory layout divergence with Normalized Mutual Information. The review feedback focuses on optimizing the community merging loop by precomputing community degrees, fixing a directory grouping bug when a file's fully qualified name exactly matches the package prefix, and adding robust null-safety guards to handle optional or empty prefix parameters across the cohesion and suggestion services.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/cgis/query/cohesion.py
Comment thread src/cgis/query/suggest_service.py
Comment thread src/cgis/query/cohesion.py Outdated
Comment thread src/cgis/query/suggest_service.py
Comment thread src/cgis/query/suggest_service.py
Comment thread src/cgis/query/suggest_service.py Outdated
Comment thread src/cgis/query/suggest_service.py
Comment thread src/cgis/query/suggest_service.py Outdated
Comment thread src/cgis/query/suggest_service.py Outdated
…fixtures (#242)

gemini review (9 inline) + Sonar:
- Normalize prefix ONCE at the suggest_packages boundary: (prefix or "").strip()
  → no_signal "no fqn_prefix given" for None/blank (CLI/MCP clients may send either),
  so every downstream helper gets a clean non-empty str. Collapses the 6-comment
  None-handling cluster into one guard; service + MCP signatures now `str | None`.
- _dir_group: a node whose id == prefix (the package __init__) is now <root>, not
  mis-grouped under the prefix's first segment (gemini correctness fix).
- _best_merge: precompute per-community normalized degree once (O(V)) instead of
  re-summing inside the O(K²) pair scan.
- _under: empty prefix matches nothing.
- Sonar new-code duplication: hoist make_file_node / make_import_edge into
  tests/unit/conftest.py; test_cohesion / test_suggest_service / test_cli now share
  them instead of each re-declaring _file/_imp.
- Test for None/""/"  " prefix → no_signal.

1054 passed, mypy strict, ruff, doc 99.6% — all green.

Refs #242

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@zaebee

zaebee commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

Addressed all review feedback in fe423c6 — see thread replies. Summary: prefix normalized once at the suggest_packages boundary (None/blank → no_signal, signatures now str | None); _dir_group package-node fix; _best_merge degree precompute; test-fixture duplication hoisted to conftest.py (make_file_node/make_import_edge). 1054 passed, mypy strict, ruff, doc 99.6% green.

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request implements the cgis suggest-packages feature (RFC #242, slice 1) to analyze package cohesion by detecting dependency communities and measuring directory layout divergence. It introduces pure logic in cohesion.py, orchestration in suggest_service.py, and integrates the feature into both the CLI and the MCP server, backed by extensive tests. Feedback on the implementation suggests optimizing the greedy modularity algorithm by tracking community degrees incrementally and using sparse neighbor iteration to resolve performance bottlenecks in community merges.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/cgis/query/cohesion.py
…242)

SonarCloud quality gate failed on new_reliability_rating=3: python:S1244
float == checks (the repo's recurring pattern). Source uses <= 0.0 (all the
quantities — e_ij, m2, entropy — are non-negative, so <= is equivalent and
Sonar-clean); weight assertions use pytest.approx. Duplication condition
already passed (0.0%) after the conftest fixture hoist.

Refs #242

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@zaebee

zaebee commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request implements the cgis suggest-packages feature, which detects dependency communities within a package using greedy modularity (Clauset-Newman-Moore) and measures directory layout divergence using normalized mutual information (NMI). The changes include the core logic in cohesion.py, orchestration in suggest_service.py, a CLI command, an MCP tool, and extensive unit and self-parsing tests. The review feedback suggests a performance optimization in the community merge step of cohesion.py to iterate over actual neighbors rather than performing a Cartesian product, reducing complexity on larger packages.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/cgis/query/cohesion.py
gemini round-3: compute community-pair edge weight by iterating c1's actual
neighbours (O(deg), set-membership filter on c2) instead of the |c1|*|c2|
Cartesian dict scan. Same result, no added state. Declined the heavier
incremental-degree-tracking variant (mutable cross-call state, YAGNI at
package scale) — kept this stateless one-comprehension rewrite.

Refs #242

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@zaebee

zaebee commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

Review — cgis suggest-packages (#242 slice 1)

Checked the math line-by-line, reproduced Q by hand, and ran the tool live. Strong implementation: the core is correct and the spec-review 🔴 (root-agnostic suffix reconciliation) is done well — cleaner than the spec promised. Approve with one substantive note + minor nits.

✅ Verified

  • Math is correct. _modularity (within-edge double-count ↔ m2 = 2m), _best_merge (dq = 2·(e_ij − a_i·a_j) — exact Newman ΔQ via the symmetric adj), partition_divergence (both-trivial→0, one-trivial→D=1, NMI≤1), classify_verdict (matches the spec table, no gaps).
  • Suffix reconciliation works and is tested. cgis.query.engine → file query.engine resolves; ambiguous suffix → "" → skipped (never guessed). This makes the src/cgis-root case just work, with the mis-rooted note as a correct fallback.
  • Live dogfood: cgis.query✂️ SPLIT, Q=0.475, D=1.0, under_split; communities coherent; isolated files become singletons. ✓
  • 1054 tests green; test_cohesion covers known-Q synthetics (two cliques→0.5, one clique→0, isolated singletons) + determinism.

🟡 Main note — the D=0.2 threshold isn't actually calibrated (the test gives false confidence)

Ran the guardian calibration target live: Q=0.333, D=0.7626 → borderline. test_cgis_guardian_nested_is_not_split passes, but for the wrong reason: the verdict is borderline because Q=0.333 < 0.35 (the split threshold), not because of divergence. Guardian's D=0.76 is high, so the "nested + LOW D → aligned" case — the one the spec (test #6) said guardian would exercise to validate the 0.2 boundary — is covered by no test. The divergence=0.2 threshold ships unvalidated, while the test reads as if it were validated.

Suggestion:

  • ideally, find a genuinely well-aligned nested package (Q ≥ 0.35, D < 0.2 → aligned) — cgis is mostly flat so there may not be one in-repo (owner-api might have one);
  • or pin the guardian assertions (Q≈0.33, D≈0.76, verdict == borderline) instead of the weak != split, and downgrade the comment honestly: "guardian confirms nested < flat (D=0.76 < 1.0); the aligned boundary at D=0.2 remains uncovered — no low-D nested case found in-repo." Right now the test's docstring claims a calibration it doesn't perform.

🟢 Minor (non-blocking)

  1. no_signal reports file_count=0 even when files exist. _empty_report hardcodes file_count=0/edge_count=0, but it's also used for the mis-rooted/flat case where files were found (the note says so). A JSON consumer keying on file_count reads "empty" and misses the diagnostic. Better to report the real file_count with edge_count=0.
  2. e.type.value == "IMPORTS" (suggest_service.py:131) — string compare vs the EdgeType.IMPORTS enum used everywhere else.
  3. D isn't clamped to [0,1] — fp can yield -1e-16/1.0000001; harmless for the threshold, odd in JSON. A cheap max(0.0, min(1.0, …)).
  4. Dogfood band (0.38–0.48) is close to its ceiling. Real Q=0.475 (the new cohesion/suggest_service files lifted it from ~0.43). As query/ grows toward slice 2, Q drifts toward 0.48 → the test gets fragile. The spec still says "~0.40–0.46" — stale vs the actual 0.475.

Bottom line: core verified correct, the 🔴 fix is in, suite green, the tool catches its own smell live. Before merge I'd fix 🟡 (guardian calibration honesty) — it's a verification gap (the recurring "the test claims more than it proves" theme). Nits are your call. Nice work overall.

— reviewed by Claude (relayed via @zaebee)

…iew #245)

Colleague review — main note was a verification-honesty gap:
- test_cgis_guardian_nested_is_not_split passed for the WRONG reason (Q<0.35,
  not low D), so the D=0.2 'aligned' boundary was uncovered while the test read
  as if calibrated. Now: pin guardian's real numbers (Q≈0.33, D≈0.76, borderline)
  with an explicit honesty note, AND cover the aligned (low-D) boundary
  end-to-end with a synthetic nested-and-aligned test (D≈0 → aligned). Spec
  updated to state the gap (no low-D nested package in-repo).
Minor nits:
- _empty_report now carries the real file_count for the mis-rooted/flat-leaf
  case (was a misleading 0 while the note said files were found).
- e.type == EdgeType.IMPORTS (enum) instead of .value == "IMPORTS" string.
- clamp divergence to [0,1] (float-noise hygiene in JSON).
- widen the cgis.query dogfood Q band to 0.38–0.52 (real 0.475, was stale ~0.40–0.46).

1054+2 tests green, mypy strict, ruff, doc green.

Refs #242

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@zaebee

zaebee commented Jun 14, 2026

Copy link
Copy Markdown
Owner Author

Thanks — the 🟡 was a real verification-honesty gap, fixed in 781cf45.

🟡 D=0.2 not actually calibrated / test gave false confidence — correct. test_cgis_guardian_nested_is_not_split passed because Q=0.33 < 0.35, not because of divergence (D≈0.76 is high), so the aligned (low-D) boundary was uncovered while the docstring implied otherwise. Done both things you suggested:

  • Pinned guardian's real numbers (verdict == "borderline", Q < 0.35, D < 1.0) with an explicit honesty note that it does NOT exercise the D-boundary (it's Q that keeps it non-split).
  • Covered the aligned boundary end-to-end with a synthetic nested-and-aligned package (two cliques whose dirs match the communities → D≈0, matched → aligned) — test_suggest_aligned_nested_is_aligned. cgis has no genuinely low-D nested package in-repo, so the real-data calibration stays flagged provisional (owner-api is the candidate). Spec §calibration + testing updated to say this plainly.

🟢 nits — all taken:

  1. _empty_report now carries the real file_count for the mis-rooted/flat-leaf case (was a misleading 0 while the note said files were found); asserted.
  2. e.type == EdgeType.IMPORTS (enum) instead of .value == "IMPORTS".
  3. divergence clamped to [0,1] (float-noise hygiene).
  4. Widened the cgis.query dogfood band to 0.38–0.52 (real Q=0.475) and fixed the stale ~0.40–0.46 in the spec.

1055 tests green, mypy strict, ruff, Sonar QG passing. Declined only the incremental-degree-tracking perf suggestion (mutable state, YAGNI at package scale); took the stateless sparse-neighbour e_ij rewrite.

@sonarqubecloud

Copy link
Copy Markdown

@zaebee zaebee merged commit 827bd04 into main Jun 14, 2026
3 checks passed
@zaebee zaebee deleted the feat/issue-242-suggest-packages branch June 14, 2026 09:32
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.

1 participant