[BREAKING][misc] refactor: modularize SDK dependencies - #270
Open
JoyboyBrian wants to merge 6 commits into
Open
Conversation
JoyboyBrian
added
breaking
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
cubic analysis
3 issues found and verified against the latest diff
Confidence score: 3/5
- In
osmosis_ai/rollout/trajectory/_atif.py, allowingllm_call_counton user/system steps can produce trajectories that violate ATIF v1.7 and may persist invalid data that downstream tooling misinterprets — addllm_call_countto the agent-only validation list. - In
tests/unit/test_public_api_imports.py, the(osmosis_ai.rollout.integrations.openai_agents, OsmosisAgent, agents, openai-agents)case depends onraise_optional_dependency_errorpreservingModuleNotFoundError.exc.name == "agents"; if that wrapping behavior changes, the test may fail for exception-shape reasons rather than true import regressions — assert the intended error contract explicitly in the helper or relax the test to the user-facing message. - In
CONTRIBUTING.md, documentingpip install ... --group devwill fail at the command line because--groupis uv-only, creating immediate setup friction for contributors — update the pip instructions to a pip-compatible dev-dependency install path (or switch the command to uv consistently).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="CONTRIBUTING.md">
<violation number="1" location="CONTRIBUTING.md:22">
P0: `pip install` does not support `--group dev` — this flag is uv-only and will error at the command line. Replace the pip-based instructions to either remove the `--group dev` part and tell users to install dev deps manually, add a `dev` extra under `[project.optional-dependencies]`, or (simplest) remove the pip instructions and point pip users toward uv.</violation>
</file>
<file name="osmosis_ai/rollout/trajectory/_atif.py">
<violation number="1" location="osmosis_ai/rollout/trajectory/_atif.py:185">
P2: User and system steps can currently carry `llm_call_count`, although its ATIF v1.7 semantics apply to agent steps. Include it in the existing agent-only validation list to prevent invalid persisted trajectories.</violation>
</file>
<file name="tests/unit/test_public_api_imports.py">
<violation number="1" location="tests/unit/test_public_api_imports.py:174">
P2: Test case (osmosis_ai.rollout.integrations.openai_agents, OsmosisAgent, agents, openai-agents) relies on the re-raised ModuleNotFoundError preserving exc.name == 'agents'. Check that raise_optional_dependency_error preserves the original exc.name attribute on the re-raised exception, otherwise the assertion fails.</violation>
</file>
Linked issue analysis
Linked issue: OSM-1655: [SDK] Split CLI and rollout dependencies
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Define supported extras and make the default osmosis-ai distribution include the CLI + framework‑neutral rollout core (extras: server, strands, openai‑agents, harbor, rubric, parquet, full) | README, docs, and pyproject.toml were updated to document and wire the new extras and the default distribution intent. |
| ✅ | Move heavy/server/framework/back-end dependencies into extras and update dependency groups (remove/replace legacy extras) so CLI/rollout installs don't pull unrelated heavy deps | pyproject.toml and uv.lock were modified and new dependency-group semantics are introduced; CONTRIBUTING and docs reflect the group/install changes. |
| ✅ | Replace eager package facades with leaf-level lazy exports and surface actionable missing‑extra errors | New lazy-export helpers and many __getattr__/resolve_lazy_export patterns were added; modules now raise informative missing‑extra errors on import when optional deps are absent. |
| ✅ | Make trajectory persistence backend‑agnostic by adding SDK‑owned ATIF v1.7 models and serializer and update converter/save to use them | A new ATIF model module and serializer were added, and converter/save were updated to use the SDK-owned models; tests were added to lock the contract. |
| Add exact wheel metadata checks and eight clean‑installation smoke scenarios (clean‑wheel install verification) | A substantial verify-wheel-install script was added and the PR documents clean-install testing steps, but I can't verify from the diff that the eight smoke scenarios are fully wired into CI or unit tests—script exists, workflow changes touch installs but don't explicitly run the new verifier in the shown workflow patch. | |
| ✅ | Update scaffold, preflight, docs, and migration guidance and call out breaking changes | Templates, docs, READMEs, and CONTRIBUTING were updated and the PR title includes [BREAKING]; migration notes and scaffolding reflect the new module paths and extras. |
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Fix all with cubic | Re-trigger cubic
- keep agent integrations under their canonical integrations.agents paths - validate full-extra membership and clean core imports in wheel smoke tests - provide an actionable error when OpenAI Agents lacks LiteLLM - document migration steps and cap generated rollouts below 0.4
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
1 participant
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.
What
osmosis-aidistribution whose bare install includes the CLI and framework-neutral rollout core.server,strands,openai-agents,harbor,rubric, andparquetextras; makefullaggregate them.Why
Implements OSM-1655. Default users still get the complete everyday CLI, while unrelated framework, backend, server, rubric, and dataset-validation dependency closures no longer install or import by accident.
This intentionally accepts 0.3 breaking changes: server and framework integrations now use explicit module paths, the old
platformand publisheddevextras are removed, and development tools live in thedevdependency group.NativeHarborBackendis intentionally out of scope so the existingbrian/harborwork can be rebased and refactored separately.How to Test
uv sync --locked --all-extras --group devuv run pytest— 1,804 passeduv run ruff check .uv run ruff format --check .uv run pyright osmosis_ai/uv pip check, import/API checks, dependency ownership checks, and sandbox-package prohibitions.Checklist
Summary by cubic
Refactors the SDK into a single
osmosis-aiwith opt‑in extras and strict, lazy exports; the rollout core stays framework‑neutral and integrations move to extras, implementing OSM‑1655. Adds SDK‑owned ATIF v1.7 models and formatter underosmosis_ai.rollout.trajectory.atif, hardens optional dependency boundaries, and extends CI to verify isolated wheels andfullextra membership.New Features
server,strands,openai-agents,harbor,rubric,parquet;fullaggregates all.osmosis_ai.rolloutexports only framework‑neutral core; integrations live underosmosis_ai.rollout.integrations.agents.*; server/Harbor load via explicit modules with actionable “install the extra” errors; OpenAI Agents warns iflitellmis missing.osmosis_ai.rollout.trajectory.atif; trajectory saving is backend‑agnostic and rejects non‑finite values.fullextra membership and clean core imports, and normalizes colored smoke output; Daytona removed, Harbor’sskypilotremains runtime‑provided.osmosis-ai[server]>=0.3.0rc1,<0.4.Migration
from osmosis_ai.rollout.server import create_rollout_server, ControllerAuthfrom osmosis_ai.rollout.backend.harbor import HarborBackend, OsmosisInstalledAgentfrom osmosis_ai.rollout.integrations.agents.strands import ...from osmosis_ai.rollout.integrations.agents.openai_agents import ...pip install "osmosis-ai[server]","osmosis-ai[strands]","osmosis-ai[openai-agents]","osmosis-ai[harbor]","osmosis-ai[rubric]","osmosis-ai[parquet]", or"osmosis-ai[full]".platformand publisheddevextras are removed; use theparquetextra for dataset validation and thedevdependency group (e.g.,uv sync --all-extras --group devorpython -m pip install -e ".[full]" --group dev).Written for commit f9854d2. Summary will update on new commits.