fix(sdk): filter captured module imports to those referenced by body (follow-up to #478)#479
Conversation
Follow-up to #478 (v0.29.2). The prior fix shipped *every* module-level import to the worker pod, including imports used only by the decorator itself (e.g. `import basilica`, `from basilica import WorldSize`). Worker containers run user code in a `pytorch/cuda` image that does NOT have `basilica-sdk` installed, so the worker raised `ModuleNotFoundError: No module named 'basilica'` at runtime. Runtime trace from basilica-backend#419 Stage 4 take-3 verification: ``` File "/tmp/__basilica_source.py", line 3, in <module> import basilica ModuleNotFoundError: No module named 'basilica' ``` This fix walks the function body's AST and collects: - every free `Name` reference, and - the leftmost identifier of every `Attribute` chain (e.g. `os` in `os.environ.get`). Module-level imports are then filtered to only those whose bound name appears in that set. `import foo` binds `foo`; `import foo.bar` binds `foo`; `import foo as bar` binds `bar`; `from x import foo` binds `foo`; `from x import foo as bar` binds `bar`; `from x import *` is kept verbatim (we cannot statically know its exposed names). Applied at all three call sites: - DistributedFunction._extract_source - DeployedFunction._extract_source - SourcePackager.from_function Adds a regression test (`test_extracted_source_filters_unused_module_imports`) pinning the contract: a body that only references `os` from module scope must produce a head with `import os` and WITHOUT `import basilica` / `import pytest` / `from typing import Optional` (those are imported in the test module but never referenced by the body). Bumps version to 0.29.3.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Summary
Follow-up to #478 (v0.29.2). The prior fix captured every module-level import; that's too broad — it dragged
import basilica(used only by the decorator) into the worker source, and the worker pod'spytorch/cudaimage doesn't havebasilica-sdkinstalled.Runtime trace from basilica-backend#419 Stage 4 take-3 D2 verification:
Fix
Walk the function body's AST. Collect every free
Namereference and the leftmost identifier of everyAttributechain (soos.environ.get(...)contributesos). Then filter the module's top-level imports down to those whose bound names appear in that set.Binding rules:
import foobindsfooimport foo.barbindsfoo(Python module-resolution semantics)import foo as barbindsbarfrom x import foobindsfoofrom x import foo as barbindsbarfrom x import *is kept verbatim (cannot statically enumerate exposed names)Applied at all three call sites:
DistributedFunction._extract_source(decorators.py)DeployedFunction._extract_source(decorators.py)SourcePackager.from_function(source.py)Verification
Local repro against the exact ex20-shape decorator:
Pre-fix (v0.29.2) produced source:
Post-fix (v0.29.3) produced source:
Only the imports the body actually uses make it into the shipped source.
Tests
Adds
test_extracted_source_filters_unused_module_importsto pin the contract: a body that only referencesosfrom module scope must NOT carryimport basilica,import pytest, orfrom typing import Optionalin its head (those are imported at module level in the test file but never referenced by the body).Full SDK test suite: 139 passed, 0 failed.
Cross-repo
one-covenant/basilica-backend#419Stage 4 take-3 Cell B + D2 runtime verificationTest plan
import osonly (verified local)