Skip to content

Conversation

mzegla
Copy link
Collaborator

@mzegla mzegla commented Oct 3, 2025

With that change hermes3 parser is capable of handling more complex chunks in streaming mode.
Additionally some parts of the logic have been wrapped in separate functions and processing phases in parseChunk are described in details in the comments.

@mzegla mzegla requested a review from Copilot October 3, 2025 14:49
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the Hermes3 tool parser to improve its streaming mode capabilities by handling more complex chunks and reorganizing the processing logic. The changes add better support for edge cases and provide clearer separation of processing phases.

  • Refactored streaming parser with improved chunk handling and state management
  • Enhanced test coverage with additional tool call scenarios and edge cases
  • Added proper error handling with stream cache clearing on exceptions

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
third_party/llm_engine/llm_engine.bzl Updated commit date in comment from 2025-09-13 to 2025-09-23
src/test/llm/output_parsers/hermes3_output_parser_test.cpp Extended test cases with new tool call scenarios and updated comment
src/llm/io_processing/output_parser.cpp Added try-catch blocks to clear stream cache on parser exceptions
src/llm/io_processing/hermes3/tool_parser.hpp Added new private methods for chunk processing and state management
src/llm/io_processing/hermes3/tool_parser.cpp Complete refactor of parseChunk method with improved logic and detailed comments

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

// 2) (text)*</tool_call>
// 3) <tool_call>(text)*

// We assume single chunk will not contain whole tool call i.e. <tool_call>(text)*</tool_call>
Copy link
Preview

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

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

The abbreviation 'i.e.' should be followed by a comma. It should be 'i.e., <tool_call>(text)*</tool_call>'.

Suggested change
// We assume single chunk will not contain whole tool call i.e. <tool_call>(text)*</tool_call>
// We assume single chunk will not contain whole tool call, i.e., <tool_call>(text)*</tool_call>

Copilot uses AI. Check for mistakes.

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.

1 participant