[ISSUE-07] Implement CompatibilitySelector and deterministic route scoring#74
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52d4ec4231
ℹ️ 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 "region_order" in active_constraints: | ||
| orders = [tuple(active_constraints["region_order"])] |
There was a problem hiding this comment.
Validate constrained region order against requested regions
When constraints["region_order"] is present, the selector replaces region_identities with that tuple directly, so a caller can pass a subset (or different set) of regions and get a "successful" route that omits required products. For example, requesting ['a','b'] with region_order=['a'] yields no missing regions and missing_required_products=0, which can cause downstream planning to skip required lvl1 inputs. The constrained order should be validated to be a permutation of the requested regions (or missing regions should still be computed against region_identities).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Added validation in
CompatibilitySelector.select_lvl2_route(...)soconstraints["region_order"]must match the requestedregion_identities; otherwise, the selector now returns a rejected/blocked route with a constraint violation and all requested regions marked as missing. This prevents false “success” when constrained order omits required regions. src/buildcompiler/inventory/selector.pyL94-L117 - Kept the rejection payload explicit by setting
missing_required_products/missing_lvl1_plasmidsto the full requested count and preserving deterministic tie-break identity output in the blocked route path. src/buildcompiler/inventory/selector.pyL109-L114 - Added a unit test that reproduces the reported subset-order issue and verifies the selector rejects it and reports the full requested missing regions. tests/unit/inventory/test_selector.pyL63-L73
Testing
- ✅
pytest tests/unit/inventory/test_selector.py -q
Motivation
Description
src/buildcompiler/inventory/compatibility.py:RouteScore(with explicitsort_key()),Lvl1Route,Lvl2Route, andRouteSelectionfor structured, explainable outputs.CompatibilitySelectorinsrc/buildcompiler/inventory/selector.pywithselect_lvl1_route(...)andselect_lvl2_route(...), including hard-constraint filtering (allowed_identities,forbidden_identities,antibiotic), deterministic candidate ranking honoringBuildOptions.selectionpreferences, exhaustive-search bound handling for lvl2, stable SBOL identity tie-breaks, and returning the selected route plus up to 3 rejected alternatives.src/buildcompiler/inventory/__init__.pyand add focused unit tests intests/unit/inventory/test_compatibility.pyandtests/unit/inventory/test_selector.py.Testing
python -c "from buildcompiler.inventory import CompatibilitySelector",python -c "from buildcompiler.inventory import RouteScore",python -c "from buildcompiler.api.options import BuildOptions; BuildOptions()".pytest tests/unit/inventoryandpytest tests/unit/domain tests/unit/api tests/unit/inventory tests/unit/planning— automated test suites executed and passed (all unit tests exercised by the run completed successfully; final run showed 33 passed).uv-based flow (uv run pytest ...) failed due to network-restricted fetch of an optional git dependency (SBOLInventory) producing a 403 during clone; this is an environment/network issue, not a regression from this change.ruff check .reported pre-existing unrelated lint errors in other repository files that were not introduced by this PR.Codex Task