Load testing - #45
Merged
Merged
Conversation
jakepresent
added a commit
that referenced
this pull request
May 12, 2026
The previous commits isolate target-side LLMInputError. Auditor-side LLMInputError (where the auditor's adversarial prompt itself trips the provider's content filter / Prompt Shields jailbreak detector) was still re-raised by _run_auditor_target_loop's auditor catch block, which aborted the whole rollout stage on the first occurrence. This was hit live on the May 12 PR #44 mix-1k validation run: at scenario 853 of 1000, the auditor's adversarial prompt for one seed was refused by Azure Prompt Shields with the same Bad request / 'flagged as potentially violating our usage policy' signature. Aaron flagged this auditor-side refusal pattern in the May 12 standup and worked around it in PR #45 with a tame auditor system prompt for benchmarking; the right fix in product code is the same per-seed isolation as the target-side case. Fix: catch LLMInputError separately from the other classified errors in _run_auditor_target_loop's auditor block. Record an '[AUDITOR INPUT REFUSED: ...]' system event in the transcript, set stop_reason='auditor_input_refused', and end the conversation cleanly so the worker moves on to the next seed. Auth/rate-limit/5xx errors still propagate (they're global pipeline problems, not seed-specific). Tests: - test_run_rollout_isolates_auditor_input_refusal_to_one_seed: 5-seed scenario batch where one seed's auditor call raises LLMInputError; all 5 transcripts are written, the refused seed carries stop_reason='auditor_input_refused', the batch returns successfully. All 51 rollout + exception-handling tests pass.
jakepresent
added a commit
that referenced
this pull request
May 12, 2026
The previous commits isolate target-side LLMInputError and auditor-side LLMInputError in rollout. The judge stage had the same fail-fast catch for LLMInputError: a single content-filter rejection on a transcript the judge LLM can't process would kill the whole stage and discard the hundreds of seeds already scored. This was hit live on the May 12 PR #44 mix-1k validation rerun (after the auditor-side fix from f265154 unblocked rollout): the judge stage scored 539 of 1000 transcripts at the lowered concurrency, then died when one transcript's adversarial content tripped Azure's content filter on the judge call itself. Same Bad request / 'flagged as potentially violating our usage policy' signature, just on the judge side. This is the third logically-independent place the same adversarial-eval-as-attacker dynamic can fire. Fix: catch LLMInputError separately in the judge worker. Record a synthesized score row with judge_status='filter_skipped' and judge_error='judge_input_refused: ...' so the seed isn't lost and infer_judge_status downstream correctly counts it as a non-ok seed. Auth/rate-limit/provider-5xx errors still propagate (global pipeline problems, not seed-specific). The filter_skipped status flows cleanly through the existing infer_judge_status() contract in p2m/core/judge.py: it's a non-'ok' status, so it's treated as judge_failed by downstream consumers (results.py, viewer_read_model.py). This matches Aaron's PR #45 _install_judge_skip_blocked shim, but lands the behavior in product code instead of as a benchmark monkey-patch.
…cooldowns Survives Azure rate-limit storms during high-concurrency benchmark runs that previously killed the entire judge / rollout stage on the first LLMRateLimitError that exhausted retries. Per-row resilience (judge & rollout stages): * Workers no longer re-raise LLMInputError / LLMRateLimitError / LLMProviderError; failed rows are skipped and counted in the new errored_count summary field. LLMAuthError still re-raises (fail fast, save tokens on a misconfigured key). * Rollout no longer synthesizes RuntimeError from target_error transcripts; counted in target_error_count instead. The transcript is kept on disk for inspection. * Stages only fail when every row failed AND no prior cache exists. Failed rows are not written to scores.jsonl / transcripts.jsonl, so re-running the suite picks them up via the existing resume logic. Retry-After visibility: * report_rate_limit tags every cooldown log line with its source (Retry-After / escalation / active) so users can tell when Azure explicitly told us to back off vs when we escalated on our own. * _with_retries now logs 'honoring server Retry-After=Xs' when the server supplied a delay, distinct from 'waiting for coordinated cooldown' fallback. Per-task retry budget: * _with_retries tracks own_429s and own_5xx independently of total iterations. Waiting through another task's coordinated cooldown no longer consumes this task's retry budget. A safety_iterations_cap of (_MAX_RETRIES + 1) * 4 = 24 guarantees termination if other tasks keep refreshing the cooldown indefinitely. Sticky escalation with slow decay: * The previous report_success popped the escalated base on the very first successful call after a 429 storm. With ~19 sibling tasks waking from a coordinated cooldown, the first success would revert the base to the default and the next 429 would re-escalate from scratch — the base never climbed past 2x default, producing the oscillation pattern observed at c=20. * report_success now tracks consecutive successes per model and only halves the base after _DECAY_AFTER_SUCCESSES (10) clean calls in a row, floored at _DEFAULT_COOLDOWN_S. Any 429 resets the counter. The base now climbs to a sustainable rate (4 -> 8 -> 16 -> ...) and decays only after the deployment has clearly recovered. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extends the per-row resilience contract from judge & rollout to seeds,
which was the last stage able to kill the pipeline on a single bad LLM
response. Triggered by a 1000-seed benchmark where a single batch came
back with a malformed payload (missing 'seeds' list) at seeds.py:665,
raising ValueError out of gather_limited and cancelling all sibling
batches mid-flight — same all-or-nothing failure mode that judge and
rollout previously had.
Per-batch resilience:
* _process now catches LLMInputError / LLMRateLimitError /
LLMProviderError and ValueError / JSONDecodeError per batch and
returns a structured error sentinel instead of raising. LLMAuthError
still re-raises (fail fast — never transient, save tokens on a
misconfigured key).
* _generate_records aggregates errors after gather_limited; only
re-raises when *every* batch failed, signalling a systemic problem
(auth, schema, config). Otherwise logs a warning summary and returns
partial records plus an errored_count.
* _generate_records return type changed from list[dict] to
dict{records, errored_count}; this is a private helper so no public
API change.
* run_seeds aggregates errored_count across kinds and surfaces it in
the returned summary alongside saved_count / prompt_count /
scenario_count, so the runner / metrics / benchmark CSV can show
partial-success runs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…acheability
Two changes that together unlock provider-side prefix caching for the
high-volume judge and seeds stages without changing what the model sees
semantically.
## Visibility (model_client)
UsageStats now carries cached_input_tokens and cache_creation_input_tokens.
_normalize_usage extracts them from both major shapes:
- OpenAI/Azure: prompt_tokens_details.cached_tokens (Chat Completions)
and input_tokens_details.cached_tokens (Responses API)
- Anthropic: top-level cache_read_input_tokens / cache_creation_input_tokens
summarize_response and the per-call DEBUG log line surface the cached
count when nonzero, so judge / seeds runs at concurrency now show whether
the provider is actually serving the static prefix from cache.
## Seed prompt reordering
Both seeds_scenario_single.md and seeds_direct_single.md previously
interleaved per-batch dynamic content (focus behavior, definition,
behavior-specific examples) with the static instruction body, breaking
the cache prefix at roughly line 30 of an 8K-token prompt.
Templates are now reordered so the entire static instruction surface
(policy_body, concept, target context, tool_instructions, role, task,
quality criteria, process, boundaries, output format) appears at the
top in run-stable order, and a dedicated # This Batch section at the
end carries everything that varies per call (focus behavior, definition,
behavior-specific examples, and {{batch_guidance}}).
For Azure GPT-5/GPT-4o auto-cache (≥1024-token threshold, ~5–10 min TTL,
50%+ discount on cached input tokens) this grows the cacheable prefix
from ~1–1.5K tokens to ~5–6K tokens of a typical ~7–8K total. Same
applies for any provider that does prefix caching; no provider-specific
changes were added.
The judge stage already sends a fully static system message followed by
the variable transcript, so it should benefit immediately with no
template change.
## Tests
Seven new tests in test_model_client.py cover:
- OpenAI Chat Completions cached_tokens extraction
- OpenAI Responses API input_tokens_details.cached_tokens extraction
- Anthropic top-level cache_read / cache_creation extraction
- Top-level field taking precedence when both shapes are present
- No-cache-metadata case leaving fields None
- summarize_response surfacing cache when nonzero
- summarize_response omitting cache when zero (no payload bloat)
Full suite: 628 passed, 14 skipped. The same 4 pre-existing Windows-only
flakes (chmod 0o000 / TempDir cleanup races in test_exception_handling
and test_logging_config) are unaffected.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a process-wide UsageAccumulator + track_usage() context manager in
model_client.py so every generate*() call records its UsageStats into the
currently-active accumulator without each stage having to thread usage
through its return values.
The runner wraps each stage in track_usage() and prints a compact suffix
on the existing stage completion line, plus a run-totals line and a new
metrics.json artifact in the run directory:
[seeds] ✓ Generated 100 test cases (41.3s) | 15 calls · 75K in / 5K out · 40.0% cached
[rollout] ✓ Completed 100 rollouts (380.1s) | 600 calls · 957.9K in / 44.7K out · 14.4% cached
[judge] ✓ Scored 100 transcripts (112.0s) | 100 calls · 800K in / 12K out · 75.0% cached
Token usage: 715 calls · 1.8M in / 61.7K out · 36.8% cached
Pipeline completed (533.4s)
Metrics: artifacts/results/<suite>/<run>/metrics.json
metrics.json carries per-stage and per-model breakdowns so post-hoc tools
(or results.csv columns later) can inspect cache effectiveness without
parsing transcripts.jsonl.
Tests: 19 new (UsageAccumulator + track_usage + runner formatting +
metrics aggregation), all 35 in scope green; full suite 643 passed
(same 4 pre-existing Windows-only flakes).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When sample_size is large enough that each covering-array tuple's budget exceeds DEFAULT_GENERATION_MAX_TOKENS (3000), the LLM response truncates mid-JSON and the entire batch is silently dropped. Observed at s=1000: 65/67 batches failed, only 30/1000 seeds survived. Each scenario seed carries a system_prompt + opening_message + factor metadata (~300-500 output tokens). 15 of them in a single response blows the 3000-token budget. Splitting per-tuple budgets into chunks of MAX_SEEDS_PER_BATCH (=5) keeps every call comfortably inside the budget and lifts effective concurrency (more, smaller jobs run in parallel). Diagnostic counts after fix: s=100 -> 67 jobs (unchanged) s=200 -> 67 jobs (unchanged) s=500 -> 134 jobs (was 67) s=1000 -> 201 jobs (was 67) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Lifts the four product-code changes from PR #44 into our branch and removes the corresponding benchmark monkey-patches that were doing the same job out-of-tree. Changes: * CallableSession.run_turn now reclassifies user-callable provider errors via `_classify_llm_error` so litellm BadRequestError / ContentPolicyViolationError emerge as typed `LLMInputError` (with `__cause__` preserved) instead of bypassing the typed-exception layer the rollout/judge stages key off. Unclassified exceptions propagate untouched. (PR #44 commit 0184d8d) * `_run_prompt_seed` records a `[TARGET INPUT REFUSED: ...]` event in the transcript and sets `stop_reason='target_input_refused'` when the target's run_turn raises `LLMInputError`. Other classified errors still propagate. (PR #44 commit 82cf339) * `_run_auditor_target_loop` splits the combined classified-error catch into a typed `LLMInputError` branch that records `[TARGET INPUT REFUSED: ...]` / `[AUDITOR INPUT REFUSED: ...]` events and sets `stop_reason='target_input_refused'` / `'auditor_input_refused'` respectively. Auth / rate-limit / 5xx errors still propagate. (PR #44 commits 82cf339 + f265154) * judge `_worker` splits the combined catch so `LLMInputError` becomes a synthesized `score_row` with `judge_status='filter_skipped'` and `judge_error='judge_input_refused: ...'`. Rate-limit / provider 5xx still go to the per-row error path. Auth still fails fast. (PR #44 commit dcaa91f) * New `P2M_ROLLOUT_ERROR_FAIL_RATIO` env var (default 0.10) caps the rollout stage's tolerance for *untyped* worker errors. Below the threshold: warn and continue (existing soft-fail). Above: raise the first error. Typed refusals (target_input_refused, auditor_input_refused, target_error) are NOT counted toward the ratio — they're recorded data, not real failures. Inspired by PR #44 commit 15332c8 but scoped to untyped errors only instead of #44's blanket runtime_error catch-all (which would have hidden real bugs behind the same threshold). * scripts/benchmark.py: drops `_install_content_filter_tolerance`, `_install_judge_skip_blocked`, and `_is_content_policy_violation` helpers (~210 lines). The product-code typed handlers above replace them. The benchmark now scans `transcripts.jsonl` and `scores.jsonl` after the run completes and counts typed stop_reasons / judge_status values. CSV columns updated to typed names: `target_input_refused_count`, `auditor_input_refused_count`, `target_error_count`, `judge_filter_skipped_count` (replaces the legacy `content_filter_blocked` / `target_error_tolerated` aggregates). `--no-tolerate-content-filter` flag preserved as a no-op for back-compat. Tests: * test_run_turn_reclassifies_litellm_bad_request_as_input_error * test_run_turn_passes_through_unclassified_exceptions * test_run_prompt_seed_records_target_input_refusal * test_run_rollout_isolates_target_input_refusal_to_one_seed * test_run_rollout_isolates_auditor_input_refusal_to_one_seed * test_run_rollout_still_fails_fast_on_provider_5xx * test_run_rollout_fails_when_untyped_error_ratio_exceeds_threshold (new — covers the threshold introduced in this commit) * test_run_judge_isolates_input_refusal_to_one_seed * test_run_rollout_keeps_partial_successful_transcripts_when_later_worker_fails (updated — sets P2M_ROLLOUT_ERROR_FAIL_RATIO=0.6 since the original 2-seed/1-failure setup now exceeds the production 10% default) 654 passed, 14 skipped, 13 subtests, 2 deselected (pre-existing Windows tempdir flakes in test_logging_config.py from main's PR #22). 0 new test failures. viewer check: 0 errors / 0 warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow-up to 7e8cfef from a code-review pass: * scripts/benchmark.py: delete `_REFUSAL_COUNTS_DEFAULT`. It was added as a "kept for back-compat" placeholder, but nothing references it (not even external scripts — the predecessor counters `_CONTENT_FILTER_BLOCKS` / `_TARGET_ERROR_TOLERATED` were also internal). `_scan_run_artifacts` builds its own counts dict each call, so the constant is purely vestigial. * p2m/stages/rollout.py: drop the `auditor_messages.append(<target_error>...)` line in the typed `target_input_refused` catch in `_run_auditor_target_loop`. It was copy-pasted from the `target_error` catch below it (which DOES use `auditor_messages` later), but in the typed-refusal branch we immediately `break` out of the turn loop AND the caller at `_run_scenario_seed:915` discards the return tuple's auditor_messages slot (`stop_reason, _, _ = ...`). So the append never has any effect. No behaviour change. Tests still pass: 92/94 in rollout_stage + exception_handling + measurement_fixes (2 failing are pre-existing Windows flakes unrelated to this work). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
Review pass found two correctness issues I think we should fix before relying on this for load-test results.
Relevant refs:
Suggested fix: either make non-zero seed
There’s a second schema mismatch too: the existing result-summary helpers compute scenario metrics from Relevant refs:
Suggested fix: have Related smaller issue: rollout/judge return |
Conflict in p2m/stages/seeds.py: combined main's per-job `seeds_response_schema(min_items=count, max_items=count)` (PR #40) with our per-batch try/except resilience wrapper. Each batch now (a) gets a schema that pins both bounds to the requested count, and (b) is wrapped in a per-batch error sentinel so a single bad payload no longer kills the stage. Tests: 666 passed; 4 pre-existing Windows flakes unrelated to the merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ark CSV outcome columns Addresses Jake's review on the PR #45 absorb. Three correctness/observability bugs: 1. Cache + partial seeds: _generate_records returns errored_count > 0 without raising. Before this fix, the runner finalized the cacheable artifact anyway, writing artifact.json next to a partial seeds.jsonl - a future cache hit silently reused the smaller-than-requested file. Now the runner skips finalize_artifact_plan when _summary.errored_count > 0; partial output stays in the version dir for inspection but has no sidecar, so _latest_matching_metadata skips it on the next run (next call allocates v(N+1)). 2. Benchmark CSV outcome columns wired to wrong metrics.json schema. The runner now writes metrics.json as token-usage-only telemetry, so the scenario_metrics / policy_violation / overrefusal fields benchmark.py was reading were always blank. _load_metrics_summary now reads scores.jsonl via load_run_summary (canonical reader) and seeds.jsonl via count_seed_kinds; metrics.json is no longer consulted for outcome data. 3. errored_count was dropped from rollout/judge stage wrappers (matching the seeds wrapper bug). Now surfaced in _summary for all three stages so the runner gate in (1) can see it. Note: existing partial-seed cache entries from prior runs still have valid sidecars and will keep matching their input hashes. Run with --no-cache once to force regeneration if that matters. Tests: - test_runner_artifact_cache.py: new test_partial_seeds_skips_artifact_finalization asserts seeds.jsonl exists in v0001 but artifact.json does not, and a second run allocates v0002. - test_benchmark_summary.py (new): 3 tests for _load_metrics_summary covering happy path (reads scores.jsonl), missing run dir, and "does not read metrics.json even when present with the old schema." - 670 passed, 14 skipped, 13 subtests, 4 pre-existing Windows tempdir flakes unrelated to this work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jakepresent
approved these changes
May 14, 2026
jakepresent
left a comment
jakepresent
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the quick fixes. The two correctness concerns I raised look addressed to me:
- Partial artifacts no longer get cache sidecars when
_summary.errored_count > 0, so a partial seed output should not be reused as a complete cached artifact. scripts/benchmark.pynow reads outcome columns from the seed/score artifacts instead of the token-usage-onlymetrics.json.
Focused tests looked good locally (tests/test_benchmark_summary.py plus the partial-seeds cache regression test), and CI is green.
Two small non-blocking follow-ups:
- Clean up the
ΓÇömojibake in a few comments/tests. - Confirm the desired behavior for partial-seeds runs that exit successfully but skip cache finalization: since the suite-root compatibility
seeds.jsonlmay not refresh,scenario_seeds_generatedin the benchmark summary could be blank or stale unless the summary reads the active artifact/run manifest instead. I don't think this blocks this PR, but it is worth deciding explicitly.
Four em-dashes I typed into PowerShell during the last commit (4dec901) got mangled: my host console interpreted the UTF-8 em-dash bytes (E2 80 94) as cp437, displayed them as the three glyphs Gamma / C-cedilla / o-diaeresis, then those three glyphs were re-encoded as UTF-8 and written to disk. The result on disk was the literal byte sequence CE 93 C3 87 C3 B6 (Greek Gamma + C-cedilla + o-diaeresis) inside otherwise ASCII comments. Replaced all four occurrences with plain ASCII '--' to keep the comments robust to console encoding regardless of where future edits originate. Files affected: - p2m/stages/judge.py:389 - p2m/stages/rollout.py:1293 - tests/test_runner_artifact_cache.py:502, :518 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sonl Follow-up to Jake's review on the partial-seeds-cache gate. The previous benchmark summary read scenario_seeds_generated from the suite-root compatibility file <suite>/seeds.jsonl, which is written by finalize_artifact_plan -> refresh_compatibility_files. But the new runner gate intentionally skips finalize_artifact_plan when the seeds stage reports errored_count > 0 (to keep partial output cache-invisible). That meant a partial-seeds-but-exit-0 run would either show the previous successful run's stale count OR a blank cell (first-ever run), even though we have an exact per-run count sitting in scores.jsonl. Switch the source to load_run_summary's scenario_metrics.total. That field counts every scenario row in this run's scores.jsonl, which is exactly "how many seeds reached scoring stage" -- the load-test-scale number the matrix is asking for. As a side effect: * CSV row is now internally consistent: every column describes THIS run. * No dependency on a file written by a side-effect of a step we now intentionally skip. * count_seed_kinds import dropped from this scanner. Caveat: column name is still scenario_seeds_generated but the new semantics is closer to "scenarios attempted at scoring time". For no-rollout-error normal runs these are identical. If we later want to distinguish "generated" vs "attempted", add a separate seeds_errored_count column sourced from manifest.errored_count (now plumbed through stage _summary by the previous commit). Tests: * test_reads_scenarios_scored_and_dimension_rates_from_scores_jsonl unchanged behavior (4 scenarios still expected) but now hits via scenario_metrics.total instead of count_seed_kinds. * test_ignores_stale_suite_root_seeds_jsonl (new): plants a deliberately-misleading suite-root seeds.jsonl with 99 records and asserts the scanner reports the count from THIS run's 2 score rows, regression-locking the staleness fix. 671 passed, 14 skipped, 4 pre-existing Windows tempdir flakes unrelated to this work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jakepresent
approved these changes
May 15, 2026
jakepresent
left a comment
jakepresent
left a comment
Collaborator
There was a problem hiding this comment.
Re-approving after the two follow-up commits. The mojibake cleanup is comment-only, and the benchmark summary change is the right direction: for partial-seeds runs where cache finalization is skipped, sourcing the count from this run's scores avoids stale suite-root seeds.jsonl. Targeted benchmark/artifact-cache tests passed locally.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.