Skip to content

Conversation

@Pouyanpi
Copy link
Collaborator

@Pouyanpi Pouyanpi commented Nov 6, 2025

Description

Added support for extracting reasoning traces and tool calls from LangChain 1.x content blocks format, with automatic fallback to the legacy format for backward compatibility.

The implementation:

  • extracts reasoning from content_blocks with type: "reasoning"
  • extracts tool calls from content_blocks with type: "tool_call"
  • falls back to additional_kwargs and tool_calls attribute for providers using the legacy format

references:

requires:

#1472

@Pouyanpi Pouyanpi added this to the v0.19.0 milestone Nov 6, 2025
@Pouyanpi Pouyanpi self-assigned this Nov 6, 2025
@Pouyanpi Pouyanpi marked this pull request as ready for review November 6, 2025 14:49
@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

Added support for extracting reasoning traces and tool calls from LangChain 1.x content_blocks format, with automatic fallback to legacy additional_kwargs and tool_calls attribute for backward compatibility.

Key changes:

  • Refactored _store_reasoning_traces() to try content_blocks first, then fall back to additional_kwargs
  • Refactored _store_tool_calls() to try content_blocks first, then fall back to tool_calls attribute
  • Split extraction logic into separate helper functions for better modularity
  • Added comprehensive test coverage with 28 new tests covering edge cases and fallback behavior

Testing:

  • Tests cover both extraction methods, fallback behavior, multiple content blocks, and edge cases
  • Tests verify proper prioritization (content_blocks over legacy format)
  • Tests include both MockResponse and real LangChain AIMessage objects

Confidence Score: 4/5

  • This PR is safe to merge with minor risk from a KeyError edge case
  • Score reflects solid implementation with comprehensive tests and backward compatibility, but deducted one point for the KeyError risk in reasoning extraction when blocks have type "reasoning" but missing the key
  • Pay close attention to nemoguardrails/actions/llm/utils.py line 252 for the KeyError issue

Important Files Changed

File Analysis

Filename Score Overview
nemoguardrails/actions/llm/utils.py 4/5 Added LangChain 1.x content_blocks support with fallback to legacy format. Implementation is solid but has one KeyError risk in reasoning extraction.
tests/test_actions_llm_utils.py 5/5 Comprehensive test coverage for both extraction methods, fallback behavior, and edge cases with MockResponse and real AIMessage objects.

Sequence Diagram

sequenceDiagram
    participant LLM as LLM Call
    participant Store as _store_reasoning_traces/_store_tool_calls
    participant ExtractCB as _extract_from_content_blocks
    participant ExtractLegacy as _extract_from_legacy
    participant CtxVar as Context Variable

    LLM->>Store: response object
    Store->>ExtractCB: try content_blocks first
    alt content_blocks exists
        ExtractCB->>ExtractCB: iterate blocks
        ExtractCB->>ExtractCB: filter by type
        ExtractCB-->>Store: return extracted data
        Store->>CtxVar: set value
    else no content_blocks
        ExtractCB-->>Store: return None
        Store->>ExtractLegacy: fallback to legacy format
        alt legacy format exists
            ExtractLegacy-->>Store: return extracted data
            Store->>CtxVar: set value
        else no legacy format
            ExtractLegacy-->>Store: return None
            Store->>CtxVar: set None or original value
        end
    end
Loading

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: Pouyan <[email protected]>
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

Refactored LLM response processing to support LangChain 1.x content blocks format while maintaining backward compatibility with the legacy format.

Key Changes:

  • Extracted reasoning traces from content_blocks with type: "reasoning" (LangChain 1.x standard)
  • Extracted tool calls from content_blocks with type: "tool_call" (LangChain 1.x standard)
  • Implemented automatic fallback to additional_kwargs and tool_calls attribute for providers using legacy format
  • Refactored extraction logic into separate functions for better modularity and testability
  • Added comprehensive test coverage for both new content blocks format and legacy format

Architecture:
The implementation follows a try-first-then-fallback pattern: it first attempts to extract data from the newer content_blocks format, and only if that fails, falls back to the legacy format. This ensures compatibility with both old and new LangChain versions.

Confidence Score: 4/5

  • Safe to merge with one minor defensive check already flagged in previous comments
  • The refactoring is well-structured with good separation of concerns and comprehensive test coverage. The fallback mechanism ensures backward compatibility. One minor issue (missing key existence check in line 252) was already identified in previous comments and should be addressed before merging.
  • No files require special attention beyond addressing the previously flagged issue in nemoguardrails/actions/llm/utils.py:252

Important Files Changed

File Analysis

Filename Score Overview
nemoguardrails/actions/llm/utils.py 4/5 Refactored reasoning and tool call extraction to support LangChain 1.x content blocks with fallback to legacy format. Added proper defensive checks. One minor issue already flagged in previous comments.

Sequence Diagram

sequenceDiagram
    participant LLM as LLM Call
    participant Store as _store_reasoning_traces / _store_tool_calls
    participant CB as Content Blocks Extractor
    participant AK as Additional Kwargs Extractor
    participant CV as Context Variable

    LLM->>Store: response object
    
    alt Reasoning Extraction
        Store->>CB: _extract_reasoning_from_content_blocks
        CB->>CB: Check hasattr(response, "content_blocks")
        alt content_blocks exists
            CB->>CB: Iterate blocks
            CB->>CB: Find type="reasoning" with "reasoning" key
            CB-->>Store: Return reasoning string or None
        else no content_blocks
            CB-->>Store: Return None
        end
        
        alt No reasoning from content_blocks
            Store->>AK: _extract_reasoning_from_additional_kwargs
            AK->>AK: Check hasattr(response, "additional_kwargs")
            AK->>AK: Get "reasoning_content" from dict
            AK-->>Store: Return reasoning or None
        end
        
        Store->>CV: reasoning_trace_var.set(reasoning_content)
    end
    
    alt Tool Calls Extraction
        Store->>CB: _extract_tool_calls_from_content_blocks
        CB->>CB: Check hasattr(response, "content_blocks")
        alt content_blocks exists
            CB->>CB: Collect all type="tool_call" blocks
            CB-->>Store: Return list or None
        else no content_blocks
            CB-->>Store: Return None
        end
        
        alt No tool_calls from content_blocks
            Store->>AK: _extract_tool_calls_from_attribute
            AK->>AK: getattr(response, "tool_calls", None)
            AK-->>Store: Return tool_calls or None
        end
        
        Store->>CV: tool_calls_var.set(tool_calls)
    end
Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

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