Skip to content

Cachafla/hotfix descriptions#323

Merged
cachafla merged 2 commits intorelease-2.7from
cachafla/hotfix-descriptions
Feb 24, 2025
Merged

Cachafla/hotfix descriptions#323
cachafla merged 2 commits intorelease-2.7from
cachafla/hotfix-descriptions

Conversation

@cachafla
Copy link
Contributor

Internal Notes for Reviewers

External Release Notes

@github-actions
Copy link
Contributor

PR Summary

This pull request introduces an enhancement to the _truncate_summary function in the validmind/ai/test_descriptions.py file. The function now includes a check to handle NoneType for the summary parameter. Previously, the function assumed summary was always a string, which could lead to errors if None was passed. The updated function now checks if summary is None before attempting to evaluate its length, ensuring robust handling of NoneType inputs. This change improves the function's reliability and prevents potential runtime errors when None is passed as an argument.

Test Suggestions

  • Test _truncate_summary with a None value for summary to ensure it returns None without errors.
  • Test _truncate_summary with a string shorter than max_tokens to verify it returns the string unchanged.
  • Test _truncate_summary with a string longer than max_tokens to ensure it truncates correctly.
  • Test _truncate_summary with edge cases, such as an empty string and a string exactly equal to max_tokens.

Copy link
Contributor

@johnwalz97 johnwalz97 left a comment

Choose a reason for hiding this comment

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

checked that branch looks good

@cachafla cachafla added the internal Not to be externalized in the release notes label Feb 24, 2025
@cachafla cachafla merged commit d42c79d into release-2.7 Feb 24, 2025
3 of 4 checks passed
@cachafla cachafla deleted the cachafla/hotfix-descriptions branch February 24, 2025 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Not to be externalized in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants