fix(agents): emit a single delta when copilot/qwen/bob repeat a payload across fields - #153
AniruddhaAdak wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, preserves field precedence, avoids emitting empty deltas, and is covered by focused regression tests for the reported duplication cases.
Pull request overview
This PR fixes duplicate HTML append behavior in the copilot, qwen, and bob stdout line parsers by emitting at most one delta per JSON line when the same payload is repeated across multiple top-level string fields.
Changes:
- Update
copilotparsing to single-pick the first non-empty ofresponsethentext. - Update
qwen/bobparsing to single-pick the first non-empty oftext,content, thenmessage. - Add regression tests ensuring repeated-payload lines emit exactly one delta, and that empty-first-field falls through to the next populated field.
File summaries
| File | Description |
|---|---|
| next/src/lib/agents/argv.ts | Dedupes per-line delta emission for copilot/qwen/bob by selecting only the first non-empty candidate field. |
| next/src/lib/agents/tests/argv.test.ts | Adds regression tests covering repeated-payload and empty-first-field fallthrough scenarios. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
lefarcen
left a comment
There was a problem hiding this comment.
Hey @AniruddhaAdak! 🎉 First PR here — welcome!
The approach is exactly right: the .find(…) single-pick idiom you've used here already powers the opencode parser a few lines away in this file, and mirrors the sawStreamEventText guard pattern in the cursor-agent/gemini/qoder parsers you cited. Aligning all three of these parsers to the same idiom is the clean direction. Preserving the original field-precedence order (response before text; text before content before message) while dropping duplicate emissions is the correct contract for this fix.
The test plan is thorough. The four regression cases — repeated-payload for copilot, qwen, and bob, plus the empty-first-field fallthrough — directly cover the reported bug, and your honesty about what you couldn't verify locally (live CLI SSE output) is exactly the context a reviewer needs to know where to focus their own testing.
One thing worth confirming on the live-CLI side: the fix is premised on those fields always carrying identical content when they co-appear in a single line. If a CLI ever emits meaningfully different content across (say) response and text simultaneously — rather than the same echoed payload — response would win silently and text would be dropped. The regression tests validate the single-pick behavior, but not the "fields are always identical" premise against real output. Worth a quick spot-check against live copilot/qwen/bob streams before merge.
mergeStateStatus is showing UNSTABLE and gh pr checks reports nothing on the branch — if CI is supposed to run here, a quick pnpm -F @html-anything/next test + pnpm -F @html-anything/next typecheck confirmation from a maintainer's environment would close the loop.
Nothing blocking from my read. ✨
Summary
The
copilot/qwen/bobstream parsers innext/src/lib/agents/argv.tspushed onedeltaper candidate field unconditionally, so a turn payload echoed under several keys (response+text, ortext+content+message) was appended to the converted HTML two or three times. They now single-pick the first populated field — the same.find(...)idiom theopencodeparser in this file already uses (and the same dedupe idea behind thecursor-agent/gemini/qodersawStreamEventTextguards).Behavior notes:
responsebeforetext;textbeforecontentbeforemessage).{text: "", content: "hi"}produced an empty delta plus the real one); single-field payloads behave exactly as before.Test plan
next/src/lib/agents/__tests__/argv.test.ts(4 cases: copilot/qwen/bob repeated-payload lines emit exactly one delta; empty-first-field falls through). Verified they fail before the fix (2x/3x deltas plus an empty delta) and pass after.pnpm -F @html-anything/next test: 185/186 — the only failure is the pre-existinginstall-rejectionssymlink test, which fails at fixture setup with WindowsEPERM(no symlink privilege on this host) and is unrelated to this change (skill-install area, untouched files).pnpm -F @html-anything/next typecheck: clean.pnpm exec tsx scripts/guard.ts: passed.pnpm -F @html-anything/next build: production build succeeds.argv.test.tspattern). Per CONTRIBUTING, attaching an SSE log/screenshot is expected for streaming changes — I don't have a logged-in copilot/qwen/bob CLI here, so reviewers with access may want to confirm against live output.No duplicate work: searched open/merged issues and PRs for dedupe/double-emit/argv parser fixes — related parser PRs (#50, #59, #92, #102) cover other agents or single-field shapes, none single-picks these three.