fix: implement CodeRabbit suggestions

- Use proper fixture naming and declaration convention
- Avoid possessive 's with inanimate objects
- Hardcoded name breaks consistency with emit_thought_start and emit_thought_chunk
- Assert the END events in the concurrent lifecycle test.
- Drop the unused make_intermediate_step fixture fom tests
- Instantiate the profiler callback per request, or protect state mutations with locks and add cleanup

Signed-off-by: Patrick Chin <8509935+thepatrickchin@users.noreply.github.com>

Author: thepatrickchin

docs/source/build-workflows/workflow-configuration.md

@@ -75,7 +75,7 @@ The `functions` section contains the tools used in the workflow, in our example
 
 Agent responses in the NeMo Agent Toolkit UI include a thought process display which shows a step-by-step view of what the workflow is doing.
 
-To customize the text shown for a function in the thought process display, set `thought_description` in that function's configuration. Unlike `description`, which provides the tool's help text that the agent uses to decide when and how to call the tool, `thought_description` only affects the label shown in the thought process display. For example:
+To customize the text shown for a function in the thought process display, set the `thought_description` attribute. Unlike `description`, which provides help text that the agent uses to decide when and how to call the tool, `thought_description` only affects the message shown in the thought process display UI as well as in observability traces. For example:
 
 ```yaml
 functions:

packages/nvidia_nat_core/src/nat/builder/thought.py

@@ -87,7 +87,7 @@ def emit_thought_start(context: "Context", thought_text: str, name: str | None =
     return thought_uuid
 
 
-def emit_thought_chunk(context: "Context", thought_uuid: str, thought_text: str) -> None:
+def emit_thought_chunk(context: "Context", thought_uuid: str, thought_text: str, name: str | None = None) -> None:
     """Emit an update to a streaming thought started with emit_thought_start().
 
     This updates the thought text in the UI, useful for showing progress updates.
@@ -96,24 +96,33 @@ def emit_thought_chunk(context: "Context", thought_uuid: str, thought_text: str)
         context: The NAT context object (obtain via Context.get())
         thought_uuid: The UUID returned from emit_thought_start()
         thought_text: The updated thought text to display
+        name: Optional name for the thought (defaults to "custom_thought", should match emit_thought_start)
     """
+    thought_name = name or "custom_thought"
+
     context.intermediate_step_manager.push_intermediate_step(
         IntermediateStepPayload(UUID=thought_uuid,
                                 event_type=IntermediateStepType.SPAN_CHUNK,
-                                name="custom_thought",
+                                name=thought_name,
                                 data=StreamEventData(chunk=thought_text)))
 
 
-def emit_thought_end(context: "Context", thought_uuid: str, thought_text: str | None = None) -> None:
+def emit_thought_end(context: "Context",
+                     thought_uuid: str,
+                     thought_text: str | None = None,
+                     name: str | None = None) -> None:
     """Complete a streaming thought started with emit_thought_start().
 
     Args:
         context: The NAT context object (obtain via Context.get())
         thought_uuid: The UUID returned from emit_thought_start()
         thought_text: Optional final thought text (if None, keeps the last chunk text)
+        name: Optional name for the thought (defaults to "custom_thought", should match emit_thought_start)
     """
+    thought_name = name or "custom_thought"
+
     context.intermediate_step_manager.push_intermediate_step(
         IntermediateStepPayload(UUID=thought_uuid,
                                 event_type=IntermediateStepType.SPAN_END,
-                                name="custom_thought",
+                                name=thought_name,
                                 data=StreamEventData(output=thought_text)))

packages/nvidia_nat_core/tests/nat/builder/test_thought.py

@@ -47,7 +47,7 @@ def __init__(self, name="fn", fid=None, parent_id=None, parent_name=None):
 
 
 @pytest.fixture(name="ctx_state")
-def ctx_state_fixture():
+def fixture_ctx_state():
     """Fresh context state for each test."""
     s = ContextState()
 
@@ -59,12 +59,12 @@ def ctx_state_fixture():
 
 
 @pytest.fixture(name="output_steps")
-def output_steps_fixture():
+def fixture_output_steps():
     return []
 
 
 @pytest.fixture(name="ctx")
-def ctx_fixture(ctx_state: ContextState, output_steps):
+def fixture_ctx(ctx_state: ContextState, output_steps):
     """Context with intermediate step manager subscribed to output_steps."""
     ctx = Context(ctx_state)
 
@@ -237,6 +237,15 @@ def test_multiple_concurrent_thoughts(ctx: Context, output_steps: list[Intermedi
     assert output_steps[0].payload.UUID == uuid1
     assert output_steps[1].payload.UUID == uuid2
 
+    output_steps.clear()
+
     # Pop in reverse order (LIFO)
     emit_thought_end(ctx, uuid2)
     emit_thought_end(ctx, uuid1)
+
+    # Should have 2 END events with correct UUIDs in LIFO order
+    assert len(output_steps) == 2
+    assert output_steps[0].payload.UUID == uuid2
+    assert output_steps[0].payload.event_type == IntermediateStepType.SPAN_END
+    assert output_steps[1].payload.UUID == uuid1
+    assert output_steps[1].payload.event_type == IntermediateStepType.SPAN_END

packages/nvidia_nat_core/tests/nat/front_ends/fastapi/test_step_adaptor.py

@@ -26,14 +26,14 @@
 from nat.front_ends.fastapi.step_adaptor import StepAdaptor
 
 
-@pytest.fixture
-def default_config():
+@pytest.fixture(name="default_config")
+def fixture_default_config():
     """Return a default StepAdaptorConfig object (mode=DEFAULT)."""
     return StepAdaptorConfig(mode=StepAdaptorMode.DEFAULT, custom_event_types=[])
 
 
-@pytest.fixture
-def custom_config():
+@pytest.fixture(name="custom_config")
+def fixture_custom_config():
     """Return a custom StepAdaptorConfig object (mode=CUSTOM) with custom types."""
     return StepAdaptorConfig(
         mode=StepAdaptorMode.CUSTOM,
@@ -44,8 +44,8 @@ def custom_config():
     )
 
 
-@pytest.fixture
-def disabled_config():
+@pytest.fixture(name="disabled_config")
+def fixture_disabled_config():
     """Return a custom StepAdaptorConfig object that disables intermediate steps."""
     return StepAdaptorConfig(
         mode=StepAdaptorMode.OFF,
@@ -56,26 +56,26 @@ def disabled_config():
     )
 
 
-@pytest.fixture
-def step_adaptor_default(default_config):
+@pytest.fixture(name="step_adaptor_default")
+def fixture_step_adaptor_default(default_config):
     """Return a StepAdaptor using the default config."""
     return StepAdaptor(config=default_config)
 
 
-@pytest.fixture
-def step_adaptor_custom(custom_config):
+@pytest.fixture(name="step_adaptor_custom")
+def fixture_step_adaptor_custom(custom_config):
     """Return a StepAdaptor using the custom config."""
     return StepAdaptor(config=custom_config)
 
 
-@pytest.fixture
-def step_adaptor_disabled(disabled_config):
+@pytest.fixture(name="step_adaptor_disabled")
+def fixture_step_adaptor_disabled(disabled_config):
     """Return a StepAdaptor using the disabled config."""
     return StepAdaptor(config=disabled_config)
 
 
-@pytest.fixture
-def make_intermediate_step():
+@pytest.fixture(name="make_intermediate_step")
+def fixture_make_intermediate_step():
     """A factory fixture to create an IntermediateStep with minimal defaults."""
 
     def _make_step(event_type: IntermediateStepType, data_input=None, data_output=None, name=None, UUID=None):
@@ -670,7 +670,7 @@ def test_function_events_in_custom_mode(step_adaptor_custom, make_intermediate_s
 # --------------------
 # Tests for thought_description override
 # --------------------
-def test_function_start_with_thought_description_override(step_adaptor_default, make_intermediate_step):
+def test_function_start_with_thought_description_override(step_adaptor_default):
     """FUNCTION_START events use custom thought_description from metadata when provided."""
     custom_thought = "Authenticating user credentials"
     payload = IntermediateStepPayload(
@@ -693,7 +693,7 @@ def test_function_start_with_thought_description_override(step_adaptor_default,
     assert result.thought_text == f"{custom_thought}..."
 
 
-def test_function_end_with_thought_description_override(step_adaptor_default, make_intermediate_step):
+def test_function_end_with_thought_description_override(step_adaptor_default):
     """FUNCTION_END events use custom thought_description from start event metadata."""
     uuid = "function-uuid-custom-end"
     custom_thought = "Generating SQL query"
@@ -747,7 +747,7 @@ def test_function_without_thought_description_uses_default(step_adaptor_default,
     assert result.thought_text == "Running function: test_function..."
 
 
-def test_tool_start_with_thought_description_override(step_adaptor_default, make_intermediate_step):
+def test_tool_start_with_thought_description_override(step_adaptor_default):
     """TOOL_START events use custom thought_description from metadata when provided."""
     custom_thought = "Executing database query"
     payload = IntermediateStepPayload(
@@ -770,7 +770,7 @@ def test_tool_start_with_thought_description_override(step_adaptor_default, make
     assert result.thought_text == f"{custom_thought}..."
 
 
-def test_tool_end_with_thought_description_override(step_adaptor_default, make_intermediate_step):
+def test_tool_end_with_thought_description_override(step_adaptor_default):
     """TOOL_END events use custom thought_description from start event metadata."""
     uuid = "tool-uuid-custom-end"
     custom_thought = "Executing database query"
@@ -863,6 +863,38 @@ def test_span_chunk_with_custom_thought(step_adaptor_default):
     assert result.thought_text == custom_thought_update
 
 
+def test_span_end_with_custom_thought(step_adaptor_default):
+    """Test that SPAN_END events extract thought_text from data.output."""
+    uuid = "span-uuid-end"
+
+    payload_start = IntermediateStepPayload(event_type=IntermediateStepType.SPAN_START,
+                                            name="custom_thought",
+                                            data=StreamEventData(input="Starting process..."),
+                                            UUID=uuid)
+    step_start = IntermediateStep(parent_id="root",
+                                  function_ancestry=InvocationNode(parent_id="abc",
+                                                                   function_id="def",
+                                                                   function_name="xyz"),
+                                  payload=payload_start)
+    step_adaptor_default.process(step_start)
+
+    custom_thought_final = "Process completed successfully"
+    payload_end = IntermediateStepPayload(event_type=IntermediateStepType.SPAN_END,
+                                          name="custom_thought",
+                                          data=StreamEventData(output=custom_thought_final),
+                                          UUID=uuid)
+    step_end = IntermediateStep(parent_id="root",
+                                function_ancestry=InvocationNode(parent_id="abc",
+                                                                 function_id="def",
+                                                                 function_name="xyz"),
+                                payload=payload_end)
+    result = step_adaptor_default.process(step_end)
+
+    assert result is not None
+    assert isinstance(result, ResponseIntermediateStep)
+    assert result.thought_text == custom_thought_final
+
+
 def test_span_without_thought_text_returns_none(step_adaptor_default):
     """Test that SPAN events without thought_text in data return None."""
     payload = IntermediateStepPayload(event_type=IntermediateStepType.SPAN_START,

packages/nvidia_nat_langchain/src/nat/plugins/langchain/agent/react_agent/register.py

@@ -117,7 +117,7 @@ async def react_agent_workflow(config: ReActAgentWorkflowConfig, builder: Builde
         llm=llm,
         prompt=prompt,
         tools=tools,
-        callbacks=[LangchainProfilerHandler()],
+        callbacks=[],
         use_tool_schema=config.include_tool_input_schema_in_tool_description,
         detailed_logs=config.verbose,
         log_response_max_chars=config.log_response_max_chars,
@@ -154,8 +154,12 @@ async def _response_fn(chat_request_or_message: ChatRequestOrMessage) -> ChatRes
 
             state = ReActGraphState(messages=messages)
 
-            # run the ReAct Agent Graph
-            state = await graph.ainvoke(state, config={'recursion_limit': (config.max_tool_calls + 1) * 2})
+            # run the ReAct Agent Graph with a new callback handler instance per request
+            state = await graph.ainvoke(state,
+                                        config={
+                                            'recursion_limit': (config.max_tool_calls + 1) * 2,
+                                            'callbacks': [LangchainProfilerHandler()]
+                                        })
             # setting recursion_limit: 4 allows 1 tool call
             #   - allows the ReAct Agent to perform 1 cycle / call 1 single tool,
             #   - but stops the agent when it tries to call a tool a second time