Skip to content

DDOS-7931: create kwargs for protein/ligand metadata - #655

Merged
sg-s merged 5 commits into
mainfrom
ddos-7931-create-metadata
Oct 6, 2026
Merged

sg-s merged 5 commits into
mainfrom
ddos-7931-create-metadata

Conversation

@sg-s

@sg-s sg-s commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Add state, preparation, structure_hash, and mapped origin to Entities.create_protein
  • Add mapped origin to Entities.create_ligand (platform-aligned: ligands do not accept protein prep fields)
  • Translate public origin (kind, entity_type, entity_id) to origin_* platform columns; omit server-managed origin_entity_display_id on write
  • Return the new metadata columns on create via updated returning field lists

Merge-ready status

Auto-updated — cycle 3, last updated: 2026-10-06T00:38:18Z

Check Status
Branch vs main ✅ synced at 9bc59a0
Head b7ca03e
CI ✅ required green
Copilot ✅ reviewed latest push, 0 open threads
Review threads 0 human/Bugbot, 0 Copilot

Recent activity

  • Copilot re-reviewed b7ca03e with no new comments.
  • Required CI green after PyPI flake rerun.
  • merge-ready label confirmed.

Test plan

  • pytest tests/test_entities.py::test_create_protein_forwards_metadata_payload tests/test_entities.py::test_create_ligand_forwards_origin_payload tests/test_entities.py::test_create_protein_metadata_lv1 tests/test_entities.py::test_create_ligand_origin_lv1 --env local

Unblocks toolbox registration work (e.g. DDOS-8071).

https://deeporigin.atlassian.net/browse/DDOS-7931

Extend Entities.create_protein with state, preparation, structure_hash,
and a mapped origin value; create_ligand accepts origin only. Return the
new columns on create responses and cover payload translation with tests.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The required demo notebook for the new public SDK capability is missing.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Adds entity provenance mapping and protein metadata support to creation APIs.

Changes:

  • Adds protein state, preparation, structure hash, and origin arguments.
  • Adds ligand origin mapping and expanded returned fields.
  • Adds payload and mock-server integration tests.
File Description
src/​platform/​entities.py Implements metadata arguments, origin translation, and return fields.
tests/​test_entities.py Verifies payload forwarding and persistence.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/platform/entities.py
Demonstrate state, preparation, structure_hash, and mapped origin on create
so the DDOS-7931 public kwargs have a runnable Entities-layer example.
Retriggers required Test Python Code after the docs-only notebook push
(path filter is **/*.py only).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The new platform-facing unit tests use a prohibited custom SDK client stub instead of the mock-server fixture.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Use client fixture and mock-server routes instead of SDK client stub

tests/​test_entities.py:560

This custom SDK client stub conflicts with the repository's platform-testing guidance: platform-facing behavior should go through the client fixture and mock-server routes rather than mocking DeepOriginClient methods. Please move any request-shape assertion needed here into the mock server and exercise it through the existing fixture (the integration tests below already cover the returned metadata).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Batch creation does not honor the new origin contract, and live-capable tests use mock-only protein IDs.

Review effort: Balanced
Findings: None

Previously missed (3)

In code that hasn't changed since last review

Medium severity Batch ligand creation does not map the origin field

src/​platform/​entities.py:571

batch_create_ligands documents each row as accepting the optional fields of create_ligand, but this new nested field is only translated on the singular path. The batch path passes each row through _writable_ligand_set_fields, so {"origin": ...} is sent as a raw origin column instead of the required origin_* columns. Map origin for every batch row using the same payload builder, or explicitly narrow the batch API contract so callers are not directed to send an unsupported field.

Medium severity Test uses invalid hard-coded protein ID in dev

tests/​test_entities.py:652

This lv1 test can also run with --env dev, where the mock-only ID "brd" is not the protein uploaded above. The test therefore records invalid provenance (or fails if the platform validates the reference) outside local mode. Resolve or create the source protein from _BRD_PDB_REMOTE and pass its returned ID instead, as the notebook example does.

Medium severity Integration test hard-codes a nonexistent protein ID

tests/​test_entities.py:676

This integration test also hard-codes the local mock protein ID even though the client fixture supports live environments. Uploading _BRD_PDB_REMOTE does not create a protein row, so on dev the ligand's origin is nonexistent or rejected. Create/search the source protein first and use the actual returned ID.

MOCK_CANONICAL_PROTEIN_ID is mock-only; dev API rejects it as origin_entity_id.
@sg-s
sg-s deployed to staging October 6, 2026 12:19 — with GitHub Actions Active
@sg-s
sg-s merged commit 785ee0b into main Oct 6, 2026
11 of 12 checks passed
@sg-s
sg-s deleted the ddos-7931-create-metadata branch October 6, 2026 12:24

This branch had an error being deployed

1 failed and 3 active deployments
docs — b7706a9a Deployed Oct 6, 2026 by sg-s via build-docs #852
prod — b7706a9a Deployed Oct 6, 2026 by sg-s via level-1-tests (3.13, prod) #901
staging — b7706a9a Deployed Oct 6, 2026 by sg-s via level-1-tests (3.13, staging) #901
dev — b7706a9a Deployed Oct 6, 2026 by sg-s via level-1-tests (3.13, dev) #901
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-ready All merge-ready checks passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants