add serverCacheThreshold option to act/observe/extract#2358
Conversation
Adds a per-action override of the server cache validation threshold. The SDK option serverCacheThreshold maps to options.cacheThreshold on the wire; the server persists it on the cache entry so later requests that omit it keep honoring it.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 5/5
- In
packages/core/lib/v3/types/public/api.ts, thecacheThresholdoption is documented without its cloud-only limitation, so local-mode users may assume it works and misconfigure caching expectations; this is mainly a clarity risk rather than a runtime regression—mark it cloud-only (and ideally explicitly ignored in local mode) in all three option schema docs before or right after merge.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/types/public/api.ts">
<violation number="1" location="packages/core/lib/v3/types/public/api.ts:627">
P3: Local-mode users cannot tell this option has no effect because its public schema docs omit the cloud-only limitation. Label `cacheThreshold` cloud-only (and ideally note it is ignored locally) in all three option schemas.
(Based on your team's feedback about cloud-only schema fields.) [FEEDBACK_USED]</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as User Code
participant SDK as Stagehand SDK (api.ts)
participant Execute as execute()
participant API as Stagehand API Server
participant Cache as Redis Cache Entry
participant LD as LaunchDarkly Flag
Note over Client,LD: NEW: serverCacheThreshold flow
Client->>SDK: stagehand.act("click", { serverCacheThreshold: 5 })
Note over SDK: destructure options,<br/>strip page, serverCache,<br/>extract serverCacheThreshold
SDK->>SDK: map serverCacheThreshold → cacheThreshold
SDK->>Execute: execute({<br/>method: "act",<br/>args: {<br/> input: "click",<br/> options: { timeout,<br/> cacheThreshold: 5 }<br/>},<br/>serverCache: undefined<br/>})
alt API mode
Execute->>API: HTTP POST /act with wire options
Note over API: Threshold resolution priority:<br/>1) request cacheThreshold<br/>2) eval env override<br/>3) sticky per-entry value<br/>4) project LD flag
API->>Cache: GET cache entry (key excludes threshold)
Cache-->>API: entry found (count=3, threshold=2)
API->>Cache: CHANGED: update threshold to 5<br/>(persists on cache entry)
API->>API: check: count (3) >= threshold (5)? → no
API->>API: execute action (cache miss, compute)
API-->>Execute: response with result
else Local mode (SDK)
Note over SDK: serverCacheThreshold ignored<br/>(same as serverCache)
SDK->>Client: execute action locally
end
Execute-->>SDK: result
SDK-->>Client: ActResult
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
| @@ -624,6 +624,11 @@ export const ActOptionsSchema = z | |||
| description: "Timeout in ms for the action", | |||
There was a problem hiding this comment.
P3: Local-mode users cannot tell this option has no effect because its public schema docs omit the cloud-only limitation. Label cacheThreshold cloud-only (and ideally note it is ignored locally) in all three option schemas.
(Based on your team's feedback about cloud-only schema fields.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/types/public/api.ts, line 627:
<comment>Local-mode users cannot tell this option has no effect because its public schema docs omit the cloud-only limitation. Label `cacheThreshold` cloud-only (and ideally note it is ignored locally) in all three option schemas.
(Based on your team's feedback about cloud-only schema fields.) </comment>
<file context>
@@ -624,6 +624,11 @@ export const ActOptionsSchema = z
description: "Timeout in ms for the action",
example: 30000,
}),
+ cacheThreshold: z.number().int().nonnegative().optional().meta({
+ description:
+ "Per-action override of the server cache validation threshold — how many times this action must be seen before the cached result is served. Persists on the cache entry for later requests that omit it.",
</file context>
sameelarif
changed the title
sameelarif
changed the title
| const mappedOptions = { | ||
| ...restOptions, | ||
| ...(serverCacheThreshold !== undefined && { | ||
| cacheThreshold: serverCacheThreshold, |
There was a problem hiding this comment.
assuming it would require a server side change but wouldn't it make more sense to have the both named serverCacheThreshold ?
When the server cache misses and reports why, attach the reason (e.g. threshold, empty_array, timeout, error) to the result next to cacheStatus. The server sends it in the finished SSE message; the field is absent on hits and on first-time misses with no prior entry, and against older servers that don't send it.
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/api.ts">
<violation number="1" location="packages/core/lib/v3/api.ts:949">
P2: The new `cacheMissReason` population logic in `attachCacheStatus` has no accompanying unit tests. The PR only adds type-shape tests, but the runtime behavior — that the reason is only attached on MISS, only when provided, and is suppressed when caching is disabled — is untested. Without coverage, regressions or silent breaks in this response-handling path (which can affect callers inspecting why a result missed the cache) won't be caught.</violation>
</file>
<file name="packages/core/lib/v3/types/public/methods.ts">
<violation number="1" location="packages/core/lib/v3/types/public/methods.ts:44">
P2: Extract schemas that legitimately return a `cacheMissReason` field now get an unusable intersection type and have their extracted value overwritten on cache misses. Keep cache metadata separate from arbitrary extraction keys (or reserve/strip this key before augmenting the result).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * "empty_array", "timeout", "error"). Absent on hits and on first-time | ||
| * misses with no prior cache entry. | ||
| */ | ||
| cacheMissReason?: string; |
There was a problem hiding this comment.
P2: Extract schemas that legitimately return a cacheMissReason field now get an unusable intersection type and have their extracted value overwritten on cache misses. Keep cache metadata separate from arbitrary extraction keys (or reserve/strip this key before augmenting the result).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/types/public/methods.ts, line 44:
<comment>Extract schemas that legitimately return a `cacheMissReason` field now get an unusable intersection type and have their extracted value overwritten on cache misses. Keep cache metadata separate from arbitrary extraction keys (or reserve/strip this key before augmenting the result).</comment>
<file context>
@@ -36,11 +36,18 @@ export interface ActResult {
+ * "empty_array", "timeout", "error"). Absent on hits and on first-time
+ * misses with no prior cache entry.
+ */
+ cacheMissReason?: string;
}
</file context>
| finalCacheStatus === "MISS" && | ||
| typeof eventData.data.cacheMissReason === "string" | ||
| ) { | ||
| cacheAwareResult.cacheMissReason = eventData.data.cacheMissReason; |
There was a problem hiding this comment.
P2: The new cacheMissReason population logic in attachCacheStatus has no accompanying unit tests. The PR only adds type-shape tests, but the runtime behavior — that the reason is only attached on MISS, only when provided, and is suppressed when caching is disabled — is untested. Without coverage, regressions or silent breaks in this response-handling path (which can affect callers inspecting why a result missed the cache) won't be caught.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/api.ts, line 949:
<comment>The new `cacheMissReason` population logic in `attachCacheStatus` has no accompanying unit tests. The PR only adds type-shape tests, but the runtime behavior — that the reason is only attached on MISS, only when provided, and is suppressed when caching is disabled — is untested. Without coverage, regressions or silent breaks in this response-handling path (which can affect callers inspecting why a result missed the cache) won't be caught.</comment>
<file context>
@@ -936,9 +936,18 @@ export class StagehandAPIClient {
+ finalCacheStatus === "MISS" &&
+ typeof eventData.data.cacheMissReason === "string"
+ ) {
+ cacheAwareResult.cacheMissReason = eventData.data.cacheMissReason;
+ }
}
</file context>
The API now accepts serverCacheThreshold directly, so drop the SDK-to-wire rename and send the option under its own name. Server must deploy before this releases: older servers strip the unknown key and fall back to the default threshold.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/tests/unit/api-client-serialization.test.ts">
<violation number="1" location="packages/core/tests/unit/api-client-serialization.test.ts:168">
P1: Custom agent: **Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test**
The PR description states that `serverCacheThreshold` should be mapped to `options.cacheThreshold` on the wire, following the existing `serverCache` pattern. However, looking at the API client in `packages/core/lib/v3/api.ts`, the `act()`, `observe()`, and `extract()` methods all destructure `serverCacheThreshold` from options and then spread it back into the wire payload under the same SDK name (`serverCacheThreshold`) — without renaming it to `cacheThreshold`. This means the wire schemas (which expect `cacheThreshold`) will never receive the cache threshold value. The unit tests were also updated to match the incorrect behavior, expecting `serverCacheThreshold` in the wire payload instead of `cacheThreshold`. Please update the API client to map `serverCacheThreshold` → `cacheThreshold` when building wire options, and fix the test expectations accordingly.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| input: "click the submit button", | ||
| options: { | ||
| timeout: 30000, | ||
| serverCacheThreshold: 5, |
There was a problem hiding this comment.
P1: Custom agent: Any breaking changes to Stagehand REST API client / server implementation must be covered by an integration test under packages/server/test
The PR description states that serverCacheThreshold should be mapped to options.cacheThreshold on the wire, following the existing serverCache pattern. However, looking at the API client in packages/core/lib/v3/api.ts, the act(), observe(), and extract() methods all destructure serverCacheThreshold from options and then spread it back into the wire payload under the same SDK name (serverCacheThreshold) — without renaming it to cacheThreshold. This means the wire schemas (which expect cacheThreshold) will never receive the cache threshold value. The unit tests were also updated to match the incorrect behavior, expecting serverCacheThreshold in the wire payload instead of cacheThreshold. Please update the API client to map serverCacheThreshold → cacheThreshold when building wire options, and fix the test expectations accordingly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/tests/unit/api-client-serialization.test.ts, line 168:
<comment>The PR description states that `serverCacheThreshold` should be mapped to `options.cacheThreshold` on the wire, following the existing `serverCache` pattern. However, looking at the API client in `packages/core/lib/v3/api.ts`, the `act()`, `observe()`, and `extract()` methods all destructure `serverCacheThreshold` from options and then spread it back into the wire payload under the same SDK name (`serverCacheThreshold`) — without renaming it to `cacheThreshold`. This means the wire schemas (which expect `cacheThreshold`) will never receive the cache threshold value. The unit tests were also updated to match the incorrect behavior, expecting `serverCacheThreshold` in the wire payload instead of `cacheThreshold`. Please update the API client to map `serverCacheThreshold` → `cacheThreshold` when building wire options, and fix the test expectations accordingly.</comment>
<file context>
@@ -165,15 +165,15 @@ describe("StagehandAPIClient variable serialization", () => {
options: {
timeout: 30000,
- cacheThreshold: 5,
+ serverCacheThreshold: 5,
},
frameId: undefined,
</file context>
Apply cache gating to final buffered SSE events and cover cache miss reason attachment behavior so bypassed requests cannot expose server cache metadata. Co-authored-by: Cursor <cursoragent@cursor.com>
Expose cache hit counts and input/output token savings on action results while preserving cache-bypass metadata suppression. Co-authored-by: Cursor <cursoragent@cursor.com>
Use the clearer client-facing cacheHitCount name while retaining the server wire field for compatibility. Co-authored-by: Cursor <cursoragent@cursor.com>
…data Keep cacheStatus on results by default, and only attach cacheHitCount, tokensSaved, and cacheMissReason when callers opt in via includeCacheMetadata. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/api.ts">
<violation number="1" location="packages/core/lib/v3/api.ts:310">
P3: `extract()` and `observe()` now have independent metadata-forwarding paths, but coverage exercises only `act()`, so a regression in either duplicated path will go unnoticed. Add parity serialization/result tests for both methods.
(Based on your team's feedback about adding unit tests for new behavior.) [FEEDBACK_USED].</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // Strip non-serializable `page` and SDK-only fields from options before wire serialization | ||
| let wireOptions: Api.ActRequest["options"]; | ||
| let serverCache: boolean | undefined; | ||
| let includeCacheMetadata: boolean | undefined; |
There was a problem hiding this comment.
P3: extract() and observe() now have independent metadata-forwarding paths, but coverage exercises only act(), so a regression in either duplicated path will go unnoticed. Add parity serialization/result tests for both methods.
(Based on your team's feedback about adding unit tests for new behavior.) .
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/api.ts, line 310:
<comment>`extract()` and `observe()` now have independent metadata-forwarding paths, but coverage exercises only `act()`, so a regression in either duplicated path will go unnoticed. Add parity serialization/result tests for both methods.
(Based on your team's feedback about adding unit tests for new behavior.) .</comment>
<file context>
@@ -302,15 +307,18 @@ export class StagehandAPIClient {
// Strip non-serializable `page` and SDK-only fields from options before wire serialization
let wireOptions: Api.ActRequest["options"];
let serverCache: boolean | undefined;
+ let includeCacheMetadata: boolean | undefined;
if (options) {
const {
</file context>
2 participants
Summary
Adds the SDK side of caching configurability: a per-action override of the server cache validation threshold.
serverCacheThreshold?: number(non-negative integer) added toActOptions,ObserveOptions, andExtractOptionsoptions.cacheThresholdon the wire and strips the SDK-named field, following the existingserverCachepatternActOptionsSchema/ObserveOptionsSchema/ExtractOptionsSchema) gain thecacheThresholdfieldserverCacheTests
serverCacheThresholdmaps tooptions.cacheThresholdfor act/observe/extract and is never sent under its SDK nameSummary by cubic
Adds a per-action server cache threshold and an opt-in flag for detailed cache metadata on
act,observe, andextract.cacheStatusis still attached by default when caching is used, and all cache fields remain suppressed when caching is disabled.New Features
serverCacheThreshold?: numbertoActOptions,ObserveOptions, andExtractOptions; sent asserverCacheThresholdand included in the wire schemas.includeCacheMetadata?: boolean; when true, results includecacheHitCount,tokensSaved(input/output/total), andcacheMissReasonon misses.cacheStatusis always present when caching is used.TokenSavingstype. Ignored in local (non-API) mode. Meets STG-2564, STG-2602, STG-2614.Bug Fixes
cacheStatus,cacheMissReason,cacheHitCount,tokensSaved) whenserverCache: false, including for final buffered SSE events (STG-2602).cacheCountascacheHitCounton results when metadata is included (STG-2602).Written for commit d91acab. Summary will update on new commits.