UI features by pendingintent · Pull Request #87 · pendingintent/soa-workbench

pendingintent · 2026-02-06T17:46:29Z

ISO validation of Timing window
all-or-nothing validation of Timing window values
ISO validation of Timing Value

…ng test for window values

Copilot

Pull request overview

Adds ISO 8601 duration validation for Timing value and window fields, and updates the UI/tests to reflect the new duration format and validation behavior.

Changes:

Added Pydantic validators for ISO 8601 duration parsing on value, window_lower, and window_upper.
Updated the timings UI to guide users with ISO 8601 patterns and added client-side “all-or-none” window validation.
Expanded router tests to cover acceptance/rejection of duration formats for API/UI endpoints.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
tests/test_timings.py	Updates expectations to use ISO 8601 durations (e.g., `P5D`, `-P1D`).
tests/test_routers_timings.py	Adds API/UI tests for rejecting invalid duration strings and accepting valid ones.
src/soa_builder/web/templates/timings.html	Adds input patterns/titles for ISO durations, rearranges fields, and adds client-side window group validation.
src/soa_builder/web/templates/instances.html	Renames “Default Condition” labels to “Next Instance (default condition)”.
src/soa_builder/web/schemas.py	Adds ISO 8601 duration regex + field validators to Timing create/update schemas.
src/soa_builder/web/routers/timings.py	Converts UI create/update validation errors into HTTP 400 with summarized messages.

Copilot · 2026-02-06T17:48:36Z

src/soa_builder/web/schemas.py

 class TimingCreate(BaseModel):
    name: str
    label: Optional[str] = None
    description: Optional[str] = None
    type: Optional[str] = None
    value: Optional[str] = None
    value_label: Optional[str] = None
    relative_to_from: Optional[str] = None
    relative_from_schedule_instance: Optional[str] = None
    relative_to_schedule_instance: Optional[str] = None
    window_label: Optional[str] = None
    window_upper: Optional[str] = None
    window_lower: Optional[str] = None
    member_of_timeline: Optional[str] = None


The PR description mentions “all-or-nothing validation of Timing window values”, but backend validation currently only checks ISO formatting per-field. As a result, API callers can submit any subset of window_lower/window_upper/window_label and it will pass schema validation (the UI JS can be bypassed). Add a backend validator (e.g., a model-level validator) to enforce that either all three window fields are provided (non-empty after stripping) or none are provided, for both TimingCreate and TimingUpdate.

Copilot · 2026-02-06T17:48:37Z

src/soa_builder/web/schemas.py

+    @field_validator("value", "window_lower", "window_upper", mode="before")
+    @classmethod
+    def check_iso8601_duration(cls, v: Optional[str]) -> Optional[str]:
+        return _validate_iso8601_duration(v)


The PR description mentions “all-or-nothing validation of Timing window values”, but backend validation currently only checks ISO formatting per-field. As a result, API callers can submit any subset of window_lower/window_upper/window_label and it will pass schema validation (the UI JS can be bypassed). Add a backend validator (e.g., a model-level validator) to enforce that either all three window fields are provided (non-empty after stripping) or none are provided, for both TimingCreate and TimingUpdate.

Copilot · 2026-02-06T17:48:37Z

src/soa_builder/web/schemas.py

+# ISO 8601 duration pattern supporting both standard (-P2D) and USDM (P-2D) conventions
+_ISO8601_DURATION_RE = re.compile(
+    r"^-?P-?(?:(\d+)Y)?(?:(\d+)M)?(?:(\d+)W)?(?:(\d+)D)?(?:T(?:(\d+)H)?(?:(\d+)M)?(?:(\d+)S)?)?$"
+)


The pattern ^-?P-? allows a double-negative prefix like -P-2D, which is likely unintended (it’s neither standard ISO -P2D nor the USDM-style P-2D). Consider changing the regex to allow exactly one sign position (either before P or immediately after P), but not both.

Copilot · 2026-02-06T17:48:37Z

src/soa_builder/web/templates/timings.html

@@ -178,4 +178,32 @@ <h2>Timings for SoA {{ soa_id }}</h2>
  <tr><td colspan="12" style="padding:6px;color:#777;">No timings yet.</td></tr>


The table now has more than 12 columns (additional/reordered headers were added), but the “No timings yet.” row still uses colspan="12", which will render misaligned. Update the colspan to match the current number of <th> columns.

Suggested change

<tr><td colspan="12" style="padding:6px;color:#777;">No timings yet.</td></tr>

<tr><td colspan="16" style="padding:6px;color:#777;">No timings yet.</td></tr>

Copilot · 2026-02-06T17:48:37Z

src/soa_builder/web/templates/timings.html

    <div style="display:flex;flex-direction:column;gap:2px;">
      <label><strong>Value</strong></label>
-      <input name="value" placeholder="Value (optional)" />
+      <input name="value" placeholder="e.g. P7D (optional)" pattern="-?P-?(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?" title="ISO 8601 duration, e.g. P1D, P2W, PT8H, -P2D" />


The ISO 8601 duration pattern="..." is duplicated in multiple inputs (create + edit rows, value + window fields). This increases the chance of UI/backend drift (and currently mirrors the backend regex that also accepts -P-2D). Consider centralizing the pattern (e.g., Jinja variable/macro include) so changes happen in one place.

Copilot · 2026-02-06T17:48:38Z

src/soa_builder/web/templates/timings.html

          {% endfor %}
        </select>
      </td>
+      <td style="padding: 4px;"><input name="value" value="{{ t.value or '' }}" placeholder="e.g. P7D" pattern="-?P-?(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?" title="ISO 8601 duration, e.g. P1D, P2W, PT8H, -P2D" /></td>


The ISO 8601 duration pattern="..." is duplicated in multiple inputs (create + edit rows, value + window fields). This increases the chance of UI/backend drift (and currently mirrors the backend regex that also accepts -P-2D). Consider centralizing the pattern (e.g., Jinja variable/macro include) so changes happen in one place.

Copilot · 2026-02-06T17:48:38Z

src/soa_builder/web/templates/timings.html

+      <td style="padding: 4px;"><input name="window_lower" value="{{ t.window_lower or '' }}" placeholder="e.g. -P2D" pattern="-?P-?(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?" title="ISO 8601 duration, e.g. P1D, -P2D, PT8H" /></td>
+      <td style="padding: 4px;"><input name="window_upper" value="{{ t.window_upper or '' }}" placeholder="e.g. P3D" pattern="-?P-?(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?" title="ISO 8601 duration, e.g. P1D, P3D, PT8H" /></td>


The ISO 8601 duration pattern="..." is duplicated in multiple inputs (create + edit rows, value + window fields). This increases the chance of UI/backend drift (and currently mirrors the backend regex that also accepts -P-2D). Consider centralizing the pattern (e.g., Jinja variable/macro include) so changes happen in one place.

Copilot · 2026-02-06T17:48:39Z

src/soa_builder/web/routers/timings.py

+            member_of_timeline=member_of_timeline,
+        )
+    except ValidationError as exc:
+        msgs = "; ".join(e["msg"] for e in exc.errors())


The UI error message concatenates only e["msg"] and drops which field failed (loc). If multiple fields fail validation, users won’t know which input to fix. Consider including the field path (e.g., derived from e["loc"]) in the message, or returning structured error details that the template can render next to the offending field(s).

Suggested change

msgs = "; ".join(e["msg"] for e in exc.errors())

# Include field path (loc) in error messages so users know which input failed

error_messages = []

for e in exc.errors():

loc = e.get("loc")

if loc:

# Pydantic typically includes "body" as the first element; skip it for UI friendliness

if isinstance(loc, (list, tuple)):

loc_parts = [str(part) for part in loc if part != "body"]

field_path = ".".join(loc_parts) if loc_parts else ""

else:

field_path = str(loc)

if field_path:

error_messages.append(f"{field_path}: {e.get('msg')}")

else:

error_messages.append(e.get("msg"))

else:

error_messages.append(e.get("msg"))

msgs = "; ".join(m for m in error_messages if m)

Copilot · 2026-02-06T17:48:39Z

tests/test_routers_timings.py

+def test_window_all_or_none_accepts_all_three():
+    """Test that providing all three window fields is accepted."""


The “all-or-none” window behavior is only tested for the two accepted cases (all three present / none present). Add negative tests asserting rejection when only 1 or 2 of the 3 window fields are provided (for both JSON API and UI form endpoints), especially if backend validation is added to enforce this consistently.

Copilot · 2026-02-06T17:48:39Z

tests/test_routers_timings.py

+def test_window_all_or_none_accepts_none():
+    """Test that providing no window fields is accepted."""


The “all-or-none” window behavior is only tested for the two accepted cases (all three present / none present). Add negative tests asserting rejection when only 1 or 2 of the 3 window fields are provided (for both JSON API and UI form endpoints), especially if backend validation is added to enforce this consistently.

Copilot

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Copilot · 2026-02-08T21:54:13Z

src/soa_builder/web/templates/timings.html

    <div style="display:flex;flex-direction:column;gap:2px;">
      <label><strong>Value</strong></label>
-      <input name="value" placeholder="Value (optional)" />
+      <input name="value" placeholder="e.g. P7D (optional)" pattern="-?P-?(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?" title="ISO 8601 duration, e.g. P1D, P2W, PT8H, -P2D" />


The HTML pattern allows a bare P (all components are optional), but the backend explicitly rejects it (not any(m.group(...))). This will lead to confusing UX where the browser accepts input that the server rejects. Adjust the pattern to require at least one duration component (and ideally keep it aligned with the backend regex), or render the backend pattern from a shared constant to avoid drift.

Suggested change

<input name="value" placeholder="e.g. P7D (optional)" pattern="-?P-?(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?" title="ISO 8601 duration, e.g. P1D, P2W, PT8H, -P2D" />

<input name="value" placeholder="e.g. P7D (optional)" pattern="-?P-?(?=(?:\d+Y|\d+M|\d+W|\d+D|T\d+H|T\d+M|T\d+S))(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?" title="ISO 8601 duration, e.g. P1D, P2W, PT8H, -P2D" />

Copilot · 2026-02-08T21:54:14Z

src/soa_builder/web/schemas.py

+
+# ISO 8601 duration pattern supporting both standard (-P2D) and USDM (P-2D) conventions
+_ISO8601_DURATION_RE = re.compile(
+    r"^-?P-?(?:(\d+)Y)?(?:(\d+)M)?(?:(\d+)W)?(?:(\d+)D)?(?:T(?:(\d+)H)?(?:(\d+)M)?(?:(\d+)S)?)?$"


This regex will accept strings like P1DT (a trailing T with no time components) because the time components after T are all optional. If you intend strict ISO-8601 duration validation, require at least one time component when T is present (e.g., ensure T is followed by \d+H or \d+M or \d+S). If you don’t intend strict ISO, consider updating the comment/error text to reflect the accepted superset.

Copilot · 2026-02-08T21:54:14Z

tests/test_routers_timings.py

+def test_window_accepts_valid_iso8601_durations():
+    """Test that various valid ISO 8601 durations are accepted."""


This test includes P-2D, which is called out elsewhere as a USDM convention (and is not standard ISO-8601). The docstring (and possibly the test name) should be updated to reflect that the accepted formats include both standard ISO (e.g., -P2D) and USDM-style (e.g., P-2D) durations.

Suggested change

def test_window_accepts_valid_iso8601_durations():

"""Test that various valid ISO 8601 durations are accepted."""

def test_window_accepts_iso8601_and_usdm_durations():

"""Test that various valid ISO 8601 and USDM-style durations are accepted."""

…rkbench into ui-features Update

pendingintent added 4 commits February 6, 2026 11:23

Ordered timing fields in logical way; added hover help text

5693cd1

Changed Default Condition header

a84ff8d

Rearranged headers to make timings more intuitive

18e6102

Added ISO validation rules for windows, value as well as all-or-nothi…

ef1a7ca

…ng test for window values

pendingintent added this to the v1.0 milestone Feb 6, 2026

pendingintent self-assigned this Feb 6, 2026

Copilot AI review requested due to automatic review settings February 6, 2026 17:46

pendingintent added the enhancement New feature or request label Feb 6, 2026

Copilot AI reviewed Feb 6, 2026

View reviewed changes

pendingintent added 2 commits February 8, 2026 15:51

Removed numpy dependency

ee05f7e

Merge branch 'master' into ui-features

564fab7

Copilot AI review requested due to automatic review settings February 8, 2026 21:52

Copilot AI reviewed Feb 8, 2026

View reviewed changes

pendingintent added 2 commits February 8, 2026 15:58

Ensure window values are all-or-nothing

f52d47c

Merge branch 'ui-features' of https://github.com/pendingintent/soa-wo…

1511c1e

…rkbench into ui-features Update

pendingintent closed this Feb 8, 2026

		@@ -178,4 +178,32 @@ <h2>Timings for SoA {{ soa_id }}</h2>
		<tr><td colspan="12" style="padding:6px;color:#777;">No timings yet.</td></tr>

	<tr><td colspan="12" style="padding:6px;color:#777;">No timings yet.</td></tr>
	<tr><td colspan="16" style="padding:6px;color:#777;">No timings yet.</td></tr>

		<td style="padding: 4px;"><input name="window_lower" value="{{ t.window_lower or '' }}" placeholder="e.g. -P2D" pattern="-?P-?(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?" title="ISO 8601 duration, e.g. P1D, -P2D, PT8H" /></td>
		<td style="padding: 4px;"><input name="window_upper" value="{{ t.window_upper or '' }}" placeholder="e.g. P3D" pattern="-?P-?(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?" title="ISO 8601 duration, e.g. P1D, P3D, PT8H" /></td>

-        msgs = "; ".join(e["msg"] for e in exc.errors())
+        # Include field path (loc) in error messages so users know which input failed
+        error_messages = []
+        for e in exc.errors():
+            loc = e.get("loc")
+            if loc:
+                # Pydantic typically includes "body" as the first element; skip it for UI friendliness
+                if isinstance(loc, (list, tuple)):
+                    loc_parts = [str(part) for part in loc if part != "body"]
+                    field_path = ".".join(loc_parts) if loc_parts else ""
+                else:
+                    field_path = str(loc)
+                if field_path:
+                    error_messages.append(f"{field_path}: {e.get('msg')}")
+                else:
+                    error_messages.append(e.get("msg"))
+            else:
+                error_messages.append(e.get("msg"))
+        msgs = "; ".join(m for m in error_messages if m)

		def test_window_all_or_none_accepts_all_three():
		"""Test that providing all three window fields is accepted."""

		def test_window_all_or_none_accepts_none():
		"""Test that providing no window fields is accepted."""

	<input name="value" placeholder="e.g. P7D (optional)" pattern="-?P-?(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?" title="ISO 8601 duration, e.g. P1D, P2W, PT8H, -P2D" />
	<input name="value" placeholder="e.g. P7D (optional)" pattern="-?P-?(?=(?:\d+Y\|\d+M\|\d+W\|\d+D\|T\d+H\|T\d+M\|T\d+S))(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?" title="ISO 8601 duration, e.g. P1D, P2W, PT8H, -P2D" />

		def test_window_accepts_valid_iso8601_durations():
		"""Test that various valid ISO 8601 durations are accepted."""

Conversation

pendingintent commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

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 Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants