Skip to content

fix(gooddata-eval): let --gate decide a what-if item, not pass@K alone - #1831

Open
Tomkess wants to merge 1 commit into
masterfrom
fix/gate-what-if
Open

Tomkess wants to merge 1 commit into
masterfrom
fix/gate-what-if

Conversation

@Tomkess

@Tomkess Tomkess commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

The bug

run_agentic_what_if computes pass_power_k. The evaluator never read it:

if not summary.pass_at_k:      # pass^K computed, then ignored

So --gate power on a flaky item passes on one good run out of K — the exact case the gate exists to catch, and the report says the opposite of what happened.

This is live: what-if merged this morning in #1799.

The fix

What-if now does what the other eight multi-run kinds already do:

  • takes gate: EvalGate = DEFAULT_GATE, and the dispatch passes it
  • asks gate_passed(...) for the verdict
  • carries gate_failure_note(...) into the assertion message — this matters, because the message body describes the best run, which under pass^K can be a run that passed, so without the note a failing item reads like a passing one
  • stamps the gate on the run metadata and publishes pass@K / pass^K / gate_passed, so a gated run is readable in Langfuse and not only in the exit code

Default behaviour is unchanged. Without --gate this is pass@K exactly as before, pinned by its own test.

Verification

Three tests: the power gate failing a 1-of-2 item, any still passing the same item, and the default matching any. Reverting the one-line verdict change makes the power test fail, so it is testing the fix rather than the scaffolding.

1209 tests pass, ruff check and ruff format --check clean.

Related

Found by CodeRabbit on #1798, where the same gap is fixed for forecasting. agentic_anomaly_detection has it too — fixed on #1801, since that evaluator is not on master yet. After those three, the only ungated kind is agentic_conversation, which is ungated by design.

🤖 Generated with Claude Code

`run_agentic_what_if` already computed `pass_power_k`; the evaluator never read
it, asking `if not summary.pass_at_k` for the verdict. `--gate power` on a flaky
item therefore reported a pass on the strength of one good run out of K -- the
exact case the gate exists to catch, reporting the opposite of what happened.

The evaluator now takes `gate` like every other multi-run kind, asks
`gate_passed(...)`, and carries the gate's note into the assertion message, which
matters here because the message body describes the BEST run and under pass^K that
can be one that passed. It also stamps the gate on the run metadata and publishes
pass@K/pass^K/gate_passed, so a gated run is readable in Langfuse rather than only
in the exit code.

The default is unchanged: without --gate this is pass@K exactly as before, pinned
by a test alongside the power case. The power test fails against the previous
code.

Found by CodeRabbit on #1798, where the same gap was fixed for forecasting.
`agentic_anomaly_detection` has it too and is fixed on its own branch, #1801,
because that evaluator does not exist on master yet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 3 billable files and costs up to $0.75.

Or wait 33 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 95733749-672f-4bfc-996a-4e4aa392e6ff

📥 Commits

Reviewing files that changed from the base of the PR and between 9be051b and 313505a.

📒 Files selected for processing (3)
  • packages/gooddata-eval/src/gooddata_eval/cli/agentic_runner.py
  • packages/gooddata-eval/src/gooddata_eval/core/agentic/what_if.py
  • packages/gooddata-eval/tests/test_agentic_what_if.py

Comment @coderabbitai help to get the list of available commands.

Tomkess added a commit that referenced this pull request Sep 24, 2026
`run_agentic_anomaly_detection` already computed `pass_power_k`; the evaluator
never read it, asking `if not summary.pass_at_k` for the verdict, so `--gate
power` on a flaky item reported a pass on the strength of one good run out of K.

Wired the same way as the other multi-run kinds: the evaluator takes `gate`, the
dispatch passes it, `gate_passed(...)` decides, the gate's note goes into the
assertion message -- which matters because the body describes the BEST run, and
under pass^K that can be one that passed -- and pass@K/pass^K/gate_passed reach
Langfuse alongside the run metadata stamp.

The default is unchanged: without --gate this is pass@K exactly as before, pinned
by a test beside the power case. The power test fails against the previous code.

Found by CodeRabbit on #1798. Forecasting is fixed there; what-if, which is
already on master, in #1831.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.08%. Comparing base (9be051b) to head (313505a).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1831   +/-   ##
=======================================
  Coverage   83.08%   83.08%           
=======================================
  Files         330      330           
  Lines       21763    21767    +4     
=======================================
+ Hits        18082    18086    +4     
  Misses       3681     3681           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Tomkess added a commit that referenced this pull request Sep 24, 2026
`run_agentic_anomaly_detection` already computed `pass_power_k`; the evaluator
never read it, asking `if not summary.pass_at_k` for the verdict, so `--gate
power` on a flaky item reported a pass on the strength of one good run out of K.

Wired the same way as the other multi-run kinds: the evaluator takes `gate`, the
dispatch passes it, `gate_passed(...)` decides, the gate's note goes into the
assertion message -- which matters because the body describes the BEST run, and
under pass^K that can be one that passed -- and pass@K/pass^K/gate_passed reach
Langfuse alongside the run metadata stamp.

The default is unchanged: without --gate this is pass@K exactly as before, pinned
by a test beside the power case. The power test fails against the previous code.

Found by CodeRabbit on #1798. Forecasting is fixed there; what-if, which is
already on master, in #1831.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit 5632da8)
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.

1 participant