[ISSUE-06] Implement planner, classifier, and combinatorial expansion#73
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: 3196051a9a
ℹ️ 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".
| if count > 1: | ||
| return BuildRequest( |
There was a problem hiding this comment.
Any ComponentDefinition with more than one subcomponent is immediately emitted as an ASSEMBLY_LVL1 request here, so invalid level-1 designs (e.g. 2 parts, missing CDS, duplicate promoter) are treated as buildable with no warnings. This bypasses the new validate_lvl1_cardinality/ordering logic entirely, which means malformed inputs can flow downstream and fail later in execution instead of being flagged during planning.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex good catch, lets check in planning that the design has 4 parts and those are promoter, RBS, CDS, terminator. If it does not contain those four then return a warning, we suggest to use those four for synthetic bacterial genes.
There was a problem hiding this comment.
Summary
- Added a level-1 planning guard in the classifier so multi-component designs are only classified as
ASSEMBLY_LVL1when they match the expected 4-part structure (promoter,rbs,cds,terminator) when role evidence is available; otherwise they are returned asUnsupportedPlanningRecordwith a warning-style message and structured metadata (component_count,observed_role_counts,suggested_parts). src/buildcompiler/planning/classifier.pyL47-L88 - Added/updated classifier tests to:
- verify a valid 4-part design is still classified as
ASSEMBLY_LVL1, and - verify an invalid part mix is flagged as unsupported with the recommendation message. tests/unit/planning/test_classifier.pyL8-L40
- verify a valid 4-part design is still classified as
- Updated planner integration test fixtures so the lvl1 design input is a valid promoter/RBS/CDS/terminator 4-part construct, keeping expectations aligned with the new classifier behavior. tests/unit/planning/test_full_build_planner.pyL6-L31
Testing
- ✅
pytest tests/unit/planning/test_classifier.py tests/unit/planning/test_full_build_planner.py -q
Also completed:
- Committed changes:
dc833e8 - Created follow-up PR message via
make_prtool.
|
|
||
|
|
||
| def _collect_variant_sets(derivation): | ||
| variables = sorted(list(derivation.variableComponents), key=lambda v: v.identity) |
There was a problem hiding this comment.
The expansion currently sorts variableComponents by their identity and then reuses that sequence as part_order. Because variable-component IDs are arbitrary in SBOL, this can reorder promoter/RBS/CDS/terminator positions and produce incorrect assembly constraints even when the role multiset check passes. Variant ordering should follow the template’s sequence order (or variable references), not lexical URI order.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Fixed combinatorial expansion ordering so variable components are no longer ordered by arbitrary variable-component IDs; they’re now ordered by each variable component’s referenced variable (
vc.variable), with identity as a deterministic tiebreaker. This ensures emittedconstraints["part_order"]follows template-variable positions instead of lexical URI ordering artifacts. src/buildcompiler/planning/combinatorial.pyL11-L16 - Added a regression test that constructs a combinatorial derivation with intentionally scrambled variable-component IDs and verifies the resulting
part_ordermatches template-variable sequence, preventing the reported ordering bug from regressing. tests/unit/planning/test_combinatorial.pyL51-L84
Testing
- ✅
pytest tests/unit/planning/test_combinatorial.py tests/unit/planning/test_full_build_planner.py -q - ✅
git commit -m "Fix combinatorial part ordering by variable reference"
1 participant
Motivation
BuildWarnings.Description
src/buildcompiler/planning/models.pyand updatedsrc/buildcompiler/planning/__init__.pyto exposeBuildPlan,UnsupportedPlanningRecord, andFullBuildPlanner.src/buildcompiler/planning/classifier.pythat mapsModuleDefinition -> ASSEMBLY_LVL2, multi-componentComponentDefinition -> ASSEMBLY_LVL1, single supported-roleComponentDefinition -> DOMESTICATION, and unsupported shapes toUnsupportedPlanningRecord.src/buildcompiler/planning/validation.pyto classify part roles, enforce exact cardinality (4 parts), respect SBOL order when unambiguous, and otherwise fall back to canonical order while emittingBuildWarnings.src/buildcompiler/planning/combinatorial.pythat enumerates variant cartesian products (for the supported variable/variant shape used in tests), enforcesBuildOptions().planning.combinatorial.max_variantswith opt-inallow_large_expansion, skips invalid variants with warnings, and returns structured unsupported records if expansion is blocked or all variants are invalid.FullBuildPlannerinsrc/buildcompiler/planning/full_build_planner.pywhich aggregates lvl2/lvl1/domestication requests, warnings, and unsupported records without touching inventory or executing stages.tests/unit/planning/*.Testing
python -c "from buildcompiler.planning import FullBuildPlanner",python -c "from buildcompiler.planning import BuildPlan", andpython -c "from buildcompiler.api.options import BuildOptions; BuildOptions()", and they succeeded.pytest tests/unit/planning -qand they passed (6 passed).pytest tests/unit/planning tests/unit/domain tests/unit/api tests/unit/sbol tests/unit/inventory -qand it passed (28 passed).uv-backed test commanduv run pytest tests/unit/planningfailed due to an external optional dependency fetch error cloningSBOLInventoryover the network (git fetch/proxy 403), but directpytestruns succeeded locally;ruff check src/buildcompiler/planning tests/unit/planningandruff format --check src/buildcompiler/planning tests/unit/planningcompleted successfully after formatting.Implemented files/classes/functions added or updated:
planning/models.py(BuildPlan,UnsupportedPlanningRecord),planning/classifier.py(request_id_for,classify_non_combinatorial),planning/validation.py(classify_part_role,validate_lvl1_cardinality,ordered_lvl1_parts),planning/combinatorial.py(expand_combinatorial_derivation),planning/full_build_planner.py(FullBuildPlanner), andplanning/__init__.pyexports.Tests added/updated:
tests/unit/planning/test_classifier.py(classification + IDs),tests/unit/planning/test_validation.py(lvl1 cardinality and ordering behavior),tests/unit/planning/test_combinatorial.py(expansion, blocking, invalid-variant handling), andtests/unit/planning/test_full_build_planner.py(mixed-input aggregation).Assumptions and notes: combinatorial support in this PR implements a minimal deterministic expansion path for
variableComponents[].variantsshapes used by tests rather than the full pySBOL2 combinatorial API breadth, blocked large expansions are represented as a structuredUnsupportedPlanningRecordplus aplanning.combinatorial.expansion_blockedBuildWarning, and ambiguous SBOL order is detected whengetInSequentialOrder()cannot produce a full unambiguous 4-role sequence so the planner falls back to canonical promoter/RBS/CDS/terminator order with a warning.Blockers and follow-ups: route optimization/selector logic and the execution/full-build loop are intentionally out of scope and should be implemented in subsequent issues, and running the
uvrunner in CI may require allowing network access or pinning optional dependencies to avoid theSBOLInventorygit fetch failure observed during oneuvinvocation.Codex Task