refactor(library): migrate specialized HTTP clients - #2213
Merged
Conversation
github-actions
Bot
added
status: needs triage
This was referenced Jul 23, 2026
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Pouyanpi
force-pushed
the
pouyanpi/rail-library-stack-14-http-specialized-migrations
branch
from
July 23, 2026 14:42
37f0772 to
694c268
Compare
Pouyanpi
force-pushed
the
pouyanpi/rail-library-stack-13-http-action-migrations
branch
2 times, most recently
from
July 23, 2026 16:21
1f8c466 to
9852187
Compare
Pouyanpi
force-pushed
the
pouyanpi/rail-library-stack-14-http-specialized-migrations
branch
2 times, most recently
from
July 23, 2026 16:28
282faee to
e7d4172
Compare
Pouyanpi
force-pushed
the
pouyanpi/rail-library-stack-13-http-action-migrations
branch
from
July 23, 2026 16:28
9852187 to
736a206
Compare
Pouyanpi
added
status: triaged
Pouyanpi
marked this pull request as ready for review
July 23, 2026 17:16
Contributor
Greptile SummaryThis PR completes the canonical HTTP subsystem migration for the Hugging Face classifier and Clavata integrations, replacing direct
|
| Filename | Overview |
|---|---|
| nemoguardrails/http/types.py | Adds HTTPTLSConfig frozen dataclass with paired cert/key validation in post_init; clean addition to the type layer |
| nemoguardrails/http/transport.py | Accepts optional HTTPTLSConfig and correctly rejects TLS config combined with an injected client; verify/cert translation logic is sound |
| nemoguardrails/http/runtime.py | Threads HTTPTLSConfig through to HttpxHTTPClient unchanged; minimal, correct change |
| nemoguardrails/library/clavata/request.py | Replaces aiohttp with canonical HTTP stack and correct RetryPolicy; outer except-Exception wrapper preserves pre-existing behaviour but limits error specificity; conditional kwargs pattern is unnecessarily verbose |
| nemoguardrails/library/hf_classifier/backends.py | Replaces httpx directly with canonical transport; get_backend correctly bypasses cache for injected clients but silently ignores http_client for local engines; retry policy correctly covers transport errors only |
| nemoguardrails/library/clavata/actions.py | Adds http_client injection to evaluate_with_policy and clavata_check; conditional kwargs passthrough is overly defensive but otherwise correct |
| nemoguardrails/library/hf_classifier/actions.py | Adds http_client injection to all three action entry points; same conditional kwargs pattern as Clavata actions |
| tests/http/test_library_boundary.py | New AST-based conformance test that rejects direct httpx/aiohttp/requests/urllib3 imports under the library root; sound approach, correctly walks all AST nodes |
| tests/http/test_transport.py | New TLS tests cover owned-client configuration, injected-client rejection, and paired-credential validation; comprehensive and correct |
| tests/test_clavata.py | Replaces aioresponses mocks with RecordingHTTPClient; cleaner and transport-neutral; first test adds URL/method/header assertions |
| tests/test_clavata_models.py | Adds retry and transport-failure tests for ClavataClient; the retry test incurs a real sleep due to uninjected asyncio.sleep in the policy |
| tests/test_hf_classifier.py | Updates SSL/TLS assertions to match new HTTPTLSConfig field names; adds injected-client and transport-retry tests for VLLMBackend; correct |
Sequence Diagram
sequenceDiagram
participant Action as clavata_check / hf_classifier_check_*
participant Backend as ClavataClient / _RemoteBackend
participant Retry as RetryingHTTPClient
participant Transport as HttpxHTTPClient (owned)
participant Injected as Injected HTTPClient (caller-owned)
alt Injected client provided
Action->>Backend: "call(http_client=injected)"
Backend->>Retry: wrap(injected, policy)
Retry->>Injected: request(POST, url)
Injected-->>Retry: HTTPResponse / HTTPConnectionError
Retry-->>Backend: response (after policy retries)
Note over Injected: NOT closed — caller owns it
else No injected client (factory path)
Action->>Backend: "call(http_client=None)"
Backend->>Transport: create_http_client(timeout, tls)
Backend->>Retry: wrap(Transport, policy)
Retry->>Transport: request(POST, url)
Transport-->>Retry: HTTPResponse
Retry-->>Backend: response
Backend->>Transport: close() via _resolve_http_client
end
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
nemoguardrails/library/hf_classifier/backends.py:504-511
**Injected client silently ignored for local engine**
When `http_client` is provided but `config.engine == "local"`, the injected client is silently dropped and the local pipeline backend is created or returned from cache without it. A caller who passes an `http_client` expecting it to influence behaviour receives no warning that their argument was discarded. Logging a debug-level warning here would make this easier to diagnose.
### Issue 2 of 4
nemoguardrails/library/clavata/actions.py:256-257
**Unnecessary conditional kwargs indirection**
`evaluate_with_policy` already declares `http_client: HTTPClient | None = None` as a parameter, so it handles `None` safely. The conditional dict construction adds verbosity without any benefit — passing `http_client=http_client` directly is equivalent and clearer.
```suggestion
result = await evaluate_with_policy(text, str(policy_id), clavata_config, http_client=http_client)
```
### Issue 3 of 4
nemoguardrails/library/hf_classifier/actions.py:53-54
**Same unnecessary conditional kwargs pattern**
`get_backend` already accepts `http_client=None` and handles it correctly (falls through to caching). Passing it directly simplifies the call site. This pattern appears in all three action functions.
```suggestion
backend = get_backend(classifier_config, name=classifier_name, http_client=http_client)
```
### Issue 4 of 4
tests/test_clavata_models.py:270-291
**Retry test incurs real sleep**
`_CLAVATA_RETRY_POLICY` sets `initial_delay=0.1`, and `RetryingHTTPClient` uses `asyncio.sleep` for backoff between attempts. Because neither `sleep` nor `random_value` is injected in this test, the first retry waits up to 100 ms of real wall time. Injecting a no-op sleep via `RetryingHTTPClient(transport, _CLAVATA_RETRY_POLICY, sleep=lambda _: asyncio.sleep(0))` would keep the test instant while still verifying the retry count.
Reviews (1): Last reviewed commit: "test(http): enforce the library transpor..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+504
to
+511
| http_client: HTTPClient | None = None, | ||
| ) -> ClassifierBackend: | ||
| """Get or create a cached backend instance from classifier config.""" | ||
| cls = _BACKENDS.get(config.engine) | ||
| if cls is None: | ||
| raise ValueError(f"Unknown hf_classifier engine: '{config.engine}'. Supported: {', '.join(_BACKENDS)}") | ||
| if http_client is not None and config.engine != "local": | ||
| return cls(config, http_client=http_client) |
Contributor
There was a problem hiding this comment.
When http_client is provided but config.engine == "local", the injected client is silently dropped and the local pipeline backend is created or returned from cache without it. A caller who passes an http_client expecting it to influence behaviour receives no warning that their argument was discarded. Logging a debug-level warning here would make this easier to diagnose.
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/library/hf_classifier/backends.py
Line: 504-511
Comment:
**Injected client silently ignored for local engine**
When `http_client` is provided but `config.engine == "local"`, the injected client is silently dropped and the local pipeline backend is created or returned from cache without it. A caller who passes an `http_client` expecting it to influence behaviour receives no warning that their argument was discarded. Logging a debug-level warning here would make this easier to diagnose.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+256
to
+257
| evaluate_kwargs = {"http_client": http_client} if http_client is not None else {} | ||
| result = await evaluate_with_policy(text, str(policy_id), clavata_config, **evaluate_kwargs) |
Contributor
There was a problem hiding this comment.
evaluate_with_policy already declares http_client: HTTPClient | None = None as a parameter, so it handles None safely. The conditional dict construction adds verbosity without any benefit — passing http_client=http_client directly is equivalent and clearer.
Suggested change
| evaluate_kwargs = {"http_client": http_client} if http_client is not None else {} | |
| result = await evaluate_with_policy(text, str(policy_id), clavata_config, **evaluate_kwargs) | |
| result = await evaluate_with_policy(text, str(policy_id), clavata_config, http_client=http_client) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/library/clavata/actions.py
Line: 256-257
Comment:
**Unnecessary conditional kwargs indirection**
`evaluate_with_policy` already declares `http_client: HTTPClient | None = None` as a parameter, so it handles `None` safely. The conditional dict construction adds verbosity without any benefit — passing `http_client=http_client` directly is equivalent and clearer.
```suggestion
result = await evaluate_with_policy(text, str(policy_id), clavata_config, http_client=http_client)
```
How can I resolve this? If you propose a fix, please make it concise.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!
Comment on lines
+53
to
+54
| backend_kwargs = {"http_client": http_client} if http_client is not None else {} | ||
| backend = get_backend(classifier_config, name=classifier_name, **backend_kwargs) |
Contributor
There was a problem hiding this comment.
get_backend already accepts http_client=None and handles it correctly (falls through to caching). Passing it directly simplifies the call site. This pattern appears in all three action functions.
Suggested change
| backend_kwargs = {"http_client": http_client} if http_client is not None else {} | |
| backend = get_backend(classifier_config, name=classifier_name, **backend_kwargs) | |
| backend = get_backend(classifier_config, name=classifier_name, http_client=http_client) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/library/hf_classifier/actions.py
Line: 53-54
Comment:
**Same unnecessary conditional kwargs pattern**
`get_backend` already accepts `http_client=None` and handles it correctly (falls through to caching). Passing it directly simplifies the call site. This pattern appears in all three action functions.
```suggestion
backend = get_backend(classifier_config, name=classifier_name, http_client=http_client)
```
How can I resolve this? If you propose a fix, please make it concise.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!
Comment on lines
+270
to
+291
| @pytest.mark.asyncio | ||
| async def test_clavata_client_uses_shared_retry_policy(self): | ||
| response = CreateJobResponse( | ||
| job=Job( | ||
| status="JOB_STATUS_COMPLETED", | ||
| results=[Result(report=Report(result="OUTCOME_FALSE", sectionEvaluationReports=[]))], | ||
| ) | ||
| ) | ||
| transport = RecordingHTTPClient( | ||
| [ | ||
| HTTPResponse(status_code=429), | ||
| HTTPResponse(status_code=200, content=response.model_dump_json().encode()), | ||
| ] | ||
| ) | ||
| job = await ClavataClient( | ||
| "https://clavata.example", | ||
| api_key="test-key", | ||
| http_client=transport, | ||
| ).create_job("hello", "policy-id") | ||
|
|
||
| assert job.status == "JOB_STATUS_COMPLETED" | ||
| assert len(transport.requests) == 2 |
Contributor
There was a problem hiding this comment.
_CLAVATA_RETRY_POLICY sets initial_delay=0.1, and RetryingHTTPClient uses asyncio.sleep for backoff between attempts. Because neither sleep nor random_value is injected in this test, the first retry waits up to 100 ms of real wall time. Injecting a no-op sleep via RetryingHTTPClient(transport, _CLAVATA_RETRY_POLICY, sleep=lambda _: asyncio.sleep(0)) would keep the test instant while still verifying the retry count.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_clavata_models.py
Line: 270-291
Comment:
**Retry test incurs real sleep**
`_CLAVATA_RETRY_POLICY` sets `initial_delay=0.1`, and `RetryingHTTPClient` uses `asyncio.sleep` for backoff between attempts. Because neither `sleep` nor `random_value` is injected in this test, the first retry waits up to 100 ms of real wall time. Injecting a no-op sleep via `RetryingHTTPClient(transport, _CLAVATA_RETRY_POLICY, sleep=lambda _: asyncio.sleep(0))` would keep the test instant while still verifying the retry count.
How can I resolve this? If you propose a fix, please make it concise.
Pouyanpi
merged commit Jul 24, 2026
644faa4
into
pouyanpi/rail-library-stack-13-http-action-migrations
Pouyanpi
force-pushed
the
pouyanpi/rail-library-stack-13-http-action-migrations
branch
from
July 24, 2026 10:14
736a206 to
644faa4
Compare
Pouyanpi
deleted the
pouyanpi/rail-library-stack-14-http-specialized-migrations
branch
July 24, 2026 10:14
Pouyanpi
force-pushed
the
pouyanpi/rail-library-stack-14-http-specialized-migrations
branch
from
July 24, 2026 10:14
e7d4172 to
644faa4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Migrate the Hugging Face classifier and Clavata integrations to the canonical
HTTP subsystem while preserving their specialized retry and TLS requirements.
Add a transport-neutral
HTTPTLSConfigfor verification control, custom CAbundles, and paired client certificate and key paths. TLS configuration is
consumed only at the transport boundary and cannot be combined with an injected
HTTPX client, which keeps ownership and connection construction unambiguous.
Hugging Face and Clavata wrap injected clients with their own retry policies
without taking ownership. Their fallback factories return fully composed
managed clients so
http_callcan close the entire owned stack. POST retrybehavior is explicit for both integrations.
An AST-based conformance test enforces the library boundary by rejecting direct
imports of HTTPX, aiohttp, Requests, or urllib3 under
nemoguardrails/library/.Impact
construction details.
deterministically.
Stack
pouyanpi/rail-library-stack-9-http-contractdeveloppouyanpi/rail-library-stack-11-http-observabilitypouyanpi/rail-library-stack-12-http-request-migrationspouyanpi/rail-library-stack-13-http-action-migrationspouyanpi/rail-library-stack-14-http-specialized-migrationsRelated Issue(s)
Verification
make test TEST="tests/http tests/test_activefence_rail.py tests/test_fact_checking.py tests/test_gliner.py tests/test_jailbreak_request.py tests/test_polygraf.py tests/test_ai_defense.py tests/test_f5_guardrails.py tests/test_fiddler_rails.py tests/test_pangea_ai_guard.py tests/test_patronus_evaluate_api.py tests/test_policyai_rail.py tests/test_clavata.py tests/test_clavata_models.py tests/test_hf_classifier.py tests/test_llmrails.py tests/guardrails/test_guardrails.py"— 587 passed, 4 skipped.uv run --locked pre-commit run --from-ref origin/develop --to-ref pouyanpi/rail-library-stack-14-http-specialized-migrations— passed.AI Assistance
Codex assisted with implementation analysis, validation, stack restructuring,
and this draft. Check the disclosure box after human review.
Checklist