refactor(wizard): split integration health validators into grouped modules by 7vignesh · Pull Request #958 · Tracer-Cloud/opensre

7vignesh · 2026-04-25T21:26:30Z

Fixes #873

Describe the changes you have made in this PR -

This PR refactors app/cli/wizard/integration_health.py into smaller, grouped validator modules while preserving the stable public import surface.

What changed:

Split validators into focused modules under app/cli/wizard/integration_validators/:
- client_validators.py — 11 validators backed by service clients (Grafana, Datadog, Honeycomb, Coralogix, AWS, Sentry, GitLab, Vercel, Alertmanager, OpsGenie, BetterStack)
- http_probe_validators.py — 5 HTTP-based validators (Slack webhook, Notion, Jira, Google Docs, Discord)
- mcp_validators.py — 2 MCP validators (GitHub MCP, OpenClaw)
- shared.py — IntegrationHealthResult dataclass
- __init__.py — Module marker
Kept app/cli/wizard/integration_health.py as the stable public API by re-exporting all validators and the result type, so external imports remain unchanged.
Updated test monkeypatch targets to point at new implementation modules while maintaining backward-compatible public function imports in tests.
Added compatibility test that verifies validators are still accessible through the legacy public module path.

Test updates:

Updated 31 existing tests to patch implementation symbols in new modules
All tests still import public functions from app.cli.wizard.integration_health
Added explicit compatibility check for legacy public import surface

No validator behavior changed; this is a pure refactoring for maintainability.

Demo/Screenshot for feature changes and bug fixes -

Validation run for this change:

✅ python -m pytest tests/cli/wizard/test_integration_health.py — 31 passed
✅ python -m ruff check app/cli/wizard/integration_health.py app/cli/wizard/integration_validators tests/cli/wizard/test_integration_health.py
✅ python -m ruff format --check app/cli/wizard/integration_health.py app/cli/wizard/integration_validators tests/cli/wizard/test_integration_health.py
✅ python -m mypy app/cli/wizard/integration_health.py app/cli/wizard/integration_validators (no issues)

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

Yes, I used AI assistance (GitHub Copilot)

If you used AI assistance:

I have reviewed every single line of the AI-generated code
I can explain the purpose and logic of each function/component I added
I have tested edge cases and understand how the code handles them
I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:

The refactoring grouped validators by their implementation style (client-backed, HTTP-probe, MCP) for clarity and separation of concerns, while maintaining a stable re-export surface so that all external callers and test imports remain unaffected. The monkeypatch paths were updated to target the actual implementation modules rather than the re-export surface, ensuring test isolation and precise mocking.

Checklist before requesting a review

I have added proper PR title and linked to the issue
I have performed a self-review of my code
I can explain the purpose of every function, class, and logic block I added
I understand why my changes work and have tested them thoroughly
I have considered potential edge cases and how my code handles them
If it is a core feature, I have added thorough tests
My code follows the project's style guidelines and conventions

greptile-apps · 2026-04-25T21:29:13Z

Greptile Summary

This PR splits the monolithic integration_health.py (632 lines) into four focused sub-modules under app/cli/wizard/integration_validators/, while keeping the original file as a stable re-export facade with a full __all__. All 31 existing tests pass after updating monkeypatch targets to the new implementation paths.

One point worth noting: validate_slack_webhook was quietly switched from requests to httpx (exception type changed from requests.RequestException to httpx.RequestError), which contradicts the PR's claim of a pure refactoring. The added compatibility test also only verifies one of the 18 public symbols.

Confidence Score: 5/5

Safe to merge; all findings are P2 style/documentation concerns with no runtime impact.

The refactoring is structurally sound — the public API surface is fully preserved, all tests pass, and the module split follows clear grouping logic. The two findings (undocumented library swap in validate_slack_webhook and weak compatibility test) are P2 quality suggestions that do not affect correctness or production behavior.

app/cli/wizard/integration_validators/http_probe_validators.py (undisclosed requests→httpx swap) and tests/cli/wizard/test_integration_health.py (compatibility test coverage).

Important Files Changed

Filename	Overview
app/cli/wizard/integration_health.py	Reduced to a thin re-export facade; all 18 public symbols preserved in all, backward compatibility maintained.
app/cli/wizard/integration_validators/client_validators.py	11 client-backed validators extracted verbatim from the original module; logic is unchanged.
app/cli/wizard/integration_validators/http_probe_validators.py	5 HTTP validators extracted; validate_slack_webhook silently switched from requests to httpx, contradicting the "pure refactoring" claim in the PR description.
app/cli/wizard/integration_validators/mcp_validators.py	2 MCP validators extracted unchanged; logic is identical to the original.
app/cli/wizard/integration_validators/shared.py	IntegrationHealthResult dataclass extracted to shared.py; clean separation with a necessary dependency on GitHubMCPValidationResult.
tests/cli/wizard/test_integration_health.py	31 monkeypatch targets correctly updated to new implementation modules; compatibility test added but only covers 1 of 18 public exports.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["app.cli.wizard.integration_health\n(stable public facade / __all__)"]

    A --> B["integration_validators/\nclient_validators.py\n(Grafana, Datadog, Honeycomb,\nCoralogix, AWS, Sentry,\nGitLab, Vercel, Alertmanager,\nOpsGenie, BetterStack)"]

    A --> C["integration_validators/\nhttp_probe_validators.py\n(Slack, Notion, Jira,\nGoogle Docs, Discord)"]

    A --> D["integration_validators/\nmcp_validators.py\n(GitHub MCP, OpenClaw)"]

    B & C & D --> E["integration_validators/\nshared.py\nIntegrationHealthResult"]

    E --> F["app.integrations.github_mcp\nGitHubMCPValidationResult"]

Comments Outside Diff (2)

app/cli/wizard/integration_validators/http_probe_validators.py, line 1092-1106 (link)

Undisclosed HTTP client swap in validate_slack_webhook

The PR description states "No validator behavior changed; this is a pure refactoring," but validate_slack_webhook was silently switched from requests to httpx. The caught exception type changed from requests.RequestException to httpx.RequestError, which has a different exception hierarchy. While the common-case behavior is equivalent, any caller relying on the original requests.RequestException semantics (e.g., downstream error handling or logging that inspects the exception type) would see a different type. The commit message and description should acknowledge this behavioral delta.
tests/cli/wizard/test_integration_health.py, line 1380-1384 (link)

Compatibility test checks only one of 18 public symbols

The new compatibility test verifies that validate_slack_webhook is accessible via the legacy module path, but __all__ in integration_health.py lists 18 symbols. If any of the other 17 re-exports were accidentally dropped or misnamed, this test would pass silently. Consider asserting against the full __all__ list:
```
def test_legacy_integration_health_import_surface_still_exports_validators() -> None:
    import app.cli.wizard.integration_health as mod

    for name in mod.__all__:
        assert hasattr(mod, name), f"Missing from legacy surface: {name}"
```

_{Reviews (1): Last reviewed commit: "refactor(wizard): split integration heal..." | Re-trigger Greptile}

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

7vignesh · 2026-04-26T08:52:23Z

@VaibhavUpreti
Resolved the merge conflicts in app/cli/wizard/integration_health.py and tests/cli/wizard/test_integration_health.py by rebasing onto the latest main. Force-pushed the updated branch — no conflicts remaining. Please review when you get a chance!

muddlebee · 2026-04-28T08:57:20Z

+class IntegrationHealthResult:
+    """Result of validating an optional integration."""
+
+    ok: bool


shared.py imports GitHubMCPValidationResult from a concrete integration module, which means every consumer of IntegrationHealthResult silently pulls in the GitHub MCP import chain. The field is only meaningful for one validator. Consider using object | None (or a Protocol) here and letting mcp_validators.py own the typed field, or move GitHubMCPValidationResult into shared.py to keep the dependency explicit in the right layer.

muddlebee · 2026-04-28T08:57:28Z

+                ok=False, detail="Jira credentials invalid. Check email and API token."
+            )
+        if resp.status_code == 404:
+            return IntegrationHealthResult(


validate_google_docs_integration belongs in client_validators.py. It uses GoogleDocsClient (a service client), not a raw HTTP probe — the only reason it's here is that the original file had it adjacent to Notion/Jira. The module docstring says 'HTTP-probe validators'; this function violates that contract and will confuse the next person deciding where to put a new validator.

muddlebee · 2026-04-28T08:58:20Z

+def test_legacy_integration_health_import_surface_still_exports_validators() -> None:
+    module = import_module("app.cli.wizard.integration_health")
+
+    assert hasattr(module, "validate_slack_webhook")


Spot-checking one symbol out of 18 exported gives false confidence. Assert against the full all list so a future refactor can't silently drop exports: assert set(module.all) == {all 18 names}.

7vignesh · 2026-04-28T11:26:04Z

Hi @muddlebee, addressed all 3 points in the latest push:

IntegrationHealthResult now uses a type-only reference to GitHubMCPValidationResult in shared.py — no more runtime import chain for every consumer.
validate_google_docs_integration moved to client_validators.py where it belongs.
Compatibility test now checks the full all surface instead of a single symbol.

All tests + lint/type checks pass.

muddlebee

Looks good.

muddlebee · 2026-04-28T12:54:46Z

thank you looks good. I will wait additional reviews, @VaibhavUpreti @hamzzaaamalik @yashksaini-coder

hamzzaaamalik · 2026-04-29T09:59:32Z

Looked at this locally and ran the tests. It's a clean refactor integration_health.py becomes a re-export façade, validators move into 3 grouped modules, tests pass.

Spot-checked every validator body against the old file: 13 are byte-identical, the other 5 only have cosmetic tweaks (hoisting import httpx to the top of the file, renaming e to err for consistency, an em-dash → -- in one comment). No behaviour change.

Other things I checked:

Every place in the codebase that imports from integration_health still works — all 19 symbols are re-exported and the new compat test guards that.
IntegrationHealthResult no longer pulls in github_mcp at runtime (verified — fresh import doesn't load that module).
validate_google_docs_integration is in client_validators.py like @muddlebee asked.
31/31 tests pass, 203/203 in the broader tests/cli/ sweep.
Tiny nit: PR description says "5 HTTP validators" but it's actually 4 (Google Docs got moved). Cosmetic only.

LGTM, approve and merge.

github-actions · 2026-04-29T10:01:04Z

🎯 Bullseye. @7vignesh opened a PR, kept the vibes clean, and got it merged. Absolute cinema. 🎬

👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome.

Copilot AI review requested due to automatic review settings April 25, 2026 21:26

Copilot started reviewing on behalf of 7vignesh April 25, 2026 21:27 View session

7vignesh mentioned this pull request Apr 25, 2026

Split app/cli/wizard/integration_health.py into smaller validator modules #873

Closed

Copilot AI reviewed Apr 25, 2026

View reviewed changes

refactor(wizard): split integration health validators

9277017

7vignesh force-pushed the issue/873-split-integration-health-validators branch from c91d6df to 9277017 Compare April 26, 2026 08:45

muddlebee reviewed Apr 28, 2026

View reviewed changes

refactor(wizard): address integration validator review feedback

4c909bf

muddlebee approved these changes Apr 28, 2026

View reviewed changes

muddlebee merged commit 17c5bbd into Tracer-Cloud:main Apr 29, 2026
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(wizard): split integration health validators into grouped modules#958

refactor(wizard): split integration health validators into grouped modules#958
muddlebee merged 2 commits intoTracer-Cloud:mainfrom
7vignesh:issue/873-split-integration-health-validators

7vignesh commented Apr 25, 2026

Uh oh!

greptile-apps Bot commented Apr 25, 2026 •

edited

Loading

Comments Outside Diff (2)

Uh oh!

Copilot AI left a comment

Uh oh!

7vignesh commented Apr 26, 2026

Uh oh!

muddlebee Apr 28, 2026

Uh oh!

muddlebee Apr 28, 2026

Uh oh!

muddlebee Apr 28, 2026

Uh oh!

7vignesh commented Apr 28, 2026

Uh oh!

muddlebee left a comment •

edited

Loading

Uh oh!

muddlebee commented Apr 28, 2026

Uh oh!

hamzzaaamalik commented Apr 29, 2026

Uh oh!

Uh oh!

github-actions Bot commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

7vignesh commented Apr 25, 2026

Describe the changes you have made in this PR -

Test updates:

Demo/Screenshot for feature changes and bug fixes -

Code Understanding and AI Usage

Checklist before requesting a review

Uh oh!

greptile-apps Bot commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Comments Outside Diff (2)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

7vignesh commented Apr 26, 2026

Uh oh!

muddlebee Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

muddlebee Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

muddlebee Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

7vignesh commented Apr 28, 2026

Uh oh!

muddlebee left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

muddlebee commented Apr 28, 2026

Uh oh!

hamzzaaamalik commented Apr 29, 2026

Uh oh!

Uh oh!

github-actions Bot commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

greptile-apps Bot commented Apr 25, 2026 •

edited

Loading

muddlebee left a comment •

edited

Loading