Skip to content

feat: synthetic config, screenshot streaming is not called when False…#128

Draft
davidamo9 wants to merge 2 commits intomozarkai:mainfrom
davidamo9:feature/toggle_continuous_streaming
Draft

feat: synthetic config, screenshot streaming is not called when False…#128
davidamo9 wants to merge 2 commits intomozarkai:mainfrom
davidamo9:feature/toggle_continuous_streaming

Conversation

@davidamo9
Copy link
Copy Markdown
Contributor

…, single StrategyManager for ActionKeyword and Verifier

Copilot AI review requested due to automatic review settings July 15, 2025 06:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a synthetic configuration flag to control whether screenshot streaming or single captures are used, centralizes StrategyManager instantiation in the builder, and updates API clients to use the shared instance.

  • Introduce synthetic flag and helper get_test_mode() to toggle streaming vs single-frame screenshot logic.
  • Update LocatorStrategy.assert_elements methods to branch on synthetic, using streaming when True and single-capture when False.
  • Add get_strategy_manager() to OpticsBuilder and update Verifier and ActionKeyword to use the builder‐provided StrategyManager.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
common/strategies.py Added get_test_mode, synthetic branch in assert_elements, and timestamp/annotation variables.
common/optics_builder.py Added _strategy_manager cache, get_strategy_manager() method.
common/config_handler.py Added synthetic: bool to the config model.
api/verifier.py Switched to builder.get_strategy_manager() instead of direct instantiation.
api/action_keyword.py Same change: use builder.get_strategy_manager().
Comments suppressed due to low confidence (4)

optics_framework/common/strategies.py:15

  • [nitpick] The function name get_test_mode may be misleading since the config flag is named synthetic. Consider renaming this to is_synthetic_mode or use_synthetic_mode for clarity and consistency.
def get_test_mode() -> bool:

optics_framework/common/optics_builder.py:64

  • The docstring mentions raising a ValueError if configurations are missing, but there is no validation in the method. Either implement the checks or update the docstring to match the actual behavior.
    def get_strategy_manager(self):

optics_framework/common/config_handler.py:52

  • The new synthetic field should be documented in the project's configuration reference or README so users know its effect and default value.
    synthetic: bool = False

optics_framework/common/strategies.py:15

  • There are no existing tests covering the new get_test_mode() helper or the synthetic vs non-synthetic branches in assert_elements. Add unit tests to ensure both flows behave as expected.
def get_test_mode() -> bool:

@davidamo9 davidamo9 force-pushed the feature/toggle_continuous_streaming branch from 3f397b6 to 529f448 Compare July 15, 2025 07:01
@malto101 malto101 marked this pull request as draft July 17, 2025 10:20
…, single StrategyManager for ActionKeyword and Verifier
@davidamo9 davidamo9 force-pushed the feature/toggle_continuous_streaming branch from 529f448 to 6c09ef5 Compare August 7, 2025 17:38
@sonarqubecloud
Copy link
Copy Markdown

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