feat: vendor direct TinyHumans dependencies - #60
Conversation
Update the wiki subproject to point to a new commit, incorporating the latest changes from the upstream repository. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Tiny Sweeper reviewThis PR vendors TinyHumans dependencies from git to submodules, renames `tinyinference` to `tinyinference_llm`, adds recursive submodule checkout to CI, and introduces agent registry configuration in OpenHuman examples. However, it introduces vendored dependencies for the library crate `tinyhivemind`, violating repository rules, and leaves unresolved path dependencies, unpinned versions, and a missing Cargo.lock update. State: Changes requested Review snapshot
Completeness: Complete What changedReplaced git dependencies with vendored submodules for openhuman, tinyinference, tinytools, tinyjevclient in .gitmodules and Cargo.toml. Updated Cargo.toml excludes to include vendor. Changed workspace dependency `tinyinference` to `tinyinference-llm` with a path dependency. Added recursive submodule checkout to CI jobs in .github/workflows/ci.yml. Added `openhuman-core` path dependency and agent registry entries to examples/openhuman. Updated test assertions in retry tests to reflect sanitized provider failure handling. Updated wiki submodule commit. Features
TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Previously reported and still active
Resolved this pass
Before merge
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Warning Review paused — included plan limit reachedKeep your review moving with free on-demand reviews.
On-demand reviews are free for one more day. Promotion and pricing detailsOn-demand reviews are free for one more day. After that, they cost $0.25 per reviewed file. Review limit detailsOr wait 30 minutes for your next included review. Limit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (12)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change vendors four Rust dependencies as Git submodules, switches dependency declarations to local paths, renames the example dependency to ChangesVendored dependencies
Wiki submodule revision
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (10 skipped: 10 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
A rabbit checks the vendor trail Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e4ecd115a
ℹ️ 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.
Requesting changes: 4 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0210 · 372,795 in / 27,595 out · 31,176 cached (8%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 441 embedded
critique: $0.0111 · 192,127 in / 11,114 out · 16,063 cached (8%) · gpt-5.6-luna, deepseek-v4-flash
security: $0.0091 · 156,285 in / 6,312 out · 12,553 cached (8%) · gpt-5.6-luna
tests: $0.0003 · 14,593 in / 1,772 out · 1,536 cached (11%) · deepseek-v4-flash
description: $0.0003 · 5,344 in / 6,710 out · 1,024 cached (19%) · deepseek-v4-flash
Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0040 · 72,198 in / 17,783 out · 7,186 cached (10%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 482 embedded
critique: $0.0019 · 28,018 in / 2,134 out · 2,066 cached (7%) · gpt-5.6-luna
security: $0.0010 · 12,612 in / 1,218 out · 0 cached (0%) · gpt-5.6-luna
tests: $0.0004 · 15,634 in / 2,611 out · 1,024 cached (7%) · deepseek-v4-flash
description: $0.0004 · 6,353 in / 8,678 out · 1,024 cached (16%) · deepseek-v4-flash
Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 475e5764be
ℹ️ 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".
| # adapter keeps default features off because it only binds already-built agents; | ||
| # the consuming host chooses the runtime capabilities it enables. | ||
| openhuman-embed = { git = "https://github.com/tinyhumansai/openhuman", rev = "93e9938f388f3989b7c0acff3f72007a31bb3128", default-features = false } | ||
| openhuman-embed = { path = "vendor/openhuman/crates/openhuman-embed", default-features = false } |
There was a problem hiding this comment.
Keep library dependencies out of nested submodules
Although this revision initializes submodules in ci.yml, the documented consumer workflow in README.md still adds tinyhivemind as a non-recursive submodule, while crates/tinyhivemind-openhuman/Cargo.toml takes this entry as a normal library dependency. Consumers using that adapter therefore receive an empty vendor/openhuman directory and fail during Cargo manifest resolution; retain a revision-pinned Git dependency for this library edge rather than requiring nested submodule initialization.
AGENTS.md reference: AGENTS.md:L293-L296
Useful? React with 👍 / 👎.
| # docs/adr/0013-a-vendored-crate-is-an-example-dependency.md. A consumer builds | ||
| # `crates/*` and never the examples, so it never resolves this. | ||
| tinyinference = { git = "https://github.com/tinyhumansai/tinyinference", rev = "cc8aca484bb995fbab3b7358f0d72ab5056b587c" } | ||
| tinyinference-llm = { path = "vendor/openhuman/vendor/tinyagents/vendor/tinyinference/crates/tinyinference-llm" } |
There was a problem hiding this comment.
Point the direct dependencies at their own pins
The newly added vendor/tinyinference gitlink is never referenced; this dependency instead descends through OpenHuman, TinyAgents, and TinyAgents' nested TinyInference checkout. Consequently updating the recorded direct TinyInference pin has no effect, and building the desk example unnecessarily requires the OpenHuman subtree. Point this entry at the direct submodule, and do the same for tinytools on line 83, whose new top-level gitlink is likewise unused.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0147 · 249,142 in / 32,229 out · 11,908 cached (5%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 902 embedded
critique: $0.0074 · 116,042 in / 12,250 out · 6,207 cached (5%) · gpt-5.6-luna, deepseek-v4-flash
security: $0.0061 · 88,459 in / 6,106 out · 3,653 cached (4%) · gpt-5.6-luna
tests: $0.0005 · 18,070 in / 4,267 out · 1,024 cached (6%) · deepseek-v4-flash
description: $0.0004 · 8,516 in / 7,194 out · 1,024 cached (12%) · deepseek-v4-flash
| .filter_map(|(_, request)| request["messages"].as_array()?.last()?["content"].as_str()) | ||
| .collect::<std::collections::BTreeSet<_>>(); | ||
| assert_eq!(lead_prompts.len(), MAX_SEAT_ATTEMPTS as usize); | ||
| assert_eq!(lead_prompts.len(), 1, "sanitized failures are not retried"); |
There was a problem hiding this comment.
Do not replace retry coverage with a sanitized-failure assumption
The 429 fixture is a retryable provider failure, yet this assertion explicitly requires only one lead request. It changes the test from checking bounded exhaustion to checking fail-fast behavior and can conceal an accidental removal of retries. The test should continue to assert the configured number of attempts for the retryable case; sanitization should be tested with a fixture that represents a sanitized/non-retryable error, not with HTTP 429.
[RULE] regression-coverage ·
| futures = "0.3" | ||
| # The live hive uses OpenHuman's native runtime, agents, sessions, and TinyCortex module. | ||
| openhuman-embed = { git = "https://github.com/tinyhumansai/openhuman", rev = "93e9938f388f3989b7c0acff3f72007a31bb3128", default-features = false, features = ["mcp", "modules"] } | ||
| openhuman-embed = { path = "../../vendor/openhuman/crates/openhuman-embed", default-features = false, features = ["mcp", "modules"] } |
There was a problem hiding this comment.
Keep the standalone example independent of the vendor submodule
This changes the standalone example from a pinned git dependency to a path under vendor/. A checkout without the vendor submodule can no longer resolve or build the example, contradicting the standalone-build contract. Use a revision-pinned git dependency instead of requiring repository-local vendored sources.
Additional critique observation
Pin the example dependency to a revision
[RULE] dependency-pinning
The previous git dependency was pinned to a specific revision, but this change replaces it with an unversioned local path. That makes the example's dependency contents depend on whatever checkout happens to be present in vendor/openhuman, defeating reproducible standalone builds and allowing an unrelated submodule update to change the dependency. Use a pinned git revision (or another reproducibly versioned source) instead.
Additional critique observation
Update Cargo.lock with the new dependency sources
[RULE] lockfile-consistency
Changing openhuman-embed from a git source to a local path, and adding the separate openhuman package, changes the package source graph. The checked-in standalone examples/openhuman/Cargo.lock must be regenerated so it records the same sources and dependency relationships; otherwise locked builds can reject this manifest or resolve a graph that does not match it.
Additional critique observation
Keep dependency repositories out of .gitmodules
[RULE] submodule-dependency
This path dependency makes the example depend on the vendor/openhuman repository being present as a git submodule. The repository currently declares that dependency repository in .gitmodules, so a fresh clone that follows the normal checkout without initializing submodules cannot resolve this manifest. Avoid making the example's dependency resolution depend on a separately checked-out repository, or provide a supported vendored/reproducible source in this crate.
Additional security observation
Update Cargo.lock with the new dependency sources
[RULE] stale-lockfile
The manifest changes the source of openhuman-embed and adds a direct openhuman dependency, but the pull request does not update examples/openhuman/Cargo.lock. The committed lockfile therefore does not describe the manifest's dependency graph or sources, defeating the reproducible standalone build contract. Regenerate and commit the example lockfile after choosing the final revision-pinned sources.
[RULE] vendored-dependency ·
| openhuman-embed = { path = "../../vendor/openhuman/crates/openhuman-embed", default-features = false, features = ["mcp", "modules"] } | ||
| # The DeepSWE adapter registers its per-seat definitions with OpenHuman's | ||
| # runtime registry so the current hosted harness can resolve them at turn time. | ||
| openhuman = { path = "../../vendor/openhuman/crates/openhuman-core", default-features = false } |
There was a problem hiding this comment.
Pin the example dependency to a repository revision
The new OpenHuman core dependency is an unpinned local path dependency. This makes the build depend on whatever source happens to be present in the vendor submodule rather than a reproducible revision, and it violates the repository rule that example-only external dependencies are pinned by revision. Reference the upstream repository and an immutable revision.
Additional critique observation
Keep example-only dependencies pinned by revision
[RULE] dependency-pinning
The newly added openhuman dependency is also an unversioned path into the vendor submodule. Because this is an example-only dependency, it should use the repository's pinned-revision policy rather than inheriting arbitrary local submodule contents; otherwise the standalone example can resolve a different API than the committed lockfile and source revision.
[RULE] unpinned-dependency ·
Replaces parent-level TinyHumans Git dependencies with paths into vendored submodules pinned to current upstream main.\n\nVerification:\n- TinyHivemind: cargo test --workspace --all-targets\n- TinyCortex: cargo check --workspace --all-targets, focused API/contract tests (full suite has one pre-existing source-weight assertion failure unrelated to this change).
Summary by CodeRabbit