Skip to content

Conversation

@pan-x-c
Copy link
Collaborator

@pan-x-c pan-x-c commented Aug 7, 2025

Description

Fix logprobs length in multi-turn experience

Checklist

Please check the following items before code is ready to be reviewed.

  • Code has passed all tests
  • Docstrings have been added/updated in Google Style
  • Documentation has been updated
  • Code is ready for review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @pan-x-c, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

I've implemented a fix to ensure the correct length of logprobs in multi-turn experiences, which was previously misaligned. This involved adjusting how logprobs are sliced and adding a corresponding test. Additionally, I've refined the asynchronous iteration handling across the codebase, ensuring that StopAsyncIteration is consistently raised and caught where appropriate, improving the robustness and clarity of asynchronous data processing.

Highlights

  • Corrected logprobs length for multi-turn experiences: I've addressed an issue where the logprobs (log probabilities) were not correctly sized for multi-turn experiences. This fix ensures that the logprobs tensor accurately reflects the generated tokens by applying the correct slicing based on the prompt length. A new test assertion has also been added to verify this behavior.
  • Standardized Asynchronous Iteration Handling: I've standardized the handling of asynchronous iteration termination across various buffer readers and consumer modules. Specifically, file_reader.py now correctly converts synchronous StopIteration into StopAsyncIteration for its read_async methods. Conversely, an incorrect conversion from StopAsyncIteration to StopIteration was removed in queue_reader.py. Additionally, modules like explorer.py and trainer.py have been updated to directly catch StopAsyncIteration, simplifying and unifying the error handling for exhausted asynchronous data sources.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@pan-x-c
Copy link
Collaborator Author

pan-x-c commented Aug 7, 2025

/unittest-all

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces two key changes: a fix for the log-probability slicing in multi-turn scenarios and a significant improvement in asynchronous iteration handling. The logprobs are now correctly sliced to only include the response tokens, which resolves a potential bug. The exception handling has been refactored to consistently use StopAsyncIteration for async readers, which simplifies the consumer code in Explorer and Trainer by removing complex error-checking logic.

My review includes one suggestion to address code duplication in trinity/buffer/reader/file_reader.py, where several classes share an identical read_async implementation. Consolidating this into a base class would enhance maintainability. Overall, the changes are a solid improvement to the codebase's correctness and clarity.

@pan-x-c
Copy link
Collaborator Author

pan-x-c commented Aug 7, 2025

/unittest-module-common

@pan-x-c pan-x-c force-pushed the fix/multi_turn_logprobs branch from 9157c6b to 22377d3 Compare August 7, 2025 06:24
@pan-x-c
Copy link
Collaborator Author

pan-x-c commented Aug 7, 2025

/unittest-module-common

@pan-x-c pan-x-c merged commit f9270dc into agentscope-ai:main Aug 7, 2025
1 check passed
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