Fix/compact disabled during approval

[thepetk]

Summary

The PR addresses an issue with the Compact now button which was clickable while the agent had pending approvals, however this wasn't possible on the backend. I think is because of this and I believe that's the right behavior. In my PR I'm just disabling the button when isLoading is True.

Testing

Compact Now Button

1. Build and launch the desktop app (just run-ui).
2. Submit a prompt that causes a long streaming response.
3. While the streaming indicator is active, check the "Compact now" button in the context bar.
   • On main: button is enabled and fires.
   • On this branch: button is greyed out/disabled; re-enables only when streaming finishes.
4. Added a unit test in AlertBox.test.tsx covering the disabled state while loading.

Related Issues

Relates to #9958
Discussion: LINK (if any)

Screenshots/Demos (for UX changes)

Before: "Compact now" button remains active (clickable) while agent is streaming.

After: "Compact now" button is disabled (greyed out, not-allowed cursor) while agent is streaming; re-enables once the stream finishes.

Assisted-by: Claude Sonnet 4.6

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7505c55feb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58fcadfd38

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e97589a687

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5d8b6b4dbe

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 36daaf9797

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[thepetk]

I've added more tests to capture the fix, but also the further changes requested by copilot.

Now I've also added Some(stored) => (stored as usize).max(estimated_tokens). This was done as the token usage stored in the session is recorded right after the LLM call, so before tool responses are appended.

This means if we have large tool output this can go undetected by the threshold check. An approach here would be to take the max of the stored value and a local estimate of the current conversation, so compaction triggers correctly even when the stored count is stale.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 23e1c62bf4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[DOsinga]

Thanks for this @thepetk!

The one-line UI fix here — disabling the Compact now button while a request is in flight (disabled={isLoading}) — is exactly the fix for what the PR describes, and I'm happy to take that on its own.

The backend changes, though, go well beyond "compact button disabled during approval" and I'd like to drop them from this PR:

• check_if_compaction_needed now always tokenizes and uses max(stored, estimated). This changes token-accounting behavior for every session, not just approval-heavy ones. session.usage.total_tokens is the provider's authoritative count from the last call; estimated_tokens is a local heuristic that ignores tool schemas / prompt framing / provider overhead. Taking the max lets the estimate override ground truth and biases us toward compacting early on every check. The fact that this needs the comment // The stored value is recorded before tool responses are added... use whichever count is higher to make sense is a sign the code is expressing "pick the bigger number" when the real intent is "the snapshot is stale after tool output" — those aren't the same thing.
• The in-loop auto-compact in reply_internal and the metadata-preservation change in compact_messages are also separate behavioral changes to the compaction engine.

These are all reasonable things to discuss, but they're distinct features/semantics changes that deserve their own issue and review rather than riding in under a UI-button bugfix. Could you strip this PR down to just the button disablement? If you'd like to pursue the mid-loop compaction / token-recount behavior, let's open an issue to discuss the approach first.

[thepetk]

Thanks for this @thepetk!

The one-line UI fix here — disabling the Compact now button while a request is in flight (disabled={isLoading}) — is exactly the fix for what the PR describes, and I'm happy to take that on its own.

The backend changes, though, go well beyond "compact button disabled during approval" and I'd like to drop them from this PR:

• check_if_compaction_needed now always tokenizes and uses max(stored, estimated). This changes token-accounting behavior for every session, not just approval-heavy ones. session.usage.total_tokens is the provider's authoritative count from the last call; estimated_tokens is a local heuristic that ignores tool schemas / prompt framing / provider overhead. Taking the max lets the estimate override ground truth and biases us toward compacting early on every check. The fact that this needs the comment // The stored value is recorded before tool responses are added... use whichever count is higher to make sense is a sign the code is expressing "pick the bigger number" when the real intent is "the snapshot is stale after tool output" — those aren't the same thing.
• The in-loop auto-compact in reply_internal and the metadata-preservation change in compact_messages are also separate behavioral changes to the compaction engine.

These are all reasonable things to discuss, but they're distinct features/semantics changes that deserve their own issue and review rather than riding in under a UI-button bugfix. Could you strip this PR down to just the button disablement? If you'd like to pursue the mid-loop compaction / token-recount behavior, let's open an issue to discuss the approach first.

Yeah @DOsinga I think you have a fair point.

Just tried to handle this as an "easy fix", mostly for my own curiosity on how we could handle this on the backend, but I do agree it's not. That said, I'll remove this part from the PR and proceed only with the UI fix.

[thepetk]

Thanks for this @thepetk!
The one-line UI fix here — disabling the Compact now button while a request is in flight (disabled={isLoading}) — is exactly the fix for what the PR describes, and I'm happy to take that on its own.
The backend changes, though, go well beyond "compact button disabled during approval" and I'd like to drop them from this PR:

• check_if_compaction_needed now always tokenizes and uses max(stored, estimated). This changes token-accounting behavior for every session, not just approval-heavy ones. session.usage.total_tokens is the provider's authoritative count from the last call; estimated_tokens is a local heuristic that ignores tool schemas / prompt framing / provider overhead. Taking the max lets the estimate override ground truth and biases us toward compacting early on every check. The fact that this needs the comment // The stored value is recorded before tool responses are added... use whichever count is higher to make sense is a sign the code is expressing "pick the bigger number" when the real intent is "the snapshot is stale after tool output" — those aren't the same thing.
• The in-loop auto-compact in reply_internal and the metadata-preservation change in compact_messages are also separate behavioral changes to the compaction engine.

These are all reasonable things to discuss, but they're distinct features/semantics changes that deserve their own issue and review rather than riding in under a UI-button bugfix. Could you strip this PR down to just the button disablement? If you'd like to pursue the mid-loop compaction / token-recount behavior, let's open an issue to discuss the approach first.

Yeah @DOsinga I think you have a fair point.

Just tried to handle this as an "easy fix", mostly for my own curiosity on how we could handle this on the backend, but I do agree it's not. That said, I'll remove this part from the PR and proceed only with the UI fix.

@DOsinga I've updated the PR

I would be happy to start a discussion over the auto-compaction. Could do first a more thorough investigation and then bring it up here or in discord. Do you think it would worth the effort?

[DOsinga]

Thanks — this is now focused on the UI regression, and I removed the redundant AlertBox tests that did not cover the changed wiring.

[thepetk]

Thanks — this is now focused on the UI regression, and I removed the redundant AlertBox tests that did not cover the changed wiring.

Thanks! Then I guess my "assisted-by" is not accurate anymore :D cause the only contrib left was those tests

@DOsinga let me know if I should do anything further to merge it