From 83f7712233636435252d11525a10f0923d9cf428 Mon Sep 17 00:00:00 2001
From: Jeremy Lewi <jeremy@lewi.us>
Date: Tue, 15 Oct 2024 18:23:52 -0700
Subject: [PATCH] Fix bug in evaluation report (#303)

CellsMatchResult can be nil. I think this is the result of default
values not being serialized in the protojson.

Update the results manager to include default values when serializing to
JSON. This should make SQL queries easier.
---
 app/pkg/eval/evaluator.go       | 14 ++++++++++++--
 app/pkg/eval/results_manager.go |  6 +++++-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/app/pkg/eval/evaluator.go b/app/pkg/eval/evaluator.go
index b6ebb0a..fbc70d6 100644
--- a/app/pkg/eval/evaluator.go
+++ b/app/pkg/eval/evaluator.go
@@ -640,9 +640,19 @@ func (e *Evaluator) buildExperimentReport(ctx context.Context, name string, mana
 	r.CellsMatchCounts = make(map[string]int32)
 
 	for _, c := range counts {
+		if c.MatchResult == nil {
+			// N.B. I think for unknown it ends up being a nil value. I suspect this is because default values are
+			// elided when marshalling to JSON. We should fix that by changing the JSON serialization.
+			key := v1alpha1.CellsMatchResult_UNKNOWN_CellsMatchResult.String()
+			if _, ok := r.CellsMatchCounts[key]; !ok {
+				r.CellsMatchCounts[key] = 0
+			}
+			r.CellsMatchCounts[key] = r.CellsMatchCounts[key] + int32(c.Count)
+			continue
+		}
 		s, ok := c.MatchResult.(string)
 		if !ok {
-			return r, errors.Wrapf(err, "Failed to convert cellsMatchResult to string")
+			return r, errors.New("Failed to convert cellsMatchResult to string")
 		}
 		r.CellsMatchCounts[s] = int32(c.Count)
 	}
@@ -679,7 +689,7 @@ func (e *Evaluator) buildExperimentReport(ctx context.Context, name string, mana
 		}
 	}
 
-	percentiles, err := computePercentilesOfInts(generateTimes, []float64{.9, .95})
+	percentiles, err := computePercentilesOfInts(generateTimes, []float64{.5, .75, .9, .95})
 	if err != nil {
 		return r, errors.Wrapf(err, "Failed to compute percentiles")
 	}
diff --git a/app/pkg/eval/results_manager.go b/app/pkg/eval/results_manager.go
index f35541b..a76a0a8 100644
--- a/app/pkg/eval/results_manager.go
+++ b/app/pkg/eval/results_manager.go
@@ -214,7 +214,11 @@ func (m *ResultsManager) ListResults(ctx context.Context, cursor *time.Time, pag
 }
 
 func protoToRowUpdate(result *v1alpha1.EvalResult) (*fsql.UpdateResultParams, error) {
-	protoJson, err := protojson.Marshal(result)
+	// Emit default values. Otherwise SQL queries become more complex.
+	opts := protojson.MarshalOptions{
+		EmitDefaultValues: true,
+	}
+	protoJson, err := opts.Marshal(result)
 	if err != nil {
 		return nil, errors.Wrapf(err, "Failed to serialize EvalResult to JSON")
 	}