[clain23]

Summary

A LogQL range vector is (start, end] for every range function, not only the log-line ones. The proxy got both edges wrong for the unwrapped functions, in opposite directions.

Lower bound. aggregateManualWindow computed excludeStart := isLogRangeWindowFunc(functionName), so sum_over_time, avg_over_time, min_over_time, max_over_time, first_over_time, quantile_over_time, stddev_over_time and stdvar_over_time kept the sample sitting exactly on windowStart, the one Loki drops, and answered one sample high on every step. Loki v3.7.1, pkg/logql/range_vector.go, batchRangeVectorIterator.load:

// the lower bound of the range is not inclusive
if sample.Timestamp <= start {
    _ = r.iter.Next()
    continue
}

That iterator is what unwrapped ranges evaluate over too, so the bound is not a log-line special case.

Upper bound. VictoriaLogs' /select/logsql/query treats end as exclusive where Loki's window is inclusive at its end. The proxy compensated with a 1 ns extension of the fetch end, but only for quantile and the log-range functions. Correcting the lower bound alone would have left every other unwrapped function open at both ends, one sample short instead of one sample high. The extension is now unconditional. In measurement this is the larger change: on the e2e stack an unwrapped sum_over_time over [1s] read 8x low at the upper edge (4724 against Loki's 39196) and returned data at the lower edge where Loki returns none.

Concretely, over samples {0:1, 10:2, 20:3} and window (0, 20]:

function	before	after (Loki)
sum_over_time	6	5
avg_over_time	2	2.5
min_over_time	1	2
first_over_time	1	2

count_over_time, rate, bytes_over_time and bytes_rate are unchanged; they already excluded the lower bound.

rate_counter stays non-Loki. It takes the same (start, end] window, but Loki evaluates it through Prometheus' extrapolatedRate, which needs the sample timestamps this evaluator discards. Its value is still (last - first) / range over the samples in the window, and the CHANGELOG says so instead of listing it among the fixed functions.

Changes

• aggregateManualWindow: the lower bound is exclusive unconditionally, and the slice-building branch (quantile, stddev, stdvar, rate_counter) passes the same flag instead of functionName == "quantile".
• The 1 ns fetch-end extension is unconditional.
• The two dead reference implementations buildBareParserMetricMatrix and buildBareParserMetricVector, which still included windowStart for every function, are deleted. TestBoundedBareParserMetricOutput pinned the live evaluator against them and only passed because its samples never touched a window edge; it now pins the live buildBoundedBareParserMetric against explicit (t-range, t] results with a sample on each edge, and fails if either bound moves (checked by mutating the lower bound). TestProxyHelpers_BuildBareParserMetricMatrix tested the deleted function and goes with it.
• TestLowerBoundIsExclusiveForEveryRangeFunction (new) fails on main; first and quantile(q=0) carry the proof, max is there for coverage of the branch.
• TestCompatHelpers_AggregateManualWindow encoded the old bounds for sum/avg/min/first/stddev/stdvar/rate_counter; updated to Loki's, with the citation in the comment.

Validation

gofmt -s -l empty, go build ./..., go vet ./..., golangci-lint run (v2.13.2, 0 issues), scripts/ci/check_changelog_pr.py and scripts/ci/check_conformance.py ok, go test ./... -count=1 green across all packages, on top of current main (v1.86.0).

Found while syncing a downstream fork onto v1.81.0 and checking it against a real Loki 3.7.1 in test/e2e-compat.

[szibis]

Reviewed end to end. The core claim is correct and both halves were confirmed against the live stack.

Entry at T in {app="otel-collector"}, sum(sum_over_time({app="otel-collector"} | json | unwrap received [1s])):

eval time	window	Loki	main	this branch
T	(T-1s, T]	39196	4724	39196
T+1s	(T, T+1s]	empty	34472	empty

So on main an unwrapped sum_over_time reads 8x low at the upper edge and returns data where Loki returns none at the lower edge. I also confirmed the premise directly: VictoriaLogs /select/logsql/query treats end as exclusive, which is what makes the unconditional fetchEnd.Add(time.Nanosecond) a real fix.

Before merge:

1. Two reference implementations now disagree. aggregateManualWindow excludes windowStart, but buildBareParserMetricMatrix and buildBareParserMetricVector still include it for every function. Both are dead outside tests (the live bare-parser path is buildBoundedBareParserMetric, which is already correct), so this is not a live bug, but TestBoundedBareParserMetricOutput pins the live evaluator against the wrong one and only passes because its samples never touch a window edge. Please delete the dead pair or fix it here.
2. rate_counter. The new expectation is a different wrong value, and the comment says so: Loki runs it through Prometheus' extrapolatedRate and the proxy cannot reproduce that. The CHANGELOG lists rate_counter among the functions that "read one sample high against Loki", which reads as a parity fix. Please state plainly that it remains non-Loki, or drop it from that list.
3. The PR body describes only the lower bound; the second commit changes the upper bound for every function, which in measurement is the larger behaviour change. It is in the CHANGELOG but not the body.
4. max in the new test does not discriminate between the two candidate windows, as its own comment notes; first and quantile(q=0) carry the proof.
5. Attribution: the commits carry Co-Authored-By: Claude …. This repo's rule is no AI attribution anywhere; we will strip it in the squash message, but please remove it at the source.

Merge order: land this before #574. #574 puts evaluation points on whole-second grid positions, which turns the boundary case fixed here from rare into systematic on second-precision ingest.

[szibis]

Decision: accept, merge second, right after #572 and before #574, once two things are fixed.

Confirmed live in both directions: on main an unwrapped sum_over_time reads 8x low at the upper edge (4724 vs Loki 39196) and returns data where Loki returns none at the lower edge.

Please do:

1. Delete or fix the two dead reference implementations (buildBareParserMetricMatrix, buildBareParserMetricVector) that still include windowStart; TestBoundedBareParserMetricOutput currently pins the live evaluator against the wrong one and passes only because its samples never touch a window edge.
2. State in the CHANGELOG that rate_counter stays non-Loki (Prometheus extrapolatedRate), or drop it from the list of fixed functions; the new expectation is a different wrong value, as the code comment admits.
3. Update the PR body: the second commit changes the upper bound for every function, which is the larger behaviour change and is only in the CHANGELOG.

Also remove Co-Authored-By: Claude … from the commits: no AI attribution in this repo.

Merge order matters: this must land before #574, which puts evaluation points on whole-second grid positions and would turn this boundary case from rare into systematic.

[clain23]

Rebased onto current main (v1.86.0) and squashed to one commit:

1. Dead reference pair. buildBareParserMetricMatrix and buildBareParserMetricVector are deleted. TestBoundedBareParserMetricOutput now pins the live buildBoundedBareParserMetric against explicit (t-range, t] results with a sample on each edge (range 1s, samples at 1s and 2s); flipping the lower bound to inclusive makes it fail (checked). TestProxyHelpers_BuildBareParserMetricMatrix tested the deleted function and goes with it.
2. rate_counter. The CHANGELOG states it stays non-Loki (Prometheus' extrapolatedRate needs the timestamps this evaluator discards) and lists it separately from the fixed functions.
3. PR body. Rewritten to describe both bounds, with the upper bound named as the larger change and your 4724-vs-39196 measurement.
4. max is noted in the body as branch coverage; first and quantile(q=0) carry the proof.
5. Attribution. Removed from the commit.

Local: gofmt, build, vet, golangci-lint (0 issues), changelog and conformance gates, go test ./... -count=1 green.

[szibis]

Thank you @clain23. This change landed in #885 as a cherry-picked commit with you as author: the raw-row range evaluator now reads the window (t-range, t] for every function, released in v2.39.0 with your co-author credit. Closing this one as merged through #885.