SF-3657 Move project progress calculation to back end #3626

Nateowami · 2026-01-07T13:23:10Z

This change is

codecov · 2026-01-07T13:25:13Z

Codecov Report

❌ Patch coverage is 27.56410% with 113 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.55%. Comparing base (98ec5cd) to head (8920f29).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
.../SIL.XForge.Scripture/Services/SFProjectService.cs	1.17%	84 Missing ⚠️
...e.Scripture/Controllers/SFProjectsRpcController.cs	0.00%	17 Missing ⚠️
...translate-overview/translate-overview.component.ts	55.55%	3 Missing and 1 partial ⚠️
...d/book-multi-select/book-multi-select.component.ts	57.14%	2 Missing and 1 partial ⚠️
src/SIL.XForge.Scripture/Models/ProjectProgress.cs	0.00%	3 Missing ⚠️
...pture/ClientApp/src/app/core/sf-project.service.ts	0.00%	1 Missing ⚠️
...rc/app/shared/progress-service/progress.service.ts	96.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3626      +/-   ##
==========================================
- Coverage   82.82%   82.55%   -0.27%     
==========================================
  Files         610      611       +1     
  Lines       37438    37504      +66     
  Branches     6160     6160              
==========================================
- Hits        31008    30962      -46     
- Misses       5495     5591      +96     
- Partials      935      951      +16

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

Copilot

Pull request overview

This PR moves project progress calculation from the frontend to the backend, replacing real-time client-side computation with server-side MongoDB aggregation. The change improves performance by using database-level aggregation and implements a caching layer to reduce redundant API calls.

Key Changes

Backend MongoDB aggregation pipeline calculates progress by counting verse segments and blank segments per book
Frontend ProgressService simplified from real-time subscription model to cached promise-based API
Progress data structure changed from text-based to book-based with explicit verse segment counts

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
src/SIL.XForge.Scripture/Services/SFProjectService.cs	Adds GetProjectProgressAsync method with MongoDB aggregation pipeline to calculate book progress
src/SIL.XForge.Scripture/Services/ISFProjectService.cs	Adds interface method signature for GetProjectProgressAsync
src/SIL.XForge.Scripture/Models/ProjectProgress.cs	New BookProgress model to represent verse segment counts per book
src/SIL.XForge.Scripture/Controllers/SFProjectsRpcController.cs	New RPC endpoint GetProjectProgress exposing backend calculation to frontend
src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.ts	Updated to call new progress API with caching and use ProjectProgress model
src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.html	Template updated to work with book-based progress data structure
src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.spec.ts	Tests updated to mock new getProgress API instead of observables
src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.ts	Updated to fetch and use book progress data for training book selection
src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.spec.ts	Tests updated to mock ProjectProgress instead of TextProgress
src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts	Complete rewrite from real-time subscription model to cached promise-based API with deduplication
src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.spec.ts	New test suite for cache behavior, concurrency handling, and progress calculations
src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.ts	Updated to fetch progress via new API and added projectId input requirement
src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.spec.ts	Tests updated to mock new ProjectProgress data structure
src/SIL.XForge.Scripture/ClientApp/src/app/core/sf-project.service.ts	Added getProjectProgress method to call backend RPC endpoint

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts

Copilot · 2026-01-07T13:29:01Z

src/SIL.XForge.Scripture/ClientApp/src/app/core/sf-project.service.ts

+  async getProjectProgress(projectId: string): Promise<BookProgress[]> {
+    // TODO remove non-null assertion after #3622 is merged
+    return (await this.onlineInvoke<BookProgress[]>('getProjectProgress', { projectId }))!;


The TODO comment references issue #3622, but the non-null assertion operator is being used without checking if this will actually be resolved by that issue. The backend method GetProjectProgress returns BookProgress[] directly, not a nullable type, so the non-null assertion may not be needed at all. Verify if this TODO is accurate and if the assertion is actually necessary.

Suggested change

async getProjectProgress(projectId: string): Promise<BookProgress[]> {

// TODO remove non-null assertion after #3622 is merged

return (await this.onlineInvoke<BookProgress[]>('getProjectProgress', { projectId }))!;

async getProjectProgress(projectId: string): Promise<BookProgress[] | undefined> {

// TODO remove non-null assertion after #3622 is merged

return await this.onlineInvoke<BookProgress[]>('getProjectProgress', { projectId });

...L.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.ts

...rge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.ts

Copilot · 2026-01-07T13:29:02Z

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts

+      .catch(error => {
+        this.requestCache.delete(projectId);


The error handling doesn't notify users when progress data fails to load. When the promise rejects, the error is simply re-thrown, but there's no user-facing notification. Consider using the NoticeService to display an error message to the user when progress data cannot be fetched, similar to patterns used elsewhere in the codebase.

Suggested change

.catch(error => {

this.requestCache.delete(projectId);

.catch((error: unknown) => {

this.requestCache.delete(projectId);

this.noticeService.show('An error occurred while loading project progress.');

...rc/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.ts

src/SIL.XForge.Scripture/Services/SFProjectService.cs

...e.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.html

...rc/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.ts

Copilot · 2026-01-07T13:29:05Z

src/SIL.XForge.Scripture/Services/SFProjectService.cs

+            new BsonDocument
+            {
+                { "input", new BsonDocument("$ifNull", new BsonArray { "$$segment.attributes.segment", "" }) },
+                { "regex", "^verse_" },


The regex pattern "^verse_" will match any segment starting with "verse_", but this doesn't validate that what follows is a valid verse identifier. This could potentially match segments like "verse_" (with nothing after), "verse_invalid", etc. Consider if additional validation is needed to ensure only properly formatted verse segments are counted, or document that this is intentional behavior.

Suggested change

{ "regex", "^verse_" },

{ "regex", "^verse_\\d" },

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.

...XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.html

Copilot · 2026-01-07T14:57:13Z

src/SIL.XForge.Scripture/Services/SFProjectService.cs

+    public async Task<BookProgress[]> GetProjectProgressAsync(string curUserId, string projectId)
+    {
+        // Check that the project exists and user has permission to view it
+        SFProject project = await GetProjectAsync(projectId);
+        if (!project.UserRoles.ContainsKey(curUserId))
+            throw new ForbiddenException();
+
+        // Checks whether a segment is a verse segment by checking if the attributes.segment field starts with "verse_"
+        BsonDocument isVerseSegmentIdExpression = new BsonDocument(
+            "$regexMatch",
+            new BsonDocument
+            {
+                { "input", new BsonDocument("$ifNull", new BsonArray { "$$segment.attributes.segment", "" }) },
+                { "regex", "^verse_" },
+            }
+        );
+
+        // Filters for ops that are verse segments (i.e., attributes.segment starts with "verse_")
+        BsonDocument verseSegmentOpsFilterExpression = new BsonDocument
+        {
+            { "input", "$ops" },
+            { "as", "segment" },
+            { "cond", isVerseSegmentIdExpression },
+        };
+        // Same as above filter, except that insert.blank must also be true in order to match a segment
+        BsonDocument blankVerseSegmentOpsFilterExpression = new BsonDocument
+        {
+            { "input", "$ops" },
+            { "as", "segment" },
+            {
+                "cond",
+                new BsonDocument(
+                    "$and",
+                    new BsonArray
+                    {
+                        isVerseSegmentIdExpression,
+                        new BsonDocument("$eq", new BsonArray { "$$segment.insert.blank", true }),
+                    }
+                )
+            },
+        };
+
+        List<BsonDocument> results = await _database
+            .GetCollection<BsonDocument>("texts")
+            .Aggregate()
+            // Filter for text documents that belong to the specified project
+            .Match(Builders<BsonDocument>.Filter.Regex("_id", new BsonRegularExpression($"^{projectId}:")))
+            // Project:
+            // - Extract the book ID from the document ID
+            // - Count the number of verse segments
+            // - Count the number of blank verse segments
+            .Project(
+                new BsonDocument
+                {
+                    { "_id", 1 },
+                    {
+                        "book",
+                        new BsonDocument(
+                            "$arrayElemAt",
+                            new BsonArray { new BsonDocument("$split", new BsonArray { "$_id", ":" }), 1 }
+                        )
+                    },
+                    {
+                        "verseSegments",
+                        new BsonDocument("$size", new BsonDocument("$filter", verseSegmentOpsFilterExpression))
+                    },
+                    {
+                        "blankVerseSegments",
+                        new BsonDocument("$size", new BsonDocument("$filter", blankVerseSegmentOpsFilterExpression))
+                    },
+                }
+            )
+            // Group progress by book and count the total verse segments and blank verse segments for each book
+            .Group(
+                new BsonDocument
+                {
+                    { "_id", "$book" },
+                    { "verseSegments", new BsonDocument("$sum", "$verseSegments") },
+                    { "blankVerseSegments", new BsonDocument("$sum", "$blankVerseSegments") },
+                }
+            )
+            .ToListAsync();
+
+        var books = results
+            .Select(doc => new BookProgress
+            {
+                BookId = doc["_id"].AsString,
+                VerseSegments = doc["verseSegments"].AsInt32,
+                BlankVerseSegments = doc["blankVerseSegments"].AsInt32,
+            })
+            .ToArray();
+
+        return books;
+    }


The new GetProjectProgressAsync method in SFProjectService lacks test coverage. Since the test file SFProjectServiceTests.cs exists with comprehensive tests for other methods in this service, tests should be added for this new method to maintain consistent test coverage, especially given the complexity of the MongoDB aggregation pipeline logic.

Copilot · 2026-01-07T14:57:13Z

...rge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.ts

+  getBookNameFromId(bookId: string): string {
+    const bookNum = Canon.bookIdToNumber(bookId);
+    return this.i18n.localizeBook(bookNum);
+  }
+
+  get hasMinimumSegmentsToTrainStatisticalEngine(): boolean {
+    return (this.projectProgress?.translatedVerseSegments ?? 0) >= 10;
+  }
+
+  bookTranslatedSegments(bookProgress: BookProgress): number {
+    return bookProgress.verseSegments - bookProgress.blankVerseSegments;
+  }
+
+  bookTranslationRatio(bookProgress: BookProgress): number {
+    if (bookProgress.verseSegments === 0) {
+      return 0;
+    }
+    return this.bookTranslatedSegments(bookProgress) / bookProgress.verseSegments;
+  }
+
  getBookName(text: TextInfo): string {
    return this.i18n.localizeBook(text.bookNum);
  }


The method getBookNameFromId has been added but the existing method getBookName(text: TextInfo) still exists and serves a similar purpose. Consider consolidating these methods or adding a comment explaining why both are needed to avoid confusion and potential duplication.

Copilot · 2026-01-07T14:57:14Z

...L.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.ts

+      if (this.projectId == null) {
+        console.error('app-book-multi-select requires a projectId input to initialize when not in basic mode');
+        return;
+      }


The error message is logged to console.error but doesn't notify the user or handle the error gracefully. When projectId is missing in non-basic mode, the component returns early but leaves the UI in a partially initialized state. Consider throwing an error or showing a user-facing error message using the noticeService to make this issue more visible.

Copilot · 2026-01-07T14:57:14Z

...L.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.ts

-    await firstValueFrom(this.progressService.isLoaded$.pipe(filter(loaded => loaded)));
+    // Only load progress if not in basic mode
+    let progress: ProjectProgress | undefined;
+    if (this.basicMode !== true) {


The comparison basicMode !== true is used instead of a simple falsy check. However, according to the coding guidelines, you should use explicit comparisons rather than relying on truthy/falsy. Since basicMode is a boolean, consider using basicMode === false instead of basicMode !== true for better clarity about what state you're checking for.

Copilot · 2026-01-07T14:57:14Z

...e.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.html

        <mat-list role="list">
-          @for (textProgress of progressService.texts; track trackTextByBookNum($index, textProgress)) {
-            <mat-list-item role="listitem" [appRouterLink]="['./', getBookId(textProgress.text)]">
+          @for (bookProgress of projectProgress.books; track bookProgress.bookId) {


The template is now using trackTextByBookId with bookProgress.bookId directly in the @for loop instead of using the trackBy function. Consider removing the trackTextByBookId function if it's no longer needed, or update the @for directive to use it: @for (bookProgress of projectProgress.books; track trackTextByBookId($index, bookProgress))

Suggested change

@for (bookProgress of projectProgress.books; track bookProgress.bookId) {

@for (bookProgress of projectProgress.books; track trackTextByBookId($index, bookProgress)) {

Copilot · 2026-01-07T14:57:15Z

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts

+  verseSegments = this.books.reduce((acc, book) => acc + book.verseSegments, 0);
+  blankVerseSegments = this.books.reduce((acc, book) => acc + book.blankVerseSegments, 0);
+  translatedVerseSegments = this.verseSegments - this.blankVerseSegments;
+  ratio = this.verseSegments === 0 ? 0 : this.translatedVerseSegments / this.verseSegments;


The fields verseSegments, blankVerseSegments, translatedVerseSegments, and ratio are declared without explicit type annotations. According to the coding guidelines, variables should have explicit type annotations. Add type annotations for these fields (e.g., verseSegments: number = this.books.reduce...).

RaymondLuong3

It looks like there is some test coverage issues with this PR. Particularly I think the coverage for SFProjectsRpcController can easily be added.
I tested this out and the progress appears to be fast and the caching is working nicely.
Can you fill out the acceptance tests for this on the issue?

@RaymondLuong3 reviewed 17 files and all commit messages, and made 8 comments.
Reviewable status: all files reviewed, 19 unresolved discussions (waiting on @for and @Nateowami).

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation-steps/draft-generation-steps.component.ts line 63 at r4 (raw file):

// The project needs at least 10 translated segments for the statistical engine to be able to train. Additionally, the
// segments need to be aligned with training data in the source project, though this component doesn't check for that.
const MIN_TRANSLATED_SEGMENTS_FOR_STATISTICAL_ENGINE = 10 as const;

This might not be named right. It is being used to determine if a book should be auto selected for training data in the NMT engine.

Code quote:

const MIN_TRANSLATED_SEGMENTS_FOR_STATISTICAL_ENGINE = 10 as const;

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts line 104 at r4 (raw file):

 * For most books, this will be the same as the ratio of translated segments to total segments, but if a book has very
 * few segments but many expected verses (total segments < 10% of expected verses), it's unlikely the book actually
 * combines verses so much that it's produced this ratio, and more likely the book is just missing most verses. In this

typo

Code quote:

it's produced this ratio,

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts line 111 at r4 (raw file):

  const translatedSegments = bookProgress.verseSegments - bookProgress.blankVerseSegments;
  const expectedNumberOfVerses = verseCounts[bookProgress.bookId] ?? 0;
  if (expectedNumberOfVerses !== 0 && bookProgress.verseSegments < expectedNumberOfVerses * 0.1) {

I would give the 0.1 a meaningful name.

src/SIL.XForge.Scripture/ClientApp/src/app/shared/book-multi-select/book-multi-select.component.ts line 152 at r4 (raw file):

  }

  /** Takes a number between 0 and 100, and rounds it by flooring any number between 99 and 100 to 99 */

Seems like a strange thing to do. What problem does this fix? Do percentages round up?

Code quote:

  /** Takes a number between 0 and 100, and rounds it by flooring any number between 99 and 100 to 99 */

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.spec.ts line 23 at r4 (raw file):

      { bookId: 'MAT', verseSegments: 50, blankVerseSegments: 10 }
    ];
    when(mockedProjectService.getProjectProgress(projectId)).thenReturn(Promise.resolve(expectedBooks));

Nit: You should be able to use thenResolve(...) to make it cleaner

Code quote:

thenReturn(Promise.resolve(expectedBooks));

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.spec.ts line 64 at r4 (raw file):

    when(mockedProjectService.getProjectProgress(projectId))
      .thenReturn(Promise.resolve(firstBooks))

nit: You should be able to use .thenResolve(...)

Code quote:

      .thenReturn(Promise.resolve(firstBooks))

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.spec.ts line 134 at r4 (raw file):

    const project2Books: BookProgress[] = [{ bookId: 'MAT', verseSegments: 50, blankVerseSegments: 10 }];

    when(mockedProjectService.getProjectProgress('project1')).thenReturn(Promise.resolve(project1Books));

nit: This can probably use thenResolve(...)

Code quote:

thenReturn(Promise.resolve(project1Books));

Nateowami requested a review from Copilot January 7, 2026 13:23

Nateowami added the will require testing PR should not be merged until testers confirm testing is complete label Jan 7, 2026

Copilot started reviewing on behalf of Nateowami January 7, 2026 13:23 View session

Copilot AI reviewed Jan 7, 2026

View reviewed changes

Nateowami force-pushed the feature/SF-3657-back-end-progress-calculation branch from 954adec to 2f98d91 Compare January 7, 2026 14:31

Nateowami requested a review from Copilot January 7, 2026 14:52

Copilot started reviewing on behalf of Nateowami January 7, 2026 14:52 View session

Copilot AI reviewed Jan 7, 2026

View reviewed changes

Nateowami force-pushed the feature/SF-3657-back-end-progress-calculation branch 2 times, most recently from 2af4104 to 49d180d Compare January 8, 2026 15:20

SF-3657 Move project progress calculation to back end

8920f29

Nateowami force-pushed the feature/SF-3657-back-end-progress-calculation branch from 49d180d to 8920f29 Compare January 8, 2026 17:43

RaymondLuong3 requested changes Jan 9, 2026

View reviewed changes

RaymondLuong3 self-assigned this Jan 9, 2026

	@for (bookProgress of projectProgress.books; track bookProgress.bookId) {
	@for (bookProgress of projectProgress.books; track trackTextByBookId($index, bookProgress)) {

Uh oh!

SF-3657 Move project progress calculation to back end #3626

Are you sure you want to change the base?

SF-3657 Move project progress calculation to back end #3626

Uh oh!

Conversation

Nateowami commented Jan 7, 2026 • edited by irahopkinson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

RaymondLuong3 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Nateowami commented Jan 7, 2026 •

edited by irahopkinson

Loading

codecov bot commented Jan 7, 2026 •

edited

Loading