Skip to content

Add wall-clock timeout to reflect operations#643

Merged
nicoloboschi merged 3 commits intovectorize-io:mainfrom
ThePlenkov:copilot/fix-issue-642
Mar 23, 2026
Merged

Add wall-clock timeout to reflect operations#643
nicoloboschi merged 3 commits intovectorize-io:mainfrom
ThePlenkov:copilot/fix-issue-642

Conversation

@ThePlenkov
Copy link
Contributor

@ThePlenkov ThePlenkov commented Mar 21, 2026

Reflect has no overall timeout. With high budget (20 iterations × 120s LLM timeout), a single call can hang for ~40 minutes.

Changes

  • Config: HINDSIGHT_API_REFLECT_WALL_TIMEOUT env var (default 300s)
  • Engine: Wrap run_reflect_agent() in asyncio.wait_for() inside reflect_async()
  • HTTP: Catch TimeoutError in reflect endpoint, return 504
  • Test: Unit test for timeout enforcement

MCP callers are already covered by the existing except Exception handler—no changes needed there.

# memory_engine.py
try:
    agent_result = await asyncio.wait_for(
        run_reflect_agent(...),
        timeout=wall_timeout,
    )
except asyncio.TimeoutError:
    raise TimeoutError(
        f"Reflect operation timed out after {wall_timeout} seconds. "
        f"Consider reducing the budget or simplifying the query."
    )

Timeout is enforced at the reflect_async() level so both HTTP and MCP callers are protected.

Reflect operations have no overall wall-clock timeout ��� only per-LLM-call timeouts. With high budget (20 iterations �� 120s default LLM timeout), a reflect call can hang for ~40 minutes.

Fixes REDACTED` (http block)

  • https://api.github.com/repos/vectorize-io/hindsight/pulls
    • Triggering command: /usr/bin/curl curl -s -w \n%{http_code} -X POST -H Authorization: token ****** -H Accept: application/vnd.github+json REDACTED -d { "title": "feat: add wall-clock timeout to reflect operations (fixes #642)", "body": "## Summary\n\nReflect operations have no overall wall-clock timeout ��� only per-LLM-call timeouts. With high budget (20 iterations �� 120s default LLM timeout) (http block)
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (http block)

If you need me to access, download, or install something from one of these locations, you can either:


⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.

Copilot AI and others added 2 commits March 21, 2026 10:02
…o#642)

Add a configurable wall-clock timeout (default: 300s / 5 minutes) for
the entire reflect operation. This prevents reflect calls from hanging
for up to 40 minutes when LLM calls are slow or iteration counts are
high.

Changes:
- Add DEFAULT_REFLECT_WALL_TIMEOUT (300s) config constant
- Add HINDSIGHT_API_REFLECT_WALL_TIMEOUT env variable support
- Wrap run_reflect_agent() with asyncio.wait_for() in reflect_async()
- Return HTTP 504 on timeout in the reflect HTTP endpoint
- Add unit test for wall-clock timeout enforcement

Co-authored-by: ThePlenkov <6381507+ThePlenkov@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ThePlenkov/hindsight/sessions/a123d68b-aca1-4040-8bba-8c4f0fab2e2c
@ThePlenkov ThePlenkov changed the title Copilot/fix issue 642 Add wall-clock timeout to reflect operations Mar 21, 2026
…py TypeError, overlapping exceptions, lazy logging)

Co-authored-by: ThePlenkov <6381507+ThePlenkov@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ThePlenkov/hindsight/sessions/dd574a88-53a3-4f9e-bba7-5a40b0eddb99
Copy link
Collaborator

@nicoloboschi nicoloboschi left a comment

Choose a reason for hiding this comment

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

Great idea, LGTM

@nicoloboschi nicoloboschi merged commit 8ce06e3 into vectorize-io:main Mar 23, 2026
nicoloboschi added a commit that referenced this pull request Mar 23, 2026
…nt build

progenitor-impl-0.11.2 panics with `assertion failed: response_types.len() <= 1`
when an endpoint declares more than one response type. PR #643 added
`responses={504: ...}` to the reflect decorator, which injected a second
response type into the generated OpenAPI spec and broke the Rust client build.

Remove the `responses=` kwarg — the 504 is still raised at runtime via
JSONResponse(status_code=504), it just won't appear in the OpenAPI schema.
Regenerate openapi.json accordingly.
nicoloboschi added a commit that referenced this pull request Mar 23, 2026
…accept_with() enrichment (#650)

* test: add unit tests for pg_trgm auto-detection and ValidationResult.accept_with() enrichment

Two recent PRs landed without dedicated tests:
- #626/#649 (pg_trgm fallback in EntityResolver): add 5 mocked unit tests
  covering the trigram→full fallback, single-check guarantee, and sticky
  downgrade behaviour.
- #639 (accept_with() enrichment): add 7 pure unit tests for the factory
  method plus 5 integration tests verifying the engine applies enriched
  contents (retain) and tags/tag_groups (recall) returned by validators.
  Also verifies RecallContext carries tag filter state.

* fix: remove 504 from reflect OpenAPI spec to fix progenitor Rust client build

progenitor-impl-0.11.2 panics with `assertion failed: response_types.len() <= 1`
when an endpoint declares more than one response type. PR #643 added
`responses={504: ...}` to the reflect decorator, which injected a second
response type into the generated OpenAPI spec and broke the Rust client build.

Remove the `responses=` kwarg — the 504 is still raised at runtime via
JSONResponse(status_code=504), it just won't appear in the OpenAPI schema.
Regenerate openapi.json accordingly.

* chore: sync generated files and ruff formatting (lint + docs skill)
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.

3 participants