Skip to content

SF-3852 Show quality estimates#3990

Open
Nateowami wants to merge 1 commit into
masterfrom
feature/SF-3852-show-quality-estimates
Open

SF-3852 Show quality estimates#3990
Nateowami wants to merge 1 commit into
masterfrom
feature/SF-3852-show-quality-estimates

Conversation

@Nateowami

@Nateowami Nateowami commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator
  • I did not put this behind a feature flag because it will only show up for projects that have quality estimates configured.
  • The UI design is probably not ideal, but I did discuss it with EITL as a simple first step.
Screenshot from 2026-07-08 16-58-04

This change is Reviewable

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.61017% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.03%. Comparing base (6e818b7) to head (0b85867).
⚠️ Report is 3 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
.../build-confidences/display-confidence.component.ts 80.00% 0 Missing and 1 partial ⚠️
...SIL.XForge.Scripture/Services/MachineApiService.cs 98.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3990      +/-   ##
==========================================
- Coverage   81.03%   81.03%   -0.01%     
==========================================
  Files         645      645              
  Lines       41522    41526       +4     
  Branches     6763     6758       -5     
==========================================
  Hits        33649    33649              
- Misses       6761     6762       +1     
- Partials     1112     1115       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@Nateowami
Nateowami temporarily deployed to screenshot_diff July 8, 2026 23:25 — with GitHub Actions Inactive
@Nateowami Nateowami added will require testing PR should not be merged until testers confirm testing is complete and removed testing not required labels Jul 8, 2026
@pmachapman pmachapman self-assigned this Jul 12, 2026
@pmachapman
pmachapman self-requested a review July 12, 2026 18:36

@pmachapman pmachapman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@pmachapman reviewed 8 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Nateowami).


src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts line 144 at r1 (raw file):

        .pipe(quietTakeUntilDestroyed(this.destroyRef), takeUntil(this.entryChanged))
        .subscribe(confidences => (this._buildConfidences = confidences));
    }

This results in a large amount of requests being sent to the backend - one request per build, every time the page is loaded.

Would it be possible to instead update MachineApiService.MapDtoAsync() to load the corresponding draft metrics for the build, and populate the book level build confidences into the model ServalBuildDto for any builds that have quality estimation configured?

Code quote:

    // Get the quality estimation confidences for this build, if available
    this._buildConfidences = undefined;
    const buildId = this._entry?.additionalInfo?.buildId;
    if (this.activatedProjectService.projectId != null && buildId != null && buildId !== '') {
      this.draftGenerationService
        .getBuildConfidences(this.activatedProjectService.projectId, buildId)
        .pipe(quietTakeUntilDestroyed(this.destroyRef), takeUntil(this.entryChanged))
        .subscribe(confidences => (this._buildConfidences = confidences));
    }

@Nateowami
Nateowami force-pushed the feature/SF-3852-show-quality-estimates branch from 9bc3406 to 0b85867 Compare July 16, 2026 20:37

@Nateowami Nateowami left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@Nateowami made 1 comment.
Reviewable status: 6 of 18 files reviewed, 1 unresolved discussion (waiting on pmachapman).


src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.ts line 144 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

This results in a large amount of requests being sent to the backend - one request per build, every time the page is loaded.

Would it be possible to instead update MachineApiService.MapDtoAsync() to load the corresponding draft metrics for the build, and populate the book level build confidences into the model ServalBuildDto for any builds that have quality estimation configured?

Good point. I've made the change and ripped out the unused endpoint. Do you think that was too aggressive?

@Nateowami
Nateowami temporarily deployed to screenshot_diff July 16, 2026 20:44 — with GitHub Actions Inactive

@pmachapman pmachapman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

:lgtm:

@pmachapman reviewed 12 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Nateowami).

@pmachapman pmachapman added ready to test will require testing PR should not be merged until testers confirm testing is complete and removed will require testing PR should not be merged until testers confirm testing is complete ready to test labels Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

will require testing PR should not be merged until testers confirm testing is complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants