-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix (v3): global verbosity check in v3Logger console fallback #1542
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
Greptile SummaryThis PR attempts to fix debug logs appearing in console fallback when
The AgentProvider.ts import fix ( Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Stagehand
participant v3Logger
participant AsyncLocalStorage
participant StagehandLogger
participant Console
User->>Stagehand: new Stagehand({verbose: 0, disablePino: true})
Stagehand->>StagehandLogger: create with verbose=0
Stagehand->>AsyncLocalStorage: bindInstanceLogger(instanceId, logger)
Note over Stagehand,AsyncLocalStorage: Normal flow - context preserved
User->>Stagehand: perform operation
Stagehand->>v3Logger: v3Logger({level: 2, message: "debug"})
v3Logger->>AsyncLocalStorage: getStore()
AsyncLocalStorage-->>v3Logger: instanceId
v3Logger->>StagehandLogger: log(line)
StagehandLogger->>StagehandLogger: check: level (2) > verbose (0)?
Note over StagehandLogger: YES - suppressed ✓
Note over Stagehand,Console: Context lost scenario (setTimeout, etc)
User->>Stagehand: perform async operation
Stagehand->>v3Logger: v3Logger({level: 2, message: "debug"})
v3Logger->>AsyncLocalStorage: getStore()
AsyncLocalStorage-->>v3Logger: undefined (context lost)
v3Logger->>v3Logger: check: level (2) > FALLBACK_VERBOSITY (1)?
Note over v3Logger: YES - suppressed
Note over v3Logger,Console: Issue: Uses hardcoded value,<br/>not user's verbose setting
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, 2 comments
| if (lvl > FALLBACK_VERBOSITY) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: hardcoded FALLBACK_VERBOSITY = 1 doesn't respect user's configured verbosity level
when user sets verbose: 2 (show debug), fallback will still suppress debug logs. when user sets verbose: 0 (errors only), fallback will show info logs.
this will also break the existing test at logger-initialization.spec.ts:311 which expects all 3 log levels (error, info, debug) to be logged to console fallback.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/lib/v3/logger.ts
Line: 81:83
Comment:
**logic:** hardcoded `FALLBACK_VERBOSITY = 1` doesn't respect user's configured verbosity level
when user sets `verbose: 2` (show debug), fallback will still suppress debug logs. when user sets `verbose: 0` (errors only), fallback will show info logs.
this will also break the existing test at `logger-initialization.spec.ts:311` which expects all 3 log levels (error, info, debug) to be logged to console fallback.
How can I resolve this? If you propose a fix, please make it concise.| // Per-instance routing using AsyncLocalStorage | ||
| const logContext = new AsyncLocalStorage<string>(); | ||
| const instanceLoggers = new Map<string, (line: LogLine) => void>(); | ||
| const FALLBACK_VERBOSITY = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: consider storing per-instance verbosity settings in a Map (similar to instanceLoggers) instead of hardcoding fallback verbosity. this would allow the fallback path to respect each instance's configured verbosity level.
const instanceVerbosity = new Map<string, 0 | 1 | 2>();
export function bindInstanceLogger(
instanceId: string,
logger: (line: LogLine) => void,
verbosity: 0 | 1 | 2 = 1,
): void {
instanceLoggers.set(instanceId, logger);
instanceVerbosity.set(instanceId, verbosity);
}Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/lib/v3/logger.ts
Line: 34:34
Comment:
**style:** consider storing per-instance verbosity settings in a Map (similar to `instanceLoggers`) instead of hardcoding fallback verbosity. this would allow the fallback path to respect each instance's configured verbosity level.
```typescript
const instanceVerbosity = new Map<string, 0 | 1 | 2>();
export function bindInstanceLogger(
instanceId: string,
logger: (line: LogLine) => void,
verbosity: 0 | 1 | 2 = 1,
): void {
instanceLoggers.set(instanceId, logger);
instanceVerbosity.set(instanceId, verbosity);
}
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this 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 2 files
9eb36f5 to
36b82cc
Compare
Problem
When using Stagehand V3 with
verbose: 0, debug-level logs are still printed ifv3Loggerfalls back to console logging.The fallback path bypasses verbosity filtering, resulting in unexpected debug output.Solution
Applied a default verbosity threshold check to the fallback logging path to ensure debug messages are suppressed unless explicitly allowed.
Changes
v3LoggerFixes #1490
Summary by cubic
Stops debug logs from printing when verbose: 0 by adding a fallback verbosity threshold (1) to the v3Logger console fallback. Fixes #1490.
Written for commit 36b82cc. Summary will update on new commits.