fix(agent): escape Harmony compaction markers#5190
Merged
Conversation
Escaped Harmony control-token markers when compaction serializes transcripts into plain summary prompts so Copilot gpt-5.6 models do not reject analysis-channel text. Added regression coverage for summary-bound Harmony serialization while keeping native transcript rendering unchanged. Fixes #5184
|
@roboomp ci tests failure in this pr: https://github.com/can1357/oh-my-pi/actions/runs/29154669302/job/86549758686?pr=5190 |
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
3 participants
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.
Repro
Copilot-backed
gpt-5.6-*Responses models reject payload text containing the Harmony analysis marker<|channel|>analysis. On the OMP side,bun test packages/ai/test/transcript-render.test.ts -t harmonyconfirms the Harmony transcript renderer intentionally emits<|start|>assistant<|channel|>analysis<|message|>...<|end|>, matching the marker in the reporter's minimized rejection payload.Cause
packages/agent/src/compaction/compaction.tsandpackages/agent/src/compaction/branch-summarization.tsusedserializeConversation(..., preferredDialect(model.id))directly when building plain summarization prompt text. For OpenAI-family model ids,packages/catalog/src/identity/dialect.tsselects the Harmony dialect, so assistant thinking became raw Harmony control-token text inside<conversation>...</conversation>and could be rejected before the summarizer ran.Fix
serializeConversationForSummaryinpackages/agent/src/compaction/utils.tsto preserve native transcript readability while escaping Harmony control tokens for plain summary input.<|channel|>analysiswhile native Harmony transcript rendering remains unchanged.packages/agent.Verification
bun test packages/agent/test/serialize-conversation.test.ts packages/agent/test/remote-compaction.test.tspassed: 30 tests, 148 expectations.gh_push_branchcompleted the pre-publish gate and pushedfarm/d665db31/sanitize-harmony-compactionat9831386deba2. Fixes #5184