Skip to content

Commit 6192d08

Browse files
dotsdlclaude
andcommitted
Fix inverted EXTENDS direction in _TransformationData.to_subgraph
The previous to_subgraph emitted Relationship.type("EXTENDS")(etask_node, task_node, ...) which creates ``(etask)-[:EXTENDS]->(task)``, i.e. "extended_task extends task" -- the opposite of what the source graph encoded: OPTIONAL MATCH (task)-[:EXTENDS]->(extended_task:Task) and what every other place in the store writes (``Neo4jStore.create_tasks`` at line 3424: ``Relationship.type("EXTENDS")(task_node, extends_task_node, ...)``) and reads (lines 2392, 3152, 3547, 3634, 4219, all traversing ``(task)-[:EXTENDS]->(other_task)``). No existing test exercised the cloned EXTENDS direction (the store- level test_merge_networks only counted PDRRs, and my own client tests used independent Tasks with no EXTENDS chain), so the bug shipped silently as part of the original PR's inline to_subgraph. Swap the argument order and add a focused regression test: - test_copy_network_preserves_extends_direction stages a base complete Task plus an extending complete Task on the same Transformation (using ``create_tasks(transformation_sks, extends=base_task_sks)``), copies the source network into a different scope, then asserts via raw Cypher that exactly one EXTENDS edge exists in the target scope with ``extender == extending_task.gufe_key`` and ``extended == base_task.gufe_key`` -- i.e. the direction matches the source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e6b7aa6 commit 6192d08

3 files changed

Lines changed: 74 additions & 10 deletions

File tree

alchemiscale/interface/client.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -293,13 +293,11 @@ def copy_network(
293293
existing Task (regardless of status) and its associated
294294
ProtocolDAGResultRefs.
295295
296-
Cloned Tasks are wired to their Transformations via ``PERFORMS`` and
297-
are reachable through standard network traversals
298-
(``get_network_tasks``, ``get_network_results``, etc.). They are
299-
intentionally **not** actioned to the new network's TaskHub; to
300-
pick up errored or waiting Tasks for execution on the merged
301-
network, call :meth:`action_tasks` with the new network's
302-
``ScopedKey`` after the copy completes.
296+
Cloned Tasks are intentionally **not** actioned; to pick up errored or
297+
waiting Tasks for execution on the merged network, call
298+
:meth:`action_tasks` with the new network's ``ScopedKey`` after the
299+
copy completes, and set the Tasks' status to back to `waiting` with
300+
:meth:`set_tasks_status`.
303301
304302
Parameters
305303
----------
@@ -377,7 +375,7 @@ def merge_scopes(
377375
378376
Each source AlchemicalNetwork is copied via :meth:`copy_network`,
379377
preserving its name, Tasks, and ProtocolDAGResultRefs. Cloned
380-
Tasks are not actioned to the target network's TaskHub.
378+
Tasks are not actioned.
381379
382380
Parameters
383381
----------

alchemiscale/storage/statestore.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,15 +238,21 @@ def record_to_node(record):
238238
# this edge the Task is unreachable from get_network_tasks
239239
# and every other PERFORMS-based traversal
240240
subgraph |= Relationship.type("PERFORMS")(task_node, tf_node, **scope_props)
241-
# create the task node this task extends if it exists
241+
# create the task node this task extends if it exists. The
242+
# source query returns ``record["extended_task"]`` as the Task
243+
# that ``record["task"]`` extends -- i.e. in the source graph,
244+
# ``(task)-[:EXTENDS]->(extended_task)``. The relationship's
245+
# direction must be preserved on the clone so the standard
246+
# downstream traversals (e.g. lines 2392, 3152, 3547, 3634:
247+
# ``(task)-[:EXTENDS]->(other_task)``) keep working.
242248
etask_node = (
243249
None
244250
if not record["extended_task"]
245251
else record_to_node(record["extended_task"])
246252
)
247253
if etask_node:
248254
subgraph |= Relationship.type("EXTENDS")(
249-
etask_node, task_node, **scope_props
255+
task_node, etask_node, **scope_props
250256
)
251257
# clone all result refs for the task
252258
for pdrr_record in record["pdrrs"]:

alchemiscale/tests/integration/interface/client/test_client.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,66 @@ def test_copy_network_with_rename(
536536
copied_an = user_client.get_network(copied_sk)
537537
assert copied_an.name == new_name
538538

539+
def test_copy_network_preserves_extends_direction(
540+
self,
541+
scope_test,
542+
multiple_scopes,
543+
n4js_preloaded,
544+
user_client: client.AlchemiscaleClient,
545+
):
546+
"""The cloned EXTENDS edge must match the source direction:
547+
``(task)-[:EXTENDS]->(extended_task)``. Regressing this direction
548+
would silently invert every EXTENDS chain on copied networks and
549+
break the downstream traversals that read it forward (e.g.
550+
``get_task_extends``, ``get_task_results``)."""
551+
source_sks = user_client.query_networks(scope=scope_test, state=None)
552+
source_sk = source_sks[0]
553+
554+
transformation_sks = n4js_preloaded.get_network_transformations(source_sk)
555+
assert transformation_sks
556+
557+
# base Task: complete with ok PDRR
558+
base_task_sks = n4js_preloaded.create_tasks(transformation_sks[:1])
559+
n4js_preloaded.set_task_running(base_task_sks)
560+
n4js_preloaded.set_task_complete(base_task_sks)
561+
base_pdrr = ProtocolDAGResultRef(
562+
obj_key=f"ProtocolDAGResult-{uuid.uuid4()}",
563+
scope=base_task_sks[0].scope,
564+
ok=True,
565+
)
566+
n4js_preloaded.set_task_result(base_task_sks[0], base_pdrr)
567+
568+
# extending Task: extends the base, also complete; the EXTENDS edge
569+
# in the source graph is ``(extending)-[:EXTENDS]->(base)``
570+
extending_task_sks = n4js_preloaded.create_tasks(
571+
transformation_sks[:1], extends=base_task_sks
572+
)
573+
n4js_preloaded.set_task_running(extending_task_sks)
574+
n4js_preloaded.set_task_complete(extending_task_sks)
575+
ext_pdrr = ProtocolDAGResultRef(
576+
obj_key=f"ProtocolDAGResult-{uuid.uuid4()}",
577+
scope=extending_task_sks[0].scope,
578+
ok=True,
579+
)
580+
n4js_preloaded.set_task_result(extending_task_sks[0], ext_pdrr)
581+
582+
target_scope = multiple_scopes[2]
583+
user_client.copy_network(network=source_sk, scope=target_scope, visualize=False)
584+
585+
# exactly one EXTENDS edge between the two cloned Tasks must exist
586+
# in the target scope, with the same direction as the source:
587+
# extending -> EXTENDS -> base
588+
edges = n4js_preloaded.execute_query(
589+
"""
590+
MATCH (a:Task {`_project`: $project})-[:EXTENDS]->(b:Task {`_project`: $project})
591+
RETURN a._gufe_key AS extender, b._gufe_key AS extended
592+
""",
593+
project=target_scope.project,
594+
).records
595+
assert len(edges) == 1
596+
assert edges[0]["extender"] == str(extending_task_sks[0].gufe_key)
597+
assert edges[0]["extended"] == str(base_task_sks[0].gufe_key)
598+
539599
def test_copy_network_rejects_wildcard_scope(
540600
self,
541601
n4js_preloaded,

0 commit comments

Comments
 (0)