From 5bd8244a99468e57a4fede18ee37227b9231b162 Mon Sep 17 00:00:00 2001 From: Muhammad Waqar Anwar Date: Sat, 31 Oct 2026 21:13:22 +0500 Subject: [PATCH 1/2] FIX: Reread recently modified scorer metrics instead of caching them The scenario progress read model keyed its scorer-metrics cache on the file's (mtime_ns, ctime_ns, size, inode). Filesystems update timestamps at the clock tick, so a same-length rewrite within one tick kept the key unchanged and the GUI kept serving the old metrics. Treat metadata as proof the file is unchanged only once its mtime is older than a 2-second window; until then, read without caching. Backdate the metrics file in the cache-reuse test so it no longer depends on timing, and add regression tests for both sides of the window. Fixes #3696 --- .../services/scenario_progress_read_model.py | 17 ++++++ .../test_scenario_progress_read_model.py | 59 ++++++++++++++++++- 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/pyrit/backend/services/scenario_progress_read_model.py b/pyrit/backend/services/scenario_progress_read_model.py index 1a1284e890..e6a662c899 100644 --- a/pyrit/backend/services/scenario_progress_read_model.py +++ b/pyrit/backend/services/scenario_progress_read_model.py @@ -5,6 +5,7 @@ import asyncio import logging +import time import uuid from collections import OrderedDict from collections.abc import AsyncGenerator, Iterable, Mapping, Sequence @@ -151,6 +152,8 @@ class ScenarioProgressReadModel: _CACHE_MAX_RUNS = 32 _STORAGE_PAGE_SIZE = 500 + # Covers coarse filesystem timestamps, up to FAT's 2-second resolution. + _RECENT_FILE_WINDOW_NS = 2_000_000_000 def __init__(self, *, memory: MemoryInterface) -> None: """Initialize a read model over the scenario-result store.""" @@ -671,6 +674,10 @@ def _load_objective_metrics(*, eval_hash: str) -> ScenarioObjectiveScorerMetrics except FileNotFoundError: file_version = None else: + # A same-length rewrite within one timestamp tick keeps this metadata, so it only proves + # the file is unchanged once the tick has passed. Until then, read without caching. + if time.time_ns() - file_stat.st_mtime_ns < ScenarioProgressReadModel._RECENT_FILE_WINDOW_NS: + return ScenarioProgressReadModel._read_objective_metrics(eval_hash=eval_hash, file_path=file_path) # Check file metadata so unchanged metrics can be reused without rereading the file. file_version = (file_stat.st_mtime_ns, file_stat.st_ctime_ns, file_stat.st_size, file_stat.st_ino) @@ -696,6 +703,16 @@ def _load_unchanged_objective_metrics( The file version is part of the cache key, so edits trigger a fresh lookup. Cache key includes file_path and _file_version; changes to either bypass the cache. + Returns: + ScenarioObjectiveScorerMetrics | None: Display metrics, or None if no entry exists or read fails. + """ + return ScenarioProgressReadModel._read_objective_metrics(eval_hash=eval_hash, file_path=file_path) + + @staticmethod + def _read_objective_metrics(*, eval_hash: str, file_path: Path) -> ScenarioObjectiveScorerMetrics | None: + """ + Read a scorer's saved metrics from the metrics file without caching. + Returns: ScenarioObjectiveScorerMetrics | None: Display metrics, or None if no entry exists or read fails. """ diff --git a/tests/unit/backend/test_scenario_progress_read_model.py b/tests/unit/backend/test_scenario_progress_read_model.py index e97e23296a..389597a77d 100644 --- a/tests/unit/backend/test_scenario_progress_read_model.py +++ b/tests/unit/backend/test_scenario_progress_read_model.py @@ -5,7 +5,9 @@ import asyncio import json +import os import threading +import time import uuid from collections.abc import Callable from datetime import UTC, datetime, timedelta @@ -95,7 +97,10 @@ async def _get_snapshot_async(*, read_model: ScenarioProgressReadModel, run_id: ) -def _write_objective_metrics(*, file_path: Path, scorer_identifier: ComponentIdentifier, accuracy: float) -> None: +def _write_objective_metrics( + *, file_path: Path, scorer_identifier: ComponentIdentifier, accuracy: float, age_seconds: int | None = None +) -> None: + """Write one metrics entry, optionally backdating the file so the read model may cache it.""" file_path.parent.mkdir(parents=True, exist_ok=True) file_path.write_text( json.dumps( @@ -115,6 +120,9 @@ def _write_objective_metrics(*, file_path: Path, scorer_identifier: ComponentIde + "\n", encoding="utf-8", ) + if age_seconds is not None: + mtime_ns = time.time_ns() - age_seconds * 1_000_000_000 + os.utime(file_path, ns=(mtime_ns, mtime_ns)) @pytest.mark.parametrize("terminal", [False, True]) @@ -130,6 +138,7 @@ async def test_progress_reuses_metrics_until_file_changes_even_for_finished_runs file_path=file_path, scorer_identifier=scorer_identifier, accuracy=initial_accuracy, + age_seconds=30, ) memory = MagicMock(spec=MemoryInterface) memory.get_scenario_attack_result_deltas_async = AsyncMock(return_value=([], False)) @@ -165,6 +174,7 @@ async def read_progress_async(run_id: str = "run") -> ScenarioProgressSnapshot: file_path=file_path, scorer_identifier=scorer_identifier, accuracy=updated_accuracy, + age_seconds=20, ) updated = await read_progress_async() assert updated.summary.objective_scorer.metrics.accuracy == updated_accuracy @@ -183,12 +193,59 @@ async def read_progress_async(run_id: str = "run") -> ScenarioProgressSnapshot: file_path=file_path, scorer_identifier=scorer_identifier, accuracy=initial_accuracy, + age_seconds=10, ) restored = await read_progress_async() assert restored.summary.objective_scorer.metrics.accuracy == initial_accuracy assert load_metrics.call_count == 4 +def test_load_objective_metrics_rereads_recent_file_with_unchanged_metadata(tmp_path: Path) -> None: + file_path = tmp_path / "objective" / "objective_achieved_metrics.jsonl" + scorer_identifier = ComponentIdentifier(class_name="RecentScorer", class_module="tests") + eval_hash = ScorerEvaluationIdentifier(scorer_identifier).eval_hash + _write_objective_metrics(file_path=file_path, scorer_identifier=scorer_identifier, accuracy=0.75) + # A same-length rewrite within one timestamp tick leaves every field the cache key reads unchanged. + tick_stat = file_path.stat() + real_stat = Path.stat + + def stat_within_tick(path: Path, **kwargs: bool) -> os.stat_result: + return tick_stat if path == file_path else real_stat(path, **kwargs) + + with ( + patch.object(progress_module, "SCORER_EVALS_PATH", tmp_path), + patch.object(Path, "stat", autospec=True, side_effect=stat_within_tick), + ): + first = ScenarioProgressReadModel._load_objective_metrics(eval_hash=eval_hash) + _write_objective_metrics(file_path=file_path, scorer_identifier=scorer_identifier, accuracy=0.85) + rewritten = ScenarioProgressReadModel._load_objective_metrics(eval_hash=eval_hash) + + assert first.accuracy == 0.75 + assert rewritten.accuracy == 0.85 + + +def test_load_objective_metrics_reuses_cache_once_file_is_older_than_recent_window(tmp_path: Path) -> None: + file_path = tmp_path / "objective" / "objective_achieved_metrics.jsonl" + scorer_identifier = ComponentIdentifier(class_name="SettledScorer", class_module="tests") + eval_hash = ScorerEvaluationIdentifier(scorer_identifier).eval_hash + _write_objective_metrics(file_path=file_path, scorer_identifier=scorer_identifier, accuracy=0.75, age_seconds=10) + + with ( + patch.object(progress_module, "SCORER_EVALS_PATH", tmp_path), + patch.object( + progress_module, + "find_objective_metrics_by_eval_hash", + wraps=progress_module.find_objective_metrics_by_eval_hash, + ) as load_metrics, + ): + first = ScenarioProgressReadModel._load_objective_metrics(eval_hash=eval_hash) + second = ScenarioProgressReadModel._load_objective_metrics(eval_hash=eval_hash) + + assert first is second + assert first.accuracy == 0.75 + load_metrics.assert_called_once() + + async def test_progress_updates_scorer_identity_without_new_results(tmp_path: Path) -> None: file_path = tmp_path / "objective" / "objective_achieved_metrics.jsonl" first_scorer = ComponentIdentifier(class_name="FirstScorer", class_module="tests") From 86a6f38ad3a9deb3ac5e89797e9858f9f146d84e Mon Sep 17 00:00:00 2001 From: Roman Lutz Date: Sat, 31 Oct 2026 23:49:56 -0700 Subject: [PATCH 2/2] TST: Harden scorer metrics cache timing regressions Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../test_scenario_progress_read_model.py | 46 ++++++++++++++++--- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/tests/unit/backend/test_scenario_progress_read_model.py b/tests/unit/backend/test_scenario_progress_read_model.py index 389597a77d..85643d192d 100644 --- a/tests/unit/backend/test_scenario_progress_read_model.py +++ b/tests/unit/backend/test_scenario_progress_read_model.py @@ -215,23 +215,55 @@ def stat_within_tick(path: Path, **kwargs: bool) -> os.stat_result: with ( patch.object(progress_module, "SCORER_EVALS_PATH", tmp_path), patch.object(Path, "stat", autospec=True, side_effect=stat_within_tick), + patch.object( + progress_module.time, "time_ns", return_value=tick_stat.st_mtime_ns + 1_000_000_000 + ) as current_time, + patch.object( + progress_module, + "find_objective_metrics_by_eval_hash", + wraps=progress_module.find_objective_metrics_by_eval_hash, + ) as load_metrics, ): first = ScenarioProgressReadModel._load_objective_metrics(eval_hash=eval_hash) _write_objective_metrics(file_path=file_path, scorer_identifier=scorer_identifier, accuracy=0.85) rewritten = ScenarioProgressReadModel._load_objective_metrics(eval_hash=eval_hash) + current_time.return_value = tick_stat.st_mtime_ns + 2_000_000_000 + settled = ScenarioProgressReadModel._load_objective_metrics(eval_hash=eval_hash) + unchanged = ScenarioProgressReadModel._load_objective_metrics(eval_hash=eval_hash) + assert first is not None + assert rewritten is not None + assert settled is not None assert first.accuracy == 0.75 assert rewritten.accuracy == 0.85 + assert settled.accuracy == 0.85 + assert unchanged is settled + assert load_metrics.call_count == 3 -def test_load_objective_metrics_reuses_cache_once_file_is_older_than_recent_window(tmp_path: Path) -> None: +@pytest.mark.parametrize( + ("age_ns", "expected_reads"), + [ + pytest.param(-2_000_000_000, 2, id="future-timestamp"), + pytest.param(0, 2, id="just-modified"), + pytest.param(1_999_999_999, 2, id="before-boundary"), + pytest.param(2_000_000_000, 1, id="at-boundary"), + pytest.param(2_000_000_001, 1, id="after-boundary"), + pytest.param(10_000_000_000, 1, id="settled"), + ], +) +def test_load_objective_metrics_cache_policy_at_file_age_boundaries( + *, tmp_path: Path, age_ns: int, expected_reads: int +) -> None: file_path = tmp_path / "objective" / "objective_achieved_metrics.jsonl" - scorer_identifier = ComponentIdentifier(class_name="SettledScorer", class_module="tests") + scorer_identifier = ComponentIdentifier(class_name="BoundaryScorer", class_module="tests") eval_hash = ScorerEvaluationIdentifier(scorer_identifier).eval_hash - _write_objective_metrics(file_path=file_path, scorer_identifier=scorer_identifier, accuracy=0.75, age_seconds=10) + _write_objective_metrics(file_path=file_path, scorer_identifier=scorer_identifier, accuracy=0.75) + mtime_ns = file_path.stat().st_mtime_ns with ( patch.object(progress_module, "SCORER_EVALS_PATH", tmp_path), + patch.object(progress_module.time, "time_ns", return_value=mtime_ns + age_ns), patch.object( progress_module, "find_objective_metrics_by_eval_hash", @@ -241,9 +273,11 @@ def test_load_objective_metrics_reuses_cache_once_file_is_older_than_recent_wind first = ScenarioProgressReadModel._load_objective_metrics(eval_hash=eval_hash) second = ScenarioProgressReadModel._load_objective_metrics(eval_hash=eval_hash) - assert first is second - assert first.accuracy == 0.75 - load_metrics.assert_called_once() + assert first is not None + assert second is not None + assert first.accuracy == second.accuracy == 0.75 + assert (first is second) == (expected_reads == 1) + assert load_metrics.call_count == expected_reads async def test_progress_updates_scorer_identity_without_new_results(tmp_path: Path) -> None: