feat(core): compile-time media feature gate (media_generation + image) (#4804)#4840
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds a ChangesMedia feature gate
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 910c087769
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…sai#4804) The Tauri app consumes openhuman_core with default-features = false, so the compile-time `media` gate defaults OFF in the desktop build even though the root crate keeps it in `default`. That would drop the media_generate_* agent tools from the shipped app. Forward the gate explicitly to keep the desktop build byte-identical, per AGENTS.md.
max_iteration_checkpoint_uses_deterministic_fallback_and_hooks fed the wrap-up call a non-empty response (tool-call XML text) plus a streamed delta, so summarize_turn_wrapup returned that text as the checkpoint and the deterministic fallback the test is named for never ran — the (1 steps) assertion failed under the FULL instrumented suite. Make the wrap-up yield nothing (empty text, no deltas) so build_deterministic_checkpoint is the answer, and assert no iteration-2 wrap-up delta is streamed. Surfaced by tinyhumansai#4804's Cargo.toml change forcing the full coverage suite; the checkpoint-selection logic predates tinyhumansai#4386.
|
Maintainer fix pass (pushed CI — Fix ( Review feedback: none outstanding — |
|
Re @chatgpt-codex-connector's P1 "Keep media enabled for the embedded desktop core" — already addressed in current HEAD. openhuman_core = { path = "../..", package = "openhuman", default-features = false, features = ["media"] }with a comment explaining that |
|
To use Codex here, create a Codex account and connect to github. |
M3gA-Mind
left a comment
There was a problem hiding this comment.
Reviewed — CI green and all CodeRabbit/Codex review threads resolved. Approving.
🤖 Council Merge Gate — APPROVE (merge failed)The model council review gate returned APPROVE (session However, The PR is left OPEN for manual investigation. Posted automatically by the |
Conflict in app/src-tauri/Cargo.toml: the media feature gate (tinyhumansai#4804/tinyhumansai#4840) landed on main and hit the same trap as tinyhumansai#4901 — `default-features = false` silently dropping a default-ON core gate from the shipped desktop build. It forwarded `media`; this branch forwards `voice`. Both are required, so the resolution keeps both rather than either side. Merged the two comments into one rule ("every default-ON gate must be forwarded explicitly, a missing gate vanishes silently") and noted that `tokenjuice-treesitter` is the remaining un-forwarded default (tinyhumansai#4918). Verified after merge: `cargo tree -e features -i openhuman` resolves both `feature "media"` and `feature "voice"`; shell `cargo check` green (so the VOICE_COMPILED_IN assert passes); 212 related Vitest tests pass; fmt clean.
tinyhumansai#4804) (tinyhumansai#4840) Co-authored-by: M3gA-Mind <elvin@tinyhumans.ai>
Summary
mediafeature (default-ON) that gates themedia_generation(media_generate_*agent tools) +imagedomains, composing with the runtimeDomainSet::mediaflag from feat(core): DomainSet on CoreBuilder + DomainRegistration filter seam (feature-gate prerequisite) #4796.media_generationhas a single caller andimageis unwired scaffold, so no stub facade is needed — both modules are plain#[cfg(feature = "media")] pub mod …and the one call site is#[cfg]'d.Problem
#4795 wants one compile-time feature gate per subsystem family so slim harness builds drop code + deps; #4796 shipped the runtime
DomainSetaxis. This PR adds the compile-time half for media. Two scope realities the issue over-stated, confirmed by a full sweep:reqwest, shared) and theimagecrate is shared with channel media upload — neither is exclusive to the gated domains. This is a surface-only gate (drops the tool code + modules), not a dependency-shedding one. The issue's "sheds media processing dependencies" DoD line is superseded.DomainGroup::Mediainsrc/core/all.rsbeyond themedia_generate_*tools. The "controllers unregistered / stores not initialized / subscribers not registered" DoD lines don't apply.imageis dead scaffold — added in feat(image): add image tool contracts #2997 ("image tool contracts"), wired to nothing (zero consumers). Folded under the gate per issue scope; gating it is cosmetic.Solution
Cargo.toml: newmedia = []feature (nodep:— no exclusive crate), added todefault.src/openhuman/mod.rsmarkspub mod media_generation;andpub mod image;as#[cfg(feature = "media")]; the single consumer (build_media_toolsextend insrc/openhuman/tools/ops.rs) is#[cfg(feature = "media")]. The remainingmedia_generationreferences are doc-comments only (src/openhuman/file_storage/tools.rs), so nothing else needs acfg.rust-feature-gate-smokelane already builds--no-default-features --features tokenjuice-treesitter(which turns every default gate off), so it covers this gate too — updated its name + comment; no new step needed.media_tools_registered_when_feature_on(default build) andmedia_tools_absent_when_feature_off(disabled build) assert the on/off behavior.mediarow + a "leaf-gate variant" note contrasting it with the voice stub-facade.When off: the
media_generate_image/video+media_list_modelsagent tools are absent from the tool list; themedia_generation+imagemodules are excluded from the build.Submission Checklist
#[cfg]attrs, amedia = []feature, docs, CI yaml).diff-coverreports no lines with coverage information; the two new registration tests cover the on/off behavior. CI coverage-gate authoritative. — Diff coverage ≥ 80%N/A: compile-time build-config change, no user-facing feature row## Related—N/A: no matrix rows affectedCloses #NNNin the## RelatedsectionImpact
mediaon and is byte-identical.--no-default-features(withoutmedia) drops themedia_generate_*tools + themedia_generation/imagemodules. No deps shed (image crate + reqwest are shared).Related
DomainSetseam. Follows the feat(core): feature gate — voice #4803 voice pathfinder (PR feat(core): compile-time voice feature gate (voice + audio_toolkit) (#4803) #4833); demonstrates the facade-free leaf-gate variant. Tool/controller gating convergence follow-up: feat(core): converge tool gating with controller gating via per-Tool DomainGroup metadata #4821.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/4804-media-feature-gate910c0877697d73c3dca370f4c0859c287a73d802Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckopenhuman::tools::ops::tests::media_tools_registered_when_feature_on(default, passed) +..::media_tools_absent_when_feature_off(disabled build, passed)cargo fmt --checkclean · defaultcargo checkexit 0 · disabledcargo check --no-default-features --features tokenjuice-treesitterexit 0 ·cargo clippy --lib0 warnings in changed files (both configs)Validation Blocked
command:cargo test --lib(full)error:OOM on local machine (known)impact:full matrix runs in CI; locally scoped to the two media gate tests + default/disabledcargo check— all green in both feature configsBehavior Changes
mediacompile-time gate; default unchangedSummary by CodeRabbit
New Features
mediafeature to enable image and media-generation tools.Documentation
mediavsvoicebehavior and scope.Tests
Chores