fix: add variant-groups.ndjson to skip check for models

[dbasunag]

Pull Request

Summary

Related Issues

• Fixes:
• JIRA:

How it has been tested

• Locally
• Jenkins

Additional Requirements

• If this PR introduces a new test image, did you create a PR to mirror it in disconnected environment?
• If this PR introduces new marker(s)/adds a new component, was relevant ticket created to update relevant Jenkins job?

Summary by CodeRabbit

• Tests
  • Updated validation test logic to broaden file-based exclusions (now excludes two specific provider files), which changes which providers are processed during test runs to improve coverage consistency.

[github-actions]

The following are automatically added/executed:

• PR size label.
• Run pre-commit
• Run tox
• Add PR author as the PR assignee
• Build image based on the PR

Available user actions:

• To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
• To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
• To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
  lgtm label removed on each new commit push.
• To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
  verified label removed on each new commit push.
• To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
  and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
• To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
  pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.

Supported labels

{'/hold', '/verified', '/build-push-pr-image', '/wip', '/lgtm', '/cherry-pick'}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: be031189-bc0a-4869-b923-d4fb5f20f334

📥 Commits

Reviewing files that changed from the base of the PR and between 3735f72 and 52f5ea3.

📒 Files selected for processing (1)

• tests/model_registry/model_catalog/search/utils.py

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/model_registry/model_catalog/search/utils.py

---

📝 Walkthrough

Walkthrough

Broadened the skip condition in a test helper so validate_performance_data_files_on_pod now excludes variant-groups.ndjson in addition to manifest.json when deciding which provider files to skip during validation.

Changes

Cohort / File(s)	Summary
Test Validation Skip Condition tests/model_registry/model_catalog/search/utils.py	Expanded the exclusion check in validate_performance_data_files_on_pod to skip both manifest.json and variant-groups.ndjson files when iterating provider files for validation. Minor line changes only.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is a blank template with no substantive content filled in. Summary, related issues, testing status, and additional requirements are all empty.	Fill in the Summary section explaining why variant-groups.ndjson needs to be skipped. Check the testing boxes or explain test coverage. Reference any related GitHub issues or JIRA tickets.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the primary change: adding variant-groups.ndjson to the skip check for models in the test utilities.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/model_registry/model_catalog/search/utils.py (1)

350-353: ⚠️ Potential issue | 🟠 Major

Bug: in-place list mutation affects subsequent model iterations.

required_files.remove() mutates the list shared across all models within the same provider. If Mistral-Small-24B-Instruct-2501 is processed first, evaluations.ndjson is removed from required_files and subsequent models in the same provider won't be validated for that file.

🐛 Proposed fix: copy the list per model iteration

         for model in models.splitlines():
             if model == "provider.json":
                 continue
+            # Copy to avoid mutation affecting subsequent models
+            model_required_files = required_files.copy()
             # Remove data for specific RH models based on
             # https://redhat-internal.slack.com/archives/C09570S9VV0/p1762164394782969?thread_ts=1761834621.645019&cid=C09570S9VV0
             if model == "Mistral-Small-24B-Instruct-2501":
-                required_files.remove("evaluations.ndjson")
+                model_required_files.remove("evaluations.ndjson")
             elif model == "granite-3.1-8b-instruct-quantized.w8a8":
-                required_files.remove("performance.ndjson")
+                model_required_files.remove("performance.ndjson")

             result = model_catalog_pod.execute(
                 container=CATALOG_CONTAINER, command=["ls", f"{PERFORMANCE_DATA_DIR}/{provider}/{model}"]
             )

             # Check which required files are missing
-            missing_files = [f for f in required_files if f not in result]
+            missing_files = [f for f in model_required_files if f not in result]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/model_registry/model_catalog/search/utils.py` around lines 350 - 353,
The current code mutates the shared required_files list (via
required_files.remove(...)) when handling specific models, causing later models
in the same provider to see a modified list; instead, create a per-model copy of
required_files at the start of the model-processing loop (e.g., model_required =
list(required_files) or required_files.copy()) and perform the conditional
removals against that copy (the branches referencing model ==
"Mistral-Small-24B-Instruct-2501" and model ==
"granite-3.1-8b-instruct-quantized.w8a8" should update model_required rather
than required_files) so each model is validated independently.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@tests/model_registry/model_catalog/search/utils.py`:
- Around line 350-353: The current code mutates the shared required_files list
(via required_files.remove(...)) when handling specific models, causing later
models in the same provider to see a modified list; instead, create a per-model
copy of required_files at the start of the model-processing loop (e.g.,
model_required = list(required_files) or required_files.copy()) and perform the
conditional removals against that copy (the branches referencing model ==
"Mistral-Small-24B-Instruct-2501" and model ==
"granite-3.1-8b-instruct-quantized.w8a8" should update model_required rather
than required_files) so each model is validated independently.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 11079fc1-9044-4b96-8f45-d5f4049d9bd8

📥 Commits

Reviewing files that changed from the base of the PR and between 292742e and 3735f72.

📒 Files selected for processing (1)

• tests/model_registry/model_catalog/search/utils.py

[fege]

/lgtm

[github-actions]

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.