Skip to content

fix(tier3): keep skill lift consistent when rebuilding reports - #212

Merged
chrisknvidia merged 4 commits into
NVIDIA:mainfrom
YMuskrat:fix/judge-aware-lift
Oct 20, 2026
Merged

chrisknvidia merged 4 commits into
NVIDIA:mainfrom
YMuskrat:fix/judge-aware-lift

Conversation

@YMuskrat

@YMuskrat YMuskrat commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Rebuilding a saved Tier 3 report could turn −0.08 skill lift into +0.06 without running another evaluation. The collector excluded comparisons scored by different judge models, but report calculations included them again.

This PR preserves those exclusions across evaluator, dimension, overall, and dashboard lift. Judge-free cases keep valid comparisons; capped report loads use recorded lift when exclusion counts support it. Native multi-step results retain the judges of the scoring steps, using only the final step for FINAL. Unavailable CLI lift and baseline values render as n/a.

Verification

  • Familiar with the Contributing Guidelines
  • Added focused regression tests — 669 tests passed, 5 skipped across judge lift, collectors, reports, dashboard, and CLI
  • Updated documentation for user-visible changes
  • Ran make lint
  • Full make test pass — broader run stopped after workspace repository-context/provenance failures; the same failures reproduce on the unchanged original PR commit cc5da4c
  • Ran make build
  • Did not add credentials, private datasets, or proprietary benchmark content

Merged current upstream main through 6068b3a. GitHub workflows require maintainer approval.

Release Impact

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

@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.

Automated review: 4 findings (0 P1, 2 P2, 2 P3); 4 posted inline.

@rng1995

rng1995 commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

@YMuskrat thanks. I re-ran my reproductions at bcc68ef, and all four findings are fixed, so the threads are resolved. The rebuilt report's lift now matches the collector for judge-free cases, capped loads and final-strategy multi-step trials, and the CLI shows n/a instead of crashing.

#209 merged into main just now, and this branch now conflicts with it in evaluation/tier3_report.py, reporting/cli.py and CHANGELOG.md. Could you merge main and push? When you resolve it, please keep #209's rule: lift is unavailable (INCONCLUSIVE) for an agent that never activated the skill, and that should hold in the judge-aware lift path too. Also keep the CLI's n/a handling for missing lift values, since #209 adds new cases where lift is None.

A note on CI time: the four 513-case capped-load tests are slow (2–4 minutes each locally). They run only in the Ubuntu jobs, so they should not fail CI, but they will make those jobs noticeably longer.

@rng1995

rng1995 commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

@YMuskrat I have the merge with current main (including #209) ready for this branch, with conflicts resolved and tests passing, but I can't push it: "Allow edits by maintainers" is off for this PR. Could you tick that box in the PR sidebar? I'll push the merge straight away. If you'd rather keep it off, merge main yourself and I'll re-check.

@YMuskrat

Copy link
Copy Markdown
Contributor Author

Thanks for sorting out the #209 conflicts, @rng1995. I've turned on maintainer edits here too, so you're good to push the merge.

chrisknvidia added a commit to YMuskrat/SkillEvaluator that referenced this pull request Oct 20, 2026
Preserve activation eligibility alongside judge-aware lift, retain unavailable CLI comparisons, and keep both changelog entries. Add regression coverage for activated and never-activated saved reports.

Signed-off-by: Christopher Kevin <[email protected]>
YMuskrat and others added 4 commits October 20, 2026 09:17
Keep judge-free and capped report comparisons available, handle unavailable
CLI comparisons, and follow native FINAL judge provenance.

Signed-off-by: YMuskrat <[email protected]>
Keep the new dashboard's score differences aligned with canonical lift,
including negative and unavailable matching-judge comparisons.

Signed-off-by: YMuskrat <[email protected]>
Preserve activation eligibility alongside judge-aware lift, retain unavailable CLI comparisons, and keep both changelog entries. Add regression coverage for activated and never-activated saved reports.

Signed-off-by: Christopher Kevin <[email protected]>
@chrisknvidia
chrisknvidia merged commit d235a5e into NVIDIA:main Oct 20, 2026
17 checks passed
rng1995 added a commit that referenced this pull request Oct 22, 2026
Merge main 3728e2f (#215 custom execution backends, #212 judge-aware
lift, v0.6.0 release).

Conflicts:
- src/skillevaluator/tier3/harbor/runner.py: the unsupported env_mode
  failure keeps the PR's coded INVALID_CONFIGURATION result with main's
  _supported_env_modes() list (built-in plus installed custom backends).
  The runtime-plan failure keeps the PR's coded result, followed by
  main's new try/except around the agent_import_paths selection.
- CHANGELOG.md (auto-merged): git placed the PR's error-code entry in
  the released 0.6.0 section; moved it to Unreleased under ### Added.
  Main's 0.6.0 section is unchanged.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Narendran Raghavan <[email protected]>
rng1995 added a commit that referenced this pull request Oct 22, 2026
Brings in main up to 3728e2f: #212 judge-aware lift, 15adf98 (schema
"unavailable" rejection) and #215 custom execution backends.

Conflicts:
- docs/cli-reference.mdx: kept the branch's experimental hermes agent note
  and main's custom execution backends sentence on --env-mode.
- src/skillevaluator/cli.py: import MAX_TRIAL_RETRIES and
  PLUGIN_LOAD_CHOICES with main's all_env_modes; --env-mode lists
  installed backends.
- src/skillevaluator/tier3_environments.py: kept the branch's plugin-load
  and trial-retry constants and main's custom backend registry.
- src/skillevaluator/tier3/harbor/collector.py: main's strategy-aware
  aggregate matcher keeps the branch's N/A-metric filter; main's
  constituent judge models merge runs before the branch's
  artifact-aware failure and N/A merges.
- src/skillevaluator/tier3/harbor/runner.py: an explicit agent import path
  may be a native plugin wrapper or a custom backend's wrapper; agent
  import paths and the arm base paths are resolved inside main's backend
  error handling before native plugin wrappers are chosen. The arm base
  for a custom backend is the backend's own agent wrapper (else the stock
  agent), as build_harbor_run_command launches it, so a native wrapper
  over an unknown base falls back or refuses.
- src/skillevaluator/evaluation/tier3_report.py: both helper sets kept.
  Lift data uses main's type check and the branch's activation-aware
  intervals; dimensions take main's judge_lift and the branch's baseline
  N/A flag; evaluators keep lift None without a comparison (main's
  judge exclusion included). When judge-mixed cases exist, the
  effectiveness headline (or the legacy members comparison) and its basis
  come from the matching-judge comparison (#212), the case-weighted
  interval that pairs those cases is dropped, partial and never-activated
  comparisons still get no lift, and the lift note says when no matching-
  judge comparison remains. Main's _dimension_score calls use the
  branch's weighted_dimension_score.
- src/skillevaluator/reporting/cli.py: the branch's None-safe evaluator
  table renders a missing score, baseline or lift as main's "n/a".
- tests/reporting/test_reporters.py: both test sets kept.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Narendran Raghavan <[email protected]>
rng1995 added a commit that referenced this pull request Oct 22, 2026
Main's #212 renders an unavailable baseline or lift in the CLI evaluator
table as "n/a" (its test_render_agent_eval_with_unavailable_comparison
checks this). The branch's test for a run without a baseline still
expected "N/A" in that table; it now expects main's wording. The
Markdown table keeps "N/A".

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Narendran Raghavan <[email protected]>
@rng1995 rng1995 mentioned this pull request Oct 30, 2026
15 of 20 tasks
chrisknvidia pushed a commit that referenced this pull request Oct 30, 2026
Merge main 3728e2f (#215 custom execution backends, #212 judge-aware
lift, v0.6.0 release).

Conflicts:
- src/skillevaluator/tier3/harbor/runner.py: the unsupported env_mode
  failure keeps the PR's coded INVALID_CONFIGURATION result with main's
  _supported_env_modes() list (built-in plus installed custom backends).
  The runtime-plan failure keeps the PR's coded result, followed by
  main's new try/except around the agent_import_paths selection.
- CHANGELOG.md (auto-merged): git placed the PR's error-code entry in
  the released 0.6.0 section; moved it to Unreleased under ### Added.
  Main's 0.6.0 section is unchanged.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Narendran Raghavan <[email protected]>
chrisknvidia pushed a commit that referenced this pull request Oct 30, 2026
Brings in main up to 3728e2f: #212 judge-aware lift, 15adf98 (schema
"unavailable" rejection) and #215 custom execution backends.

Conflicts:
- docs/cli-reference.mdx: kept the branch's experimental hermes agent note
  and main's custom execution backends sentence on --env-mode.
- src/skillevaluator/cli.py: import MAX_TRIAL_RETRIES and
  PLUGIN_LOAD_CHOICES with main's all_env_modes; --env-mode lists
  installed backends.
- src/skillevaluator/tier3_environments.py: kept the branch's plugin-load
  and trial-retry constants and main's custom backend registry.
- src/skillevaluator/tier3/harbor/collector.py: main's strategy-aware
  aggregate matcher keeps the branch's N/A-metric filter; main's
  constituent judge models merge runs before the branch's
  artifact-aware failure and N/A merges.
- src/skillevaluator/tier3/harbor/runner.py: an explicit agent import path
  may be a native plugin wrapper or a custom backend's wrapper; agent
  import paths and the arm base paths are resolved inside main's backend
  error handling before native plugin wrappers are chosen. The arm base
  for a custom backend is the backend's own agent wrapper (else the stock
  agent), as build_harbor_run_command launches it, so a native wrapper
  over an unknown base falls back or refuses.
- src/skillevaluator/evaluation/tier3_report.py: both helper sets kept.
  Lift data uses main's type check and the branch's activation-aware
  intervals; dimensions take main's judge_lift and the branch's baseline
  N/A flag; evaluators keep lift None without a comparison (main's
  judge exclusion included). When judge-mixed cases exist, the
  effectiveness headline (or the legacy members comparison) and its basis
  come from the matching-judge comparison (#212), the case-weighted
  interval that pairs those cases is dropped, partial and never-activated
  comparisons still get no lift, and the lift note says when no matching-
  judge comparison remains. Main's _dimension_score calls use the
  branch's weighted_dimension_score.
- src/skillevaluator/reporting/cli.py: the branch's None-safe evaluator
  table renders a missing score, baseline or lift as main's "n/a".
- tests/reporting/test_reporters.py: both test sets kept.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Narendran Raghavan <[email protected]>
chrisknvidia pushed a commit that referenced this pull request Oct 30, 2026
Main's #212 renders an unavailable baseline or lift in the CLI evaluator
table as "n/a" (its test_render_agent_eval_with_unavailable_comparison
checks this). The branch's test for a run without a baseline still
expected "N/A" in that table; it now expects main's wording. The
Markdown table keeps "N/A".

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Narendran Raghavan <[email protected]>
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