refactor(code): validate ask_user args with pydantic instead of a custom error - #5682
Merged
Merged
Conversation
github-actions
Bot
added
dcode
![open-swe[bot]](https://avatars.githubusercontent.com/in/1395372?s=60&v=4){kind=small}
Mason Daugherty (mdrxy)
changed the title
ask_user args with pydantic instead of a custom error
Mason Daugherty (mdrxy)
added a commit
that referenced
this pull request
Aug 20, 2026
`min_length=1` accepts whitespace-only strings, so `{"question": " "}`
reached `interrupt()` and rendered a visually blank prompt instead of a
retryable validation error. Add an `AfterValidator` on `question` requiring a
non-whitespace character; the empty string now lands on the same "blank"
rejection since the validator runs before `min_length` is consulted.
Addresses PR #5682 review discussion_r3818509589.
…tom error Prompted by review on #5659: the schema can carry the validation, so the bespoke recovery path is unnecessary. Move the `ask_user` argument rules out of the imperative `_validate_questions` body and onto the tool schema — `Literal`/`strict` bool/`min_length` for the per-field rules, `AfterValidator` for the cross-field ones (blank choice values, the `multi_select` comma ban, choice/non-choice `choices` consistency) and for the non-empty-list rule on `questions` itself. A rejection now surfaces as a pydantic `ValidationError` during argument parsing, which `ToolNode` already converts to an error `ToolMessage` — the same retry loop the model previously got only for schema-shape errors. The curated message text is preserved via the tool's `handle_validation_error` callback (`` `ask_user` failed: <loc>: <detail>. Fix the input and retry. ``). That makes the #5659 machinery redundant, so it is removed: the `ToolErrorMiddleware` instance, its `_tool_arg_validation_on_error` handler, the `_TOOL_ARG_VALIDATION_TOOLS` scope list, and the `ToolArgumentError` class. Recovery no longer keys off a custom exception type at all; anything pydantic rejects is model-fixable, and anything the tool body raises after parsing (a `ValueError` from an internal invariant, or the `interrupt()` control-flow signal) still propagates and halts the run. The raw-payload backstop in `auto_mode._ask_user_question_count` is unchanged: it reads the unvalidated tool-call args off message history and is independent of how the tool validates. Behavior is unchanged for the model-facing path; this is a refactor, so no changelog entry is warranted beyond the commit itself.
`min_length=1` accepts whitespace-only strings, so `{"question": " "}`
reached `interrupt()` and rendered a visually blank prompt instead of a
retryable validation error. Add an `AfterValidator` on `question` requiring a
non-whitespace character; the empty string now lands on the same "blank"
rejection since the validator runs before `min_length` is consulted.
Addresses PR #5682 review discussion_r3818509589.
Mason Daugherty (mdrxy)
force-pushed
the
optimize-pr-5659-discussi
branch
from
August 20, 2026 16:14
7233386 to
02050cc
Compare
`_format_validation_error` reported every tool-call `ValidationError` to the model as its own bad input. Two kinds of failure reach that handler which the model cannot act on: - `tool_call_id` and `runtime` are harness-injected but sit on the same `args_schema` as `questions`, so pydantic rejects them the same way. LangGraph filters these in `_filter_validation_errors`; registering `handle_validation_error` bypasses that filter. - `BaseTool.run` runs the tool body inside the same `try` as argument parsing, so a `ValidationError` raised after the interrupt resumes lands there too. The handler now keeps only errors that name a model-authored argument and re-raises when none do, so both cases stay fatal. It also logs every rejection: the run survives one, and the handled error makes `BaseTool.run` call `on_tool_end`, so nothing else records that the model sent bad arguments. `AskUserRequest.questions` becomes `list[ValidatedQuestion]` so the cross-field `choices` rules also apply where `tui.textual_adapter` re-validates the interrupt payload. Docstring corrections: `min_length=1` runs after the `AfterValidator`, not before, so it constrains only the advertised JSON schema; `_validate_choice` cannot see a missing `value`; and the tool's own `handle_validation_error`, not `ToolNode`, builds the error `ToolMessage`.
The tool set `handle_validation_error` to format its own error text. That field is not part of the documented LangChain v1 surface, and the v1 migration guide says to leave schema mismatches to the framework. Setting it also costs three things, because it intercepts inside `BaseTool.run`: - It bypasses `_filter_validation_errors`, so an error on a harness-injected argument (`runtime`, `tool_call_id`) reaches the model as its own bad input. The model cannot rewrite those fields, so it retries until the recursion limit. - The handled error leaves `error_to_raise` unset, so `BaseTool.run` calls `on_tool_end`. Tracing records a rejected call as a success. - The formatter named only the first error, so the model corrected one rule per retry. `ToolNode` already does this conversion, strips the injected arguments first, lists every error, and lets `on_tool_error` fire. So the tool now wires no error handling and the custom formatter is deleted. The model-facing text changes. It is the framework default, which echoes the model's own arguments and ends with "Please fix the error and try again".
Argument validation moved onto the tool's pydantic schema, but the tool-level wiring was untested and three framework claims in the comments were wrong. Enforcement: - Guard the tool body against a `ValidationError`. `ToolNode` wraps the body in the same `try` as argument parsing, so one escaping from there is reported to the model as *its* bad input — naming fields absent from the schema, against arguments the model wrote correctly, while the user's answer is discarded and the run continues. Re-raised as `RuntimeError`, a type `_default_handle_tool_errors` refuses to convert. Nothing raises one today; the guard is for the next edit. `TestBodyFaultsStayFatal` previously claimed this guarantee while covering only `ValueError` and `RuntimeError`. - Carry `_validate_questions` onto `AskUserRequest.questions`. The empty-list rule lived only on the tool parameter, so `TypeAdapter(AskUserRequest)` accepted `questions: []` and `AskUserMenu([])` rendered a titled prompt with no question widgets and nothing focusable. - Log a rejected call in `wrap_tool_call`. The deleted `_tool_arg_validation_on_error` said it was "the only record that the model sent bad arguments"; `tool_node.py` has no logger, and the repo's only callback handler implements LLM events only, so a model looping on malformed arguments left no operator-visible record. The result type is the discriminant: the tool always returns a `Command`, so a `ToolMessage` means the call never entered the body. `awrap_tool_call` is defined too, so the async path keeps executing tools asynchronously. Comments: - `runtime` and `tool_call_id` stay out of the model-facing message by two different mechanisms, and neither covers the other. `_filter_validation_errors` builds its name set from state/store/runtime only, so it drops `runtime` but does not know about `InjectedToolCallId`; `tool_call_id` stays out because `ToolInvocationError` is built from the pre-injection `call["args"]`. - `_validate_choice` is attached to the item annotation inside `Question.choices`, not to `Choice`, so `TypeAdapter(Choice)` does not apply it. - `ValidatedQuestion` closed by describing only the tool path, contradicting the paragraph above it about client re-validation, where the outcome is a re-raise. - `_validate_question_text` is the only rule that *rejects* blank text; `_ask_user_question_count` also tests it but degrades. Says outright not to reorder the annotation and the `Field` beside it. Tests. Reverting the tool's annotation to `list[Question]` previously left the suite green: both cross-field rules were covered only against a parallel `TypeAdapter`, never through the tool. Adding the two tool-level cases makes that mutant fail. Also pinned through a real invocation: blank question text, `required: "false"`, and `minLength` reaching the model-facing schema. `_filter_validation_errors` now has a load-bearing test. The end-to-end assertions could not cover it — `ToolNode` injects both arguments correctly on every real call, so neither is ever the field that failed, and both assertions held even with the filtering bypassed. That test's docstring now says it checks message shape, and the filtering is forced at the unit level instead. Adds a `TypeAdapter(AskUserRequest)` class for the client boundary, which had no coverage at all. The zero-question case in `test_ask_user_cancelled_marks_row_rejected_and_halts` is dropped: an empty list is no longer representable there, and zero took the same singular branch as one. Every fix above is mutation-tested.
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.
A malformed
ask_usercall still surfaces to the model as a retryable errorToolMessage. The validation now lives on the tool's pydantic schema. The hand-rolled validation function, theToolErrorMiddleware, theToolArgumentErrortype, theon_errorhandler, and the scope list are all deleted.The rules on the schema
Per-field, on
Question:questionhas anAfterValidatorthat rejects blank and whitespace-only text. The field keepsmin_length=1, but that constrains only the JSON schema the model reads. The validator is the inner annotation, so it runs first and is the only runtime gate.typestays aLiteral.requiredstays a strict bool.Choicehas anAfterValidatorthat rejects a blankvalue. A blank value renders as an unlabelled option whose answer reads as "no answer".Cross-field, on the
ValidatedQuestionalias and thequestionsparameter:choiceslist.choices.questionsmust not be empty.AskUserRequest.questionsislist[ValidatedQuestion]too. The cross-field rules then also apply wheretui.textual_adapterre-validates the interrupt payload withTypeAdapter(AskUserRequest). Before this, a choice question with nochoicesreached the client and degraded to a text box.The tool wires no error handling
A rejection is a pydantic
ValidationErrorraised during argument parsing.ToolNodeconverts it to an errorToolMessage. The tool sets neitherhandle_validation_errornorhandle_tool_error, and that is deliberate.An earlier revision of this PR set
handle_validation_errorto get a shorter message. That field is not part of the documented LangChain v1 surface, and the v1 migration guide lists schema mismatches under "do NOT handle — already auto-handled by the framework". It also intercepts insideBaseTool.run, which costs three things:Harness faults reach the model.
tool_call_idandruntimeare on the sameargs_schemaasquestions, so pydantic reports them the same way. LangGraph strips them in_filter_validation_errors; a handler on the tool runs earlier and bypasses that. The model cannot rewrite those fields, so it retries until the recursion limit.Tracing records a success. A handled error leaves
error_to_raiseunset, soBaseTool.runcallson_tool_end. With no handler the exception escapes,on_tool_errorfires, and a rejected call traces as an error.The model corrects one rule per retry. The formatter named only the first error.
ToolNodelists all of them.Measured on one call that breaks two rules:
The message is longer and echoes the model's own arguments. That is the cost of the framework default, and it buys correct tracing, injected-argument filtering, and one retry instead of two.
Known limitation
A
ValidationErrorraised by the tool body after parsing succeeds is still reported to the model as bad input.BaseTool.runandToolNodeboth wrap the body in the sametryas argument parsing, so this is framework behavior, not specific toask_user. Nothing in the body raises one today:_parse_answersraises plainValueError, which stays fatal.Unchanged
The raw-payload backstop in
auto_mode(_ask_user_question_countre-reading unvalidated tool-call args off message history) does not depend on how the tool validates. It still guards the same-turn authorization path.