You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Pin deeporigin.admet-properties to tool major 2 and extend Admet for 2.x: run() for ≤100 ligands (sync), start()/wait() for 101+ with auto ligands_file + ligands_count, and project-wide runs via Admet(ligands=[]) scoped to client.project_id.
Docking-style Quoted handling on run() (None + confirm()), workflow get_results() via admetproperty result-explorer with jobOutputs fallback, and from_dto for file/project inputs.
Resolve project inputs before generating mock prediction rows
tests/mock_server/routers/tools.py:1699
Project-wide inputs never produce mock predictions here: _admet_prediction_rows_from_inputs() only resolves ligands/ligands_file, so inputs={"project": ...} yields rows=[]. The execution then completes with no result-explorer records (and quotes it as one billable ligand), making the documented start() → wait() → get_results() project flow fail under --env local. Resolve the project's mock ligands before synthesizing rows, and extend the project test through get_results().
Remove unreachable empty-ligand validation branch
src/drug_discovery/admet.py:519
This branch is unreachable because _is_project_run() is exactly len(self._ligands) == 0 and already returns via the exception above. Removing it avoids implying that an empty ligand list can reach a different validation path.
Test platform hydration for sa_score and fraction_csp3
src/drug_discovery/structures/ligand.py:120
The existing pinned-molprops hydration test does not exercise either newly added mapping, even though this PR promises loading both sa_score and fraction_csp3 from platform records. Add both fields to test_ligand_from_platform_record_hydrates_molprops and assert the resulting named attributes, so platform-key or attribute regressions are covered separately from the combined-tool response test.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The quoted-confirmation workflow omits required completion polling in several documents and lacks end-to-end coverage.
Review effort: Balanced Findings: None
Previously missed (5)
In code that hasn't changed since last review
Test confirm, wait, and get_results workflow
tests/test_admet_local.py:314
This test stops at Quoted, so the newly documented confirmation path is untested; in particular, it would not catch fetching results before confirmation has finished. Exercise confirm(), wait(), and get_results() here.
Document polling before retrieving results
CONTEXT.md:32
A confirmed quote may transition to Running, so get_results() is not yet available. Record the polling step here to avoid encoding an invalid SDK workflow in the project context.
Wait for execution completion before fetching results
docs/dd/how-to/ligands.md:613
This sequence can fetch results before a confirmed execution completes. Add wait() or watch() between confirmation and result retrieval.
Add wait/watch before retrieving execution results
docs/dd/tools/admet.md:42
Confirmation starts the execution and can return while it is still Running; get_results() at that point has no rows. Include the required wait/watch step in this workflow.
Scope exception message to statuses reaching this branch
src/drug_discovery/admet.py:609
The Quoted case already returns at line 600, so this exception can never be raised for that status. The confirmation guidance is therefore misleading for the actual failure states reaching this branch; keep the message scoped to the observed status and execution ID.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
Negative quote amounts still mutate ligands, and several lifecycle and documentation inconsistencies remain.
Review effort: Balanced Findings: None
Previously missed (7)
In code that hasn't changed since last review
Handle all negative approval amounts as quote-only
src/drug_discovery/admet.py:591
The quote-only check only recognizes -1, but the execution API treats any negative approve_amount as quote-only. For example, run(approve_amount=-2) registers and mutates the ligands even though no inference runs. Skip synchronization for every negative amount.
Document conditional polling after confirmation
CONTEXT.md:32
This abbreviated workflow omits polling after confirm(), although confirmation may leave ADMET running. Keep the context contract accurate by mentioning the conditional wait before result retrieval.
Tell users to wait when confirmation is nonterminal
docs/dd/how-to/ligands.md:613
A confirmation is not guaranteed to complete the execution; ADMET confirmation is Running in the local mock. Calling get_results() immediately can therefore report missing predictions. Tell users to wait when status is nonterminal.
Wait for nonterminal executions before retrieving results
docs/dd/tools/admet.md:42
confirm() can return while the ADMET execution is still running, so this sequence can call get_results() before any rows exist. Include wait()/watch() when confirmation is nonterminal.
Document waiting after nonterminal confirmation
src/drug_discovery/admet.py:581
confirm() may leave an execution in Created or Running (and the mock confirmation route does so for ADMET), so an immediate get_results() can raise because neither result-explorer nor jobOutputs has rows yet. Document the required wait for nonterminal confirmations.
Document the newly exposed fraction_csp3 attribute
src/drug_discovery/structures/ligand.py:261
The public class documentation’s exhaustive Molprops attribute list still ends at sa_score, so the newly exposed fraction_csp3 field is omitted. Add it to the list near src/drug_discovery/structures/ligand.py:225.
Update ADR to reflect the dependency version change
src/platform/constants.py:164
The accepted ADR at docs/adr/0004-admet-properties-from-tool-definition.md:10-24 explicitly says this pin remains "latest" and explains why. Pinning major 2 leaves that architectural record false; update or supersede the ADR as part of this change.
…ops fixes
Extend Admet for tool v2 (pin major 2): sync run up to 100 ligands, workflow
start for 101+ with auto ligands_file, project-wide runs via empty ligands and
client.project_id, Docking-style Quoted handling, and admetproperty results.
Add fraction_csp3 molprops support, sa_score platform load, docs, mock async
path, and remove legacy per-property fixture folders.
Keep run() sync blocking; hydrate async start from DTO; reset status on
duplicate; skip ligand sync only for quote. Extend mock molprops for
fraction_csp3 and async ADMET wait coverage.
- Stub _ensure_run_ligand_count / _ensure_platform_inputs in the BALTO-447
restriction test; Admet.run() now calls both before submit.
- Sonar S8513: single endswith() with a tuple.
- ADR 0004: tool_version is pinned to major "2", not "latest".
- admet.md: define the ligand set used in the bulk example.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…olism _ligand_payloads (DDOS-8122)
Sonar gate on 1792911: new_coverage 77.3% (<80), new_duplicated_lines_density
3.7% (>3). admet._ligand_payloads was a 13-line copy of
metabolism._ligand_payloads; import it instead. Add unit tests for
_ligands_from_list_file_bytes (valid + corrupt bodies) and
_rows_from_result_explorer.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
deeporigin.admet-propertiesto tool major 2 and extendAdmetfor 2.x:run()for ≤100 ligands (sync),start()/wait()for 101+ with autoligands_file+ligands_count, and project-wide runs viaAdmet(ligands=[])scoped toclient.project_id.run()(None+confirm()), workflowget_results()viaadmetpropertyresult-explorer withjobOutputsfallback, andfrom_dtofor file/project inputs.fraction_csp3key; loadsa_score/fraction_csp3from platform pins; docs (docs/dd/tools/admet.md, molprops/ligand copy); mock async ADMET path; remove legacy per-property molprops fixture folders.Merge-ready status
Auto-updated — cycle 3, last updated: 2026-10-01T00:45:00Z
main59095f76604664f6604664f, 0 open threadsRecent activity
make test(run(quote=True)skips ligand sync).6604664f.Test plan
uv run pytest --env local tests/test_admet_local.py tests/test_admet_unit.pyuv run pytest --env local tests/test_molprops_local.py tests/test_molprops.pymake test(local)Links