Skip to content

implement screenshot plugin#5691

Merged
adhami3310 merged 3 commits into
mainfrom
implement-screenshot-plugin
Aug 7, 2025
Merged

implement screenshot plugin#5691
adhami3310 merged 3 commits into
mainfrom
implement-screenshot-plugin

Conversation

@adhami3310

Copy link
Copy Markdown
Member

No description provided.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR implements a screenshot plugin system for Reflex applications. The changes introduce a new plugin lifecycle hook (post_compile) that allows plugins to perform setup tasks after app compilation is complete, and adds a ScreenshotPlugin that provides two API endpoints for screenshot functionality.

The core architectural changes include:

  1. Plugin Lifecycle Extension: A new PostCompileContext type and post_compile method are added to the base Plugin class, enabling plugins to access the fully compiled App instance for post-compilation setup.

  2. App Integration: The app initialization process now calls post_compile hooks on all registered plugins after compilation completes, providing them with the app context.

  3. Screenshot Plugin: A new _ScreenshotPlugin is implemented with two endpoints:

    • /_active_connections: Exposes active WebSocket connections via the internal token_to_sid mapping
    • /_clone_state: Allows deep copying of application states, including substates and their relationships
  4. State Manager Changes: The StateManagerMemory class is modified to change state persistence behavior - states are now stored using just the client token portion, and automatic state persistence after modification is removed.

The plugin follows Reflex's modular architecture, allowing screenshot functionality to be optionally enabled. The implementation includes custom state deep-copying logic to handle substate relationships and the _was_touched flag management needed for consistent screenshot capture.

Confidence score: 2/5

  • This PR introduces significant security and architectural concerns that make it risky to merge
  • Score reflects the exposure of internal connection data without authentication and complex state manipulation logic that could cause runtime failures
  • Pay close attention to reflex/plugins/_screenshot.py and reflex/istate/manager.py for potential security vulnerabilities and error handling issues

5 files reviewed, 5 comments

Edit Code Review Bot Settings | Greptile

Comment thread reflex/istate/manager.py
Comment thread reflex/plugins/_screenshot.py Outdated
Comment thread reflex/plugins/_screenshot.py
Comment thread reflex/app.py
Comment thread reflex/plugins/__init__.py
@codspeed-hq

codspeed-hq Bot commented Aug 7, 2025

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #5691 will not alter performance

Comparing implement-screenshot-plugin (97a9f83) with main (c106be7)

Summary

✅ 8 untouched benchmarks

@masenf masenf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the road, so didn't run it, but it looks good 😊

@adhami3310 adhami3310 merged commit 1b3ba8f into main Aug 7, 2025
41 checks passed
@adhami3310 adhami3310 deleted the implement-screenshot-plugin branch August 7, 2025 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants