-
Notifications
You must be signed in to change notification settings - Fork 662
Add session metadata for model name if env: BROWSERBASE
#765
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?
Changes from all commits
a810c41
0b39408
8e1672e
2ffbf09
7923aff
591ea05
144e87f
3bb57b3
3fce452
3e9579c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@browserbasehq/stagehand": patch | ||
--- | ||
|
||
Add session create metadata when env is BROWSERBASE for model client + name |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/** | ||
* Just extends sessions.test.ts but with a custom client. | ||
*/ | ||
import { test, expect } from "@playwright/test"; | ||
import { Stagehand } from "@browserbasehq/stagehand"; | ||
import StagehandConfig from "@/evals/deterministic/stagehand.config"; | ||
import Browserbase from "@browserbasehq/sdk"; | ||
import { AISdkClient } from "@/examples/external_clients/aisdk"; | ||
import { google } from "@ai-sdk/google/dist"; | ||
|
||
test.describe("Browserbase Sessions with custom client metadata", () => { | ||
let browserbase: Browserbase; | ||
let sessionId: string; | ||
let bigStagehand: Stagehand; | ||
|
||
test.beforeAll(async () => { | ||
browserbase = new Browserbase({ | ||
apiKey: process.env.BROWSERBASE_API_KEY, | ||
}); | ||
bigStagehand = new Stagehand({ | ||
...StagehandConfig, | ||
env: "BROWSERBASE", | ||
modelName: undefined, | ||
llmClient: new AISdkClient({ | ||
model: google("gemini-2.0-flash"), | ||
}), | ||
}); | ||
await bigStagehand.init(); | ||
await bigStagehand.page.goto( | ||
"https://docs.stagehand.dev/get_started/introduction", | ||
); | ||
sessionId = bigStagehand.browserbaseSessionID; | ||
if (!sessionId) { | ||
throw new Error("Failed to get browserbase session ID"); | ||
} | ||
}); | ||
test.afterAll(async () => { | ||
await bigStagehand.close(); | ||
}); | ||
test("creates the right session metadata", async () => { | ||
const session = await browserbase.sessions.retrieve(sessionId); | ||
expect(session.userMetadata.stagehand).toBe("true"); | ||
expect(session.userMetadata.modelName).toBe("NO_MODEL_DEFINED"); | ||
expect(session.userMetadata.usingCustomClient).toBe("true"); | ||
Comment on lines
+42
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Values in userMetadata are stored as strings ('true', 'NO_MODEL_DEFINED'). Consider using a type-safe enum or constants for these values to prevent typos and ensure consistency. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. k |
||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,6 +74,8 @@ async function getBrowser( | |
env: "LOCAL" | "BROWSERBASE" = "LOCAL", | ||
headless: boolean = false, | ||
logger: (message: LogLine) => void, | ||
modelName: string, | ||
usingCustomClient: boolean, | ||
browserbaseSessionCreateParams?: Browserbase.Sessions.SessionCreateParams, | ||
browserbaseSessionID?: string, | ||
localBrowserLaunchOptions?: LocalBrowserLaunchOptions, | ||
|
@@ -165,6 +167,8 @@ async function getBrowser( | |
userMetadata: { | ||
...(browserbaseSessionCreateParams?.userMetadata || {}), | ||
stagehand: "true", | ||
modelName: modelName?.replace("/", "_") || "unknown", | ||
usingCustomClient: usingCustomClient.toString(), | ||
}, | ||
}); | ||
|
||
|
@@ -403,6 +407,7 @@ export class Stagehand { | |
private _browser: Browser | undefined; | ||
private _isClosed: boolean = false; | ||
private _history: Array<HistoryEntry> = []; | ||
private _usingCustomClient: boolean; | ||
public get history(): ReadonlyArray<HistoryEntry> { | ||
return Object.freeze([...this._history]); | ||
} | ||
|
@@ -557,6 +562,7 @@ export class Stagehand { | |
// Update logger verbosity level | ||
this.stagehandLogger.setVerbosity(this.verbose); | ||
this.modelName = modelName ?? DEFAULT_MODEL_NAME; | ||
this._usingCustomClient = !modelName; | ||
|
||
let modelApiKey: string | undefined; | ||
|
||
|
@@ -640,7 +646,6 @@ export class Stagehand { | |
} | ||
this.logInferenceToFile = logInferenceToFile; | ||
this.selfHeal = selfHeal; | ||
this.disablePino = disablePino; | ||
} | ||
|
||
private registerSignalHandlers() { | ||
|
@@ -739,6 +744,8 @@ export class Stagehand { | |
this.env, | ||
this.headless, | ||
this.logger, | ||
this._usingCustomClient ? "NO_MODEL_DEFINED" : this.modelName, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider using a constant for 'NO_MODEL_DEFINED' to avoid magic strings |
||
this._usingCustomClient, | ||
this.browserbaseSessionCreateParams, | ||
this.browserbaseSessionID, | ||
this.localBrowserLaunchOptions, | ||
|
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: Setting modelName to undefined while providing an llmClient with a specific model (gemini-2.0-flash) seems inconsistent. Consider setting modelName to match the provided model or document why this is intentionally different.
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.
this is intentional to mock when users don't provide a modelName