Discover Copy()-test types by reflection instead of CRDT registration

[myieye]

EntityCopyMethodTests drove its theory from crdtConfig.ObjectTypes (the CRDT sync registration), which lists only root entities. Owned types that define their own Copy() — Translation, ViewField, ViewWritingSystem, MultiString, RichMultiString, RichString, RichSpan — were therefore only covered transitively (via their owners), and a brand-new model could be left off entirely.

This switches discovery to reflection over the MiniLcm assembly for any concrete type that declares a parameterless Copy() (DeclaredOnly, so inherit-only subclasses such as the private RichStringPrimitive JSON-converter helper are excluded). Coverage now tracks the Copy() contract itself rather than a registration list that can drift. The row count goes 11 → 18, with the owned types now verified as direct, named cases.

Split out of #2322, whose doc comment already points at this test.

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors entity copy validation testing in EntityCopyMethodTests.cs. The test now discovers entity types via reflection instead of CRDT configuration, and invokes the Copy method through reflection rather than direct calls. This decouples the test from the CRDT kernel configuration.

Changes

Entity Copy Method Test Refactoring

Layer / File(s)	Summary
Type discovery via reflection backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs	GetEntityTypes() now discovers entity types by reflecting over the assembly containing IObjectWithId to find concrete non-abstract classes with a public instance parameterless Copy method. New CopyMethod(Type) helper locates that method. Using directives updated to remove SIL.Harmony.Resource dependency.
Test copy invocation via reflection backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs	Parameterized test now invokes Copy via CopyMethod(type)!.Invoke(entity, null)! instead of direct entity.Copy() calls.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

💻 FW Lite

Suggested reviewers

• rmunn
• hahn-kev

Poem

A rabbit reflects on the code with care,
Discovering types dancing through the air,
No CRDT chains to weigh it down—
Reflection sets the testing crown! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% 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 accurately summarizes the main change: switching from CRDT registration to reflection-based discovery for test type enumeration.
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 the rationale for switching from CRDT registration-based test discovery to reflection-based discovery, mentions specific owned types affected, and documents the test coverage improvement.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch entity-copy-test-auto-discover

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

UI unit Tests

  1 files   59 suites   30s ⏱️
177 tests 177 ✅ 0 💤 0 ❌
246 runs  246 ✅ 0 💤 0 ❌

Results for commit 985d58a.

♻️ This comment has been updated with latest results.

[argos-ci]

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

Build	Status	Details	Updated (UTC)
default (Inspect)	✅ No changes detected	-	Jun 2, 2026, 10:51 AM

[coderabbitai]

🧹 Nitpick comments (1)

backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs (1)

19-26: ⚡ Quick win

Tighten discovery to declared Copy() implementations.

Line 20 / Line 26 currently match any visible public parameterless Copy, including inherited methods. That makes the test set broader than the stated "types that declare their own Copy()" contract and can pull in future classes whose Copy() is inherited or shaped differently.

Suggested change

     public static IEnumerable<object[]> GetEntityTypes()
     {
         return typeof(IObjectWithId).Assembly.GetTypes()
             .Where(t => t is { IsClass: true, IsAbstract: false } && CopyMethod(t) is not null)
             .Select(t => new object[] { t });
     }

     private static MethodInfo? CopyMethod(Type type)
     {
-        return type.GetMethod("Copy", BindingFlags.Public | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null);
+        var method = type.GetMethod(
+            "Copy",
+            BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly,
+            binder: null,
+            Type.EmptyTypes,
+            modifiers: null);
+
+        return method?.ReturnType == type ? method : null;
     }

🤖 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/LcmCrdt.Tests/EntityCopyMethodTests.cs` around lines 19 - 26,
The discovery currently picks up inherited public parameterless Copy() methods;
update the CopyMethod(Type type) so it only returns a Copy declared on the type
itself (e.g., use BindingFlags.DeclaredOnly together with BindingFlags.Public |
BindingFlags.Instance when calling GetMethod, or alternatively locate
parameterless methods named "Copy" and ensure method.DeclaringType == type) so
only types that declare their own Copy() are included by the test.

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

Nitpick comments:
In `@backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs`:
- Around line 19-26: The discovery currently picks up inherited public
parameterless Copy() methods; update the CopyMethod(Type type) so it only
returns a Copy declared on the type itself (e.g., use BindingFlags.DeclaredOnly
together with BindingFlags.Public | BindingFlags.Instance when calling
GetMethod, or alternatively locate parameterless methods named "Copy" and ensure
method.DeclaringType == type) so only types that declare their own Copy() are
included by the test.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9edc9692-a92e-48e3-9af2-bb33e80412e5

📥 Commits

Reviewing files that changed from the base of the PR and between 4afa868 and 7c3e978.

📒 Files selected for processing (1)

• backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs

[myieye]

Closing — this adds no real coverage over develop's existing test.

EntityCopyMethodTests on develop already enumerates every CRDT root (crdtConfig.ObjectTypes) and tests each directly. AutoFaker fully populates them (RepeatCount=5) and AssertDeepCopy recurses, so every owned type's Copy() is already exercised transitively — Translation, MultiString, RichMultiString, RichString, RichSpan, ViewField, ViewWritingSystem are all reachable from a root. Roots are self-enforcing (you can't ship a syncable entity without registering it, which auto-adds it to the test), and children must live in the graph to be reachable, so nothing can silently slip through.

The reflection-based discovery here only converts transitive coverage into direct rows, at the cost of extra machinery and a RichStringPrimitive edge case the original never had. Keeping develop's simpler, more elegant approach.