Skip to content

FIX: Reread recently modified scorer metrics instead of caching them - #3697

Merged
Roman Lutz (romanlutz) merged 10 commits into
microsoft:mainfrom
WaqarAnwar:fix/scenario-progress-racy-metrics-cache
Nov 1, 2026
Merged

Roman Lutz (romanlutz) merged 10 commits into
microsoft:mainfrom
WaqarAnwar:fix/scenario-progress-racy-metrics-cache

Conversation

@WaqarAnwar

Copy link
Copy Markdown
Contributor

Description

Fixes #3696.

ScenarioProgressReadModel._load_objective_metrics caches scorer metrics under the metrics file's (st_mtime_ns, st_ctime_ns, st_size, st_ino). Filesystems update timestamps at the clock tick, so a same-length rewrite within one tick leaves the key unchanged. The GUI then keeps showing the old metrics. The same collision makes test_progress_reuses_metrics_until_file_changes_even_for_finished_runs flaky (assert 3 == 4).

This applies the racy-timestamp rule Git uses for its index: matching metadata proves the file is unchanged only once its mtime is older than the timestamp resolution. While the mtime is within _RECENT_FILE_WINDOW_NS (2 s, which covers FAT's resolution) of the current time, the metrics are read without caching. After that, caching works as before, so unchanged files are still never re-read and the I/O savings from #3348 are kept. The uncached read moved into _read_objective_metrics, which the cached loader also calls.

Tests and Documentation

  • test_progress_reuses_metrics_until_file_changes_even_for_finished_runs now backdates each write with os.utime, so cache hits and misses no longer depend on timing. 30 repeats pass; on main it failed about half the time when run on its own.
  • test_load_objective_metrics_rereads_recent_file_with_unchanged_metadata makes Path.stat return the same metadata across a same-length rewrite. It fails 10/10 without the fix (assert 0.75 == 0.85) and passes with it.
  • test_load_objective_metrics_reuses_cache_once_file_is_older_than_recent_window checks that a file older than the window is still served from the cache.
  • The repro script in BUG: Scenario progress serves stale scorer metrics after a same-length rewrite within one timestamp tick #3696 goes from 196–197/200 stale polls to 0/200.
  • tests/unit/backend: 3,313 passed. ruff check and ruff format are clean, and ty reports no new diagnostics on the changed files.

No documentation changes; the behavior is internal to the GUI backend.

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 microsoft#3696
@WaqarAnwar

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@romanlutz Roman Lutz (romanlutz) self-assigned this Nov 1, 2026
Roman Lutz (romanlutz) added a commit to FazeelUsmani/PyRIT that referenced this pull request Nov 1, 2026
Integrate microsoft#3770, microsoft#3697 and microsoft#3695 normally. As requested, retain the upstream deterministic one-node mapping-budget assertion, truncation and exact totals; do not restore the removed wall-clock benchmark.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Nov 1, 2026
Merged via the queue into microsoft:main with commit b365b03 Nov 1, 2026
65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: Scenario progress serves stale scorer metrics after a same-length rewrite within one timestamp tick

2 participants