Skip to content

feat(tier3): enforce structured JSON schemas and rate-limit retries in LLM judges - #145

Merged
rng1995 merged 19 commits into
NVIDIA:mainfrom
kweinmeister:fix/llm-judge-rate-limit-retry
Oct 2, 2026
Merged

rng1995 merged 19 commits into
NVIDIA:mainfrom
kweinmeister:fix/llm-judge-rate-limit-retry

Conversation

@kweinmeister

Copy link
Copy Markdown
Contributor

Summary

When running Tier 3 evaluations with higher concurrency or across multi-case datasets, LLM judge grading (accuracy, goal_accuracy, and behavior_check) can fail in two ways:

  1. Judge models with internal reasoning tokens or verbose output can emit preamble text or exhaust their output token budget before closing the JSON object, causing _parse_json_response to fail even with max_tokens=4096.
  2. Parallel Harbor trial containers hitting the same judge endpoint at the end of a wave can trigger HTTP 429 rate limits or transient 502/503/504 gateway errors.

This change updates both the host judge path (src/skillevaluator/inference/client.py, src/skillevaluator/tier3/eval_core/llm_judge.py) and the container verifier (src/skillevaluator/tier3/harbor/templates/eval.py) to:

  • Pass explicit response_schema and schema_name definitions (ACCURACY_JUDGE_SCHEMA, GOAL_ACCURACY_JUDGE_SCHEMA, BEHAVIOR_CHECK_JUDGE_SCHEMA) to the LLM client (response_format for OpenAI-compatible endpoints and output_config for Anthropic endpoints).
  • Automatically downgrade once without the schema constraint (response_format={"type": "json_object"} for OpenAI-compatible, or prompt-based JSON for Anthropic) if a provider returns HTTP 400 or 422, logging a warning and memoizing the target so subsequent judge calls do not repeat failed schema requests.
  • Retry HTTP 429 and transient 5xx/connection errors using exponential backoff with full jitter and Retry-After header parsing, configurable via SKILL_EVAL_LLM_MAX_RETRIES, SKILL_EVAL_LLM_RETRY_BASE_DELAY, and SKILL_EVAL_LLM_RETRY_MAX_DELAY.

Verification

  • I am familiar with the Contributing Guidelines
  • Added or updated focused tests
  • Updated documentation for user-visible changes
  • Ran make lint
  • Ran make test
  • Ran make build
  • Did not add credentials, private datasets, or proprietary benchmark content

Release Impact

  • No user-visible release note needed
  • Updated CHANGELOG.md

… for LLM judges

Implement zero-dependency exponential backoff with full jitter, header-aware Retry-After parsing, and defensive environment variable overrides for LLM judge calls across both the Harbor container verifier (eval.py) and the host runtime client (LLMClient).

- Add skillevaluator.inference.retry with Full Jitter backoff, RFC-7231 HTTP date parsing, UTC timezone normalization, and safe defaults.
- Embed self-contained retry loop in Harbor verifier template (eval.py) with socket cleanup and fail-fast behavior on non-retriable errors.
- Forward retry configuration (SKILL_EVAL_LLM_MAX_RETRIES, SKILL_EVAL_LLM_RETRY_BASE_DELAY, SKILL_EVAL_LLM_RETRY_MAX_DELAY and aliases) into container task.toml via adapter allowlist.
- Wrap LLMClient.completions with retry while strictly preserving agent execution logs, token counts, and trial telemetry.
- Add comprehensive unit test coverage for retry logic, header parsing, container template execution, and task configuration forwarding.

Signed-off-by: Karl Weinmeister <kweinmeister@google.com>
Signed-off-by: Karl Weinmeister <kweinmeister@google.com>
…22 downgrade

Signed-off-by: Karl Weinmeister <kweinmeister@google.com>
…ate schema fallback

Signed-off-by: Karl Weinmeister <kweinmeister@google.com>
Signed-off-by: Karl Weinmeister <kweinmeister@google.com>

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kweinmeister Two issues remain in the new retry/schema behavior: SDK retries multiply the configured budget, and unrelated HTTP 400/422 responses permanently disable structured output for the target. Details and reproductions are inline.

Local validation: 879 focused tests passed. Separate reproductions using the real OpenAI/Anthropic SDKs with mock HTTP transports exposed these gaps. GitHub reports all 17 checks passing. Please address the two cases and add transport-level regressions before approval.

Comment thread src/skillevaluator/inference/client.py Outdated
Comment thread src/skillevaluator/inference/client.py Outdated
… schema errors

Disable SDK-level retries in OpenAI and Anthropic clients by passing max_retries=0 to ensure the outer backoff loop owns the retry budget. Add http_client parameter to LLMClient to support transport-level testing. Restrict schema error detection to match genuine unsupported-option errors and defer target memoization until downgrade succeeds. Mirror schema capability checks and deferred memoization in the Harbor container verifier template. Add transport-level request count regressions and unrelated 400 schema persistence tests.

Signed-off-by: Karl Weinmeister <kweinmeister@google.com>
@kweinmeister

Copy link
Copy Markdown
Contributor Author

@rng1995 Thanks for the review and the clear reproductions. Both issues have been resolved in commit 7280545:

  1. Outer loop owns retry budget: Passed max_retries=0 to both OpenAI and Anthropic constructors and exposed http_client on LLMClient. Verified via mock HTTP transports that max_retries=0 sends 1 request and max_retries=3 sends 4 requests for both providers.
  2. Selective schema fallback & deferred memoization: Restricted schema failure detection to genuine unsupported-option error payloads in both host and container template runtimes, and deferred _SCHEMA_UNSUPPORTED_TARGETS memoization until the prompt-only fallback confirms success. Verified that two context_length_exceeded errors followed by a valid prompt maintain [True, True, True] schema flags.

All 6,519 tests and make lint / package build pass cleanly. Ready for another look!

kweinmeister and others added 4 commits September 24, 2026 09:01
Signed-off-by: Karl Weinmeister <kweinmeister@google.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>
Signed-off-by: Christopher Kevin <christopherk@nvidia.com>

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kweinmeister Thanks for addressing both earlier findings. I verified that SDK retries are disabled and that schema fallback only caches a genuine unsupported-option response after a successful downgrade. Both existing threads can remain resolved.

One integration issue remains: the normal Harbor runner drops the three retry environment variables before staging and launching the verifier, so the documented settings are ignored there. Details and the reproduction are inline. Please forward those controls through the runner and add an integration regression before approval.

Validation on 248d129: 764 focused tests passed locally, plus independent SDK/schema and environment-forwarding probes. GitHub reports all 17 checks passing; the PR has no merge conflicts.

Comment thread src/skillevaluator/tier3/harbor/adapter.py
rng1995 and others added 3 commits September 26, 2026 11:14
@kweinmeister

Copy link
Copy Markdown
Contributor Author

@rng1995 Thanks for catching that. I pushed an update to forward the three retry environment variables through the Harbor runner into both task staging and the Harbor subprocess environment, added an integration regression test covering the full path from os.environ through verifier config resolution, and verified it with an end-to-end Tier 3 smoke test. Ready for another look when you have a chance.

Comment thread src/skillevaluator/tier3/harbor/templates/eval.py

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kweinmeister Thank you for your contribution and the updates. The original SDK retry and schema-fallback findings remain fixed, and host forwarding now works. Two existing blockers remain: native-task overrides can produce invalid TOML, and Bedrock ignores the configured judge retry policy. I confirmed both independently and followed up in the existing threads.

Local validation: 839 focused tests passed, one deselected; Ruff and diff checks passed. GitHub reports all 17 CI checks passing and no merge conflicts. Approval is pending the two remaining fixes.

rng1995 and others added 2 commits September 29, 2026 11:50
…k retries

- Replace colliding [verifier.env] keys in-place during native task staging and strip inline TOML comments
- Enforce bounded judge retry policy and disable internal botocore retries for Bedrock Converse
- Add configurable SKILL_EVAL_LLM_JUDGE_BUDGET_SEC wall-time deadline with 180s fallback
- Refactor loose tuples to SchemaTargetKey and EvalRetryConfig NamedTuples
- Consolidate shared test fixtures in conftest.py, parameterize retry tests, and sanitize dummy credentials

Signed-off-by: Karl Weinmeister <kweinmeister@google.com>
@kweinmeister

Copy link
Copy Markdown
Contributor Author

Pushed update in commit c7efb5a addressing the latest review feedback and code standards:

  • Native Harbor TOML collisions: Implemented in-place replacement for colliding keys in [verifier.env] during native staging and normalized table header comment parsing (adapter.py).
  • Bedrock Converse retries: Disabled internal botocore retries (max_attempts: 0) and applied the bounded judge retry loop and deadline enforcement to Converse calls (eval.py).
  • Configurable judge wall-time budget: Added SKILL_EVAL_LLM_JUDGE_BUDGET_SEC (default 180s fallback) forwarded through Harbor and documented in docs/environment-variables.mdx.
  • Data clumps refactored: Consolidated loose tuples into SchemaTargetKey and EvalRetryConfig NamedTuples, and unified shared test fixtures in conftest.py.
  • Verification: All 7,117 tests, ruff check, ruff format, and ty check pass cleanly.

Signed-off-by: Karl Weinmeister <kweinmeister@google.com>
Signed-off-by: Karl Weinmeister <kweinmeister@google.com>

# Conflicts:
#	CHANGELOG.md
#	src/skillevaluator/tier3/harbor/adapter.py
#	src/skillevaluator/tier3/harbor/runner.py

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kweinmeister Thank you for your patience through several review rounds, and for your contribution to SkillEvaluator!

I re-reviewed c992b1f. Both remaining blockers are fixed:

  • Native-task [verifier.env] collisions: colliding retry settings are now replaced by key, so staged task.toml files parse and resolve to the host value for all three retry settings in default and default_plus_custom. Authored values are kept when no host override is set.
  • Bedrock judge retries: botocore retries are disabled, and Converse now follows the configured judge retry count and delay bounds. Against a loopback Converse endpoint, MAX_RETRIES=0 with AWS_MAX_ATTEMPTS=4 sends one request, and MAX_RETRIES=3 with AWS_MAX_ATTEMPTS=1 succeeds on the third request. The new transport-level tests cover both cases.

Validation: Ruff passes, as do 1,076 focused tests and the full local suite (9,696 passed, 18 skipped). All 17 CI checks are green.

A non-blocking follow-up that also exists on main, so not something this PR needs to fix: if an authored task spells the verifier env table another way (env.KEY = ... under [verifier], env = { ... }, or [ verifier.env ]), staging writes a duplicate [verifier.env] table. The staged file then fails to parse, and nothing reports the error. Parsing the staged task.toml after editing would surface it clearly.

Approved.

@rng1995
rng1995 merged commit 266f597 into NVIDIA:main Oct 2, 2026
17 checks passed
rng1995 added a commit to kweinmeister/SkillEvaluator that referenced this pull request Oct 2, 2026
Resolve conflicts with NVIDIA#145 (LLM judge retries and structured schemas):
- inference/client.py: keep the ADC token refresh on 401 inside the
  OpenAI-compatible branch of _invoke_provider, now wrapping
  _call_with_schema_fallback, under retry_call_with_backoff.
- templates/eval.py: keep the Vertex OpenAPI helpers next to main's schema
  builders, and apply the in-process ADC refresh on 401 around
  _urlopen_with_schema_fallback, rebuilding the request with the new token.
- tests/test_tier3_public_runtime.py and CHANGELOG.md: keep both sides.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
rng1995 added a commit to kweinmeister/SkillEvaluator that referenced this pull request Oct 2, 2026
Resolve conflicts with NVIDIA#145 (LLM judge retries):
- runner.py: import both _VERIFIER_RETRY_ENV_VARS and _native_entry_id.
- tests/test_tier3_public_runtime.py: keep both sides' new tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
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.

3 participants