last_interaction_index is computed as len(trace.interactions) - 1 unconditionally, rather than "did this step add any interactions". A step that adds none therefore claims the previous step's interaction.
This is what PR #2604 fixes; filing so the bug is tracked independently of the PR, and to record the sibling instance and the test gap.
Reproduced on main
3-step scenario where step 3 has checks only:
step0: interacts=1 last_interaction_index=0
step1: interacts=1 last_interaction_index=1
step2: interacts=0 last_interaction_index=1 <-- claims step1's interaction
The field's own docstring (core/result.py:418-421) promises None when the step added no interactions, so the implementation contradicts its documented contract.
Locations
libs/giskard-checks/src/giskard/checks/scenarios/runner.py:179-181 — main path
libs/giskard-checks/src/giskard/checks/scenarios/runner.py:200-202 — sibling instance for skipped steps, same shape. Reproduced: steps: [('fail', 0), ('skip', 0)] — the skipped step claims index 0, owned by the failed step.
Worth confirming PR #2604 covers both, not just the first.
Impact
The documented consumer is the Giskard Hub upload flow, so check results attach to the wrong interaction in the UI — a wrong-but-plausible attribution, which is harder to notice than an outright error.
Test gap
grep last_interaction_index libs/giskard-checks/tests/ returns nothing. Add coverage for zero-interaction steps and skipped steps as part of the fix.
Note on PR overlap
PR #2604 and PR #2718 touch an identical file set — scenarios/runner.py and tests/core/test_scenario.py — so they will conflict. Reconcile into one before merging.
last_interaction_indexis computed aslen(trace.interactions) - 1unconditionally, rather than "did this step add any interactions". A step that adds none therefore claims the previous step's interaction.This is what PR #2604 fixes; filing so the bug is tracked independently of the PR, and to record the sibling instance and the test gap.
Reproduced on main
3-step scenario where step 3 has checks only:
The field's own docstring (
core/result.py:418-421) promisesNonewhen the step added no interactions, so the implementation contradicts its documented contract.Locations
libs/giskard-checks/src/giskard/checks/scenarios/runner.py:179-181— main pathlibs/giskard-checks/src/giskard/checks/scenarios/runner.py:200-202— sibling instance for skipped steps, same shape. Reproduced:steps: [('fail', 0), ('skip', 0)]— the skipped step claims index 0, owned by the failed step.Worth confirming PR #2604 covers both, not just the first.
Impact
The documented consumer is the Giskard Hub upload flow, so check results attach to the wrong interaction in the UI — a wrong-but-plausible attribution, which is harder to notice than an outright error.
Test gap
grep last_interaction_index libs/giskard-checks/tests/returns nothing. Add coverage for zero-interaction steps and skipped steps as part of the fix.Note on PR overlap
PR #2604 and PR #2718 touch an identical file set —
scenarios/runner.pyandtests/core/test_scenario.py— so they will conflict. Reconcile into one before merging.