Repository navigation
fix(tier3): honor per-step tokens when detecting runtime failures - #199
Conversation
Honor recorded per-step token usage when optional aggregate ATIF metrics are absent or zero. Signed-off-by: YMuskrat <Y.Laaouach@sussex.ac.uk>
Signed-off-by: YMuskrat <Y.Laaouach@sussex.ac.uk>
Signed-off-by: YMuskrat <Y.Laaouach@sussex.ac.uk>
chrisknvidia
left a comment
There was a problem hiding this comment.
Reviewed ce62cba1b449294cca1c9df091eda8334514d439. No actionable new code defect found in per-step token detection, explicit runtime-error precedence, or downstream score/report handling. There were no earlier review threads to address.
Not ready to merge yet:
- GitHub reports
CONFLICTING/DIRTYagainst current main098f96a0d169878ab85b6801a8f4609b29d11e50. A read-only merge-tree check isolates the conflict to CHANGELOG.md. Update the branch and resolve it. - Current-head CI, DCO, and Security runs are all
action_required; they have not executed. Authorize the updated-head workflows and require passing checks after conflict resolution.
Verification: 395 local tests passed with Harbor 0.24.0, covering runtime failures, multi-step trajectories, negative-control evidence, unified reports and HTML reporting; changed-file Ruff passed. These use constructed inputs and real-library/report integration. No live agent/provider E2E or full-suite pass is claimed; the Docker server probe timed out.
|
@YMuskrat, please update this branch with the latest |
rng1995
left a comment
There was a problem hiding this comment.
Reviewed the change and its tests; no findings. The new tests fail on main and pass here, and the related test modules pass. The branch needs main merged in to clear a CHANGELOG.md conflict before it can merge.
Resolve the CHANGELOG.md conflict by keeping both Fixed entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Resolve the CHANGELOG.md conflict with NVIDIA#188 and NVIDIA#193 by keeping both sides' Fixed entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Resolve the CHANGELOG.md conflict with NVIDIA#199 by keeping both sides' Fixed entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Resolve the CHANGELOG.md conflict with #199 by keeping both sides' Fixed entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Resolve the CHANGELOG.md conflict with NVIDIA#199 by keeping both sides' Fixed entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Resolve the CHANGELOG.md conflict with NVIDIA#199 by keeping both sides' Fixed entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Brings in #184, #187 (SkillSpector version in the HTML report), #188 (GPT-6.1 Sol / Claude Opus 5.5 defaults, reasoning effort, structured output, token sizing, refusal fallback), #191 (multiline catalog descriptions), #193 (Docker agent bootstrap) and #199 (per-step token runtime-failure detection). Conflicts: - .gitleaks.toml: keep every allowlist from both sides (the PR's three history allowlists and main's Bedrock model ID allowlist). - CHANGELOG.md: every entry from both sides under Unreleased, one Added, Changed, Fixed and Security heading each; the shared docs entry once. - deduplication/intra_skill/intra_skill_validator.py: the client keeps the PR's LLM_VERIFY_MAX_TOKENS and gains main's medium reasoning effort. - deduplication/intra_skill/llm_analyzer.py: the PR's bounded prompt and verdict parsing, with main's structured-output schema request. - embedding/registry.py: keep the PR's _embed_texts refactor (main only reformatted the old loop); main's multiline-description check merged. - inference/__init__.py: export both sides' names. - reporting/html.py: the PR's run-status tables plus main's SkillSpector version helper; is_tier2_validator_name stays in run_status.py. - tier3/eval_core/llm_judge.py: drop _is_native_openai_chat_url (removed by main) and _configured_secret_values (moved by the PR). - tier3/harbor/collector.py: keep the PR's shared-metric pass lift and main's judge-model helpers; _compute_lift takes main's per-trial rewards and the PR's keyword-only N/A lists, and its overall uses the paired scores with the PR's N/A-aware basket check; lift.json keeps the PR's _paired_lift. - tier3/harbor/local_agents.py: the PR's SkillEvaluatorCodex (subagent rollouts, setup network errors) now extends main's bootstrap-aware installed_agents.SkillEvaluatorCodex; NVIDIA Build Claude Code keeps SetupNetworkErrorAgent on main's SkillEvaluatorClaudeCode. - tier3/harbor/runner.py: imports from both sides; Docker uses main's DOCKER_AGENT_IMPORT_PATHS, other Harbor backends the PR's HARBOR_AGENT_IMPORT_PATHS; agent kwargs from main's _harbor_agent_kwargs. - tier3/harbor/templates/eval.py: the PR's skill-metric N/A helpers and allow_not_applicable judges, with main's 16,384-token judge ceiling and judge_model_used / reasoning effort / fallback provenance. - utils/tool_runner.py: main's pinned SkillSpector v2.12.0 hint plus the PR's scanner hints. - validators/security.py: imports from both sides. - tests/inference/test_llm_client.py: keep both sides' tests. - tests/test_harbor_local_mode.py: main's parametrized Docker route check plus the PR's Codex subclass assertion. - tests/tier3/test_judge_parse_robustness.py: the PR's partial-salvage assertions plus main's new provenance and refusal tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Brings in NVIDIA#188 (GPT-6.1 Sol / Claude Opus 5.5 defaults, reasoning effort, refusal handling, per-model token sizing), NVIDIA#193 (Docker agent Node/npm bootstrap) and NVIDIA#199 (per-step token runtime-failure evidence). Conflicts: - src/skillevaluator/inference/client.py: keep main's per-provider _openai_kwargs builder, reasoning effort and _extract_choice_content refusal/truncation checks; wrap that call in the PR's one-shot ADC token refresh on 401 for Vertex AI OpenAPI endpoints. - src/skillevaluator/tier3/harbor/runner.py: import both sides' provider_config names; docker and gke share the PR's branch, with docker defaulting to main's DOCKER_AGENT_IMPORT_PATHS and gke to the PR's CONTAINER_AGENT_IMPORT_PATHS, and the GKE secure environment kept. - src/skillevaluator/tier3/harbor/templates/eval.py: take main's _call_openai_compatible refactor (refusal fallback, empty-reply fallback, judge-call recording) and move the PR's Vertex ADC 401 refresh into _call_openai_compatible. - tests/test_provider_agent_routing.py: union of both sides' imports. - tests/validators/test_security.py: keep the PR's Vertex ADC SkillSpector tests and main's SkillSpector model/effort tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Summary
A generated answer such as
401currently invalidates an otherwise completed, rewarded trial when optional final ATIF token totals are absent, despite positive per-step token usage. Treat step usage as evidence that the trajectory is not a tokenless startup failure. Explicit Harbor exceptions and structured provider error events still invalidate trials.Closes #198
The focused collector module passes 129 tests, including 11 added regressions. Full suite: 11,204 passed, 53 skipped, 25 baseline failures.
Verification
make lint(passed)make testmake build(wheel and source distribution passed)The full suite used the repository's locked dependencies on Python 3.12 and pytest-xdist with four workers. It has 25 failures, all reproduced against unchanged main at f32c884. The identical failing test IDs occur on all three independent fixes; none is in a changed module. These include restricted Unix sockets/local subprocesses and existing SDK/CLI/report contracts. The full suite is not claimed green.
Release Impact