Update study cells

[pendingintent]

Updated requirements.txt
Upated encounters
Update study cells

[Copilot]

Pull request overview

This PR refactors and expands the SoA Workbench UI/API around study-level navigation and study cells: it introduces a dedicated Study Cells page/router, updates several UI headings to show study metadata, and changes the encounters reorder API contract/path (with corresponding test updates).

Changes:

• Added a new routers/cells.py router + study_cells.html page to create/delete/reorder study cells (and updated tests to enforce per-row unique StudyCell UIDs).
• Updated multiple UI pages to display study_label/study_name instead of raw soa_id, and added a Help page + navigation links.
• Changed visits reorder endpoint usage to POST /soa/{soa_id}/visits/reorder with JSON body { "order": [...] }, updating templates/tests accordingly.

Reviewed changes

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

Show a summary per file

File	Description
tests/test_study_cell_uid_reuse_later.py	Updates assertions/endpoint to enforce unique StudyCell UID per row and new UI create route.
tests/test_study_cell_uid_reuse.py	Renames test and changes expectations to UID uniqueness + new UI create route.
tests/test_routers_visits.py	Updates reorder endpoint path/body to {order: [...]}.
src/soa_builder/web/templates/timings.html	Header updated to display study label/name.
src/soa_builder/web/templates/study_cells.html	New Study Cells UI with create/delete + client-side reordering controls.
src/soa_builder/web/templates/schedule_timelines.html	Header updated to display study label/name.
src/soa_builder/web/templates/rules.html	Header updated to display study label/name.
src/soa_builder/web/templates/instances.html	Header updated to display study label/name.
src/soa_builder/web/templates/help.html	New Help/How-To page content.
src/soa_builder/web/templates/epochs.html	Header updated to display study label/name.
src/soa_builder/web/templates/encounters.html	Adds encounter reorder UI controls + header updated to display study label/name.
src/soa_builder/web/templates/elements.html	Header updated to display study label/name.
src/soa_builder/web/templates/edit.html	Removes inline Study Cells section; tweaks edit header.
src/soa_builder/web/templates/base.html	Adds nav links for Study Cells and Help.
src/soa_builder/web/templates/arms.html	Header updated to display study label/name.
src/soa_builder/web/schemas.py	Adds StudyCellCreate/StudyCellUpdate schemas.
src/soa_builder/web/routers/visits.py	Adds study metadata to template context; changes reorder route path/body contract.
src/soa_builder/web/routers/timings.py	Adds study metadata to template context.
src/soa_builder/web/routers/schedule_timelines.py	Adds study metadata to template context.
src/soa_builder/web/routers/rules.py	Adds study metadata to template context.
src/soa_builder/web/routers/instances.py	Adds study metadata to template context.
src/soa_builder/web/routers/epochs.py	Adds study metadata to template context; adjusts reorder comment.
src/soa_builder/web/routers/elements.py	Adds study metadata to template context.
src/soa_builder/web/routers/cells.py	New Study Cells API/UI router (CRUD + reorder + UID generation).
src/soa_builder/web/routers/arms.py	Adds study metadata to template context.
src/soa_builder/web/migrate_database.py	Adds migration to introduce study_cell.order_index.
src/soa_builder/web/app.py	Registers new router/migration; changes instance ordering; adds /ui/help; removes old inline study-cell UI endpoints.
requirements.txt	Removes several unused dependencies.

[Copilot]

reorder_study_cells_api() is new behavior (mutates ordering + writes audit) but there are no tests covering success cases, invalid IDs, and empty-order behavior. Adding a small router test similar to tests/test_routers_visits.py would help prevent regressions.

[Copilot]

/soa/{soa_id}/visits/reorder is now defined here, but the same method+path already exists in src/soa_builder/web/app.py (app-level route). Because include_router() happens before the app-level route definition, the app-level handler will override this router handler, and it expects a different request body shape (raw list vs {order: [...]}), which will break the UI/tests added in this PR. Remove/disable the duplicate in app.py or change one of the paths so only one handler owns this route and the request body contract is consistent.

Suggested change

	order: List[int] = Body(..., embed=True),
	order: List[int] = Body(...),

[Copilot]

The study metadata fetch block is duplicated across many routers (visits, arms, epochs, elements, timings, etc.). This repetition increases maintenance cost and risk of inconsistencies. Consider factoring it into a shared helper (e.g., utils.get_study_meta(soa_id) that returns a dict) and reusing it across routers.

[Copilot]

Save Order is always enabled; when there are no study cells, the JS posts an empty order array, but the API rejects empty lists with 400. Consider disabling/hiding the button when there are 0 (or <2) rows, or treat an empty order as a no-op success in the API.

[Copilot]

Typos in this help text reduce clarity (e.g., "Scheuled", "Vist", "wil", "Optioinally"). Please correct the spelling/grammar so the instructions read cleanly.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

src/soa_builder/web/app.py:3878

• Unused data: The study_cells variable is fetched via _list_study_cells(soa_id) on line 3737 and passed to the template on line 3878, but the study cells section was removed from edit.html in this PR. Consider removing this fetch to avoid unnecessary database queries, or update the comment on line 1088 to clarify that the function is still used by the edit page even though it's redundant.

    study_cells = _list_study_cells(soa_id)

    # Transition Rules list
    conn_tr = _connect()
    cur_tr = conn_tr.cursor()
    cur_tr.execute(
        "SELECT transition_rule_uid,name,label,description,text,order_index,created_at FROM transition_rule WHERE soa_id=? ORDER BY order_index",
        (soa_id,),
    )
    transition_rules = [
        dict(
            transition_rule_uid=r[0],
            name=r[1],
            label=r[2],
            description=r[3],
            text=r[4],
            order_index=r[5],
            created_at=r[6],
        )
        for r in cur_tr.fetchall()
    ]
    conn_tr.close()

    # Load Timings for dropdown
    conn_tm = _connect()
    cur_tm = conn_tm.cursor()
    cur_tm.execute(
        "SELECT id,name FROM timing WHERE soa_id=? ORDER BY id",
        (soa_id,),
    )
    timings = [{"id": r[0], "name": r[1]} for r in cur_tm.fetchall()]
    conn_tm.close()

    # Load instances
    conn_inst = _connect()
    cur_inst = conn_inst.cursor()
    cur_inst.execute(
        """
        SELECT i.id,i.name,i.instance_uid,i.label,i.member_of_timeline,st.name AS timeline_name,st.label AS timeline_label,
        v.name AS encounter_name,v.label AS encounter_label,e.name AS epoch_name,e.epoch_label as epoch_label,tm.window_label,tm.label AS timing_label,tm.name AS timing_name,tm.value AS study_day
        FROM instances i
        LEFT JOIN schedule_timelines st ON st.schedule_timeline_uid = i.member_of_timeline AND st.soa_id = i.soa_id
        LEFT JOIN visit v ON v.encounter_uid = i.encounter_uid AND v.soa_id = i.soa_id
        LEFT JOIN epoch e ON e.epoch_uid = i.epoch_uid AND e.soa_id = i.soa_id
        LEFT JOIN timing tm ON tm.id = v.scheduledAtId AND tm.soa_id = v.soa_id
        WHERE i.soa_id=?
        ORDER BY COALESCE(i.member_of_timeline, 'zzz'), i.order_index, i.id
            """,
        (soa_id,),
    )
    instances = [
        {
            "id": r[0],
            "name": r[1],
            "instance_uid": r[2],
            "label": r[3],
            "member_of_timeline": r[4],
            "timeline_name": r[5],
            "timeline_label": r[6],
            "encounter_name": r[7],
            "encounter_label": r[8],
            "epoch_name": r[9],
            "epoch_label": r[10],
            "window_label": r[11],
            "timing_label": r[12],
            "timing_name": r[13],
            "study_day": iso_duration_to_days(r[14]),
        }
        for r in cur_inst.fetchall()
    ]
    cur_inst.close()

    # Load Schedule Timelines for timeline selector
    conn_tl = _connect()
    cur_tl = conn_tl.cursor()
    cur_tl.execute(
        """
        SELECT schedule_timeline_uid,name,main_timeline
        FROM schedule_timelines
        WHERE soa_id=?
        ORDER BY main_timeline DESC, name
        """,
        (soa_id,),
    )
    timelines = [
        {
            "schedule_timeline_uid": r[0],
            "name": r[1],
            "main_timeline": bool(r[2]),
        }
        for r in cur_tl.fetchall()
    ]
    conn_tl.close()

    # Group instances by timeline
    instances_by_timeline = {}
    for inst in instances:
        timeline_key = inst.get("member_of_timeline") or "unassigned"
        if timeline_key not in instances_by_timeline:
            instances_by_timeline[timeline_key] = []
        instances_by_timeline[timeline_key].append(inst)

    # Determine default timeline (main_timeline or first available)
    default_timeline = None
    for tl in timelines:
        if tl["main_timeline"]:
            default_timeline = tl["schedule_timeline_uid"]
            break
    if not default_timeline and timelines:
        default_timeline = timelines[0]["schedule_timeline_uid"]

    # If no default timeline found or no timelines exist, check if there are unassigned instances
    if not default_timeline and "unassigned" in instances_by_timeline:
        default_timeline = "unassigned"

    return templates.TemplateResponse(
        request,
        "edit.html",
        {
            "soa_id": soa_id,
            "epochs": epochs,
            "instances": instances,
            "activities": activities_page,
            "elements": elements,
            "arms": arms_enriched,
            "cell_map": cell_map,
            "concepts": concepts,
            "activity_concepts": activity_concepts,
            "concepts_empty": len(concepts) == 0,
            "concepts_diag": concepts_diag,
            "concepts_last_fetch_iso": last_fetch_iso,
            "concepts_last_fetch_relative": last_fetch_relative,
            "freezes": freeze_list,
            "freeze_count": len(freeze_list),
            "last_frozen_at": last_frozen_at,
            **study_meta,
            "protocol_terminology_C174222": protocol_terminology_C174222,
            "ddf_terminology_C188727": ddf_terminology_C188727,
            # Epoch Type options (C99079)
            "epoch_type_options": epoch_type_options,
            # Study Cells
            "study_cells": study_cells,

[Copilot]

Mismatched quote tags: The string contains </quote> which appears to be a typo - there's no opening <quote> tag. This should just be ) without the closing tag.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated 7 comments.

[Copilot]

/soa/{soa_id}/visits/reorder is now defined in this router, but src/soa_builder/web/app.py still defines an @app.post("/soa/{soa_id}/visits/reorder") handler with a different request body shape (list vs {order: [...]}). Keeping both creates duplicate routes and can lead to ambiguous behavior / 422s depending on route resolution and OpenAPI generation. Remove the app.py handler (or this one) and keep a single implementation + single request schema.

[Copilot]

templates.TemplateResponse is called without a context dict here. In this codebase the TemplateResponse helper is used as TemplateResponse(request, template_name, context); omitting the context will raise a TypeError at runtime. Pass an explicit context (at least an empty dict) so the help page renders correctly.

Suggested change

	{"request": request},
	{},

[Copilot]

This migration adds study_cell.order_index but doesn’t backfill existing rows, leaving order_index as NULL for pre-existing study cells. Since the new UI/API sorts by order_index and new inserts compute MAX(order_index), NULLs can cause unstable ordering and confusing reordering behavior for existing databases. Consider backfilling order_index for existing rows (e.g., by id order) and/or setting a non-null default.

[Copilot]

update_study_cell allows changing arm_uid / epoch_uid / element_uid without verifying the referenced arm/epoch/element rows exist in this SoA. This can create dangling references (and then list_study_cells with INNER JOINs will silently drop those rows from API responses). Add existence checks for any field being changed, similar to add_study_cell.

Suggested change

	# Validate referenced arm/epoch/element exist in this SOA if changed
	if new_arm != curr_arm:
	cur.execute(
	"SELECT 1 FROM arm WHERE soa_id=? AND arm_uid=?",
	(soa_id, new_arm),
	)
	if not cur.fetchone():
	conn.close()
	raise HTTPException(400, "Arm UID not found in this SOA")
	if new_epoch != curr_epoch:
	cur.execute(
	"SELECT 1 FROM epoch WHERE soa_id=? AND epoch_uid=?",
	(soa_id, new_epoch),
	)
	if not cur.fetchone():
	conn.close()
	raise HTTPException(400, "Epoch UID not found in this SOA")
	if new_el != curr_el:
	cur.execute(
	"SELECT 1 FROM element WHERE soa_id=? AND element_uid=?",
	(soa_id, new_el),
	)
	if not cur.fetchone():
	conn.close()
	raise HTTPException(400, "Element UID not found in this SOA")

[Copilot]

Typo in user-facing help text: "Scheuled" should be "Scheduled".

[Copilot]

Typo in user-facing help text: "Optioinally" should be "Optionally".

[Copilot]

Typo in user-facing help text: "wil" should be "will".

[Copilot]

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated 5 comments.

[Copilot]

Deprecated code is being "commented out" via a standalone triple-quoted string. This leaves a pointless string literal statement in the module body and makes it easy to accidentally re-enable/merge conflicts. Prefer removing the dead endpoint entirely, or guard it with an explicit if False: block and a clear comment.

[Copilot]

StudyCellCreate marks all fields as optional, but add_study_cell() treats arm_uid, epoch_uid, and element_uid as required and returns 400 when missing. Making these fields required in the schema (and keeping only StudyCellUpdate optional) will give API clients immediate validation errors and keep the contract consistent.

[Copilot]

_nz() is defined but never used in this router, which adds noise and makes it harder to tell what helpers are relevant. Remove it, or use it consistently when normalizing form/payload fields.

Suggested change

	# Helper: Normalization
	def _nz(s: Optional[str]) -> Optional[str]:
	s = (s or "").strip()
	return s or None

[Copilot]

_next_study_cell_uid() computes the next UID by loading all study_cell rows and all study_cell_audit rows and JSON-parsing each record on every insert. As audit history grows, creating a study cell will become increasingly expensive. Consider tracking the max numeric suffix with a single SQL query (e.g., MAX(CAST(SUBSTR(...) AS INT)) over live + audit) or persisting a per-SoA counter to avoid full-table scans.

Suggested change

	Checks both the live table and the audit trail so that UIDs from
	deleted study cells are never reused.
	"""
	max_n = 0
	# Current rows
	cur.execute("SELECT study_cell_uid FROM study_cell WHERE soa_id=?", (soa_id,))
	for (uid,) in cur.fetchall():
	if isinstance(uid, str) and uid.startswith("StudyCell_"):
	try:
	n = int(uid.split("_")[-1])
	if n > max_n:
	max_n = n
	except Exception:
	pass
	# Historically used UIDs from audit trail
	cur.execute(
	"SELECT before_json, after_json FROM study_cell_audit WHERE soa_id=?",
	(soa_id,),
	)
	for before_raw, after_raw in cur.fetchall():
	for raw in (before_raw, after_raw):
	if not raw:
	continue
	try:
	uid = json.loads(raw).get("study_cell_uid", "")
	if isinstance(uid, str) and uid.startswith("StudyCell_"):
	n = int(uid.split("_")[-1])
	if n > max_n:
	max_n = n
	except Exception:
	pass
	Uses SQL aggregation over both the live table and the audit trail so that
	UIDs from deleted study cells are never reused, without performing
	full-table scans and per-row JSON parsing in Python.
	"""
	max_n = 0
	# Max numeric suffix from current rows
	cur.execute(
	"""
	SELECT MAX(
	CAST(
	SUBSTR(study_cell_uid, LENGTH('StudyCell_') + 1) AS INTEGER
	)
	)
	FROM study_cell
	WHERE soa_id = ?
	AND study_cell_uid LIKE 'StudyCell_%'
	""",
	(soa_id,),
	)
	row = cur.fetchone()
	if row and row[0] is not None:
	try:
	max_n = int(row[0])
	except (TypeError, ValueError):
	pass
	# Max numeric suffix from historically used UIDs in the audit trail
	# Note: relies on SQLite JSON1 extension (json_extract) being available.
	cur.execute(
	"""
	SELECT MAX(
	CAST(
	SUBSTR(
	json_extract(src, '$.study_cell_uid'),
	LENGTH('StudyCell_') + 1
	) AS INTEGER
	)
	)
	FROM (
	SELECT before_json AS src
	FROM study_cell_audit
	WHERE soa_id = ?
	UNION ALL
	SELECT after_json AS src
	FROM study_cell_audit
	WHERE soa_id = ?
	)
	WHERE json_extract(src, '$.study_cell_uid') LIKE 'StudyCell_%'
	""",
	(soa_id, soa_id),
	)
	row = cur.fetchone()
	if row and row[0] is not None:
	try:
	audit_max = int(row[0])
	if audit_max > max_n:
	max_n = audit_max
	except (TypeError, ValueError):
	pass

[Copilot]

The new study-cell reorder API (POST /soa/{soa_id}/study_cells/reorder) and its UI wiring are not covered by tests. Adding tests for successful reorder, invalid IDs, and empty order (matching the behavior you expect) would help prevent regressions, especially since ordering is now persisted via order_index.