Skip to content

SF-3951 Fix issue where chapters with unsupported USFM were not synced - #4210

Open
pmachapman wants to merge 2 commits into
masterfrom
fix/SF-3951
Open

pmachapman wants to merge 2 commits into
masterfrom
fix/SF-3951

Conversation

@pmachapman

@pmachapman pmachapman commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

This PR enables chapters that have been applied from drafts, but which have invalid USFM (i.e. because the Source USFM used non-standard USFM or USFM that SF does not support, such as tables), to be synced to Paratext.

I have also fixed an issue that prevented projects which were affected by this bug, and had one successful sync, from resyncing. The sync for these projects would crash with an error along the lines of:

Paratext.Data.ChapterizationException Text: ABC Book: NUM
The chapter number(s) in parentheses are invalid, out of order, or duplicated:
3 (3) 4 5 6 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 
    unknown Paratext.Data.ScrText.SplitIntoChapters(string scrTextName, int bookNum, string bookText)
    unknown Paratext.Data.ScrText.PutText(int bookNum, int chapterNum, bool doMapOut, string usfm, WriteLock textLock)
    /home/runner/work/web-xforge/web-xforge/src/SIL.XForge.Scripture/Services/ParatextService.cs:3879 SIL.XForge.Scripture.Services.ParatextService+<>c__DisplayClass131_0.<WriteChapterToScrText>b__0()
    unknown Paratext.Data.Users.PermissionManager.RunWithEditPermision(int bookNum, Action action)
    /home/runner/work/web-xforge/web-xforge/src/SIL.XForge.Scripture/Services/ParatextService.cs:3877 SIL.XForge.Scripture.Services.ParatextService.WriteChapterToScrText(ScrText scrText, string userId, int bookNum, int chapterNum, string usfm)
    /home/runner/work/web-xforge/web-xforge/src/SIL.XForge.Scripture/Services/ParatextService.cs:1219 SIL.XForge.Scripture.Services.ParatextService+<PutBookText>d__63.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    unknown System.Runtime.CompilerServices.TaskAwaiter<TResult>.GetResult()
    /home/runner/work/web-xforge/web-xforge/src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs:1145 SIL.XForge.Scripture.Services.ParatextSyncRunner+<UpdateParatextBookAsync>d__41.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    /home/runner/work/web-xforge/web-xforge/src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs:578 SIL.XForge.Scripture.Services.ParatextSyncRunner+<GetAndUpdateParatextBooksAndNotes>d__30.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    /home/runner/work/web-xforge/web-xforge/src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs:229 SIL.XForge.Scripture.Services.ParatextSyncRunner+<RunAsync>d__29.MoveNext()

This change is Reviewable

@pmachapman pmachapman added the will require testing PR should not be merged until testers confirm testing is complete label Oct 20, 2026
@codecov

codecov Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.20%. Comparing base (bc6f970) to head (966b06a).
⚠️ Report is 2 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...IL.XForge.Scripture/Services/ParatextSyncRunner.cs 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4210   +/-   ##
=======================================
  Coverage   81.20%   81.20%           
=======================================
  Files         670      670           
  Lines       42913    42922    +9     
  Branches     7105     7112    +7     
=======================================
+ Hits        34846    34854    +8     
  Misses       6902     6902           
- Partials     1165     1166    +1     

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

@pmachapman
pmachapman deployed to screenshot_diff October 20, 2026 23:55 — with GitHub Actions Active
@pmachapman pmachapman changed the title SF-3951 Sync chapters generated by drafts with invalid USFM SF-3951 Fix issue where chapters with tables were not synced Oct 21, 2026
@pmachapman pmachapman changed the title SF-3951 Fix issue where chapters with tables were not synced SF-3951 Fix issue where chapters with unsupported USFM were not synced Oct 21, 2026

@RaymondLuong3 RaymondLuong3 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.

Thanks for this fix.

@RaymondLuong3 reviewed 3 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on pmachapman).


src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs line 1155 at r1 (raw file):

                    c.LastVerse,
                    // If a draft was applied but has invalid USFM, sync it so the user can resolve in Paratext
                    c.IsValid || (c.DraftApplied ?? false),

It looks like we should probably have a test to show that this works correctly for invalid chapters when a draft was applied.

Code quote:

                    c.IsValid || (c.DraftApplied ?? false),

@RaymondLuong3 RaymondLuong3 self-assigned this Oct 22, 2026

@pmachapman pmachapman 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.

@pmachapman made 1 comment.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on RaymondLuong3).


src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs line 1155 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

It looks like we should probably have a test to show that this works correctly for invalid chapters when a draft was applied.

Done. Thank you!

@pmachapman
pmachapman deployed to screenshot_diff October 26, 2026 19:09 — with GitHub Actions Active

@RaymondLuong3 RaymondLuong3 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:

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

@RaymondLuong3 RaymondLuong3 added ready to test and removed will require testing PR should not be merged until testers confirm testing is complete labels Oct 27, 2026

This branch was successfully deployed

1 active deployment
screenshot_diff — 966b06a9 Deployed Oct 26, 2026 by pmachapman via Compare Screenshots #1199
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.

2 participants