Skip to content

fix(runtime): bound text history summarizer inputs - #3113

Merged
M4n5ter merged 2 commits into
apache:mainfrom
Sun-GLiang:fix/3013-bound-text-history-inputs
Aug 16, 2026
Merged

fix(runtime): bound text history summarizer inputs#3113
M4n5ter merged 2 commits into
apache:mainfrom
Sun-GLiang:fix/3013-bound-text-history-inputs

Conversation

@Sun-GLiang

Copy link
Copy Markdown
Contributor

Summary

  • Honor the history compaction input budget in the text LLM summarizer before provider dispatch.
  • Share deterministic request fitting with provider-native compaction, omitting older Tool Result payloads while preserving call/result pairs and newer grounded context.
  • Charge fixed summarization instructions against the budget, fail locally with typed input_too_large when the request still cannot fit, and preserve the existing provider-specific helper export.

Fixes #3013

Verification

  • node --test packages/runtime/dist/__tests__/history-compact-input-fit.test.js packages/runtime/dist/__tests__/history-compact-summarizer.test.js packages/runtime/dist/__tests__/openai-codex-history-compactor.test.js — 22 passed, 0 failed.
  • npm run lint — passed.
  • npm run format:check — passed.
  • npm --workspace @maka/runtime run typecheck — passed.
  • npm --workspace @maka/runtime run test — affected tests pass; the latest full run had 2,861 passed, 1 failed, and 12 skipped because the pre-existing Unix PTY lifecycle test timed out after 10 seconds. The same test failed on the clean baseline and also passed on intervening full runs.
  • npm run typecheck — blocked by the existing desktop test fixture at apps/desktop/src/main/__tests__/runtime-host-external-sessions-ipc-main.test.ts:19, which is missing the required importState field.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the runtime fitting helper, integrations, tests, review fixes, and verification.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions
github-actions Bot requested a lite review from Copilot August 16, 2026 09:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 444dcff5-a492-4afb-aa59-e2e7e109f8b6

📥 Commits

Reviewing files that changed from the base of the PR and between b121f75 and db344ac.

📒 Files selected for processing (1)
  • packages/runtime-host/src/__tests__/execution-model-composition.test.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Summary

This PR prevents the runtime text history summarizer from sending requests that exceed inputBudget. It bounds older Tool Result payloads while preserving complete Tool Call/Tool Result pairs and newer grounded context. It also includes fixed summarization instructions in the budget calculation.

If the remaining non-tool history cannot fit, the summarizer throws a typed input_too_large error before provider dispatch. Existing fail-open and diagnostic behavior remains unchanged. Rolling-summary behavior continues to include previousCheckpoint and newly folded events.

Design

The PR extends the existing compaction behavior through the shared fitHistoryCompactMessages helper. Provider-native compaction re-exports this helper as fitOpenAiCodexCompactionMessages. The change does not create a parallel fitting algorithm.

The solution is small and coherent. It centralizes deterministic request-fitting logic and removes the duplicated OpenAI Codex implementation. The placeholder logic is necessary to preserve chronology, output type, and Tool Call/Tool Result pairing while reducing input size.

The deleted provider-specific fitting code and its obsolete tests do not weaken coverage. The shared fitter tests cover the same provider-fitting behavior. The summarizer tests add coverage for the runtime path.

Validation

Tests cover:

  • Oversized Tool Result handling.
  • Tool Call/Tool Result pairing.
  • Preservation of newer context.
  • Oversized non-tool history.
  • Fixed summarization instruction accounting.
  • Already-fitting and unbudgeted projections.
  • Rolling-summary behavior.
  • Provider-specific helper compatibility.

The reported validation passed lint, formatting, runtime typechecking, and affected tests. One full runtime test had a pre-existing Unix PTY lifecycle timeout. Repository-wide typechecking remains blocked by an existing desktop test fixture issue. Required check status is unverified from direct evidence.

Review-relevant risks

The change can alter outgoing summarization input by replacing older Tool Result payloads with placeholders. This can affect summary content. Material changes in user-visible behavior require independent human review under repository policy.

The change preserves the exported fitOpenAiCodexCompactionMessages name but changes its implementation to use shared logic. Material changes to a public contract require independent human review under repository policy.

No security, licensing, release, or governance effect was identified in the current diff.

The person performing the merge reviews the final diff. A maintainer makes the final determination.

Walkthrough

The change adds shared input fitting for history compaction. It bounds older tool outputs, preserves tool-call/result pairing, accounts for prompt overhead, and raises input_too_large before dispatch when the remaining history exceeds the budget.

Changes

History compaction input fitting

Layer / File(s) Summary
Shared input-fitting implementation
packages/runtime/src/history-compact-input-fit.ts, packages/runtime/src/__tests__/history-compact-input-fit.test.ts
Adds budget estimation, tool-result replacement, unchanged fast paths, and typed oversized-input failures.
Text summarizer budget enforcement
packages/runtime/src/history-compact-summarizer.ts, packages/runtime/src/__tests__/history-compact-summarizer.test.ts, packages/runtime-host/src/__tests__/execution-model-composition.test.ts
Fits projected history after checkpoint insertion and tests tool-result truncation, prompt overhead, non-tool failures, rolling summaries, and adjusted high-water fixtures.
Codex compactor consolidation
packages/runtime/src/openai-codex-history-compactor.ts, packages/runtime/src/__tests__/openai-codex-history-compactor.test.ts
Reuses the shared fitter and retains coverage for already-bounded Codex input.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to db344

The PR bounds text history summarizer inputs and locally rejects requests that still exceed the budget; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant HistorySummarizer
  participant fitHistoryCompactMessages
  participant ModelProvider
  HistorySummarizer->>fitHistoryCompactMessages: projected messages, token budget, prompt length
  fitHistoryCompactMessages-->>HistorySummarizer: bounded messages or input_too_large
  HistorySummarizer->>ModelProvider: fitted summarization request
Loading

Possibly related PRs

Suggested reviewers: m4n5ter

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Ai Use Disclosure ⚠️ Warning The PR names Codex and says it implemented the helper, integrations, and tests, but db344ac has no Generated-by trailer; only b121f75 has one. Add Generated-by: Codex to db344ac (or amend/squash so each material-AI commit carries it). See CONTRIBUTING.md §Human ownership and AI attribution; ensure it survives squash or amend.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: bounding text history summarizer inputs.
Description check ✅ Passed The description covers the summary, linked issue, verification results, AI use, checklist, and behavior change with appropriate detail.
Linked Issues check ✅ Passed The changes implement issue #3013 requirements, including budget fitting, tool-result handling, local input_too_large failure, and preserved summarizer behavior.
Out of Scope Changes check ✅ Passed The changes remain focused on history-compaction input fitting, related tests, and necessary runtime-host test adjustments.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions
github-actions Bot requested a lite review from Copilot August 16, 2026 09:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I have manually reviewed the final diff and take responsibility for this approval. The implementation is focused, correct, and adequately tested.

I will ensure the required Generated-by: Codex trailer is included in the squash commit.

@M4n5ter
M4n5ter merged commit ae2a4b8 into apache:main Aug 16, 2026
12 checks passed
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the fix — the direction (bounding before dispatch) is right, the shared fit helper is the correct seam, and the unit tests are behavior-level. Since this is already merged, these are follow-up findings rather than blockers.

One accounting bug in the shared fit helper (verified by reproduction), plus two boundary notes.

P2-1 — fitHistoryCompactMessages mis-accounts a single tool message with multiple tool-result parts.
history-compact-input-fit.ts:30,46-49 captures message once outside the parts loop, rebuilds content from the original message on every replacement, and writes only the last replacement back to bounded — but credits every replacement's savings to estimatedChars. I reproduced against the compiled output: one tool message with two 500-char tool results, budget 360 → returned projection is 781 chars (2.2× over budget) with the first part's original payload intact, while estimatedChars claims it fits. The repo's own producers (history-compact-summarizer.ts, openai-codex-history-compactor.ts) only emit single-part tool messages, so this is latent today — but the helper is a public export (fitOpenAiCodexCompactionMessages via @maka/runtime/openai-codex-history-compactor), and the AI SDK message type allows multi-part tool content, so external callers can hit it. Fix is ~1 line (re-read bounded[messageIndex] inside the parts loop); a unit test with a multi-part oversized tool message would pin it.

P2-2 — the summarizer request's output side is not bounded. history-compact-summarizer.ts:112-121 calls generateText without maxOutputTokens, and the fit only guarantees input ≤ capacity − reserve. On narrow windows (e.g., 32k window, 8k reserve, provider default max output 8–16k), a near-limit summarizer input can still exceed the window on the wire — the exact "post-dispatch overflow" #3013 wanted to remove. Setting maxOutputTokens (e.g., 1,024) and reserving it would close this.

P2-1' — a text-heavy fold can now regress a turn from completed to context_budget_exhausted, and the E2E test was parameter-tuned around the new failure path rather than asserting it. execution-model-composition.test.ts lowers HISTORY_PRESSURE 160→128 with a comment explaining it avoids the text-only compact input exceeding the fixture's summarizer budget. That's an avoidance, not coverage: when the fold span falls in (capacity−reserve, capacity] and the history is text-heavy, the new local input_too_large → fail-open → main request overruns the window → turn terminates. A smaller fold could have fit the summarizer budget and saved the turn, but nothing tries. Consider a planner retry with a smaller covered count, or at least a backend-level test asserting the new terminal outcome.

Optional nits (P3): coordinate the rebase with #3039/#3040/#3046 (they share files and build the prompt differently — SUMMARIZATION_SYSTEM_PROMPT.length charging is sensitive to their prompt refactor); the input_too_large reason has no end-to-end test at the caller level; two newly-added inputBudget test params are inert; the codex re-export test is near-tautological (its value is API-compat documentation, fine to keep).


AI-assisted review disclosure: this review was produced with AI assistance (two pi review subagents on ollama-cloud/deepseek-v4-flash). The P2-1 accounting bug was reproduced against the compiled output with an isolated script; the P2-2 output-side analysis and P2-1' regression analysis are traced from the code, not observed in a live session. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS(已合并,follow-up 建议)。① P2-1(已实证复现):共享 fit helper 对单条 tool 消息含多个 tool-result part 时核算错误——每次替换从原始 content 重建,只有最后一次替换存活但每次节省都计入;budget=360 时返回 781 字符(超 2.2×)。仓库内生产者当前只生成单 part(latent),但 helper 是公共导出(fitOpenAiCodexCompactionMessages),外部调用方可命中。修复约 1 行(parts 循环内重读 bounded[messageIndex])+ 补多 part 单测。② P2-2:summarizer 调用未设 maxOutputTokens、输出侧未预留,窄窗口下 dispatch 后仍可能超窗——正是 #3013 想消除的"dispatch 后超限"。③ P2-1':text-heavy 折叠可能让 turn 从 completed 回归为 context_budget_exhausted,且 E2E 测试把 HISTORY_PRESSURE 160→128 调参绕开新失败路径而非断言它;建议 planner 缩小折叠跨度重试或补后端级测试。P3 可选:与 #3039/#3040/#3046 的 rebase 协调(共享文件、prompt 长度计费对其重构敏感)、input_too_large 无端到端测试、两个惰性 inputBudget 测试参数。

UncertaintyDeterminesYou4ndMe added a commit to UncertaintyDeterminesYou4ndMe/maka-agent that referenced this pull request Aug 17, 2026
…ured summary

Rebase past apache#3113: its new bounding test stubs a free-form summary and
covers a >10k-token span at charsPerToken=1, so this PR's validation
rejects it by design (missing sections, then the size floor). The stub
now returns a structured summary proportionate to the fold; the test's
own assertions (bounded input, preserved pairs) are unchanged.

Generated-by: Claude Code
UncertaintyDeterminesYou4ndMe added a commit to UncertaintyDeterminesYou4ndMe/maka-agent that referenced this pull request Aug 19, 2026
…ured summary

Rebase past apache#3113: its new bounding test stubs a free-form summary and
covers a >10k-token span at charsPerToken=1, so this PR's validation
rejects it by design (missing sections, then the size floor). The stub
now returns a structured summary proportionate to the fold; the test's
own assertions (bounded input, preserved pairs) are unchanged.

Generated-by: Claude Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(runtime): bound text history summarizer inputs

4 participants