Repository navigation
Conversation
There was a problem hiding this comment.
Self-review pass on this branch turned up one bug, now fixed in 58650de.
_thread_id fell back to runtime.config, which LangGraph's Runtime does not declare in any 1.x release, confirmed via dataclasses.fields. The execution_info leg does not cover the gap: on 1.1.3 the field exists but carries no thread_id, so the conversation ID was unset on everything up to somewhere in 1.1.4 to 1.1.9, which includes every version langchain 1.0.x allows. Measured across 1.0.0, 1.0.10, 1.1.2, 1.1.3, 1.1.9, 1.2.0 and 1.2.7 with real checkpointed StateGraph runs: both legs returned None on the first four.
It now falls back to langgraph.config.get_config(), which returns the thread ID on every version tested, and catches the RuntimeError that call raises outside a runnable context. The old test pinned a SimpleNamespace(execution_info=None, config=...) runtime that LangGraph never produces; it now runs the hooks inside a real node of a checkpointed graph.
Two things I am leaving open rather than changing, happy to take direction:
- The round-trip ID is keyed on the AI-message count, so a trimming middleware such as
SummarizationMiddlewarecan hold that count steady and make later calls rejoin a stale ID. Either key it on something trimming cannot rewind, or document the limitation. - Tool legs are out of scope here. Once #2499 lands on the same middleware its tool checks run outside this round-trip scope, so pairing for tool legs needs deciding in one place or the other.
74c44b6 to
a48f994
Compare
a48f994 to
5f0f139
Compare
AsyncWorkQueue workers ran every item in the context they started in. A context variable set by the caller of generate_async or check_async never reached the rails, and one set by the first caller (whose submit starts the workers lazily) was frozen into every later request. Each item now runs in a copy of its submitter's context, so request-scoped context variables reach the work and changes made by one item do not leak into the next. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
Rails that report to an external service have no way to learn which conversation a request belongs to: IORails actions receive only the request messages, and the context message role does not reach them. nemoguardrails.context now holds a conversation_id_var, a conversation_id() context manager that sets it for the calls made inside the block, and a get_conversation_id() getter for rails and actions. It works for generate, stream and check on both engines, is never added to the messages, and is restored when the block exits. Concurrent requests each see their own ID. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
A chat completion that names a thread now runs its rails inside conversation_id(thread_id), so rails and actions that report to an external service can group the checks of one conversation with get_conversation_id(). This covers generate and stream; for streams each chunk is pulled with the ID set, so rails that run while the response body is sent, such as streaming output rails, see it too. An ID the application running the server already set is kept. Requests without a thread_id see no ID, and the Colang 2.x rejection and thread history replay are unchanged. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
GuardrailsMiddleware now runs its input and output checks with the graph's thread ID (runtime.execution_info.thread_id, else runtime.config["configurable"]["thread_id"]) as the request-scoped conversation ID, so rails that group checks by conversation see one conversation per thread. A conversation ID the application already set is kept, and nothing is set outside a checkpointed run. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
A rail that reports to an external service has to pair the check of what goes to the model with the check of what comes back, and the engine is the only thing that knows the two halves belong to the same call. `nemoguardrails.context` now holds a `roundtrip_id_var`, a `roundtrip_id()` context manager for callers that know where a model call starts, a `new_roundtrip_id()` that mints one only when nothing is set, and a `get_roundtrip_id()` getter for rails and actions. `generate_async`, `stream_async` and the two `IORails` queue entry points open a round trip, so a caller that runs a whole call through one of them has nothing to set. A stream resumes in the context of whoever pulls the chunk, which for an HTTP response body is not the caller that created it. Both IDs are therefore captured where the stream is created and re-applied around each pull rather than held across the yields of the generator, so a library caller that stores an iterator and drains it elsewhere still runs its rails with the conversation and round trip the request was made in. An agent framework is the case the engine cannot see: `GuardrailsMiddleware` runs the two legs of one model call as separate `check_async` calls in separate graph nodes, so it carries the ID in the agent state and publishes it for the duration of each check. The ID is recorded against the model call it was opened for: the first hook to reach a call opens the round trip and the rest join it, and an ID left behind by an earlier call is ignored. That keeps both legs on one ID when `InputRailsMiddleware` is composed with `OutputRailsMiddleware`, and stops a checkpointed thread reporting a turn's response leg under the previous turn's ID. The after-model hook prefers an ID the application set. The new tests run real agents built with `create_agent`, which is what exercises the state schema that carries the ID between graph nodes. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
The conversation ID fell back to runtime.config["configurable"]["thread_id"] when the runtime carried no execution info, but a LangGraph Runtime has no config attribute, and the releases that langchain 1.0 pins have no execution info either, so on those releases the input and output checks of a checkpointed run reported no conversation ID at all. The fallback now reads the thread ID of the run in progress from langgraph.config.get_config(), which every release populates for the node being executed, and still yields nothing outside a run. The test that pinned the old fallback described a runtime LangGraph does not produce; both shapes are now exercised from inside a real graph invoked on a thread. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
thread_id was length-checked only. It is the conversation ID of every rail that runs for the request, and a rail that reports to an external service puts it on the wire, where a control character or surrounding whitespace makes the message unsendable. A thread ID ending in a space therefore failed every check made for that conversation, or, with a rail configured to fail open, silently disabled it. The server now rejects a thread ID that is not printable ASCII or that is surrounded by whitespace, so the caller learns at the boundary. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
LangGraph binds the run config to a node's context by creating the node's task with asyncio's context argument, which arrived in Python 3.11. Below that an async node runs without it, get_config() raises, and the fallback that reads the thread ID from the run config yields nothing, so checks given a runtime that carries no thread ID of its own report no conversation. A runtime that does carry one still names the conversation on every supported Python, and so does the run config on the sync path, which is now pinned too. The async expectation follows what each version can deliver. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
…tors Behavior is unchanged on every production path. - stream_with_request_ids takes a str round-trip ID and always applies it: both engines pass a minted ID, so the None guard only served a test fake, which now passes ensure_roundtrip_id() like the engines do. - _thread_id_from_run_config drops the isinstance guards kept from the getattr(runtime, "config") draft; get_config() returns a RunnableConfig or raises RuntimeError, which is already caught. - _roundtrip_id_for_call returns the recorded ID directly: the before-model hook writes the ID and its call index together, and the ID is never empty. - LLMRails._generate_async takes keyword-only arguments without defaults, since its only caller passes all five. - The middleware API reference no longer calls OutputRailsMiddleware's abefore_model a no-op; it now opens the round trip for the output check. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
…nerator stream_async(generator=...) returned the output-rail iterator directly, before the point where the non-generator path mints a round-trip ID and captures the conversation ID. Those rails run lazily, as the consumer pulls chunks, so they ran under whatever context drained the response body: no round trip at all, and either no conversation or, when the consumer had set one of its own, somebody else's. A rail that reports to an external service filed its checks under the wrong conversation and no one saw an error. The branch now captures the IDs of the creating request and applies them around each pull, the same shape the other streaming paths use, because an async generator resumes in its consumer's context and cannot hold a token across a yield. A round-trip ID the caller already opened is kept rather than replaced. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
5f0f139 to
f6f9f37
Compare
The thread ID validator rejected every character outside printable ASCII, so a deployment naming threads in its own language started getting a 422 on requests that had worked. The field had accepted any string of 16 to 255 characters. A rail percent-encodes the ID before it goes on the wire, so a non-ASCII ID needs no protecting. A control character or surrounding whitespace is almost always a client defect, and is still rejected where the caller learns why. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
The hook minted the round-trip ID whether or not it had an input check to pair it with, so a middleware configured for output only started writing two keys into the graph state on every model call and driving an event loop where it had previously returned at once. OutputRailsMiddleware is the case that shows it. The ID is still what pairs the two legs when both run. With no before-model leg to pair with, the output check opens the round trip itself rather than reporting under no ID at all. Also imports langgraph lazily. It is not a declared dependency of this package, and reading a LangGraph run config is the only thing that needs it. Restores the assertions in the existing middleware tests that this had changed. Signed-off-by: Justin Brooks <justin.lance.brooks@gmail.com>
|
@tgasser-nv, per our conversation in HiddenLayer meeting |
Description
Rails and actions cannot tell which conversation a request belongs to, or which two checks are the two legs of one model call. A rail that reports to an external service sees a stream of unrelated checks, and nothing can pair a request with its response.
This adds two request-scoped context variables,
conversation_idandroundtrip_id, set by the embedding application and read by rails and actions. No public API signature changes.This adds no server-side state
Worth stating plainly, because it was the main concern on the 28 Oct call: Guardrails keeps scaling statelessly. NeMo Guardrails does not interpret either ID, does not pass them to a model, and does not store them. They are labels for the duration of a block, carried in context variables, and nothing persists them.
The identity already exists at the boundary. The server accepts
guardrails.thread_id, and the LangChain middleware runs on a LangGraph thread. This stops that identity being thrown away before a rail can see it. The server derives the conversation ID from thethread_idthe request already carries, so a deployment using threads gets this with no client change, and an ID the application set itself wins over the request's thread.Storing prior turns server-side so a client need not resend them is a different feature, and we agreed on the call that it belongs at the deployment layer rather than here: a horizontally scaled Guardrails behind a load balancer would need a persistent store the compute nodes read from, or consistent hashing, and that is a NeMo Helix question. This PR deliberately does not go there. Parking it as discussed; we are looking at Helix and will bring a proposal.
thread_idis now rejected if it holds a control character or leading or trailing whitespace. An ID that was only ever a datastore key can now leave the process in an HTTP header, and an unsendable one would fail checks that should pass.Three fixes make the IDs reach a rail
AsyncWorkQueueran each work item in the worker's context rather than the submitter's, so any context variable a caller set was dropped on the way in. The queue now copies the submitter's context. This stands on its own: it applies to every context variable, not only these two.IORails.generate_asyncandcheck_asyncopen a round trip, since one call covers both legs of a model call. A caller that already opened one keeps its ID, which is what lets an agent framework pair two separatecheck_asynccalls.Related Issue(s)
Raised as its own PR at Monika's request on 19 Oct ("agree session ids is important, love the idea, please do a separate PR").
Verification
make pre-commitandmake testpass, rebased on currentdevelop: 12821 passed, 201 skipped.Six failures remain, in
tests/server/test_api.py,tests/server/test_schema_utils.pyandtests/integrations/langchain/llm/test_langchain_integration.py. They reproduce on a cleandevelopcheckout in the same environment and are unrelated: they reach the network and fail on aNotFoundErrorfrom the OpenAI client.New tests cover both IDs across LLMRails, IORails, the server, the LangChain middleware and streaming, including concurrent requests each seeing their own ID.
AI Assistance
Checklist