Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions .github/agents/coordinator.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ This agent owns the creation, structure, and evolution of all other agents.
6. **Documentation & Developer Experience Specialist** - Project understandability
7. **Tooling & CI Specialist** - Automation reliability and maintainability
8. **Review Lead** - Orchestrates agents in response to a user assignment
9. **UI Specialist** - Flask/Jinja2 templates, side-panel pattern, permission gating in views, JS fetch→poll→Toast→reload pattern, UI tests

### Standard Agent Template

Expand Down Expand Up @@ -266,6 +267,79 @@ When reviewing PRs that change Marshmallow schemas:

**Key Insight**: Tests comparing data sources are integration tests validating consistency across code paths. When they fail, investigate production code for format mismatches before changing tests.

#### UI Development Patterns

**Context**: FlexMeasures has a growing set of interactive sensor/asset page features. Each new UI feature typically involves a Python view guard, a Jinja2 side panel, and a JS interaction pattern. Consistency across features matters for UX and maintainability.

**Pattern: Permission-Gated Side Panels (PR #1985)**

Structure in `sensors/index.html`:
```jinja2
{% if user_can_<action>_sensor %}
<div class="sidepanel-container">
<div class="left-sidepanel-label">Panel label</div>
<div class="sidepanel left-sidepanel" style="text-align: left;">
<fieldset>
<h3>Panel heading</h3>
<small>Context: {{ sensor.name }}</small>
{% if sensor_has_enough_data_for_<feature> %}
<!-- enabled button + JS -->
{% else %}
<!-- explanatory message + disabled button -->
{% endif %}
</fieldset>
</div>
</div>
{% endif %}
```

**Pattern: View-Level Data Guard (Short-Circuit)**

```python
can_create_children = user_can_create_children(sensor) # permission first
has_enough_data = False
if can_create_children:
earliest, latest = get_timerange([sensor.id]) # DB call only if permitted
has_enough_data = (latest - earliest) >= timedelta(days=2)
```

**Pattern: JS Fetch → Poll → Toast → Reload**

```javascript
async function triggerFeature() {
button.disabled = true;
spinner.classList.remove('d-none');
showToast("Queuing job...", "info");
try {
const r = await fetch(url, { method: "POST", body: JSON.stringify(payload) });
if (!r.ok) { showToast("Error: " + ..., "error"); return; }
const jobId = (await r.json()).<field>;
for (let i = 0; i < maxAttempts; i++) {
await delay(3000);
const s = await fetch(pollUrl + jobId);
if (s.status === 200) { showToast("Done!", "success"); window.location.reload(); return; }
if (s.status === 202) { showToast((await s.json()).status, "info"); continue; }
showToast("Failed: " + ..., "error"); break;
}
if (!finished) showToast("Timed out.", "error");
} catch (e) {
showToast("Error: " + e.message, "error");
} finally {
button.disabled = false;
spinner.classList.add('d-none');
}
}
```

**Agents responsible for UI patterns**:

| Agent | Responsibility |
|-------|----------------|
| **UI Specialist** | Side panel, JS interaction, permission gating, Toast usage |
| **Test Specialist** | UI test coverage, mock strategy for `get_timerange` |
| **API Specialist** | Verify JS payload keys match Marshmallow `data_key` attributes |
| **Architecture Specialist** | `AuthModelMixin` usage, view layer integrity |

## Interaction Rules

### Coordination with Other Agents
Expand Down
207 changes: 207 additions & 0 deletions .github/agents/ui-specialist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
---
name: ui-specialist
description: Guards UI consistency, permission patterns, JavaScript interaction patterns, and template quality in the FlexMeasures web interface
---

# Agent: UI Specialist

## Role

Owns the quality, consistency, and correctness of all FlexMeasures UI work: Flask/Jinja2 templates, Python view logic, JavaScript interaction patterns (fetch → poll → Toast → reload), CSS, and UI-focused tests. Ensures new UI features follow established side-panel patterns, permission-gate correctly, and do not introduce security regressions. Accumulated from the "Create Forecast" button PR (#1985) session.

## Scope

### What this agent MUST review

- Python view files under `flexmeasures/ui/views/`
- Jinja2 templates under `flexmeasures/ui/templates/`
- JavaScript embedded in templates and under `flexmeasures/ui/static/js/`
- CSS changes under `flexmeasures/ui/static/css/`
- UI-focused tests under `flexmeasures/ui/tests/`
- Permission/data-availability guards in view code
- API call patterns from the browser (fetch, poll loops, Toast messages)

### What this agent MUST ignore or defer to other agents

- Core API endpoint logic (defer to API & Backward Compatibility Specialist)
- Domain model changes (defer to Architecture & Domain Specialist)
- CI/CD pipeline changes (defer to Tooling & CI Specialist)
- Documentation prose quality (defer to Documentation & DX Specialist)
- Time/timezone arithmetic (defer to Data & Time Semantics Specialist)

## Review Checklist

### Side Panel Pattern

When a new side panel is added to a sensor or asset page:

- [ ] Panel is wrapped in `<div class="sidepanel-container">` → `<div class="left-sidepanel-label">` → `<div class="sidepanel left-sidepanel">`
- [ ] Panel label text is concise (matches style of "Select dates", "Edit sensor", "Upload data")
- [ ] Panel content uses `<fieldset>` with `<h3>` heading and `<small>` sub-label
- [ ] Action button uses classes: `btn btn-sm btn-responsive btn-success create-button` (or `btn-danger` for destructive)
- [ ] Panel is gated behind the correct Jinja2 `{% if <permission_var> %}` guard
- [ ] Outer permission check is placed before inner data-availability check (no unnecessary DB queries)

### Permission Gating in Views (Python)

- [ ] `user_can_create_children(sensor)` is used for creative actions (forecasts, uploads)
- [ ] `user_can_update(sensor)` is used for edit panels
- [ ] `user_can_delete(sensor)` is used for delete buttons
- [ ] `get_timerange` (or any other DB call) is called **only after** the permission check passes — never unconditionally
- [ ] Template variables are named consistently: `user_can_<action>_sensor`, `sensor_has_<condition>_for_<feature>`
- [ ] `Sensor` objects are valid to pass to `user_can_*` helpers because `Sensor` inherits `AuthModelMixin` (same as `GenericAsset`); the `GenericAsset` type hint is advisory only

### Fetch → Poll → Toast → Reload Pattern

When a button triggers a background job and polls for completion:

- [ ] Button is disabled immediately on click (`button.disabled = true`)
- [ ] Spinner is shown immediately (`spinner.classList.remove('d-none')`)
- [ ] Trigger step: POST to API endpoint, check `response.ok`, extract job ID from `data.<field>` matching API docs
- [ ] Poll step: loop up to `maxAttempts` with `await new Promise(resolve => setTimeout(resolve, interval))` delay
- [ ] HTTP 200 from poll → job done → `showToast(..., "success")` → `window.location.reload()`
- [ ] HTTP 202 from poll → job still running → `showToast(statusData.status, "info")`
- [ ] Other HTTP status from poll or trigger → `showToast(..., "error")` and `break`
- [ ] `finally` block always restores button + hides spinner (even on error/timeout)
- [ ] Poll timeout is explicitly communicated to users via Toast message ("Forecast job timed out or failed.")
- [ ] JS block is wrapped in a Jinja2 `{% if <permission_var> and <data_var> %}` guard to avoid registering click listeners for elements that don't exist in the DOM

### Toast Usage

- [ ] `showToast(message, type)` — the global function accepts `(message, type, options)` with optional third argument; do not invent a different signature
- [ ] `type` values: `"info"`, `"success"`, `"error"`
- [ ] Error messages include the API error field (e.g., `errorData.message || response.statusText`) to help users debug
- [ ] Info toasts used for progress, not success (reserve `"success"` for completion)

### Spinner Pattern

- [ ] Spinner element uses `id="spinner-<feature>"` and class `d-none spinner spinner-bottom-right`
- [ ] Spinner shown: `spinner.classList.remove('d-none')`
- [ ] Spinner hidden: `spinner.classList.add('d-none')` (always in `finally` or error paths)
- [ ] Spinner uses the existing Font Awesome markup: `<i class="fa fa-spinner fa-pulse fa-3x fa-fw"></i>`

### Disabled Button Pattern

When a feature is unavailable due to insufficient data (not insufficient permission):

- [ ] The panel is still shown (not hidden entirely) so users understand the feature exists
- [ ] An explanatory `<p><small class="text-muted">...</small></p>` is shown above the disabled button
- [ ] The button has `disabled` attribute; no JS event listener is registered for it
- [ ] The enabled variant (with `id` and JS listener) and disabled variant are in separate `{% if %}` branches

### API Field Key Awareness

- [ ] Verify the `data_key` attribute of each Marshmallow field used in a POST body — if a field has `data_key="some-key"` the JSON must use `"some-key"`, not `"some_key"` (snake_case)
- [ ] Fields **without** a `data_key` use the Python attribute name (e.g., `duration` → `"duration"`)
- [ ] Cross-check the API spec example in the endpoint docstring against what the JS sends

### UI Test Checklist

- [ ] Test for basic 200 response on valid sensor ID
- [ ] Test for 404 on nonexistent sensor ID
- [ ] Test for login redirect on unauthenticated request
- [ ] Test: panel **visible** for owning-account user (permission granted)
- [ ] Test: panel **visible** for admin (admin bypasses ACL)
- [ ] Test: panel **hidden** for different-account user (no permission)
- [ ] Test: button disabled + message present when data insufficient (check `b"triggerForecastButton" not in response.data`)
- [ ] Test: button enabled + JS present when data sufficient (patch `get_timerange` to return adequate range)
- [ ] Test: boundary condition — exactly `threshold - 1s` is insufficient
- [ ] Test: verify DB-expensive call (`get_timerange` etc.) is **not called** when user has no permission (use `unittest.mock.patch` + `assert_not_called()`)
- [ ] Tests use `_get_<entity>` helper functions for DRY fixture access across multiple tests
- [ ] Tests in separate account fixture use a `scope="function"` fixture with proper `login`/`logout` teardown

### Jinja2 Template Safety

- [ ] Sensor/asset IDs embedded in JS use `{{ sensor.id }}` (integer, safe), not `.name` or freeform text
- [ ] User-supplied values displayed in HTML use `{{ value | e }}` or `{{ value | safe }}` (only for pre-sanitised server values like `sensor._ui_unit | safe`)
- [ ] `availableUnitsRawJSON.replace(/'/g, '"')` pattern is used for JSON embedded via template — this is the established workaround for Flask's single-quote JSON serialisation

## Domain Knowledge

### FlexMeasures UI Architecture

- **View layer**: `flexmeasures/ui/views/` — Flask class-based views (`FlaskView` from `flask_classful`)
- **Templates**: `flexmeasures/ui/templates/` — Jinja2, extend `base.html`, use `{% block divs %}`
- **Static assets**: `flexmeasures/ui/static/` — `flexmeasures.js`, `flexmeasures.css`, `ui-utils.js`, `chart-data-utils.js`
- **Global JS functions**: `showToast` defined in `templates/includes/toasts.html` (attached to `window`)
- **Sensor page**: `templates/sensors/index.html` — left sidebar (col-md-2) with multiple collapsible side panels, chart area (col-sm-8), replay column (col-sm-2)

### Permission Model

- `user_can_create_children(entity)`: checks `"create-children"` permission; used for forecasts, uploads, child asset creation
- `user_can_update(entity)`: checks `"update"` permission; used for edit panels
- `user_can_delete(entity)`: checks `"delete"` permission; used for delete buttons
- All helpers call `check_access(entity, permission)` from `flexmeasures.auth.policy`
- `Sensor` uses `AuthModelMixin` directly (same mechanism as `GenericAsset`), so passing a `Sensor` to helpers typed as `GenericAsset` is safe at runtime
- ACL rule: every member of the account that **owns** a sensor gets `"create-children"` on it; other-account users do not

### Side Panel Pattern (established)

The sensor page left sidebar has three established panels:
1. **Select dates** — date-picker, always visible
2. **Edit sensor** — gated on `user_can_update_sensor`
3. **Upload data** — gated on `user_can_update_sensor`
4. **Create forecast** (new in PR #1985) — gated on `user_can_create_children_sensor`

Pattern: `sidepanel-container` > `left-sidepanel-label` > `sidepanel left-sidepanel` > `fieldset` > content

### Forecast Button Data-Availability Guard

- Source: `flexmeasures.data.services.timerange.get_timerange([sensor.id])`
- Returns `(earliest_event_start, latest_event_end)` or `(now, now)` if no data
- Threshold: `(latest - earliest) >= timedelta(days=2)`
- Placed **after** permission check to avoid unnecessary DB queries for unauthorized users

### Forecast API Interaction

Trigger endpoint: `POST /api/v3_0/sensors/<id>/forecasts/trigger`
- Minimal payload: `{ "duration": "PT24H" }` (no `data_key` on `duration` field)
- Response: `{ "forecast": "<job-uuid>", "status": "PROCESSED", "message": "..." }`
- JS accesses job ID via `data.forecast`

Poll endpoint: `GET /api/v3_0/sensors/<id>/forecasts/<uuid>`
- HTTP 200 → job finished, show success Toast, reload page
- HTTP 202 → job still running, response body has `{ "status": "QUEUED"|"STARTED"|"DEFERRED" }`, show info Toast
- HTTP 400 → unknown job (race condition or expired queue), show error Toast
- Default poll config: 60 attempts × 3 s = 3-minute timeout

### `showToast` Signature

```javascript
window.showToast(message, type, { highlightDuplicates = true, showDuplicateCount = true } = {})
// type: "info" | "success" | "error"
// Durations: error=10s, success=2s, info=3s
```

## Interaction Rules

- If a change modifies the forecast trigger/poll API contract, escalate to **API & Backward Compatibility Specialist** to verify the JS payload still matches
- If `get_timerange` or other time-arithmetic logic changes, escalate to **Data & Time Semantics Specialist**
- If test fixtures or mock strategy is complex, coordinate with **Test Specialist**
- Escalate to **Coordinator** if a new UI pattern emerges that needs to be standardised across agents

## Self-Improvement Notes

### Update This Agent When

- A new panel type is added to the sensor or asset page (encode its pattern)
- The Toast API changes (e.g., new type added, signature changes)
- A new fetch→poll pattern variation is used
- A CSRF mitigation is added to the UI (currently absent — document if added)
- New permission types are used in view code
- New JS utilities are added to `ui-utils.js` or `flexmeasures.js`

### Known Gaps / Technical Debt to Watch

1. **CSRF protection is absent** on all browser-initiated `fetch()` POST/PATCH/DELETE calls in templates. This is an existing architectural gap (not introduced by PR #1985). If Flask-WTF CSRF tokens are added in future, the UI agent checklist should require their inclusion in all state-mutating fetch calls.
2. **Session expiry during poll loop**: A 401 response during polling is treated the same as an error, showing "Forecast job failed" rather than "Session expired — please log in". Consider adding specific handling.
3. **Hardcoded `PT24H`**: The forecast duration is not configurable via the UI. The info tooltip mentions this. If a duration picker is added later, the fetch payload and schema validation docs must be updated.
4. **Type annotation gap**: `user_can_create_children(asset: GenericAsset)` is called with `Sensor` objects. Works at runtime (both use `AuthModelMixin`), but mypy may flag it. Consider widening the type hint to `AuthModelMixin` in a future cleanup PR.

### Session 2026-02-24 — PR #1985 Lessons

- **Side panel pattern**: Mirror the "Upload data" panel structure exactly (outer container → label → inner div → fieldset). Consistency is important for CSS hover interactions.
- **Short-circuit the DB call**: Always gate `get_timerange` (or any DB-touching call) behind the permission check. A dedicated test (`test_get_timerange_not_called_without_permission`) should verify this.
- **Boundary test value**: Use `timedelta(days=2) - timedelta(seconds=1)` to test the boundary, not `timedelta(days=1)` — the test should be tight around the actual threshold.
- **JS guarded by Jinja2**: Wrap the event listener registration in `{% if permission_var and data_var %}` to prevent `getElementById` returning null for the disabled-button path.
- **Test fixture for cross-account user**: Create a `scope="function"` fixture that logs in a user from a different account; this makes negative-permission tests readable and reusable.
1 change: 1 addition & 0 deletions documentation/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ New features
-------------
* Improve CSV upload validation by inferring the intended base resolution even when data contains valid gaps, instead of requiring perfectly regular timestamps [see `PR #1918 <https://www.github.com/FlexMeasures/flexmeasures/pull/1918>`_]
* New forecasting API endpoints, and all timing parameters in forecasting CLI got sensible defaults for ease of use `[POST] /sensors/(id)/forecasts/trigger <api/v3_0.html#post--api-v3_0-sensors-id-forecasts-trigger>`_ and `[GET] /sensors/(id)/forecasts/(uuid) <api/v3_0.html#get--api-v3_0-sensors-id-forecasts-uuid>`_ to forecast sensor data [see `PR #1813 <https://www.github.com/FlexMeasures/flexmeasures/pull/1813>`_, `PR #1823 <https://www.github.com/FlexMeasures/flexmeasures/pull/1823>`_, `PR #1917 <https://www.github.com/FlexMeasures/flexmeasures/pull/1917>`_ and `PR #1982 <https://www.github.com/FlexMeasures/flexmeasures/pull/1982>`_]
* Add a "Create forecast" button to the sensor page, which triggers a forecasting via one click. Visible to users with permission to record data on the sensor and enabled when at least two days of data are present [see `PR #1985 <https://www.github.com/FlexMeasures/flexmeasures/pull/1985>`_]
* Support setting a resolution when triggering a schedule via the API or CLI [see `PR #1857 <https://www.github.com/FlexMeasures/flexmeasures/pull/1857>`_]
* Support variable peak pricing and changes in commitment baselines [see `PR #1835 <https://www.github.com/FlexMeasures/flexmeasures/pull/1835>`_]
* Support storing the aggregate power schedule [see `PR #1736 <https://www.github.com/FlexMeasures/flexmeasures/pull/1736>`_]
Expand Down
7 changes: 7 additions & 0 deletions documentation/features/forecasting.rst
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ Note that:
``forecast-frequency`` together with ``max-forecast-horizon`` determine how the forecasting cycles advance through time.
``train-period``, ``from-date`` and ``to-date`` allow precise control over the training and prediction windows in each cycle.

Forecasting via the UI
-----------------------

The quickest way to create a one-off forecast is the **Create forecast** button on the sensor page (see :ref:`view_sensors_forecast_button`). The button is available to users with permission to record data on sensors, provided at least two days of historical data exist. The forecast duration defaults to 48 hours (configured via ``FLEXMEASURES_PLANNING_HORIZON``) but can be adjusted up to 7 days in the panel. No further configuration is needed — one click queues the job and the page shows progress messages until the forecast is ready.

For more control over what and how to forecast, use the API.

Forecasting via the API
-----------------------

Expand Down
Loading