Skip to content

Hotfix: Update content_id logic in metadata update function#371

Merged
AnilSorathiya merged 4 commits intomainfrom
fix-logged-none-content-ids
May 15, 2025
Merged

Hotfix: Update content_id logic in metadata update function#371
AnilSorathiya merged 4 commits intomainfrom
fix-logged-none-content-ids

Conversation

@panchicore
Copy link
Member

@panchicore panchicore commented May 13, 2025

Modified the content_id handling to include a fallback when it is not provided. This ensures proper fallback to a test description with result_id and revision_name, preventing potential issues with missing IDs.

Pull Request Description

What

The test description was not displayed on validation report evidences.
https://validmind.slack.com/archives/C04U7TKHH62/p1746749735016559

Why

A condition was missing for content_id while logging test result description.

How to Test

  1. Run a validation notebook
  2. Link evidences for the risk assessment and check the test description of the evidences

Pull Request Dependencies

External Release Notes

Deployment Notes

Hotfix for validmind-library

Breaking Changes

Screenshots/Videos (Frontend Only)

Checklist

  • PR body describes what, why, and how to test
  • Release notes written
  • Deployment notes written
  • Breaking changes identified
  • Labels applied
  • PR linked to Shortcut
  • Screenshots/videos added (Frontend)
  • Unit tests added (Backend)
  • Tested locally
  • Documentation updated (if required)

Areas Needing Special Review

Additional Notes

Modified the content_id handling to include a fallback when it is not provided. This ensures proper fallback to a test description with result_id and revision_name, preventing potential issues with missing IDs.
@panchicore panchicore requested a review from AnilSorathiya May 13, 2025 20:32
@AnilSorathiya AnilSorathiya added bug Something isn't working internal Not to be externalized in the release notes labels May 14, 2025
@AnilSorathiya AnilSorathiya requested review from cachafla and validbeck and removed request for AnilSorathiya May 14, 2025 10:43
@AnilSorathiya AnilSorathiya changed the title Update content_id logic in metadata update function Hotfix: Update content_id logic in metadata update function May 14, 2025
Copy link
Contributor

@cachafla cachafla left a comment

Choose a reason for hiding this comment

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

Looks good, I suggest testing scenarios where result_id is non-empty and empty.

What was the version of this change before it broke?

@AnilSorathiya
Copy link
Contributor

AnilSorathiya commented May 14, 2025

Looks good, I suggest testing scenarios where result_id is non-empty and empty.

Will add tests for these.

What was the version of this change before it broke?

2.8.21

@github-actions
Copy link
Contributor

PR Summary

This pull request introduces a comprehensive suite of unit tests for the TestResult and related classes in the validmind project. The tests cover various aspects of the TestResult, ErrorResult, TextGenerationResult, and ResultTable classes, including initialization, method functionality, and serialization. Additionally, the PR refines the log_async method in the result.py file to improve the handling of logging tasks, particularly by separating the conditions for logging tables, figures, and descriptions. This change ensures that each component is logged only if it is present, and it corrects the handling of content_id in metadata updates.

Key changes include:

  • Addition of unit tests for RawData, ResultTable, ErrorResult, TestResult, and TextGenerationResult classes.
  • Mocking of asynchronous API calls to test logging functionality without actual network requests.
  • Refinement of the log_async method to separate the logging of tables, figures, and descriptions, and correct the content_id handling in metadata updates.

Test Suggestions

  • Run all newly added unit tests to ensure they pass successfully.
  • Test the log_async method with various combinations of tables, figures, and descriptions to verify correct task creation.
  • Verify that the content_id is correctly formatted and used in metadata updates.
  • Check the behavior of the validate_log_config method with both valid and invalid configurations.
  • Ensure that the mocked API calls in the tests are correctly simulating the expected responses.

@AnilSorathiya AnilSorathiya merged commit e353682 into main May 15, 2025
7 checks passed
@AnilSorathiya AnilSorathiya deleted the fix-logged-none-content-ids branch May 15, 2025 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working internal Not to be externalized in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants