refactor(memory): retire legacy sync compatibility paths - #5246
Conversation
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
📝 WalkthroughWalkthroughChangesThe PR migrates canonicalization and synchronization consumers from legacy TinyCortex memory migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
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
📒 Files selected for processing (35)
src/bin/library_profile/scenarios/memory_ingest.rssrc/bin/slack_backfill.rssrc/openhuman/agent/harness/archivist/tree_ingest.rssrc/openhuman/agent_meetings/ops.rssrc/openhuman/memory/ingest_pipeline.rssrc/openhuman/memory/query/ingest_document.rssrc/openhuman/memory/read_rpc_tests.rssrc/openhuman/memory/sync_pipeline_e2e_tests.rssrc/openhuman/memory/tree_e2e_tests.rssrc/openhuman/memory_sources/README.mdsrc/openhuman/memory_sources/rpc.rssrc/openhuman/memory_sources/sync.rssrc/openhuman/memory_sync/canonicalize/README.mdsrc/openhuman/memory_sync/canonicalize/mod.rssrc/openhuman/memory_sync/composio/periodic.rssrc/openhuman/memory_sync/mod.rssrc/openhuman/memory_sync/sources/audit.rssrc/openhuman/memory_sync/sources/github.rssrc/openhuman/memory_sync/sources/mod.rssrc/openhuman/memory_sync/sources/rebuild.rssrc/openhuman/memory_sync/traits.rssrc/openhuman/memory_sync/workspace/periodic.rssrc/openhuman/memory_sync/workspace/watcher.rssrc/openhuman/memory_tree/retrieval/benchmarks.rssrc/openhuman/memory_tree/retrieval/integration_tests.rssrc/openhuman/memory_tree/tree/rpc.rssrc/openhuman/orchestration/effect_executor.rssrc/openhuman/tinycortex/mod.rssrc/openhuman/tinycortex/sync.rstests/agent_retrieval_e2e.rstests/memory_artifacts_e2e.rstests/memory_fast_retrieve_e2e.rstests/memory_sync_pipeline_e2e.rstests/raw_coverage/memory_raw_coverage_e2e.rstests/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
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 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".
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
src/openhuman/memory/README.mdsrc/openhuman/memory_sync/composio/periodic.rssrc/openhuman/memory_sync/workspace/periodic.rssrc/openhuman/tinycortex/mod.rssrc/openhuman/tinycortex/sync.rstests/json_rpc_e2e.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/openhuman/tinycortex/mod.rs
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Summary
memory_synccompatibility modules whose implementation already lives in TinyCortex.tinycortex.openhuman::tinycortex.Problem
memory_syncretained duplicate type funnels and thin source wrappers after the TinyCortex engine cutover.Solution
memory_sync/sourcesand expose the required OpenHuman-specific configuration adapters from the existing TinyCortex seam.Submission Checklist
docs/TEST-COVERAGE-MATRIX.md.Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
migrate-memory-sync-tinycortexd0fc4069eValidation Run
pnpm --filter openhuman-app format:checkis not applicable.pnpm typecheckis not applicable.composio_sync_mock— 12 passed; OpenHumancargo check --testspassed.cargo fmt --all -- --check;cargo check --lib.Validation Blocked
command:Initial filteredcargo test --libbaselineerror:Stopped after more than 15 minutes of resource-bound full default-feature test-harness code generation; no compile error occurred.impact:Replaced with passingcargo check --tests, crate mocked sync tests, and post-mergecargo check --lib.Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
Refactor
memory_synccompatibility modules (canonicalize/sources/traits).Bug Fixes
Documentation
Tests