fix(checks): last_interaction_index wrongly attributes results when a step adds zero new interactions 🤖🤖🤖🤖

[chuenchen309]

Problem

ScenarioRunner._run_once computed each step's last_interaction_index as len(trace.interactions) - 1 if trace.interactions else None — i.e. None only when the whole cumulative trace is empty, not when this step's interacts added zero new interactions. A step whose interaction spec yields nothing (e.g. a conditional interaction generator that sometimes produces no interaction) after an earlier step already populated the trace gets wrongly attributed to the prior step's last interaction instead of None, contradicting the field's own documented contract ("None when the step added no interactions").

TestCaseResult.last_interaction_index's docstring explicitly documents consumers (e.g. the Giskard Hub upload flow) using this field to attribute check results to a specific interaction — a wrong index here means a check result gets attributed to the wrong interaction in that downstream flow.

Fix

Track interactions_before = len(trace.interactions) before running the step's interacts, then compare against the count after — only report an index when interactions actually grew for this step.

Testing

• Added test_last_interaction_index_is_none_when_step_adds_no_new_interactions, using an interaction spec that generates zero interactions after an earlier step already added one, asserting the second step correctly reports None (not the first step's index).
• Confirmed red→green: reverting only runner.py reproduces assert 0 is None (wrongly attributed to the prior step); reapplying passes.
• Full libs/giskard-checks/tests/ suite: 739 passed, 4 skipped (unrelated).
• ruff check/ruff format --check and basedpyright — 0 errors (pre-existing unrelated warnings elsewhere in the file untouched).
• pre-commit run --files — all hooks pass.

Note on overlap: #2599 ("fix(checks): record input generation errors") also touches this function and adds a new except-block that reproduces the same unfixed len(trace.interactions) - 1 if trace.interactions else None pattern in a different code path (input-generation-failure handling). This PR doesn't touch that new block, so it's not a duplicate, but merging both may need a small rebase depending on order — happy to reconcile once I know which lands first.

AI-Generated disclosure

Found via an AI-assisted code review pass (Claude Code) over giskard-checks/src/giskard/checks/scenarios/. I personally traced the per-step vs whole-trace distinction against the field's documented contract, reproduced the wrong attribution, verified the fix, and ran the full test suite plus lint/type checks before submitting.

[gemini-code-assist]

Code Review

This pull request fixes an issue in the scenario runner where last_interaction_index could incorrectly point to a prior step's interaction if the current step added no new interactions. The fix compares the interaction count before and after the step is executed, setting the index to None if no new interactions were added. A corresponding unit test has been added to verify this behavior. There are no review comments, and I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[kevinmessiaen]

Thanks for your contribution. While the fix is correct, it would break uploading to the Hub the results using the Hub SDK

I'll take a look more in depth to fix the Hub before merging this fix

[chuenchen309]

Thanks for looking into it, and for flagging the Hub SDK coupling — that's exactly the kind of downstream impact I couldn't see from outside the repo. Happy to defer to your timeline on the Hub fix. If it would help to land this without waiting, I'm also glad to adjust the PR to preserve the old last_interaction_index value as a fallback (or gate the new behavior behind a flag) so the Hub SDK keeps working during the transition — just let me know which shape you'd prefer.

[chuenchen309]

Thanks @kevinmessiaen — that makes sense, and I appreciate you checking the Hub SDK side before merging. Happy to reshape the fix if it'd make the Hub-side change cleaner on your end (e.g. keeping the old attribution reachable behind a flag), just let me know what fits best. No rush from my side.

[chuenchen309]

Thanks for checking the downstream side — that's exactly the impact I had no way to see from here.

One fact that might narrow the scope of the Hub work, since it changes the framing from "this PR introduces None" to "None is already reachable":

last_interaction_index=None already happens on main today, without this PR. A scenario that adds no interactions at all takes the same else None branch:

# on upstream/main (175670e37), no patch applied
零-interaction scenario -> step0.last_interaction_index = None

That's the shape test_scenario_with_only_checks (tests/core/test_scenario.py:559) already exercises, and TestCaseResult.last_interaction_index is declared int | None with default=None and documents "None when the step added no interactions (e.g. skipped)".

So if the Hub upload can't handle None, that gap exists in production right now for any checks-only scenario — this PR doesn't create it, it just makes None reachable in a second case (a step whose interacts yields nothing mid-scenario). Whatever None-handling the Hub needs is arguably worth having regardless of this PR.

No rush from my side at all — happy for this to sit until the Hub is ready, and happy to close it if you'd rather fix both sides together in one go. Just flagging the above in case it makes the Hub change smaller than expected.