feat (CritPt): improve AA scoring recovery and add key rotation - #1944
Conversation
…s; replay tool Signed-off-by: Martina G. Vilas <mgonzalezvil@nvidia.com>
Signed-off-by: Martina G. Vilas <mgonzalezvil@nvidia.com>
…reshed per-batch is sufficient Signed-off-by: Martina G. Vilas <mgonzalezvil@nvidia.com>
Signed-off-by: Martina G. Vilas <mgonzalezvil@nvidia.com>
Signed-off-by: Martina G. Vilas <mgonzalezvil@nvidia.com>
- Isolate cache files per server launch. - Anchor relative CRITPT_CACHE_DIR to the repo root. - Add replay --fire-after support for smoke runs. - Document replay recovery and add focused tests. Signed-off-by: Martina G. Vilas <mgonzalezvil@nvidia.com>
Signed-off-by: Martina G. Vilas <mgonzalezvil@nvidia.com>
Signed-off-by: Martina G. Vilas <mgonzalezvil@nvidia.com>
Signed-off-by: Martina G. Vilas <mgonzalezvil@nvidia.com>
9cb872a to
b7f25bc
Compare
| return key | ||
|
|
||
| # ────────────────────────────────────────────────────────── | ||
| # Persistence helpers (cache_dir-gated; no-op when unset) |
There was a problem hiding this comment.
Can we add unit tests for the caching + replay paths?
verify()test withcache_dirset that fires a batch and checks the 3 files get written correctly (submissions.jsonl, aa_responses.jsonl, partial_metrics.json)main_async()test that seeds a temp cache, checks it skips already-scored submissions and ships only pending batch, returns right exit codes, etc.
There was a problem hiding this comment.
Done. Added unit coverage for both paths:
verify()withcache_dirset now fires a mocked batch and assertssubmissions.jsonl,aa_responses.jsonl, andpartial_metrics.jsonare written with the expected contents.replay.main_async()now has temp-cache tests for skipping already-scored submissions, shipping only pending batches, and the expected exit codes for success/nothing pending, missing cache, and quota exhaustion.
| f"timeout_rate={response.get('timeout_rate')}" | ||
| ) | ||
|
|
||
| print(f"Replay complete. Rejudged {rejudged} batches.") |
There was a problem hiding this comment.
(non-blocking) This is minor but should we call _refresh_partial_metrics() again here? It might be confusing if a replay results in another incomplete run. We might have to move it out of the resource server class and into the module if so. If it is not worth the effort we can leave it as is since it's mostly cosmetic.
There was a problem hiding this comment.
Good point, thanks. I addressed this by moving the metrics refresh logic into a module-level refresh_partial_metrics(cache_dir) helper and reusing it from both the resource server and replay tool.
replay.py now refreshes partial_metrics.json after successful replay completion, and also on quota exhaustion if at least one batch was scored before the 429. That keeps the metrics file aligned with the on-disk submissions.jsonl / aa_responses.jsonl state even after an incomplete replay.
Add unit tests for the on-disk scoring cache and replay paths: - verify() writes submissions.jsonl, aa_responses.jsonl, and partial_metrics.json on a full batch - replay ships only pending submissions and skips already-scored ones - replay exit codes: 0 (nothing pending/success), 2 (missing cache), 3 (AA quota exhausted, no response appended) Signed-off-by: Martina G. Vilas <mgonzalezvil@nvidia.com>
Signed-off-by: Martina G. Vilas <mgonzalezvil@nvidia.com>
2 participants
Summary
This MR improves CritPt resilience around Artificial Analysis scoring failures.
resources_servers.critpt.replayto rescore cached submissions later without rerunning inference, if AA key fails.Testing
python -m pytest resources_servers/critpt/tests/ -q