[ISSUE-04] Add BuildCompiler API skeleton#71
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b3298cea6
ℹ️ 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".
| compiler = BuildCompiler( | ||
| inventory=inventory, | ||
| planner=planner, | ||
| executor=executor, | ||
| adapters=adapters, | ||
| options=compiler_options, | ||
| **kwargs, |
There was a problem hiding this comment.
sbol_document in module-level full_build
The module-level full_build(...) accepts an sbol_document argument, but in the lightweight constructor path it is never passed to BuildCompiler, so callers who provide sbol_document (without SynBioHub args) silently lose that input and run with compiler.sbol_document=None. This breaks dependency-injected flows where planner/executor expect the document to be present and makes the public API contract misleading.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Fixed the module-level
full_build(...)lightweight path to correctly forwardsbol_documentintoBuildCompiler(...), so callers don’t lose that injected input when SynBioHub args are not used. - Added a regression test that verifies
sbol_documentis preserved in the lightweight path by monkeypatchingBuildCompiler.full_buildand asserting the captured instance value matches the provided document. - Committed the fix on the current branch with commit message:
Fix full_build forwarding of sbol_document.
Testing
- ✅
pytest tests/unit/api/test_compiler_api.py
There was a problem hiding this comment.
Summary
- Fixed the module-level
full_build(...)lightweight path to forwardsbol_documentintoBuildCompiler(...), preventing silent loss of caller-provided SBOL input when SynBioHub args are not used. src/buildcompiler/api/compiler.pyL105-L116 - Added a regression test that monkeypatches
BuildCompiler.full_build, invokes module-levelfull_build(...)withsbol_document, and asserts the constructed compiler preserves that exact object. tests/unit/api/test_compiler_api.pyL118-L132 - Committed the changes on the current branch with commit message:
Fix full_build forwarding of sbol_document(commit5295347).
Testing
- ✅
pytest tests/unit/api/test_compiler_api.py
1 participant
Motivation
Description
src/buildcompiler/api/compiler.pywith aBuildCompilerdataclass that is dependency-injected, storesinventory,sbol_document,planner,executor,adapters, andoptions, and exposesplan,execute, andfull_buildinstance methods that either delegate to injected dependencies or raiseNotImplementedErroras an explicit placeholder.BuildCompiler.from_synbiohub(...)as the only reserved SynBioHub factory boundary and made it raiseNotImplementedErrorwhen actual non-emptycollectionsloading is requested while remaining offline-friendly for empty/no-op calls.full_build(...)function that chooses the lightweight constructor path or thefrom_synbiohub(...)factory path based on presence of SynBioHub-related args and does not import optional automation modules.BuildCompilerandfull_buildfromsrc/buildcompiler/api/__init__.pyand added tests intests/unit/api/test_compiler_api.pyto cover imports, constructor injection, placeholder behavior, optional-dependency import guards, and delegation to injected fake planner/executor objects.Testing
python -c "from buildcompiler.api import BuildCompiler, full_build"andpython -c "from buildcompiler.api.options import BuildOptions; BuildOptions()", both succeeded.pytest tests/unit/apiwhich passed, andpytest tests/unit/domain tests/unit/apiwhich also passed.uv run pytest tests/unit/apiwhich failed in this environment due to a network-restricted fetch of the optional git dependencySBOLInventoryused by the environment setup, so theuv-backed commands are blocked by network restrictions here.ruff check .which failed due to pre-existing repository-wide lint issues outside the scope of this API skeleton change (these are unrelated to the added API files).Codex Task