Skip to content

[Evals] Disable screenshot collector interval by default #1421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
tkattkat merged 2 commits into from
Dec 18, 2025
Merged

[Evals] Disable screenshot collector interval by default #1421

tkattkat merged 2 commits into from
Dec 18, 2025

Conversation

@miguelg719
Copy link
Collaborator

@miguelg719 miguelg719 commented Dec 15, 2025 •
edited
Loading

why

Screenshot collector via interval was deprecated and replaced with improved agent loop event subscriptions.

what changed

Refactored ScreenshotCollector to use V3 instance instead of Page, enabling event-driven screenshot collection and improving memory management.

Key changes:

  • Constructor now takes V3 instead of Page for access to context and event bus
  • Interval-based capture is now opt-in - only activates if interval option is provided
  • Default mode is event-driven via addScreenshot() method (for use with V3 event bus)
  • Added stopped flag to prevent race conditions during shutdown
  • stop() now clears internal buffers after returning screenshots (memory cleanup)
  • stop() attempts final screenshot capture with graceful error handling (won't fail if CDP disconnected)
  • Removed deprecated captureOnNavigation option
  • Updated all agent eval tasks to pass v3 instead of page

test plan

Summary by cubic

Switches screenshot capture to event-driven mode via the V3 event bus and disables interval polling by default. Reduces overhead and aligns with Linear STG-1072; interval capture is now opt-in via options.interval.

  • Refactors

    • ScreenshotCollector now takes V3 (not Page).
    • Interval is optional; start() is a no-op unless interval is set.
    • Removed captureOnNavigation; use addScreenshot() for event-driven capture.
    • stop() attempts a final capture, then returns screenshots and clears state.
    • onlineMind2Web and webvoyager now use v3.bus "agent_screensot_taken_event" and no longer call start() in event-driven mode; adjusted maxScreenshots.

  • Migration

    • Instantiate: new ScreenshotCollector(v3, { maxScreenshots?, interval? }).
    • Event-driven: v3.bus.on("agent_screensot_taken_event", (b) => collector.addScreenshot(b)); call v3.bus.off(...) on cleanup; then collector.stop().
    • Polling (optional): provide interval and call start().
    • Remove any captureOnNavigation usage.

Written for commit bc70a36. Summary will update automatically on new commits.

@changeset-bot
Copy link

changeset-bot bot commented Dec 15, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: bc70a36

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@miguelg719 miguelg719 changed the title Disable screenshot collector interval by default [Evals] Disable screenshot collector interval by default Dec 15, 2025
@miguelg719 miguelg719 force-pushed the miguelgonzalez/stg-1072-default-disable-screenshot-collector-via-interval branch from bb1abb7 to bc70a36 Compare December 18, 2025 00:27
@miguelg719 miguelg719 marked this pull request as ready for review December 18, 2025 00:32
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 18, 2025

Greptile Summary

Refactored ScreenshotCollector to use V3 instead of Page, enabling event-driven screenshot collection via the V3 event bus and making interval-based polling opt-in. This improves memory management and prevents race conditions during shutdown.

  • Core changes: ScreenshotCollector now accepts V3 in constructor, added stopped flag for race condition prevention, stop() clears internal buffers after returning
  • Breaking change: Removed captureOnNavigation option (deprecated in V3)
  • Eval tasks: Updated 18 agent eval tasks to pass v3 instead of page; onlineMind2Web and webvoyager switched to event-driven mode via agent_screensot_taken_event
  • Minor cleanup: onlineMind2Web removed unused fs import and manual screenshot write

Confidence Score: 5/5

  • This PR is safe to merge - well-structured refactor with consistent changes across all eval tasks.
  • The changes are straightforward API updates with improved memory management and race condition handling. All eval tasks were consistently updated, and the event-driven pattern aligns with existing V3 infrastructure.
  • No files require special attention. Minor cleanup: ScreenshotCapablePage type in types file is now unused.

Important Files Changed

Filename Overview
packages/evals/utils/ScreenshotCollector.ts Core refactor: Changed from Page to V3, added stopped flag for race condition prevention, interval capture is now opt-in, improved memory cleanup in stop().
packages/evals/types/screenshotCollector.ts Removed deprecated captureOnNavigation option, added JSDoc for interval, contains unused ScreenshotCapablePage type.
packages/evals/tasks/agent/onlineMind2Web.ts Switched to event-driven screenshot collection via V3 event bus, removed fs import and manual screenshot write.
packages/evals/tasks/agent/webvoyager.ts Switched to event-driven screenshot collection via V3 event bus with proper event listener cleanup.

Sequence Diagram

sequenceDiagram
    participant EvalTask as Eval Task
    participant SC as ScreenshotCollector
    participant V3 as V3 Instance
    participant Bus as V3 Event Bus
    participant Page as Active Page

    Note over EvalTask,Page: Event-Driven Mode (onlineMind2Web, webvoyager)
    EvalTask->>SC: new ScreenshotCollector(v3, {maxScreenshots})
    EvalTask->>Bus: bus.on("agent_screensot_taken_event", handler)
    EvalTask->>V3: agent.execute()
    V3->>Page: screenshot()
    V3->>Bus: emit("agent_screensot_taken_event", buffer)
    Bus->>SC: addScreenshot(buffer)
    SC->>SC: MSE/SSIM comparison
    SC->>SC: Store if different enough
    EvalTask->>Bus: bus.off("agent_screensot_taken_event", handler)
    EvalTask->>SC: stop()
    SC-->>EvalTask: Buffer[] (clears internal state)

    Note over EvalTask,Page: Interval Mode (other eval tasks)
    EvalTask->>SC: new ScreenshotCollector(v3, {interval, maxScreenshots})
    EvalTask->>SC: start()
    loop Every interval ms
        SC->>V3: context.awaitActivePage()
        V3-->>SC: page
        SC->>Page: screenshot()
        Page-->>SC: buffer
        SC->>SC: MSE/SSIM comparison
        SC->>SC: Store if different enough
    end
    EvalTask->>SC: stop()
    SC->>SC: Capture final screenshot
    SC-->>EvalTask: Buffer[] (clears internal state)
Loading

greptile-apps[bot]
greptile-apps bot reviewed Dec 18, 2025
View reviewed changes
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. packages/evals/types/screenshotCollector.ts, line 12-16 (link)

    style: ScreenshotCapablePage type is now unused since the refactor to use V3 directly. Consider removing this dead code.

22 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

cubic-dev-ai[bot]
cubic-dev-ai bot reviewed Dec 18, 2025
View reviewed changes
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 22 files

@tkattkat tkattkat merged commit b48c9c6 into main Dec 18, 2025
32 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cubic-dev-ai cubic-dev-ai[bot] cubic-dev-ai[bot] left review comments

@tkattkat tkattkat tkattkat approved these changes

+1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@miguelg719 @tkattkat