fix(coding-agent): make hindsight request timeouts per-op and configurable
The single 30s hang-guard from #4290 (sized for metadata fetches per #4229) wrapped every Hindsight op including reflect, an agentic retrieve+synthesize call whose healthy latency routinely exceeds 30s, so ordinary successful reflects were aborted client-side. - Give HindsightApi per-op deadlines with sensible defaults (reflect 120s, retain 60s, recall/request 30s) plus a HindsightTimeouts option bag. - Report the effective seconds in the timeout error instead of a hard-coded "after 30s". - Add hindsight.requestTimeoutMs / reflectTimeoutMs / recallTimeoutMs / retainTimeoutMs settings and matching HINDSIGHT_*_TIMEOUT_MS env vars. - Preserve caller-abort merging via withTimeoutSignal. Fixes #6125
This commit is contained in:
@@ -2,6 +2,14 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- Added configurable Hindsight client request deadlines via `hindsight.requestTimeoutMs` / `reflectTimeoutMs` / `recallTimeoutMs` / `retainTimeoutMs` settings (and matching `HINDSIGHT_*_TIMEOUT_MS` env vars).
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed the single 30s Hindsight client timeout aborting healthy `reflect` calls, which are agentic retrieve+synthesize ops that routinely exceed 30s; each op now has its own deadline (reflect defaults to 120s) and the timeout error reports the effective seconds ([#6125](https://github.com/can1357/oh-my-pi/issues/6125)).
|
||||
|
||||
## [17.0.5] - 2026-07-18
|
||||
|
||||
### Added
|
||||
|
||||
@@ -2861,6 +2861,11 @@ export const SETTINGS_SCHEMA = {
|
||||
|
||||
"hindsight.debug": { type: "boolean", default: false },
|
||||
|
||||
"hindsight.requestTimeoutMs": { type: "number", default: 30_000 },
|
||||
"hindsight.reflectTimeoutMs": { type: "number", default: 120_000 },
|
||||
"hindsight.recallTimeoutMs": { type: "number", default: 30_000 },
|
||||
"hindsight.retainTimeoutMs": { type: "number", default: 60_000 },
|
||||
|
||||
"hindsight.mentalModelsEnabled": {
|
||||
type: "boolean",
|
||||
default: true,
|
||||
|
||||
@@ -31,3 +31,45 @@ describe("HindsightApi fetch cancellation", () => {
|
||||
expect(requestSignal?.reason).toBe(caller.signal.reason);
|
||||
});
|
||||
});
|
||||
|
||||
describe("HindsightApi per-op timeouts", () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("reports the effective per-op deadline in the timeout error", async () => {
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(
|
||||
Object.assign(
|
||||
async (): Promise<Response> => {
|
||||
throw new DOMException("The operation timed out.", "TimeoutError");
|
||||
},
|
||||
{ preconnect: globalThis.fetch.preconnect },
|
||||
),
|
||||
);
|
||||
|
||||
const client = new HindsightApi({
|
||||
baseUrl: "https://hindsight.example",
|
||||
timeouts: { reflect: 90_000, recall: 15_000 },
|
||||
});
|
||||
|
||||
await expect(client.reflect("bank", "q")).rejects.toThrow("reflect request timed out after 90s");
|
||||
await expect(client.recall("bank", "q")).rejects.toThrow("recall request timed out after 15s");
|
||||
});
|
||||
|
||||
it("falls back to the client request default for ops without an override", async () => {
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(
|
||||
Object.assign(
|
||||
async (): Promise<Response> => {
|
||||
throw new DOMException("The operation timed out.", "TimeoutError");
|
||||
},
|
||||
{ preconnect: globalThis.fetch.preconnect },
|
||||
),
|
||||
);
|
||||
const client = new HindsightApi({
|
||||
baseUrl: "https://hindsight.example",
|
||||
timeouts: { request: 45_000 },
|
||||
});
|
||||
|
||||
await expect(client.createBank("bank")).rejects.toThrow("createBank request timed out after 45s");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -13,17 +13,32 @@ import type { HindsightConfig } from "./config";
|
||||
|
||||
const USER_AGENT = "oh-my-pi-coding-agent";
|
||||
const DEFAULT_USER_AGENT = USER_AGENT;
|
||||
const HINDSIGHT_REQUEST_TIMEOUT_MS = 30_000;
|
||||
/** Fallback deadlines (ms) applied when the caller supplies no override. */
|
||||
const DEFAULT_REQUEST_TIMEOUT_MS = 30_000;
|
||||
const DEFAULT_REFLECT_TIMEOUT_MS = 120_000;
|
||||
const DEFAULT_RECALL_TIMEOUT_MS = 30_000;
|
||||
const DEFAULT_RETAIN_TIMEOUT_MS = 60_000;
|
||||
|
||||
export type Budget = "low" | "mid" | "high" | string;
|
||||
export type TagsMatch = "any" | "all" | "any_strict" | "all_strict";
|
||||
export type UpdateMode = "replace" | "append";
|
||||
export type ConsolidationState = "failed" | "pending" | "done";
|
||||
|
||||
/** Per-operation request deadlines (ms). Each falls back to a built-in default. */
|
||||
export interface HindsightTimeouts {
|
||||
/** Default deadline for ops without a specific override. */
|
||||
request?: number;
|
||||
reflect?: number;
|
||||
recall?: number;
|
||||
retain?: number;
|
||||
}
|
||||
|
||||
export interface HindsightApiOptions {
|
||||
baseUrl: string;
|
||||
apiKey?: string;
|
||||
userAgent?: string;
|
||||
/** Per-op deadlines; unset entries fall back to built-in defaults. */
|
||||
timeouts?: HindsightTimeouts;
|
||||
}
|
||||
|
||||
/** Caller cancellation shared by Hindsight request option bags. */
|
||||
@@ -216,11 +231,17 @@ interface RequestOptions {
|
||||
/** Return null instead of throwing on a 404 response. */
|
||||
allow404?: boolean;
|
||||
signal?: AbortSignal;
|
||||
/** Op deadline (ms); defaults to the client's request timeout. */
|
||||
timeoutMs?: number;
|
||||
}
|
||||
|
||||
export class HindsightApi {
|
||||
#baseUrl: string;
|
||||
#headers: Record<string, string>;
|
||||
#reflectTimeoutMs: number;
|
||||
#recallTimeoutMs: number;
|
||||
#retainTimeoutMs: number;
|
||||
#requestTimeoutMs: number;
|
||||
|
||||
constructor(options: HindsightApiOptions) {
|
||||
this.#baseUrl = options.baseUrl.replace(/\/+$/, "");
|
||||
@@ -231,6 +252,11 @@ export class HindsightApi {
|
||||
if (options.apiKey) {
|
||||
this.#headers.Authorization = `Bearer ${options.apiKey}`;
|
||||
}
|
||||
const timeouts = options.timeouts;
|
||||
this.#requestTimeoutMs = timeouts?.request ?? DEFAULT_REQUEST_TIMEOUT_MS;
|
||||
this.#reflectTimeoutMs = timeouts?.reflect ?? DEFAULT_REFLECT_TIMEOUT_MS;
|
||||
this.#recallTimeoutMs = timeouts?.recall ?? DEFAULT_RECALL_TIMEOUT_MS;
|
||||
this.#retainTimeoutMs = timeouts?.retain ?? DEFAULT_RETAIN_TIMEOUT_MS;
|
||||
}
|
||||
|
||||
async retain(bankId: string, content: string, options?: RetainOptions): Promise<RetainResponse> {
|
||||
@@ -251,6 +277,7 @@ export class HindsightApi {
|
||||
{
|
||||
body: { items: [item], async: options?.async },
|
||||
signal: options?.signal,
|
||||
timeoutMs: this.#retainTimeoutMs,
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -282,6 +309,7 @@ export class HindsightApi {
|
||||
async: options?.async,
|
||||
},
|
||||
signal: options?.signal,
|
||||
timeoutMs: this.#retainTimeoutMs,
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -301,6 +329,7 @@ export class HindsightApi {
|
||||
tags_match: options?.tagsMatch,
|
||||
},
|
||||
signal: options?.signal,
|
||||
timeoutMs: this.#recallTimeoutMs,
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -319,6 +348,7 @@ export class HindsightApi {
|
||||
tags_match: options?.tagsMatch,
|
||||
},
|
||||
signal: options?.signal,
|
||||
timeoutMs: this.#reflectTimeoutMs,
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -506,10 +536,11 @@ export class HindsightApi {
|
||||
if (qs) url += `?${qs}`;
|
||||
}
|
||||
|
||||
const timeoutMs = opts?.timeoutMs ?? this.#requestTimeoutMs;
|
||||
const init: RequestInit = {
|
||||
method,
|
||||
headers: this.#headers,
|
||||
signal: withTimeoutSignal(HINDSIGHT_REQUEST_TIMEOUT_MS, opts?.signal),
|
||||
signal: withTimeoutSignal(timeoutMs, opts?.signal),
|
||||
};
|
||||
if (opts?.body !== undefined) {
|
||||
init.body = JSON.stringify(pruneUndefined(opts.body));
|
||||
@@ -520,7 +551,7 @@ export class HindsightApi {
|
||||
response = await fetch(url, init);
|
||||
} catch (err) {
|
||||
const message = isTimeoutError(err)
|
||||
? `${operation} request timed out after 30s`
|
||||
? `${operation} request timed out after ${Math.round(timeoutMs / 1000)}s`
|
||||
: `${operation} request failed: ${err instanceof Error ? err.message : String(err)}`;
|
||||
throw new HindsightError(message, undefined, err);
|
||||
}
|
||||
@@ -639,5 +670,11 @@ export function createHindsightClient(config: HindsightConfig & { hindsightApiUr
|
||||
baseUrl: config.hindsightApiUrl,
|
||||
apiKey: config.hindsightApiToken ?? undefined,
|
||||
userAgent: USER_AGENT,
|
||||
timeouts: {
|
||||
request: config.requestTimeoutMs,
|
||||
reflect: config.reflectTimeoutMs,
|
||||
recall: config.recallTimeoutMs,
|
||||
retain: config.retainTimeoutMs,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
@@ -42,6 +42,15 @@ export interface HindsightConfig {
|
||||
|
||||
debug: boolean;
|
||||
|
||||
/** Default per-request client deadline (ms) for ops without a specific override. */
|
||||
requestTimeoutMs: number;
|
||||
/** Client deadline (ms) for reflect (agentic synthesis; costlier than a metadata fetch). */
|
||||
reflectTimeoutMs: number;
|
||||
/** Client deadline (ms) for recall. */
|
||||
recallTimeoutMs: number;
|
||||
/** Client deadline (ms) for retain / retainBatch. */
|
||||
retainTimeoutMs: number;
|
||||
|
||||
mentalModelsEnabled: boolean;
|
||||
mentalModelAutoSeed: boolean;
|
||||
mentalModelRefreshIntervalMs: number;
|
||||
@@ -115,6 +124,10 @@ export function loadHindsightConfig(settings: Settings, env: NodeJS.ProcessEnv =
|
||||
const recallContextTurnsEnv = envInt(env.HINDSIGHT_RECALL_CONTEXT_TURNS);
|
||||
const recallMaxQueryCharsEnv = envInt(env.HINDSIGHT_RECALL_MAX_QUERY_CHARS);
|
||||
const retainEveryNTurnsEnv = envInt(env.HINDSIGHT_RETAIN_EVERY_N_TURNS);
|
||||
const requestTimeoutMsEnv = envInt(env.HINDSIGHT_REQUEST_TIMEOUT_MS);
|
||||
const reflectTimeoutMsEnv = envInt(env.HINDSIGHT_REFLECT_TIMEOUT_MS);
|
||||
const recallTimeoutMsEnv = envInt(env.HINDSIGHT_RECALL_TIMEOUT_MS);
|
||||
const retainTimeoutMsEnv = envInt(env.HINDSIGHT_RETAIN_TIMEOUT_MS);
|
||||
|
||||
// Read from settings (each falls back to its schema default).
|
||||
const settingsRetainMode = pickRetainMode(settings.get("hindsight.retainMode"));
|
||||
@@ -158,6 +171,11 @@ export function loadHindsightConfig(settings: Settings, env: NodeJS.ProcessEnv =
|
||||
|
||||
debug: debugEnv ?? settings.get("hindsight.debug"),
|
||||
|
||||
requestTimeoutMs: requestTimeoutMsEnv ?? settings.get("hindsight.requestTimeoutMs"),
|
||||
reflectTimeoutMs: reflectTimeoutMsEnv ?? settings.get("hindsight.reflectTimeoutMs"),
|
||||
recallTimeoutMs: recallTimeoutMsEnv ?? settings.get("hindsight.recallTimeoutMs"),
|
||||
retainTimeoutMs: retainTimeoutMsEnv ?? settings.get("hindsight.retainTimeoutMs"),
|
||||
|
||||
mentalModelsEnabled: settings.get("hindsight.mentalModelsEnabled"),
|
||||
mentalModelAutoSeed: settings.get("hindsight.mentalModelAutoSeed"),
|
||||
mentalModelRefreshIntervalMs: settings.get("hindsight.mentalModelRefreshIntervalMs"),
|
||||
|
||||
@@ -47,7 +47,7 @@ For independent per-item chains (review → verify, fetch → extract → score)
|
||||
schema: FINDINGS_SCHEMA,
|
||||
});
|
||||
return await parallel(found.findings.map((f) => async () => ({
|
||||
...f,
|
||||
…f,
|
||||
verdict: await agent(
|
||||
`Refute if you can (default refuted when unsure): ${f.title}`,
|
||||
{ label: `verify:${f.file}`, schema: VERDICT_SCHEMA },
|
||||
@@ -57,8 +57,6 @@ For independent per-item chains (review → verify, fetch → extract → score)
|
||||
phase("Review");
|
||||
const results = await parallel(DIMENSIONS.map((d) => async () => reviewAndVerify(d)));
|
||||
const confirmed = results.flat().filter((f) => f.verdict.is_real);
|
||||
|
||||
|
||||
Reach for `pipeline()` only when a stage genuinely needs ALL of the previous stage first — dedup/merge across the whole set, early-exit on zero, or "compare against the other findings" — because its inter-stage barrier makes every item wait for the slowest peer:
|
||||
|
||||
**Python (`eval`, Python backend):**
|
||||
@@ -80,8 +78,6 @@ Reach for `pipeline()` only when a stage genuinely needs ALL of the previous sta
|
||||
const verdicts = await parallel(findings.map((f) => async () =>
|
||||
await agent(verifyPrompt(f), { schema: VERDICT_SCHEMA }),
|
||||
));
|
||||
|
||||
|
||||
Use ordinary code between calls to flatten/map/filter; don't add a barrier just for that. Nested `parallel()` pools each cap independently, so keep total fan-out sane.
|
||||
</structure>
|
||||
|
||||
|
||||
@@ -60,6 +60,10 @@ const baseConfig = (overrides: Partial<HindsightConfig> = {}): HindsightConfig =
|
||||
recallMaxQueryChars: 800,
|
||||
recallPromptPreamble: "preamble",
|
||||
debug: false,
|
||||
requestTimeoutMs: 30_000,
|
||||
reflectTimeoutMs: 120_000,
|
||||
recallTimeoutMs: 30_000,
|
||||
retainTimeoutMs: 60_000,
|
||||
mentalModelsEnabled: false,
|
||||
mentalModelAutoSeed: false,
|
||||
mentalModelRefreshIntervalMs: 5 * 60 * 1000,
|
||||
|
||||
@@ -64,6 +64,10 @@ function makeConfig(overrides: Partial<HindsightConfig> = {}): HindsightConfig {
|
||||
recallMaxQueryChars: 800,
|
||||
recallPromptPreamble: "preamble",
|
||||
debug: false,
|
||||
requestTimeoutMs: 30_000,
|
||||
reflectTimeoutMs: 120_000,
|
||||
recallTimeoutMs: 30_000,
|
||||
retainTimeoutMs: 60_000,
|
||||
mentalModelsEnabled: false,
|
||||
mentalModelAutoSeed: false,
|
||||
mentalModelRefreshIntervalMs: 5 * 60 * 1000,
|
||||
|
||||
Reference in New Issue
Block a user