Add timing by pendingintent · Pull Request #42 · pendingintent/soa-workbench

pendingintent · 2025-12-17T16:26:14Z

Added USDM JSON generators for arms and epochs.

Added new Stuidy Timing editor as a preliminary step to introduce ScheduledActivityInstances to the SOA

…C201264

…ended tests

src/soa_builder/web/routers/timings.py

+        window_lower=window_lower,
+    )
+    create_timing(soa_id, payload)
+    return RedirectResponse(url=f"/ui/soa/{soa_id}/timings", status_code=303)


src/soa_builder/web/routers/timings.py

+        window_lower=window_lower,
+    )
+    update_timing(soa_id, timing_id, payload)
+    return RedirectResponse(url=f"/ui/soa/{soa_id}/timings", status_code=303)


src/soa_builder/web/routers/timings.py

+@router.post("/ui/soa/{soa_id}/timings/{timing_id}/delete")
+def ui_delete_timing(request: Request, soa_id: int, timing_id: int):
+    delete_timing(soa_id, timing_id)
+    return RedirectResponse(url=f"/ui/soa/{soa_id}/timings", status_code=303)


Copilot

Pull request overview

This PR introduces a Study Timing editor as part of the USDM (Unified Study Definition Model) integration effort. The changes enable users to create, edit, and manage timing information for Schedule of Activities (SoA), laying groundwork for ScheduledActivityInstances.

Key changes:

New timing CRUD endpoints with audit trail support
USDM JSON generators for arms, epochs, and timings
UI templates for managing study timing entities
Code junction logic to prevent duplicate code entries when updating timing attributes

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`src/soa_builder/web/routers/timings.py`	Core timing CRUD API and UI endpoints with code junction logic
`src/soa_builder/web/schemas.py`	Pydantic schemas for timing creation and updates
`src/soa_builder/web/templates/timings.html`	HTML template for timing management UI
`src/soa_builder/web/templates/base.html`	Navigation link added for Study Timing page
`src/soa_builder/web/initialize_database.py`	Database schema for timing and timing_audit tables
`src/soa_builder/web/audit.py`	Audit trail function for timing operations
`src/soa_builder/web/utils.py`	Helper function to fetch study timing types from DDF terminology
`src/soa_builder/web/app.py`	Router registration and schema relocation
`src/usdm/generate_study_timings.py`	USDM JSON generator for timings
`src/usdm/generate_study_epochs.py`	USDM JSON generator for epochs
`src/usdm/generate_arms.py`	USDM JSON generator for arms
`tests/test_timings.py`	API endpoint tests for timing CRUD operations
`tests/test_timing_audit.py`	Database-level audit trail verification tests
`tests/test_timing_audit_endpoint.py`	HTTP endpoint tests for timing audit
`tests/test_timings_code_junction.py`	Tests verifying code junction behavior for type and relative_to_from fields

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

src/soa_builder/web/utils.py

Copilot · 2025-12-17T16:30:05Z

src/usdm/generate_study_epochs.py

+    str,
+    str,
+]:
+    logger = logging.getLogger("usdm.generate_arms")


Logger name should be 'usdm.generate_epochs' to match the module name, not 'usdm.generate_arms'.

Copilot · 2025-12-17T16:30:05Z

src/usdm/generate_study_epochs.py

+    else:
+        content = resp.json()
+        parsed_url = urlparse(url)
+        code_system = parsed_url.scheme + "//:" + parsed_url.netloc


URL scheme construction is incorrect. The format 'scheme://:netloc' creates an invalid URL. Should be 'scheme://netloc' (removing the colon after //).

Suggested change

code_system = parsed_url.scheme + "//:" + parsed_url.netloc

code_system = parsed_url.scheme + "://" + parsed_url.netloc

Copilot · 2025-12-17T16:30:05Z

src/usdm/generate_study_epochs.py

+    return s or None
+
+
+def _get_epoch_code_values(soa_id: int, type: str, code: str) -> Tuple[


Parameter 'type' shadows the built-in type. Consider renaming to 'type_value' or 'epoch_type'.

Copilot · 2025-12-17T16:30:05Z

src/usdm/generate_study_epochs.py

+
+
+def build_usdm_activities(soa_id: int) -> List[Dict[str, Any]]:
+    """


Function name 'build_usdm_activities' is misleading; it builds epochs, not activities. Should be renamed to 'build_usdm_epochs'.

Suggested change

"""

"""

Backwards-compatible wrapper for :func:`build_usdm_epochs`.

This wrapper is retained to avoid breaking existing callers that still

reference :func:`build_usdm_activities`. New code should call

:func:`build_usdm_epochs` directly.

"""

return build_usdm_epochs(soa_id)

def build_usdm_epochs(soa_id: int) -> List[Dict[str, Any]]:

"""

Copilot · 2025-12-17T16:30:06Z

src/usdm/generate_study_epochs.py

+      - previousId?: string | null
+      - nextId?: string | null
+      - notes?: string[]
+      - instanceType: "Activity"


Documentation incorrectly states instanceType as 'Activity' when the code sets it to 'StudyEpoch' (line 150). Update documentation to match implementation.

Suggested change

- instanceType: "Activity"

- instanceType: "StudyEpoch"

src/usdm/generate_arms.py

Copilot · 2025-12-17T16:30:06Z

src/usdm/generate_arms.py

+        - decode: string
+        - instanceTye: "Code"
+        }
+      - popultionIds?: int[]


Corrected spelling of 'popultionIds' to 'populationIds'.

Suggested change

- popultionIds?: int[]

- populationIds?: int[]

Copilot · 2025-12-17T16:30:07Z

src/usdm/generate_study_epochs.py

+                decode = term.get("submissionValue")
+
+    return code_system, code_system_version, decode


Variable 'decode' is only assigned inside the for loop when a matching term is found. If no match is found, the function returns an undefined variable, causing a NameError. Initialize decode to a default value (e.g., None or empty string) before the loop.

Spelling Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 10 comments.

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

Copilot · 2025-12-17T16:52:19Z

src/usdm/generate_study_epochs.py

+    str,
+    str,
+]:
+    logger = logging.getLogger("usdm.generate_arms")


Logger name 'usdm.generate_arms' does not match the module name. It should be 'usdm.generate_epochs' to correctly identify log messages from this module.

Suggested change

logger = logging.getLogger("usdm.generate_arms")

logger = logging.getLogger("usdm.generate_epochs")

Copilot · 2025-12-17T16:52:19Z

src/usdm/generate_study_epochs.py

+    return code_system, code_system_version, decode
+
+
+def build_usdm_activities(soa_id: int) -> List[Dict[str, Any]]:


Function name 'build_usdm_activities' is misleading. It should be named 'build_usdm_epochs' to match the module's purpose and the returned data structure which contains StudyEpoch objects.

Copilot · 2025-12-17T16:52:19Z

src/usdm/generate_study_epochs.py

+        code_system, code_system_version, decode = _get_epoch_code_values(
+            soa_id, type, code
+        )


Function '_get_epoch_code_values' may not return all three expected values if the API call fails or no matching term is found, leading to unpacking errors. Ensure the function always returns a tuple of three values or handle potential exceptions.

Suggested change

code_system, code_system_version, decode = _get_epoch_code_values(

soa_id, type, code

)

try:

code_system, code_system_version, decode = _get_epoch_code_values(

soa_id, type, code

)

except Exception:

code_system = None

code_system_version = None

decode = None

Copilot · 2025-12-17T16:52:20Z

src/usdm/generate_arms.py

+    return s or None
+
+
+def _get_type_code_tuple(soa_id: int, code_uid: str) -> Tuple[str, str, str, str]:


Return type should be Tuple[List[str], List[str], List[str], List[str]] as the function returns lists, not individual strings. The return statement shows lists being returned for all four values.

Copilot · 2025-12-17T16:52:20Z

src/usdm/generate_arms.py

+    code_decode = [r[2] for r in rows]
+    code_system_version = [r[3] for r in rows]
+
+    return code_code, code_decode, code_system, code_system_version


Return type should be Tuple[List[str], List[str], List[str], List[str]] as the function returns lists, not individual strings. The return statement shows lists being returned for all four values.

Copilot · 2025-12-17T16:52:20Z

src/usdm/generate_arms.py

+def _get_data_origin_type_tuple(
+    soa_id: int, code_uid: str
+) -> Tuple[str, str, str, str]:


Return type should be Tuple[List[str], List[str], List[str], List[str]] as the function returns lists, not individual strings. The return statement shows lists being returned for all four values.

Copilot · 2025-12-17T16:52:20Z

src/usdm/generate_arms.py

+    code_decode = [r[2] for r in rows]
+    code_system_version = [r[3] for r in rows]
+
+    return code_code, code_decode, code_system, code_system_version


Return type should be Tuple[List[str], List[str], List[str], List[str]] as the function returns lists, not individual strings. The return statement shows lists being returned for all four values.

Copilot · 2025-12-17T16:52:20Z

src/soa_builder/web/templates/base.html

    <a href="/">Home</a> |
+    <a href="/ui/soa/{{ soa_id }}/timings">Study Timing</a> |


The template variable 'soa_id' may be undefined on pages where no SOA context exists, causing broken links or template errors. Consider conditionally rendering this link only when 'soa_id' is available.

Suggested change

<a href="/">Home</a> |

<a href="/ui/soa/{{ soa_id }}/timings">Study Timing</a> |

<a href="/">Home</a> |

{% if soa_id %}

<a href="/ui/soa/{{ soa_id }}/timings">Study Timing</a> |

{% endif %}

Copilot · 2025-12-17T16:52:21Z

src/soa_builder/web/routers/timings.py

+        sv_to_code = {}
+    # Mapping for Relative To/From (C201265)
+    try:
+        sv_to_code_rtf = get_study_timing_type("C201265")


Variable name 'sv_to_code_rtf' is ambiguous. Consider renaming to 'sv_to_code_relative_to_from' for clarity.

Copilot · 2025-12-17T16:52:21Z

src/soa_builder/web/app.py

 """Visit update handled in routers/visits.py"""
-
-
 """Visit detail handled in routers/visits.py"""
-
-
 """Activity creation handled in routers/activities.py"""
-
-
 """Activity update handled in routers/activities.py"""
-
-
 """Activity detail handled in routers/activities.py"""
-
-
 """Epoch CRUD and reorder endpoints refactored into epochs_router."""


These comment strings lack context and do not follow proper docstring or comment formatting. Consider removing them or converting to proper single-line comments with '#' for better code clarity.

Spelling Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…kbench into add-timing

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

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

Copilot · 2025-12-17T16:58:58Z

src/usdm/generate_study_epochs.py

+    str,
+    str,
+]:
+    logger = logging.getLogger("usdm.generate_arms")


Logger name 'usdm.generate_arms' is incorrect for the epochs module. It should be 'usdm.generate_epochs' to match the module name.

Copilot · 2025-12-17T16:58:58Z

src/usdm/generate_study_epochs.py

+        top_terms = content.get("terms")
+        for term in top_terms:
+            if term.get("conceptId") == code:
+                decode = term.get("submissionValue")


Variable 'decode' is assigned inside a conditional loop but used outside without initialization. If no matching term is found, 'decode' will be undefined when returned, causing a NameError.

Copilot · 2025-12-17T16:58:58Z

src/usdm/generate_study_epochs.py

+    return s or None
+
+
+def _get_epoch_code_values(soa_id: int, type: str, code: str) -> Tuple[


The _get_epoch_code_values function lacks test coverage. Consider adding tests to verify the CDISC API integration, error handling when the API is unavailable, and correct parsing of the response.

Copilot · 2025-12-17T16:58:58Z

src/soa_builder/web/routers/timings.py

+                used_nums.add(int(tail))
+            else:
+                logger.warning(
+                    "Invalid timing_uid format encountered (ignored): %s",


The warning message 'Invalid timing_uid format encountered (ignored)' could be more helpful by explaining what the expected format is. Consider: 'Invalid timing_uid format encountered (expected Timing_): %s'

Suggested change

"Invalid timing_uid format encountered (ignored): %s",

"Invalid timing_uid format encountered (expected Timing_<number>, ignored): %s",

pendingintent added 8 commits December 15, 2025 14:10

Study Arms USDM JSON generator

6c69a5a

Generate Epoch USDM JSON

a8c75be

API endpoints, database, auditing for timings

4dce479

Added tests for timings endpoints

fbf8aec

Added endpoints & UI page for Study Timings

f764242

Generate the Study Timings USDM JSON

4ccc02a

Added timing type selection from ddf_terminology where codelist_code=…

c7f2a18

…C201264

New Code_N value each time a type, relativeToFrom mapping chosen; ext…

d134658

…ended tests

Copilot AI review requested due to automatic review settings December 17, 2025 16:26

github-advanced-security bot found potential problems Dec 17, 2025

View reviewed changes

pendingintent self-assigned this Dec 17, 2025

pendingintent added the enhancement New feature or request label Dec 17, 2025

pendingintent added this to the v1.2-beta milestone Dec 17, 2025

Copilot AI reviewed Dec 17, 2025

View reviewed changes

Update src/soa_builder/web/utils.py

3d7198a

Spelling Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings December 17, 2025 16:46

Updated to populated all properties

89c2167

Copilot AI reviewed Dec 17, 2025

View reviewed changes

Update src/usdm/generate_arms.py

2569e9c

Spelling Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings December 17, 2025 16:55

Merge branch 'add-timing' of https://github.com/pendingintent/soa-wor…

a33582b

…kbench into add-timing

Copilot AI reviewed Dec 17, 2025

View reviewed changes

Fixed syntax, added conditionals

8e267df

pendingintent merged commit 7cd6f28 into master Dec 17, 2025
6 checks passed

	code_system = parsed_url.scheme + "//:" + parsed_url.netloc
	code_system = parsed_url.scheme + "://" + parsed_url.netloc

		return s or None


		def _get_epoch_code_values(soa_id: int, type: str, code: str) -> Tuple[



		def build_usdm_activities(soa_id: int) -> List[Dict[str, Any]]:
		"""

-    """
+    """
+    Backwards-compatible wrapper for :func:`build_usdm_epochs`.
+    This wrapper is retained to avoid breaking existing callers that still
+    reference :func:`build_usdm_activities`. New code should call
+    :func:`build_usdm_epochs` directly.
+    """
+    return build_usdm_epochs(soa_id)
+def build_usdm_epochs(soa_id: int) -> List[Dict[str, Any]]:
+    """

		decode = term.get("submissionValue")

		return code_system, code_system_version, decode

	logger = logging.getLogger("usdm.generate_arms")
	logger = logging.getLogger("usdm.generate_epochs")

		return s or None


		def _get_type_code_tuple(soa_id: int, code_uid: str) -> Tuple[str, str, str, str]:

		<a href="/">Home</a> \|
		<a href="/ui/soa/{{ soa_id }}/timings">Study Timing</a> \|

	"Invalid timing_uid format encountered (ignored): %s",
	"Invalid timing_uid format encountered (expected Timing_<number>, ignored): %s",

Conversation

pendingintent commented Dec 17, 2025

Uh oh!

Check warning

Uh oh!

Copilot Autofix

Check warning

Uh oh!

Copilot Autofix

Check warning

Uh oh!

Copilot Autofix

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment