Skip to content

test(extension): replace model-resolution it.todo placeholders - #2696

Open
HaoChiBao wants to merge 2 commits into
browserbase:mainfrom
HaoChiBao:test/model-resolution-coverage
Open

test(extension): replace model-resolution it.todo placeholders#2696
HaoChiBao wants to merge 2 commits into
browserbase:mainfrom
HaoChiBao:test/model-resolution-coverage

Conversation

@HaoChiBao

@HaoChiBao HaoChiBao commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace it.todo placeholders in packages/extension/tests/model-resolution.test.ts with controller-level coverage for model string and object resolution paths.
  • Align a misleading test name with current controller behavior (keyless models are forwarded; rejection happens later in llmService).

Test plan

  • pnpm --filter @browserbasehq/stagehand-extension exec vitest run tests/model-resolution.test.ts (or equivalent package filter) passes
  • Confirm no remaining it.todo in that file

Summary by cubic

Replaces it.todo tests in packages/extension/tests/model-resolution.test.ts with controller-level coverage for model resolution and Browserbase gateway routing, and dedupes primitive spy scaffolding via shared helpers. Increases confidence in inference selection without changing runtime behavior.

  • Direct inference: uses provider auth; preferred even when a Browserbase session exists (gateway still forwarded; selection happens in llmService).
  • Browserbase managed inference: keyless explicit model or no model routes via the gateway; rejects when no model and no gateway; rejects managed inference with a local browser; forwards a keyless explicit model with a local browser (later rejected by llmService); rejects a missing model with a local browser.
  • Client inference: model.source: "client" uses the connected SDK callback.
  • Per-call models: uses the initialized model by default; a per-call model fully overrides; initialized credentials do not inherit into per-call models.
  • Aligns a misleading test name with current controller behavior.

Written for commit 74c6825. Summary will update on new commits.

Review in cubic

Add controller-level coverage for model string and object resolution paths that were previously marked it.todo.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 1 file

Architecture diagram
sequenceDiagram
    participant T as Test Suite
    participant C as Stagehand Controller
    participant R as Runtime
    participant BS as Browserbase Service
    participant S as act/extract/observe Services
    participant LLM as LLM Service

    Note over T,LLM: Model Resolution and Gateway Routing Paths

    Note over T: Direct Inference with Provider Auth
    
    T->>C: init(model={name, apiKey})
    C->>R: Configure runtime
    R-->>C: Runtime ready
    T->>C: act/extract/observe
    C->>S: call service(model, gateway=undefined)
    S-->>T: Verify no gateway passed

    Note over T: Browserbase Managed Inference
    
    T->>C: init(apiKey, browser, keyless model)
    C->>R: Configure runtime
    R-->>C: Runtime ready
    T->>C: act/extract/observe
    C->>BS: Resolve gateway (apiUrl, sessionId)
    BS-->>C: Gateway config
    C->>S: call service(model, gateway)
    S-->>T: Verify gateway routing

    alt Local Browser with Keyless Model
        T->>C: init(browserCdpUrl, keyless model)
        C->>R: Configure runtime
        R-->>C: Runtime ready
        T->>C: act/extract/observe
        C->>S: Forward model (gateway=undefined)
        S->>LLM: Attempt inference
        LLM-->>S: Reject (no gateway available)
        S-->>T: Verify forwarding behavior
    else Local Browser without Model
        T->>C: init(browserCdpUrl)
        C->>R: Configure runtime
        R-->>C: Runtime ready
        T->>C: act/extract/observe
        C-->>T: Error: "An LLM was not configured"
    end

    Note over T: Client Inference Path
    
    T->>C: init(model={source: "client"})
    C->>R: Configure runtime with SDK callback
    R-->>C: Runtime ready
    T->>C: act/extract/observe
    C->>S: call service(client callback)
    S-->>T: Verify SDK callback usage

    Note over T: Per-Call Model Overrides
    
    T->>C: init(modelA)
    C->>R: Configure runtime
    R-->>C: Runtime ready
    T->>C: act(model=modelB)
    C->>S: call service(modelB)
    S-->>T: Verify override, no credential inheritance
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/extension/tests/model-resolution.test.ts
Extract shared helpers so each case only states init params and expected
forwarded fields, without repeating act/extract/observe setup.
@changeset-bot

changeset-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 74c6825

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant