Bug: horizon minutes not working with latest forecast

[peterdudfield]

Currently when pulling a forecast using dp.GetForecastAsTimeseriesRequest and using horizon_mins=120 is no different from horizon_mins=None for future timestamps. For past forecasts it is different.

I think we need to do change this line
from

AND f.init_time_utc <= sqlc.arg(pivot_timestamp)::TIMESTAMP

to

AND f.init_time_utc <= sqlc.arg(pivot_timestamp)::TIMESTAMP - MAKE_INTERVAL(mins => sqlc.arg(horizon_mins)::INTEGER)

[AnujNayak108]

As I understand it, the issue is that for latest forecasts we currently only constrain init_time_utc by the pivot timestamp, so the horizon isn’t actually enforced for future targets. Subtracting horizon_mins from the pivot would ensure that only forecasts initialized far enough back are eligible, which matches the expected definition of horizon. So This makes sense to me.

One thing I wanted to double-check: is pivot_timestamp always representing the target time we’re anchoring the horizon against (rather than request time)?

A small follow-up idea (optional): would it be worth explicitly handling horizon_mins IS NULL in the query (i.e. keeping current behaviour in that case), to make the semantics a bit clearer and avoid subtle differences later?