fix: return hits for contextual HSG queries instead of timing out - #213
fix: return hits for contextual HSG queries instead of timing out#213vincenzopalazzo wants to merge 2 commits into
Conversation
Default openmemory_query runs HSG with no timeout, so a hung embed or vector writeback dropped the Streamable HTTP POST. Catch HSG failures as MCP tool errors, time out after 12s, and cap unpatterned factual dumps at k. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
hsg_query embedded across all 5 sectors, ran vector search, and then awaited
salience/feedback/waypoint/associative reinforcement (and, when regeneration
is on, an embed network call per hit) on the same request. That tail did not
fit inside the MCP fail-soft query budget, so a perfectly good search surfaced
as 'HSG search timed out after 8000ms'.
Extract the reinforcement tail into a helper and run it fire-and-forget AFTER
ranking + caching is done, so hsg_query returns the hits as soon as they are
ready. Fail-soft is untouched: a failed/timed-out query still returns a
readable tool error and never drops the StreamableHTTP session.
- TS: reinforce_query_hits(top, tids).catch(() => {})
- PY: asyncio.create_task(_reinforce_query_hits(top)) + done callback
- list/get/store/delete/factual behavior unchanged
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ac7aff9cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } finally { | ||
| if (timer) clearTimeout(timer); | ||
| // Timed-out work may still reject later; swallow so the HTTP | ||
| // session is not taken down by an unhandled rejection. | ||
| p.catch(() => {}); |
There was a problem hiding this comment.
When an embedding or vector-search promise never settles, Promise.race returns the timeout error but leaves p running; the added .catch only observes a later rejection and does not cancel it. Because the underlying hsg_query therefore never reaches its finally block, its active_queries slot remains occupied, and after env.max_active timed-out calls every subsequent contextual query is rejected by the rate limiter until the hung provider recovers or the process restarts.
Useful? React with 👍 / 👎.
| patterned = _has_fact_pattern(fp) | ||
| fact_limit = max(top_k, 32) if patterned else top_k | ||
| try: | ||
| facts = await asyncio.wait_for( |
There was a problem hiding this comment.
For a large or locked SQLite database, this timeout cannot fire: query_facts_at_time performs its entire query through synchronous db.fetchall at temporal_graph/query.py:38 without yielding, so asyncio.wait_for cannot regain control until the scan and sort have already completed. Consequently a slow factual or unified query still blocks the MCP server beyond OM_MCP_QUERY_TIMEOUT_MS, which defeats the fail-soft behavior this path introduces.
Useful? React with 👍 / 👎.
| _task = asyncio.create_task(_reinforce_query_hits(top)) | ||
| _task.add_done_callback(lambda t: t.exception()) |
There was a problem hiding this comment.
When completed queries include compressed memories whose on_query_hit must re-embed them, every query now creates a detached task that may remain pending on the embedding provider while new queries continue spawning more tasks. The previous awaited path provided backpressure, but there is now no semaphore, queue, or tracked task set, so sustained distinct queries can accumulate unbounded reinforcement work and overload the provider or process even though the MCP request itself returns promptly.
Useful? React with 👍 / 👎.
1 participant
What this fixes
The default MCP query is
type=contextual. It was failing live with the toolerror
HSG search timed out after 8000ms(the local fail-soft wrapper doingits job), so the default path returned no hits at all.
Root cause:
hsg_queryembeds across all 5 sectors and runs vector search,then awaits the post-hit reinforcement tail on the same request:
feedback EMA, salience bump, waypoint + associative propagation, and (when
regeneration is on) an embed network call per hit. That tail did not fit in
the MCP query budget, so a perfectly good search surfaced as a timeout.
What changed
Move the reinforcement tail off the request path.
hsg_querynow returns theranked hits as soon as ranking + caching is done and runs the writebacks
fire-and-forget. Each write stays independently guarded, so a failure in one
hit can never surface on the request path. Results are byte-for-byte the same.
packages/openmemory-js/src/memory/hsg.ts): extractedreinforce_query_hits(top, tids)and callreinforce_query_hits(top, tids).catch(() => {})beforecache.set+ return.packages/openmemory-py/src/openmemory/memory/hsg.py): extracted_reinforce_query_hits(top)and run it viaasyncio.create_task(...)(+done-callback to consume the exception) before
cache[cache_key]+ return.What's intentionally unchanged
tool error, never
Error POSTing to endpoint, and does not drop theStreamableHTTP session. That wrapper is local (not in upstream main) and was
added by the companion commit that stopped the default query from dropping
HTTP; this PR builds on top of it.
list/get/store/delete/ factual behavior unchanged. Unboundedfactual dumps remain capped if that patch is present.
Verification
npx tsc --noEmit-> exit 0npx vitest run(openmemory-js) -> 14 files / 65 tests passedpython3 -m py_compileonhsg.py+decay.py-> OKhsg_query, so behavior parity is by inspection.Out of scope
Issue #180 (not touching that file).