Skip to content

[v4]: update /scroll route stub#1875

Merged
seanmcguire12 merged 3 commits intomainfrom
seanmcguire/stg-1644-unify-scroll-scrollto
Mar 27, 2026
Merged

[v4]: update /scroll route stub#1875
seanmcguire12 merged 3 commits intomainfrom
seanmcguire/stg-1644-unify-scroll-scrollto

Conversation

@seanmcguire12
Copy link
Copy Markdown
Member

@seanmcguire12 seanmcguire12 commented Mar 23, 2026

why

  • the existing /page/scroll endpoint had an awkward union of PageScrollElementParams (element selector + percentage) and PageScrollCoordinateParams (coordinate selector + deltaX/deltaY).
  • these two modes were hard to distinguish and didn't cover common scrolling patterns like "scroll N pages" or "scroll until this element is visible"

what changed

  • replaced the two old scroll param schemas with a cleaner 3-way discriminated union:
    • PageScrollByOffsetParams: scroll by pixel delta ({ cursorPosition?, offset: {x, y} })
    • PageScrollByPagesParams: scroll by viewport heights ({ cursorPosition?, pages: number, delayBetweenMs? })
    • PageScrollToTargetParams: scroll until a target is visible ({ target: Selector })
  • cursorPosition is an optional Selector that determines where the mouse cursor is placed before scrolling. this controls which scrollable container receives the event.

test plan

  • updated the route stub and integration test to match the new schema

Summary by cubic

Refactors the v4 /page/scroll route into a simple 3‑mode API and updates OpenAPI to return a typed PageScrollActionOutput. The stub now returns {x: 0, y: 0}; adds position for target scrolls and delayBetweenMs for page scrolling, aligning with Linear STG-1644.

  • Refactors

    • Replaced old params with 3 modes: by offset {offset:{x,y}}, by pages {pages, delayBetweenMs?}, or to target {target, position?}; optional {cursorPosition} selects the scroll container.
    • OpenAPI now uses PageScrollActionOutput and PageScroll*ParamsOutput (defaults resolved, stricter requireds) and wires it into the PageAction union.
    • Route stub execution simplified to always return {x: 0, y: 0}.
  • Migration

    • Stop using selector + percentage and selector + deltaX/deltaY.
    • Send one of: {offset:{x,y}}, {pages, delayBetweenMs?, cursorPosition?}, or {target, position?}.
    • Expect {x,y} in the result; the action shape is now PageScrollActionOutput.

Written for commit b8a85a3. Summary will update on new commits. Review in cubic

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 23, 2026

⚠️ No Changeset found

Latest commit: b8a85a3

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

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a 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 4 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.
Architecture diagram
sequenceDiagram
    participant Client
    participant API as API Server (v4)
    participant Val as Validation (Zod)
    participant Engine as Browser Engine

    Note over Client,Engine: Action Request Flow (Scroll)

    Client->>API: POST /page/scroll { params }
    API->>Val: NEW: Validate 3-way Discriminated Union
    
    alt Mode: PageScrollByOffset
        Val-->>API: Validated { offset: {x, y}, cursorPosition? }
    else Mode: PageScrollByPages
        Val-->>API: Validated { pages: N, delayBetweenMs?, cursorPosition? }
    else Mode: PageScrollToTarget
        Val-->>API: Validated { target: Selector }
    end

    opt NEW: cursorPosition provided
        API->>Engine: Move mouse to cursorPosition selector
        Note right of Engine: Focuses specific scrollable container
    end

    API->>Engine: Execute Scroll Logic (Offset, Pages, or Target)
    Engine-->>API: NEW: Return final scroll coordinates { x, y }
    
    API-->>Client: 200 OK with PageScrollResult { x, y }

    Note over Client,Engine: Unified Action Result Flow (Click/Hover/Drag)

    Client->>API: POST /page/click { selector, returnSelector: true }
    API->>Engine: Perform Action
    Engine-->>API: Return interaction details
    API->>API: NEW: Construct ResultSelector (xpath, css, text, coords)
    API-->>Client: 200 OK with CHANGED: PageActionOutput (includes ResultSelector)
Loading

cursorPosition: SelectorSchema.optional(),
offset: z
.object({
x: z.number(),
Copy link
Copy Markdown
Member

@pirate pirate Mar 25, 2026

Choose a reason for hiding this comment

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

this should be optional or at least default to 0

Suggested change
x: z.number(),
x: z.number().default(0),

selector: ElementSelectorSchema,
percentage: z.number().min(0).max(100),
export const PageScrollByOffsetParamsSchema = PageWithPageIdSchema.extend({
cursorPosition: SelectorSchema.optional(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
cursorPosition: SelectorSchema.optional(),
element: SelectorSchema.optional(),

I would really call this element or container


export const PageScrollByPagesParamsSchema = PageWithPageIdSchema.extend({
cursorPosition: SelectorSchema.optional(),
pages: z.number(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
pages: z.number(),
pages: z.number().lte(100).gte(-100).default(1),

I would define min and max values here and have the min allow negative values so agents reading the api spec can tell that negatives and floats are allowed.

deltaX: z.number().optional(),
deltaY: z.number(),
export const PageScrollToTargetParamsSchema = PageWithPageIdSchema.extend({
target: SelectorSchema,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
target: SelectorSchema,
target: SelectorSchema,
position: z.enum(['center', 'top', 'bottom']).default('center'),

Copy link
Copy Markdown
Member

@pirate pirate left a comment

Choose a reason for hiding this comment

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

Approved but ideally I would change the shapes slightly (see comments)

@seanmcguire12 seanmcguire12 force-pushed the seanmcguire/stg-1644-unify-scroll-scrollto branch from 2adf994 to 10c534e Compare March 26, 2026 17:03
@seanmcguire12 seanmcguire12 force-pushed the seanmcguire/stg-1644-unify-scroll-scrollto branch from 10c534e to b8a85a3 Compare March 27, 2026 19:01
@seanmcguire12 seanmcguire12 merged commit c50a5d8 into main Mar 27, 2026
29 checks passed
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.

2 participants