Add variant support to FwLite (backend + viewer UI)#2410
Closed
myieye wants to merge 21 commits into
Closed
Conversation
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>
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>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
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>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…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>
Variant of / Variants entry fields with per-link variant-type menu, demo data seed, i18n strings, Playwright coverage. New variants default to Unspecified Variant like FLEx. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
💻 FW Lite
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds Variant and VariantType support across backend, sync, FwData bridge, frontend, demo data, and tests. The PR introduces new models and APIs, persists and syncs variant links and types, exposes them in the viewer, and updates snapshots, fixtures, and localization. ChangesVariant Support
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎ Awaiting the start of a new Argos build… |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
frontend/viewer/src/lib/entry-editor/field-editors/VariantTypesMenuItems.svelte (1)
19-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMutating
variantprop directly instead of using a callback/bindable.
toggleTypereassignsvariant.typeson the passed-in prop object rather than emitting the new value; it relies on the caller reassigning the outer array to force re-render (see the workaround comments inVariantOf.svelte/Variants.svelte). Consider makingvariant$bindable()or returning the updated types viaonchangeto make the data flow explicit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/viewer/src/lib/entry-editor/field-editors/VariantTypesMenuItems.svelte` around lines 19 - 32, The `toggleType` handler in `VariantTypesMenuItems.svelte` is mutating the passed-in `variant` prop directly by reassigning `variant.types`, which makes the data flow implicit and depends on parent-side rerenders. Update the component to use an explicit binding/callback pattern instead: make `variant` bindable or have `toggleType` emit the updated types through `onchange` so the parent owns the state update. Keep the existing `hasType` and `toggleType` symbols as the main places to adjust the flow.frontend/viewer/src/lib/entry-editor/field-editors/VariantOf.svelte (1)
1-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate logic with
Variants.svelte.
VariantOf.svelteandVariants.svelteshare almost identical structure (props shape,addVariant*construction,disableEntry/disableSense), differing only in which side (mainEntryId/variantEntryId) is fixed to the current entry. Consider extracting a shared parameterized component/composable to avoid drift between the two as future changes land.
[medium]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/viewer/src/lib/entry-editor/field-editors/VariantOf.svelte` around lines 1 - 62, `VariantOf.svelte` duplicates nearly the same props, `addVariantOf`, `disableEntry`, and `disableSense` logic found in `Variants.svelte`; refactor this into a shared parameterized component or composable so the only difference is which side of the relationship is anchored to the current entry. Keep the existing behavior in `addVariantOf`, `disableEntry`, and `disableSense`, but move the common `IVariant` construction and entry/sense disabling rules into reusable logic to prevent future drift between the two editors.backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs (1)
145-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
MainSenseIdis always nulled — sense-scoped variant links are never exercised by this fixture.The analogous
CreateComplexFormComponentEntryhelper actually creates a matchingSenseon the linked entry whenComponentSenseIdis set, so that scenario is exercised in tests.CreateVariantLinkEntryinstead unconditionally nullsMainSenseId, meaning any test relying on this shared faker helper (e.g.VariantTests.cs,VariantSyncTests.cs,VariantTestsBase.cs) will never generate/roundtrip a variant that targets a specific main-entry sense.Consider mirroring the complex-form-component pattern: when
MainSenseIdis set, create a matchingSenseon the created main entry instead of always clearing it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs` around lines 145 - 190, `CreateVariantLinkEntry` is clearing `MainSenseId` unconditionally, so sense-scoped variant links are never created in this faker. Update the helper to mirror `CreateComplexFormComponentEntry`: when a variant has `MainSenseId` set, create or attach a matching `Sense` on the linked main entry before creating the entry, and only null it when no sense target exists. Keep the fix localized in `CreateVariantLinkEntry` so tests using this shared helper can exercise and roundtrip sense-specific variant links.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/FwLite/LcmCrdt/LcmCrdtKernel.cs`:
- Around line 328-353: Refresh embedded variant type copies when a VariantType
is renamed, because JsonPatchChange<VariantType> currently updates only the
canonical record while Variant.Types JSONB snapshots remain stale. Update the
rename flow in the VariantType handling path so that, after changing the type
name, any Variant rows referencing it are rewritten and their Types payload
reserialized with the new Name; use the VariantType and Variant
mapping/configuration in LcmCrdtKernel to locate the affected entities.
In `@backend/FwLite/VARIANTS.md`:
- Around line 85-98: The Variant sketch’s Comment field is modeled too loosely
compared with the implementation in Variant.cs. Update the Variant record
definition in VARIANTS.md to match the actual model by making Comment
non-nullable RichMultiString and reflecting that it has a default instance, so
the sketch no longer implies unnecessary null handling.
In `@frontend/viewer/src/lib/entry-editor/field-editors/VariantOf.svelte`:
- Around line 50-61: The VariantOf.svelte picker currently blocks only direct
reverse variant links in disableEntry/disableSense, but it still allows
selections that create a cycle through an existing complex-form relationship.
Update the selection guards in disableEntry and disableSense to also reject any
entry/sense that would introduce a mixed complex-form cycle, using the existing
entry.variantOf and entry.variants checks as the starting point and extending
them to cover indirect cyclic paths before the user can select them.
---
Nitpick comments:
In `@backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs`:
- Around line 145-190: `CreateVariantLinkEntry` is clearing `MainSenseId`
unconditionally, so sense-scoped variant links are never created in this faker.
Update the helper to mirror `CreateComplexFormComponentEntry`: when a variant
has `MainSenseId` set, create or attach a matching `Sense` on the linked main
entry before creating the entry, and only null it when no sense target exists.
Keep the fix localized in `CreateVariantLinkEntry` so tests using this shared
helper can exercise and roundtrip sense-specific variant links.
In `@frontend/viewer/src/lib/entry-editor/field-editors/VariantOf.svelte`:
- Around line 1-62: `VariantOf.svelte` duplicates nearly the same props,
`addVariantOf`, `disableEntry`, and `disableSense` logic found in
`Variants.svelte`; refactor this into a shared parameterized component or
composable so the only difference is which side of the relationship is anchored
to the current entry. Keep the existing behavior in `addVariantOf`,
`disableEntry`, and `disableSense`, but move the common `IVariant` construction
and entry/sense disabling rules into reusable logic to prevent future drift
between the two editors.
In
`@frontend/viewer/src/lib/entry-editor/field-editors/VariantTypesMenuItems.svelte`:
- Around line 19-32: The `toggleType` handler in `VariantTypesMenuItems.svelte`
is mutating the passed-in `variant` prop directly by reassigning
`variant.types`, which makes the data flow implicit and depends on parent-side
rerenders. Update the component to use an explicit binding/callback pattern
instead: make `variant` bindable or have `toggleType` emit the updated types
through `onchange` so the parent owns the state update. Keep the existing
`hasType` and `toggleType` symbols as the main places to adjust the flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cb9b7020-0708-4d77-b731-0b165799ebba
📒 Files selected for processing (103)
backend/FwLite/FwDataMiniLcmBridge.Tests/MiniLcmTests/VariantTests.csbackend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.csbackend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateVariantProxy.csbackend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateVariantTypeProxy.csbackend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.csbackend/FwLite/FwLiteProjectSync.Tests/Import/ResumableTests.csbackend/FwLite/FwLiteProjectSync.Tests/Snapshots/sena-3_snapshot.2026-06-18.verified.txtbackend/FwLite/FwLiteProjectSync.Tests/SyncTests.csbackend/FwLite/FwLiteProjectSync.Tests/UpdateDiffTests.csbackend/FwLite/FwLiteProjectSync.Tests/VariantSyncTests.csbackend/FwLite/FwLiteProjectSync.Tests/sena-3-live.verified.sqlitebackend/FwLite/FwLiteProjectSync.Tests/sena-3-live_snapshot.verified.txtbackend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.csbackend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.csbackend/FwLite/FwLiteProjectSync/Import/ResumableImportApi.csbackend/FwLite/FwLiteShared/Services/MiniLcmApiNotifyWrapper.csbackend/FwLite/FwLiteShared/Services/MiniLcmJsInvokable.csbackend/FwLite/FwLiteWeb/Hubs/MiniLcmApiHubBase.csbackend/FwLite/LcmCrdt.Tests/Changes/ChangeDeserializationRegressionData.latest.verified.txtbackend/FwLite/LcmCrdt.Tests/Changes/UseChangesTests.csbackend/FwLite/LcmCrdt.Tests/Changes/VariantChangeTests.csbackend/FwLite/LcmCrdt.Tests/Data/MigrationTests.csbackend/FwLite/LcmCrdt.Tests/Data/MigrationTests_FromScriptedDb.v1.ProjectSnapshot.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/MigrationTests_FromScriptedDb.v1.Snapshots.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/MigrationTests_FromScriptedDb.v2.ProjectSnapshot.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/MigrationTests_FromScriptedDb.v2.Snapshots.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/SnapshotDeserializationRegressionData.latest.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/SnapshotDeserializationRegressionData.legacy.verified.txtbackend/FwLite/LcmCrdt.Tests/Data/VerifyRegeneratedSnapshotsAfterMigrationFromScriptedDb.v1.verified.jsonbackend/FwLite/LcmCrdt.Tests/Data/VerifyRegeneratedSnapshotsAfterMigrationFromScriptedDb.v2.verified.jsonbackend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyChangeModels.verified.txtbackend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyDbModel.verified.txtbackend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyIObjectWithIdModels.verified.txtbackend/FwLite/LcmCrdt.Tests/MiniLcmTests/VariantTests.csbackend/FwLite/LcmCrdt.Tests/SnapshotAtCommitServiceTests.csbackend/FwLite/LcmCrdt/Changes/CreateVariantType.csbackend/FwLite/LcmCrdt/Changes/Entries/AddVariantChange.csbackend/FwLite/LcmCrdt/Changes/Entries/AddVariantTypeChange.csbackend/FwLite/LcmCrdt/Changes/Entries/RemoveVariantTypeChange.csbackend/FwLite/LcmCrdt/CrdtMiniLcmApi.csbackend/FwLite/LcmCrdt/Data/EntryQueryHelpers.csbackend/FwLite/LcmCrdt/Data/MiniLcmRepository.csbackend/FwLite/LcmCrdt/LcmCrdtDbContext.csbackend/FwLite/LcmCrdt/LcmCrdtKernel.csbackend/FwLite/LcmCrdt/Migrations/20260703134525_AddVariants.Designer.csbackend/FwLite/LcmCrdt/Migrations/20260703134525_AddVariants.csbackend/FwLite/LcmCrdt/Migrations/LcmCrdtDbContextModelSnapshot.csbackend/FwLite/LcmCrdt/QueryHelpers.csbackend/FwLite/LcmCrdt/Templates/blank-project-template.jsonbackend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.csbackend/FwLite/MiniLcm.Tests/FluentAssertGlobalConfig.csbackend/FwLite/MiniLcm.Tests/Validators/VariantTypeValidationTests.csbackend/FwLite/MiniLcm.Tests/Validators/VariantValidationTests.csbackend/FwLite/MiniLcm.Tests/VariantTestsBase.csbackend/FwLite/MiniLcm/CreateEntryOptions.csbackend/FwLite/MiniLcm/IMiniLcmReadApi.csbackend/FwLite/MiniLcm/IMiniLcmWriteApi.csbackend/FwLite/MiniLcm/Import/ProjectImporter.csbackend/FwLite/MiniLcm/MiniLcmApiExtensions.csbackend/FwLite/MiniLcm/Models/Entry.csbackend/FwLite/MiniLcm/Models/IObjectWithId.csbackend/FwLite/MiniLcm/Models/Variant.csbackend/FwLite/MiniLcm/Models/VariantType.csbackend/FwLite/MiniLcm/Normalization/MiniLcmApiWriteNormalizationWrapper.csbackend/FwLite/MiniLcm/ProjectSnapshot.csbackend/FwLite/MiniLcm/SyncHelpers/EntrySync.csbackend/FwLite/MiniLcm/SyncHelpers/VariantSync.csbackend/FwLite/MiniLcm/SyncHelpers/VariantTypeSync.csbackend/FwLite/MiniLcm/Validators/EntryValidator.csbackend/FwLite/MiniLcm/Validators/MiniLcmApiValidationWrapper.csbackend/FwLite/MiniLcm/Validators/MiniLcmValidators.csbackend/FwLite/MiniLcm/Validators/VariantTypeValidator.csbackend/FwLite/MiniLcm/Validators/VariantValidator.csbackend/FwLite/VARIANTS.mdbackend/LfClassicData/LfClassicMiniLcmApi.csfrontend/viewer/src/lib/dotnet-types/generated-types/FwLiteShared/Services/IMiniLcmJsInvokable.tsfrontend/viewer/src/lib/dotnet-types/generated-types/MiniLcm/Models/IEntry.tsfrontend/viewer/src/lib/dotnet-types/generated-types/MiniLcm/Models/IVariant.tsfrontend/viewer/src/lib/dotnet-types/generated-types/MiniLcm/Models/IVariantType.tsfrontend/viewer/src/lib/dotnet-types/index.tsfrontend/viewer/src/lib/entry-editor/EntryOrSenseItemList.sveltefrontend/viewer/src/lib/entry-editor/field-editors/VariantOf.sveltefrontend/viewer/src/lib/entry-editor/field-editors/VariantTypesMenuItems.sveltefrontend/viewer/src/lib/entry-editor/field-editors/Variants.sveltefrontend/viewer/src/lib/entry-editor/object-editors/EntryEditorPrimitive.sveltefrontend/viewer/src/lib/utils.tsfrontend/viewer/src/lib/views/entity-config.tsfrontend/viewer/src/lib/views/view-data.tsfrontend/viewer/src/locales/en.pofrontend/viewer/src/locales/es.pofrontend/viewer/src/locales/fr.pofrontend/viewer/src/locales/id.pofrontend/viewer/src/locales/ko.pofrontend/viewer/src/locales/ms.pofrontend/viewer/src/locales/sw.pofrontend/viewer/src/locales/vi.pofrontend/viewer/src/project/data/index.tsfrontend/viewer/src/project/data/variant-types.tsfrontend/viewer/src/project/demo/demo-entry-data.tsfrontend/viewer/src/project/demo/in-memory-demo-api.tsfrontend/viewer/src/stories/editor/entity-primitives/entry-editor-primitive.stories.sveltefrontend/viewer/tests/entry-api-helper.tsfrontend/viewer/tests/variants.test.ts
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- 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>
…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>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hahn-kev
reviewed
Jul 6, 2026
hahn-kev
reviewed
Jul 6, 2026
hahn-kev
left a comment
hahn-kev
left a comment
Collaborator
There was a problem hiding this comment.
not yet done, but there's some stuff to do still
- 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>
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>
- Entry list rows show a muted "Variant" badge next to the headword, with the main entries in the tooltip - The dictionary preview renders "var. of X" (classic) / "variant of X" (lite) after a variant''s headword, print-dictionary style - Custom views already inherit the variantOf/variants toggles via entityConfig; locked with a contract test - i18n extracted with translator context; Playwright coverage for badge and preview Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 — its type no longer exists),
DbModel/ChangeModels snapshots, sena-3-live snapshot, TS types
(IVariantTypeRef).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Superseded by a 3-PR stack for chunked review and merge (content-identical: the stack top diffs empty against this branch merged with develop):
All review feedback here is implemented on the stack (see the inline replies for commit refs), including the per-item variant type ordering rework. Closing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cumulative PR for the variants feature — the backend step reviewed to green in #2408 (CodeRabbit handled, Devin clean, CI green), the viewer UI, plus post-#2408 hardening that still needs human eyes:
0d03aed28aligns cycle detection with liblcm's exactAllComponentssemantics and changes replay behavior of the shippedAddEntryComponentChange(justification: VARIANTS.md decision 7; skew story: VARIANTS.md rollout notes), along with3b05ad8c1/5d6b0c050(validator tests, docs) and follow-up test hardening. Part of an iterative review workflow; steps get merged one by one once everything is reviewed.Backend (#2408 + the commits above): one MiniLcm
Variantlink = one FieldWorksLexEntryRef(RefType=Variant) target pair with per-linkTypes/HideMinorEntry/Commentfor lossless FLEx round-trip;VariantTypeas a first-class synced object; cycle guard mirrors liblcm's combined complex-form+variant acyclicity including its sense-target no-recurse rule; per-link edits split FLEx's shared multi-target refs before mutating; flattened type reads expose the Irregularly Inflected Form subtypes (forum thread); Sena-3 verified data regenerated. Full design log:backend/FwLite/VARIANTS.md.UI: "Variant of" and "Variants" entry-editor fields with a per-link "Variant type" checkbox menu; new links default to Unspecified Variant like FLEx; demo project seeded; Playwright covers both directions plus the type toggle.
HideMinorEntry/Commentare synced but deliberately have no editor UI yet.Tests: conformance suites on both
IMiniLcmApiimplementations, including a seeded random-graph differential test against a port of LCM'sAllComponentsoracle, and targeted mutation checks that the new cycle/shared-ref tests actually kill faults.Rollout: new Harmony change types — deploy FwHeadless before/with a client release that writes variants. The
AddEntryComponentChangesemantics change rides along on any deploy and affects complex-form projects during version skew (rare shape, converges on auto-update — see VARIANTS.md rollout notes). Known gap: pre-existing FwLite-only projects have no variant types seeded, so their new links are untyped (follow-up).🤖 Generated with Claude Code