Fix what the asset status page reports about its sensors - #2474
Ahmad-Wahid wants to merge 12 commits into
Conversation
Three things went wrong for a sensor that another asset refers to, such as the day-ahead price sensor in the toy tutorial: - The most-recent-only fast track of a belief search picks the single most recent event in the database, and only afterwards drops beliefs that were formed after the moment we report the status. For a sensor with a knowledge horizon, that can drop the only row we fetched, which made a sensor with plenty of data report "no data recorded". Search again without the fast track when that happens, and pick the most recent event ourselves. - Data whose knowledge time still lies ahead was held to the minimum lead time that only forecasters and schedulers should be held to, so prices recorded for tomorrow showed a red light. Such data is fresh, not stale. - The reported relation was computed against the asset that the sensor belongs to, rather than against the asset whose status page is shown, so every sensor claimed to belong to the asset shown. The status endpoint now accepts the asset to report on, and the status page passes it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The status page only looked for the default source types, so a sensor recorded by any other type reported "no data recorded", however much data it held. The toy tutorial hits this: `flexmeasures add beliefs` records under the type "CLI script", so the day-ahead price sensor showed a red light with no data. Look for the source types that actually recorded on the sensor, which the sensor_data_source summary table answers as a plain lookup, on top of the default types. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Documentation build overview
52 files changed ·
|
There was a problem hiding this comment.
🟡 Changes recommended
The updated test module mixes db and fresh_db fixtures, which can deadlock/hang CI in this repository.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes multiple inaccuracies on the asset status page about sensor data freshness, source types, and sensor–asset relations, and extends the sensor status API so the UI can request relation info in the context of the asset page being viewed.
Changes:
- UI: Pass
asset_idtoGET /api/v3_0/sensors/<id>/statusso sensor–asset relation text is computed against the asset whose status page is shown. - Backend: Improve staleness/source-type detection (including unknown source types) and correct “ahead-of-knowledge-time” handling so future data isn’t incorrectly flagged stale.
- Docs/tests: Document the API change and add regression tests for the reported misbehaviors.
File summaries
| File | Description |
|---|---|
| flexmeasures/ui/templates/sensors/status.html | Adds asset_id query parameter when fetching sensor status; tweaks tooltip text. |
| flexmeasures/ui/static/openapi-specs.json | Documents optional asset_id query parameter for sensor status endpoint. |
| flexmeasures/data/services/sensors.py | Extends source-type scanning, fixes most-recent lookup edge case, refines staleness logic, and computes sensor relation relative to a provided asset context. |
| flexmeasures/api/v3_0/sensors.py | Adds optional asset_id query param and access check; passes asset context into status serialization. |
| flexmeasures/api/common/schemas/tests/test_sensor_data_schema.py | Adds regression tests covering new status behaviors and asset-context relation reporting. |
| documentation/changelog.rst | Adds a user-facing changelog entry describing the status-page fixes. |
| documentation/api/change_log.rst | Adds an API changelog entry documenting the new asset_id query parameter. |
Review details
Suppressed comments (3)
flexmeasures/api/common/schemas/tests/test_sensor_data_schema.py:557
- Inline comments should end lines only after punctuation. This comment line currently ends without punctuation.
# An hour after recording those prices, but still an hour before their knowledge time
flexmeasures/api/common/schemas/tests/test_sensor_data_schema.py:587
- Inline comments should end lines only after punctuation. This comment line currently ends without punctuation.
# On the status page of the battery, the price sensor is only related via the flex-context
flexmeasures/api/common/schemas/tests/test_sensor_data_schema.py:599
- Inline comments should end lines only after punctuation. This comment line currently ends without punctuation.
# Without an asset context, the sensor is reported relative to its own asset
- Files reviewed: 7/7 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ahmad Wahid <59763365+Ahmad-Wahid@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ahmad Wahid <59763365+Ahmad-Wahid@users.noreply.github.com>
A module that mixes the module-scoped db fixture with the function-scoped fresh_db one can hang, because fresh_db drops all tables while the module-scoped connection is still open. Move the three new tests to a sibling module, following the *_fresh_db.py convention the repo already uses. One existing case asserted the behaviour this branch fixes: at a moment when only the sensor's most recent data is not knowable yet, it expected "no data recorded", where the sensor does hold older data we could know about by then. It now expects that data, 12 hours old. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> # Conflicts: # flexmeasures/api/common/schemas/tests/test_sensor_data_schema.py
|
For this PR, I have one more wish: I noticed that the status response contains a "reason" field which explains why a sensor is labelled as stale. |
Signed-off-by: Ahmad-Wahid <ahmedwahid16101@gmail.com>
…1718-status-page-sensor-relation # Conflicts: # documentation/changelog.rst # flexmeasures/data/services/sensors.py
|
Can you fix the merge conflicts on this one @Ahmad-Wahid ? |
nhoening
left a comment
nhoening
left a comment
There was a problem hiding this comment.
I have one question and one thing that should be improved.
Also, resolve merge conflicts, then we see clearer.
| if bdf.empty and staleness_search.get("beliefs_before") is not None: | ||
| # The most-recent-only fast track picks the single most recent event in the database, | ||
| # and only afterwards drops beliefs that were formed after `beliefs_before`. | ||
| # For sensors with a knowledge horizon, such as day-ahead prices, that can drop the only row we fetched, |
There was a problem hiding this comment.
I don't really understand this one.
When the knowledge horizon exists, why wouldn't we update the beliefs_before parameter so the fast track doesn't drop the beliefs we're interested in?
There was a problem hiding this comment.
Because those beliefs should be dropped widening beliefs_before would show data we can't know yet. Tomorrow's day ahead prices stored with a belief time at their 12:00 knowledge time aren't available at 10:00. The bug is what happens after the drop, most_recent_only fetches the single most recent event first then filters by belief time. So it throws away its one row and returns nothing even though the sensor holds older data we could know about by now. That empty result is the "no data recorded".
The retry asks what we actually mean, of the events knowable by now, which is the most recent? It only runs when the fast track came back empty.
There was a problem hiding this comment.
It seems we are not talking about the same - or the explanation is not yet clear. I don't know what "widening beliefs_before" means to you.
Can we not combine the fast track with beliefs_before, so we would only find the most recent belief that was formed before the cutoff time we allow for the sensor in question? (to me, that would be "tightening", not "widening") .
Then nothing would need to be dropped, the one most recent is already most recent w.r.t. the cutoff point.
For the example of day-ahead prices, you mean we already have them, but their belief time is in the future, and they should thus be invisible? Is that not an edge case, more for simulations?
…1718-status-page-sensor-relation # Conflicts: # documentation/api/change_log.rst # documentation/changelog.rst # flexmeasures/data/services/sensors.py # flexmeasures/ui/templates/sensors/status.html
Answering review feedback: the comment explained what the retry does, but not why widening `beliefs_before` is not the fix instead. Those are exactly the beliefs we should not be using yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sensor-relation # Conflicts: # documentation/api/change_log.rst
Closes #1718.
The status page misreported sensors in a few ways. Running the toy battery tutorial shows all of them at once, on the day-ahead price sensor.
What was wrong, and why
1. "no data recorded" for a sensor full of data. The page only looked for seven hardcoded source types.
flexmeasures add beliefsrecords under the typeCLI script, which is not one of them, so the tutorial's prices were invisible. It now also looks at the source types that actually recorded on the sensor, which thesensor_data_sourcesummary table answers as a plain lookup.2. "no data recorded", second cause. The belief search uses a fast track that takes the single most recent event in SQL, and only afterwards drops beliefs formed after the moment we report on. For a sensor with a knowledge horizon, that can throw away the only row it fetched. When that happens, we now search again without the fast track and pick the most recent event ourselves.
3. A red light for data that is simply early. Tomorrow's prices are not knowable until today at noon. Such data was judged by the 12-hour lead time that only forecasters and schedulers should be held to, so it looked stale. It is fresh, and is now reported as such.
4. "sensor belongs to this asset", even when it doesn't. The relation was computed against the sensor's own asset, so the check was always true.
GET /sensors/<id>/statusnow accepts the asset whose page is being shown, and the status page passes it. The asset is permission-checked; without it, the sensor is still reported relative to its own asset.Screenshots
Before:
After:
How to check it yourself
Run the toy tutorial and open the building's status page. Before this PR, the day-ahead price sensor shows a red light, source type "None" and "Never"; after it, a green light, source type "CLI script", and a relation naming the NL transmission zone.
Testing
Three regression tests, each confirmed to fail without its fix. The API change is documented in the API change log; the four bugfixes share one changelog entry.
🤖 Generated with Claude Code