Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: OPTIC-1120: Draft lead time incorrectly calculated which can cause annotation lead time to be wrong #6406

Merged
merged 4 commits into from
Sep 25, 2024

Conversation

bmartel
Copy link
Contributor

@bmartel bmartel commented Sep 18, 2024

PR fulfills these requirements

  • Commit message(s) and PR title follows the format [fix|feat|ci|chore|doc]: TICKET-ID: Short description of change made ex. fix: DEV-XXXX: Removed inconsistent code usage causing intermittent errors
  • Tests for the changes have been added/updated (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)
  • Best efforts were made to ensure docs/code are concise and coherent (checked for spelling/grammatical errors, commented out code, debug logs etc.)
  • Self-reviewed and ran all changes on a local instance (for bug fixes/features)

Change has impacts in these area(s)

(check all that apply)

  • Product design
  • Backend (Database)
  • Backend (API)
  • Frontend

Describe the reason for change

When the current annotation would be a draft, based on the logic previously there it would assign total accumulated leadTime to both the current draft and any annotations. There was a point in time where the draft and annotation would be loaded into state, and the calculation made here would not properly factor in that drafts accumulate and eventually add the current running total of the draft over to the annotation once submitted. Because of this, we would end up doubling the value stored each time a draft would be saved, and once applied to the annotation.

An example I was able to reproduce was as follows:

  1. Open a task in quick view with no current annotations from my user.
  2. Annotate something and allow a draft to save
  3. Let some time pass noting the approximate time elapsed.
  4. Submit the annotation.
  5. Annotate or change something to produce the next draft
  6. Leave the task
  7. Return to the task after some amount of time noting the approximate time elapsed.
  8. Annotate or change something to produce the next draft save
  9. And continue to cycle draft, draft update, annotation submit, leaving and returning to task to simulate normal usage.
  10. Eventually the value of lead time would become extremely large, larger than even the total time from first time the task was opened and the first draft was produced.

The numbers I was able to find just infrequently working through a task with the above steps over the course of 4 hours, was:

  • Reported LeadTime: 119 hours
  • Approximate LeadTime: 1.7 hours (this is what I calculated it should have been)

It cannot be possible to have a lead time that is larger than the time from which the task was first taken and the first interaction made.

This is how it should be calculating:

Draft Created → 0s DraftLeadTime
Elapsed → 10s
Draft Save → 10s DraftLeadTime
Elapsed → 5s
Draft Save → 15s DraftLeadTime
Draft Submitted → 0 + 15 = 15s LeadTime
Draft Created → 0s DraftLeadTime
Elapsed → 20s
Draft Save → 20s DraftLeadTime
Elapsed → 25s
Draft Save → 45s DraftLeadTime
Draft Submitted → 15 + 45 = 60s LeadTime

The draft values should be rolling up to the annotation it would be converted to on submit, not incorrectly adding all previous annotation values on every draft save.

Does this PR introduce a breaking change?

(check only one)

  • Yes, and covered entirely by feature flag(s)
  • Yes, and covered partially by feature flag(s)
  • No
  • Not sure (briefly explain the situation below)

Which logical domain(s) does this change affect?

(Annotation, Draft, History) Lead Time

@github-actions github-actions bot added the fix label Sep 18, 2024
Copy link

netlify bot commented Sep 18, 2024

Deploy Preview for label-studio-docs-new-theme ready!

Name Link
🔨 Latest commit 8c74131
🔍 Latest deploy log https://app.netlify.com/sites/label-studio-docs-new-theme/deploys/66f4004dbf91c20008bd83de
😎 Deploy Preview https://deploy-preview-6406--label-studio-docs-new-theme.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Sep 18, 2024

Deploy Preview for heartex-docs ready!

Name Link
🔨 Latest commit 8c74131
🔍 Latest deploy log https://app.netlify.com/sites/heartex-docs/deploys/66f4004d2c25f90008e528b8
😎 Deploy Preview https://deploy-preview-6406--heartex-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@yyassi-heartex yyassi-heartex left a comment

Choose a reason for hiding this comment

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

minor tweak for readability otherwise approved

@bmartel
Copy link
Contributor Author

bmartel commented Sep 25, 2024

/git merge develop

Workflow run
Successfully merged: create mode 100644 web/libs/editor/tests/integration/e2e/linking_modes/to_comments.cy.ts

@bmartel bmartel merged commit 55fd471 into develop Sep 25, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants