Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions devlog/2026-08-13_v1-tool-result-truncation/DESIGN.md
Original file line number Diff line number Diff line change
@@ -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<string, unknown>`. 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.
60 changes: 60 additions & 0 deletions devlog/2026-08-13_v1-tool-result-truncation/REQ.md
Original file line number Diff line number Diff line change
@@ -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.
77 changes: 77 additions & 0 deletions devlog/2026-08-13_v1-tool-result-truncation/WORKLOG.md
Original file line number Diff line number Diff line change
@@ -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.
38 changes: 30 additions & 8 deletions packages/billion-context-opencode/src/messages-v1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,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<string, string[]>
Expand Down Expand Up @@ -116,19 +125,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)
}
Expand Down Expand Up @@ -249,7 +252,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))
Expand All @@ -271,6 +276,23 @@ export function reassemble(
return result
}

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,
Expand Down
59 changes: 59 additions & 0 deletions packages/billion-context-opencode/tests/messages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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("reassemble: assistant text parts carry no acp tag", () => {
const core = createCore()
const config = defaultConfig(200000)
Expand Down
Loading