Skip to content

fix(devin): consolidate streamed tool results per invocation - #6584

Closed
lidi1011 wants to merge 1 commit into
lidge-jun:devfrom
lidi1011:fix/devin-streamed-tool-results
Closed

lidi1011 wants to merge 1 commit into
lidge-jun:devfrom
lidi1011:fix/devin-streamed-tool-results

Conversation

@lidi1011

@lidi1011 lidi1011 commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Codex code-mode tools can append multiple progress/final outputs for one call_id, including a late output after a new user message. The Devin mapper currently serializes each output as another tool prompt with the same ID. A supplied reproduction report observed invalid_argument for this shape and success when the results were manually combined.

  • Consolidate results per invocation at the Devin mapping boundary, retaining the first result slot and appending chunks in arrival order with a visible separator. Keep intervening user/assistant messages intact.
  • Preserve text, images, error flags and in-band error markers without mutating the input history. Keep parallel call IDs separate and reset the result slot when an assistant declares a new invocation reusing an ID.
  • Add nine parser/mapping/encoded-request regressions and document the chronology tradeoff. Other providers, credentials, model selection and saved conversations are unaffected.

Verification

Validated against upstream dev at a6bc60816cc1b51c4279defa41c7ff2d2370d47e with Bun 1.4.0 in an isolated source checkout.

  • Regression proof: the supplied five-case reproducer failed 3/5 tests on unchanged dev; the new nine-case suite failed 9/9 before the fix and passes 9/9 after it.
  • bun test tests/providers/devin-*.test.ts tests/claude-integration/claude-devin-output-order.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts tests/ci-workflows/structure-ssot.test.ts — 561 pass, 0 fail, 26 files. Includes actual protobuf prompt-count, call-ID, image-field and error-flag assertions.
  • bun run typecheck — passed.
  • bun run structure:check — passed.
  • bun run privacy:scan — passed.
  • cd docs-site && ASTRO_TELEMETRY_DISABLED=1 bun run build — passed; 561 pages built and internal links checked. Telemetry is disabled to keep the build within the isolated checkout.
  • bun run test:changed — incomplete, not green. Stopped after more than six minutes: the captured output contains 12,309 passing cases, 34 skipped cases and 12 failures; tests/codex-integration/issue-452-empty-503.test.ts was still running after 296 seconds at interruption. The test runner buffers output until exit. Broader validation remains for CI; no full-suite success is claimed.
  • Baseline comparison for those failures: bun test --isolate tests/adapters/anthropic/anthropic-fast-opt-in.test.ts tests/server/management-provider-validation.test.ts tests/claude-integration/claude-picker-runtime.test.ts tests/codex-integration/native-codex-toggle.test.ts — 172 pass / 11 fail on pristine upstream a6bc6081 and the same 172 pass / 11 fail on PR head 0ec00fcd, with identical failing test names. These failures concern provider-management HTTP status and native Codex toggle state. The twelfth failure (picker port-release timing) did not reproduce on either revision in this isolated rerun. The long-running empty-503 test was not rerun. These unrelated areas are left unchanged.

No fresh paid provider request was sent and no installed proxy was patched or restarted. The original report's live before/after results are supporting evidence supplied by the reporter, not a live result independently reproduced in this PR. This fixes the duplicate-result shape, not every possible invalid_argument. The full suite and upstream CI are not claimed as passing; this contributor PR remains a draft pending the repository's review-readiness process.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. No credential, authentication, logging or permission behavior changes; fixtures contain only synthetic data.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@github-actions github-actions Bot added the bug Something isn't working label Oct 4, 2026
@Ingwannu

Ingwannu commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Scoped regression checkpoint at 0ec00fcdbb204a06651e2f563455e278a671fb4f, base a6bc60816cc1b51c4279defa41c7ff2d2370d47e; not approval. I inspected the seven-file diff and independently ran bun test tests/providers/devin-stream-results.test.ts: 9 pass, 0 fail, 54 assertions, exit 0 with Bun 1.4.0. The cases use the actual Responses parser, mapper and protobuf encoder: progress/final output becomes one result, parallel IDs remain separate, a new invocation resets a reused ID, late output retains intervening messages, text/images/error markers survive, and the input history is unchanged. This verifies the supplied synthetic shapes, not a real Devin acceptance result or every invalid_argument.

One ubuntu cgroup, exact-head detached worktree, fresh private HOME/CODEX_HOME/OPENCODEX_HOME/TMPDIR: CPUQuota=75%, MemoryHigh=1280M, MemoryMax=1536M, swap=0, TasksMax=64, IOWeight=20, nice=10, RuntimeMaxSec=120 and TimeoutStopSec=5. Effective limits were inspected; the workload terminated and protected real-home fingerprints/modes and the main dirty checkout were unchanged.

I did not rerun your 561-test set, typecheck, docs/gates, interrupted changed suite or baseline comparisons. Preserve those broader failures/incomplete results as disclosed, not green evidence. Draft readiness remains 0/4 and Cross-platform CI 37209037875 / React Doctor 37209037933 await workflow approval. The first-slot placement tradeoff is explicit in your contract. No new scan, provider/native call, sponsorship waiver, approval, integration or deployment was performed.

New integration hold: trusted dev advanced to 584b92a53cd275ef6daab66458c693e7257ffe39 through #6583. GitHub reports this head as non-mergeable, and a read-only three-way merge inspection reproduces conflict markers in tests/fixtures/test-layout-expected.json. Please synchronize your own branch with current dev, retaining the compacted fixture and your new explicit test entry, then rerun the layout/ratchet checks and re-attest validation on the resulting head. Existing head-bound tests/CI are not evidence for an unvalidated conflict resolution. I have not edited or pushed your branch.

lidge-jun added a commit that referenced this pull request Oct 5, 2026
…#6584) (#6636)

Devin rejected histories where several progress/final outputs for one tool
call became separate tool prompts with the same call id. Consecutive results
sharing a call id now merge into one prompt, keeping chunk order, images and
error markers; any intervening message or a different id keeps chronological
slots, and the parsed request is not mutated.

Narrowed carry of #6584: consolidation never crosses an intervening turn.

Co-authored-by: lidi1011 <283302260+lidi1011@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Thank you, @lidi1011. The consolidation for consecutive results of one invocation landed on dev in #6636 (37189ca), carried with a Co-authored-by trailer for you. One part was intentionally narrowed: results are combined only when no other message sits between them, so a later result is never moved back across an intervening user or assistant turn. If you have a captured history where Devin still rejects interleaved results, please open a focused issue or PR with it.

@lidge-jun lidge-jun closed this Oct 5, 2026
@lidge-jun lidge-jun mentioned this pull request Oct 6, 2026
4 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants