[SYNPY-1894] feat: synchronize active grid session by linglp · Pull Request #1440 · Sage-Bionetworks/synapsePythonClient

linglp · 2026-08-04T08:43:29Z

Problem

A grid session captures the schema version of its bound RecordSet at creation time and never picks up a newer version on its own. Once a data manager registers and binds a new JSON schema version to a RecordSet, contributors' already-open grid sessions have no way to see the new columns.

The old Grid.synchronize() only supports synchronizing recordset-based grid sessions.

Solution

Relaxed Grid.synchronize()/synchronize_async() to support both recordset-based grids and file-view based grid.
Added CurationTask.synchronize_active_grid_session()/_async() as the task-level "synchronize the tasks" function: it creates an active grid session first if the task doesn't have one yet, otherwise reuses the existing active session, then delegates to Grid.synchronize().

Design decision

For FileBasedMetadataTaskProperties, there's only one meaningful synchronize behavior — PULL_PUSH — since file-based grids write annotations directly back to files with no intermediate versioned state to preview; there's nothing to choose.

For RecordBasedMetadataTaskProperties, the choice between PULL and PULL_PUSH is consequential: PULL_PUSH commits the grid's current state as a new RecordSet version, an action with a lasting, visible side effect for every other collaborator on that RecordSet, while PULL does not commit anything.

If it defaulted to PULL_PUSH (which is the implicit default of the synapse server), users might be surprised by the JSON Schema version bump. If it defaulted to PULL, users might get an unexpected uncommitted preview when they meant to finalize all their changes and push them. Currently, the method requires the caller to explicitly state their intent and will raise an error if sync_type is not provided, rather than silently picking a default and letting the wrong choice surface later as a data surprise.

linglp · 2026-08-06T05:07:44Z

            task = cls().fill_from_dict(synapse_response=task_dict)
            yield task

+    @otel_trace_method(


There's integration test for grid_synchronize so I skipped adding integration test for this function synchronize_active_grid_session_async and only added a unit test.

linglp · 2026-08-06T05:27:37Z

 ```

-### Step 4: Download record-based metadata as a local CSV
+### Step 4: Synchronize the grid session to pick up schema updates


I currently place the synchronization step right after session creation (Step 3), framed around picking up a schema change before starting to edit. There's also a valid case for calling synchronize after importing local CSV edits (Step 6). Please let me know if you would want me to move it. @cconrad8

Copilot

Pull request overview

Adds explicit synchronization support for active Grid sessions used by curation tasks, enabling already-open sessions to pick up newer RecordSet schema versions and extending synchronization to file-view-backed grids.

Changes:

Introduces SyncType and updates Grid.synchronize() / synchronize_async() to accept a sync mode.
Adds CurationTask.synchronize_active_grid_session_async() (and sync wrapper) to create/reuse the active session and delegate to Grid.synchronize.
Expands unit/integration tests and updates curator docs/guides to document the new behavior and recommended workflows.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/unit/synapseclient/models/async/unit_test_curation_async.py	Adds unit tests for task-level active-session synchronization and `SynchronizeGridRequest` syncType serialization/validation.
tests/integration/synapseclient/models/async/test_grid_async.py	Updates/extends integration coverage for synchronizing entity-view and RecordSet-backed grid sessions.
synapseclient/models/curation.py	Adds `SyncType`, implements async task-level synchronization, and threads `sync_type` through grid sync requests.
synapseclient/models/init.py	Exports `SyncType` from the public `synapseclient.models` namespace.
docs/reference/experimental/sync/curator.md	Adds reference docs entry for `synchronize_active_grid_session` and `SyncType`.
docs/reference/experimental/async/curator.md	Adds reference docs entry for `synchronize_active_grid_session_async` and `SyncType`.
docs/guides/extensions/curator/metadata_contribution.md	Updates the contributor workflow guide to include a schema-refresh synchronization step and renumbers subsequent steps.

Suppressed comments (2)

synapseclient/models/curation.py:3914

Grid.synchronize() is documented and tested with string values (e.g., "PULL_PUSH"), and SynchronizeGridRequest.sync_type explicitly allows Union[SyncType, str]. Update the Grid.synchronize() type hint to include str so IDE/type-checking matches the supported API.

        self,
        *,
        sync_type: Optional[SyncType] = None,
        timeout: int = 120,
        synapse_client: Optional[Synapse] = None,

synapseclient/models/curation.py:5051

Grid.synchronize_async() is used with string values in integration tests (e.g., sync_type="PULL_PUSH"), and SynchronizeGridRequest.sync_type accepts Union[SyncType, str]. Update the sync_type annotation here as well so type hints reflect the supported input types.

        self,
        *,
        sync_type: Optional[SyncType] = None,
        timeout: int = 120,
        synapse_client: Optional[Synapse] = None,
    ) -> "Grid":

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+        client = Synapse.get_client(synapse_client=synapse_client)
+
+        if not self.task_properties:
+            await self.get_async(synapse_client=synapse_client)
+
+        if isinstance(self.task_properties, FileBasedMetadataTaskProperties):


thomasyu888

🔥 I'll defer to the team on the fine grained technical review after sage week but on a high level, this looks great - just had a tiny comment.

thomasyu888 · 2026-08-06T18:11:35Z

+- Download the CSV (Step 5) as a local reference, make your edits locally, then copy-paste the values back into the Grid UI
 - Make edits directly in the Synapse Grid UI — Step 3 prints the session URL (`https://www.synapse.org/Grid:default?sessionId=...`) after creating the session

 **Use `synchronize()` instead of `export_to_record_set()`.** After editing in the Grid UI, push your changes back to the underlying files:


For discussing, should we deprecate the export to record set function and push people to use this?

I actually noticed some problems if we remove export_to_record_set_async. As an example, the following is working as expected:

grid = Grid(session_id="MTM4Mzc0") grid = await grid.import_csv_async(path="test_grid_import.csv") await grid.export_to_record_set_async() grid = await task.synchronize_active_grid_session_async(sync_type="PULL") grid.download_csv(destination="/Users/lpeng/Downloads")

But if I remove export_to_record_set_async and change sync_type to PULL_PUSH, then I noticed that I can't actually "push" the changes from my CSV to the grid. I will ask other devs in curator-dev channel after Sage week.

andrewelamb

Looking good! I found a few issues we should address.

andrewelamb · 2026-08-10T15:32:57Z

+            )
+            active_grid_session_id = active_grid_session.session_id
+        else:
+            active_grid_session_id = status.execution_details.active_session_id


Should there be a guard for when there is no active session id?

hmm if there's no active session id, then the if statement should already create an active grid session.

The guard covers execution_details is None, but not execution_details being present with active_session_id set to None. In that path curation.py:2330 assigns None into Grid(session_id=None) and calls synchronize_async with a null session. active_session_id is declared str | None = None at curation.py:411 and is populated via .get("activeSessionId") at line 426, so a response with execution details but no active session yields exactly that. See this script as an example:

import uuid from synapseclient import Synapse from synapseclient.models import ( CurationTask, EntityView, FileBasedMetadataTaskProperties, Folder, GridExecutionDetails, Project, ViewTypeMask, ) syn = Synapse() syn.login() project = Project(name=f"demo_{uuid.uuid4()}").store() print(f"Created project {project.id}") try: folder = Folder(name=str(uuid.uuid4()), parent_id=project.id).store() entity_view = EntityView( name=str(uuid.uuid4()), parent_id=project.id, scope_ids=[folder.id], view_type_mask=ViewTypeMask.FILE.value, ).store() task = CurationTask( data_type=f"demo_data_type_{str(uuid.uuid4()).replace('-', '_')}", project_id=project.id, instructions="Demo of the missing activeSessionId guard.", task_properties=FileBasedMetadataTaskProperties( upload_folder_id=folder.id, file_view_id=entity_view.id, ), ).store() print(f"Created file-based CurationTask {task.task_id}") # Put the task into the state under discussion: executionDetails present, # activeSessionId absent. GridExecutionDetails.to_synapse_request() omits # activeSessionId when it is None, so the request body's executionDetails is # just {"concreteType": "...GridExecutionDetails"}. status = CurationTask(task_id=task.task_id).get_status() status.execution_details = GridExecutionDetails(active_session_id=None) print(f"Sending status update: {status.to_synapse_request()}") CurationTask(task_id=task.task_id).update_status(curation_task_status=status) status = CurationTask(task_id=task.task_id).get_status() print(f"execution_details : {status.execution_details}") print(f"execution_details is None : {status.execution_details is None}") if status.execution_details is not None: print(f"active_session_id : {status.execution_details.active_session_id}") # The guard in synchronize_active_grid_session only checks # `status.execution_details is None`, so this call takes the else-branch and # builds Grid(session_id=None) instead of creating a session. try: grid = CurationTask(task_id=task.task_id).synchronize_active_grid_session() print(f"synchronize_active_grid_session returned grid {grid.session_id}") except Exception as exc: print(f"synchronize_active_grid_session raised {type(exc).__name__}: {exc}") print() print( "Expected: either create a grid session for this task (same as the\n" "executionDetails-is-None path), or raise an error naming the task, e.g.\n" f'"CurationTask {task.task_id} has grid execution details but no active ' 'session id."' ) finally: Project(id=project.id).delete() print(f"Deleted project {project.id}")

Thanks for the example. I am curious why would someone do something like below in practice?

status = CurationTask(task_id=task.task_id).get_status() status.execution_details = GridExecutionDetails(active_session_id=None) CurationTask(task_id=task.task_id).update_status(curation_task_status=status)

Based on the docstring, updating status should be something like:

status = CurationTask(task_id=task.task_id).get_status() current.state = TaskState.COMPLETED updated = task.update_status(curation_task_status=current)

That said, it doesn't hurt to add the guard, so I'll add it.

andrewelamb

I found a few thigns that shoudl be adressed.

andrewelamb · 2026-08-12T14:44:57Z

+
+        if isinstance(self.task_properties, FileBasedMetadataTaskProperties):
+            if sync_type is not None and sync_type != SyncType.PULL_PUSH:
+                client.logger.warning(


I'm thinking this should be a ValueError or a TypeError rather than a warning.

I think for record-based grids, sync_type = None is ambiguous. But for file-based grids, PULL_PUSH is the only valid value, so I think it is safe to log a warning and override the user's input?

andrewelamb · 2026-08-12T14:52:42Z

+
+A Grid session captures the JSON schema in place at the moment it's created — it does not automatically pick up a newer schema version. If the administrator adds or changes a column in the schema *after* you created your session in Step 3, synchronize the session to pull in the latest schema and data from the RecordSet before you continue working.
+
+For record-based grids, `sync_type` is required — you must explicitly choose `"PULL"` or `"PULL_PUSH"`:


This is a bit unclear to users. sync_type is actually optional and defaults to None. What happens then? What about file-based grids?

sync_type is optional for file-based grids because PULL_PUSH is the only supported value — there's nothing to choose. When omitted (or set to anything else), it defaults to PULL_PUSH and a warning is logged if you passed something different. Since this section is only for record-based task, I didn't include the explanation of file-based task. But if you look at the section of "File-Based Curation Tasks", you should be able to see the note!

"For record-based grids, sync_type is required — you must explicitly choose "PULL" or "PULL_PUSH":"

async def synchronize_async( self, *, sync_type: Optional[Union[SyncType, str]] = None, timeout: int = 120, synapse_client: Optional[Synapse] = None, )

sync_type is not required

BryanFauble

I have a few questions on behavior

andrewelamb

I find a few more thigns to fix, but ovberall LGTM

andrewelamb · 2026-08-13T15:21:06Z

        """
        return Grid()

+    def synchronize_active_grid_session(


The types and docstring need to be updated here to match the async version

andrewelamb · 2026-08-13T15:26:38Z

+
+A Grid session captures the JSON schema in place at the moment it's created — it does not automatically pick up a newer schema version. If the administrator adds or changes a column in the schema *after* you created your session in Step 3, synchronize the session to pull in the latest schema and data from the RecordSet before you continue working.
+
+For record-based grids, `sync_type` is required — you must explicitly choose `"PULL"` or `"PULL_PUSH"`:


"For record-based grids, sync_type is required — you must explicitly choose "PULL" or "PULL_PUSH":"

async def synchronize_async( self, *, sync_type: Optional[Union[SyncType, str]] = None, timeout: int = 120, synapse_client: Optional[Synapse] = None, )

sync_type is not required

andrewelamb · 2026-08-13T15:31:56Z

+                "synapseclient.models.curation.get_curation_task_status",
+                new_callable=AsyncMock,
+                return_value=_get_curation_task_status_response(),
+            ) as mock_get_status,


nit: mock_get_status doesn't appear to get used anywhere

andrewelamb · 2026-08-13T15:33:19Z

+    def init_syn(self, syn: Synapse) -> None:
+        self.syn = syn
+
+    async def test_record_based_creates_session_when_none_active(self) -> None:


This needs to be renamed, see line 1439 below

linglp added 3 commits July 31, 2026 14:12

add sync type to synchronize request

9beeeee

fix test; make sure SyncType could accept both string and enum field

9944b83

add synchronize method to curation task class

c7b2d42

linglp changed the title ~~[SYNPY-1894]~~ [SYNPY-1894] feat: synchronize active grid session Aug 5, 2026

linglp added 4 commits August 5, 2026 02:01

added unit tests

be12e60

added integration tests

4d07296

add method in doc

1b81508

update documentation

5f8d252

linglp commented Aug 6, 2026

View reviewed changes

linglp marked this pull request as ready for review August 6, 2026 05:27

Copilot AI lite review requested due to automatic review settings August 6, 2026 05:27

linglp requested a review from a team as a code owner August 6, 2026 05:27

Copilot started reviewing on behalf of linglp August 6, 2026 05:28 View session

linglp requested a review from cconrad8 August 6, 2026 05:30

Copilot AI reviewed Aug 6, 2026

View reviewed changes

Comment thread synapseclient/models/curation.py

Comment on lines +2284 to +2289

client = Synapse.get_client(synapse_client=synapse_client)

if not self.task_properties:

await self.get_async(synapse_client=synapse_client)

if isinstance(self.task_properties, FileBasedMetadataTaskProperties):

simplify the doc

eef80c0

thomasyu888 reviewed Aug 6, 2026

View reviewed changes

andrewelamb requested changes Aug 10, 2026

View reviewed changes

linglp added 3 commits August 10, 2026 18:51

resolve conflicts

a06ae49

raise error message for unsupported type

b1152b2

make sure that sync type is coerced

931910d

linglp requested a review from andrewelamb August 11, 2026 01:53

andrewelamb requested changes Aug 12, 2026

View reviewed changes

thomasyu888 requested a review from BryanFauble August 12, 2026 16:54

BryanFauble requested changes Aug 12, 2026

View reviewed changes

Comment thread synapseclient/models/curation.py Outdated

Comment thread synapseclient/models/curation.py Outdated

Comment thread synapseclient/models/curation.py Outdated

linglp added 4 commits August 13, 2026 03:39

Merge branch 'develop' into SYNPY-1894

05bd260

added class ForwardCompatibleStrEnum

c2b3ca5

if there is no active grid session, changed the behavior to no op

110d0c7

fix type hints

c48924e

add a guard

60e155e

linglp requested review from BryanFauble and andrewelamb August 13, 2026 09:57

andrewelamb approved these changes Aug 13, 2026

View reviewed changes


		A Grid session captures the JSON schema in place at the moment it's created — it does not automatically pick up a newer schema version. If the administrator adds or changes a column in the schema after you created your session in Step 3, synchronize the session to pull in the latest schema and data from the RecordSet before you continue working.

		For record-based grids, `sync_type` is required — you must explicitly choose `"PULL"` or `"PULL_PUSH"`:

Uh oh!

Conversation

linglp commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Design decision

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

thomasyu888 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewelamb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linglp Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linglp Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrewelamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linglp Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BryanFauble left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewelamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

linglp commented Aug 4, 2026 •

edited

Loading

thomasyu888 left a comment •

edited

Loading

linglp Aug 11, 2026 •

edited

Loading

linglp Aug 13, 2026 •

edited

Loading

linglp Aug 13, 2026 •

edited

Loading