Conversation
Preserve the assistant turn opened by native prompt rendering instead of merging generated output into supplied assistant history. Separate stored hop input and output so cold replay matches the live transcript. Cancel abandoned backend generation before waiting for final usage so tool dispatch does not wait for discarded tokens to reach EOS. Files changed: - sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc - sdk_v2/cpp/src/inferencing/generative/chat/chat_transcript.cc - sdk_v2/cpp/src/inferencing/generative/chat/chat_transcript.h - sdk_v2/cpp/src/inferencing/generative/openresponses/response_converter.cc - sdk_v2/cpp/src/inferencing/session/request.h - sdk_v2/cpp/test/internal_api/chat/chat_session_test.cc - sdk_v2/cpp/test/internal_api/chat/chat_transcript_test.cc - sdk_v2/cpp/test/internal_api/replay_equivalence_test.cc Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 28c98e1b-686e-4f48-ac63-47e6c0ceb192
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Three added lines exceed the mandatory 120-character C++ line limit.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Preserves assistant-turn boundaries during live generation and replay, while safely canceling asynchronous host-ended generation before usage retrieval.
Changes:
- Separates supplied assistant input from generated replies.
- Adds cancellation ordering for typed and Chat Completions requests.
- Expands boundary, streaming, cutoff, and replay regression tests.
File summaries
| File | Description |
|---|---|
replay_equivalence_test.cc |
Tests warm/cold boundary equivalence. |
chat_transcript_test.cc |
Tests separate assistant messages and undo behavior. |
chat_session_test.cc |
Tests native templates, streaming, and cancellation ordering. |
request.h |
Clarifies replay segment semantics. |
response_converter.cc |
Separates replayed input and output segments. |
chat_transcript.h |
Updates transcript boundary API and documentation. |
chat_transcript.cc |
Commits generated output as a separate message. |
chat_session.cc |
Cancels host-ended generation before usage retrieval. |
Review details
Suppressed comments (1)
sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc:1445
- This added line is 123 characters long, exceeding the repository's 120-character C++ line limit. Wrap the arguments to keep the call within the project limit.
original_request.canceled, stop_sequence_matched, /*host_output_limit_reached=*/false,
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Address Copilot comment 4030853444 by wrapping both finalization calls in chat_session.cc (original lines 1138 and 1445). Address Copilot comment 4030853511 by placing the 12 test cases on separate lines in chat_session_test.cc (original line 1195). Keep a trailing initializer comma so clang-format does not collapse the matrix. Files changed: - sdk_v2/cpp/src/inferencing/generative/chat/chat_session.cc - sdk_v2/cpp/test/internal_api/chat/chat_session_test.cc Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 28c98e1b-686e-4f48-ac63-47e6c0ceb192
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation consistently preserves turn boundaries and covers host-ending paths with focused regression tests.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #1083 addressing the two remaining material review findings.
add_generation_prompt=true; merging the reply into trailing assistant input changed the conversation on subsequent turns. Remove the corresponding guard seeding and preserve the same input/output boundary when replaying stored Responses chains.tool_calls, and truncated raw-envelope output is not treated as naturally complete.The existing adjacency grouping within supplied input and within one generated output remains unchanged.
Regression coverage
Validation
lintrunnerandgit diff --checkBoth runs include all 11 checked-in CPU Engine lifecycle cases and the existing raw-envelope finalization coverage. No selected tests were skipped or disabled, and no negative test filters were used. Both builds use
build.pyand the repository's pinned native dependencies.