Skip to content

feat(checks): add Trace.steps() helper for step-based grouping#2481

Open
harsh21234i wants to merge 6 commits into
Giskard-AI:mainfrom
harsh21234i:feat/trace-steps-helper
Open

feat(checks): add Trace.steps() helper for step-based grouping#2481
harsh21234i wants to merge 6 commits into
Giskard-AI:mainfrom
harsh21234i:feat/trace-steps-helper

Conversation

@harsh21234i

Copy link
Copy Markdown
Contributor

Closes #2419

Summary

  • add Trace.steps() to group interactions by logical scenario step
  • stamp step_index metadata onto interactions at the scenario runner boundary
  • preserve backward compatibility by treating interactions without step_index as step 0
  • add tests for empty traces, single-step traces, multi-step traces, and real runner integration

Design

Trace itself does not know Scenario.steps, so the grouping signal is recorded at runtime where step boundaries are
actually known: ScenarioRunner.

This keeps the architecture aligned with the existing design:

  • Scenario owns step structure
  • ScenarioRunner materializes interactions
  • Trace remains an immutable history object with helper accessors over recorded data

Testing

  • uv run -m pytest -q libs/giskard-checks/tests/trace/test_trace.py
  • uv run ruff check libs/giskard-checks/src/giskard/checks/core/interaction/trace.py libs/giskard-checks/src/giskard/ checks/scenarios/runner.py libs/giskard-checks/tests/trace/test_trace.py

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces logical step grouping for interactions within a trace. It adds a steps() method to the Trace class to group interactions by a step_index metadata field and updates the ScenarioRunner to automatically tag interactions with these indices during scenario execution. Comprehensive tests were added to verify both the grouping logic and the tagging process. Review feedback suggests simplifying the type signature of the new tagging helper by removing unused generic parameters and optimizing list operations to avoid redundant allocations.

Comment thread libs/giskard-checks/src/giskard/checks/scenarios/runner.py Outdated
Comment thread libs/giskard-checks/src/giskard/checks/scenarios/runner.py Outdated

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces logical step grouping for interactions within a trace by adding a steps() method to the Trace class and updating the ScenarioRunner to tag interactions with a step_index. Review feedback suggests using a more idiomatic type check for the step_index to strictly ensure it is an integer and raises concerns regarding the O(S^2) performance complexity of the tagging implementation due to repeated list copying in long scenarios.

Comment thread libs/giskard-checks/src/giskard/checks/core/interaction/trace.py Outdated
Comment thread libs/giskard-checks/src/giskard/checks/scenarios/runner.py Outdated
@kevinmessiaen

Copy link
Copy Markdown
Member

Thanks for your contribution! I will take a look afterward.

Quick note: I think it could be nice to have a last_step property (similar to last interaction)

@harsh21234i

Copy link
Copy Markdown
Contributor Author

I addressed the remaining comments in two parts:

  • Trace.steps() now uses a strict type(step_index) is not int check
  • step tagging no longer rewrites the whole trace after each step

Instead of post-processing trace.interactions step-by-step, ScenarioRunner now wraps each step interaction spec and stamps step_index on each
yielded Interaction before it is added to the trace. That keeps the trace immutable while avoiding the repeated full-trace copy pattern the earlier
implementation had.

I also added coverage for multi-yield interactions within a single step to make sure all generated interactions keep the same step index.

@harsh21234i

Copy link
Copy Markdown
Contributor Author

hey @kevinmessiaen can you look into this??

@kevinmessiaen kevinmessiaen self-requested a review June 5, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

giskard-checks: add Trace.steps() helper for step-based interaction grouping

2 participants