fix(strategy): externalize task images before applying transcript cap - #2103
fix(strategy): externalize task images before applying transcript cap#2103SparshM8 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes #2063 by aligning the subagent task transcript storage pipeline with the main session transcript pipeline so that inline base64 images are externalized before applying the 50MB blob cap, preventing transcripts from being dropped prematurely when externalization would have reduced them below the limit.
Changes:
- Reordered
prepareTaskTranscriptForStorageto run image externalization before enforcingagent.MaxChunkSize. - Added a boundary-focused test covering (1) oversized raw-but-externalizable images and (2) decoded-oversize images that must remain inline and still hit the cap.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/entire/cli/strategy/manual_commit_condensation.go | Moves the task transcript blob-size guard to run after image externalization and improves related warning logging. |
| cmd/entire/cli/strategy/manual_commit_condensation_test.go | Adds a regression test to ensure externalization can prevent transcript loss when raw base64 exceeds the cap. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // cap measures the size of the transcript that will be stored in git (after | ||
| // sanitization and image externalization, but before redaction). Oversized | ||
| // transcripts are dropped to keep the metadata branch pushable. | ||
| if len(externalized) > agent.MaxChunkSize { |
| imgData := make([]byte, 40*1024*1024) | ||
| for i := range imgData { | ||
| imgData[i] = byte(i % 256) | ||
| } | ||
| base64Image := base64.StdEncoding.EncodeToString(imgData) | ||
| line := `{"role":"assistant","content":[{"type":"image","source":{"type":"base64","media_type":"image/png","data":"` + base64Image + `"}}],"message":{"id":"msg_123"}}` + "\n" | ||
|
|
||
| require.NoError(t, os.WriteFile(agentTranscriptPath, []byte(line), 0o644)) |
|
Hi @entireio/cli-maintainers, I've raised PR #2103 to address the transcript cap issue (#2063). I've included a targeted boundary test that verifies the fix by simulating large base64 images that exceed the cap before externalization but stay well within it after. Could you please approve the CI workflows to run? I'm happy to address any feedback or adjust the aggregate asset policy if needed. Thanks! |
|
I've refined the documentation to accurately reflect the pipeline order: |
This PR fixes issue #2063 where large task transcripts containing base64 images were being capped before externalization, leading to lost transcripts even when externalization was enabled.
Changes:
Native test passed: TestCondenseSession_TaskRecordImageExternalizationBoundary