Update OpenHuman integration - #779
Conversation
Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Warning Review limit reached
Next review available in: 29 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request updates Cargo workspace boundaries, vendored dependency setup, Rust CI coverage, Tokio runtime configuration, and provider response defaults. ChangesBuild configuration
Application runtime configuration
Provider response defaults
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
the
priority: p2
oxoxDev
left a comment
oxoxDev
left a comment
There was a problem hiding this comment.
The structural change is well explained at the site, which matters because it is the kind of thing a future reader would otherwise try to undo:
OpenHuman's modular dependencies (TinyBus, TinyMemory, TinyDocs, and TinyWallet) each own nested workspaces, which Cargo cannot resolve beneath another workspace root. The Tauri package under
frontend/src-tauriis therefore its own workspace.
That constraint is forced by the bump rather than chosen, and recording it in Cargo.toml where the [workspace] block used to be is the right place for it.
Scope of this review, stated plainly: two submodule pins move here, so the substance of the change is upstream code I have not read. I checked the shape of the integration — the workspace restructure, the lockfile split, and how this lands against what else is open — not OpenHuman's or TinyAgents' diffs.
0 major on what I can assess. 2 coordination findings, and the first needs a decision rather than a merge. Approving.
This and #772 make opposite decisions about frontend/src-tauri/
They conflict five ways, including a modify/delete:
CONFLICT (content): .github/workflows/ci.yml
CONFLICT (content): Cargo.lock
CONFLICT (content): Cargo.toml
CONFLICT (modify/delete): frontend/src-tauri/Cargo.toml deleted in #772, modified here
CONFLICT (content): frontend/test/e2e/desktop-connections.spec.ts
This PR invests in frontend/src-tauri/ — giving it its own workspace and a new 5,879-line Cargo.lock. #772 deletes it, with evidence: no CI lane builds it, npm run tauri:build packaged it only because the Tauri CLI searches subfolders, and its root-workspace membership is what dragged 148 packages — webkit2gtk and GTK — into the root lockfile.
The two are not symmetric, and I think they are mostly orthogonal:
- The root-workspace removal here is forced by the nested-workspace constraint and holds whether or not that directory survives.
- The deletion in #772 is independent of it — that app is a third desktop entry point nobody builds.
So the likely correct combined outcome is both: drop the root workspace and delete the stale app — in which case frontend/src-tauri/Cargo.lock should never be created, and roughly 5,879 lines of this diff are for a directory that is about to go. Resolving these conflicts mechanically will very likely produce the wrong answer in one direction or the other: taking this side resurrects an app #772 shows is dead, taking that side drops a restructure the bump requires.
Worth settling deliberately with @CodeGhost21 before either lands, rather than by merge order.
#777 cites both submodules, and both pins move here
#777 corrects a load-bearing claim — that openhuman "genuinely cannot be resumed here" — and its evidence is openhuman's own ToolPolicyDecision::RequireApproval doc plus TinyAgents' wrap_tool_retries_next_until_success pinning that wrap_tool may call next.run zero or more times.
Both of those pins move in this PR. The correction is very likely still true, and it is the kind of citation that quietly stops being accurate across a version bump. Since #777 is the PR that establishes four earlier workarounds rested on an unverified premise, it would be a poor outcome for its replacement premise to go stale the same way. Worth a quick re-check of those two references against the new pins.
Before merging: MERGEABLE against main and base is current, but the Desktop and Rust lanes are the ones to watch on a bump of this size — and frontend/src-tauri now having its own lockfile means CI must build it separately or it is unbuilt in a new way, which is the shape #772 is written about.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/harness/provider.rs`:
- Line 534: Add a module-local regression test near the existing tests for
model_response_from_payload that parses a valid payload and asserts the
resulting response.served_from_cache is false. Ensure the test runs under the
feature configuration used by src/harness.
🪄 Autofix
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 Plus
Run ID: b8c6bc05-e62e-4de1-8ca4-f33e31dd454c
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockfrontend/src-tauri/Cargo.lockis excluded by!**/*.locksrc-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/workflows/ci.ymlCargo.tomlfrontend/src-tauri/Cargo.tomlsrc/bin/opencompany.rssrc/harness/provider.rsvendor/openhumanvendor/tinyagents
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0416 · 31,375 in / 11,476 out · 15,417 cached (49%) · z-ai/glm-5.2
critique: $0.0252 · 12,113 in / 7,945 out · 4,793 cached (40%) · z-ai/glm-5.2
security: $0.0086 · 10,131 in / 1,394 out · 3,762 cached (37%) · z-ai/glm-5.2
tests: $0.0037 · 4,314 in / 1,006 out · 3,185 cached (74%) · z-ai/glm-5.2
description: $0.0040 · 4,817 in / 1,131 out · 3,677 cached (76%) · z-ai/glm-5.2
What this change touches5 files, +72 -39 across 5 components. It reaches 6 untouched components (60 graph nodes walked). 5 further components left out to keep the diagram readable. flowchart LR
n0[".github/workflows<br/>1 file +35 -13"]:::changed
n1["root<br/>1 file +10 -21"]:::changed
n2["src/bin<br/>1 file +20 -2"]:::changed
n3["frontend/src-tauri<br/>1 file +6 -3"]:::changed
n4["src/harness<br/>1 file +1 -0"]:::changed
n5["src/runtime<br/>4 files reached"]:::impacted
n6["src/store<br/>3 files reached"]:::impacted
n7["src/company<br/>2 files reached"]:::impacted
n8["src/harness<br/>2 files reached"]:::impacted
n9["src/ports<br/>2 files reached"]:::impacted
n10["src/server<br/>2 files reached"]:::impacted
n8 -->|9 refs| n9
n8 -->|6 refs| n7
n2 -->|4 refs| n5
n2 -->|3 refs| n6
n5 -->|3 refs| n9
n6 -->|3 refs| n9
n8 -->|3 refs| n5
n10 -->|3 refs| n9
n2 -->|2 refs| n7
n2 -->|2 refs| n8
n2 -->|2 refs| n9
n5 -->|2 refs| n6
n5 -->|2 refs| n8
n10 -->|2 refs| n8
n2 -->|1 ref| n10
n5 -->|1 ref| n7
n6 -->|1 ref| n5
n6 -->|1 ref| n7
n8 -->|1 ref| n6
n9 -->|1 ref| n7
n10 -->|1 ref| n5
n10 -->|1 ref| n6
n10 -->|1 ref| n7
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed. Grey: untouched, reached through an import or a call. Orange: has findings. Red: has a finding that blocks the merge.
Changed files
|
tinysweeper
Bot
added
priority: p3
Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Follow-up on the two coordination points from @oxoxDev's review:
The latest push also fixes the gated CI failure at its root by applying OpenHuman's stack requirement to the entire gated job, including its later runner-lane test invocation. |
2 participants
Summary
mainat2826259d0ModelResponsecontractWhy
Current OpenHuman split several dependencies into their own Cargo workspaces and added new direct submodule dependencies. The previous OpenCompany workspace caused Cargo to resolve inherited workspace fields against the wrong root, while the older TinyAgents pin and response initializer no longer matched OpenHuman's API. Agent-loop tests also overflowed Rust's default test-thread stack.
This change makes OpenCompany build and boot against the latest OpenHuman revision while preserving locked, explicit CI coverage for the server and desktop packages.
Validation
cargo test --lockedRUST_MIN_STACK=16777216 cargo test --locked --features openhuman,tinycortex --testscargo clippy --locked --no-deps --features openhuman,tinycortex --all-targets -- -D warningscargo fmt --all -- --checkcargo clippy --manifest-path src-tauri/Cargo.toml --locked --all-targets --no-deps -- -D warningscargo test --manifest-path src-tauri/Cargo.toml --lockedcargo clippy --manifest-path frontend/src-tauri/Cargo.toml --locked --all-targets --no-deps -- -D warningscargo test --manifest-path frontend/src-tauri/Cargo.toml --lockedopenhuman-demo-cowithopenhuman,tinycortex;/healthzreturned{"status":"ok"}and/specreported OpenHuman and TinyAgents enabledSummary by CodeRabbit
Bug Fixes
Performance & Reliability
Quality Improvements