refactor: replace if/elif classifier chain with registry and unify guardrail helpers

[0xpaulx]

Summary

• Catalog classifier registry — _classify_service_instance in _catalog_impl.py was a 900-line function with 46 sequential if key == "X": branches doing identical try/validate/return logic. Each branch is now its own _classify_<name> function and a _CLASSIFIERS: dict[str, _ClassifyFn] dispatches to it. Adding a new integration requires one function + one dict entry instead of editing the monolith.

• Unified guardrail helpers — the pattern get engine → check is_active → loop messages → apply was copy-pasted inline across 6 call sites (llm_client.py ×3, agent_llm_client.py, chat/agent.py, llm_cli/runner.py) with slight variations. Extracted into app/guardrails/apply.py with three focused helpers: apply_guardrails_to_messages, apply_guardrails_to_text, and apply_guardrails_to_converse_payload (moved from bedrock_converse.py). A guardrail contract change now requires editing one file.

• Zombie directory removal — six directories (app/nodes/, app/delivery/, app/agents/, app/correlation/, app/hermes/, app/alerts/) contained only __pycache__ after previous refactors moved their source files. Deleted to avoid confusing new contributors.

Test plan

• make lint — passes clean
• make typecheck — 794 source files, no issues
• uv run pytest tests/test_guardrails/ tests/services/test_llm_client.py tests/services/test_agent_llm_client.py tests/integrations/ tests/agent/ — all pass
• make test-cov — 9,273 tests pass (live OpenAI routing failures are pre-existing rate-limit issues, unrelated to this change)

🤖 Generated with Claude Code

[github-actions]

Greptile code review

This repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md.

Run a review — add a PR comment with:

@greptile review

Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5.

Optional: automate with the greploop skill.

[greptile-apps]

Greptile Summary

This PR refactors two large orthogonal areas: (1) replaces a 900-line if/elif classifier chain in _catalog_impl.py with a _CLASSIFIERS registry, and (2) extracts duplicated guardrail application logic from six call sites into a single app/guardrails/apply.py module.

• Catalog classifier registry: each of the 46 integration classifiers is now its own _classify_<name> function registered in _CLASSIFIERS: dict[str, _ClassifyFn], with an added passthrough fallback for unknown keys.
• Unified guardrail helpers: apply_guardrails_to_messages, apply_guardrails_to_text, and apply_guardrails_to_converse_payload (moved from bedrock_converse.py) are centralized in apply.py and used consistently across all LLM call sites; a new test module verifies end-to-end redaction across each surface.

Confidence Score: 4/5

Safe to merge; the guardrail and catalog changes are well-isolated, and the existing test suite covers the primary code paths.

The refactoring is clean and the three new guardrail helpers are functionally equivalent to the inline code they replace. Two minor concerns keep this from a perfect score: the if system falsy check in apply_guardrails_to_messages is inconsistent with if system is not None used in apply_guardrails_to_converse_payload, and the new passthrough fallback in _classify_service_instance silently accepts unknown integration keys that would previously have caused a visible error. Neither is a current defect, but both introduce subtle edge-case divergences worth addressing before the helpers accumulate more call sites.

app/guardrails/apply.py (system-prompt guard consistency) and app/integrations/_catalog_impl.py (unknown-key fallback behavior).

Important Files Changed

Filename	Overview
app/guardrails/apply.py	New module with three guardrail helpers; minor inconsistency between apply_guardrails_to_messages (if system) and apply_guardrails_to_converse_payload (if system is not None) for empty-string system prompts.
app/integrations/_catalog_impl.py	46 inline if/elif branches replaced by individual classifier functions and _CLASSIFIERS dict; grafana_local correctly mapped; new passthrough fallback for unknown keys changes behavior from potential TypeError to silent inclusion.
app/services/llm_client.py	Inline guardrail calls replaced with apply_guardrails_to_messages; behavior preserved across LLMClient, BedrockLLMClient._invoke_anthropic, and BedrockLLMClient._invoke_converse.
app/services/agent_llm_client.py	BedrockConverseAgentClient now uses apply_guardrails_to_converse_payload from the new module; AnthropicAgentClient and OpenAIAgentClient are unchanged (no guardrails at this layer, consistent with pre-PR behavior).
tests/test_guardrails/test_llm_integration.py	New shared fixtures reduce duplication in TestOverlappingRedactionReachesDownstream, but earlier test classes still inline fake clients; no tests for apply_guardrails_to_converse_payload or apply_guardrails_to_text directly.
app/agent/chat/agent.py	Local _apply_guardrails helper removed; _prepare_messages now delegates to apply_guardrails_to_messages; functionally equivalent.
app/integrations/llm_cli/runner.py	Inline guardrail block replaced with apply_guardrails_to_text; behavior preserved.
app/services/bedrock_converse.py	apply_guardrails_to_converse_payload moved out to apply.py; file now focuses on schema helpers and converse message building.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[LLM call site] --> B{Which client?}
    B -->|LLMClient / BedrockLLMClient Anthropic| C[_normalize_messages]
    B -->|OpenAILLMClient| D[_normalize_messages_openai]
    B -->|BedrockLLMClient Converse| E[_normalize_messages]
    B -->|CLIBackedLLMClient| F[flatten_messages_to_prompt]
    B -->|BedrockConverseAgentClient| G[to_converse_messages]
    B -->|chat agent _prepare_messages| H[messages_to_invocation_dicts]
    C --> C2[apply_guardrails_to_messages]
    D --> D2[apply_guardrails_to_messages]
    E --> E2[apply_guardrails_to_messages]
    F --> F2[apply_guardrails_to_text]
    G --> G2[apply_guardrails_to_converse_payload]
    H --> H2[apply_guardrails_to_messages]
    subgraph apply.py
        C2
        D2
        E2
        F2
        G2
        H2
    end
    C2 --> Z[get_guardrail_engine]
    D2 --> Z
    E2 --> Z
    F2 --> Z
    G2 --> Z
    H2 --> Z
    Z --> |is_active = False| PASS[Return unchanged]
    Z --> |is_active = True| APPLY[engine.apply per content field]
    APPLY --> |block rule| ERR[GuardrailBlockedError]
    APPLY --> |redact rule| REDACTED[Redacted payload]
    REDACTED --> API[Send to LLM provider]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[LLM call site] --> B{Which client?}
    B -->|LLMClient / BedrockLLMClient Anthropic| C[_normalize_messages]
    B -->|OpenAILLMClient| D[_normalize_messages_openai]
    B -->|BedrockLLMClient Converse| E[_normalize_messages]
    B -->|CLIBackedLLMClient| F[flatten_messages_to_prompt]
    B -->|BedrockConverseAgentClient| G[to_converse_messages]
    B -->|chat agent _prepare_messages| H[messages_to_invocation_dicts]
    C --> C2[apply_guardrails_to_messages]
    D --> D2[apply_guardrails_to_messages]
    E --> E2[apply_guardrails_to_messages]
    F --> F2[apply_guardrails_to_text]
    G --> G2[apply_guardrails_to_converse_payload]
    H --> H2[apply_guardrails_to_messages]
    subgraph apply.py
        C2
        D2
        E2
        F2
        G2
        H2
    end
    C2 --> Z[get_guardrail_engine]
    D2 --> Z
    E2 --> Z
    F2 --> Z
    G2 --> Z
    H2 --> Z
    Z --> |is_active = False| PASS[Return unchanged]
    Z --> |is_active = True| APPLY[engine.apply per content field]
    APPLY --> |block rule| ERR[GuardrailBlockedError]
    APPLY --> |redact rule| REDACTED[Redacted payload]
    REDACTED --> API[Send to LLM provider]

Loading

Comments Outside Diff (1)

1. tests/test_guardrails/test_llm_integration.py, line 1-20 (link)

   Missing coverage for two of the three new helpers

   apply_guardrails_to_text (used by CLIBackedLLMClient) and apply_guardrails_to_converse_payload (used by BedrockConverseAgentClient) have no direct unit tests in this file or any other test module visible in the changeset. A regression in either helper — e.g. the block-rule path in apply_guardrails_to_text, or the list-of-blocks branch in apply_guardrails_to_converse_payload — would not be caught by the existing suite.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

_{Reviews (1): Last reviewed commit: "fix(ci): format _catalog_impl.py and upd..." | Re-trigger Greptile}

[greptile-apps]

apply_guardrails_to_messages guards the system prompt with a falsy check (if system), so an empty-string system prompt skips the engine entirely. apply_guardrails_to_converse_payload (line 84) uses the stricter if system is not None instead. Neither path is broken today because callers always pass None or a non-empty string, but the inconsistency is a latent trap for future callers who pass system="" expecting it to be scanned.

Suggested change

	guarded_system = engine.apply(system) if system else system
	guarded_system = engine.apply(system) if system is not None else None

[greptile-apps]

Unknown-key passthrough is a silent behavioral change

The old monolithic function had no explicit catch-all: an unrecognized key would fall through and return None (causing a TypeError at the unpacking site, which would be visible). The new fallback silently returns {"credentials": credentials, "integration_id": record_id} — a non-None tuple — so the instance passes the if flat_view is None or flat_key is None: continue gate and lands in resolved[key] with raw credentials attached. If an integration type is registered in the database before its classifier ships, its credentials will now appear in the resolved catalog under their raw key instead of being filtered out. Consider returning (None, None) here and reserving the passthrough for an explicit opt-in.

[Devesh36]

Nice work 🙌 🫡

[github-actions]

🌮 @0xpaulx's PR: showed up unannounced, improved everything, left zero bugs. Just like a perfect taco. 🌮

---

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