Redundant per-request timeout overrides client-level default

timeout: this.timeoutMs is set in both the OpenAI constructor (line 182) and the embeddings.create per-request options (line 195). The OpenAI SDK applies the constructor-level timeout as the default for every request; the per-request option then overrides it with the exact same value. The per-request call is redundant.

One or the other is sufficient. Keeping the constructor-level setting is idiomatic and covers any future methods added to the class automatically:

If you'd prefer the intent to be explicit at the call site instead, remove timeout: this.timeoutMs from the constructor options and keep only the per-request option.

This is a comment left during a code review.
Path: extensions/memory-lancedb/index.ts
Line: 179-195

Comment:
**Redundant per-request timeout overrides client-level default**

`timeout: this.timeoutMs` is set in both the `OpenAI` constructor (line 182) and the `embeddings.create` per-request options (line 195). The OpenAI SDK applies the constructor-level timeout as the default for every request; the per-request option then overrides it with the exact same value. The per-request call is redundant.

One or the other is sufficient. Keeping the constructor-level setting is idiomatic and covers any future methods added to the class automatically:

```suggestion
    const response = await this.client.embeddings.create(params);
```

If you'd prefer the intent to be explicit at the call site instead, remove `timeout: this.timeoutMs` from the constructor options and keep only the per-request option.

How can I resolve this? If you propose a fix, please make it concise.

No bounds validation for timeoutMs or maxRetries

The parser accepts any numeric value for both fields without range checks. A timeoutMs of 0 would cause every embedding request to time out immediately (or behave unpredictably depending on the OpenAI SDK version), and a negative maxRetries is also not meaningful.

Consider adding guards, for example:

timeoutMs:
  typeof embedding.timeoutMs === "number" && embedding.timeoutMs > 0
    ? embedding.timeoutMs
    : undefined,
maxRetries:
  typeof embedding.maxRetries === "number" && embedding.maxRetries >= 0
    ? embedding.maxRetries
    : undefined,

This is a comment left during a code review.
Path: extensions/memory-lancedb/config.ts
Line: 132-133

Comment:
**No bounds validation for `timeoutMs` or `maxRetries`**

The parser accepts any numeric value for both fields without range checks. A `timeoutMs` of `0` would cause every embedding request to time out immediately (or behave unpredictably depending on the OpenAI SDK version), and a negative `maxRetries` is also not meaningful.

Consider adding guards, for example:

```ts
timeoutMs:
  typeof embedding.timeoutMs === "number" && embedding.timeoutMs > 0
    ? embedding.timeoutMs
    : undefined,
maxRetries:
  typeof embedding.maxRetries === "number" && embedding.maxRetries >= 0
    ? embedding.maxRetries
    : undefined,
```

How can I resolve this? If you propose a fix, please make it concise.

Test doesn't import production policy values from run.ts

The test constructs its own BackoffPolicy with hardcoded values (initialMs: 250, maxMs: 30_000, factor: 2) that mirror the production defaults in run.ts. If a future change updates the production defaults (e.g. initialMs or factor), these tests will continue to pass without catching the regression.

Consider exporting the default policy constants from run.ts (or a separate constants file) and importing them in the test, so the test stays coupled to the production values:

// In run.ts — export the defaults so tests can reference them
export const OVERLOAD_FAILOVER_BACKOFF_DEFAULTS = {
  initialMs: 250,
  factor: 2,
  jitter: 0.2,
  defaultMaxMs: 30_000,
} as const;

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/run.overload-backoff.test.ts
Line: 1-15

Comment:
**Test doesn't import production policy values from `run.ts`**

The test constructs its own `BackoffPolicy` with hardcoded values (`initialMs: 250`, `maxMs: 30_000`, `factor: 2`) that mirror the production defaults in `run.ts`. If a future change updates the production defaults (e.g. `initialMs` or `factor`), these tests will continue to pass without catching the regression.

Consider exporting the default policy constants from `run.ts` (or a separate constants file) and importing them in the test, so the test stays coupled to the production values:

```ts
// In run.ts — export the defaults so tests can reference them
export const OVERLOAD_FAILOVER_BACKOFF_DEFAULTS = {
  initialMs: 250,
  factor: 2,
  jitter: 0.2,
  defaultMaxMs: 30_000,
} as const;
```

How can I resolve this? If you propose a fix, please make it concise.

Allow overloadBackoffMaxMs through config validation

This new knob is read at runtime, but it is not actually accepted by the config schema, so users cannot set it without hitting a validation error. runEmbeddedPiAgent now reads agents.defaults.embeddedPi.overloadBackoffMaxMs, yet embeddedPi remains a strict object with only projectSettingsPolicy in the validation schemas (src/config/zod-schema.agent-defaults.ts and the generated base schema), so the override is rejected before this code runs.

Useful? React with 👍 / 👎.

This adds help metadata for agents.defaults.embeddedPi.overloadBackoffMaxMs, but the generated JSON schema for agents.defaults.embeddedPi (earlier in this file) still does not define overloadBackoffMaxMs as an allowed property. That leaves docs/UI/schema consumers inconsistent and may cause the setting to be rejected/stripped. Regenerate/update the schema so the key exists under embeddedPi.properties (and aligns with the Zod validator).

embedding.timeoutMs / embedding.maxRetries are accepted as numbers but aren’t range-checked. This allows negative / non-integer values, which can disable timeouts (or cause immediate timeouts) and can break OpenAI client retry logic. Validate these in parse() (e.g., timeoutMs int > 0, maxRetries int >= 0) and throw a clear error when out of range.

The PR description focuses on memory-lancedb embedding timeout/retries and launchd throttling, but this change also introduces a new agent config option (agents.defaults.embeddedPi.overloadBackoffMaxMs). Either update the PR description to cover this behavior change or split it into a separate PR so reviewers/operators don’t miss the new knob.

overloadBackoffMaxMs is read from params.config, but the config validator currently uses a strict AgentDefaultsSchema that only allows projectSettingsPolicy under agents.defaults.embeddedPi. As a result, configs that set agents.defaults.embeddedPi.overloadBackoffMaxMs will be rejected and this override will never take effect. Add this field to src/config/zod-schema.agent-defaults.ts (and any related schema/label generation) so it’s accepted and validated (e.g., int + nonnegative/positive).