Repository navigation
🪟 fix: Summarize the Pruned Turn Window Safely - #647
lia-by-librechat[bot] wants to merge 23 commits into
Conversation
|
Head: 091fa4a. Compaction now consumes the pruned masked window, preserves history on failure, retries transient provider errors, and emits classified failure codes. Focused compaction tests and TypeScript checks passed; independent review, extended regressions, build verification, and CI are running for this head. |
|
Head: ddb8ed5. Fixed the verified review findings: pruned-window skip (P1), stale session-summary commits (P1), retry callback leakage (P2), and explicit retention overrides (P2). Added regression coverage and corrected tight-budget fixtures. Local focused checks: 751 tests passing, TypeScript clean, touched-file lint/import ordering clean. Fresh independent review, package build, and CI are running for this head. |
|
Head: 80f101d. The invariant sweep fixes tool-only recent-tail retention (P2) and compression against residual input space (P2). Added paired native/canonical tool-window, residual-budget, and pre-commit cancellation regressions. The live thinking fixture now reserves explicit output and input headroom. Fresh independent review, final focused checks, package build, and CI are running for this head. |
|
Head: 18fb9dc. Fixed per-fallback input budgeting (P2), with regressions for larger context windows, smaller output reserves, and a too-small fallback before a viable one. Local checks: 766 focused tests, TypeScript, build, and touched-file static checks passed. The Anthropic extended-thinking failure remains under investigation; this head adds content-safe completion diagnostics for CI. |
|
Head: cba605d. CI diagnosed the remaining Anthropic failure: the summarizer returned a calculator call with thinking but no summary text, below its output limit. Summary requests now set no-tool choice on supported providers without removing tool definitions or changing reasoning. Bedrock retains its supported options. Offline provider-wire, fallback, and retry tests pass; fresh exact-head review, full focused checks, build, and CI are running. |
|
Head: 8cca79a.
|
|
@codex review the latest head, final review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cca79a83a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } = await executeSummarizationWithFallback({ | ||
| agentContext, | ||
| messages: messagesToRefine, | ||
| messages: windowMessages, |
There was a problem hiding this comment.
When token pruning triggers compaction, Graph.ts puts only the surviving context in windowMessages, while the older messages that actually caused the trigger are in the separate messagesToRefine array. Passing windowMessages here means the model summarizes only recent messages; after success, the node emits removeAll and retains just the recent tail, permanently discarding every older pruned message without representing it in the summary. This loses conversation history on the first compaction of any thread larger than the provider window.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Intentional trade-off in the requested design, not a regression to reverse here. The handoff explicitly says: “Unsummarized history outside the window is dropped” and “Recovering it would need a multi-call summary over cold input, which is the latency this fix removes.” Compaction must summarize the same pruned, masked window the turn sees; restoring omitted history would recreate the oversized, cache-cold request. The 720-message regressions assert that only that window plus the instruction is sent. Retaining all archival history would require a separate multi-call design.
|
Head: 55d95d3. Addressed four Codex P2 findings: fallback delta delivery/isolation, serving-model token counts, cancellation propagation, and unsent-overhead reservation. Rejected the P1 archival-history proposal because it contradicts the handoff’s explicit bounded-window trade-off; rationale is in its thread. Local: 965 focused tests passed, TypeScript/build/touched-file lint/import checks clean. Fresh independent review and CI are running for this exact head. |
|
@codex review the latest head, final review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55d95d3500
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Head: a2a62da. The remaining Anthropic failure returned reasoning without summary text. Those unusable responses now share the existing three-attempt retry cap and abortable backoff; failed reasoning chunks stay unpublished. Explicit truncation, refusal, and tool-use outcomes are excluded. Exhaustion preserves history and allows configured fallbacks. Regression tests passed without weakening the live CI assertion. Fresh independent review, full focused checks/build, and CI are running for this exact head. |
|
Head: 4fb264e.
No package publication or deployment. LibreChat UI wiring, demo verification, and live Langfuse export verification remain outside this SDK delivery. |
|
@codex review the latest head, final review |
|
Head: d96547d.
No package publication or deployment. LibreChat UI wiring remains blocked by separate-repository workspace access. Demo and live Langfuse export verification were not run. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fb264e453
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Head: bb4253f.
No model-end billing events are replayed. No package publication or deployment. LibreChat UI wiring, demo verification, and live Langfuse export verification remain outside this SDK delivery. |
|
@codex review the latest head, final review |
|
Head: ab0c2d6. The four new Codex findings remain fixed and resolved. Addressed the latest independent review finding too: empty/whitespace reasoning markers no longer trigger paid retries; genuine structured/encrypted reasoning remains eligible. Added shared-detector controls and a real Google thought-converter regression. Pre-push tests/typecheck/build passed. Final-head checks, CI, and fresh independent review are running. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb4253f611
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Head: d725ca1. Closed the three latest review boundaries: fixed Google thinking budgets are included in default output/input reserves; all reasoning payload formats require meaningful evidence; and completed usage is observed before invocation-level truncation validation. Provider terminal stops, explicit output caps, and normal billing callbacks remain intact. Focused validation passed before push. Fresh exact-head CI, independent review, and final local build/checks are running. |
|
Head: 242a672. Fixed the new Codex cache-token accounting finding and the latest independent findings: retry ownership is centralized, Bedrock history checkpoints survive system-prefix replay, and nonempty string-form reasoning summaries remain recoverable. Actual OpenAI transport tests verify three HTTP attempts per configured provider. Native Bedrock cache-prefix/accounting tests pass. Local: 1,345 focused tests, typecheck/build/static checks passed. Fresh exact-head CI and independent review are running. |
|
Head: cab79ae.
No publication or deployment. LibreChat UI wiring remains blocked by separate-repository workspace access. Demo long-chat/cache-read validation, live Moonshot service checks, and live Langfuse export verification were not run. |
|
@codex review the latest head, final review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cab79ae1fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Head: 5e0f96e.
The archival-history P1 proposal remains rejected per the explicit bounded-window trade-off. No publication or deployment. LibreChat UI wiring, demo validation, live Moonshot checks, and live Langfuse export verification remain outside this SDK delivery. |
|
@codex review the latest head, final review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Head: c709cfa. Reopened verification after the external review identified a visible-history coverage defect. Input bounding now protects the entire compactable head: compress tool output, then omit only the retained tail; if the head still cannot fit, leave history unchanged. Summary output is capped separately from agent output, with explicit input/window budget provenance. Anthropic tool choice matches the ordinary turn, tool-only replies recover without execution, and overload/rate-limit retries use seconds plus Retry-After. Added removed-message coverage, net-budget, retained-ID/signature, and failure-preservation regressions. Local: 1,456 focused tests, typecheck/build/static checks passed. Fresh exact-head CI and independent review are running. |
|
Head: 92f32b3. Fixed the independent review’s overflow-unit finding: a corrected input ceiling stays in input units for compaction and inherited fallbacks, then restores configured units on reset. The protected-head invariant remains unchanged. Tight synthetic CI scenarios now use explicit checkpoint instruction/headroom settings rather than depending on lossy shrinking; their assertions are unchanged. Focused overflow/protected-history/type checks passed. Final-head local tests/build/static checks, CI, and fresh independent review are running. |
|
Head: 7260b06. Budget modes now follow the source of their ceiling: inherited input budgets cannot be relabeled by a mode-only fallback override. Explicit fallback ceilings keep their own units. Added coverage for every inherited-mode variant. Default checkpoint instructions now explicitly forbid tools/calculations/task continuation, while Anthropic tool choice remains identical to the normal turn for cache reuse. Protected-head deletion coverage and unchanged-history failures remain intact. Pre-push focused tests/typecheck/static checks passed. Final exact-head CI, independent review, and full focused local checks/build are running. |
|
Head: e5d4d07.
The original cold-history-outside-the-window trade-off is unchanged. No publication or deployment. LibreChat UI wiring remains blocked by workspace access. Demo long-chat/message-history-cache validation, live Moonshot checks, and live Langfuse export verification were not run. |
|
Head: d443094.
Partial overflow compaction remains deferred in favor of unchanged-history failure. Retained IDs/signatures are covered. Reasoning without an inherited output cap retains the bounded default, not an inferred provider maximum. No publication/deployment. Demo, live Moonshot, and live Langfuse export verification were not run. |
|
Head: 0d25cde.
Partial overflow compaction remains deferred; oversized protected heads fail without deleting history. Retained IDs/signatures are covered. Reasoning without an inherited limit retains the bounded default. No publication/deployment. Demo, live Moonshot, and live Langfuse export validation were not run. |
|
Head and reviewed SHA: ed29ef3.
Partial overflow remains deferred; oversized protected heads fail unchanged. Retained IDs/signatures are covered. No inferred provider maxima when an inherited cap is absent. No merge, publication, or deployment. Demo long-chat/cache validation, live Moonshot, and live Langfuse export checks were not run. Tooling note: reviewer flagged the loaded codegraph skill as stale (2026-10-23.2 vs 2026-10-29.1); frozen-head source locations were independently confirmed. |
|
@codex review the latest head, final review |
There was a problem hiding this comment.
💡 Codex Review
agents/src/summarization/node.ts
Lines 1854 to 1857 in ed29ef3
Restore canonical messages before persisting the retained tail
When compaction follows a prune pass that masked or truncated a recent consumed tool result, Graph.ts supplies the pruner's mutable provider projection as windowMessages, so messagesToRetain contains the shortened projection rather than the canonical state message. Returning that tail after removeAll permanently replaces the full recent tool result in graph/checkpoint state with its masked form, even though this tail is supposed to survive compaction verbatim; retain the corresponding messages from state.messages while continuing to send the bounded projection to the summarizer.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (openai) nativeLimit = native?.max_output_tokens; | ||
| else if (anthropic) nativeLimit = native?.max_tokens; | ||
| return (nativeLimit ?? (openai ? options.maxCompletionTokens : undefined) ?? | ||
| options[getMaxOutputTokensKey(provider)]) as number | undefined; |
There was a problem hiding this comment.
When a caller sets an explicit per-summary cap through summarizationConfig.parameters.modelKwargs.max_completion_tokens (or the legacy max_tokens), this reader ignores it, and clearSummaryOutputLimits immediately deletes it. The request then uses the inherited agent allowance or default instead, which can request far more output and reserve the wrong amount of a window-mode context budget; read these accepted native aliases before canonicalizing them.
Useful? React with 👍 / 👎.
| maxContextTokens: fb.maxContextTokens, | ||
| contextBudgetMode: fb.contextBudgetMode, | ||
| config: fbConfig, |
There was a problem hiding this comment.
When a public tryFallbackProviders caller still uses prepareProviderMessages, a fallback configured with contextBudgetMode never exposes that value to the callback: it is included only in preparationInput for prepareProviderRequest, while the legacy callback is invoked with a separately constructed object that omits it. Such callers therefore cannot distinguish an input-token ceiling from a full-window ceiling and may either over-prune or send an oversized fallback request despite the callback type advertising this field.
Useful? React with 👍 / 👎.
| const canDisableTools = | ||
| family === 'openai' || | ||
| family === 'google' || | ||
| provider === Providers.MOONSHOT; | ||
| if (disableToolCalling !== true || !canDisableTools) | ||
| return model.bindTools(tools); |
There was a problem hiding this comment.
When an Anthropic agent has bound tools, disableToolCalling: true still takes the plain bindTools(tools) branch because the Anthropic family is excluded here, even though this repository's CustomAnthropic adapter explicitly supports tool_choice: 'none'. The summarizer can therefore emit a tool call instead of checkpoint text, causing repeated billed attempts and eventual compaction failure; include the Anthropic family in the supported capability set.
Useful? React with 👍 / 👎.
Summary
Compaction rebuilt stored history and restored masked tool output, producing oversized, cache-cold requests. Later bounding also risked deleting visible messages that were never summarized.
Verification
Current head: ed29ef3.
Cold history outside the original turn window remains the accepted trade-off. No visible history omitted by summary bounding is deleted. Partial overflow remains deferred in favor of unchanged-history failure. Reasoning without an inherited cap retains the bounded default; no provider maxima are inferred.
No publication or deployment. LibreChat UI wiring remains blocked by workspace access. Demo long-chat/message-history-cache validation, live Moonshot checks, and live Langfuse export verification were not run.