docs(http): document canonical outbound HTTP clients - #2260
Conversation
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
ce03258 to
0e4a8e6
Compare
Greptile SummaryThe PR documents the canonical outbound HTTP client and its lifecycle, retries, telemetry, and testing contract, while adding references for
|
| Filename | Overview |
|---|---|
| docs/configure-rails/actions/outbound-http.mdx | Adds accurate canonical HTTP usage, ownership, retry, telemetry, and testing guidance. |
| docs/configure-rails/actions/rail-outcomes.mdx | Documents explicit rail decisions, transforms, validation, and migration from removed implicit mappings. |
| docs/configure-rails/actions/creating-actions.mdx | Updates action guidance for canonical HTTP and explicit outcomes, but retains boolean safety examples that conflict with the new return-value contract. |
| docs/reference/rail-manifests.mdx | Adds a comprehensive reference consistent with the public manifest constructors, exports, discovery, and validation behavior. |
| docs/observability/metrics/reference.mdx | Adds canonical HTTP duration metrics and attributes consistent with implementation. |
| docs/observability/tracing/span-reference.mdx | Adds outbound HTTP span names, attributes, privacy behavior, and nesting guidance consistent with implementation. |
| docs/index.yml | Registers the new action and manifest pages in documentation navigation. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Action[Custom or library action] --> Call[http_call]
Call --> Choice{Injected client?}
Choice -->|Yes| Shared[Borrow shared HTTPClient]
Choice -->|No| Scoped[Create call-scoped client]
Shared --> Request[Send materialized request]
Scoped --> Request
Request --> Retry[Optional bounded retries]
Retry --> Telemetry[Optional logical-request telemetry]
Telemetry --> Response[HTTPResponse]
Response --> Close{Client owner}
Close -->|Caller-owned| Keep[Leave client open]
Close -->|Call-scoped| Dispose[Close client]
Prompt To Fix All With AI
### Issue 1
docs/configure-rails/actions/creating-actions.mdx:96-98
**Align boolean safety examples**
This guidance says ordinary boolean returns have no implicit safety meaning, but the validation examples later on the page still label bare `True` and `False` values as allowing or blocking content without showing a consuming Colang branch. Readers can copy those examples expecting enforcement that does not occur, so either demonstrate the explicit flow branch or return `RailOutcome`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "docs(http): document canonical outbound ..." | Re-trigger Greptile
| The `@action` decorator does not interpret an action's return value as a safety decision. Ordinary custom actions can return strings, booleans, numbers, dictionaries, or other Python values for a Colang flow to consume explicitly. | ||
|
|
||
| You can also define a custom mapping function for more complex logic: | ||
| When the action itself makes a rail decision, return a [`RailOutcome`](/configure-guardrails/actions/rail-outcomes). It carries an explicit allow, block, or transform decision without relying on implicit boolean or numeric conventions. |
There was a problem hiding this comment.
This guidance says ordinary boolean returns have no implicit safety meaning, but the validation examples later on the page still label bare True and False values as allowing or blocking content without showing a consuming Colang branch. Readers can copy those examples expecting enforcement that does not occur, so either demonstrate the explicit flow branch or return RailOutcome.
Knowledge Base Used: Actions Framework
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/configure-rails/actions/creating-actions.mdx
Line: 96-98
Comment:
**Align boolean safety examples**
This guidance says ordinary boolean returns have no implicit safety meaning, but the validation examples later on the page still label bare `True` and `False` values as allowing or blocking content without showing a consuming Colang branch. Readers can copy those examples expecting enforcement that does not occur, so either demonstrate the explicit flow branch or return `RailOutcome`.
**Knowledge Base Used:** [Actions Framework](https://app.greptile.com/nvidia-public-github/-/custom-context/knowledge-base/nvidia-nemo/guardrails/-/docs/actions.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.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!
📝 WalkthroughWalkthroughThis documentation update adds references for RailOutcome, outbound HTTP, observability, and Rail Manifests. It updates action examples for canonical HTTP clients, lifecycle ownership, timeout handling, and migration from ChangesDocumentation references
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/configure-rails/actions/creating-actions.mdx`:
- Around line 82-91: Import action in each standalone example before its `@action`
decorator: docs/configure-rails/actions/creating-actions.mdx lines 82-91,
189-201, and 248-263. Preserve the existing HTTP imports and example
implementations.
In `@docs/configure-rails/actions/outbound-http.mdx`:
- Around line 149-154: Update the RetryPolicy example to preserve the default
retryable methods while adding POST to the set, or explicitly state that
retrying only POST is intentional. Adjust the retryable_methods configuration in
the shown policy example without changing unrelated settings.
In `@docs/configure-rails/actions/rail-outcomes.mdx`:
- Around line 78-80: Update the metadata guidance in the rail outcomes
documentation to recommend only redacted provider status and error codes as
provider-related evidence. Explicitly state that raw request and response
payloads, including user content, must not be stored in metadata, while
preserving the existing guidance that metadata is non-load-bearing and
potentially observable.
In `@docs/observability/tracing/span-reference.mdx`:
- Around line 11-12: Update the introductory sentence to refer to “the NVIDIA
NeMo Guardrails library” instead of “The NeMo Guardrails library,” preserving
the rest of the span-reference description unchanged.
In `@docs/reference/rail-manifests.mdx`:
- Line 68: Update the example rail’s docs_url value to use a rendered Fern route
defined in docs/index.yml; alternatively remove docs_url if no target page
exists. Verify the selected route is included in navigation and is not a
source-file path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 84146a49-a13e-4b94-95ac-49feb26ee140
📒 Files selected for processing (10)
docs/configure-rails/actions/creating-actions.mdxdocs/configure-rails/actions/index.mdxdocs/configure-rails/actions/outbound-http.mdxdocs/configure-rails/actions/rail-outcomes.mdxdocs/configure-rails/actions/registering-actions.mdxdocs/configure-rails/custom-initialization/init-function.mdxdocs/index.ymldocs/observability/metrics/reference.mdxdocs/observability/tracing/span-reference.mdxdocs/reference/rail-manifests.mdx
| from nemoguardrails.http import HTTPClient, http_call | ||
|
|
||
| @action(execute_async=True) | ||
| async def call_external_api(endpoint: str): | ||
| async def call_external_api( | ||
| endpoint: str, | ||
| http_client: HTTPClient | None = None, | ||
| ): | ||
| """Call an external API without blocking event processing.""" | ||
| response = await http_client.get(endpoint) | ||
| response = await http_call(http_client, "GET", endpoint) | ||
| return response.json() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Import action in every standalone example.
Each block uses @action but imports only HTTP symbols. Copying any block raises NameError.
docs/configure-rails/actions/creating-actions.mdx#L82-L91: importactionbefore the decorator.docs/configure-rails/actions/creating-actions.mdx#L189-L201: importactionbefore the decorator.docs/configure-rails/actions/creating-actions.mdx#L248-L263: importactionbefore the decorator.
Proposed fix
+from nemoguardrails.actions import action
from nemoguardrails.http import HTTPClient, http_call📍 Affects 1 file
docs/configure-rails/actions/creating-actions.mdx#L82-L91(this comment)docs/configure-rails/actions/creating-actions.mdx#L189-L201docs/configure-rails/actions/creating-actions.mdx#L248-L263
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/configure-rails/actions/creating-actions.mdx` around lines 82 - 91,
Import action in each standalone example before its `@action` decorator:
docs/configure-rails/actions/creating-actions.mdx lines 82-91, 189-201, and
248-263. Preserve the existing HTTP imports and example implementations.
| ```python | ||
| policy = RetryPolicy( | ||
| max_attempts=3, | ||
| retryable_methods=frozenset({"POST"}), | ||
| ) | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 -P '^\s*class\s+RetryPolicy\b|^\s*retryable_methods\s*[:=]|retryable_methods\s*=' nemoguardrails/http
rg -n -C 6 -P 'RetryPolicy\(|retryable_methods' tests docsRepository: NVIDIA-NeMo/Guardrails
Length of output: 17627
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
retry_path = Path("nemoguardrails/http/retry.py")
test_path = Path("tests/http/test_retry.py")
docs_path = Path("docs/configure-rails/actions/outbound-http.mdx")
retry_tree = ast.parse(retry_path.read_text())
policy = next(
node for node in retry_tree.body
if isinstance(node, ast.ClassDef) and node.name == "RetryPolicy"
)
methods_field = next(
node for node in policy.body
if isinstance(node, ast.AnnAssign)
and isinstance(node.target, ast.Name)
and node.target.id == "retryable_methods"
)
default_call = methods_field.value
assert isinstance(default_call, ast.Call)
assert isinstance(default_call.func, ast.Name)
assert default_call.func.id == "field"
factory = next(
keyword.value for keyword in default_call.keywords
if keyword.arg == "default_factory"
)
assert isinstance(factory, ast.Lambda)
default_frozenset = ast.literal_eval(factory.body)
assert default_frozenset == frozenset(
{"DELETE", "GET", "HEAD", "OPTIONS", "PUT", "TRACE"}
)
test_text = test_path.read_text()
assert 'RetryPolicy(retryable_methods=frozenset({"post"}))' in test_text
assert 'await client.request("GET", "https://example.com")' in test_text
docs_text = docs_path.read_text()
assert "Add `POST` only" in docs_text
assert 'retryable_methods=frozenset({"POST"})' in docs_text
print("default retryable methods:", sorted(default_frozenset))
print("explicit POST set:", ["POST"])
print("explicit POST set extends defaults:", bool(default_frozenset | {"POST"} == {"POST"}))
print("source/test evidence: explicit method set is supplied directly and GET is tested as non-retryable")
print("documentation wording: says to add POST, but example contains only POST")
PYRepository: NVIDIA-NeMo/Guardrails
Length of output: 832
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
retry_path = Path("nemoguardrails/http/retry.py")
test_path = Path("tests/http/test_retry.py")
docs_path = Path("docs/configure-rails/actions/outbound-http.mdx")
tree = ast.parse(retry_path.read_text())
policy = next(
node for node in tree.body
if isinstance(node, ast.ClassDef) and node.name == "RetryPolicy"
)
field_node = next(
node for node in policy.body
if isinstance(node, ast.AnnAssign)
and isinstance(node.target, ast.Name)
and node.target.id == "retryable_methods"
)
field_call = field_node.value
factory = next(
keyword.value for keyword in field_call.keywords
if keyword.arg == "default_factory"
)
default_expr = factory.body
assert isinstance(default_expr, ast.Call)
assert isinstance(default_expr.func, ast.Name)
assert default_expr.func.id == "frozenset"
default_methods = frozenset(
ast.literal_eval(element)
for element in default_expr.args[0].elts
)
explicit_methods = frozenset({"POST"})
assert "POST" not in default_methods
assert explicit_methods != default_methods
assert default_methods | explicit_methods != explicit_methods
membership = []
for node in ast.walk(tree):
if isinstance(node, ast.Compare) and any(isinstance(op, ast.In) for op in node.ops):
membership.append(ast.unparse(node))
assert any("retryable_methods" in expression for expression in membership)
test_text = test_path.read_text()
docs_text = docs_path.read_text()
assert 'RetryPolicy(retryable_methods=frozenset({"post"}))' in test_text
assert 'await client.request("GET", "https://example.com")' in test_text
assert "Add `POST` only" in docs_text
assert 'retryable_methods=frozenset({"POST"})' in docs_text
print("default retryable methods:", sorted(default_methods))
print("explicit retryable methods:", sorted(explicit_methods))
print("explicit value extends the default:", bool(default_methods | explicit_methods == explicit_methods))
print("retry-method membership checks:", membership)
print("test and documentation evidence: an explicit POST-only set is used")
PYRepository: NVIDIA-NeMo/Guardrails
Length of output: 518
Preserve the default retryable methods when adding POST.
retryable_methods replaces the default set, so this example enables retries only for POST. Include the default methods or state that the policy intentionally retries only POST.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/configure-rails/actions/outbound-http.mdx` around lines 149 - 154,
Update the RetryPolicy example to preserve the default retryable methods while
adding POST to the set, or explicitly state that retrying only POST is
intentional. Adjust the retryable_methods configuration in the shown policy
example without changing unrelated settings.
| Use `reason` for a neutral, human-readable explanation of the decision. Use `metadata` for structured evidence such as categories, scores, detections, or provider response details. | ||
|
|
||
| Do not make `metadata` load-bearing for the decision. Consumers should use `decision`, `is_blocked`, or `is_transform` to determine the outcome. Treat metadata as potentially observable data and avoid storing secrets, credentials, or unnecessary user content. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Do not recommend raw provider response details in metadata.
Line 78 can lead users to store raw provider responses in metadata. Those payloads can contain user content. Line 80 states that metadata can be observable. Limit the example to redacted provider status and error codes. Explicitly exclude raw request and response payloads.
Proposed documentation change
-Use `reason` for a neutral, human-readable explanation of the decision. Use `metadata` for structured evidence such as categories, scores, detections, or provider response details.
+Use `reason` for a neutral, human-readable explanation of the decision. Use `metadata` for structured evidence such as categories, scores, detections, or redacted provider status and error codes.
-Do not make `metadata` load-bearing for the decision. Consumers should use `decision`, `is_blocked`, or `is_transform` to determine the outcome. Treat metadata as potentially observable data and avoid storing secrets, credentials, or unnecessary user content.
+Do not make `metadata` load-bearing for the decision. Consumers should use `decision`, `is_blocked`, or `is_transform` to determine the outcome. Treat metadata as potentially observable data. Do not store secrets, credentials, raw provider payloads, or unnecessary user content.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Use `reason` for a neutral, human-readable explanation of the decision. Use `metadata` for structured evidence such as categories, scores, detections, or provider response details. | |
| Do not make `metadata` load-bearing for the decision. Consumers should use `decision`, `is_blocked`, or `is_transform` to determine the outcome. Treat metadata as potentially observable data and avoid storing secrets, credentials, or unnecessary user content. | |
| Use `reason` for a neutral, human-readable explanation of the decision. Use `metadata` for structured evidence such as categories, scores, detections, or redacted provider status and error codes. | |
| Do not make `metadata` load-bearing for the decision. Consumers should use `decision`, `is_blocked`, or `is_transform` to determine the outcome. Treat metadata as potentially observable data. Do not store secrets, credentials, raw provider payloads, or unnecessary user content. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/configure-rails/actions/rail-outcomes.mdx` around lines 78 - 80, Update
the metadata guidance in the rail outcomes documentation to recommend only
redacted provider status and error codes as provider-related evidence.
Explicitly state that raw request and response payloads, including user content,
must not be stored in metadata, while preserving the existing guidance that
metadata is non-load-bearing and potentially observable.
| The NeMo Guardrails library emits OpenTelemetry spans, allowing you to trace individual requests. | ||
| This reference documents the spans and attributes each engine produces. It covers the default LLMRails engine first, then the opt-in IORails engine. | ||
| This reference documents shared outbound HTTP client spans and the spans each engine produces. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the required package name.
Replace “The NeMo Guardrails library” with “the NVIDIA NeMo Guardrails library.”
As per coding guidelines, "docs/**/*.{md,mdx}: Refer to this package as “the NVIDIA NeMo Guardrails library”."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/observability/tracing/span-reference.mdx` around lines 11 - 12, Update
the introductory sentence to refer to “the NVIDIA NeMo Guardrails library”
instead of “The NeMo Guardrails library,” preserving the rest of the
span-reference description unchanged.
Source: Coding guidelines
Documents the canonical outbound HTTP client contract, lifecycle, and observability behavior.
Summary by CodeRabbit