redundant condition - remaining >= 0 is always true in this loop since remaining = files.length - i - 1 and i < files.length

This is a comment left during a code review.
Path: src/agents/pi-extensions/compaction-safeguard.ts
Line: 163

Comment:
redundant condition - `remaining >= 0` is always true in this loop since `remaining = files.length - i - 1` and `i < files.length`

How can I resolve this? If you propose a fix, please make it concise.

Preserve workspace rules when enforcing summary cap

Apply the size cap before appending workspace-critical rules, not after: this line truncates from the front-preserving prefix (capCompactionSummary uses summary.slice(0, budget)), so whenever historySummary is already near 16k chars, the trailing <workspace-critical-rules> block (Session Startup/Red Lines) gets cut off. In long sessions this removes the very post-compaction guardrails this extension is trying to restore, which can change agent behavior after compaction.

Useful? React with 👍 / 👎.

Good catch — this was a real truncation-order issue.

I updated the compaction safeguard summary capping so the main summary body is capped first and the <workspace-critical-rules> suffix is preserved within the final budget. I also added a regression test to ensure the workspace rules block survives summary truncation.

Cap main summary before appending diagnostic suffix sections

When summaries approach the 16k cap, this call truncates from the front-preserved prefix (capCompactionSummary uses slice(0, budget)), so the sections appended just before it (tool failures and file operations) are the first content dropped in long sessions. That silently removes exactly the high-signal diagnostics this extension adds, which can reduce continuity after compaction. Applying the cap to the main history body before appending those sections (or preserving them in a reserved suffix budget) would keep them reliably present.

Useful? React with 👍 / 👎.

Useful: slice(0, budget) keeps the beginning and trims the end, causing tool failures and file ops to be dropped first—though these are the most valuable diagnostics for compaction.

Preserve section separators when building reserved suffix

Building diagnosticSuffix from appendSummarySection("", toolFailureSection) strips the leading \n\n from the first suffix block, and capCompactionSummaryPreservingSuffix later concatenates body and suffix directly (${cappedBody}${suffix}). When the summary body does not end with a newline (for example buildStructuredFallbackSummary), this merges content like ...## Exact identifiers## Tool Failures or ...<workspace-critical-rules> into the prior line, which degrades heading/tag structure and can make the preserved diagnostics/rules less reliably interpreted after compaction.

Useful? React with 👍 / 👎.

Preserve recent-turn suffix when capping compaction summary

summaryWithPreservedTurns is built earlier in this function by appending ## Recent turns preserved verbatim to the end of the model summary, but this new cap path truncates the body with a prefix-preserving slice(0, budget) before adding the reserved suffix. In sessions where the summary exceeds the 16k budget, the tail is dropped first, so the “preserved recent turns” block can disappear even though it was explicitly extracted for continuity; this regression only appears on long summaries but directly weakens the compaction handoff.

Useful? React with 👍 / 👎.

Preserve tail sections when reserved suffix exceeds cap

When suffix.length >= maxChars, this branch returns suffix.slice(0, maxChars), which keeps only the start of the reserved suffix and drops everything at the end. In this code path the suffix is ordered as preserved turns → tool failures → file ops → <workspace-critical-rules>, so long preserved-turn blocks can still cut off the workspace rules and diagnostics the safeguard is meant to guarantee. Fresh evidence is the new capCompactionSummaryPreservingSuffix branch itself: it reintroduces tail-loss even after moving critical sections into the suffix.

Useful? React with 👍 / 👎.

Keep body prefix when truncation marker cannot fit

When maxChars is smaller than SUMMARY_TRUNCATED_MARKER.length, this branch returns a sliced marker instead of any summary content. In the capCompactionSummaryPreservingSuffix path, this happens whenever the reserved suffix consumes almost the entire 16k budget, so the remaining body budget can be tiny and you end up dropping the body entirely even though some body characters could still fit. In long compactions this can erase top-level sections (decisions/constraints) and replace them with a partial marker fragment.

Useful? React with 👍 / 👎.

Preserve split-turn context in the capped summary suffix

This cap now truncates lastSummaryWithoutPreservedTurns with a prefix-preserving slice before appending the reserved suffix, which means the tail of the body is dropped first when over budget. In split-turn compactions, the **Turn Context (split turn):** block is appended at the end of summaryWithoutPreservedTurns, so long summaries can silently lose that split-turn context while keeping earlier body text; that regresses continuity specifically for preparation.isSplitTurn sessions. Consider reserving suffix budget for the split-turn block (or moving it into the preserved suffix set) so it is not the first content removed.

Useful? React with 👍 / 👎.

Honor last successful summary after retry failure

When a quality-guard retry fails after at least one successful attempt, the catch block sets summary = lastSuccessfulSummary and breaks, but the final assignment here unconditionally rebuilds summary from lastHistorySummary plus suffix. In split-turn sessions this is especially problematic because lastSplitTurnSection is reset before the failing prefix summarization call, so the returned summary can silently drop split-turn context and ignore the intended fallback-to-last-successful behavior under transient model/API errors.

Useful? React with 👍 / 👎.

Preserve retry-fallback tail context during summary capping

In the usedLastSuccessfulSummary branch, the cap is applied to the entire prior summary body while the reserved suffix only includes diagnostics/workspace data, so split-turn context and ## Recent turns preserved verbatim remain in the cap-truncated body tail. Because capCompactionSummary keeps the prefix and drops the end, long fallback summaries (for example after a retry error on attempt 2+) can still lose those continuity sections even though they were explicitly preserved earlier; this is fresh evidence in the new retry-fallback branch, not the normal path.

Useful? React with 👍 / 👎.

Let one file-op list consume the full section budget

In read-only or edit-only compactions, formatFileOperations caps the populated list at MAX_FILE_OPS_LIST_CHARS (900) before the 2000-character section budget is applied. Because formatBoundedFileList already counts its tags inside that 900-character limit and sections.join("\n\n") adds at most four more characters, the outer capCompactionSummary(..., MAX_FILE_OPS_SECTION_CHARS) is unreachable here; a single non-empty list therefore drops roughly half of the available file-path budget even when the overall summary is still well under the cap.

Useful? React with 👍 / 👎.

Guard the summary fallback against suffix duplication

If summarizeInStages() comes back with an empty string while recent-turn preservation or split-turn context is active, lastHistorySummary stays falsy and this fallback uses summary as the body even though summary already includes lastSplitTurnSection / preservedTurnsSection from the loop. reservedSuffix is then appended a second time, so the final compaction summary duplicates the preserved-turn or split-turn blocks and can blow past the budget again. This is reachable because summarizeChunks() returns summary ?? DEFAULT_SUMMARY_FALLBACK in src/agents/compaction.ts, so a blank model response is preserved as "" instead of being normalized.

Useful? React with 👍 / 👎.

Return the validated split-turn summary instead of rebuilding it

For split-turn compactions, summaryWithoutPreservedTurns is built with a \n\n---\n\n divider before **Turn Context (split turn):**, and that combined text is what auditSummaryQuality() checks. This new return path throws that string away and reconstructs the summary from lastHistorySummary plus lastSplitTurnSection, so every split-turn summary now loses the divider even when it is well under 16k. That folds the prefix context back into the trailing structured section instead of keeping it as a distinct handoff block, which weakens continuity for any preparation.isSplitTurn session.

Useful? React with 👍 / 👎.