Skip to content

Nh/hitl conditional interrupts#1

Open
DhirenMhatre wants to merge 10 commits into
masterfrom
nh/hitl-conditional-interrupts
Open

Nh/hitl conditional interrupts#1
DhirenMhatre wants to merge 10 commits into
masterfrom
nh/hitl-conditional-interrupts

Conversation

@DhirenMhatre
Copy link
Copy Markdown

Fixes #


Read the full contributing guidelines: https://docs.langchain.com/oss/python/contributing/overview

All contributions must be in English. See the language policy.

If you paste a large clearly AI generated description here your PR may be IGNORED or CLOSED!

Thank you for contributing to LangChain! Follow these steps to have your pull request considered as ready for review.

  1. PR title: Should follow the format: TYPE(SCOPE): DESCRIPTION
  1. PR description:
  • Write 1-2 sentences summarizing the change.
  • The Fixes #xx line at the top is required for external contributions — update the issue number and keep the keyword. This links your PR to the approved issue and auto-closes it on merge.
  • If there are any breaking changes, please clearly describe them.
  • If this PR depends on another PR being merged first, please include "Depends on #PR_NUMBER" in the description.
  1. Run make format, make lint and make test from the root of the package(s) you've modified.
  • We will not consider a PR unless these three are passing in CI.
  1. How did you verify your code works?

Additional guidelines:

  • All external PRs must link to an issue or discussion where a solution has been approved by a maintainer, and you must be assigned to that issue. PRs without prior approval will be closed.
  • PRs should not touch more than one package unless absolutely necessary.
  • Do not update the uv.lock files or add dependencies to pyproject.toml files (even optional ones) unless you have explicit permission to do so by a maintainer.

Social handles (optional)

Twitter: @
LinkedIn: https://linkedin.com/in/

@codity-dm
Copy link
Copy Markdown

codity-dm Bot commented May 17, 2026

Policy Check Failed

✗ 3/3 policy checks failed:

• Need 2 more approval(s) (0/2) — comment LGTM or approve via review
• Missing ticket reference (expected: JIRA-, ENG-, #*)
• 2 code file(s) changed but no test files added


To merge this PR:

  1. Address the failed checks listed above
  2. Ensure branch protection requires the codity/policy-check status

Configure policies in your dashboard

@codity-dm
Copy link
Copy Markdown

codity-dm Bot commented May 17, 2026

PR Summary

What Changed

  • Added interrupt_when predicate to InterruptOnConfig for conditional HITL interrupts based on runtime tool call context.
  • Introduced ToolRuntime dataclass exposing tool name, arguments, and state to predicate functions.
  • Modified HumanInTheLoopMiddleware to evaluate predicates before triggering interrupts, enabling auto-approval when predicates return False.

Key Changes by Area

Human-in-the-Loop Middleware: Added conditional interrupt logic with _InterruptWhen protocol and _should_interrupt() method that constructs ToolRuntime and evaluates user-defined predicates.

Testing: Added 6 unit tests covering true/false predicates, mixed tool calls, ToolRuntime field verification, and exception handling.

Files Changed

File Changes Summary
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py Added _InterruptWhen protocol, interrupt_when field, ToolRuntime dataclass, and _should_interrupt() method; updated after_model() to check predicates
libs/langchain_v1/tests/unit_tests/agents/middleware/implementations/test_human_in_the_loop.py Added 6 test cases for conditional interrupt predicates and ToolRuntime behavior

Review Focus Areas

  • Predicate exception handling: exceptions in interrupt_when functions propagate as-is. Confirm this matches expected error handling patterns.
  • ToolRuntime field completeness: verify exposed fields (name, args, state) are sufficient for common predicate use cases.
  • Backward compatibility: ensure existing InterruptOnConfig usage without interrupt_when continues to interrupt unconditionally.

Architecture

Design Decisions

  • ToolRuntime is a frozen dataclass to prevent predicate side effects on state. This is intentional immutability at the predicate boundary.
  • Predicate exceptions propagate unwrapped rather than being converted to default interrupt behavior. This is deliberate: silent failures would hide logic errors in user-defined predicates.

Risks

  • Predicate performance: slow interrupt_when functions will block the agent loop. This is an intentional tradeoff. Documented limitation, not a bug.
  • ToolRuntime currently exposes raw state dict. Future state structure changes could break user predicates. Consider versioning or shallow copying if state mutability becomes a concern.

Merge Status

MERGEABLE — PR Score 66/100, above threshold (50). All gates passed.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 17, 2026

Greptile Summary

This PR adds a conditional-interrupt feature to HumanInTheLoopMiddleware: an optional interrupt_when predicate on InterruptOnConfig that receives the ToolCall and a slim ToolRuntime and returns True to interrupt or False to auto-approve. Eight new unit tests cover the happy path, mixed-tool scenarios, runtime field forwarding, and exception propagation.

  • New _InterruptWhen Protocol and _should_interrupt helper build a restricted ToolRuntime (with tools=[], config={}, execution_info=None, server_info=None) for predicate evaluation, preserving all pre-existing interrupt behavior when no predicate is supplied.
  • after_model refactor replaces a single if-block with a two-check guard (config is None → continue, not _should_interrupt → continue), keeping order-preserving decision replay intact.

Confidence Score: 3/5

Safe to merge after addressing the async-predicate silent-failure in _should_interrupt; all other changes are well-tested and structurally sound.

The new _should_interrupt helper calls interrupt_when(tool_call, tool_runtime) and returns the result directly. If a caller passes an async def as the predicate, the call returns an unawaited coroutine object which is always truthy, so every such tool call interrupts regardless of predicate logic, with no error and only a background RuntimeWarning. The rest of the implementation is well-structured and the 8 new tests give good coverage of the synchronous cases.

libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py — specifically the return statement at the end of _should_interrupt.

Important Files Changed

Filename Overview
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py Adds interrupt_when predicate support to InterruptOnConfig and a _should_interrupt helper; async predicates silently always-interrupt due to truthy coroutine object being returned without a guard.
libs/langchain_v1/tests/unit_tests/agents/middleware/implementations/test_human_in_the_loop.py Adds 8 focused tests covering predicate-true, predicate-false, mixed tool calls, runtime field forwarding, and exception propagation; async predicate failure mode is not covered.

Reviews (1): Last reviewed commit: "test(langchain): assert `ToolRuntime.con..." | Re-trigger Greptile

Comment on lines +276 to +290
interrupt_when = config.get("interrupt_when")
if interrupt_when is None:
return True
tool_runtime: ToolRuntime[ContextT, Any] = ToolRuntime(
state=state,
context=runtime.context,
config={},
stream_writer=runtime.stream_writer,
tool_call_id=tool_call["id"],
store=runtime.store,
tools=[],
execution_info=None,
server_info=None,
)
return interrupt_when(tool_call, tool_runtime)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Async predicate silently bypasses user logic

If a user passes an async def function as interrupt_when, calling interrupt_when(tool_call, tool_runtime) returns a coroutine object rather than a bool. Python coroutine objects are always truthy, so _should_interrupt returns True unconditionally — the interrupt fires regardless of what the predicate was intended to decide, and the coroutine is never awaited (producing only a background RuntimeWarning). Since aafter_model delegates synchronously to after_model, there is no path to correctly handle async predicates. A runtime guard after the call (e.g. asyncio.iscoroutine(result)) would surface this as a clear TypeError rather than silent wrong behavior.

Suggested change
interrupt_when = config.get("interrupt_when")
if interrupt_when is None:
return True
tool_runtime: ToolRuntime[ContextT, Any] = ToolRuntime(
state=state,
context=runtime.context,
config={},
stream_writer=runtime.stream_writer,
tool_call_id=tool_call["id"],
store=runtime.store,
tools=[],
execution_info=None,
server_info=None,
)
return interrupt_when(tool_call, tool_runtime)
import asyncio
result = interrupt_when(tool_call, tool_runtime)
if asyncio.iscoroutine(result):
result.close()
msg = (
"interrupt_when predicate returned a coroutine. "
"Predicates must be synchronous; pass a plain `def`, not `async def`."
)
raise TypeError(msg)
return bool(result)

@codity-dm
Copy link
Copy Markdown

codity-dm Bot commented May 17, 2026

Security Scan Summary

Metric Value
Vulnerabilities Critical: 0
Overall Risk Clean
Files Scanned 2

No critical security issues detected

Scan completed in 23.8s

Security scan powered by Codity.ai

@codity-dm
Copy link
Copy Markdown

codity-dm Bot commented May 17, 2026

License Compliance Scan

Metric Value
Packages Scanned 0
High Risk (Strong Copyleft) 0
Medium Risk (Weak Copyleft) 0
Low Risk (Permissive) 0
Unknown License 0

All licenses are low-risk and compliant

Powered by Codity.ai · Docs

@codity-dm
Copy link
Copy Markdown

codity-dm Bot commented May 17, 2026

Code Quality Report — test-org-codity/langchain · PR #1

Scanned: 2026-05-17 19:17 UTC | Score: 44/100 | Provider: github

Executive Summary

Severity Count
Critical 0
High 2
Medium 3
Low 18
Top Findings

[CQ-LLM-002] libs/langchain_v1/tests/unit_tests/agents/middleware/implementations/test_human_in_the_loop.py:884 (Duplication · HIGH)

Issue: The lambda functions used for interrupt_when in multiple tests are similar and could be extracted to a common function.
Suggestion: Extract the lambda functions into named functions to adhere to DRY principles.

lambda _tc, _rt: True

[CQ-004] libs/langchain_v1/tests/unit_tests/agents/middleware/implementations/test_human_in_the_loop.py:1093 (Error_Handling · HIGH)

Issue: Exception silently swallowed with pass
Suggestion: Log the exception or re-raise it

pass

[CQ-LLM-001] libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py:131 (Complexity · MEDIUM)

Issue: The _should_interrupt function has multiple levels of nesting, which can increase complexity.
Suggestion: Consider flattening the logic or breaking it into smaller functions to reduce nesting.

if interrupt_when is None:
            return True
        tool_runtime: ToolRuntime[ContextT, Any] = ToolRuntime(
            state=state,
            context=runtime.context,
            config={},
            stream_writer=runtime.stream_writer,
            tool_call_id=tool_call["id"],
            store=runtime.store,
            tools=[],
            execution_info=None,
            server_info=None,
        )
        return interrupt_when(tool_call, tool_runtime)

[CQ-LLM-003] libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py:191 (Error_Handling · MEDIUM)

Issue: The _should_interrupt function does not handle potential exceptions that may arise from the interrupt_when predicate.
Suggestion: Wrap the call to interrupt_when in a try-except block to handle any exceptions gracefully.

return interrupt_when(tool_call, tool_runtime)

[CQ-LLM-005] libs/langchain_v1/tests/unit_tests/agents/middleware/implementations/test_human_in_the_loop.py:884 (Testability · MEDIUM)

Issue: The tests use hard-coded lambda functions, which can make them less flexible and harder to maintain.
Suggestion: Consider using mock functions or predefined predicates to improve testability.

lambda _tc, _rt: True

[CQ-LLM-004] libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py:131 (Documentation · LOW)

Issue: The _should_interrupt function lacks a docstring explaining its purpose and parameters.
Suggestion: Add a docstring to the _should_interrupt function to improve code documentation.

def _should_interrupt(self, tool_call: ToolCall, config: InterruptOnConfig, state: AgentState[Any], runtime: Runtime[ContextT]) -> bool:

[CQ-002] libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py:235 (Complexity · LOW)

Issue: Deep nesting detected (depth ~5)
Suggestion: Extract nested blocks into helper functions

The `InterruptOnConfig` can include:

[CQ-002] libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py:236 (Complexity · LOW)

Issue: Deep nesting detected (depth ~5)
Suggestion: Extract nested blocks into helper functions

- a `description` field (`str` or `Callable`) for custom formatting

[CQ-002] libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py:237 (Complexity · LOW)

Issue: Deep nesting detected (depth ~5)
Suggestion: Extract nested blocks into helper functions

of the interrupt description, and

[CQ-002] libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py:238 (Complexity · LOW)

Issue: Deep nesting detected (depth ~5)
Suggestion: Extract nested blocks into helper functions

- an `interrupt_when` predicate `(ToolCall, ToolRuntime) -> bool`.

Per-File Breakdown

File Critical High Medium Low Total
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py 0 0 2 10 12
libs/langchain_v1/tests/unit_tests/agents/middleware/implementations/test_human_in_the_loop.py 0 2 1 8 11

Recommendations

  1. Resolve High severity issues, especially error handling gaps and performance bottlenecks.
  • Run automated tests after applying fixes to verify no regressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants