Repository navigation
refactor(memory_sync): convert ClickUp to the generic sync orchestrator (Refs #3333, stacked on #3860) - #3873
Merged
senamakel merged 9 commits intoJun 22, 2026
Conversation
…otion (Refs tinyhumansai#3333) First increment of tinyhumansai#3333: replace each provider's bespoke ~600-1000-line sync() loop with one generic run_sync orchestrator over a slim IncrementalSource primitive trait, and convert Notion as the proof. The orchestrator owns the control flow every provider copy-pasted (and which is how the tinyhumansai#3304 page-granular cap bug landed in five providers): budget check -> preamble -> per-scope page loop bounded by ItemCap::max_pages -> sync_depth_days window -> dedup -> ItemCap::clamp_batch (the precise EXACT-N cap) -> ingest -> record -> cursor advance/hold -> save state -> SyncOutcome. Providers implement only primitives: preamble (resolve identity / list scopes), fetch_page, item_dedup_key, item_sort_ts, depth_floor, ingest. SyncScope unifies flat (gmail/github/notion/linear: one implicit scope) and nested (clickup workspaces, slack channels: one scope per container) shapes behind one scope loop, so the nested providers slot in without a control-flow change. Notion now delegates sync() to run_sync(&NotionSource); its dedup key ({page_id}@{edited_time}), RFC3339 depth floor, budget-counted NOTION_GET_PAGE_MARKDOWN body fetch, and bounded-concurrency ingest move verbatim into notion/source.rs — identical cursor/dedup/budget/window behavior. gmail/github/linear/clickup/slack keep their existing sync(). Tests: generic orchestrator unit tests via a test-only FakeSource (max_items caps to EXACTLY N, no-cap full page, sync_depth_days filters, nested scopes share one cap, budget-exhaustion skip, pure select_pending dedup/cursor); Notion's existing cap e2e now rides the shared path; new Notion sync_depth_days e2e. Co-Authored-By: Claude <noreply@anthropic.com>
…mble/fetch errors) Add three orchestrator unit tests via FakeSource so the diff-cover gate sees the save-and-propagate / short-circuit branches: empty scopes -> skip outcome, preamble error -> propagate (after persisting spent budget), fetch_page error -> propagate. Co-Authored-By: Claude <noreply@anthropic.com>
Addresses Codex review (P2): in the orchestrator, fetch_page now records the page request against the daily budget for any *completed* round-trip — incl. one the provider reports as successful==false — before converting it to a sync error. A transport error (no round-trip) is still not counted. This restores parity with the per-provider loops, where a broken/unauthorized connection could not make unlimited billable failed page calls without hitting the per-day cap. fetch_page gains a &mut SyncState param (the primitive records the calls it makes, like ingest's body fetch already does); the orchestrator no longer records per page. Adds a unit test pinning that a provider-reported page failure consumes exactly one budget request. Co-Authored-By: Claude <noreply@anthropic.com>
…chestrator (Refs tinyhumansai#3333) Follow-up to tinyhumansai#3852: migrate the two flat task providers onto run_sync. GitHubProvider::sync and LinearProvider::sync are now one-line delegates; each provider's bespoke ~300/520-line loop moves to slim IncrementalSource primitives in github/source.rs and linear/source.rs (preamble identity resolve, fetch_page, item_dedup_key/item_sort_ts, ingest). Behavior-preserving — the existing per-provider cap/depth e2e stay green on the shared path. Adds one orchestrator hook for GitHub's server-side depth window: IncrementalSource::server_side_depth() (default false). When true the orchestrator skips its client-side timestamp truncation and the provider injects the window in fetch_page — GitHub keeps its first-sync-only `updated:>{date}` query qualifier exactly as before (incremental syncs are bounded by `updated:>{cursor}`). Linear uses the default client-side window. Both are flat-but-identity-scoped: preamble resolves the login / viewer id and carries it as the single SyncScope's id, which fetch_page reads back. GitHub paginates by 1-indexed page (opaque cursor = next page number); Linear by GraphQL endCursor. GitHub ingests sequentially (parity); Linear keeps its bounded-concurrency buffered ingest. Note: GitHub now scans the whole boundary page before stopping (matching notion/linear/clickup) instead of breaking mid-page — slightly more thorough (ingests old-but-never-synced items on the boundary page); no test regresses. Tests: orchestrator server_side_depth unit test + github/linear dedup-key tests + the moved linear buffered-ingest tests; existing github cap/no-cap/ depth + linear cap e2e now ride run_sync; new linear sync_depth_days e2e. clickup, slack, gmail remain on their existing sync(). Co-Authored-By: Claude <noreply@anthropic.com>
…estrator
CI caught a parity break: the orchestrator emitted generic results_fetched /
results_persisted detail keys, but github/linear historically emitted
issues_fetched / issues_persisted (memory_sync_tree_round21 asserts them).
Add IncrementalSource::detail_noun() (default "results", notion's original)
and override to "issues" for github/linear; the orchestrator now builds the
{noun}_fetched / {noun}_persisted keys dynamically. The SyncOutcome.details
blob is diagnostic-only (nothing reads these keys in production), but this
keeps each provider's exact historical shape so no test or UI consumer shifts.
(The wallet::ops keychain test that also failed in that CI run is an unrelated
pre-existing flake under cargo-llvm-cov, not touched by this change.)
Co-Authored-By: Claude <noreply@anthropic.com>
…or (Refs tinyhumansai#3333) Third increment of tinyhumansai#3333, and the first NESTED provider on the orchestrator. ClickUpProvider::sync is now a one-line delegate; the bespoke per-workspace loop moves to slim IncrementalSource primitives in clickup/source.rs. ClickUpSource::preamble resolves the authorized user id + visible workspaces and returns one SyncScope per workspace (carrying the workspace id); the shared user id is stashed in a OnceLock on the source so every per-workspace fetch_page reads it back. This exercises the SyncScope nested path end-to-end against real code, de-risking slack's two-level nesting. Uses the trait hooks added earlier with no new orchestrator changes: - depth_floor override → epoch-millis floor (date_updated is an epoch-ms string) - hold_cursor_on_ingest_failure() = false (ClickUp advances despite per-item failures) - detail_noun() = "tasks" (preserves the tasks_fetched/tasks_persisted details) The single-cursor model fits (ClickUp tracks one newest-date cursor across all workspaces); pagination is 1-indexed page (opaque cursor = next page number); bounded-concurrency buffered ingest is preserved. Behavioral parity — the existing clickup cap e2e stays green on the shared path. Tests: moved clickup buffered-ingest concurrency/failure tests + dedup-key + epoch-ms depth-floor unit tests; existing clickup cap e2e now rides run_sync; new clickup sync_depth_days e2e (epoch-ms window). slack and gmail remain on their existing sync() (next follow-ups). Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 15 minutes and 31 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
12 tasks done
# Conflicts: # src/openhuman/memory_sync/composio/providers/github/source.rs
This was referenced Jun 22, 2026
Closed
senamakel
pushed a commit
to nocstah/openhuman
that referenced
this pull request
Sep 11, 2026
…or (Refs tinyhumansai#3333, stacked on tinyhumansai#3860) (tinyhumansai#3873)\n\n
senamakel
pushed a commit
to nocstah/openhuman
that referenced
this pull request
Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
run_syncorchestrator (from refactor(memory_sync): generic Composio sync orchestrator + convert Notion (Refs #3333) #3852).ClickUpProvider::syncis now a one-line delegate; the bespoke per-workspace loop moves to slimIncrementalSourceprimitives inclickup/source.rs(provider.rs shrinks ~730 lines).SyncScopenested path end-to-end against real code — de-risking slack's two-level nesting — with no new orchestrator changes, only the trait hooks added in refactor(memory_sync): convert GitHub + Linear to the generic sync orchestrator (Refs #3333, stacked on #3852) #3860.Problem
ClickUp is the gentle nested case: it fans out over workspaces (workspaces → tasks) but still tracks a single cursor (newest
date_updatedacross all workspaces). Converting it validates thatSyncScope+ the existing hooks cover real nested fan-out before the harder slack conversion (which needs per-channel cursors and is its own follow-up).Solution
ClickUpSource::preambleresolves the authorized user id + visible workspaces and returns oneSyncScopeper workspace (scope id = workspace id). The shared user id is resolved once and stashed in aOnceLockon the source so every per-workspacefetch_pagecan read it back to scope tasks to that assignee. Preamble preserves the original's budget re-check (exhausted right after the user-id probe → skip the workspaces call → no-op outcome).Rides the trait hooks added in #3860 with zero orchestrator edits:
depth_flooroverride → epoch-millis floor (date_updatedis an epoch-ms string, so the lexicographic compare needs an epoch-ms floor, not RFC3339).hold_cursor_on_ingest_failure() = false(ClickUp advances its cursor even on per-item ingest failures).detail_noun() = "tasks"(preserves thetasks_fetched/tasks_persisteddetailskeys).Pagination is 1-indexed
page(opaque cursor = next page number); a short page ends the workspace. Bounded-concurrency buffered ingest is preserved.Submission Checklist
item_dedup_key+ epoch-msdepth_floorunit tests; existing clickup cap e2e now ridesrun_sync; newclickup_sync_depth_days_filters_old_taskse2e.preamble→ per-workspacefetch_page→ingestend-to-end). Rancargo testforcomposio::providers(465 lib tests) +--test memory_sync_providers_raw_coverage_e2e(15) locally — all green.N/A: behaviour-only refactor.## Related—N/A: behaviour-only refactor.N/A: internal sync refactor.Closes #NNN: increment of the multi-PR Generic Composio sync orchestrator so every toolkit enforces max_items and sync_depth_days #3333 migration, references (not closes) the issue.Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check—N/A: no frontend changespnpm typecheck—N/A: no frontend changescargo test --lib -- composio::providers(465 passed) +cargo test --test memory_sync_providers_raw_coverage_e2e -- --test-threads=1(15 passed)cargo fmt --checkclean;cargo check+cargo clippy --libclean on changed filesN/A: no Tauri changesValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
Duplicate / Superseded PR Handling