From 2ce032c168e2bf7ca14ca307c1824cc1ad643886 Mon Sep 17 00:00:00 2001 From: 5258MF <5258MF@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:19:33 +0800 Subject: [PATCH] fix(v1): preserve truncated tool result bodies --- .../DESIGN.md | 86 +++++++++++++++++++ .../REQ.md | 60 +++++++++++++ .../WORKLOG.md | 77 +++++++++++++++++ .../src/messages-v1.ts | 43 ++++++++-- .../tests/messages.test.ts | 59 +++++++++++++ 5 files changed, 317 insertions(+), 8 deletions(-) create mode 100644 devlog/2026-08-13_v1-tool-result-truncation/DESIGN.md create mode 100644 devlog/2026-08-13_v1-tool-result-truncation/REQ.md create mode 100644 devlog/2026-08-13_v1-tool-result-truncation/WORKLOG.md diff --git a/devlog/2026-08-13_v1-tool-result-truncation/DESIGN.md b/devlog/2026-08-13_v1-tool-result-truncation/DESIGN.md new file mode 100644 index 0000000..5f08ad9 --- /dev/null +++ b/devlog/2026-08-13_v1-tool-result-truncation/DESIGN.md @@ -0,0 +1,86 @@ +# DESIGN - V1 Tool Result Body Round Trip + +- Task ID: `2026-08-13_v1-tool-result-truncation` +- Home Repo: `billion-context-opencode` +- Created: 2026-08-13 +- Status: Accepted + +## 1. Problem Statement + +- **What problem are we solving?** The V1 converter projects a tool result into a mutable kernel core, but reassembly currently discards any kernel rewrite and restores the original host part verbatim. +- **Why now?** The old V1-only fix predates the monorepo and dual-shape refactor. It must be migrated deliberately to the current V1 converter without treating the unverified V2 host contract as equivalent. + +## 2. Goals & Non-Goals + +- **Goals**: + - Make V1 tool-result projection and writeback use the same state-aware body mapping. + - Preserve completed, ordinary error, and interrupted error state semantics. + - Preserve the call/result pairing invariant and all non-body host fields. +- **Non-Goals**: + - Do not change or infer behavior for `messages-v2.ts`. + - Do not change the shared runtime, kernel, dual-shape entry, persisted state, or provider-facing schemas. + +## 3. Current Architecture + +- **How it works today**: + + ```text + V1 OctoPart + -> octoToCoreMessages (tool-call core + optional tool-result core) + -> acp-kernel (may rewrite tool-result text) + -> reassemble (checks pair, restores original OctoPart) + ``` + +- **Pain points**: The final step observes only whether the result core survived. It does not project a changed result body back into the V1 host shape, so emergency truncation is lost. + +## 4. Proposed Architecture + +- **Overview**: + + ```text + V1 state --toolResultBody--> tool-result core + | + kernel rewrite + | + applyToolBody by original state + | + output | error | metadata.output on V1 state + ``` + +- **Key components**: + - `toolResultBody`: the single V1 projection rule used for comparison and core construction. + - `applyToolBody`: a state-preserving copy-on-change writeback used only after both halves of a tool pair survive. +- **Data flow**: + - Completed: `state.output` -> core text -> `state.output`. + - Ordinary error: adapter projection `Error: ${state.error}` -> core text -> `state.error`, removing one adapter-added prefix. + - Interrupted error with string partial output: `state.metadata.output` -> core text -> `state.metadata.output`. +- **API / interface changes**: Extend the internal structural `OctoPart.state` type with optional `metadata: Record`. No exported package entry, configuration, or persistence API changes. + +## 5. Design Decisions & Rationale + +| Decision | Options Considered | Chosen | Why | +|----------|--------------------|--------|-----| +| Scope | Update V1 and V2; update V1 only | V1 only | The V1 host shape is locally verified; this task does not claim an unverified V2 runtime contract. | +| Error representation | Store core text in `output`; preserve state-specific fields | Preserve state-specific fields | OpenCode consumes ordinary errors from `state.error` and interrupted partial output from `state.metadata.output`. | +| Unchanged bodies | Always clone; compare trailing-whitespace-normalized bodies | Normalize trailing whitespace and return original | Avoids needless mutation when formatting differs but the body does not. | +| Missing or empty result core | Clear host body; keep original part | Keep original part | The kernel truncation rewrite is non-empty; missing/empty input is not evidence that host state should be erased. | + +## 6. Impact Analysis + +- **Backward compatibility**: The dual-shape export and persisted state are untouched. The existing call/result pairing check remains the gate before writeback. Status and other state fields are retained with object spreads. +- **Performance**: One body projection and trailing-whitespace comparison per surviving terminal V1 tool part; linear in result body length and only on reassembly. +- **Security**: No new I/O, dependencies, parsing, or privilege boundaries. +- **Dependencies**: No new packages required. + +## 7. Migration Plan + +- **Steps**: + 1. Add the state-aware V1 projection helper and internal metadata type. + 2. Route existing terminal result projection through it. + 3. Apply changed core bodies during V1 reassembly after the pair-survival check. + 4. Add converter tests for completed, whitespace-equivalent, ordinary error, and interrupted error cases. +- **Feature flags / gradual rollout**: None. Unchanged bodies retain the original part reference; the changed path is limited to kernel result rewrites. + +## 8. Open Questions + +- [ ] V2 may need an analogous fix after its real host message and hook contracts can be validated; it is intentionally deferred from this change. diff --git a/devlog/2026-08-13_v1-tool-result-truncation/REQ.md b/devlog/2026-08-13_v1-tool-result-truncation/REQ.md new file mode 100644 index 0000000..2aba7f9 --- /dev/null +++ b/devlog/2026-08-13_v1-tool-result-truncation/REQ.md @@ -0,0 +1,60 @@ +# REQ - Preserve V1 Tool Result Truncation + +- Task ID: `2026-08-13_v1-tool-result-truncation` +- Home Repo: `billion-context-opencode` +- Created: 2026-08-13 +- Status: Done +- Priority: P1 +- Owner: 5258MF +- References: https://github.com/ranxianglei/billion-context-opencode/pull/3, `billion-context-pi/src/messages.ts` + +## 1. Background & Problem Statement + +- **Context**: The V1 adapter projects each completed or failed OpenCode tool part into a `tool-call` core and a `tool-result` core. At its emergency threshold, `acp-kernel` can replace the tool-result core text with a shortened body. +- **Current behavior (symptom)**: V1 reassembly checks that both halves of the tool pair survived, then restores the original tool part unchanged. The kernel's shortened result body is therefore discarded before OpenCode builds the model request. +- **Expected behavior**: Reassembly writes a changed kernel result body back to the V1 field consumed by OpenCode while preserving the original tool state and all non-body fields. +- **Impact**: Emergency truncation of large V1 tool results does not reduce the request body that reaches the model. + +## 2. Reproduction + +- **Environment**: + - Node: 24.10.0 + - OS/Arch: Windows x64 +- **Minimal reproduction steps**: + 1. Convert a V1 tool part with `octoToCoreMessages`. + 2. Replace the resulting `tool-result` core text as the kernel emergency truncation node does. + 3. Reassemble the cores and observe that the unmodified adapter returns the original V1 tool body. +- **Relevant configuration**: The production path is reached when `tokenCount >= truncate.threshold * modelContextLimit`; converter unit tests simulate only the deterministic kernel rewrite. + +## 3. Constraints & Non-Goals + +- **Constraints**: + - Backward compatibility (dual-shape export, persisted state format): Do not change the dual-shape export, persisted state, or call/result survival invariant. Preserve the original V1 status, input, error, metadata, timing, and other state fields. + - Performance requirements: Keep the normal path linear and return the original part reference when the body is unchanged modulo trailing whitespace. + - Resource limits: Add no dependencies and do not alter kernel configuration. +- **Non-Goals**: + - No changes to `messages-v2.ts` or claims about OpenCode V2 runtime behavior. + - No changes to `acp-kernel`, prompt text, compression policy, or tool registration. + - No handling for pending/running tool parts, because they have no V1 tool-result core in this conversion path. + +## 4. Acceptance Criteria + +- **Correctness**: + - [x] A changed completed result body is written to `state.output`. + - [x] A changed ordinary failure body is written to `state.error` without the adapter-added single `Error: ` prefix, and remains an error. + - [x] A changed interrupted partial result is read from and written to `state.metadata.output`, while the interruption marker and original error remain intact. + - [x] An unchanged body modulo trailing whitespace returns the original part unchanged. +- **Performance / Stability**: + - [x] Tool call/result pairing remains unchanged. + - [x] V2 adapter source remains unchanged. +- **Regression**: + - [x] Four V1 converter tests are added and pass. + - [x] `npm run typecheck`, `npm run test`, `npm run build`, and PR validation pass. + +## 5. Proposed Approach + +- **Affected modules & entry files**: + - `packages/billion-context-opencode/src/messages-v1.ts` + - `packages/billion-context-opencode/tests/messages.test.ts` +- **Risks**: Error results use an adapter-only display prefix in the core representation; writeback must remove exactly one such prefix. Interrupted failures must update partial output rather than overwrite their error message. +- **Rollback strategy**: Revert the source and four associated converter tests; no state migration or cleanup is needed. diff --git a/devlog/2026-08-13_v1-tool-result-truncation/WORKLOG.md b/devlog/2026-08-13_v1-tool-result-truncation/WORKLOG.md new file mode 100644 index 0000000..cc332ea --- /dev/null +++ b/devlog/2026-08-13_v1-tool-result-truncation/WORKLOG.md @@ -0,0 +1,77 @@ +# WORKLOG - Preserve V1 Tool Result Truncation + +- Task ID: `2026-08-13_v1-tool-result-truncation` +- Home Repo: `billion-context-opencode` +- Status: Done +- Updated: 2026-08-13 11:30 +08:00 + +## 1. Summary + +- **What was done**: Migrated the old tool-result writeback fix into the current V1 converter only, with state-aware handling for completed, ordinary error, and interrupted error results. Added four focused V1 converter tests. +- **Why**: Kernel emergency truncation rewrites the result core, but V1 reassembly previously restored the original full host part. +- **Behavior / compatibility changes**: Yes. Only a changed V1 terminal tool-result body is copied back into its original body field. V2 is explicitly unchanged. +- **Risk level**: Medium + +## 2. Change Log + +### Commits + +| Commit | Description | +|--------|-------------| +| This PR | V1 tool-result body writeback and regression coverage | + +### Key Files + +- `packages/billion-context-opencode/src/messages-v1.ts` - project and restore V1 result bodies by original state. +- `packages/billion-context-opencode/tests/messages.test.ts` - cover four V1 result writeback scenarios. +- `devlog/2026-08-13_v1-tool-result-truncation/REQ.md` - record scope and acceptance criteria. +- `devlog/2026-08-13_v1-tool-result-truncation/DESIGN.md` - document the V1-only converter data flow and deliberate V2 exclusion. + +## 3. Design & Implementation Notes + +- **Entry point / key function**: `octoToCoreMessages`, `reassemble`, `toolResultBody`, and `applyToolBody` in `messages-v1.ts`. +- **Key configuration items**: No adapter configuration changes. Kernel emergency truncation remains governed by its existing threshold. +- **Key logic explanation**: Projection and comparison share the same result-body helper. Changed bodies are copied into `output`, `error`, or `metadata.output` according to the original V1 status; object spreads retain the host state envelope. + +## 4. Testing & Verification + +### Build & Test Commands + +```sh +# From repo root +npm run typecheck +npm run test +npm run build +node smoke.mjs +bash scripts/ci/check-pr.sh 2026-08-13_v1-tool-result-truncation upstream/master +``` + +### Test Coverage + +- New/modified test files: `packages/billion-context-opencode/tests/messages.test.ts` +- Test count: 30 total, 30 pass, 0 fail (four new V1 converter cases) +- Key scenarios verified: Completed output rewrite; trailing-whitespace no-op and reference preservation; ordinary error rewrite with status/prefix handling; interrupted partial-output projection and metadata writeback. + +### Results + +- **PASS/FAIL**: `npm run typecheck` PASS; `npm run test` PASS (30/30); `npm run build` PASS; `scripts/ci/check-pr.sh` PASS; targeted kernel-to-V1 truncation round trip PASS. +- **Key logs/data**: A targeted check ran `acp-kernel` emergency truncation on a 12,010-character V1 completed result and verified that reassembly returned the 4,064-character marked result. `node smoke.mjs` could not start because the mainline script imports `dist/index.js`, while the current monorepo build writes `packages/billion-context-opencode/dist/index.js`. This pre-existing path mismatch is outside this V1 converter change and was not modified. The first typecheck attempt also ran before this fresh worktree had dependencies; after `npm ci`, typecheck passed. + +## 5. Risk Assessment & Rollback + +- **Risk points**: + - The ordinary-error core contains one adapter-added `Error: ` prefix that must not be persisted back into the host error field. + - Interrupted failures expose their model-visible partial result through `metadata.output`, not the ordinary error field. + - A future V1 host schema change could require updating the internal structural type. +- **Rollback method**: + - Revert commit(s): the eventual change commit. + - Rollback impact: Restores prior behavior where kernel result rewrites are discarded; no persisted data migration is involved. +- **Compatibility notes**: The dual-shape export, V2 converter, call/result pairing invariant, configuration, and persisted state format are unchanged. + +## 6. Lessons Learned + +- The V1 and V2 adapters must be scoped and validated independently even though they share the same runtime. + +## 7. Follow-ups + +- [ ] Consider an independent V2 investigation only when its real host contract and integration path can be validated. diff --git a/packages/billion-context-opencode/src/messages-v1.ts b/packages/billion-context-opencode/src/messages-v1.ts index 712c161..b915e9e 100644 --- a/packages/billion-context-opencode/src/messages-v1.ts +++ b/packages/billion-context-opencode/src/messages-v1.ts @@ -14,6 +14,7 @@ export interface OctoPart { input?: unknown output?: unknown error?: string + metadata?: Record title?: string } [key: string]: unknown @@ -43,6 +44,15 @@ function safeStringify(value: unknown): string { } } +function toolResultBody(state: OctoPart["state"]): string | undefined { + if (state?.status === "completed") { + return typeof state.output === "string" ? state.output : safeStringify(state.output) + } + if (state?.status !== "error") return undefined + const interruptedOutput = state.metadata?.interrupted === true ? state.metadata.output : undefined + return typeof interruptedOutput === "string" ? interruptedOutput : `Error: ${state.error ?? ""}` +} + export interface ConversionResult { cores: CoreMessage[] partIdToCoreIds: Map @@ -82,19 +92,13 @@ export function octoToCoreMessages(msgs: OctoMessage[]): ConversionResult { const ids = [callId] if (part.state?.status === "completed" || part.state?.status === "error") { const resultId = `${msg.info.id}#x${partIdx}` - const outText = - part.state.status === "completed" - ? typeof part.state.output === "string" - ? part.state.output - : safeStringify(part.state.output) - : `Error: ${part.state.error ?? ""}` cores.push({ id: resultId, role: "tool", contentType: "tool-result", toolName, toolCallId, - text: outText, + text: toolResultBody(part.state) ?? "", }) ids.push(resultId) } @@ -176,7 +180,9 @@ export function reassemble( const callAlive = outCoreById.has(ids[0]!) const resultAlive = outCoreById.has(ids[1]!) if (!(callAlive && resultAlive)) continue - parts.push(p) + // Honor kernel body mutations (emergency truncation of large + // tool-results): otherwise reassembly would restore the full V1 body. + parts.push(applyToolBody(p, outCoreById.get(ids[1]!))) continue } const survived = ids.some((id) => outCoreById.has(id)) @@ -194,6 +200,27 @@ export function reassemble( return result } +function trimEnd(s: string): string { + return s.replace(/\s+$/, "") +} + +export function applyToolBody(part: OctoPart, resultCore: CoreMessage | undefined): OctoPart { + const coreBody = resultCore?.text ?? "" + if (!coreBody) return part + const state = part.state + const originalText = toolResultBody(state) + if (!state || originalText === undefined) return part + if (trimEnd(coreBody) === trimEnd(originalText)) return part + if (state.status === "completed") { + return { ...part, state: { ...state, output: coreBody } } + } + if (state.metadata?.interrupted === true && typeof state.metadata.output === "string") { + return { ...part, state: { ...state, metadata: { ...state.metadata, output: coreBody } } } + } + const error = coreBody.startsWith("Error: ") ? coreBody.slice("Error: ".length) : coreBody + return { ...part, state: { ...state, error } } +} + export function makeNudgeMessage( id: string, sessionID: string, diff --git a/packages/billion-context-opencode/tests/messages.test.ts b/packages/billion-context-opencode/tests/messages.test.ts index fb2ae80..5446f17 100644 --- a/packages/billion-context-opencode/tests/messages.test.ts +++ b/packages/billion-context-opencode/tests/messages.test.ts @@ -110,6 +110,65 @@ test("compress + reassembly replaces covered messages with synthetic user summar assert.ok(hasU2, "recent uncompressed message preserved") }) +test("reassemble: kernel-truncated V1 tool body replaces completed output", () => { + const msgs = [toolMsg("a2", "s1", "bash", "call_1", "ORIGINAL_LONG_OUTPUT")] + const originalPart = msgs[0]!.parts[0]! + const { cores, partIdToCoreIds } = octoToCoreMessages(msgs) + const truncated = cores.map((c) => (c.id === "a2#x0" ? { ...c, text: "TRUNCATED_BODY" } : c)) + const out = reassemble(truncated, msgs, partIdToCoreIds, "s1") + const part = out[0]!.parts[0]! + assert.notEqual(part, originalPart, "changed body produces a copied part") + assert.equal(part.state!.status, "completed", "completed status preserved") + assert.equal(part.state!.output, "TRUNCATED_BODY", "truncated body applied to completed output") + assert.deepEqual(part.state!.input, { a: 1 }, "input preserved") +}) + +test("reassemble: trailing whitespace difference is not treated as a V1 tool rewrite", () => { + const msgs = [toolMsg("a2", "s1", "bash", "call_1", "done")] + const originalPart = msgs[0]!.parts[0]! + const { cores, partIdToCoreIds } = octoToCoreMessages(msgs) + const padded = cores.map((c) => (c.id === "a2#x0" ? { ...c, text: "done\n\n " } : c)) + const out = reassemble(padded, msgs, partIdToCoreIds, "s1") + assert.equal(out[0]!.parts[0], originalPart, "unchanged body keeps the original part reference") + assert.equal(out[0]!.parts[0]!.state!.output, "done", "original output kept") +}) + +test("reassemble: kernel-truncated V1 tool error preserves error state", () => { + const msg = toolMsg("a2", "s1", "bash", "call_1", "unused") + msg.parts[0]!.state = { status: "error", input: { a: 1 }, error: "ORIGINAL_LONG_ERROR", title: "bash" } + const { cores, partIdToCoreIds } = octoToCoreMessages([msg]) + assert.equal(cores.find((c) => c.id === "a2#x0")!.text, "Error: ORIGINAL_LONG_ERROR", "error projected for kernel") + const truncated = cores.map((c) => (c.id === "a2#x0" ? { ...c, text: "Error: TRUNCATED_ERROR" } : c)) + const out = reassemble(truncated, [msg], partIdToCoreIds, "s1") + const state = out[0]!.parts[0]!.state! + assert.equal(state.status, "error", "error status preserved") + assert.equal(state.error, "TRUNCATED_ERROR", "adapter prefix removed before writing error") + assert.equal(state.output, undefined, "completed output field not introduced") + assert.deepEqual(state.input, { a: 1 }, "input preserved") + assert.equal(state.title, "bash", "title preserved") +}) + +test("reassemble: interrupted V1 tool output is projected and rewritten in metadata", () => { + const msg = toolMsg("a2", "s1", "bash", "call_1", "unused") + msg.parts[0]!.state = { + status: "error", + input: { a: 1 }, + error: "Tool execution aborted", + metadata: { interrupted: true, output: "ORIGINAL_PARTIAL_OUTPUT", exitCode: 130 }, + title: "bash", + } + const { cores, partIdToCoreIds } = octoToCoreMessages([msg]) + assert.equal(cores.find((c) => c.id === "a2#x0")!.text, "ORIGINAL_PARTIAL_OUTPUT", "partial output projected for kernel") + const truncated = cores.map((c) => (c.id === "a2#x0" ? { ...c, text: "TRUNCATED_PARTIAL_OUTPUT" } : c)) + const out = reassemble(truncated, [msg], partIdToCoreIds, "s1") + const state = out[0]!.parts[0]!.state! + assert.equal(state.status, "error", "interrupted status preserved") + assert.equal(state.error, "Tool execution aborted", "original interruption error preserved") + assert.equal(state.metadata?.interrupted, true, "interrupted marker preserved") + assert.equal(state.metadata?.output, "TRUNCATED_PARTIAL_OUTPUT", "truncated partial output written where V1 reads it") + assert.equal(state.metadata?.exitCode, 130, "other metadata preserved") +}) + test("makeNudgeMessage produces a valid user message", () => { const msgs = [userMsg("u1", "s1", "hi")] const n = makeNudgeMessage("bili_nudge_0", "s1", "please compress", msgs)