feat(metrics): --exclude <segment> to drop test/vendor code from rankings (#234)

[zaebee]

What

Closes #234. Adds a repeatable --exclude <segment> option to cgis metrics
(CLI -x) and the cgis_metrics MCP tool. It drops any node whose FQN contains
the given dot-segment anywhere, so a single --exclude tests removes both
top-level tests.* and nested domains.*.tests.*, while keeping false-positives
like domains.testservice (substring, not a segment).

Surfaced by dogfooding the metrics layer on Ownima's owner-api (10 368 nodes):
test scaffolding dominated every ranking and buried the real architecture.

Why

cgis metrics ranks the whole graph. On a real backend that means:

Section	Top result before
🔌 Bottlenecks	tests.utils.utils.random_lower_string (in 129)
🏛️ God classes	Test* classes, 8 of top-12
⭐ PageRank	test helpers at #1 and #4

Result on owner-api — --exclude tests

🔌 Coupling bottlenecks
  utils.task_queue.enqueue_task                          in 40
  api.dependencies.ownership.verify_resource_ownership   in 33   ← authz choke point
  domains.reservation.validators.Rule.needs_attention    in 32
  crud.wallet.async_get_wallet_by_user_id                in 30

🏛️ God classes
  services.metrics_service.MetricsService                29
  api.dependencies.clients.base.OpenSearchClient         26
  domains.reservation.service.ReservationService         23

The real load-bearing nodes finally surface.

Design

• Segment match, not anchored prefix — chosen because backend test code lives
  both at tests.* and nested under domains.*.tests.*; a prefix flag would miss
  the nested ones. Matches a whole dot-delimited component ('.' || id || '.' LIKE '%.tests.%').
• Threaded through all three sections; for PageRank, excluded nodes leave the
  propagation graph entirely (cleanest).
• Opt-in — no default exclusion, current behaviour unchanged.
• Injection-safe — segments ride in as bound ? params via LIKE … ESCAPE '\'
  with %/_/\ escaped (a _ in a segment matches literally, never as a wildcard).

Tests

4 new (tests/unit/test_metrics.py): segment match drops tests across all three
sections + keeps the testservice false-positive; opt-in default still shows
tests; LIKE-metacharacter escaping. make format lint type-check pytest doc-coverage all green — 970 tests, mypy strict, doc 99.6%.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces an exclusion feature to the architectural metrics queries, allowing users to filter out specific FQN dot-segments (such as test or vendor scaffolding) from coupling bottlenecks, God classes, and PageRank metrics. This is exposed via the MCP server tool and a new --exclude (-x) CLI option. Feedback on the changes suggests hardening the SQL segment exclusion logic to deduplicate input segments and ignore empty or whitespace-only strings, preventing accidental exclusion of all nodes.

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.

[zaebee]

Code review — --exclude <segment> for cgis metrics (#234, #235)

Cross-lane review (I'm on the drift/ontology lane; this is the metrics layer). Clean, well-scoped PR — I verified the three load-bearing claims against the SQL rather than trusting the description. LGTM, one optional test-gap.

✅ Verified correct

• Injection-safe (the claim that matters most). Segments ride as bound ? params (params.append(f"%.{escaped}.%")), never interpolated; LIKE … ESCAPE '\' with the escape order backslash-first (\→\\, then %→\%, _→\_) — correct, escaping the escape char first avoids double-escaping. test_exclude_segment_escapes_like_metacharacters pins the _-wildcard case.
• Segment match, not substring. ('.' || id || '.') LIKE '%.tests.%' traced by hand: drops tests.utils.x and domains.resv.tests.test_x, keeps domains.testservice.x (.testservice. has no .tests.), and correctly excludes a bare module named exactly tests (.tests. matches). The fixture covers all three.
• PageRank leaves the graph, not just the ranking — this was the claim I most wanted to check. pr_edges inner-joins both endpoints to pr_nodes (JOIN pr_nodes s ON e.source=s.id JOIN pr_nodes d ON e.target=d.id), so an excluded node drops every edge touching it, and the dangling-mass term redistributes over the reduced n. No leaked propagation through excluded nodes, no dangling refs. ✓
• Opt-in (exclude=() → empty fragment → byte-identical behaviour, pinned by test_exclude_is_opt_in_default_keeps_tests) and threaded through all three sections + both CLI and MCP consistently.

🟡 Minor — multi-segment exclusion is correct-by-construction but untested

The repeatable case (-x tests -x vendor) joins clauses with AND (drop if it matches any segment) — correct, but no test exercises two segments at once. A 6-line test (get_coupling_metrics(exclude=["tests", "vendor"]) asserting both are dropped and a third survives) would lock the AND-composition, since "repeatable" is the headline feature.

🟢 Nits (non-blocking, mention-only)

• --exclude "" → pattern %..% matches nothing (no .. in normal FQNs) → silently excludes nothing. Harmless, but a likely fat-finger; a if seg skip in _segment_exclusion would make it explicit.
• LIKE is case-sensitive in DuckDB, so -x tests won't catch Tests.*. Fine for Python convention; worth half a sentence in the --help if TS graphs ever land mixed-case test dirs.

Nice dogfood-driven find — the owner-api before/after (authz choke point surfacing once test scaffolding is dropped) is exactly the motivation that makes the segment-vs-prefix choice obviously right. 🚀

🤖 Generated with Claude Code

[zaebee]

Addressed in 8914cec — thanks both.

@gemini-code-assist (dedupe + skip empty/whitespace): applied your suggestion almost verbatim — _segment_exclusion now iterates dict.fromkeys(segments) and continues on not seg.strip().

One technical correction for the record: an empty segment didn't actually exclude all nodes. The pattern is '.' || id || '.' NOT LIKE ? with param f"%.{seg}.%", so an empty seg yields %..% — requiring a literal .., which no real FQN contains → it was a silent no-op, not an exclude-all. Still worth guarding for clarity, so the change is in.

@zaebee (multi-segment test gap + the --exclude "" nit): added both as tests —

• test_exclude_multiple_segments_drops_any_match locks the AND-composition (-x tests -x vendor drops either, keeps the rest).
• test_exclude_empty_or_whitespace_segment_is_noop pins the guard above.

The case-sensitivity nit I left as-is (Python test dirs are lowercase by convention; a TS-mixed-case follow-up can revisit) — shout if you'd rather fold a LOWER() in now.

19 metric tests (4→6 new), full suite 972 green, ruff + mypy strict clean.

[gemini-code-assist]

Great work on finalizing these changes, @zaebee. The addition of the dict.fromkeys deduplication and the explicit whitespace guard in _segment_exclusion makes the implementation much more robust. It's also excellent to see the multi-segment and empty-segment edge cases now covered by dedicated tests. The PR is in great shape.

[zaebee]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces an exclusion feature (--exclude / -x) to filter out specific FQNs (such as test or vendor scaffolding) from architectural metrics, including coupling bottlenecks, God classes, and PageRank. The feedback suggests adding a guard clause to handle None values gracefully in the segment exclusion helper to prevent a potential TypeError.

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.

[zaebee]

@gemini-code-assist applied in 88e2274 — added the if not segments: return "", [] early-return.

Rigor note: the None → TypeError path isn't actually reachable today — _segment_exclusion's parameter is typed Sequence[str] (not | None), the CLI passes [], and the MCP tool already normalises with exclude or [] before the call. So this is belt-and-suspenders against a future mistyped caller. It earns its place anyway because it also short-circuits the reachable empty-sequence case (the default () every non-excluding call uses), which is covered by the existing tests.

Full suite green, ruff + mypy strict clean.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[gemini-code-assist]

The implementation looks excellent, @zaebee. The addition of the early-return guard in _segment_exclusion provides a clean, defensive approach that handles both the default empty-sequence case and any potential future misuse, while keeping the logic robust and efficient. With the test suite fully green and the quality gate passed, this feature is in great shape.

[zaebee]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces an exclusion feature to the architectural metrics tool, allowing users to filter out specific FQNs (such as test or vendor scaffolding) by dot-segments. This is implemented across the MCP server tool, the CLI, and the underlying DuckDB analyzer queries with proper SQL escaping to avoid injection and wildcard issues. Comprehensive unit tests have been added to verify the filtering behavior. I have no feedback to provide as there are no review comments.

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.