refactor(memory): complete TinyCortex engine migration - #4794
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR migrates OpenHuman memory, embeddings, ingestion, retrieval, queue, tree, and synchronization paths to TinyCortex and TinyAgents adapters. It also updates Cargo patching, provider backfill flows, session memory connections, endpoint detection, and related tests. ChangesTinyCortex migration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![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: b3880460c0
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db76303ba9
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?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: a8acb19e30
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67b1b84c55
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e03b9f31fb
ℹ️ 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".
| @@ -348,50 +348,38 @@ impl Memory for UnifiedMemory { | |||
| &self, | |||
| namespace: Option<&str>, | |||
| category: Option<&MemoryCategory>, | |||
| _session_id: Option<&str>, | |||
| session_id: Option<&str>, | |||
| ) -> anyhow::Result<Vec<MemoryEntry>> { | |||
| let ns = normalize_namespace(namespace); | |||
There was a problem hiding this comment.
When callers list a namespace containing characters that are sanitized on write (for example team alpha/#1, which upsert_document stores as team_alpha/_1), this new direct SQL path queries the unsanitized normalize_namespace value. The previous list_documents(Some(ns)) call sanitized internally, so Memory::list(Some(...)) now returns an empty list for valid namespaces with spaces, #, dots, etc.; apply UnifiedMemory::sanitize_namespace before binding the namespace, matching the write path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a42fa76dd
ℹ️ 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".
| .tick( | ||
| &pipeline_id, | ||
| &memory_config, | ||
| &source_sync_context(memory, config, false), |
There was a problem hiding this comment.
When gmail-backfill-3d calls run_gmail_backfill, this local=false context supplies no LocalDocumentSink; the Gmail pipeline therefore stores via the SkillDocSink path (HostSyncAdapter::store → store_skill_sync) instead of the old ingest_page_into_memory_tree path. Operators running the backfill get records_ingested but chunks.db/content files remain empty, and the existing integrity check just verifies zero chunks. Use a memory-tree/local-document sink for this backfill or update the CLI semantics.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 751500ca69
ℹ️ 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".
| .store_skill_sync( | ||
| &document.namespace_skill_id, | ||
| &document.connection_id, |
There was a problem hiding this comment.
When a Gmail/Slack/etc. Composio sync stores through this SkillDocSink, it writes memory_docs rows in skill-* namespaces via store_skill_sync, but composio_delete_connection(clear_memory=true) only enumerates MemoryCleanupTargets that delete memory-tree chunks and never clears these namespace documents. In that delete-with-clear-memory scenario, newly synced emails/messages/docs can remain listed/searchable after the connection is removed; include the skill-* documents for the connection in the cleanup path as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9439b9487
ℹ️ 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".
| source.max_items = max_items; | ||
| source.sync_depth_days = sync_depth_days; |
There was a problem hiding this comment.
When run_composio_connection or the provider sync default calls this helper with max_items/sync_depth_days as None, these assignments overwrite any registered source caps (and even the fallback defaults created above) with None. That makes manual, trigger-driven, and connection-created Composio syncs run uncapped instead of honoring the user's per-source limits; only replace these fields when an override is actually present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a64e623367
ℹ️ 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".
| source_id, | ||
| source_kind, | ||
| scope, | ||
| usize::MAX, |
There was a problem hiding this comment.
When memory_tree_cover_window is called for a broad window (for example the morning-brief path over a busy 24h period) with a small or default return limit, this now asks TinyCortex for usize::MAX hits and only truncates after the full response is materialized. The previous implementation bounded the DB scan to MAX_WINDOW_CHUNKS/per-source caps before building the cover, so large workspaces can now scan and allocate over every in-window chunk just to return the first 200 results; pass the caller/default cap (or restore an internal scan cap) into the engine instead of disabling the limit here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e06a1d8473
ℹ️ 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".
| let outcome = openhuman_core::openhuman::tinycortex::run_gmail_backfill( | ||
| &connection_id, | ||
| &query, | ||
| cli.max_pages as usize, | ||
| cli.page_size as usize, | ||
| &config, | ||
| ) |
There was a problem hiding this comment.
When gmail-backfill-3d --include-spam-trash is used, the new path only omits the -in:spam -in:trash query filter and then calls run_gmail_backfill, which has no way to pass the Gmail/Composio include_spam_trash flag. The previous implementation set args["include_spam_trash"] = true; without that boolean, Gmail fetches still use the default exclusion, so this CLI option no longer includes Spam/Trash despite reporting that it will.
Useful? React with 👍 / 👎.
| .tick( | ||
| &pipeline_id, | ||
| &memory_config, | ||
| &source_sync_context(memory, config, false), |
There was a problem hiding this comment.
When slack-backfill --use-search runs, this passes local=false, and source_sync_context therefore omits the LocalDocumentSink/summariser that feed memory-tree ingestion. The old run_backfill_via_search path grouped search results and called ingest_page_into_memory_tree, so operators using search backfill populated tree chunks; with only the skill-document sink available, the command can report records while memory-tree retrieval remains empty for those backfilled Slack messages.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d944413508
ℹ️ 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".
| .map_err(|e| format!("kv_set_global: {e}"))?; | ||
| Ok(()) | ||
| pub async fn kv_set_global(&self, key: &str, value: &serde_json::Value) -> Result<(), String> { | ||
| self.tinycortex_kv()?.set_global(key, value) |
There was a problem hiding this comment.
When openhuman.memory_kv_set or other KV callers persist user-provided keys/values, this direct delegation bypasses the host safety checks that previously rejected secret/PII-like keys/namespaces and sanitized JSON payloads before writing kv_global/kv_namespace (the unchanged kv_set_* tests still assert that contract). As a result, a value containing bearer tokens or a key like ssn-... can be stored and later recalled from memory instead of being redacted/rejected; keep the host safety gate here or make the crate call enforce the same contract before writing.
Useful? React with 👍 / 👎.
…te-ports # Conflicts: # tests/raw_coverage/memory_threads_raw_coverage_e2e.rs # vendor/tinyagents
The reshape that slims a verbose `GMAIL_FETCH_EMAILS` payload into one record per message — body pre-rendered as markdown — lost its caller when the sync moved into TinyCortex (tinyhumansai#4794 deleted `gmail/source.rs`, which called it). Since then the sync has been storing the raw provider payload as document content. The pipeline lives below this crate and cannot call the reshape, so wrap the executor it fetches through: every successful page is reshaped before the pipeline turns it into a document, in the same order the pre-migration sync used (pin the response-level `markdownFormatted` per message, then slim the envelope). Error responses pass through untouched so the sync's own error handling still sees provider diagnostics. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
1 participant
Summary
Config, providers, credentials, event publication, scheduling, RPC/tools, and product policy.Problem
Solution
UnifiedMemory, health diagnostics, NLP sidecar integration, and policy remain host-owned by design.Submission Checklist
Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/tinycortex-crate-portsb3880460c0ccc42e68316ac08125845ff2cd2e4cValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckGGML_NATIVE=OFF cargo check --testspassed across all host targets; post-dependencycargo check --libpassed.glib-2.0.pcbefore project code compilation.Validation Blocked
command:cargo check --manifest-path app/src-tauri/Cargo.tomlerror:local system packageglib-2.0is absent from pkg-config search pathsimpact:environment-only Linux desktop prerequisite; CI runners install the required GTK/GLib packages. The dependency graph resolves to one vendored tinyagents crate before this native build step.Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes
Refactor