Skip to content

feat(gateway): auto-delete tool progress messages after response - #4767

Closed
byjk wants to merge 3 commits into
NousResearch:mainfrom
byjk:feature/delete-progress-messages
Closed

feat(gateway): auto-delete tool progress messages after response#4767
byjk wants to merge 3 commits into
NousResearch:mainfrom
byjk:feature/delete-progress-messages

Conversation

@byjk

@byjk byjk commented Apr 3, 2026

Copy link
Copy Markdown

What

Adds display.delete_progress_messages config option (default: false). When enabled, progress messages showing tool calls are automatically deleted from the chat after the final response is sent.

Only the final progress message (the one that was last edited) is deleted, not intermediate progress messages for each tool call.

Changes

  • base.py: Add optional delete_message() method to BasePlatformAdapter (returns False by default)
  • telegram.py: Implement delete_message() with logger.warning on failure (not silent pass, per review feedback)
  • run.py: Track progress message ID via mutable holder, delete after agent completes when config is enabled
  • config.py: Add delete_progress_messages display option (default: false)

Config

display:
  delete_progress_messages: true  # default: false

Review Notes

Addressed feedback from initial review:

  • ✅ Use logger.warning() instead of silent except: pass for debugging
  • ✅ Consistent variable naming (no double underscores)
  • ✅ Docstring clarifies that only the final progress message is deleted

@britrik

britrik commented Apr 3, 2026

Copy link
Copy Markdown

Code Review: Auto-delete Progress Messages Feature

Thanks for this useful feature! The implementation looks solid overall. Here are a few suggestions for improvement:

Suggested Improvements

  1. Variable naming: The code uses double underscore prefix for local variables (, , etc.). In Python, double underscore triggers name mangling in class contexts. Consider using single underscore instead: → or with single underscore.

  2. Error handling: Silent exception swallowing with (lines in run.py) makes debugging difficult. Consider adding debug logging:

  3. Type validation: In telegram.py, the code does and without validation. Consider adding a check first to provide clearer error messages.

  4. Documentation: Consider adding a docstring explaining that only the final progress message is deleted (not all progress messages for each tool call).

What's Good

  • Feature is opt-in (defaults to false) ✓
  • Graceful fallback if platform doesn't support deletion ✓
  • Clean separation of concerns (base class + platform implementations) ✓
  • Config-based feature toggle ✓

Minor Note

The approach works but could be simplified - consider whether you need the mutable list pattern here.

Overall looks good to merge! 👀🚀

@britrik

britrik commented Apr 3, 2026

Copy link
Copy Markdown

Code Review: Auto-delete Progress Messages Feature

Thanks for this useful feature! The implementation looks solid overall. Here are a few suggestions for improvement:

Suggested Improvements

  1. Variable naming: The code uses double underscore prefix for local variables (_delete_progress, _cfg, etc.). In Python, double underscore triggers name mangling in class contexts. Consider using single underscore instead.

  2. Error handling: Silent exception swallowing with 'except Exception: pass' makes debugging difficult. Consider adding debug logging instead.

  3. Type validation: In telegram.py, the code does int(chat_id) and int(message_id) without validation. Consider adding a check first to provide clearer error messages.

  4. Documentation: Consider adding a docstring explaining that only the final progress message is deleted (not all progress messages for each tool call).

What's Good

  • Feature is opt-in (defaults to false)
  • Graceful fallback if platform does not support deletion
  • Clean separation of concerns (base class + platform implementations)
  • Config-based feature toggle

Minor Note

The progress_msg_id_holder approach works but could be simplified - consider whether you need the mutable list pattern here.

Overall looks good to merge!

@britrik

britrik commented Apr 3, 2026

Copy link
Copy Markdown

Code Review: Auto-delete tool progress messages

Thanks for this feature! It adds useful cleanup functionality. Here are some observations:

✅ Good

  • Clean separation: base class has optional method, Telegram implements it
  • Config is opt-in ( by default)
  • Progress message ID properly propagated through the agent result

⚠️ Suggestions for Improvement

  1. Error handling is too silent - The pattern makes debugging difficult. Consider logging at least at WARNING level:

    except Exception as e:
        logger.warning("Failed to delete progress message: %s", e)
  2. Config loaded on every message - is called for every incoming message. Consider caching this in the class or instance.

  3. Add platform check fallback - Instead of just checking , consider adding a property to adapters for explicit opt-in:

    if _del_adapter and getattr(_del_adapter, 'supports_delete_message', False):
  4. Minor: Inconsistent variable naming - , , use different conventions. Consider using consistent naming throughout.

🐛 Potential Issues

  • Race condition: Progress message is deleted immediately after agent finishes, potentially before the final response is fully rendered/acknowledged. Consider adding a small delay or confirming response delivery first.

  • No null guard: If is None or not in adapters, returns None correctly, but the error message could be clearer.

Overall this looks solid for merging. The suggestions above are refinements rather than blockers. Great work!

Adds display.delete_progress_messages config option (default: false).
When enabled, the final progress message (showing tool call status) is
automatically deleted from the chat after the agent's final response is
delivered.

Implementation:
- base.py: Add optional delete_message() method to BasePlatformAdapter
- telegram.py: Implement delete_message() with logger.warning on failure
  instead of silent pass (per code review feedback)
- run.py: Track progress message ID via mutable holder, delete after
  agent completes when config is enabled
- config.py: Add delete_progress_messages display option

Only the final progress message (the one that was last edited) is deleted,
not intermediate per-tool progress messages.
@byjk
byjk force-pushed the feature/delete-progress-messages branch from cd1cb33 to 0e0e0a2 Compare April 5, 2026 11:44
@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter area/config Config system, migrations, profiles labels Apr 21, 2026
@byjk byjk closed this May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants