Add variant model, CRDT changes, FwData bridge and sync (1/3)#2416

Draft

myieye wants to merge 24 commits into

feat/variants-model-sync

myieye commented Jul 6, 2026 •

edited

Loading

Collaborator

Adds variant relationships to FwLite — model, CRDT changes, FwData bridge, and FwData↔CRDT sync. Editor UI is a follow-up.

A Variant link maps to one FieldWorks LexEntryRef (RefType=Variant): per-link ordered Types, HideMinorEntry, Comment.
⚠️ Cycle checks now span the combined complex-form + variant graph, so AddEntryComponentChange replay changes for every complex-form project — see VARIANTS.md (decision 7).
Design, decisions, and rollout notes: backend/FwLite/VARIANTS.md.

Tested by conformance suites on both IMiniLcmApi implementations, FwData↔CRDT round-trip sync, and a seeded differential oracle against liblcm's cycle rule.

myieye and others added 20 commits

July 6, 2026 15:13


          Add variant model, CRDT changes and API surface

619c640

One Variant link = one LexEntryRef (RefType=Variant) with per-link Types,
HideMinorEntry and Comment (LCM Summary), per VARIANTS.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Wire variants into sync orchestration, import and tests

979f1ad

Conformance suites for both IMiniLcmApi implementations, change/usage/serialization
tests, verified-file regens, AutoFaker support. LCM enforces acyclicity over the
combined complex-form + variant graph, so AddVariantChange mirrors that check.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Add variant fields to viewer entry factories and demo data

deb8129

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Seed standard variant types in the blank project template

ef20839

Regenerated via ProjectTemplateTests.GenerateTemplate; new CRDT projects get the
7 standard FLEx variant types (well-known guids), matching FieldWorks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Sync variants FwData<->CRDT: round-trip tests, import fast path, Sena3

f11fc37

Variant lists sort by composite key on both sides (deterministic, culture-free);
BulkCreateEntries emits variant links; regenerated Sena3 live db + snapshots —
sena-3's real variants (custom Pronunciation Variant type) now round-trip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Update VARIANTS.md status

1b38e9b

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Review fixes: sense-move keeps variant links, hub exposes variant types

b1bdea7

A sense-targeted variant link is identified by its full composite key; the FwData
bridge matched by sense guid alone, so a moved sense made the sync drop the link
(new regression test covers both implementations).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Demo api parity for the extended MiniLcm JS interface

a45f5f7

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Note review outcome and PR links in VARIANTS.md

3f720ab

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          CodeRabbit fixes: sense-ownership guard on variant create, require va…

be8a6c3

…riant entry id

Both implementations now reject a MainSenseId that belongs to a different entry
(fail before mutation); EntryValidator requires VariantEntryId on Variants items.
Also corrects the stale cycle-guard decision text in VARIANTS.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Cover entry-level variant reference validation rules

9c9c270

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Correct VARIANTS.md on type-rename propagation and Comment nullability

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Match liblcm variant/component semantics found in a deep review

6a18cb4

- Cycle checks now share one liblcm-faithful walk (ComponentGraph): sense-targeted
  links resolve to the owning entry without recursing (LexEntry.AllComponents), and
  AddEntryComponentChange walks the combined complex-form + variant graph like
  liblcm instead of complex forms only
- Per-link edits split a shared (multi-component) variant ref before mutating so
  sibling links keep their own Types/HideMinorEntry/Comment
- Variant reads skip non-entry/non-sense components instead of wedging the entry
- HideMinorEntry only written when the bool view flips (LCM reserves a bitfield)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Harden variant tests: LCM-oracle differential check, HideMinorEntry b…

ebeb384

…itfield

Mutation-tested the new cycle/shared-ref guards (3/3 mutants killed); the
surviving would-be mutant exposed the missing HideMinorEntry bitfield test.
Also docs: fleet-skew rollout note for AddEntryComponentChange, badge and
type-seeding follow-ups, SubmitUpdateVariant asymmetry rationale.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Bring VARIANTS.md status and review log up to date for handoff

7d44dd4

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Review fixes: variant Submit validation gaps, patch guards, naming

b292156

- BeaKona forwarders bypassed validation on SubmitCreateVariant and
  SubmitUpdateVariant; both now validate (same class of gap as #2362)
- New VariantUpdateValidator rejects patches to Types (incl. by index,
  via new DoesNotChangePropertyOrChildren), endpoints, derived
  headwords, DeletedAt and Id
- VariantType and ComplexFormType names now reject empty per-WS values
- CreateEntryOptions.IncludeComplexFormsAndComponents renamed to
  IncludeEntryReferences (it gates variants too); generated TS updated
- BoolDiff helper (mirrors IntegerDiff); ToVariants flattened to
  Concat; FluentAssert Id-exclusion no longer names one type''s members;
  ProjectSnapshot.VariantTypes legacy-null note sharpened

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Round-trip variant type order (review: types are user-ordered in FLEx)

f2fcff4

FLEx lets users reorder a ref''s VariantEntryTypesRS (right-click, move
left/right); syncing types as a set silently lost that order. Adds
SetVariantTypesOrder to IMiniLcmWriteApi (both implementations, dry-run,
wrappers, notify) and SetVariantTypesOrderChange (unlisted types keep
their relative order after listed ones, so concurrent type adds merge).
VariantSync resets order after the add/remove diff whenever it cannot
have landed in after-order. Variant link lists themselves stay
unordered (decision 6, amended).

Covered by shared conformance tests (both impls), a CRDT change test
incl. the concurrent-add merge, and a full FwData<->CRDT sync
round-trip; change-model/regression verified files regenerated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Rework variant type ordering to per-item orders (review)

The whole-list SetVariantTypesOrderChange clobbered concurrent
reorders; per Tim''s review, type order now works like sense pictures:
Variant.Types holds VariantTypeRef objects ({typeId, fractional Order},
which also removes the stale-embedded-Name wart from decision 8),
AddVariantType takes a BetweenPosition, MoveVariantType replaces the
list-order API, and ReorderVariantTypeChange merges per type under
concurrency. VariantSync diffs types with DiffOrderable.

Regenerated: change/snapshot regression data (old branch-era shapes
auto-migrated to the legacy files; the never-shipped
SetVariantTypesOrderChange entry removed), DbModel/ChangeModels
snapshots, sena-3-live snapshot, TS types (IVariantTypeRef).

(The field-editor adaptations live in the stacked editor-UI branch.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


          Merge remote-tracking branch 'origin/develop' into feat/variants-mode…

c7b5d92

…l-sync

# Conflicts:
#	backend/FwLite/LcmCrdt.Tests/Changes/ChangeDeserializationRegressionData.latest.verified.txt
#	backend/FwLite/LcmCrdt.Tests/Changes/ChangeDeserializationRegressionData.legacy.verified.txt
#	backend/FwLite/LcmCrdt.Tests/Data/SnapshotDeserializationRegressionData.latest.verified.txt
#	backend/FwLite/LcmCrdt.Tests/Data/SnapshotDeserializationRegressionData.legacy.verified.txt
#	backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyDbModel.verified.txt
#	backend/FwLite/LcmCrdt/Migrations/LcmCrdtDbContextModelSnapshot.cs


          Point VARIANTS.md status at the 3-PR stack

054cf0d

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

Important

Review skipped

Draft detected.

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: 8d936804-98e9-4126-839d-8319ebcc1515

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/variants-model-sync

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.}

github-actions Bot added 💻 FW Lite 📦 Lexbox labels

myieye mentioned this pull request

Add variant support to FwLite (backend + viewer UI) #2410

Closed

argos-ci Bot commented Jul 6, 2026 •

edited

Loading

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

Build	Status	Details	Updated (UTC)
default (Inspect)	✅ No changes detected	-	Jul 7, 2026, 2:16 PM

myieye marked this pull request as draft

July 6, 2026 14:09

myieye and others added 4 commits

July 6, 2026 17:05


          Document Submit* purpose; complete variant Submit normalization

cf77661

Submit* are the sync's result-less fire-and-forget writes (delete-wins on a
concurrently-deleted target). The write-normalization wrapper's un-overridden
Submit* silently downgrade that to a throwing update via the interface default
body, so add SubmitCreateVariant + SubmitUpdateVariantType overrides (forward to
_api.Submit*), plus tests asserting normalization and Times.Never on the
returning method. Documents the rule in FwLite AGENTS.md and the interface.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>


          Tolerate concurrently-deleted variant types on the sync path

868dd53

Adding a variant type during sync (VariantSync → AddVariantType, and
SubmitCreateVariant for a whole link) threw when the type had been concurrently
deleted on the other replica, wedging the entire project sync. Both IMiniLcmApi
implementations now drop the missing type (delete wins), matching
Remove/MoveVariantType and AddVariantChange.NewEntity, which already tolerated
it. New conformance tests (both implementations) fail without the fix.

Also: exclude Variant.Types[].Order from the CanSyncRandomEntries round-trip
comparison — it was the only Order field not excluded, and the write path
normalizes it like every other orderable, which was the red CI. And correct
stale VARIANTS.md type-ordering docs + soften unverified FLEx-UI comments.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>


          Merge remote-tracking branch 'origin/develop' into feat/variants-mode…

49fed9d

…l-sync

# Conflicts:
#	backend/LfClassicData/LfClassicMiniLcmApi.cs


          Merge remote-tracking branch 'origin/develop' into feat/variants-mode…

a687970

…l-sync

myieye mentioned this pull request

Activity redesign: responsive split view, failure containment, review hardening (follow-on to #2388) #2434

Closed

hahn-kev reviewed

View reviewed changes

backend/FwLite/MiniLcm/Models/VariantTypeRef.cs

+              /// </summary>
+              public class VariantTypeRef : IOrderable
+              {
+                  public required virtual Guid Id { get; set; }

hahn-kev Jul 13, 2026

Collaborator

we should automate this, but it should be public virtual required

hahn-kev reviewed

View reviewed changes

backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

+                  /// </summary>
+                  internal void AddVariant(ILexEntry lexVariantEntry, Variant variant)
+                  {
+                      if (FindVariantRef(lexVariantEntry, variant) is not null) return; //idempotent, like the CRDT side

hahn-kev Jul 15, 2026

Collaborator

there should probably be an assert at the top that lexVariantEntry.Guid == variant.VariantEntryId to catch incorrect calls early

hahn-kev reviewed

View reviewed changes

backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs

+                              throw new InvalidOperationException($"Sense {variant.MainSenseId} does not belong to entry {variant.MainEntryId}, it belongs to {senseEntryId}");
+                      }
+                      var typeIds = variant.Types.Select(t => t.Id).ToArray();
+                      var knownTypeIds = (await repo.VariantTypes.Where(t => typeIds.Contains(t.Id)).Select(t => t.Id).ToArrayAsync()).ToHashSet();

hahn-kev Jul 15, 2026

Collaborator

by default there's very few types. Let's just simplify this to drop there Where then we can also drop typeIds

hahn-kev Jul 15, 2026

Collaborator

also, when bulk adding, this querry will be run once for each variant of each entry, we should probably lift it out so it only happens once on bulk insert like we do for a few other things.

hahn-kev reviewed

View reviewed changes

backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs

+                      await using var repo = await repoFactory.CreateRepoAsync();
+                      var existing = await repo.FindVariant(variant);
+                      if (existing is null) return;
+                      // type concurrently deleted on the other replica → drop the add (delete wins), matching Remove/MoveVariantType

hahn-kev Jul 15, 2026

Collaborator

not a useful comment

hahn-kev reviewed

View reviewed changes

backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs

+                      // type concurrently deleted on the other replica → drop the add (delete wins), matching Remove/MoveVariantType
+                      var variantType = await repo.VariantTypes.SingleOrDefaultAsync(vt => vt.Id == variantTypeId);
+                      if (variantType is null) return;
+                      await AddChange(new AddVariantTypeChange(existing.Id, new VariantTypeRef { Id = variantType.Id }, position));

hahn-kev Jul 15, 2026

Collaborator

should you use variantType.ToRef()?

hahn-kev reviewed

View reviewed changes

backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs

+                  {
+                      await using var repo = await repoFactory.CreateRepoAsync();
+                      var existing = await repo.FindVariant(variant);
+                      if (existing is null) return;

hahn-kev Jul 15, 2026

Collaborator

we should check that type type exists here too

hahn-kev reviewed

View reviewed changes

backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs

+                  {
+                      await using var repo = await repoFactory.CreateRepoAsync();
+                      var existing = await repo.FindVariant(variant);
+                      if (existing is null) return;

hahn-kev Jul 15, 2026

Collaborator

the type should exist here too

hahn-kev reviewed

View reviewed changes

backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs

+                  // wedge the whole project sync. The defaults forward to the returning method (correct for FwData, which still
+                  // surfaces a genuinely-missing object).
+                  //
+                  // A Submit variant is only needed where the sync diff writes to a target that could have been concurrently

hahn-kev Jul 15, 2026

Collaborator

I don't think this comment is useful, at least not here

hahn-kev reviewed

View reviewed changes

backend/FwLite/VARIANTS.md

		@@ -0,0 +1,414 @@
		# Variants in FieldWorks Lite — design & work log

hahn-kev Jul 15, 2026

Collaborator

are we going to leave this file in?

hahn-kev requested changes

View reviewed changes

hahn-kev left a comment

Collaborator

looks really good. I just have some questions and requests.

backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs

+                      var currentFwDataVariantTypes = await fwdataApi.GetVariantTypes().ToArrayAsync();
+                      // Legacy snapshots predate variant support and deserialize with an empty list; the CRDT
+                      // genuinely has no variant types yet, so that baseline correctly imports FwData's list.

hahn-kev Jul 15, 2026

Collaborator

not useful long term

backend/FwLite/LcmCrdt/Changes/Entries/AddVariantTypeChange.cs

+                      // Order is resolved from Between against live state at apply time (the ctor/serialized Order is not
+                      // authoritative), so concurrent adds converge. ReorderVariantTypeChange deliberately differs — it
+                      // stores a fixed Order — because a reorder targets a specific destination, an add just needs a slot.
+                      Order = OrderPicker.PickOrder(entity.Types, Between);

hahn-kev Jul 15, 2026

Collaborator

I don't see a point in having an Order property when we just override it always in apply change. What's it's purpose and why are we storing it in the change if it's not used?

backend/FwLite/MiniLcm.Tests/VariantTestsBase.cs

+                      variant.MainSenseId.Should().BeNull();
+                      variant.VariantHeadword.Should().Be("variant form");
+                      variant.MainHeadword.Should().Be("main entry");
+                      variant.Types.Should().BeEmpty();

hahn-kev Jul 15, 2026

Collaborator

hum, by default in FLEx variants have a default type of Unspecified Variant should we match that?

backend/FwLite/MiniLcm.Tests/VariantTestsBase.cs

+                      // LCM rejects circular component references (LexEntryRef.ValidateAddObjectInternal),
+                      // so both implementations must too
+                      await Api.CreateVariant(Variant.FromEntries(_variantEntry, _mainEntry));
+                      var act = async () => await Api.CreateVariant(Variant.FromEntries(_mainEntry, _variantEntry));

hahn-kev Jul 15, 2026

Collaborator

this test and similar only fail on CRDT because create tries to get the variant it just created, and that fails because the cycle check inside the change deletes the variant. I'm not sure this is the correct behavior, or at least I think the test should use a better signal than that. Probably out of scope for this PR.

backend/FwLite/MiniLcm.Tests/VariantTestsBase.cs

+                      var variant = await Api.CreateVariant(Variant.FromEntries(_variantEntry, _mainEntry));
+                      await Api.DeleteVariantType(type.Id);
+                      await Api.AddVariantType(variant, type.Id);

hahn-kev Jul 15, 2026

Collaborator

no change is actually recorded here since the method returns early when the type doesn't exist anymore. Should we have a dedicated test that directly records a change after the type is deleted to simulate a sync?

backend/FwLite/MiniLcm.Tests/VariantTestsBase.cs

+                  }
+                  [Fact]
+                  public async Task CreateVariant_CanTargetMultipleSensesOfSameEntry()

hahn-kev Jul 15, 2026

Collaborator

we've got code in Crdt that allows us to reference a sense Id which does not exist. Where do we test that?

backend/FwLite/MiniLcm.Tests/VariantTestsBase.cs

+                      var variantType = new VariantType() { Id = Guid.NewGuid(), Name = new() { { "en", "test" } } };
+                      await Api.CreateVariantType(variantType);
+                      var types = await Api.GetVariantTypes().ToArrayAsync();
+                      types.Should().ContainSingle(t => t.Id == variantType.Id);

hahn-kev Jul 15, 2026

Collaborator

need to test the queried type name

backend/FwLite/LcmCrdt.Tests/MiniLcmTests/VariantTests.cs

+                  public async Task Create_ChangingProperty_ProducesNewEntityId()
+                  {
+                      // When the sync diff detects a property change (e.g. MainEntryId), it does
+                      // remove + add. The "add" reuses the same input object with the old entity ID.

hahn-kev Jul 15, 2026

Collaborator

it's not clear, does that mean this test expects that first variant has been deleted? or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

💻 FW Lite 📦 Lexbox