Skip to content

refactor(memory): retire legacy sync compatibility paths - #5246

Merged
senamakel merged 12 commits into
tinyhumansai:mainfrom
senamakel:migrate-memory-sync-tinycortex
Jul 28, 2026
Merged

refactor(memory): retire legacy sync compatibility paths#5246
senamakel merged 12 commits into
tinyhumansai:mainfrom
senamakel:migrate-memory-sync-tinycortex

Conversation

@senamakel

@senamakel senamakel commented Jul 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Remove legacy memory_sync compatibility modules whose implementation already lives in TinyCortex.
  • Import canonicalization and sync pipeline contracts directly from tinycortex.
  • Consolidate host configuration, audit, rebuild, and GitHub source adapters under openhuman::tinycortex.
  • Preserve the existing RPC, scheduler, credential, event, redaction, and product-policy surfaces.

Problem

  • memory_sync retained duplicate type funnels and thin source wrappers after the TinyCortex engine cutover.
  • Those compatibility paths obscured the ownership boundary and could drift from the crate-owned implementation.

Solution

  • Delete the duplicate canonicalization and pipeline-contract modules and update consumers to use the crate API.
  • Delete memory_sync/sources and expose the required OpenHuman-specific configuration adapters from the existing TinyCortex seam.
  • Retarget existing E2E and raw-coverage tests to the crate-backed public path without changing behavior.

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
  • Diff coverage ≥ 80% — changed lines are primarily import retargeting and adapter relocation; existing TinyCortex mocked sync and OpenHuman E2E coverage exercise the paths.
  • N/A: No feature rows were added, removed, or renamed in docs/TEST-COVERAGE-MATRIX.md.
  • N/A: No affected feature IDs; this is an ownership-boundary refactor with preserved behavior.
  • No new external network dependencies introduced (mock backend used per Testing Strategy)
  • N/A: No release-cut user surface changed.
  • N/A: No linked issue was supplied for this migration slice.

Impact

  • Core-only architecture cleanup; no desktop, mobile, web, CLI, persistence-format, security-policy, or user-visible behavior change.
  • Removes duplicate host code while preserving TinyCortex sync taint, budget, status, audit, and rebuild behavior.

Related

  • Closes: N/A
  • Follow-up PR(s)/TODOs: Continue provider normalization/post-processing migration, beginning with Gmail.

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: migrate-memory-sync-tinycortex
  • Commit SHA: d0fc4069e

Validation Run

  • N/A: No frontend files changed; pnpm --filter openhuman-app format:check is not applicable.
  • N/A: No TypeScript files changed; pnpm typecheck is not applicable.
  • Focused tests: TinyCortex composio_sync_mock — 12 passed; OpenHuman cargo check --tests passed.
  • Rust fmt/check (if changed): cargo fmt --all -- --check; cargo check --lib.
  • N/A: Tauri files were not changed.

Validation Blocked

  • command: Initial filtered cargo test --lib baseline
  • error: Stopped after more than 15 minutes of resource-bound full default-feature test-harness code generation; no compile error occurred.
  • impact: Replaced with passing cargo check --tests, crate mocked sync tests, and post-merge cargo check --lib.

Behavior Changes

  • Intended behavior change: None; remove redundant compatibility ownership paths.
  • User-visible effect: None.

Parity Contract

  • Legacy behavior preserved: Canonicalization, sync outcomes, audit records, rebuilds, and GitHub source dispatch retain their existing TinyCortex implementations.
  • Guard/fallback/dispatch parity checks: Existing raw coverage and memory sync E2E imports were retargeted; TinyCortex mocked Composio suite passed all 12 cases.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): None found.
  • Canonical PR: This PR.
  • Resolution (closed/superseded/updated): N/A.

Summary by CodeRabbit

  • Refactor

    • Switched memory ingest canonicalization (chat/document/email) to TinyCortex types across pipelines and tools.
    • Updated sync auditing, cost estimation, and tree rebuild to use TinyCortex-backed helpers.
    • Removed legacy memory_sync compatibility modules (canonicalize/sources/traits).
  • Bug Fixes

    • Improved scheduling behavior when persisted audit logs are unavailable by avoiding fallback to unknown cadence.
  • Documentation

    • Updated memory sources/canonicalization references to reflect TinyCortex types.
  • Tests

    • Updated end-to-end and integration tests to use the unified TinyCortex interfaces.

@senamakel
senamakel requested a review from a team July 28, 2026 11:54

@greptile-apps greptile-apps 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.

senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

The PR migrates canonicalization and synchronization consumers from legacy memory_sync facades to TinyCortex modules, adds host-side synchronization exports, updates scheduler audit handling, removes obsolete compatibility modules and traits, and adjusts tests and documentation.

TinyCortex memory migration

Layer / File(s) Summary
Canonicalization type and pipeline wiring
src/bin/..., src/openhuman/memory/..., src/openhuman/memory_tree/..., src/openhuman/orchestration/..., tests/...
Canonical chat, document, and email types and helpers now come directly from TinyCortex.
Synchronization adapter exports and consumers
src/openhuman/tinycortex/..., src/openhuman/memory_sources/..., src/openhuman/memory_sync/...
Audit, cost, coverage, rebuild, GitHub sync, and related types are exposed through the TinyCortex adapter and used by synchronization callers.
Fallible audit handling in schedulers
src/openhuman/memory_sync/composio/periodic.rs, src/openhuman/memory_sync/workspace/periodic.rs
Schedulers preserve in-memory cadence decisions and skip sources when persisted audit data is unavailable.
Legacy module removal and test adaptation
src/openhuman/memory_sync/..., tests/raw_coverage/..., tests/json_rpc_e2e.rs
Legacy facades and pipeline traits are removed, while tests use TinyCortex outcomes, direct struct construction, and updated error text.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

Possibly related PRs

Suggested labels: rust-core, memory

Suggested reviewers: m3ga-mind

Poem

I hop through imports, neat and bright,
TinyCortex guides the bytes just right.
Old facades fade into the sun,
Sync helpers gather, one by one.
Tests bloom green beneath the moon.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 51.85% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: removing legacy memory_sync compatibility paths in favor of TinyCortex-backed APIs.

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

@coderabbitai coderabbitai Bot added memory Memory store, memory tree, recall, summarization, and embeddings in src/openhuman/memory/. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. labels Jul 28, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/openhuman/tinycortex/sync.rs`:
- Around line 72-80: The read_audit_log flow currently conflates audit-read
failures with an empty log; add a fallible or explicit-status audit-read variant
in the tinycortex sync API while preserving successful entries, and update the
periodic scheduler’s audit handling around its persisted-cadence check to skip
automatic sync when the audit fallback is unavailable instead of treating it as
no prior syncs.
- Around line 65-70: Add grep-friendly structured lifecycle diagnostics to
append_audit_entry and the related coverage, rebuild, and run_github_sync
host-sync wrappers. Log bounded entry and successful exit events, plus
external-call, branch, retry/timeout, and error outcomes where applicable;
extend run_github_sync beyond its current entry-only logging. Use redacted or
aggregated fields and avoid raw scopes, identifiers, or other PII.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b8a97d21-ac57-44af-a518-d43ed7dfc492

📥 Commits

Reviewing files that changed from the base of the PR and between 539c444 and fec734d.

📒 Files selected for processing (35)
  • src/bin/library_profile/scenarios/memory_ingest.rs
  • src/bin/slack_backfill.rs
  • src/openhuman/agent/harness/archivist/tree_ingest.rs
  • src/openhuman/agent_meetings/ops.rs
  • src/openhuman/memory/ingest_pipeline.rs
  • src/openhuman/memory/query/ingest_document.rs
  • src/openhuman/memory/read_rpc_tests.rs
  • src/openhuman/memory/sync_pipeline_e2e_tests.rs
  • src/openhuman/memory/tree_e2e_tests.rs
  • src/openhuman/memory_sources/README.md
  • src/openhuman/memory_sources/rpc.rs
  • src/openhuman/memory_sources/sync.rs
  • src/openhuman/memory_sync/canonicalize/README.md
  • src/openhuman/memory_sync/canonicalize/mod.rs
  • src/openhuman/memory_sync/composio/periodic.rs
  • src/openhuman/memory_sync/mod.rs
  • src/openhuman/memory_sync/sources/audit.rs
  • src/openhuman/memory_sync/sources/github.rs
  • src/openhuman/memory_sync/sources/mod.rs
  • src/openhuman/memory_sync/sources/rebuild.rs
  • src/openhuman/memory_sync/traits.rs
  • src/openhuman/memory_sync/workspace/periodic.rs
  • src/openhuman/memory_sync/workspace/watcher.rs
  • src/openhuman/memory_tree/retrieval/benchmarks.rs
  • src/openhuman/memory_tree/retrieval/integration_tests.rs
  • src/openhuman/memory_tree/tree/rpc.rs
  • src/openhuman/orchestration/effect_executor.rs
  • src/openhuman/tinycortex/mod.rs
  • src/openhuman/tinycortex/sync.rs
  • tests/agent_retrieval_e2e.rs
  • tests/memory_artifacts_e2e.rs
  • tests/memory_fast_retrieve_e2e.rs
  • tests/memory_sync_pipeline_e2e.rs
  • tests/raw_coverage/memory_raw_coverage_e2e.rs
  • tests/raw_coverage/memory_threads_raw_coverage_e2e.rs
💤 Files with no reviewable changes (8)
  • src/openhuman/memory_sync/canonicalize/README.md
  • src/openhuman/memory_sync/traits.rs
  • src/openhuman/memory_sync/sources/github.rs
  • src/openhuman/memory_sync/sources/mod.rs
  • src/openhuman/memory_sync/sources/rebuild.rs
  • src/openhuman/memory_sync/canonicalize/mod.rs
  • src/openhuman/memory_sync/sources/audit.rs
  • src/openhuman/memory_sync/mod.rs

Comment thread src/openhuman/tinycortex/sync.rs
Comment thread src/openhuman/tinycortex/sync.rs Outdated

@greptile-apps greptile-apps 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.

senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@greptile-apps greptile-apps 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.

senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 28, 2026

@greptile-apps greptile-apps 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.

senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b5d3a82b67

ℹ️ 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".

Comment thread src/openhuman/memory_sync/composio/periodic.rs Outdated

@greptile-apps greptile-apps 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.

senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 28, 2026

@greptile-apps greptile-apps 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.

senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@senamakel senamakel self-assigned this Jul 28, 2026

@greptile-apps greptile-apps 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.

senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@greptile-apps greptile-apps 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.

senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@greptile-apps greptile-apps 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.

senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/openhuman/memory_sync/composio/periodic.rs`:
- Around line 451-460: The scheduler’s fail-closed audit behavior lacks
regression coverage. In src/openhuman/memory_sync/composio/periodic.rs:451-460,
test that audit I/O failure records unavailable state rather than an empty
audit; in src/openhuman/memory_sync/composio/periodic.rs:523-537, verify unknown
cadence skips sync when unavailable but preserves first-sync behavior for a
readable empty log; in src/openhuman/memory_sync/workspace/periodic.rs:184-193,
test I/O failure marks workspace audit unavailable; and in
src/openhuman/memory_sync/workspace/periodic.rs:202-216, verify unknown cadence
excludes the source only when unavailable, not for a readable empty log.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bb0a7f63-3eb6-496e-89da-0a3c3971553c

📥 Commits

Reviewing files that changed from the base of the PR and between fec734d and 56903ec.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • src/openhuman/memory/README.md
  • src/openhuman/memory_sync/composio/periodic.rs
  • src/openhuman/memory_sync/workspace/periodic.rs
  • src/openhuman/tinycortex/mod.rs
  • src/openhuman/tinycortex/sync.rs
  • tests/json_rpc_e2e.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/openhuman/tinycortex/mod.rs

Comment thread src/openhuman/memory_sync/composio/periodic.rs Outdated

@greptile-apps greptile-apps 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.

senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@senamakel
senamakel merged commit 4ce2a8c into tinyhumansai:main Jul 28, 2026
19 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

memory Memory store, memory tree, recall, summarization, and embeddings in src/openhuman/memory/. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant