Skip to content

John6797/sc 7670/move test result description prompts to backend#296

Merged
johnwalz97 merged 5 commits intomainfrom
john6797/sc-7670/move-test-result-description-prompts-to-backend
Jan 21, 2025
Merged

John6797/sc 7670/move test result description prompts to backend#296
johnwalz97 merged 5 commits intomainfrom
john6797/sc-7670/move-test-result-description-prompts-to-backend

Conversation

@johnwalz97
Copy link
Contributor

@johnwalz97 johnwalz97 commented Jan 17, 2025

Internal Notes for Reviewers

Moving test result description generation to the backend.

External Release Notes

@johnwalz97 johnwalz97 added enhancement New feature or request internal Not to be externalized in the release notes labels Jan 17, 2025
@juanmleng
Copy link
Contributor

Would this change still support passing external context via VALIDMIND_LLM_DESCRIPTIONS_CONTEXT?

@cachafla
Copy link
Contributor

Would this change still support passing external context via VALIDMIND_LLM_DESCRIPTIONS_CONTEXT?

The context is still being passed to the request, except that now the request is sent to the API and unpacked here: https://github.com/validmind/backend/pull/1140/files#diff-1f286300bd4ef710c7d4c921c3702e65de730379fc39f4b68c686cc4d92c1345R412

If you check the the generate_description() function you'll see that context is still being retrieved in the same way:

context = _get_llm_global_context()

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.

Awesome 👍

Copy link
Contributor

@AnilSorathiya AnilSorathiya left a comment

Choose a reason for hiding this comment

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

left one comment. Otherwise it is LGTM 👍

@github-actions
Copy link
Contributor

PR Summary

This pull request introduces significant refactoring to the validmind/ai/test_descriptions.py module, primarily focusing on simplifying the process of generating test result descriptions. The changes include:

  • Removal of the _load_prompt and prompt_to_message functions, which were responsible for loading and formatting prompt templates for generating descriptions.
  • Elimination of the context.py, config.yaml, image_processing.py, system.jinja, and user.jinja files, which were previously used for handling context and image processing related to test result descriptions.
  • Introduction of a new function generate_test_result_description in validmind/api_client.py, which centralizes the description generation process by making an API call to generate the test result description.
  • The generate_description function in test_descriptions.py now directly calls generate_test_result_description from api_client.py, simplifying the code and removing the need for local prompt handling.

These changes aim to streamline the codebase by removing redundant code and improving the maintainability of the test description generation process.

Test Suggestions

  • Test the generate_description function to ensure it correctly calls the API and returns the expected description.
  • Verify that the removal of the _load_prompt and prompt_to_message functions does not affect other parts of the codebase.
  • Check the API endpoint used in generate_test_result_description to ensure it handles various input scenarios correctly.
  • Ensure that the refactored code handles cases where no tables, figures, or metrics are provided, raising appropriate errors.

@johnwalz97 johnwalz97 merged commit eb99068 into main Jan 21, 2025
6 checks passed
@johnwalz97 johnwalz97 deleted the john6797/sc-7670/move-test-result-description-prompts-to-backend branch January 21, 2025 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request internal Not to be externalized in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants