Update screenshot collector #1373
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: c0131eb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
Greptile OverviewGreptile SummaryUpdates the
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Caller
participant ScreenshotCollector
participant Page
participant Sharp
Caller->>ScreenshotCollector: start()
ScreenshotCollector->>Page: screenshot() [initial]
Page-->>ScreenshotCollector: Buffer
ScreenshotCollector->>ScreenshotCollector: store screenshot
loop Every interval (5000ms default)
ScreenshotCollector->>Page: screenshot()
Page-->>ScreenshotCollector: Buffer
ScreenshotCollector->>Sharp: resize & compare (MSE)
Sharp-->>ScreenshotCollector: MSE value
alt MSE >= threshold
ScreenshotCollector->>Sharp: calculate SSIM
Sharp-->>ScreenshotCollector: SSIM value
alt SSIM < threshold (different enough)
ScreenshotCollector->>ScreenshotCollector: store screenshot
end
end
end
Caller->>ScreenshotCollector: stop()
ScreenshotCollector->>Page: screenshot() [final, async]
ScreenshotCollector-->>Caller: Buffer[]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+81
to
85
| // Capture final screenshot without blocking | ||
| this.captureScreenshot("final").catch((error) => { | ||
| console.error("Failed to capture final screenshot:", error); | ||
| }); | ||
| return this.getScreenshots(); |
Contributor
There was a problem hiding this comment.
logic: The final screenshot capture is fire-and-forget, so getScreenshots() returns before the final screenshot is captured. If the caller needs the final screenshot included, this won't work as expected. Consider making stop() async and awaiting the capture, or document that the final screenshot is best-effort.
Suggested change
| // Capture final screenshot without blocking | |
| this.captureScreenshot("final").catch((error) => { | |
| console.error("Failed to capture final screenshot:", error); | |
| }); | |
| return this.getScreenshots(); | |
| // Capture final screenshot without blocking | |
| this.captureScreenshot("final").catch((error) => { | |
| console.error("Failed to capture final screenshot:", error); | |
| }); | |
| // Note: final screenshot may not be included as it's captured asynchronously | |
| return this.getScreenshots(); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/evals/utils/ScreenshotCollector.ts
Line: 81:85
Comment:
**logic:** The final screenshot capture is fire-and-forget, so `getScreenshots()` returns before the final screenshot is captured. If the caller needs the final screenshot included, this won't work as expected. Consider making `stop()` async and awaiting the capture, or document that the final screenshot is best-effort.
```suggestion
// Capture final screenshot without blocking
this.captureScreenshot("final").catch((error) => {
console.error("Failed to capture final screenshot:", error);
});
// Note: final screenshot may not be included as it's captured asynchronously
return this.getScreenshots();
```
How can I resolve this? If you propose a fix, please make it concise.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
why
what changed
test plan
Summary by cubic
Updated the evals CLI screenshot collector to the latest version, adding image-diff filtering and compatibility with v3 Stagehand pages where navigation events are disabled by default. This reduces duplicate screenshots and stabilizes capture during eval runs.
New Features
Dependencies
Written for commit c0131eb. Summary will update automatically on new commits.