Restore: Normalize user CRDT writes to NFD (#2146) by myieye · Pull Request #2318 · sillsdev/languageforge-lexbox

myieye · 2026-05-29T09:45:36Z

Re-applies PR #2146 (commit 79c015a), which was lost from develop in a force-push ~9 days ago 🫨

Single-commit cherry-pick onto current develop. Two trivial textual conflicts resolved (kept both sides):

FwLiteWeb/Routes/MiniLcmRoutes.cs: union of using directives
MiniLcm.Tests/CreateEntryTestsBase.cs: kept both HomographNumber and Order exclusions in CanCreateEntry_AutoFaker

Local builds: FwLiteWeb, FwLiteMaui (win), MiniLcm.Tests, FwLiteProjectSync.Tests, FwDataMiniLcmBridge.Tests, LcmCrdt.Tests all green.

coderabbitai · 2026-05-29T09:45:49Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 20de34d0-9ce4-4693-b68d-f15f467d690a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR refactors MiniLcm API wrapper composition to add write-normalization of user-entered linguistic text to NFD form, rename query-normalization types for clarity, establish a unified wrapper orchestrator, update all service entry points to use it, and add comprehensive test infrastructure validating the new normalization behavior.

Changes

Unicode Normalization Wrapper System

Layer / File(s)	Summary
String normalization utility `backend/FwLite/MiniLcm/Normalization/StringNormalizer.cs`	`StringNormalizer` is a static utility that normalizes `string?`, `MultiString`, `RichString?`, and `RichMultiString` to `NormalizationForm.FormD` (NFD), preserving null values and empty strings.
Write normalization wrapper `backend/FwLite/MiniLcm/Normalization/MiniLcmApiWriteNormalizationWrapper.cs`	Introduces `MiniLcmApiWriteNormalizationWrapperFactory` and `MiniLcmApiWriteNormalizationWrapper` to normalize user-entered text on all write operations for linguistic entities (PartOfSpeech, Publication, SemanticDomain, ComplexFormType, MorphType, Entry, Sense, ExampleSentence, Translation, ComplexFormComponent), including nested structures and JSON patch operations, while explicitly passing through `WritingSystem` and file operations without normalization.
Query normalization rename `backend/FwLite/MiniLcm/Normalization/MiniLcmApiQueryNormalizationWrapper.cs`	Renames `MiniLcmApiStringNormalizationWrapper` types to `MiniLcmApiQueryNormalizationWrapper` to disambiguate from write-normalization and clarify that this wrapper normalizes query/search inputs.
User-facing wrapper orchestrator `backend/FwLite/MiniLcm/Wrappers/MiniLcmApiUserFacingWrappers.cs`, `backend/FwLite/MiniLcm/Wrappers/MiniLcmWrappers.cs`	`MiniLcmApiUserFacingWrappers` enforces a standard wrapper application order: read normalization → validation → write normalization → inner wrappers. This centralizes wrapper composition and ensures validation occurs before write normalization. Documentation is added to `MiniLcmWrappers.cs` describing wrapper application order.
Dependency injection configuration `backend/FwLite/MiniLcm/Validators/MiniLcmValidators.cs`	Updates DI registrations to replace `MiniLcmApiStringNormalizationWrapperFactory` with `MiniLcmApiQueryNormalizationWrapperFactory`, `MiniLcmApiWriteNormalizationWrapperFactory`, and the new `MiniLcmApiUserFacingWrappers` orchestrator.
Service and hub wrapper wiring `backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs`, `backend/FwLite/FwLiteShared/Services/MiniLcmJsInvokable.cs`, `backend/FwLite/FwLiteWeb/Hubs/CrdtMiniLcmApiHub.cs`, `backend/FwLite/FwLiteWeb/Hubs/FwDataMiniLcmHub.cs`, `backend/FwLite/FwLiteWeb/Hubs/MiniLcmApiHubBase.cs`, `backend/FwLite/FwLiteWeb/Routes/MiniLcmRoutes.cs`	All service/hub/route entry points are updated to inject and use `MiniLcmApiUserFacingWrappers` instead of composing wrappers from individual factories, simplifying initialization logic and centralizing wrapper ordering.

Normalization Test Infrastructure and Coverage

Layer / File(s)	Summary
Test data and assertion helpers `backend/FwLite/MiniLcm.Tests/Helpers/NfcTestData.cs`, `backend/FwLite/MiniLcm.Tests/Helpers/NormalizationAssert.cs`	`NfcTestData` provides factory methods to generate test objects populated with NFC Unicode strings for consistent test data. `NormalizationAssert` recursively validates object graphs for NFC or NFD normalization compliance, including specialized handling for `MultiString`, `RichString`, `RichMultiString`, and configurable property skipping.
Write normalization test suite `backend/FwLite/MiniLcm.Tests/WriteNormalizationTests.cs`	`WriteNormalizationTests` comprehensively validates write normalization behavior across all entity types (WritingSystem, PartOfSpeech, Publication, SemanticDomain, ComplexFormType, MorphType, Entry, Sense, ExampleSentence, Translation, ComplexFormComponent) for create/update/bulk operations, JSON patch handling, and selective pass-through. `NormalizationAssertTests` validates the assertion helper's own behavior.
Existing test updates `backend/FwLite/MiniLcm.Tests/CreateEntryTestsBase.cs`, `backend/FwLite/MiniLcm.Tests/ExampleSentenceTestsBase.cs`, `backend/FwLite/MiniLcm.Tests/MiniLcmTestBase.cs`, `backend/FwLite/MiniLcm.Tests/NormalizationTests.cs`, `backend/FwLite/MiniLcm.Tests/QueryEntryTestsBase.cs`, `backend/FwLite/MiniLcm.Tests/SortingTestsBase.cs`, `backend/FwLite/MiniLcm.Tests/UpdateEntryTestsBase.cs`, `backend/FwLite/FwDataMiniLcmBridge.Tests/MiniLcmTests/UpdateEntryTests.cs`	Equivalence assertions are updated to exclude auto-generated `Order` properties from comparison; query normalization tests expect renamed `MiniLcmApiQueryNormalizationWrapper` types; `MiniLcmTestBase` wraps with query and write normalization factories; and search tests remove pre-normalization, relying instead on API write normalization.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

sillsdev/languageforge-lexbox#1788: Introduces MiniLcmApiValidationWrapper and the validation wrapper factory integrated into this PR's MiniLcmApiUserFacingWrappers stack; removes inline validator calls from underlying APIs.
sillsdev/languageforge-lexbox#2146: Extends MiniLcm wrapper infrastructure with additional normalization/validation features; shares the same wrapper composition and orchestration patterns.
sillsdev/languageforge-lexbox#1873: Earlier work on string/query normalization wrapper that this PR renames and integrates into the new unified wrapper orchestrator.

Suggested labels

💻 FW Lite

Suggested reviewers

hahn-kev
rmunn
imnasnainaec

Poem

🐰 Unicode hops with care and grace,
NFD normalizes every place,
Wrappers stack in ordered dance,
Tests ensure the strings advance!
Write and read, validation flows—
Where the normalization goes! 📝

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 3.95% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: restoring normalization of user CRDT writes to NFD form, with explicit PR number reference.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description clearly explains that this is a cherry-pick restoration of PR `#2146` with specific conflict resolutions and local build validation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch restore/nfd-normalize-2146

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2026-05-29T09:48:13Z

UI unit Tests

1 files ±0 59 suites ±0 24s ⏱️ -5s
177 tests ±0 177 ✅ ±0 0 💤 ±0 0 ❌ ±0
246 runs ±0 246 ✅ ±0 0 💤 ±0 0 ❌ ±0

Results for commit a5cb293. ± Comparison against base commit a596492.

♻️ This comment has been updated with latest results.

argos-ci · 2026-05-29T09:48:46Z

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build	Status	Details	Updated (UTC)
default (Inspect)	✅ No changes detected	-	Jun 1, 2026, 7:05 AM

rmunn

Double-checked that the cherry-pick didn't miss anything, and it didn't. The only differences between this PR and #2146 is the bit I commented on below, and one place that I didn't bother commenting on because it was an obvious typo fix.

* Normalize to NFD on write * Adapt tests to now non-mutated api input * Rename normalization wrappers and consolidate wrapper stack (#2259) * Introduce MiniLcmApiUserFacingWrappers to centralise wrapper ordering and rationale * Don't pass 'this' in normalization wrapper, because everything is normalized in first pass * Rename normalization wrappers for consistent naming

github-actions Bot added the 💻 FW Lite issues related to the fw lite application, not miniLcm or crdt related label May 29, 2026

myieye force-pushed the restore/nfd-normalize-2146 branch 2 times, most recently from 2708fcd to 4c01111 Compare May 29, 2026 10:22

rmunn approved these changes Jun 1, 2026

View reviewed changes

Comment thread backend/FwLite/MiniLcm/Normalization/MiniLcmApiWriteNormalizationWrapper.cs Outdated

myieye force-pushed the restore/nfd-normalize-2146 branch from 4c01111 to a5cb293 Compare June 1, 2026 07:02

myieye merged commit 905d44e into develop Jun 1, 2026
21 checks passed

myieye deleted the restore/nfd-normalize-2146 branch June 1, 2026 08:30

coderabbitai Bot mentioned this pull request Jun 2, 2026

Harden NFD write-normalization against dropped fields #2322

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Restore: Normalize user CRDT writes to NFD (#2146)#2318

Restore: Normalize user CRDT writes to NFD (#2146)#2318
myieye merged 1 commit into
developfrom
restore/nfd-normalize-2146

myieye commented May 29, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 29, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Review ran into problems

Uh oh!

github-actions Bot commented May 29, 2026 •

edited

Loading

Uh oh!

argos-ci Bot commented May 29, 2026 •

edited

Loading

Uh oh!

rmunn left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

myieye commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Review ran into problems

Uh oh!

github-actions Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

UI unit Tests

Uh oh!

argos-ci Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rmunn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

myieye commented May 29, 2026 •

edited

Loading

coderabbitai Bot commented May 29, 2026 •

edited

Loading

github-actions Bot commented May 29, 2026 •

edited

Loading

argos-ci Bot commented May 29, 2026 •

edited

Loading