perf: force condence ended orphaned ended sessions

[peyton-alt]

No description provided.

[cursor]

PR Summary

^{Cursor Bugbot is generating a summary for commit 091a9a4. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Comment @cursor review or bugbot run to trigger another review on this PR

[cursor]

Force-condensed log emitted even when condensation fails

Low Severity

h.forceCondensed = true and the logging.Info "force-condensed ended session" message execute unconditionally after condenseAndUpdateState, even when condensation fails and h.condensed is false. This produces a misleading info-level log claiming the session was force-condensed, right alongside the warning log from the failed condensation, making debugging harder.

 

[Copilot]

Pull request overview

This PR addresses a PostCommit performance issue where ENDED sessions with carry-forward FilesTouched but no overlap with later commits could be re-processed indefinitely, causing cumulative per-commit overhead (issue #591). It introduces a “force-condense” path to ensure those sessions get condensed once and then skipped on subsequent commits.

Changes:

• Add a force-condensation path for ENDED sessions with FilesTouched and new content even when there’s no commit overlap.
• Update PostCommit phase tests to assert the new force-condense behavior (cleared FilesTouched, reset counters, FullyCondensed set).
• Add benchmarks to measure PostCommit overhead with accumulated ENDED sessions and verify the “second commit after fix” performance.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
cmd/entire/cli/strategy/manual_commit_hooks.go	Adds forceCondensed flow to condense ENDED sessions without overlap and skip carry-forward.
cmd/entire/cli/strategy/phase_postcommit_test.go	Updates test expectations to confirm ENDED sessions are force-condensed and marked FullyCondensed.
cmd/entire/cli/strategy/accumulation_bench_test.go	Adds benchmarks and repo/session setup helpers to quantify accumulation overhead and validate the fix.

[Copilot]

forceCondensed is set and an Info log is emitted even if condenseAndUpdateState fails (returns false). This can produce misleading logs and can cause the handler state to imply a successful force-condensation when it didn’t happen. Consider only setting forceCondensed / logging when h.condensed is true (and/or log at Warn when condensation fails).

Suggested change

	h.forceCondensed = true
	logging.Info(logCtx, "post-commit: force-condensed ended session (no commit overlap)",
	slog.String("session_id", state.SessionID),
	slog.Int("files_touched", len(state.FilesTouched)),
	)
	if h.condensed {
	h.forceCondensed = true
	logging.Info(logCtx, "post-commit: force-condensed ended session (no commit overlap)",
	slog.String("session_id", state.SessionID),
	slog.Int("files_touched", len(state.FilesTouched)),
	)
	} else {
	logging.Warn(logCtx, "post-commit: force-condense failed; session not condensed",
	slog.String("session_id", state.SessionID),
	slog.Int("files_touched", len(state.FilesTouched)),
	)
	}

[Copilot]

id.Generate() returns an error, but it’s being ignored via the blank identifier. The repo’s golangci-lint config enables errcheck with check-blank: true, so this will fail linting. Please handle the error (e.g., b.Fatalf on failure) and only use the generated ID when successful.

Suggested change

	cpID, _ := id.Generate()
	cpID, err := id.Generate()
	if err != nil {
	b.Fatalf("generate checkpoint ID: %v", err)
	}

[Copilot]

os.Remove(agentFileAbs) returns an error, but it’s being ignored. With errcheck enabled (including in _test.go), this will fail linting. Please check the error and fail the benchmark setup if the cleanup didn’t succeed (or explicitly ignore with a justified //nolint:errcheck).

Suggested change

	os.Remove(agentFileAbs)
	if err := os.Remove(agentFileAbs); err != nil && !os.IsNotExist(err) {
	b.Fatalf("remove agent file %s: %v", agentFileAbs, err)
	}

[Copilot]

id.Generate() returns an error, but it’s being ignored via the blank identifier. The repo’s errcheck config flags blank-identifier error ignores even in tests/benchmarks, so this will fail linting. Please handle the error (e.g., b.Fatalf) before building the commit message.

Suggested change

	cpID, _ := id.Generate()
	cpID, err := id.Generate()
	if err != nil {
	b.Fatalf("generate checkpoint ID: %v", err)
	}