fix(suggest): connectivity guard — sparse package no longer false-'split'#257
Conversation
…lit' Modularity Q is unreliable on a near-disconnected package: when most files have no intra-package edge, a single tiny cluster holds all the edges and inflates Q, producing a false 'split' on what is really a bag of independent helpers. Found cross-repo dogfooding owner-api/utils: 12 files, 3 edges, 58% isolated → Q=0.44 → 'split', when it's the textbook "independent helpers, nothing to split". Add a connected-fraction guard: the fraction of files with >=1 intra-package edge must clear thresholds["min_connected"] (0.5) before a split/borderline verdict is trusted; below it the verdict is downgraded to 'leave' with a note explaining the sparse-graph artifact. New `connected_fraction` field on the report makes the signal visible (CLI/MCP JSON). Threshold 0.5 calibrated on owner-api + cgis: utils 0.42 (suppress) vs cgis.query 0.68 / owner-api.core 0.90 / cgis.guardian 0.89 (keep) — a clean empty gap 0.42→0.68. Regression test (sparse → leave) + guard-does-not-over-suppress test (dense → still split). Refs #242 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
/guardian review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary: Checked for logic correctness (division by zero, edge cases), type safety, and test coverage. Found critical division-by-zero risk, major logic bug in connected_fraction calculation, and minor issues with float comparisons and threshold key access. No ontology or library contract violations detected.
🤖 mistral-medium-latest · 6,757 prompt + 1,586 completion = 8,343 tokens · graph 2/3 files (67%)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a sparse-graph guard to prevent unreliable package splits on near-disconnected graphs where modularity Q is artificially inflated by a single small cluster. It adds a min_connected threshold (default 0.5) and calculates the fraction of coupled files (connected_fraction). If the fraction is below the threshold, a "split" or "borderline" verdict is downgraded to "leave". Tests are added to verify this behavior. The reviewer suggested simplifying the calculation of connected_fraction to len(graph.adj) / len(graph.files) to improve performance and readability.
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.
…review) gemini: adj keys are exactly the connected files (build_file_graph never creates empty entries), so len(graph.adj)/len(files) is equivalent and O(1). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a sparse-graph guard to prevent false "split" or "borderline" package suggestions when modularity Q is inflated by a small cluster in an otherwise disconnected package. It adds a min_connected threshold (defaulting to 0.5), calculates the fraction of coupled files, and downgrades the verdict to "leave" with an explanatory note if the connectivity is below the threshold. Unit tests are also added to verify this behavior. The review feedback suggests enhancing the diagnostic note by including the exact counts of coupled and total files to avoid rounding confusion near the threshold.
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.
gemini: include len(adj)/len(files) alongside the % so a 0.499→'50%' rounding near the threshold isn't confusing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
1 participant
Summary
Modularity Q is unreliable on a near-disconnected package: when most files have no intra-package edge, a single tiny cluster holds all the edges and inflates Q, producing a false
spliton what is really a bag of independent helpers.Found cross-repo, dogfooding
owner-api/utilsthrough the MCP tool: 12 files, only 3 internal edges, 58% of files isolated → Q=0.44 →split— the textbook "independent helpers, nothing to split" (the very counter-example the cohesion RFC used).Fix
A connected-fraction guard: the fraction of files with ≥1 intra-package edge must clear
thresholds["min_connected"](0.5) before asplit/borderlineverdict is trusted. Below it, the verdict is downgraded toleavewith a note explaining the sparse-graph artifact. A newconnected_fractionfield on the report makes the signal visible in CLI/MCP JSON.Threshold = 0.5 (calibrated, not invented)
Measured connected-fraction on owner-api + cgis for every
split/borderlinepackage — a clean empty gap separates the artifact from the real splits:0.5= "a majority of files must be coupled" — suppresses utils (0.42) with margin, keeps every legit split (≥0.68). Same "measure before fixing the threshold" discipline as the divergence gate.Test Plan
leavewith the sparse notesplitowner-api/utils→ nowleave;api.dependenciesunaffectedmake format && lint && type-check && pytest && doc-coverage— green (1064 passed, mypy strict 64 files, interrogate 99.6%)Follow-up to RFC #242, found by cross-repo validation.
🤖 Generated with Claude Code