Skip to content

feat(checks): add contains_any and contains_all checks#2597

Open
TheManishCode wants to merge 2 commits into
Giskard-AI:mainfrom
TheManishCode:feat/contains-any-all-checks
Open

feat(checks): add contains_any and contains_all checks#2597
TheManishCode wants to merge 2 commits into
Giskard-AI:mainfrom
TheManishCode:feat/contains-any-all-checks

Conversation

@TheManishCode

Copy link
Copy Markdown

Summary

  • Adds ContainsAny / ContainsAll built-in checks for validating that a response contains at least one, or all, of a list of expected strings (e.g. topic coverage), so users no longer need custom FnCheck lambdas for this.
  • Follows the existing StringMatching/RegexMatching pattern in text_matching.py: JSONPath extraction (text_key/values_key), Unicode normalization, and configurable case sensitivity.
  • Refactors the shared string-formatting helper (_format_str) out of StringMatching into a module-level function so it can be reused without changing RegexMatching's API.

Test plan

  • New unit tests in tests/builtin/test_contains_matching.py covering pass/fail cases, case sensitivity, JSONPath extraction, Unicode normalization, empty/missing/invalid values, and constructor validation errors.
  • ruff format --check / ruff check clean.
  • basedpyright --level error libs/giskard-checks — 0 errors.
  • pytest libs/giskard-checks -m "not functional" — 751 passed (pre-existing RegoPolicy/celine-regorus failures unrelated, no Windows wheel for that dependency).

Closes #2361

Add ContainsAny/ContainsAll built-in checks for validating that a
response contains at least one or all of a list of expected strings,
so users no longer need custom FnCheck lambdas for this common case.

Closes Giskard-AI#2361

@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 two new text-matching checks, ContainsAny and ContainsAll, along with comprehensive unit tests and a refactored _format_str helper function. The review feedback highlights an optimization opportunity in both checks, pointing out that the current implementation for computing matched and missing values has $O(N^2)$ complexity and performs redundant string formatting. It is recommended to apply the suggested single-pass $O(N)$ loop to partition the values more efficiently.

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.

Comment on lines +668 to +675
formatted_text = _format_str(text, self.normalization_form, self.case_sensitive)
matched = [
v
for v in values
if _format_str(v, self.normalization_form, self.case_sensitive)
in formatted_text
]
missing = [v for v in values if v not in matched]

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.

medium

The current implementation of ContainsAny has an $O(N^2)$ complexity for computing missing because it performs a linear lookup (v not in matched) for each element in values. Additionally, it calls _format_str twice per matching element. We can optimize this to a single $O(N)$ pass by iterating through values once and partitioning them into matched and missing lists directly.

Suggested change
formatted_text = _format_str(text, self.normalization_form, self.case_sensitive)
matched = [
v
for v in values
if _format_str(v, self.normalization_form, self.case_sensitive)
in formatted_text
]
missing = [v for v in values if v not in matched]
formatted_text = _format_str(text, self.normalization_form, self.case_sensitive)
matched = []
missing = []
for v in values:
if _format_str(v, self.normalization_form, self.case_sensitive) in formatted_text:
matched.append(v)
else:
missing.append(v)

Comment on lines +740 to +747
formatted_text = _format_str(text, self.normalization_form, self.case_sensitive)
matched = [
v
for v in values
if _format_str(v, self.normalization_form, self.case_sensitive)
in formatted_text
]
missing = [v for v in values if v not in matched]

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.

medium

The current implementation of ContainsAll has an $O(N^2)$ complexity for computing missing because it performs a linear lookup (v not in matched) for each element in values. Additionally, it calls _format_str twice per matching element. We can optimize this to a single $O(N)$ pass by iterating through values once and partitioning them into matched and missing lists directly.

Suggested change
formatted_text = _format_str(text, self.normalization_form, self.case_sensitive)
matched = [
v
for v in values
if _format_str(v, self.normalization_form, self.case_sensitive)
in formatted_text
]
missing = [v for v in values if v not in matched]
formatted_text = _format_str(text, self.normalization_form, self.case_sensitive)
matched = []
missing = []
for v in values:
if _format_str(v, self.normalization_form, self.case_sensitive) in formatted_text:
matched.append(v)
else:
missing.append(v)

Addresses Gemini Code Assist review feedback: computing matched/missing
via list membership was O(N^2) and reformatted each value twice.
@TheManishCode

Copy link
Copy Markdown
Author

Hi maintainers 👋 — this PR is ready for review but blocked on two org-level gates that only a maintainer can clear:

  1. authorize check (Integration Tests workflow) fails because I'm an external contributor — it needs the safe for build label.
  2. Main CI and GitHub Actions Security Analysis workflows are stuck in action_required — GitHub's "approve workflow runs from first-time contributors" gate.

In the meantime I've addressed the Gemini Code Assist review feedback (single-pass O(N) partitioning in ContainsAny/ContainsAll instead of O(N²)) and verified locally: ruff format/ruff check clean, basedpyright --level error 0 errors, vermin 3.12-compat clean, pip-audit no known vulnerabilities, and pytest libs/giskard-checks -m "not functional" 751 passed (same pre-existing unrelated celine-regorus/Windows-wheel failures noted in the PR description).

Could someone approve the workflow run and/or add the safe for build label so CI can actually execute? Thanks!

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.

Add contains_any / contains_all check

1 participant