Skip to content

First start at cross validation#4766

Merged
jadudm merged 1 commit into
jr/source-of-truth/mainfrom
jr/source-of-truth/cross-validations
Mar 11, 2025
Merged

First start at cross validation#4766
jadudm merged 1 commit into
jr/source-of-truth/mainfrom
jr/source-of-truth/cross-validations

Conversation

@jrothacker
Copy link
Copy Markdown
Contributor

Purpose

Add in the same logic for validations as in the SAC for the new SOT

How

Translated the Audit object into the same shape as the sac validation object. Searched for all usage of the shape and added logic for validating the Audit.

Testing Completed

  1. Linting
  2. Unit Tests
  3. End to End Tests.

@jrothacker jrothacker marked this pull request as ready for review March 11, 2025 00:17
Copy link
Copy Markdown
Contributor

@jperson1 jperson1 left a comment

Choose a reason for hiding this comment

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

Fantastic! Just a very minor suggestion, but everything is working as expected on my end.

Comment thread backend/audit/cross_validation/audit_validation_shape.py
Comment on lines +69 to +71
logger.error(
f"<SOT ERROR> Cross Validation Errors do not match: SAC {sac_errors}, Audit {audit_errors}"
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This does get hit for me when cross validating - but, the only difference is in the userID of submitted_by. Since we intended to change how this field works, that's expected.

We may see a significant number of these as we transition to the new models since an Auditor often creates the record (current submitted_by), and the Certifying Auditee has to hit the final submit button (future submitted_by). But the actual errors will be equivalent, so it's fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let me take a closer look at what we can do here. I want to ensure we don't break the submitted_by for migrated audits. I was trying to avoid adding a new field, but we may need to.

Comment thread backend/audit/views/submission_progress_view.py
Copy link
Copy Markdown
Contributor

@jadudm jadudm left a comment

Choose a reason for hiding this comment

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

Reviewed with HDMS, RN, JR. Looks good for merging into the long-lived branch and continued effort.

@jadudm jadudm merged commit d3bcb32 into jr/source-of-truth/main Mar 11, 2025
@jadudm jadudm deleted the jr/source-of-truth/cross-validations branch March 11, 2025 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants