Skip to content

fix(runtime): deliver the history-compact instruction as a trailing user message - #4797

Merged
Astro-Han merged 2 commits into
apache:mainfrom
ggbdpq:fix/history-compact-trailing-instruction
Sep 7, 2026
Merged

Astro-Han merged 2 commits into
apache:mainfrom
ggbdpq:fix/history-compact-trailing-instruction

Conversation

@ggbdpq

@ggbdpq ggbdpq commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the root cause of #4634: the history summarizer delivered SUMMARIZATION_SYSTEM_PROMPT as provider instructions (system role). Agentic coding models such as kimi-coding-plan / k3-256k ignore that shape and keep answering the conversation, so every fold failed open with malformed_summary_missing_section, and the distinct system prompt also forfeited the main loop's prefix cache for every discarded summarizer call.

The instruction now rides as the request's trailing user message — the request already ended with one for chat-template models (#4559); generateSummary appends a single user message combining the sectioned prompt (or its shorten/repair variant) with the existing summary request, and AiSdkGenerateTextOptions drops the now-unused instructions field.

Scope: direction 1 from the issue. Of the contributing defects listed there, the missing output cap is already fixed on current main by #4653 (DEFAULT_HISTORY_COMPACT_MAX_OUTPUT_TOKENS, landed 09-03, after the reporter's build); extending the failure circuit to provider_error/output_length and bounding the summarizer input budget remain open follow-ups best split into their own issues.

Verification

Claim Command Result
Targeted suites green npm --workspace @maka/core run build && npm --workspace @maka/storage run build && npm --workspace @maka/runtime run build, then node --test on history-compact-summarizer / effective-history-compaction / mid-turn-capacity-backend dist tests 139 tests, 139 pass, 0 fail
Red-green: the new test fails without the fix git stash push -- packages/runtime/src/history-compact-summarizer.ts, rebuild, rerun the summarizer suite 53 tests, 3 fail (the new trailing-instruction test + the two variant-prompt captures); restored → 53/53 pass
New regression test delivers the summarization instruction as the trailing user message (#4634) asserts no instructions field reaches generateText and the final user message carries both the sectioned prompt and the summary request
Format npm run format:check exit 0, 1,942 files clean
License headers npm run check:asf-headers both changed files pass

AI use

Implemented with ZCode (GLM-5.3-Flash): read the issue's reproduction data, traced the summarizer request shape in packages/runtime/src/history-compact-summarizer.ts, and made the minimal request-shape change with a red-green regression test. The commit carries the Generated-by trailer.

Checklist

  • Root-cause fix in the shared summarizer builder; no per-provider branching requested by the issue
  • Regression test fails on the old code (verified via stash/rebuild)
  • Test style follows the existing file (node:test + assert/strict, issue number in the test name)
  • No new dependencies; docs unchanged

…ser message

The history summarizer sent SUMMARIZATION_SYSTEM_PROMPT as provider
instructions (system role). Agentic coding models such as kimi k3-256k
ignore a system prompt in this shape and keep answering the conversation
instead, so mid-turn compaction failed open forever with
malformed_summary_missing_section (51 consecutive failed-open decisions
observed; 180 history_compact calls and zero checkpoints on one
workspace). A distinct system prompt also started the summarizer request
outside the main loop's cached prefix, so every discarded attempt paid a
full uncached pass (up to 88s observed).

Fold the instruction into the trailing user message the request already
ends with (apache#4559): generateSummary now appends one user message combining
the sectioned prompt (or its shorten/repair variant) with the summary
request, and AiSdkGenerateTextOptions drops the instructions field.

Fixes apache#4634

Generated-by: GLM-5.3-Flash (ZCode)
@github-actions github-actions Bot added the effort/S Under 100 readable lines label Sep 4, 2026

@hqhq1025 hqhq1025 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.

Reviewed exact head 903aac5ae22ac823724b3cc47ba83a90b342e54f.

No P0-P3 findings.

The change consistently removes the separate system-role instructions field and appends the full sectioned summarization prompt plus the existing summary request as the final user message for the initial call, output-limit retry, and malformed-summary repair. Rolling checkpoints still prepend the previous summary and replay only newly folded events. I also exercised the real AI SDK lowering with a mock language model: the provider-facing prompt retained the original conversation followed by one final user instruction and contained no system message.

Validation completed: Core, Storage, and Runtime builds; 139 focused compaction tests; full Runtime suite (3,201 passed, 13 skipped, 0 failed); repository lint, format, and ASF header checks; git diff --check; clean exact-main merge; and the hosted test check on this exact head.

Residual limitation: I did not call the live k3-256k endpoint, so provider-specific behavior is supported by the issue reproduction rather than independently re-run here.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@Astro-Han Astro-Han 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.

Thanks for the change. Approving exact head 903aac5ae22ac823724b3cc47ba83a90b342e54f following the completed review and Astro-Han’s explicit acceptance of this merge batch. Current checks pass and no review threads remain unresolved.

Some coding models ignored system-role compaction instructions and failed to produce a valid summary. Move the shared initial, shorten and repair instruction into the existing trailing user message, retaining output validation, budgets and checkpoint ownership. No persisted-data migration; cache benefit is not independently established.

AI assistance: Codex performed the review and final-state verification; Astro-Han authorized approval and merge.

中文

感谢改动。基于已完成的审查和 Astro-Han 对本批次的明确认可,批准当前精确 head;检查通过,讨论已结清。此前说明的验证边界与后续事项保持不变。本次由 Codex 执行审查和状态核对,Astro-Han 授权批准与合并。

@Astro-Han
Astro-Han merged commit e4b7396 into apache:main Sep 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants