promqltest: Properly distinguish explicit counter_reset_hint specification

This commit addresses the PR feedback for issue #17615. The previous
implementation could not distinguish between:
- No counter reset hint specified (meaning "don't care")
- counter_reset_hint:unknown explicitly specified (meaning "verify it's unknown")

Changes:
- Added CounterResetHintSet field to parser.SequenceValue to track
  whether counter_reset_hint was explicitly specified in the test file
- Modified buildHistogramFromMap to set this flag when the hint is
  present in the descriptor map
- Updated newHistogramSequenceValue helper and histogramsSeries
  functions to propagate the flag through histogram series creation
- Updated yacc grammar to use the new helper function
- Modified compareNativeHistogram to accept the flag and only compare
  hints when explicitly specified

This allows tests to:
1. Not specify a hint (no comparison, backward compatible)
2. Explicitly specify counter_reset_hint:unknown (verify it's unknown)
3. Explicitly specify counter_reset_hint:gauge/reset/not_reset (verify match)

Fixes #17615

Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>

promql/parser/generated_parser.y

@@ -790,14 +790,15 @@ series_item     : BLANK
                 // Histogram descriptions (part of unit testing).
                 | histogram_series_value
                         {
-                        $$ = []SequenceValue{{Histogram:$1}}
+                        $$ = []SequenceValue{yylex.(*parser).newHistogramSequenceValue($1)}
                         }
                 | histogram_series_value TIMES uint
                         {
                         $$ = []SequenceValue{}
                         // Add an additional value for time 0, which we ignore in tests.
+                        sv := yylex.(*parser).newHistogramSequenceValue($1)
                         for i:=uint64(0); i <= $3; i++{
-                                $$ = append($$, SequenceValue{Histogram:$1})
+                                $$ = append($$, sv)
                                 //$1 += $2
                         }
                         }

promql/parser/generated_parser.y.go

@@ -1835,15 +1835,16 @@ yydefault:
 	case 158:
 		yyDollar = yyS[yypt-1 : yypt+1]
 		{
-			yyVAL.series = []SequenceValue{{Histogram: yyDollar[1].histogram}}
+			yyVAL.series = []SequenceValue{yylex.(*parser).newHistogramSequenceValue(yyDollar[1].histogram)}
 		}
 	case 159:
 		yyDollar = yyS[yypt-3 : yypt+1]
 		{
 			yyVAL.series = []SequenceValue{}
 			// Add an additional value for time 0, which we ignore in tests.
+			sv := yylex.(*parser).newHistogramSequenceValue(yyDollar[1].histogram)
 			for i := uint64(0); i <= yyDollar[3].uint; i++ {
-				yyVAL.series = append(yyVAL.series, SequenceValue{Histogram: yyDollar[1].histogram})
+				yyVAL.series = append(yyVAL.series, sv)
 				//$1 += $2
 			}
 		}

promql/parser/parse.go

@@ -67,6 +67,11 @@ type parser struct {
 
 	generatedParserResult any
 	parseErrors           ParseErrors
+
+	// lastHistogramCounterResetHintSet is set to true when the most recently
+	// built histogram had a counter_reset_hint explicitly specified.
+	// This is used to populate CounterResetHintSet in SequenceValue.
+	lastHistogramCounterResetHintSet bool
 }
 
 type Opt func(p *parser)
@@ -234,6 +239,11 @@ type SequenceValue struct {
 	Value     float64
 	Omitted   bool
 	Histogram *histogram.FloatHistogram
+	// CounterResetHintSet is true if the counter reset hint was explicitly
+	// specified in the test file using counter_reset_hint:... syntax.
+	// This allows distinguishing between "no hint specified" (don't care)
+	// vs "counter_reset_hint:unknown" (verify it's unknown).
+	CounterResetHintSet bool
 }
 
 func (v SequenceValue) String() string {
@@ -496,25 +506,30 @@ func (p *parser) mergeMaps(left, right *map[string]any) (ret *map[string]any) {
 }
 
 func (p *parser) histogramsIncreaseSeries(base, inc *histogram.FloatHistogram, times uint64) ([]SequenceValue, error) {
-	return p.histogramsSeries(base, inc, times, func(a, b *histogram.FloatHistogram) (*histogram.FloatHistogram, error) {
+	// Capture the hint set flag immediately after inc histogram is built.
+	// The base histogram's hint set flag was already captured.
+	hintSet := p.lastHistogramCounterResetHintSet
+	return p.histogramsSeries(base, inc, times, hintSet, func(a, b *histogram.FloatHistogram) (*histogram.FloatHistogram, error) {
 		res, _, _, err := a.Add(b)
 		return res, err
 	})
 }
 
 func (p *parser) histogramsDecreaseSeries(base, inc *histogram.FloatHistogram, times uint64) ([]SequenceValue, error) {
-	return p.histogramsSeries(base, inc, times, func(a, b *histogram.FloatHistogram) (*histogram.FloatHistogram, error) {
+	// Capture the hint set flag immediately after inc histogram is built.
+	hintSet := p.lastHistogramCounterResetHintSet
+	return p.histogramsSeries(base, inc, times, hintSet, func(a, b *histogram.FloatHistogram) (*histogram.FloatHistogram, error) {
 		res, _, _, err := a.Sub(b)
 		return res, err
 	})
 }
 
-func (*parser) histogramsSeries(base, inc *histogram.FloatHistogram, times uint64,
+func (*parser) histogramsSeries(base, inc *histogram.FloatHistogram, times uint64, counterResetHintSet bool,
 	combine func(*histogram.FloatHistogram, *histogram.FloatHistogram) (*histogram.FloatHistogram, error),
 ) ([]SequenceValue, error) {
 	ret := make([]SequenceValue, times+1)
 	// Add an additional value (the base) for time 0, which we ignore in tests.
-	ret[0] = SequenceValue{Histogram: base}
+	ret[0] = SequenceValue{Histogram: base, CounterResetHintSet: counterResetHintSet}
 	cur := base
 	for i := uint64(1); i <= times; i++ {
 		if cur.Schema > inc.Schema {
@@ -526,7 +541,7 @@ func (*parser) histogramsSeries(base, inc *histogram.FloatHistogram, times uint6
 		if err != nil {
 			return ret, err
 		}
-		ret[i] = SequenceValue{Histogram: cur}
+		ret[i] = SequenceValue{Histogram: cur, CounterResetHintSet: counterResetHintSet}
 	}
 
 	return ret, nil
@@ -535,6 +550,8 @@ func (*parser) histogramsSeries(base, inc *histogram.FloatHistogram, times uint6
 // buildHistogramFromMap is used in the grammar to take then individual parts of the histogram and complete it.
 func (p *parser) buildHistogramFromMap(desc *map[string]any) *histogram.FloatHistogram {
 	output := &histogram.FloatHistogram{}
+	// Reset the flag for each new histogram being built.
+	p.lastHistogramCounterResetHintSet = false
 
 	val, ok := (*desc)["schema"]
 	if ok {
@@ -595,6 +612,8 @@ func (p *parser) buildHistogramFromMap(desc *map[string]any) *histogram.FloatHis
 
 	val, ok = (*desc)["counter_reset_hint"]
 	if ok {
+		// Mark that the counter reset hint was explicitly specified.
+		p.lastHistogramCounterResetHintSet = true
 		resetHint, ok := val.(Item)
 
 		if ok {
@@ -626,6 +645,16 @@ func (p *parser) buildHistogramFromMap(desc *map[string]any) *histogram.FloatHis
 	return output
 }
 
+// newHistogramSequenceValue creates a SequenceValue for a histogram,
+// setting CounterResetHintSet based on whether counter_reset_hint was
+// explicitly specified in the histogram description.
+func (p *parser) newHistogramSequenceValue(h *histogram.FloatHistogram) SequenceValue {
+	return SequenceValue{
+		Histogram:           h,
+		CounterResetHintSet: p.lastHistogramCounterResetHintSet,
+	}
+}
+
 func (p *parser) buildHistogramBucketsAndSpans(desc *map[string]any, bucketsKey, offsetKey string,
 ) (buckets []float64, spans []histogram.Span) {
 	bucketCount := 0

promql/promqltest/test.go

@@ -1009,7 +1009,12 @@ func (ev *evalCmd) compareResult(result parser.Value) error {
 			exp := ev.expected[hash]
 
 			var expectedFloats []promql.FPoint
-			var expectedHistograms []promql.HPoint
+			// expectedHPoint wraps HPoint with CounterResetHintSet flag from SequenceValue.
+			type expectedHPoint struct {
+				promql.HPoint
+				CounterResetHintSet bool
+			}
+			var expectedHistograms []expectedHPoint
 
 			for i, e := range exp.vals {
 				ts := ev.start.Add(time.Duration(i) * ev.step)
@@ -1021,7 +1026,10 @@ func (ev *evalCmd) compareResult(result parser.Value) error {
 				t := ts.UnixNano() / int64(time.Millisecond/time.Nanosecond)
 
 				if e.Histogram != nil {
-					expectedHistograms = append(expectedHistograms, promql.HPoint{T: t, H: e.Histogram})
+					expectedHistograms = append(expectedHistograms, expectedHPoint{
+						HPoint:              promql.HPoint{T: t, H: e.Histogram},
+						CounterResetHintSet: e.CounterResetHintSet,
+					})
 				} else if !e.Omitted {
 					expectedFloats = append(expectedFloats, promql.FPoint{T: t, F: e.Value})
 				}
@@ -1050,7 +1058,7 @@ func (ev *evalCmd) compareResult(result parser.Value) error {
 					return fmt.Errorf("expected histogram value at index %v for %s to have timestamp %v, but it had timestamp %v (result has %s)", i, ev.metrics[hash], expected.T, actual.T, formatSeriesResult(s))
 				}
 
-				if !compareNativeHistogram(expected.H.Compact(0), actual.H.Compact(0)) {
+				if !compareNativeHistogram(expected.H.Compact(0), actual.H.Compact(0), expected.CounterResetHintSet) {
 					return fmt.Errorf("expected histogram value at index %v (t=%v) for %s to be %v, but got %v (result has %s)", i, actual.T, ev.metrics[hash], expected.H.TestExpression(), actual.H.TestExpression(), formatSeriesResult(s))
 				}
 			}
@@ -1089,7 +1097,7 @@ func (ev *evalCmd) compareResult(result parser.Value) error {
 			if expH != nil && v.H == nil {
 				return fmt.Errorf("expected histogram %s for %s but got float value %v", HistogramTestExpression(expH), v.Metric, v.F)
 			}
-			if expH != nil && !compareNativeHistogram(expH.Compact(0), v.H.Compact(0)) {
+			if expH != nil && !compareNativeHistogram(expH.Compact(0), v.H.Compact(0), exp0.CounterResetHintSet) {
 				return fmt.Errorf("expected %v for %s but got %s", HistogramTestExpression(expH), v.Metric, HistogramTestExpression(v.H))
 			}
 			if !almost.Equal(exp0.Value, v.F, defaultEpsilon) {
@@ -1127,7 +1135,9 @@ func (ev *evalCmd) compareResult(result parser.Value) error {
 
 // compareNativeHistogram is helper function to compare two native histograms
 // which can tolerate some differ in the field of float type, such as Count, Sum.
-func compareNativeHistogram(exp, cur *histogram.FloatHistogram) bool {
+// The counterResetHintSet parameter indicates whether the counter reset hint was
+// explicitly specified in the expected histogram (from the test file).
+func compareNativeHistogram(exp, cur *histogram.FloatHistogram, counterResetHintSet bool) bool {
 	if exp == nil || cur == nil {
 		return false
 	}
@@ -1164,8 +1174,9 @@ func compareNativeHistogram(exp, cur *histogram.FloatHistogram) bool {
 	}
 
 	// Compare CounterResetHint only if explicitly specified in expected histogram.
-	// UnknownCounterReset (the default) means "don't care about the hint".
-	if exp.CounterResetHint != histogram.UnknownCounterReset {
+	// When counterResetHintSet is false, no hint was specified, meaning "don't care".
+	// When counterResetHintSet is true, the hint was explicitly specified and must match.
+	if counterResetHintSet {
 		if exp.CounterResetHint != cur.CounterResetHint {
 			return false
 		}