Skip to content

feat(coding-agents): add maxParallelRetains config and 429-aware drain - #3390

Draft
seppaleinen wants to merge 1 commit into
vectorize-io:mainfrom
seppaleinen:feat/max-parallel-retains
Draft

feat(coding-agents): add maxParallelRetains config and 429-aware drain#3390
seppaleinen wants to merge 1 commit into
vectorize-io:mainfrom
seppaleinen:feat/max-parallel-retains

Conversation

@seppaleinen

Copy link
Copy Markdown

Problem

The coding-agents integration floods the Hindsight API with concurrent retain-related requests and receives HTTP 429s:

  1. Unbounded concurrent op polling in drain()HindsightClient.drain() polls every pending operation with Promise.all([...pending].map(fetch)). When a session enqueues many async retains, every op is polled at once, every 5s cycle. No concurrency cap.
  2. No 429/Retry-After handling in the poll loop — a 429 was treated like any other non-ok response (if (!r.ok) return;), leaving the op pending and re-polling the full set again 5s later. The client hammers the API harder while it is rate-limiting.
  3. Fire-and-forget retain pools — deepen's chat ingestion and per-commit diff retain pools used a hardcoded CONCURRENCY = 4 that could not be tuned, and the drain it waits on was uncapped.

Root-cause evidence: a single GET /operations/<id> returns 200, so the rate limiter is burst/concurrency-triggered, not volume-triggered. drain() issues N concurrent GETs (N = pending ops) every cycle with no cap — exactly the burst pattern that trips it. There is no Retry-After handling, so the client re-bursts 5s later instead of backing off.

Change Summary

  • Config: new maxParallelRetains option (number, default 10) — the cap on concurrent retain-related requests (drain op polls + deepen retain pools). A single request returning 200 while bursts get 429s means the server is rate-limiting concurrency, so this is the knob to turn down.
    • Config file: maxParallelRetains in ~/.hindsight/coding-agent.json
    • Env: HINDSIGHT_MAX_PARALLEL_RETAINS
    • Env layer added to ENV_KEYS and ENV_NUMBERS in src/core/config.ts; resolved via resolveConfig().
  • Client drain() (src/core/hindsight.ts): rewired the per-cycle poll from Promise.all over every pending op to the existing bounded pool(items, n, fn) helper, capped at maxParallelRetains. Added 429 handling:
    • On HTTP 429, the op stays pending and the next cycle backs off by the Retry-After header (parsed as delta-seconds or HTTP-date), with a 10s floor when the header is absent or shorter.
    • Without a 429 the existing 5s cycle and 60-min maxMs bound are preserved.
    • New exported retryAfterMs() helper parses the header; new DEFAULT_MAX_PARALLEL_RETAINS constant.
  • Client option plumbing: ClientOpts accepts maxParallelRetains (default 10); threaded from config at every construction site — deepen.ts, plugin-entry.ts (opencode/kilo), cline.ts, mcp-server.ts, status.ts, and the hook.ts / retain-hook.ts / session-start.ts makeClient seams.
  • deepen pools (src/deepen.ts): removed hardcoded CONCURRENCY = 4; chat ingestion and the git-diff retain pool now use the configured cfg.maxParallelRetains (pool semantics unchanged).
  • Docs: added maxParallelRetains / HINDSIGHT_MAX_PARALLEL_RETAINS to the package README configuration reference.

Testing Done

  • New unit tests (src/core/hindsight.test.ts, 11 tests):
    • drain() issues at most N concurrent fetches (mock global.fetch, track in-flight count; 5 ids against a 2-wide pool hits exactly the cap).
    • 429 + Retry-After: 30 → backs off 30s (not the 10s floor) before the next cycle.
    • 429 + Retry-After: 2 → uses the 10s floor.
    • 429 without Retry-After → 10s floor.
    • No 429, op still pending → existing 5s cycle preserved.
    • Non-completed terminal ops counted as failed; retryAfterMs parses delta-seconds / HTTP-date / garbage.
  • New config tests (src/core/config.test.ts, 4 tests): default 10, file override, env number parse, malformed env ignored.
  • npm test (vitest): 448 passed, 1 failed — the single failure (knowledge-tools.test.ts hindsight_diagnose) is pre-existing and environment-dependent: it asserts api_token_configured: false but the test shell exports HINDSIGHT_API_TOKEN, so loadConfig sees a token. Same failure on the untouched baseline (433 passed before this change).
  • npm run build (tsup): success (ESM bundles + DTS). dist/ is gitignored and not committed.
  • npx tsc --noEmit: clean.

Notes

  • The HindsightClient class is defined in hindsight-integrations/coding-agents/src/core/hindsight.ts itself (not imported from @vectorize-io/hindsight-all), so all changes are contained in the coding-agents package.
  • The node_modules symlink tracked by the repo is untouched.

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