From 1cbb17d2a3f04dde2024c5bb60f6cdc74816c95a Mon Sep 17 00:00:00 2001 From: Srinivas Gorur-Shandilya Date: Mon, 5 Oct 2026 16:00:27 -0400 Subject: [PATCH 1/5] fix(drug-discovery): normalize Pose origin for DDOS-7952 Canonicalize legacy crystal_extract to cocrystal on every Pose instance, accept both spellings in Protein Prep crystal-pose filtering, extend the public origin vocabulary with manual, and add compatibility tests/docs. --- CONTEXT.md | 7 +++++ docs/dd/ref/pose.md | 4 +++ src/drug_discovery/protein_prep.py | 8 ++++-- src/drug_discovery/structures/pose.py | 33 +++++++++++++++++++++-- tests/test_pose.py | 39 +++++++++++++++++++++++++++ tests/test_protein_prep.py | 20 +++++++++++++- 6 files changed, 106 insertions(+), 5 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 1da60706..1951e2f0 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -348,6 +348,13 @@ ligands) registered via **Pose registration**. _Avoid_: using `Ligand` when you mean a pose result with pose-scoped identity; conflating with docking-only outputs +**Pose origin**: +Public provenance of a **Pose**: `cocrystal`, `docked`, `registered`, or +`manual`. The client presents legacy stored `crystal_extract` as canonical +`cocrystal`; unknown future values remain visible for forward compatibility. +_Avoid_: exposing `crystal_extract` as a separate public provenance; rejecting +an otherwise readable Pose solely because its origin is newer than the client + **Panel pose**: A docked pose of one ligand into one member of the Secondary Pharmacology **Panel**, stored under `result_type=panelpose` rather than `pose`. Names the diff --git a/docs/dd/ref/pose.md b/docs/dd/ref/pose.md index c190ed47..9fca7973 100644 --- a/docs/dd/ref/pose.md +++ b/docs/dd/ref/pose.md @@ -38,6 +38,10 @@ A **Pose** is a 3D ligand conformation stored in the platform pose result table (`result_type=pose`). It has its own platform **pose id** (`Pose.id`) and a parent **ligand id** (`Pose.ligand_id`) in the ligands table. +**Pose origin** on each row is one of `cocrystal`, `docked`, `registered`, or +`manual`. Older indexed rows may still store `crystal_extract`; the client +reads that as `cocrystal`. + Use :class:`Pose` / :class:`PoseSet` when you need pose-scoped identity. :meth:`~deeporigin.drug_discovery.Docking.get_results` and :meth:`~deeporigin.drug_discovery.Docking.get_poses` return :class:`PoseSet`. diff --git a/src/drug_discovery/protein_prep.py b/src/drug_discovery/protein_prep.py index 2ba4eaea..e7522bf1 100644 --- a/src/drug_discovery/protein_prep.py +++ b/src/drug_discovery/protein_prep.py @@ -45,7 +45,11 @@ from deeporigin.drug_discovery.notebook_watch_mixin import NotebookWatchMixin from deeporigin.drug_discovery.structures.ligand import Ligand from deeporigin.drug_discovery.structures.pocket import Pocket -from deeporigin.drug_discovery.structures.pose import Pose, PoseSet +from deeporigin.drug_discovery.structures.pose import ( + Pose, + PoseSet, + normalize_pose_origin, +) from deeporigin.drug_discovery.structures.protein import Protein from deeporigin.exceptions import DeepOriginException from deeporigin.platform.client import DeepOriginClient @@ -733,7 +737,7 @@ def _crystal_pose_output_rows(rows: list[Any]) -> list[dict[str, Any]]: for row in rows: if not isinstance(row, dict): continue - origin = str(row.get("origin") or "").strip() + origin = normalize_pose_origin(row.get("origin")) if origin != _PROTEIN_PREP_CRYSTAL_POSE_ORIGIN: continue filtered.append(row) diff --git a/src/drug_discovery/structures/pose.py b/src/drug_discovery/structures/pose.py index 186c8c26..9ab65d77 100644 --- a/src/drug_discovery/structures/pose.py +++ b/src/drug_discovery/structures/pose.py @@ -28,7 +28,10 @@ from deeporigin.exceptions import DeepOriginException from deeporigin.platform.client import DeepOriginClient -PoseOrigin = Literal["cocrystal", "docked", "registered"] +PoseOrigin = Literal["cocrystal", "docked", "registered", "manual"] + +_LEGACY_POSE_ORIGIN_CRYSTAL_EXTRACT = "crystal_extract" +_CANONICAL_POSE_ORIGIN_COCRYSTAL = "cocrystal" _POSE_RESULT_ID_POLL_SECONDS = 3.0 _POSE_RESULT_ID_POLL_INTERVAL = 0.5 @@ -94,6 +97,21 @@ def _strip_nonempty_str(value: Any) -> str | None: return None +def normalize_pose_origin(value: PoseOrigin | str | None) -> PoseOrigin | str | None: + """Return canonical pose provenance for client-facing :class:`Pose` objects. + + Maps the legacy indexed alias ``crystal_extract`` to ``cocrystal``. Other + known and unknown origin strings are returned stripped unchanged. + """ + + text = _strip_nonempty_str(value) + if text is None: + return None + if text == _LEGACY_POSE_ORIGIN_CRYSTAL_EXTRACT: + return _CANONICAL_POSE_ORIGIN_COCRYSTAL + return text + + def _path_points_to_existing_local_file(path: str) -> bool: """Return True if ``path`` refers to an existing regular file on disk.""" @@ -289,7 +307,9 @@ class Pose(Entity): pose_score: Docking pose score when present. binding_energy: Docking binding energy when present. best_pose: Whether this row is the best pose for its ligand in a run. - origin: Platform pose provenance (``cocrystal``, ``docked``, ``registered``). + origin: Platform pose provenance (``cocrystal``, ``docked``, ``registered``, + ``manual``). Legacy rows may store ``crystal_extract``; the client + presents that as ``cocrystal``. component_id: Protein Prep Selection component id for cocrystal poses. props: Additional metadata from the platform row. """ @@ -311,6 +331,15 @@ class Pose(Entity): _preferred_ext: ClassVar[str] = ".sdf" _project_name: str | None = field(default=None, repr=False, compare=False) + def __post_init__(self) -> None: + if self.origin is not None: + object.__setattr__(self, "origin", normalize_pose_origin(self.origin)) + + def __setattr__(self, name: str, value: object) -> None: + if name == "origin" and value is not None: + value = normalize_pose_origin(value) # type: ignore[arg-type] + super().__setattr__(name, value) + @property def mol(self) -> Chem.Mol | None: """Return the RDKit molecule, loading from disk when needed.""" diff --git a/tests/test_pose.py b/tests/test_pose.py index 273b392a..f10dc4ee 100644 --- a/tests/test_pose.py +++ b/tests/test_pose.py @@ -18,6 +18,7 @@ _optional_bool, _optional_float, _pose_row_from_registration_execution, + normalize_pose_origin, ) from deeporigin.platform.client import DeepOriginClient from deeporigin.platform.constants import TOOL_KEYS_AND_VERSIONS @@ -57,6 +58,44 @@ def test_pose_from_json_remote_path_only_row() -> None: assert pose.mol is None +@pytest.mark.parametrize( + ("raw_origin", "expected"), + [ + ("crystal_extract", "cocrystal"), + ("cocrystal", "cocrystal"), + ("docked", "docked"), + ("registered", "registered"), + ("manual", "manual"), + ("future_origin", "future_origin"), + ], +) +def test_pose_origin_normalization_direct_and_json( + raw_origin: str, + expected: str, +) -> None: + """Legacy crystal_extract and unknown origins follow the client contract.""" + pose = Pose(ligand_id="L1", origin=raw_origin) + assert pose.origin == expected + + hydrated = Pose.from_json( + [ + { + "ligand_id": "L1", + "remote_path": "entities/poses/p1.sdf", + "origin": raw_origin, + } + ] + )[0] + assert hydrated.origin == expected + + pose.origin = raw_origin + assert pose.origin == expected + + +def test_normalize_pose_origin_none() -> None: + assert normalize_pose_origin(None) is None + + def test_pose_from_json_coerces_metadata_fields() -> None: """Numeric and boolean platform fields are coerced on hydration.""" pose = Pose.from_json( diff --git a/tests/test_protein_prep.py b/tests/test_protein_prep.py index f0c00b12..4f3f0b14 100644 --- a/tests/test_protein_prep.py +++ b/tests/test_protein_prep.py @@ -1446,6 +1446,24 @@ def test_crystal_poses_from_output_rows_hydrates_metadata( assert poses[0].remote_path == "entities/ligands/extracted/exec/lig.sdf" +def test_crystal_poses_from_output_rows_accepts_legacy_crystal_extract_origin( + client: DeepOriginClient, +) -> None: + """Legacy crystal_extract rows surface as canonical cocrystal poses.""" + rows = [ + { + "component_id": "ligand:LIG:A:100", + "file_path": "entities/ligands/extracted/exec/lig.sdf", + "ligand_id": "08LIGAND0001", + "origin": "crystal_extract", + "protein_id": "09PREPAREDPROTEIN", + } + ] + poses = _crystal_poses_from_output_rows(rows, client=client) + assert len(poses) == 1 + assert poses[0].origin == "cocrystal" + + def test_crystal_poses_from_output_rows_rejects_non_cocrystal_origin( client: DeepOriginClient, ) -> None: @@ -1455,7 +1473,7 @@ def test_crystal_poses_from_output_rows_rejects_non_cocrystal_origin( "component_id": "ligand:LIG:A:100", "file_path": "entities/ligands/extracted/exec/lig.sdf", "ligand_id": "08LIGAND0001", - "origin": "crystal_extract", + "origin": "docked", "protein_id": "09PREPAREDPROTEIN", } ] From 97adba50e424a5a093c925b6a1c918867273e917 Mon Sep 17 00:00:00 2001 From: Srinivas Gorur-Shandilya Date: Mon, 5 Oct 2026 16:41:53 -0400 Subject: [PATCH 2/5] docs(pose): clarify origin list is non-exhaustive Align pose.md with the forward-compat contract so callers do not treat the known origins as a closed set. --- docs/dd/ref/pose.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/dd/ref/pose.md b/docs/dd/ref/pose.md index 9fca7973..a94d2eb8 100644 --- a/docs/dd/ref/pose.md +++ b/docs/dd/ref/pose.md @@ -38,9 +38,10 @@ A **Pose** is a 3D ligand conformation stored in the platform pose result table (`result_type=pose`). It has its own platform **pose id** (`Pose.id`) and a parent **ligand id** (`Pose.ligand_id`) in the ligands table. -**Pose origin** on each row is one of `cocrystal`, `docked`, `registered`, or +**Pose origin** known values include `cocrystal`, `docked`, `registered`, and `manual`. Older indexed rows may still store `crystal_extract`; the client -reads that as `cocrystal`. +reads that as `cocrystal`. Unknown origin strings are preserved for forward +compatibility. Use :class:`Pose` / :class:`PoseSet` when you need pose-scoped identity. :meth:`~deeporigin.drug_discovery.Docking.get_results` and From 5ecde82dab33f11af9f5f433935b50904fc5fa94 Mon Sep 17 00:00:00 2001 From: Srinivas Gorur-Shandilya Date: Mon, 5 Oct 2026 16:53:47 -0400 Subject: [PATCH 3/5] chore(pose): rephrase normalize_pose_origin docstring Retigger required Test Python Code checks after a docs-only push that did not match the workflow path filters. --- src/drug_discovery/structures/pose.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/drug_discovery/structures/pose.py b/src/drug_discovery/structures/pose.py index 9ab65d77..7db1349d 100644 --- a/src/drug_discovery/structures/pose.py +++ b/src/drug_discovery/structures/pose.py @@ -100,8 +100,8 @@ def _strip_nonempty_str(value: Any) -> str | None: def normalize_pose_origin(value: PoseOrigin | str | None) -> PoseOrigin | str | None: """Return canonical pose provenance for client-facing :class:`Pose` objects. - Maps the legacy indexed alias ``crystal_extract`` to ``cocrystal``. Other - known and unknown origin strings are returned stripped unchanged. + Maps the legacy indexed alias ``crystal_extract`` to ``cocrystal``. Known and + unknown origin strings otherwise remain stripped and unchanged. """ text = _strip_nonempty_str(value) From 7074e0d12bb47b436d61fdce383c9d3203be5649 Mon Sep 17 00:00:00 2001 From: Srinivas Gorur-Shandilya Date: Mon, 5 Oct 2026 17:29:04 -0400 Subject: [PATCH 4/5] docs(agents): record merge-ready lessons from PR 654 Capture path-filter retrigger, required-check watcher pitfalls, and the non-exhaustive Pose origin docs nit for future cycles. --- docs/agents/merge-ready-lessons.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/agents/merge-ready-lessons.md b/docs/agents/merge-ready-lessons.md index 49e5231e..72338375 100644 --- a/docs/agents/merge-ready-lessons.md +++ b/docs/agents/merge-ready-lessons.md @@ -2,6 +2,10 @@ Notes from past merge-ready cycles. Read before starting; append after success. +## 2026-10-05 — PR #654 — DDOS-7952 Pose origin normalize + +`Test Python Code` path-filters on `**/*.py`, so a docs-only push leaves required formatting/functionality checks stuck or cancelled — touch a `.py` file to retrigger. `gh pr checks --watch --fail-fast` also exits early on non-required SonarCloud failures; watch required Ubuntu checks only, and treat matrix `cancel` as not-green (fail-fast can cancel queued Ubuntu jobs while Windows already passed). Copilot’s docs nit was real: pose origin docs must say known values + unknown strings preserved, not an exhaustive four-value list. + ## 2026-10-01 — PR #650 — DDOS-8122 ADMET 2.x SDK `make test` failed because `run(quote=True)` called `_ensure_platform_inputs()` and assigned `ligand.id` while `test_admet.py` expects quote-only runs to leave SMILES-only ligands unregistered (match `SecondaryPharmacology`). Keep `sync=True` on `run()` even when `approve_amount` is set; Copilot’s second pass also needed mock project-scoped ADMET rows and `update_from_dto` on `start()`. From 56a91bd111ebe3c96ba2a4f2cdcd8124e035a6f6 Mon Sep 17 00:00:00 2001 From: Srinivas Gorur-Shandilya Date: Mon, 5 Oct 2026 17:29:18 -0400 Subject: [PATCH 5/5] chore(pose): retrigger required CI after lessons commit docs/agents-only pushes skip Test Python Code path filters; nudge a .py file so formatting and Ubuntu functionality re-run on the tip. --- src/drug_discovery/structures/pose.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drug_discovery/structures/pose.py b/src/drug_discovery/structures/pose.py index 7db1349d..f8c4f4e0 100644 --- a/src/drug_discovery/structures/pose.py +++ b/src/drug_discovery/structures/pose.py @@ -101,7 +101,7 @@ def normalize_pose_origin(value: PoseOrigin | str | None) -> PoseOrigin | str | """Return canonical pose provenance for client-facing :class:`Pose` objects. Maps the legacy indexed alias ``crystal_extract`` to ``cocrystal``. Known and - unknown origin strings otherwise remain stripped and unchanged. + unknown origin strings otherwise stay stripped and unchanged. """ text = _strip_nonempty_str(value)