Skip to content

[SC-11248] Hotfix: Test results not being logged to API when no tables present#392

Merged
juanmleng merged 3 commits intoprodfrom
juan/sc-11248/hotfix-test-results-not-being-logged-to-api-when-no-tables-present
Jul 14, 2025
Merged

[SC-11248] Hotfix: Test results not being logged to API when no tables present#392
juanmleng merged 3 commits intoprodfrom
juan/sc-11248/hotfix-test-results-not-being-logged-to-api-when-no-tables-present

Conversation

@juanmleng
Copy link
Contributor

Pull Request Description

What and why?

  • Removed the condition to log test results only if they contain tables.
  • This caused test results without tables to be invisible in the ValidMind platform.

How to test

  • Run the quickstart_model_documentation.ipynb and check all test results are logged in the UI and database.
  • Create a custom test that returns just text and check results are logged in the UI and database.

What needs special review?

Dependencies, breaking changes, and deployment notes

Release notes

Checklist

  • What and why
  • Screenshots or videos (Frontend)
  • How to test
  • What needs special review
  • Dependencies, breaking changes, and deployment notes
  • Labels applied
  • PR linked to Shortcut
  • Unit tests added (Backend)
  • Tested locally
  • Documentation updated (if required)
  • Environment variable additions/changes documented (if required)

@juanmleng juanmleng self-assigned this Jul 14, 2025
@juanmleng juanmleng added bug Something isn't working internal Not to be externalized in the release notes labels Jul 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

@github-actions
Copy link
Contributor

PR Summary

This pull request introduces two main changes:

  1. In the utility function that retrieves judge configuration, the PR fixes a bug where the wrong object was being checked for type. The condition now correctly verifies if the model of the provided judge_embeddings is an instance of the Embeddings class instead of checking judge_llm's model. This change ensures that the configuration behaves as expected when embeddings are used.

  2. In the asynchronous logging functionality, the PR refactors the logging of test results. It introduces an additional logging task by appending an API call to log test results, and removes the redundant logging block that handled table logging. The update to the content_id formatting in the metadata update call improves clarity and consistency.

Test Suggestions

  • Create tests for get_judge_config to verify that when judge_embeddings is provided with a valid Embeddings model, it returns the expected output without error.
  • Verify that providing an invalid judge_embeddings (or a model not of type Embeddings) correctly raises a ValueError.
  • Write integration tests to ensure that the asynchronous logging (alog_test_result, alog_figure, update_metadata) is called with the correct parameters under various conditions (with figures, with tables omitted, and with metrics).
  • Test the new content_id formatting for the update_metadata function to confirm that it produces consistent and expected results.

@juanmleng juanmleng merged commit 673ffc1 into prod Jul 14, 2025
7 checks passed
@juanmleng juanmleng deleted the juan/sc-11248/hotfix-test-results-not-being-logged-to-api-when-no-tables-present branch July 14, 2025 20:22
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