From c7c621213fd8b9e3c80f3e824382d4bc106f789b Mon Sep 17 00:00:00 2001
From: roha4naik <naikrohan2004@gmail.com>
Date: Fri, 3 Jul 2026 12:04:17 +0530
Subject: [PATCH] fix: surface child research errors in supervisor_tools

supervisor_tools guarded its ConductResearch exception handler with
`if is_token_limit_exceeded(...) or True:`. The `or True` made the
branch fire for every exception, so any child researcher failure was
silently treated as a normal end of the research phase. The graph then
continued to final_report_generation and could produce a final report
from failed/incomplete research.

Remove the `or True`: token-limit errors still end the phase gracefully
with the notes gathered so far, but any other exception is now re-raised
and surfaced instead of being swallowed. This mirrors the error handling
in final_report_generation, which also distinguishes token-limit errors
from genuine failures.

Add regression tests covering both paths.

Fixes #283
---
 src/open_deep_research/deep_researcher.py | 11 ++--
 tests/test_supervisor_tools.py            | 64 +++++++++++++++++++++++
 2 files changed, 72 insertions(+), 3 deletions(-)
 create mode 100644 tests/test_supervisor_tools.py

diff --git a/src/open_deep_research/deep_researcher.py b/src/open_deep_research/deep_researcher.py
index 279dbffd9..f0045cf79 100644
--- a/src/open_deep_research/deep_researcher.py
+++ b/src/open_deep_research/deep_researcher.py
@@ -330,9 +330,9 @@ async def supervisor_tools(state: SupervisorState, config: RunnableConfig) -> Co
                 update_payload["raw_notes"] = [raw_notes_concat]
                 
         except Exception as e:
-            # Handle research execution errors
-            if is_token_limit_exceeded(e, configurable.research_model) or True:
-                # Token limit exceeded or other error - end research phase
+            # Token limit exceeded: end the research phase gracefully and
+            # proceed with whatever notes have been gathered so far.
+            if is_token_limit_exceeded(e, configurable.research_model):
                 return Command(
                     goto=END,
                     update={
@@ -340,6 +340,11 @@ async def supervisor_tools(state: SupervisorState, config: RunnableConfig) -> Co
                         "research_brief": state.get("research_brief", "")
                     }
                 )
+            # Any other error is a genuine failure in a child researcher and
+            # must be surfaced rather than silently treated as a successful
+            # end of the research phase (which would otherwise still proceed
+            # to final report generation from incomplete research).
+            raise
     
     # Step 3: Return command with all tool results
     update_payload["supervisor_messages"] = all_tool_messages
diff --git a/tests/test_supervisor_tools.py b/tests/test_supervisor_tools.py
new file mode 100644
index 000000000..0e3ff1a74
--- /dev/null
+++ b/tests/test_supervisor_tools.py
@@ -0,0 +1,64 @@
+"""Unit tests for `supervisor_tools` research-phase error handling.
+
+Regression tests for the bug where any child `ConductResearch` exception was
+treated as a successful end of the research phase (see issue #283). A genuine
+child-researcher failure must be surfaced, while a token-limit error should
+still end the phase gracefully with whatever notes were gathered.
+"""
+
+import asyncio
+from unittest.mock import patch
+
+import pytest
+from langchain_core.messages import AIMessage
+from langgraph.graph import END
+
+from open_deep_research import deep_researcher
+from open_deep_research.deep_researcher import researcher_subgraph, supervisor_tools
+
+
+def _supervisor_state():
+    """Build minimal supervisor state with a single ConductResearch tool call."""
+    return {
+        "supervisor_messages": [
+            AIMessage(
+                content="",
+                tool_calls=[
+                    {
+                        "name": "ConductResearch",
+                        "args": {"research_topic": "test topic"},
+                        "id": "call-1",
+                        "type": "tool_call",
+                    }
+                ],
+            )
+        ],
+        "research_iterations": 1,
+        "research_brief": "brief",
+    }
+
+
+_CONFIG = {"configurable": {"research_model": "openai:gpt-4.1"}}
+
+
+async def _boom(*args, **kwargs):
+    raise RuntimeError("boom from child researcher")
+
+
+def test_child_research_error_is_surfaced():
+    """A non-token-limit child failure must propagate, not end as success."""
+    with patch.object(researcher_subgraph, "ainvoke", side_effect=_boom):
+        with pytest.raises(RuntimeError, match="boom from child researcher"):
+            asyncio.run(supervisor_tools(_supervisor_state(), _CONFIG))
+
+
+def test_token_limit_ends_research_phase_gracefully():
+    """A token-limit error should end the research phase with gathered notes."""
+    with (
+        patch.object(researcher_subgraph, "ainvoke", side_effect=_boom),
+        patch.object(deep_researcher, "is_token_limit_exceeded", return_value=True),
+    ):
+        command = asyncio.run(supervisor_tools(_supervisor_state(), _CONFIG))
+
+    assert command.goto == END
+    assert "research_brief" in command.update