Load only the newly selected days when a chart's time range changes

[Flix6x]

Description

Closes #101.

Changing the selected time range re-queried the whole range, even when nearly all of it was already loaded. Only a byte-identical selection was served from memory. This PR loads just the days that are actually new.

• ui/graphs: fetch all chart data through one module (chart-data-fetch.js), replacing four hand-built /chart_data query strings. No behaviour change; the generated URLs are unchanged.
• ui/graphs: add chart-data-cache.js, which works out what a newly selected window adds to the loaded one and fetches only that.
• ui/graphs: hold the widest contiguous span fetched so far and clip only what is handed to the chart, so narrowing the selection and widening it again costs nothing.
• ui/graphs: two KPI bugfixes, unrelated to Intelligent chart updating #101 but found on the way. getAssetKPIs mutated the very Date objects held by storeEndDate and previousResult.end, so on a KPI-enabled asset page both ran a day late after any selection. It also advanced an end date that both callers already pass as exclusive, while the endpoint ends its window before end as well, so every KPI covered one day more than the chart beside it (a three-day selection totalled four days).
• Added changelog item in documentation/changelog.rst

Design notes

The date picker only ever yields whole (nominal) days, so a selection is always one contiguous range and the cached span can be a single interval — no interval set, no gap solver. This follows the original prototype in empras/templates/index.html (updateData()), removed from main in ba8f900 shortly before #101 was filed.

Interval arithmetic stays on Date objects rather than millisecond counts, because a nominal day is 23 or 25 hours across a DST transition. There is a test for the 23-hour day.

A window that does not touch the cached span replaces it, so the cached span stays contiguous and cannot fragment. Memory is therefore bounded by the widest contiguous span browsed, which is what selecting that span in one go would have loaded anyway.

Merged records are de-duplicated rather than fetched as strictly non-overlapping ranges, because an event straddling the seam is returned by both halves (see the tests below). They are deliberately not re-sorted: fast-chart.js sorts each series by time itself, and Vega-Lite sorts line and area marks by their x channel.

Reproducing a fetch from records already held turns out to depend on three API behaviours, each now pinned by a test:

• A window selects events that overlap it (event_ends_after / event_starts_before), not events that start inside it. So the cache clips on overlap; filtering on event_start would drop the leading event of, say, a 50-minute sensor, whose event runs across midnight.
• Instantaneous sensors are matched inclusively on both edges.
• Resampling is anchored at the requested window start. A coarser sensor shown next to a 7-minute one therefore comes back on shifted timestamps for a window offset by a fraction of the resolution, and those records cannot be reconciled with the ones held. The cache detects this and falls back to fetching the window whole. Whole-day selections of sensors whose resolution divides a day — the common case — are always aligned and stay fully cached.

The resolution used for that arithmetic is derived, not read off a record: every sensor with a resolution is resampled to the finest one requested, while the response still reports each sensor's own (an hourly sensor next to a 15-minute one reports 3600 while its data arrives at 900).

Look & Feel

Profiled on a seeded asset with 5 sensors of 15-minute data over a year (175,200 beliefs).

Cost of one chart_data request, by window width:

window	server	payload	records
1 hour	105 ms	2.7 KiB	20
1 day	121 ms	40 KiB	480
7 days	191 ms	276 KiB	3,360
31 days	460 ms	1.2 MiB	14,880
365 days	4,402 ms	14 MiB	175,200

Where that time goes at 365 days: DB search 3,757 ms, serialisation 804 ms, transfer 142 ms (localhost), JSON.parse 99 ms, decompressChartData 74 ms. Server time is ~93% of the total, so this is worth fixing on the fetch side.

Two things worth recording. A request costs ~105 ms regardless of size (a 20-record window still costs 105 ms), plus ~24 µs per record. And that fixed floor is why splitting a range into one request per day would be a bad idea: 31 days would be 31 × 105 ms ≈ 3.3 s of pure overhead against 460 ms for a single request.

Cost of a navigation step, before and after this PR:

navigation step	before	after
step a 7-day window forward by a day	193 ms / 276 KiB	123 ms / 40 KiB	1.6×
extend a 31-day window by a day	489 ms / 1.2 MiB	121 ms / 40 KiB	4.0×
extend a 364-day window by a day	4,425 ms / 14 MiB	127 ms / 40 KiB	35×
widen a 31-day window on both sides	481 ms / 1.3 MiB	162 ms / 40 KiB	3.0×
zoom into 7 days inside a loaded 31-day window	195 ms / 276 KiB	—	no request
zoom back out to the 31-day window	464 ms / 1.2 MiB	—	no request

The both-sides figure is the sum of two sequential measurements; in the browser the two slivers are requested concurrently, so wall-clock is ~120 ms.

How to test

flexmeasures/data/tests/test_chart_data_window_splitting.py pins down the API contract the cache relies on: fetching [a, b) and [b, c) yields exactly the same beliefs, with the same values, as fetching [a, c). It covers resolutions that divide a day and one (7 minutes) that does not — in the latter case the event straddling the seam is returned on both sides, never dropped, and both sides agree on its value, which is what makes de-duplicating in the front end sufficient. It also pins the three behaviours listed under Design notes: overlap-based selection, inclusive edges for instantaneous sensors, and resampling anchored at the requested window start.

Manually, on an asset page with a lot of data, with the browser's network tab open:

1. Select a wide range, e.g. a year. One chart_data request.
2. Extend it by one day. One request, for that day only.
3. Narrow the selection to a week inside the loaded range. No request.
4. Widen it back to the original range. No request.
5. Save a change to "sensors to show". Full reload of the range, as before — the cache is reset whenever the underlying data may have changed.

Not covered by automated tests

The repo has no JavaScript test runner, so the cache logic was verified with a standalone harness driving the real modules in headless Chrome (58 checks: interval arithmetic, the 23-hour DST day, overlap clipping, the resampling-grid guard, de-duplication, request counts per navigation step, and cache invalidation). Those checks are not in CI. Adding a JS test runner feels like a separate decision, so I left it out — happy to follow up if wanted.

The live page was not driven end to end: the logic is covered as above, and both pages were verified to render with their emitted module parsing cleanly, but the wiring itself is only exercised by hand.

[read-the-docs-community]

Documentation build overview

📚 flexmeasures | 🛠️ Build #34196828 | 📁 Comparing 9f3ecd2 against latest (3272084)

  🔍 Preview build  

10 files changed · ± 10 modified

± Modified

• changelog.html
• configuration.html
• genindex.html
• _autosummary/flexmeasures.api.common.responses.html
• _autosummary/flexmeasures.api.common.utils.api_utils.html
• _autosummary/flexmeasures.data.html
• _autosummary/flexmeasures.data.models.planning.storage.html
• _autosummary/flexmeasures.data.utils.html
• api/v3_0.html
• concepts/flexibility-configuration.html

[Copilot]

Pull request overview

This PR improves asset/sensor chart navigation performance by introducing a front-end cache that reuses already-fetched chart data when the selected time window changes, so only newly needed days are requested. It also centralizes chart-data fetching into a single module, adds a backend contract test for window-splitting, and includes a small KPI-related date-mutation fix plus a changelog entry.

Changes:

• Refactor chart data and annotation fetching to go through chart-data-source.js, and integrate a new chart-data-cache.js into graphs.html.
• Add test_chart_data_window_splitting.py to pin down the backend API contract needed for safely fetching adjacent/overlapping windows.
• Add changelog entries describing the chart range-fetch optimization and the KPI/chart initial-load bugfix.

Reviewed changes

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

Show a summary per file

File	Description
flexmeasures/ui/templates/includes/graphs.html	Switch chart data/annotation requests to the new data-source module and use the new cache when the date range changes; fixes KPI/date mutation and initial window derivation.
flexmeasures/ui/static/js/chart-data-source.js	New module to build /chart_data URLs and fetch/decompress chart data (plus annotations) in one place.
flexmeasures/ui/static/js/chart-data-cache.js	New module to compute missing sub-ranges, fetch only what’s new, merge/dedupe, and return data for the selected window.
flexmeasures/data/tests/test_chart_data_window_splitting.py	New backend test module asserting that fetching a window in parts matches fetching it in one request.
documentation/changelog.rst	Adds user-facing entries for the performance improvement and KPI/chart load bugfix.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

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

Suppressed comments (6)

flexmeasures/ui/templates/includes/graphs.html:514

• chartDataCache is never populated with the initially fetched data (initialData uses fetchChartData directly, and previousResult is set, but the cache remains empty). As a result, the first user range change after page load will still fetch the entire newly selected window, even if it largely overlaps what was already loaded, which undermines the “load only newly selected days” behavior. Consider adding a way to prime the cache with {start,end,data} right after the initial fetch so subsequent changes can request only the missing slivers.

    // The module body is evaluated before the DOMContentLoaded handler that assigns storeStartDate/storeEndDate, so derive the initial window here.
    const initialStartDate = new Date('{{ event_starts_after }}');
    const initialEndDate = new Date('{{ event_ends_before }}');
    {% if active_subpage == "asset_graph" and has_kpis %}
    // Pass copies, because getAssetKPIs advances the end date it is given by a day.
    getAssetKPIs(new Date(initialStartDate), new Date(initialEndDate));
    {% endif %}
    const initialData = fetchChartData(dataPath, { start: initialStartDate, end: initialEndDate, signal: signal });
    let fetchedInitialData = await Promise.all([
        initialData,
        embedAndLoad(chartSpecsPath + 'event_starts_after=' + '{{ event_starts_after }}' + '&event_ends_before=' + '{{ event_ends_before }}' + '&', elementId, datasetName, previousResult, sessionStart, sessionEnd),
    ]).then(function (result) { return result[0] }).catch(console.error);
    $("#spinner").hide();
    if (!fastRenderActive() && vegaView) {
        vegaView.change(datasetName, vega.changeset().remove(vega.truthy).insert(fetchedInitialData)).resize().run();
    }
    var sessionStart = new Date('{{ event_starts_after }}');
    var sessionEnd = new Date('{{ event_ends_before }}');
    previousResult = {
        start: sessionStart,
        end: sessionEnd,
        data: fetchedInitialData
    };

flexmeasures/ui/templates/includes/graphs.html:462

• reloadChartData resets chartDataCache and then reloads the current window via fetchChartData, but the cache is not repopulated with the freshly fetched data. This means the next time-range change after a “sensorsToShowUpdated”/data-refresh event will again fetch the full window instead of only the new days. Consider priming the cache with the reloaded {start,end,data} after newData is received.

        $("#spinner").show();
        // The sensors shown, or the data itself, may have changed, so nothing held is still trustworthy.
        chartDataCache.reset();
        let newData = fetchChartData(dataPath, { start: storeStartDate, end: storeEndDate, signal: signal });
        newData = await Promise.all([
            newData,
            embedAndLoad(chartSpecsPath + 'event_starts_after=' + storeStartDate.toISOString() + '&event_ends_before=' + storeEndDate.toISOString() + '&', elementId, datasetName, previousResult, storeStartDate, storeEndDate),
        ]).then(function (result) { return result[0] }).catch(console.error);
        $("#spinner").hide();
        if (!fastRenderActive() && vegaView) {
            vegaView.change(datasetName, vega.changeset().remove(vega.truthy).insert(newData)).resize().run();
        }
        previousResult = {
            start: storeStartDate,
            end: storeEndDate,
            data: newData
        };

flexmeasures/data/tests/test_chart_data_window_splitting.py:7

• Module docstring wraps mid-phrase (e.g. line breaks after “time” and “asking”), which conflicts with the repo’s docstring wrapping convention (break lines only after punctuation). Please reflow the docstring so each physical line ends with punctuation (or keep the sentences on single lines).

"""Tests that a chart data window can be fetched in parts.

The asset and sensor pages reuse the data they already hold when the selected time
window changes, and fetch only the parts that are new (see chart-data-cache.js).
That is only sound if asking for [a, b) and [b, c) yields the same beliefs as asking
for [a, c) in one go, which is what these tests pin down.
"""

flexmeasures/ui/static/js/chart-data-cache.js:45

• This JSDoc comment wraps mid-phrase (line break after “after”), which conflicts with the repo’s comment wrapping convention (break lines only after punctuation). Please reflow so each physical line ends with punctuation, or keep the sentence on one line.

/**
 * Keep only the records for events that start inside the given window.
 *
 * This matches the API's own filtering, which returns events starting at or after
 * `event_starts_after`, so trimming here yields what a fetch would have returned.
 *

flexmeasures/ui/static/js/chart-data-cache.js:66

• This JSDoc block wraps mid-phrase in multiple places (e.g. line breaks after “the” and “in”), which conflicts with the repo’s comment wrapping convention (break lines only after punctuation). Please reflow the sentences so each physical line ends with punctuation.

/**
 * Drop records that describe the same belief twice.
 *
 * Splitting a window can hand back one event on both sides of the seam, when the
 * sensor's resolution does not divide the window evenly (verified in
 * test_chart_data_window_splitting.py).
 * Records are never lost that way, only repeated, so de-duplicating is enough.
 *

flexmeasures/ui/static/js/chart-data-cache.js:95

• This JSDoc block wraps mid-phrase in several places (e.g. line breaks after “on”, “browsed,” “the”, “series”), which conflicts with the repo’s comment wrapping convention (break lines only after punctuation). Please reflow the comment so line breaks occur only after punctuation.

/**
 * Create a cache that serves chart data for a window, fetching only what it lacks.
 *
 * The cache holds the widest contiguous span loaded so far, not just the span on
 * display, so narrowing the selection and widening it again costs nothing.
 * Its size is therefore bounded by the widest contiguous span the user has browsed,
 * which is what selecting that span in one go would have loaded anyway.
 *
 * Selecting a window that does not touch the cached span replaces it, keeping the
 * cached span contiguous.
 *
 * The merged records are deliberately not re-sorted: the fast chart sorts each series
 * by time itself, and Vega-Lite sorts line and area marks by their x channel.
 *

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

flexmeasures/ui/templates/includes/graphs.html:1008

• getAssetKPIs mutates its endDate argument (adds a day). The call sites here now defensively pass copies, but this side effect is easy to reintroduce elsewhere. Consider making getAssetKPIs non-mutating by cloning endDate inside the function before adjusting it, so callers can safely pass their real window dates.

            // Pass copies: getAssetKPIs advances the end date it is given by a day,
            // and this endDate is the very object held by storeEndDate and previousResult.end.
            getAssetKPIs(new Date(startDate), new Date(endDate));
            {% endif %}

flexmeasures/ui/static/js/chart-data-cache.js:180

• reusable currently requires strict overlap (end > cached.start && start < cached.end). For adjacent windows (e.g. shifting a 1‑day selection forward by 1 day so start === cached.end), this is false, so the cache is replaced instead of being extended to the union. That contradicts the “widest contiguous span fetched so far” goal and makes stepping back re-fetch. Consider allowing boundary equality here (treat adjacency as reusable/contiguous), so load will fetch the new window and then widen cached to include both.

      // Reuse only what a direct fetch would have returned identically.
      const reusable =
        cached !== null &&
        end > cached.start &&
        start < cached.end &&
        onSameResamplingGrid(cached.start, start, end, resolutionMs);
      const ranges = missingRanges(start, end, reusable ? cached : null);

[Flix6x]

Fixed in 9e3f654. The missingRanges doc now states that half-open holds for sensors with a resolution, and that instantaneous sensors are the exception, matched inclusively on both edges — with the reason it is harmless here: an instant on a boundary is fetched by the windows on either side and then de-duplicated.

[Flix6x]

Reflowed the JavaScript comments too (2ad1f0c-range: see ui/graphs: break the JavaScript comments only after punctuation), covering the JSDoc blocks and inline comments in chart-data-cache.js, chart-data-fetch.js and the graphs.html module. Verified mechanically: no comment line in the files this PR touches ends anywhere but at a comma, semicolon, colon or period.

I had previously left these on the grounds that .github/instructions/docstrings.instructions.md scoped the rule to **/*.py. That scoping was the actual problem, so the convention is now extended rather than worked around: applyTo covers .js and .html, the file is retitled "Docstrings and comments", the RST/Click/doctest guidance is marked Python-specific, and a JavaScript section spells out the rule for JSDoc and // comments with an example (@param/@returns lines stay on one line however long). The note to automated reviewers now says to report JavaScript comments that wrap mid-phrase, as it already did for Python docstrings.

108 Python tests and 72 browser checks still pass.

[Flix6x]

Second correction on the same point, and this one closes it out.

Felix asked me to investigate the "conditional handler brace" I reported. It is not a bug, and my description of it was wrong too — the same inference-from-indentation mistake, made twice.

What is actually true, from parsing both rendered pages:

page	DOMContentLoaded spans	initial window load	after storeStartDate assignment
asset	166–425	line 297, inside	yes (assigned at 198)
sensor	139–510	line 358, inside	yes (assigned at 171)

The handler closes unconditionally, at the end of the timezone-check block just before createPicker, on every page. It spans almost the whole module. There is no page-dependent extent and never was.

What misled me: the sensor upload block's upload.addEventListener('click', function (e) { is opened at eight spaces and closed at four, immediately before {% endif %}. That reads as the end of the DOMContentLoaded handler, so everything below it looks like module-level code. Purely cosmetic — nothing behaves differently.

Origin: it arrived with the upload block in 0e42c11 ("Move sensor data ingestion to job queue", #2101). The intended structure is what is there; only the indentation is off.

Consequences, in a1938ec:

• Copilot's original suggestion was right. The block now reads storeStartDate/storeEndDate instead of re-parsing the template values, as main did. The second copy of the window existed only to work around a problem that does not exist. Thanks for pushing on it — I dismissed it twice.
• Indented the upload listener's closing brace to match its opening, so the next reader is not caught the same way.
• The half of 04d6b1b about a first-visit crash was entirely spurious. Its message is already wrong in the history and force-push is blocked; the changelog, PR body and code comments are correct. The date-corruption half stands — it rests on object aliasing, not on nesting, and the separately measured KPI off-by-one is unaffected.

168 Python tests pass.

[Flix6x]

Split the KPI day-counting fix out into #2434, per @Flix6x's request, since it changes user-visible numbers while this PR is a performance change.

I tested separability rather than assuming it. On main the fix collapses to deleting one line — endDate.setDate(endDate.getDate() + 1) — which removes both the extra day and the in-place mutation, since the mutation was the advance. A trial merge of this branch into that one shows graphs.html merging cleanly: both branches converge on identical text for getAssetKPIs, so they compose in either order. The only conflict was the changelog, where both added a line in the same place, which is the routine adjacency every parallel PR here hits. I have removed the duplicate entry from this PR, so the line now lives only in #2434.

Worth being clear about what did not get split. This PR keeps the fix that stops getAssetKPIs mutating the Date objects held by storeEndDate and previousResult.end, because the cache depends on it: a corrupted end date would put the cached span's bookkeeping out by a day on any KPI-enabled asset page. Reverting that here would have created a genuine code conflict rather than avoiding one. So this PR still contains no user-visible KPI change; #2434 carries that.

[Flix6x]

A JS test suite would have caught a bug in this PR — it just did.

While investigating what JS testing would cost (@Flix6x's question), I probed an area none of the existing checks covered: two date selections in flight at once. Date selections are not serialised and their requests are not aborted — controller.abort() only runs in stopReplay() — so overlapping selections are reachable by clicking through ranges while a slow request is outstanding, and a year-wide window takes ~4.4 s.

The merge read what was held after awaiting, by which time another selection could have replaced it. Reproduced with controlled response ordering:

1. prime the cache with [1, 5)
2. select [1, 7) — reusable, fetches [5, 7)
3. select [20, 25) — disjoint, fetches the lot
4. let the second answer first

The cache then claimed [1, 25) while holding only the two ends. Selecting [10, 12) afterwards was served straight from that gap: an empty chart, with no request made. The widening selection also returned 2 of its 6 days, since it merged against the unrelated window.

Fixed in 9928c43 by snapshotting what is held on entry and merging against that snapshot, so a result's span and its records always describe the same thing, and by returning the window the call actually assembled rather than whatever is held when it finishes. Whichever selection finishes last decides what is kept; both spans are self-consistent, so the worst case is a later selection re-fetching. The scenario is now a regression check.

74 browser checks and 100 UI tests pass.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

flexmeasures/ui/templates/includes/graphs.html:693

• The date-picker "selected" handler starts new fetches without aborting any in-flight requests. If a user changes the range quickly, an older Promise.all can resolve after a newer one and overwrite the chart/previousResult with stale data. Consider aborting the previous controller (or using a monotonically increasing request id and ignoring out-of-date responses) when a new selection starts, and pass the new signal to all three async branches (chart data, annotations, embedAndLoad).

    checkDSTTransitions(startDate, endDate)
    Promise.all([
        fetchGraphDataAndKPIs(previousResult, startDate, endDate),
        // Fetch annotations
        fetchChartAnnotations(dataPath, { start: startDate, end: endDate, signal: signal }),

[Flix6x]

@Flix6x was right that the last review left something unaddressed, and it was a real bug. Fixed in 1b95e9c.

The picker's selected handler starts its requests without aborting any still in flight, and whichever Promise.all settles last draws itself and becomes previousResult. So a slow earlier selection answering after a quick later one put the older range on screen while the picker and the URL named the newer one.

This is not the race fixed in 9928c43. That one was inside the cache, and kept what is held self-consistent; it said nothing about which answer the page draws. Two separate races in the same feature, one layer apart.

Fixed with a generation counter rather than by aborting: the shared AbortController is also used by replay, and aborting would cancel the annotations and the chart embed alongside the data. The handler claims a count when it starts and drops its result if the count has moved on. reloadChartData bumps the same count, since a reload for changed data supersedes any selection in flight.

Verified the guard lands in the right scope by parsing the rendered page — the claim is a const inside the selection handler and the comparison happens after the awaited work. 100 UI tests and 74 browser checks pass.

On why I missed it, since it matters for how I read these: the review's headline said "generated no new comments", and the review-comments API returned nothing. The finding was in a collapsed Suppressed comments block in the review body, labelled "previously missed — in code that hasn't changed since the last review". Those are demoted by diff recency, not by confidence, and they do not appear in the comments API at all. Across this session's reviews, 5 said "no new comments" and 2 of those carried suppressed findings. I now read the full review body every time.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

flexmeasures/ui/templates/includes/graphs.html:515

• The initial page-load fetch (initialData / fetchedInitialData) is not protected by shownWindowGeneration, so a user selection made while that initial Promise.all is still in flight can render first and then be overwritten when this initial load resolves (chart and URL/picker can desync). Consider claiming a generation for the initial load and returning early if it’s stale before updating the chart/previousResult, consistent with the selection and reload paths.

    {% if event_starts_after and event_ends_before %}
    $("#spinner").show();
    {% if active_subpage == "asset_graph" and has_kpis %}
    getAssetKPIs(storeStartDate, storeEndDate);
    {% endif %}
    // Through the cache, so that the very first date change already only fetches the days it adds.
    const initialData = chartDataCache.load(dataPath, { start: storeStartDate, end: storeEndDate, signal: signal });
    let fetchedInitialData = await Promise.all([
        initialData,
        embedAndLoad(chartSpecsPath + 'event_starts_after=' + '{{ event_starts_after }}' + '&event_ends_before=' + '{{ event_ends_before }}' + '&', elementId, datasetName, previousResult, sessionStart, sessionEnd),
    ]).then(function (result) { return result[0] }).catch(console.error);
    $("#spinner").hide();
    if (!fastRenderActive() && vegaView) {
        vegaView.change(datasetName, vega.changeset().remove(vega.truthy).insert(fetchedInitialData)).resize().run();
    }

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

[Flix6x]

Only reference the PR.

[Flix6x]

Done in d88d5a3 — the entry now links only the PR.

I checked the other four entries I have added across this PR, #2434 and #2435: all of them already reference just their PR, so this was the only one.