test: add public-API surface and server recorded tests#2122
Open
Pouyanpi wants to merge 5 commits into
Open
Conversation
Adds recorded coverage for additional public-API surfaces (options, parity, kb, concurrency, events, state, tracing, tools, stream cancellation, colang 2.x) plus the server chat-completions suite and an embeddings-index determinism guard, on top of the library-rails stack. The colang 2.x self-check continuation test is marked xfail: a single generate_async on an llm-continuation flow returns an intermediate TimerBotAction polling state rather than the resolved answer.
github-actions
Bot
added
status: needs triage
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Adds the missing test functions that consume the five stack-6 cassettes recorded but never referenced (B6.1 reasoning_content, B1.14 log.stats, B1.9 llm_output, A1.3 GenerationResponse shape, A3.10 streaming reasoning not inlined). Each replays offline under --block-network; snapshots filled from the committed cassettes. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Pouyanpi
force-pushed
the
test/recorded-tests-public-api-surface
branch
from
8b70a2c to
d78c71d
Compare
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
11 tasks
This was referenced Jul 2, 2026
Pouyanpi
added
status: triaged
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (49)
👮 Files not reviewed due to content moderation or server errors (48)
📝 Walkthrough
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 inconclusive)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Pouyanpi
added
status: triaged
Contributor
Greptile SummaryThis PR adds a comprehensive recorded characterization test baseline for
|
| Filename | Overview |
|---|---|
| tests/recorded/rails/public_api/test_generate.py | Core generation tests covering sync/async, prompt vs messages, options, log, stats, invalid model; all paths well-guarded with isinstance checks and snapshot assertions. |
| tests/recorded/rails/public_api/test_stream_cancellation.py | Deterministic teardown tests using a tracking generator; known gap (aclose not propagating through output-rail wrapper) explicitly pinned. Return-type annotation mismatch on async generator functions flagged in previous review thread. |
| tests/recorded/rails/public_api/test_colang_v2.py | Colang 2.x path coverage; the xfail test with an empty snapshot() was flagged in a previous review thread and not yet addressed. |
| tests/recorded/rails/public_api/test_parity.py | Broad deterministic regression net over all rail-pipeline combinations (input/output/combined/parallel) across pass/block/modify decisions; snapshots fully populated. |
| tests/recorded/rails/public_api/test_concurrency.py | 16-task concurrent contextvar isolation tests; correctly uses per-task LLMRails instances with asyncio.gather and validates no cross-talk between completions. |
| tests/recorded/server/test_chat_completions.py | Server-side contract tests; the invalid-config-id error assertion uses an overly broad fallback condition that could silently pass for unrelated responses. |
| tests/recorded/rails/public_api/configs.py | Centralized config registry; cleanly separates in-memory from filesystem configs via RailsConfigSource, easy to extend for new provider scenarios. |
| tests/recorded/server/conftest.py | Server test fixtures correctly scope VCR config to ignore the in-process testserver host and reset llm_rails_instances between tests. |
| tests/rails/llm/test_live_smoke.py | Live-provider smoke suite with auto-skip on missing/dummy API keys; validates contract shape (non-empty assistant message, KB retrieval) rather than exact content to avoid drift. |
| tests/recorded/rails/public_api/test_options.py | GenerationOptions effects well-covered: input/output/dialog toggle, output_vars, internal_events, explain() alignment, name-list subset gap explicitly pinned. |
| tests/recorded/rails/public_api/test_kb.py | KB retrieval and RAG injection thoroughly tested; monkeypatches CACHE_FOLDER to tmp_path to avoid cross-test cache pollution; validates embedding requests and marker injection in cassette. |
| tests/recorded/rails/public_api/test_stream.py | Streaming tests cover passthrough, output-rail wrapping, input blocking, NIM reasoning extraction, and tool-call chunks; the _chunks helper has the same AsyncIterator annotation issue as in test_stream_cancellation. |
| tests/server/test_server_calls_with_state.py | Minor refactor only (3 lines changed): renames/re-documents the Colang 1.0 state round-trip test function; the module-level TestClient and fixture teardown are pre-existing patterns. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Test as Test (pytest)
participant VCR as VCR Cassette
participant Rails as LLMRails
participant Provider as OpenAI / NIM
Note over Test,Provider: Recorded (replay) path
Test->>VCR: activate cassette
Test->>Rails: generate_async(messages, options)
Rails->>VCR: HTTP POST /chat/completions
VCR-->>Rails: recorded response
Rails-->>Test: GenerationResponse
Test->>Test: "assert == snapshot(...)"
Note over Test,Provider: Deterministic (FakeLLM) path
Test->>Rails: "generate_async(messages, options) llm=FakeLLMModel"
Rails->>Rails: run rail pipeline
Rails-->>Test: GenerationResponse
Test->>Test: "assert == snapshot(...)"
Note over Test,Provider: Live smoke path (maintainer only)
Test->>Rails: generate_async(messages, options)
Rails->>Provider: real HTTP call
Provider-->>Rails: live response
Rails-->>Test: GenerationResponse
Test->>Test: assert contract shape (non-empty content)
%%{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"}}}%%
sequenceDiagram
participant Test as Test (pytest)
participant VCR as VCR Cassette
participant Rails as LLMRails
participant Provider as OpenAI / NIM
Note over Test,Provider: Recorded (replay) path
Test->>VCR: activate cassette
Test->>Rails: generate_async(messages, options)
Rails->>VCR: HTTP POST /chat/completions
VCR-->>Rails: recorded response
Rails-->>Test: GenerationResponse
Test->>Test: "assert == snapshot(...)"
Note over Test,Provider: Deterministic (FakeLLM) path
Test->>Rails: "generate_async(messages, options) llm=FakeLLMModel"
Rails->>Rails: run rail pipeline
Rails-->>Test: GenerationResponse
Test->>Test: "assert == snapshot(...)"
Note over Test,Provider: Live smoke path (maintainer only)
Test->>Rails: generate_async(messages, options)
Rails->>Provider: real HTTP call
Provider-->>Rails: live response
Rails-->>Test: GenerationResponse
Test->>Test: assert contract shape (non-empty content)
Reviews (3): Last reviewed commit: "test(recorded): make characterization do..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| from __future__ import annotations | ||
|
|
||
| import asyncio | ||
| from collections.abc import AsyncIterator |
Contributor
There was a problem hiding this comment.
gen function and the outer helper are both annotated as -> AsyncIterator[str], but an async generator function's correct return type is AsyncGenerator[str, None]. More critically, the tests call await stream.aclose() where stream comes from rails.stream_async(), which is typed as AsyncIterator[str]. AsyncIterator has no aclose() method — that lives on AsyncGenerator. At runtime this works because stream_async is implemented as an async generator, but the annotation mismatch would be caught if pyright coverage were extended to tests. Changing both to AsyncGenerator[str, None] makes the intent explicit and the type accurate.
Suggested change
| from collections.abc import AsyncIterator | |
| from collections.abc import AsyncGenerator, AsyncIterator |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/recorded/rails/public_api/test_stream_cancellation.py
Line: 36
Comment:
The inner `gen` function and the outer helper are both annotated as `-> AsyncIterator[str]`, but an async generator function's correct return type is `AsyncGenerator[str, None]`. More critically, the tests call `await stream.aclose()` where `stream` comes from `rails.stream_async()`, which is typed as `AsyncIterator[str]`. `AsyncIterator` has no `aclose()` method — that lives on `AsyncGenerator`. At runtime this works because `stream_async` is implemented as an async generator, but the annotation mismatch would be caught if pyright coverage were extended to tests. Changing both to `AsyncGenerator[str, None]` makes the intent explicit and the type accurate.
```suggestion
from collections.abc import AsyncGenerator, AsyncIterator
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+51
to
+59
| def _tracking_generator(values: list[str], teardown: dict) -> AsyncIterator[str]: | ||
| """Yield ``values``, then block forever; record teardown via ``finally``. | ||
|
|
||
| ``teardown["closed"]`` flips to True only when the generator is closed | ||
| (``aclose()`` or a cancellation thrown in at the blocking ``await``), which is the | ||
| observable proof that the consumer's abort propagated to the source. | ||
| """ | ||
|
|
||
| async def gen() -> AsyncIterator[str]: |
Contributor
There was a problem hiding this comment.
async def with yield) has a return type of AsyncGenerator[YieldType, SendType], not AsyncIterator. Using AsyncGenerator[str, None] here makes it legal to call aclose() on the returned object under static analysis, and accurately reflects what callers receive.
Suggested change
| def _tracking_generator(values: list[str], teardown: dict) -> AsyncIterator[str]: | |
| """Yield ``values``, then block forever; record teardown via ``finally``. | |
| ``teardown["closed"]`` flips to True only when the generator is closed | |
| (``aclose()`` or a cancellation thrown in at the blocking ``await``), which is the | |
| observable proof that the consumer's abort propagated to the source. | |
| """ | |
| async def gen() -> AsyncIterator[str]: | |
| def _tracking_generator(values: list[str], teardown: dict) -> AsyncGenerator[str, None]: | |
| """Yield ``values``, then block forever; record teardown via ``finally``. | |
| ``teardown["closed"]`` flips to True only when the generator is closed | |
| (``aclose()`` or a cancellation thrown in at the blocking ``await``), which is the | |
| observable proof that the consumer's abort propagated to the source. | |
| """ | |
| async def gen() -> AsyncGenerator[str, None]: |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/recorded/rails/public_api/test_stream_cancellation.py
Line: 51-59
Comment:
The outer function and the inner async generator function both carry the wrong return-type annotation. An async generator function (`async def` with `yield`) has a return type of `AsyncGenerator[YieldType, SendType]`, not `AsyncIterator`. Using `AsyncGenerator[str, None]` here makes it legal to call `aclose()` on the returned object under static analysis, and accurately reflects what callers receive.
```suggestion
def _tracking_generator(values: list[str], teardown: dict) -> AsyncGenerator[str, None]:
"""Yield ``values``, then block forever; record teardown via ``finally``.
``teardown["closed"]`` flips to True only when the generator is closed
(``aclose()`` or a cancellation thrown in at the blocking ``await``), which is the
observable proof that the consumer's abort propagated to the source.
"""
async def gen() -> AsyncGenerator[str, None]:
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+89
to
+111
| async def test_colang_v2_self_check_input_rail_allows_benign_input(openai_api_key): | ||
| """Allowed path — faithful to ``examples/bots/abc_v2`` (real library flows + prompt). | ||
|
|
||
| The config wires the real ``self check input`` library flow | ||
| (``nemoguardrails.library.self_check.input_check``) with abc_v2's verbatim | ||
| ``self_check_input`` prompt, and ``flow main / activate llm continuation`` so every input is | ||
| actually gated by the rail. A benign message passes the policy check ("No") and llm | ||
| continuation produces the answer. Output pinned on record. | ||
| """ | ||
| rails = LLMRails(load_config(COLANG_V2_SELF_CHECK_CONFIG), verbose=False) | ||
|
|
||
| result = await rails.generate_async(messages=[{"role": "user", "content": "Hello, can you help me?"}]) | ||
|
|
||
| assert isinstance(result, dict) | ||
| assert result.get("role") == "assistant" | ||
| assert isinstance(result.get("content"), str) and result["content"].strip() | ||
| assert result == snapshot() | ||
|
|
||
|
|
||
| @pytest.mark.vcr | ||
| async def test_colang_v2_self_check_input_rail_blocks_flagged_input(openai_api_key): | ||
| """Block branch — proves the real ``self check input`` rail actually drives the decision. | ||
|
|
Contributor
There was a problem hiding this comment.
snapshot() call in an xfail(strict=False) test
assert result == snapshot() with no expected value means the inline-snapshot is never set: on every run the comparison fails (empty sentinel ≠ actual dict), the test is recorded as xfail, and the snapshot is never populated. This is workable as a placeholder, but it means a future fix to the Colang 2.x llm continuation path requires two manual steps: (1) remove the xfail marker, and (2) populate the snapshot by running pytest --update-snapshots. There is no automation that links the two, so the snapshot could easily stay empty after the xfail is removed — causing a spurious AssertionError on first run.
Consider either capturing the intermediate output as the snapshot now (so "remove xfail → test passes" is a one-step action), or leaving an explicit TODO comment explaining the required snapshot-update step.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/recorded/rails/public_api/test_colang_v2.py
Line: 89-111
Comment:
**Empty `snapshot()` call in an `xfail(strict=False)` test**
`assert result == snapshot()` with no expected value means the inline-snapshot is never set: on every run the comparison fails (empty sentinel ≠ actual dict), the test is recorded as `xfail`, and the snapshot is never populated. This is workable as a placeholder, but it means a future fix to the Colang 2.x `llm continuation` path requires two manual steps: (1) remove the `xfail` marker, and (2) populate the snapshot by running `pytest --update-snapshots`. There is no automation that links the two, so the snapshot could easily stay empty after the xfail is removed — causing a spurious `AssertionError` on first run.
Consider either capturing the intermediate output as the snapshot now (so "remove xfail → test passes" is a one-step action), or leaving an explicit TODO comment explaining the required snapshot-update step.
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!
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Pouyanpi
force-pushed
the
test/recorded-tests-public-api-surface
branch
from
c82a5fb to
9490eb1
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
Adds the recorded characterization baseline for the existing
LLMRailspublic API and server-facing behavior. This baseline is intended to support behavior-preserving review of the decomposition stack in #2123–#2131.What Changed
tests/recorded.Review Guidance
Use the LLMRails decomposition review matrix as the scenario index. It maps each characterized behavior to an exact test node and identifies which rows should be checked during each decomposition PR.
These tests intentionally characterize current behavior, including compatibility quirks. A failing row during #2123–#2130 should be treated as a potential behavior regression unless the stack explicitly identifies it as an intentional semantic change in #2131.
Related Issue(s)
This is maintainer-directed test infrastructure for the linked LLMRails decomposition stack.
Verification
AI Assistance
Checklist