Skip to content

Extend default memory pre-turn wait to five seconds - #7346

Merged
senamakel merged 1 commit into
tinyhumansai:mainfrom
senamakel:memory-recall-timeout-5s-clean
Oct 10, 2026
Merged

senamakel merged 1 commit into
tinyhumansai:mainfrom
senamakel:memory-recall-timeout-5s-clean

Conversation

@senamakel

Copy link
Copy Markdown
Member

Summary

Increase OpenHuman's default memory pre-turn wait from 1,500 ms to 5,000 ms. Explicit pre_turn_timeout_ms settings still take precedence. Update the configuration example and spec to match.

The companion TinyMemory eval PR (tinyhumansai/tinymemory#254) mirrors this deadline. In a fresh 100-document live CortexDB run at 5 seconds, the lexical answer completed in 2.08 seconds and the paraphrase miss in 1.78 seconds, with no timeout. The paraphrase needs a separate retrieval-quality fix: its correct document ranked 23rd in a direct 100-event recall. Earlier 1.5-second scale runs often timed out, but these are different collections and do not establish a paired timeout-rate change.

Behavior and API

No public API change. The default wait is now 5 seconds; explicitly configured values are unaffected. A timed-out memory task still follows the existing background behavior.

Validation

  • Regression test default_pre_turn_waits_five_seconds_for_memory failed before the default change (1,500 versus 5,000).
  • cargo fmt --all -- --check — passed.
  • scripts/ci-cancel-aware.sh cargo test -p openhuman --lib default_pre_turn_waits_five_seconds_for_memory — compiling; result will be added when complete.

Related: tinyhumansai/tinymemory#251

Co-authored-by: Medulla <medulla@tinyhumans.ai>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-10T19:05:17.987776Z 67e6d37 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@senamakel
senamakel merged commit bc59324 into tinyhumansai:main Oct 10, 2026
9 of 13 checks passed
@coderabbitai

coderabbitai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e9f3a6d4-fff4-489e-b9ff-47e3388fd154

📥 Commits

Reviewing files that changed from the base of the PR and between bd5632c and 67e6d37.


📒 Files selected for processing (3)
  • crates/openhuman-core/src/config/schema/memory.rs
  • crates/openhuman-core/src/config/schema/memory_tests.rs
  • docs/specs/memory-v2.md

 ________________________________________________________________________
< This is like dependency injection, except the dependency is suffering. >
 ------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@tinysweeper

tinysweeper Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Changes requested
Priority: high
Reviewed head: 67e6d375ee99
Updated: 1791659534 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 1
Tests 1 Noted findings 0
Documentation 1 Resolved findings 0
Configuration 0 Pending checks/questions 4

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

  • high · e2e · End-to-end job `Storage e2e on MongoDB` will not run on this change — `Storage e2e on MongoDB` in `.github/workflows/storage-mongodb.yml` will not run for this pull request: its workflow's `paths:` filter matches nothing this pull request changed. Th (\.github/workflows/storage\-mongodb\.yml:27)

Pending checks: Rust E2E (mock backend), Build Playwright E2E Artifact, E2E (Playwright / web lane), Desktop E2E (full suite, 3 OS)

Before merge

  • Address End-to-end job `Storage e2e on MongoDB` will not run on this change (\.github/workflows/storage\-mongodb\.yml).
  • Wait for Rust E2E (mock backend), Build Playwright E2E Artifact, E2E (Playwright / web lane), Desktop E2E (full suite, 3 OS).
Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 3 files; 0 findings. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change only increases the default pre-turn memory recall timeout and adds a matching unit test. It looks safe to merge. 1 file was not security-reviewed: docs/specs/memory-v2.md (prose or tabular data). _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change raises the default pre-turn recall timeout from 1500 ms to 5000 ms, updating the constant, the doc-comment example and the spec together, and pins it with a new test asserting `MemoryConfig::default().recall.pre_turn_timeout_ms == 5_000`. That test would fail if the default silently reverted or drifted, which is exactly the pin a default-value change needs. Docs and code are consistent. Safe to merge. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change does exactly what the description says: raises DEFAULT_PRE_TURN_TIMEOUT_MS to 5000, updates the doc comment, the spec example, and adds a regression test. Description matches the diff; looks safe to merge. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

e2e

  • Conclusion: Failure
  • Scope reviewed: all assigned evidence
  • Lane summary: This change only raises the default `pre_turn_timeout_ms` from 1500 to 5000 in the memory recall config, with the doc example updated to match and a unit test asserting the new default. There is no route, command, flag, screen or persisted format newly introduced; the value was already user-configurable and the default is internal tuning, so no end-to-end test is required and none is missing. The change looks safe to merge. Waiting on end-to-end jobs: `Rust E2E (mock backend)`, `Build Playwright E2E Artifact`, `E2E (Playwright / web lane)`, `Desktop E2E (full suite, 3 OS)`. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._
  • Unresolved questions/checks: Rust E2E (mock backend), Build Playwright E2E Artifact, E2E (Playwright / web lane), Desktop E2E (full suite, 3 OS)
  • Evidence: \.github/workflows/storage\-mongodb\.yml — End-to-end job `Storage e2e on MongoDB` will not run on this change
Evidence and run details
  • Models: gpt-5.6-luna, glm-5.3-flash
  • Spend: $0.003883
  • Tokens: 68625 input · 2219 output · 9787 cached · 0 embedding
Head State Pass summary
67e6d375ee99 changes requested 1 active finding(s), 0 resolved finding(s) (at 1791659534)

tinysweeper 0.1.0

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Requesting changes: 1 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

Findings not posted inline

  • End-to-end job Storage e2e on MongoDB will not run on this change (.github/workflows/storage-mongodb.yml): e2e-not-triggered

    Storage e2e on MongoDB in .github/workflows/storage-mongodb.yml will not run for this pull request: its workflow's paths: filter matches nothing this pull request changed. The change is merged without its end-to-end suite having seen it.

             $0.0039 · 68,625 in / 2,219 out · 9,787 cached (14%) · gpt-5.6-luna, glm-5.3-flash
critique:    $0.0019 · 27,453 in / 689 out   · 4,141 cached (15%) · gpt-5.6-luna
security:    $0.0018 · 24,058 in / 518 out   · 1,870 cached (8%)  · gpt-5.6-luna
tests:       $0.0000 · 4,528 in  / 100 out   · 64 cached (1%)     · glm-5.3-flash
description: $0.0000 · 4,184 in  / 54 out    · 1,536 cached (37%) · glm-5.3-flash
e2e:         $0.0000 · 5,425 in  / 211 out   · 2,048 cached (38%) · glm-5.3-flash

@tinysweeper tinysweeper Bot added the priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition. label Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant