Open
Conversation
There was a problem hiding this comment.
cubic analysis
1 issue found across 3 files
Confidence score: 2/5
- There is a clear user-facing regression risk: in
stagehand/main.pythe async-first code path now returns emptyStagehandMetrics()instead of fetching from the API, which can silently drop metrics for most users. - Given the 7/10 severity and high confidence, this is likely to cause real data loss and should be addressed before merging.
- Pay close attention to
stagehand/main.py- async access to.metricsreturns empty metrics instead of fetching from the API.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="stagehand/main.py">
<violation number="1" location="stagehand/main.py:784">
P1: Silent data loss: accessing `.metrics` in an async context (the primary usage pattern) now returns empty `StagehandMetrics()` instead of fetching from the API. Since Stagehand is async-first, most users will hit this code path and silently get no data.
At minimum, log a warning so users know metrics aren't being fetched. Ideally, provide an async alternative like `await stagehand.get_metrics()` so users in async contexts can still retrieve API metrics.</violation>
</file>
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Remove nest-asyncio from pyproject.toml dependencies | Removed nest-asyncio entry from pyproject.toml |
| ✅ | Remove import of nest_asyncio from code | Deleted 'import nest_asyncio' from stagehand/main.py |
| ✅ | Eliminate nest_asyncio.apply() usage to avoid event loop patching | Removed nest_asyncio.apply() call and related handling |
| ✅ | When already in an async context, return empty StagehandMetrics instead of patching loop | Returns StagehandMetrics() when asyncio.get_running_loop() succeeds |
| ✅ | Use asyncio.run(get_replay_metrics()) only when no running loop | Calls asyncio.run(get_replay_metrics()) in RuntimeError except branch |
| ✅ | Add changeset noting removal of nest-asyncio | Added .changeset file referencing removal |
| Unblocks Python 3.14 upgrade (no asyncio-level patching dependency) | Dependency removed but no CI/tests validating Python 3.14 compatibility |
Architecture diagram
sequenceDiagram
participant Client
participant SH as Stagehand (main.py)
participant Loop as asyncio
participant RM as get_replay_metrics()
Note over SH: Attribute Access Interception (__getattribute__)
Client->>SH: Access metrics property
SH->>Loop: asyncio.get_running_loop()
alt Loop is already running (Async context)
Loop-->>SH: Running loop detected
Note over SH: CHANGED: No longer patches loop with nest-asyncio
SH-->>Client: NEW: Return empty StagehandMetrics()
else No loop running (Sync context)
Loop-->>SH: Raise RuntimeError
SH->>RM: asyncio.run(get_replay_metrics())
RM-->>SH: Replay data
SH-->>Client: Return metrics
end
opt Exception during metrics retrieval
SH-->>Client: Return empty StagehandMetrics()
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Linked issue analysis
Linked issue: STG-1374: [Ramp] Ramp,
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Remove nest-asyncio from pyproject.toml | Removed nest-asyncio entry from pyproject.toml |
| ✅ | Remove nest_asyncio import and apply calls in code | nest_asyncio import and apply calls deleted in main.py |
| ✅ | Add a synchronous replay metrics fetch path to avoid event loop patching | Added _get_replay_metrics_sync implementation in api.py |
| ✅ | Use sync fetch when an event loop is running | main.py detects running loop and calls sync fetch |
| ✅ | Preserve async fetch when no loop is running (use asyncio.run) | Retains asyncio.run path for async fetch when no loop |
| ✅ | Share parsing logic between async and sync replay metrics paths | Introduced _parse_replay_metrics_data shared parser |
| ✅ | Proper error handling and logging in new functions | Try/except and self.logger.error present in both fetchers |
| ✅ | Expose Stagehand._get_replay_metrics_sync attribute | Assigned Stagehand._get_replay_metrics_sync at module end |
| ✅ | Update module exports (__all__) to include _get_replay_metrics_sync | __all__ updated to include _get_replay_metrics_sync |
| ✅ | Ensure no other references to nest_asyncio remain | Removed import and dependency; no other references in diffs |
| Unblock Python 3.14 upgrade (remove nest-asyncio as blocker) | Removal reduces blocker but full compatibility not verified |
Collaborator
Author
|
@miguelg719 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
cubic analysis
No issues found across 4 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Linked issue analysis
Linked issue: STG-1374: [Ramp] Ramp,
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Remove nest-asyncio from pyproject.toml | Removed nest-asyncio entry from pyproject.toml |
| ✅ | Remove nest_asyncio import and apply calls in code | nest_asyncio import and apply calls deleted in main.py |
| ✅ | Add a synchronous replay metrics fetch path to avoid event loop patching | Added _get_replay_metrics_sync implementation in api.py |
| ✅ | Use sync fetch when an event loop is running | main.py detects running loop and calls sync fetch |
| ✅ | Preserve async fetch when no loop is running (use asyncio.run) | Retains asyncio.run path for async fetch when no loop |
| ✅ | Share parsing logic between async and sync replay metrics paths | Introduced _parse_replay_metrics_data shared parser |
| ✅ | Proper error handling and logging in new functions | Try/except and self.logger.error present in both fetchers |
| ✅ | Expose Stagehand._get_replay_metrics_sync attribute | Assigned Stagehand._get_replay_metrics_sync at module end |
| ✅ | Update module exports (__all__) to include _get_replay_metrics_sync | __all__ updated to include _get_replay_metrics_sync |
| ✅ | Ensure no other references to nest_asyncio remain | Removed import and dependency; no other references in diffs |
| Unblock Python 3.14 upgrade (remove nest-asyncio as blocker) | Removal reduces blocker but full compatibility not verified |
Architecture diagram
sequenceDiagram
participant User as Caller
participant SH as Stagehand (main.py)
participant Loop as asyncio Loop
participant API as API Layer (api.py)
participant BB as Browserbase API
User->>SH: Access .metrics property
SH->>SH: __getattribute__("metrics")
SH->>Loop: asyncio.get_running_loop()
alt Loop is already running (e.g. Notebook, Async App)
Loop-->>SH: loop instance
SH->>API: NEW: _get_replay_metrics_sync()
Note over API,BB: Synchronous HTTP request
API->>BB: GET /sessions/{id}/replay (Sync)
BB-->>API: JSON Response
else No loop running
Loop-->>SH: RuntimeError
SH->>Loop: asyncio.run(_get_replay_metrics())
Note over API,BB: Asynchronous HTTP request
API->>BB: GET /sessions/{id}/replay (Async)
BB-->>API: JSON Response
end
API->>API: CHANGED: _parse_replay_metrics_data(json)
Note right of API: Shared parsing logic for<br/>sync and async paths
API-->>SH: StagehandMetrics object
SH-->>User: Return metrics
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
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.
Summary by cubic
Removes the nest-asyncio dependency and switches replay metrics fetching to a sync HTTP path when an event loop is running, avoiding loop patching. This unblocks the Python 3.14 upgrade for STG-1374.
Dependencies
Refactors
Written for commit d9174e0. Summary will update on new commits. Review in cubic