Skip to content

Commit fc4e3d6

Browse files
GWealecopybara-github
GWeale
authored and
copybara-github
committed
fix: Fix double JSON encoding when saving eval set results
The `model_dump_json()` method already returns a JSON string, so wrapping it in `json.dumps()` was causing double encoding Close #3993 Co-authored-by: George Weale <[email protected]> PiperOrigin-RevId: 852455534
1 parent bfed19c commit fc4e3d6

5 files changed

Lines changed: 83 additions & 10 deletions

File tree

‎src/google/adk/evaluation/_eval_set_results_manager_utils.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@
1414

1515
from __future__ import annotations
1616

17+
import json
1718
import time
1819

20+
from pydantic import ValidationError
21+
1922
from .eval_result import EvalCaseResult
2023
from .eval_result import EvalSetResult
2124

@@ -42,3 +45,25 @@ def create_eval_set_result(
4245
creation_timestamp=timestamp,
4346
)
4447
return eval_set_result
48+
49+
50+
def parse_eval_set_result_json(
51+
eval_set_result_json: str | bytes,
52+
) -> EvalSetResult:
53+
"""Parses an EvalSetResult from JSON.
54+
55+
This is backward-compatible with legacy eval set result files that were
56+
double-encoded, where the outer JSON is a string containing the inner JSON
57+
object.
58+
"""
59+
try:
60+
return EvalSetResult.model_validate_json(eval_set_result_json)
61+
except (ValidationError, ValueError) as first_error:
62+
try:
63+
decoded = json.loads(eval_set_result_json)
64+
except json.JSONDecodeError:
65+
raise first_error
66+
67+
if isinstance(decoded, str):
68+
return EvalSetResult.model_validate_json(decoded)
69+
return EvalSetResult.model_validate(decoded)

‎src/google/adk/evaluation/gcs_eval_set_results_manager.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
from ..errors.not_found_error import NotFoundError
2424
from ._eval_set_results_manager_utils import create_eval_set_result
25+
from ._eval_set_results_manager_utils import parse_eval_set_result_json
2526
from .eval_result import EvalCaseResult
2627
from .eval_result import EvalSetResult
2728
from .eval_set_results_manager import EvalSetResultsManager
@@ -101,7 +102,7 @@ def get_eval_set_result(
101102
if not blob.exists():
102103
raise NotFoundError(f"Eval set result `{eval_set_result_id}` not found.")
103104
eval_set_result_data = blob.download_as_text()
104-
return EvalSetResult.model_validate_json(eval_set_result_data)
105+
return parse_eval_set_result_json(eval_set_result_data)
105106

106107
@override
107108
def list_eval_set_results(self, app_name: str) -> list[str]:

‎src/google/adk/evaluation/local_eval_set_results_manager.py‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@
1414

1515
from __future__ import annotations
1616

17-
import json
1817
import logging
1918
import os
2019

2120
from typing_extensions import override
2221

2322
from ..errors.not_found_error import NotFoundError
2423
from ._eval_set_results_manager_utils import create_eval_set_result
24+
from ._eval_set_results_manager_utils import parse_eval_set_result_json
2525
from .eval_result import EvalCaseResult
2626
from .eval_result import EvalSetResult
2727
from .eval_set_results_manager import EvalSetResultsManager
@@ -54,14 +54,13 @@ def save_eval_set_result(
5454
if not os.path.exists(app_eval_history_dir):
5555
os.makedirs(app_eval_history_dir)
5656
# Convert to json and write to file.
57-
eval_set_result_json = eval_set_result.model_dump_json()
5857
eval_set_result_file_path = os.path.join(
5958
app_eval_history_dir,
6059
eval_set_result.eval_set_result_name + _EVAL_SET_RESULT_FILE_EXTENSION,
6160
)
6261
logger.info("Writing eval result to file: %s", eval_set_result_file_path)
6362
with open(eval_set_result_file_path, "w", encoding="utf-8") as f:
64-
f.write(json.dumps(eval_set_result_json, indent=2))
63+
f.write(eval_set_result.model_dump_json(indent=2))
6564

6665
@override
6766
def get_eval_set_result(
@@ -79,8 +78,8 @@ def get_eval_set_result(
7978
if not os.path.exists(maybe_eval_result_file_path):
8079
raise NotFoundError(f"Eval set result `{eval_set_result_id}` not found.")
8180
with open(maybe_eval_result_file_path, "r", encoding="utf-8") as file:
82-
eval_result_data = json.load(file)
83-
return EvalSetResult.model_validate_json(eval_result_data)
81+
eval_result_data = file.read()
82+
return parse_eval_set_result_json(eval_result_data)
8483

8584
@override
8685
def list_eval_set_results(self, app_name: str) -> list[str]:

‎tests/unittests/evaluation/test_gcs_eval_set_results_manager.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
import json
16+
1517
from google.adk.errors.not_found_error import NotFoundError
1618
from google.adk.evaluation._eval_set_results_manager_utils import _sanitize_eval_set_result_name
1719
from google.adk.evaluation._eval_set_results_manager_utils import create_eval_set_result
@@ -165,6 +167,33 @@ def test_get_eval_set_result(self, gcs_eval_set_results_manager, mocker):
165167
)
166168
assert retrieved_eval_set_result == eval_set_result
167169

170+
def test_get_eval_set_result_double_encoded_legacy(
171+
self, gcs_eval_set_results_manager, mocker
172+
):
173+
mocker.patch("time.time", return_value=12345678)
174+
app_name = "test_app"
175+
eval_set_id = "test_eval_set"
176+
eval_case_results = _get_test_eval_case_results()
177+
eval_set_result = create_eval_set_result(
178+
app_name, eval_set_id, eval_case_results
179+
)
180+
181+
blob_name = gcs_eval_set_results_manager._get_eval_set_result_blob_name(
182+
app_name, eval_set_result.eval_set_result_id
183+
)
184+
blob = gcs_eval_set_results_manager.bucket.blob(blob_name)
185+
double_encoded_json = json.dumps(eval_set_result.model_dump_json())
186+
blob.upload_from_string(
187+
double_encoded_json, content_type="application/json"
188+
)
189+
190+
retrieved_eval_set_result = (
191+
gcs_eval_set_results_manager.get_eval_set_result(
192+
app_name, eval_set_result.eval_set_result_id
193+
)
194+
)
195+
assert retrieved_eval_set_result == eval_set_result
196+
168197
def test_list_eval_set_results(self, gcs_eval_set_results_manager, mocker):
169198
mocker.patch("time.time", return_value=123)
170199
app_name = "test_app"

‎tests/unittests/evaluation/test_local_eval_set_results_manager.py‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,12 @@ def test_save_eval_set_result(self, mocker):
8585
)
8686
assert os.path.exists(expected_file_path)
8787
with open(expected_file_path, "r") as f:
88-
actual_eval_set_result_json = json.load(f)
88+
actual_eval_set_result_data = json.load(f)
8989

90-
# need to convert eval_set_result to json
91-
expected_eval_set_result_json = self.eval_set_result.model_dump_json()
92-
assert expected_eval_set_result_json == actual_eval_set_result_json
90+
# Verify the file contains a proper JSON object (not double-encoded)
91+
# Use mode='json' to serialize enums to their values for comparison
92+
expected_eval_set_result_data = self.eval_set_result.model_dump(mode="json")
93+
assert expected_eval_set_result_data == actual_eval_set_result_data
9394

9495
def test_get_eval_set_result(self, mocker):
9596
mock_time = mocker.patch("time.time")
@@ -102,6 +103,24 @@ def test_get_eval_set_result(self, mocker):
102103
)
103104
assert retrieved_result == self.eval_set_result
104105

106+
def test_get_eval_set_result_double_encoded_legacy(self):
107+
eval_history_dir = os.path.join(
108+
self.agents_dir, self.app_name, _ADK_EVAL_HISTORY_DIR
109+
)
110+
os.makedirs(eval_history_dir, exist_ok=True)
111+
eval_set_result_file_path = os.path.join(
112+
eval_history_dir,
113+
self.eval_set_result_name + _EVAL_SET_RESULT_FILE_EXTENSION,
114+
)
115+
double_encoded_json = json.dumps(self.eval_set_result.model_dump_json())
116+
with open(eval_set_result_file_path, "w", encoding="utf-8") as f:
117+
f.write(double_encoded_json)
118+
119+
retrieved_result = self.manager.get_eval_set_result(
120+
self.app_name, self.eval_set_result_name
121+
)
122+
assert retrieved_result == self.eval_set_result
123+
105124
def test_get_eval_set_result_not_found(self, mocker):
106125
mock_time = mocker.patch("time.time")
107126
mock_time.return_value = self.timestamp

0 commit comments

Comments
 (0)