Skip to content

Refactor migration chain execution with unified VersionedMigrationRunner#260

Merged
GeWuYou merged 5 commits into
mainfrom
feat/data-repository-persistence
Apr 20, 2026
Merged

Refactor migration chain execution with unified VersionedMigrationRunner#260
GeWuYou merged 5 commits into
mainfrom
feat/data-repository-persistence

Conversation

@GeWuYou

@GeWuYou GeWuYou commented Apr 20, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

发布说明

  • Bug Fixes

    • 强化迁移注册与执行校验,防止声明/结果版本不一致或重复注册导致的数据损坏
    • 确保迁移失败或并发注册不会污染已存档数据,并改善加载时错误提示
  • Documentation

    • 明确序列化器配置的生命周期与并发约束,推荐在启动时统一配置
    • 细化迁移规则与失败处理行为说明
  • Tests

    • 新增迁移正确性与并发场景测试覆盖
  • Refactor

    • 统一迁移执行路径以一致化验证和失败处理

GeWuYou added 2 commits April 20, 2026 09:36
- 补充 JsonSerializer 对 settings 与 converters 生命周期的 XML 注释

- 更新序列化文档与 README,修正线程安全和组合根配置说明

- 新增 JsonSerializer 配置实例暴露契约测试,并回写 data-repository-persistence 跟踪
- 新增 internal 迁移执行器,统一 settings 与 save 的链式版本校验

- 修复 SettingsModel 重复注册、缺链回填与目标版本判定的迁移约束

- 补充 Persistence 与 SettingsModel 定向测试,并更新迁移文档和 ai-plan 跟踪
@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@GeWuYou has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 46 minutes and 55 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 46 minutes and 55 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cf89079f-b7d1-41be-9932-a7a8b572937a

📥 Commits

Reviewing files that changed from the base of the PR and between ec3de5b and 5353d5b.

📒 Files selected for processing (4)
  • GFramework.Game.Tests/Setting/SettingsModelTests.cs
  • GFramework.Game/Setting/SettingsModel.cs
  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
  • ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md
📝 Walkthrough

Walkthrough

引入内部工具类 VersionedMigrationRunner 统一执行版本迁移并集中校验,重构 SaveRepository<T>SettingsModel 使用该运行器,强化迁移注册/运行时一致性与并发快照语义,并新增针对迁移结果版本不匹配和并发注册不影响正在执行载入的持久化测试。

Changes

Cohort / File(s) Summary
迁移基础设施重构
GFramework.Game/Internal/VersionedMigrationRunner.cs
新增内部静态类,提供 ValidateForwardOnlyRegistration 和泛型 MigrateToTargetVersion<TData, TMigration>,集中执行按版本步进迁移并抛出缺链、返回 null、版本不匹配、回退或越界等错误。
持久化层迁移重构
GFramework.Game/Data/SaveRepository.cs
移除内联逐步迁移循环与逐项检查,注册时使用 ValidateForwardOnlyRegistration(...),迁移执行改为快照 _migrations 并委托 VersionedMigrationRunner.MigrateToTargetVersion(...)
设置模型迁移重构
GFramework.Game/Setting/SettingsModel.cs
RegisterMigration 增加非空检查、前向验证与重复注册拒绝(改为 TryAdd 后抛出);InitializeAsync/MigrateIfNeeded 改为以运行时内存实例为目标并委托 VersionedMigrationRunner;新增 ApplySettingsMigration 进行单步结果类型与兼容性校验。
持久化测试新增(含阻塞与不匹配场景)
GFramework.Game.Tests/Data/PersistenceTests.cs
新增两项异步测试:一项断言当迁移声明 ToVersion=2 但返回数据 Version=3LoadAsyncInvalidOperationException 且持久化文件未被污染;另一项使用阻塞迁移协同测试在进行中注册后续迁移不会影响正在进行的载入快照;新增测试迁移类 BlockingSaveMigrationV1ToV2TestSaveMigrationV1ToV2ReturningV3
序列化器文档与测试
GFramework.Game/Serializer/JsonSerializer.cs, GFramework.Game.Tests/Serializer/JsonSerializerTests.cs, GFramework.Game/README.md
文档化 JsonSerializer 直接复用传入 JsonSerializerSettings(含 Converters)且共享后不得再变更;新增单元测试验证 SettingsConverters 返回原始实例引用;README 示例调整为在组合根构建并配置序列化器。
设置模型测试补充
GFramework.Game.Tests/Setting/SettingsModelTests.cs
新增测试覆盖缓存设置在注册迁移后被重新加载/迁移的场景、重复迁移注册时抛 InvalidOperationException,以及不完整迁移链时保留当前默认实例行为;新增测试类型 TestLatestSettingsData 与迁移 TestLatestSettingsMigrationV1ToV2
文档与本地化指南更新
docs/zh-CN/game/*.md
强化迁移规则说明:每个 FromVersion 仅能注册一次、ToVersion 必须大于 FromVersion、迁移返回数据 Version 必须精确匹配声明目标;调整序列化器配置生命周期与安全默认(TypeNameHandling.None)等最佳实践描述。
规划与跟踪文档
ai-plan/public/data-repository-persistence/...
更新跟踪与任务文档,记录采用 VersionedMigrationRunner 的收敛、测试覆盖与后续编解码/管道边界评估待办事项。

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Pull request title clearly and concisely summarizes the main change: introducing a unified VersionedMigrationRunner to refactor migration chain execution, which is the core objective across all modified files.

✏️ 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 feat/data-repository-persistence

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

Copy link
Copy Markdown

Summary

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Other ❓ Flaky 🍂 Duration ⏱️
2107    ↑4 2107    ↑4 0 0 0 0 36.5s    ↑486ms

Test Results

passed 2107 passed ↑4

Details

tests 2107 tests ↑4
clock 36.5s ↑486ms
tool nunit
build CI - Build & Test arrow-right build-and-test link #895
pull-request Feat/data repository persistence link #260

Insights

Average Tests per Run Total Flaky Tests Total Failed Slowest Test (p95)
1955 0 0 4.2s

build-and-test: Run #895

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Pending ⏳ Other ❓ Flaky 🍂 Duration ⏱️
2107 2107 0 0 0 0 0 36.5s

🎉 All tests passed!

Slowest Tests

Test 📝 Results 📊 Duration (avg) ⏱️ Duration (p95) ⏱️
SendRequestAsync_Should_ResolveCqrsRuntime_OnlyOnce_When_AccessedConcurrently 2 3.9s 4.2s
Does_Not_Report_When_FieldInjectedModel_Is_Registered 4 2.7s 3.5s
Generates_Scene_Behavior_Boilerplate 2 1.9s 2.0s
CleanupDuringAcquire_Should_NotCauseRaceCondition 4 1.1s 1.1s
Append_ShouldNotBlock 4 1.0s 1.0s
Context_Caching_Should_Improve_Performance 4 779ms 788ms
Snapshot_SchemaConfigGenerator 4 348ms 714ms
PendingCount_ShouldReflectQueuedEntries 4 501ms 501ms
Cleanup_Should_NotRemoveActiveLocks 4 406ms 411ms
Cleanup_Should_RemoveUnusedLocks 4 402ms 404ms

± Comparison with run #893 at f5375c1 | 🎉 No failed tests detected across all runs. | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 4 runs.

Github Test Reporter by CTRF 💚

@github-actions

github-actions Bot commented Apr 20, 2026

Copy link
Copy Markdown

⚠️MegaLinter analysis: Success with warnings

Descriptor Linter Files Fixed Errors Warnings Elapsed time
⚠️ CSHARP dotnet-format yes 1 no 5.29s
✅ REPOSITORY gitleaks yes no no 6.47s
✅ REPOSITORY trufflehog yes no no 6.25s

Detailed Issues

⚠️ CSHARP / dotnet-format - 1 error
Welcome to .NET 9.0!
---------------------
SDK Version: 9.0.114

----------------
Installed an ASP.NET Core HTTPS development certificate.
To trust the certificate, run 'dotnet dev-certs https --trust'
Learn about HTTPS: https://aka.ms/dotnet-https

----------------
Write your first app: https://aka.ms/dotnet-hello-world
Find out what's new: https://aka.ms/dotnet-whats-new
Explore documentation: https://aka.ms/dotnet-docs
Report issues and find source on GitHub: https://github.com/dotnet/core
Use 'dotnet --help' to see available commands or visit: https://aka.ms/dotnet-cli
--------------------------------------------------------------------------------------
Unhandled exception: System.Exception: Restore operation failed.
   at Microsoft.CodeAnalysis.Tools.CodeFormatter.OpenMSBuildWorkspaceAsync(String solutionOrProjectPath, WorkspaceType workspaceType, Boolean noRestore, Boolean requiresSemantics, String binaryLogPath, Boolean logWorkspaceWarnings, ILogger logger, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.Tools.CodeFormatter.FormatWorkspaceAsync(FormatOptions formatOptions, ILogger logger, CancellationToken cancellationToken, String binaryLogPath)
   at Microsoft.CodeAnalysis.Tools.FormatCommandCommon.FormatAsync(FormatOptions formatOptions, ILogger`1 logger, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.Tools.Commands.RootFormatCommand.FormatCommandDefaultHandler.InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
   at System.CommandLine.Invocation.InvocationPipeline.InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)

See detailed reports in MegaLinter artifacts
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security
Show us your support by starring ⭐ the repository

@GeWuYou

GeWuYou commented Apr 20, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
docs/zh-CN/game/index.md (1)

686-693: ⚠️ Potential issue | 🟠 Major

TypeNameHandling.Auto 配置添加 SerializationBinder 白名单。

这段示例中的 GameDataSerializer 用于序列化/反序列化游戏存档数据(存储到磁盘),而游戏存档本质上是可篡改的——用户可以直接编辑磁盘上的 JSON 文件。在处理这类不可信数据时,启用 TypeNameHandling.Auto 而不配置受限的 SerializationBinder 会引入反序列化攻击风险。

建议改为以下方式之一:

  1. 禁用多态类型处理(推荐用于游戏存档):改为 TypeNameHandling = TypeNameHandling.None,仅在明确知道具体类型时使用。

  2. 实现类型白名单(如需多态):添加自定义 SerializationBinder 来约束允许的类型集合,参考官方模式:

public class SafeGameDataBinder : ISerializationBinder
{
    private static readonly HashSet<Type> AllowedTypes = new() 
    { 
        typeof(SaveData), typeof(PlayerData), typeof(Vector2), typeof(Color),
        // 仅列出游戏实际使用的类型
    };

    public Type BindToType(string assemblyName, string typeName)
    {
        var type = Type.GetType($"{typeName}, {assemblyName}");
        return AllowedTypes.Contains(type) ? type : null;
    }

    public void BindToName(Type serializedType, out string assemblyName, out string typeName)
    {
        assemblyName = null;
        typeName = serializedType.Name;
    }
}

然后在示例中配置:SerializationBinder = new SafeGameDataBinder()

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/zh-CN/game/index.md` around lines 686 - 693, The current
GameDataSerializer initializes _serializer with TypeNameHandling.Auto which
allows unsafe polymorphic deserialization of user-editable save files; fix by
either setting TypeNameHandling = TypeNameHandling.None for save data or, if
polymorphism is required, implement and assign a whitelist SerializationBinder
(e.g. SafeGameDataBinder implementing ISerializationBinder that only returns
types from an allowed set like SaveData, PlayerData, Vector2, etc.) and set
SerializationBinder = new SafeGameDataBinder() when constructing the
JsonSerializer; update the GameDataSerializer initialization to use one of these
safe options.
GFramework.Game/Setting/SettingsModel.cs (1)

109-135: ⚠️ Potential issue | 🟡 Minor

补齐 RegisterMigration 的新增异常契约文档。

该公共方法现在显式抛出 null、非前向版本和重复注册错误,但 XML docs 还没有记录这些异常。As per coding guidelines, “All public, protected, and internal types and members MUST include XML documentation comments (///) with ... <exception> ... where applicable.”

📝 建议补充 XML exception 文档
     /// <returns>
     ///     返回当前 ISettingsModel 实例,支持链式调用。
     /// </returns>
+    /// <exception cref="ArgumentNullException">
+    ///     <paramref name="migration" /> 为 <see langword="null" /> 时抛出。
+    /// </exception>
+    /// <exception cref="ArgumentException">
+    ///     迁移声明的目标版本不大于源版本时抛出。
+    /// </exception>
+    /// <exception cref="InvalidOperationException">
+    ///     同一设置类型与源版本已注册迁移器时抛出。
+    /// </exception>
     public ISettingsModel RegisterMigration(ISettingsMigration migration)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game/Setting/SettingsModel.cs` around lines 109 - 135, The
RegisterMigration method's XML docs are missing <exception> entries for the
errors it can throw; update the method's XML comments (above RegisterMigration)
to include <exception> tags documenting ArgumentNullException thrown by
ArgumentNullException.ThrowIfNull when migration is null, the exception thrown
by VersionedMigrationRunner.ValidateForwardOnlyRegistration when a non-forward
(invalid) migration version pair is provided (document the specific exception
type it throws and a short description), and InvalidOperationException thrown
when _migrations.TryAdd fails due to a duplicate migration registration;
reference the symbols RegisterMigration, ISettingsMigration,
ArgumentNullException.ThrowIfNull,
VersionedMigrationRunner.ValidateForwardOnlyRegistration, _migrations.TryAdd,
InvalidOperationException, and _migrationCache.TryRemove in the descriptions so
readers can find the related code.
🧹 Nitpick comments (2)
GFramework.Game.Tests/Data/PersistenceTests.cs (1)

197-222: 补一个“失败后未污染存档”的断言。

这个用例已经能证明版本不匹配会抛异常;建议再读取原槽位,确认错误的 v3 迁移结果没有被持久化,避免未来实现顺序调整导致“先写入再抛错”的数据污染回归。

建议补充断言
         var exception = Assert.ThrowsAsync<InvalidOperationException>(async () => await repository.LoadAsync(1));
         Assert.That(exception!.Message, Does.Contain("declared target version 2"));
+
+        var persisted = await storage.ReadAsync<TestVersionedSaveData>("saves/slot_1/save");
+        Assert.Multiple(() =>
+        {
+            Assert.That(persisted.Version, Is.EqualTo(1));
+            Assert.That(persisted.Name, Is.EqualTo("legacy"));
+        });
     }

Also applies to: 743-760

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Tests/Data/PersistenceTests.cs` around lines 197 - 222, The
test currently verifies LoadAsync throws for a migration that returns v3 while
declaration expects v2 but doesn't assert the original save wasn't overwritten;
after the Assert.ThrowsAsync, reopen or use the same
SaveRepository/TestVersionedSaveData and call LoadAsync (or read raw storage via
FileStorage) for slot 1 to confirm the stored data remains the original v1
values (e.g., Name "legacy", Level 3, Experience 0, Version 1) and add an
assertion that the persisted file wasn't mutated by
TestSaveMigrationV1ToV2ReturningV3; locate this behavior around the
SaveRepository, SaveAsync, LoadAsync, TestSaveMigrationV1ToV2ReturningV3 and
FileStorage usage to add the post-failure read-and-assert check.
GFramework.Game/Setting/SettingsModel.cs (1)

295-316: 为迁移缓存与 runner 委托补一段方法级说明。

这段逻辑包含缓存构建、目标版本选择、缺链/版本一致性校验委托,属于非平凡路径;建议补充 XML docs 或简短 remarks,避免后续误把目标版本改回 loaded data。As per coding guidelines, “Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling.”

📝 建议补充方法文档
+    /// <summary>
+    ///     将已加载的设置数据迁移到当前运行时设置实例声明的目标版本。
+    /// </summary>
+    /// <param name="data">从仓库读取的设置数据。</param>
+    /// <param name="latestData">当前内存中的设置实例,其 <see cref="ISettingsData.Version" /> 作为目标版本。</param>
+    /// <returns>已迁移到目标版本的数据;如果无需迁移则返回原数据。</returns>
+    /// <remarks>
+    ///     该方法按设置类型缓存迁移表,并委托 <see cref="VersionedMigrationRunner" />
+    ///     统一执行缺链、声明版本一致性和版本前进性校验。调用方负责处理迁移失败时是否覆盖当前实例。
+    /// </remarks>
     private ISettingsData MigrateIfNeeded(ISettingsData data, ISettingsData latestData)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game/Setting/SettingsModel.cs` around lines 295 - 316, Add XML
documentation to MigrateIfNeeded explaining its responsibilities: it builds and
caches a per-type migration map into _migrationCache from _migrations, chooses
latestData.Version as the migration target (and why we don't use the loaded
data's Version), and describes the delegates passed into
VersionedMigrationRunner.MigrateToTargetVersion — how the version selector
(settings => settings.Version), the migration lookup (fromVersion =>
versionMap.TryGetValue(...)), the migration-to-version selector (migration =>
migration.ToVersion), and the application delegate (migration, current) =>
ApplySettingsMigration(migration, current) behave and handle
missing/chain-broken migrations and version-equality cases; include brief notes
on thread-safety/immutability assumptions of _migrationCache and edge-case
handling for null migration lookups.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@GFramework.Game/Data/SaveRepository.cs`:
- Around line 233-248: The migration resolver currently reads from the mutable
_migrations dictionary under _migrationsLock for each fromVersion during
VersionedMigrationRunner.MigrateToTargetVersion, which can observe concurrent
RegisterMigration changes mid-run; to fix, take a one-time snapshot of the
migration table (copy _migrations into an immutable map/array while holding
_migrationsLock) before calling VersionedMigrationRunner.MigrateToTargetVersion
and change the resolver passed to MigrateToTargetVersion to read from that
snapshot (use the snapshot lookup in place of the inline
_migrations.TryGetValue), keeping references to
VersionedMigrationRunner.MigrateToTargetVersion, _migrationsLock, _migrations,
RegisterMigration, LoadAsync, TSaveData and slot to locate the change.

In `@GFramework.Game/Internal/VersionedMigrationRunner.cs`:
- Around line 50-85: The XML doc for MigrateToTargetVersion is missing
<exception> entries for the argument validation it performs; add <exception>
documentation lines for ArgumentNullException (thrown when data, getVersion,
resolveMigration, getToVersion, or applyMigration are null) and for
ArgumentException (thrown when subjectName or migrationKind is
null/empty/whitespace) alongside the existing InvalidOperationException entry so
callers can see all possible validation failures.

---

Outside diff comments:
In `@docs/zh-CN/game/index.md`:
- Around line 686-693: The current GameDataSerializer initializes _serializer
with TypeNameHandling.Auto which allows unsafe polymorphic deserialization of
user-editable save files; fix by either setting TypeNameHandling =
TypeNameHandling.None for save data or, if polymorphism is required, implement
and assign a whitelist SerializationBinder (e.g. SafeGameDataBinder implementing
ISerializationBinder that only returns types from an allowed set like SaveData,
PlayerData, Vector2, etc.) and set SerializationBinder = new
SafeGameDataBinder() when constructing the JsonSerializer; update the
GameDataSerializer initialization to use one of these safe options.

In `@GFramework.Game/Setting/SettingsModel.cs`:
- Around line 109-135: The RegisterMigration method's XML docs are missing
<exception> entries for the errors it can throw; update the method's XML
comments (above RegisterMigration) to include <exception> tags documenting
ArgumentNullException thrown by ArgumentNullException.ThrowIfNull when migration
is null, the exception thrown by
VersionedMigrationRunner.ValidateForwardOnlyRegistration when a non-forward
(invalid) migration version pair is provided (document the specific exception
type it throws and a short description), and InvalidOperationException thrown
when _migrations.TryAdd fails due to a duplicate migration registration;
reference the symbols RegisterMigration, ISettingsMigration,
ArgumentNullException.ThrowIfNull,
VersionedMigrationRunner.ValidateForwardOnlyRegistration, _migrations.TryAdd,
InvalidOperationException, and _migrationCache.TryRemove in the descriptions so
readers can find the related code.

---

Nitpick comments:
In `@GFramework.Game.Tests/Data/PersistenceTests.cs`:
- Around line 197-222: The test currently verifies LoadAsync throws for a
migration that returns v3 while declaration expects v2 but doesn't assert the
original save wasn't overwritten; after the Assert.ThrowsAsync, reopen or use
the same SaveRepository/TestVersionedSaveData and call LoadAsync (or read raw
storage via FileStorage) for slot 1 to confirm the stored data remains the
original v1 values (e.g., Name "legacy", Level 3, Experience 0, Version 1) and
add an assertion that the persisted file wasn't mutated by
TestSaveMigrationV1ToV2ReturningV3; locate this behavior around the
SaveRepository, SaveAsync, LoadAsync, TestSaveMigrationV1ToV2ReturningV3 and
FileStorage usage to add the post-failure read-and-assert check.

In `@GFramework.Game/Setting/SettingsModel.cs`:
- Around line 295-316: Add XML documentation to MigrateIfNeeded explaining its
responsibilities: it builds and caches a per-type migration map into
_migrationCache from _migrations, chooses latestData.Version as the migration
target (and why we don't use the loaded data's Version), and describes the
delegates passed into VersionedMigrationRunner.MigrateToTargetVersion — how the
version selector (settings => settings.Version), the migration lookup
(fromVersion => versionMap.TryGetValue(...)), the migration-to-version selector
(migration => migration.ToVersion), and the application delegate (migration,
current) => ApplySettingsMigration(migration, current) behave and handle
missing/chain-broken migrations and version-equality cases; include brief notes
on thread-safety/immutability assumptions of _migrationCache and edge-case
handling for null migration lookups.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 900b97df-6725-4a69-8af7-560d07ee0310

📥 Commits

Reviewing files that changed from the base of the PR and between 374db43 and 88de123.

📒 Files selected for processing (14)
  • GFramework.Game.Tests/Data/PersistenceTests.cs
  • GFramework.Game.Tests/Serializer/JsonSerializerTests.cs
  • GFramework.Game.Tests/Setting/SettingsModelTests.cs
  • GFramework.Game/Data/SaveRepository.cs
  • GFramework.Game/Internal/VersionedMigrationRunner.cs
  • GFramework.Game/README.md
  • GFramework.Game/Serializer/JsonSerializer.cs
  • GFramework.Game/Setting/SettingsModel.cs
  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
  • ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md
  • docs/zh-CN/game/data.md
  • docs/zh-CN/game/index.md
  • docs/zh-CN/game/serialization.md
  • docs/zh-CN/game/setting.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Documentation should be organized with Chinese content in docs/zh-CN/ and structured to include getting started, module-specific capabilities (Core, Game, Godot, ECS), source generator usage, tutorials, best practices, and troubleshooting

Files:

  • docs/zh-CN/game/data.md
  • docs/zh-CN/game/index.md
  • docs/zh-CN/game/serialization.md
  • docs/zh-CN/game/setting.md
docs/zh-CN/**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/zh-CN/**/*.md: When a feature is added, removed, renamed, or substantially refactored, contributors MUST update or create the corresponding user-facing integration documentation in docs/zh-CN/ in the same change. For integration-oriented features, documentation MUST cover project directory layout, file conventions, required project/package wiring, minimal working usage example, and migration/compatibility notes.
Code samples, package names, and command examples in documentation MUST be aligned with the current repository state. When an existing documentation page no longer reflects the current implementation, fix the code without fixing the documentation is considered incomplete work.
Prefer documenting behavior and design intent, not only API surface. Do not rely on 'the code is self-explanatory' for framework features that consumers need to adopt; write the adoption path down.

Files:

  • docs/zh-CN/game/data.md
  • docs/zh-CN/game/index.md
  • docs/zh-CN/game/serialization.md
  • docs/zh-CN/game/setting.md
**/README.md

📄 CodeRabbit inference engine (AGENTS.md)

Every user-facing package or module directory that contains a *.csproj intended for direct consumption MUST have a sibling README.md with canonical filename README.md. A module README MUST describe the module's purpose, relationship to adjacent runtime/abstractions/generator packages, major subdirectories/subsystems, minimum adoption path, and corresponding docs/zh-CN/ entry points.

Files:

  • GFramework.Game/README.md
**/*.cs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.cs: Apply [Log] attribute for automatic logging field and logging helper method generation
Apply [Priority] attribute for automatic priority comparison implementation generation
Apply [GenerateEnumExtensions] attribute to generate enumeration extension capabilities
Apply [ContextAware] attribute to automatically implement IContextAware boilerplate logic

**/*.cs: All public, protected, and internal types and members MUST include XML documentation comments (///) with <summary>, <param>, <returns>, <exception>, and <remarks> where applicable. Comments must explain intent, contract, and usage constraints instead of restating syntax.
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds or compatibility constraints, and registration order or lifecycle sequencing.
Core framework components such as Architecture, Module, System, Context, Registry, Service Module, and Lifecycle types MUST include high-level explanations of responsibilities, lifecycle, interaction with other components, why the abstraction exists, and when to use it instead of alternatives.
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling.
Follow the namespace pattern GFramework.{Module}.{Feature} with PascalCase segments.
Use the following C# naming conventions: Types, methods, properties, events, and constants use PascalCase; interfaces use I prefix; parameters and locals use camelCase; private fields use _camelCase.
Do not rely on implicit imports. Declare every required using explicitly. Keep using directives at the top of the file and sort them consistently.
Write null-safe code that respects nullable annotations instead of suppressing warnings by default. The project has Nullable enabled.
Use 4 spaces for indentation. Do not use tabs. Use Allman braces. Keep line length readable, around 120 characters as the preferred upper bound.
Prefer one primary type per...

Files:

  • GFramework.Game.Tests/Serializer/JsonSerializerTests.cs
  • GFramework.Game/Serializer/JsonSerializer.cs
  • GFramework.Game.Tests/Data/PersistenceTests.cs
  • GFramework.Game/Internal/VersionedMigrationRunner.cs
  • GFramework.Game/Setting/SettingsModel.cs
  • GFramework.Game/Data/SaveRepository.cs
  • GFramework.Game.Tests/Setting/SettingsModelTests.cs
ai-plan/public/**/*

📄 CodeRabbit inference engine (AGENTS.md)

ai-plan/public/**/*: Contributors MUST keep committed ai-plan/public/** content safe to publish in Git history. Never write secrets, tokens, credentials, private keys, machine usernames, home-directory paths, hostnames, IP addresses, proprietary URLs, or other sensitive environment details into any ai-plan/** file.
Never record absolute file-system paths in ai-plan/**; use repository-relative paths, branch names, PR numbers, or stable document identifiers instead.

Files:

  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
  • ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md
ai-plan/public/*/todos/**/*

📄 CodeRabbit inference engine (AGENTS.md)

When working from a tracked implementation plan, contributors MUST update the corresponding tracking document under ai-plan/public/<topic>/todos/ in the same change. Updates MUST reflect completed work, newly discovered issues, validation results, and the next recommended recovery point. Tracking files are recovery entrypoints, not append-only changelogs, and MUST stay concise.

Files:

  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
ai-plan/public/*/traces/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Contributors MUST maintain a matching execution trace under ai-plan/public/<topic>/traces/ for complex work. The trace should record the current date, key decisions, validation milestones, and the immediate next step. Update traces at each meaningful milestone before pausing or handing work off.

Files:

  • ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md
🧠 Learnings (12)
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to docs/zh-CN/**/*.md : When a feature is added, removed, renamed, or substantially refactored, contributors MUST update or create the corresponding user-facing integration documentation in `docs/zh-CN/` in the same change. For integration-oriented features, documentation MUST cover project directory layout, file conventions, required project/package wiring, minimal working usage example, and migration/compatibility notes.

Applied to files:

  • docs/zh-CN/game/data.md
  • docs/zh-CN/game/setting.md
📚 Learning: 2026-04-06T12:45:43.921Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 190
File: GFramework.Game/Config/GameConfigBootstrap.cs:1-3
Timestamp: 2026-04-06T12:45:43.921Z
Learning: In the GeWuYou/GFramework repository, C# files may omit explicit `using System*` imports because the project-wide `GlobalUsings.cs` (referenced via manual global `using` directives) supplies common namespaces (e.g., `System`, `System.Threading`, `System.Threading.Tasks`). During code review, do not flag missing `using System...` directives in `.cs` files as long as `GlobalUsings.cs` is present/used to provide those namespaces.

Applied to files:

  • GFramework.Game.Tests/Serializer/JsonSerializerTests.cs
  • GFramework.Game/Serializer/JsonSerializer.cs
  • GFramework.Game.Tests/Data/PersistenceTests.cs
  • GFramework.Game/Internal/VersionedMigrationRunner.cs
  • GFramework.Game/Setting/SettingsModel.cs
  • GFramework.Game/Data/SaveRepository.cs
  • GFramework.Game.Tests/Setting/SettingsModelTests.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.cs : Prefer least-privilege behavior for file, process, and environment access. Do not introduce unsafe deserialization, broad reflection-based activation, or dynamic code execution unless explicitly required and tightly constrained.

Applied to files:

  • docs/zh-CN/game/serialization.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.cs : Avoid hidden side effects in property getters, constructors, and registration helpers. Preserve deterministic behavior in registries, lifecycle orchestration, and generated outputs.

Applied to files:

  • docs/zh-CN/game/serialization.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.g.cs : Source generator changes MUST be covered by generator tests. Preserve snapshot-based verification patterns already used in the repository. When generator behavior changes intentionally, update snapshots together with the implementation.

Applied to files:

  • GFramework.Game.Tests/Data/PersistenceTests.cs
  • GFramework.Game.Tests/Setting/SettingsModelTests.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to ai-plan/public/*/todos/**/* : When working from a tracked implementation plan, contributors MUST update the corresponding tracking document under `ai-plan/public/<topic>/todos/` in the same change. Updates MUST reflect completed work, newly discovered issues, validation results, and the next recommended recovery point. Tracking files are recovery entrypoints, not append-only changelogs, and MUST stay concise.

Applied to files:

  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: For any multi-step refactor, migration, or cross-module task, contributors MUST create or adopt a dedicated recovery document under `ai-plan/public/<topic>/todos/` before making substantive code changes. Recovery documents MUST record the current phase, the active recovery point identifier, known risks, and the next recommended resume step so another contributor or subagent can continue the work safely.

Applied to files:

  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: When a stage inside an active topic is fully complete, move the finished artifacts into that topic's `archive/` directory instead of leaving every completed step in the default boot path. When completed and validated stages begin to accumulate, archive their detailed history out of the active `todos/` and `traces/` entry files. Keep only the current recovery point, active facts, active risks, immediate next step, and pointers to relevant archive files in the default boot path.

Applied to files:

  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
📚 Learning: 2026-04-16T13:26:01.780Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 235
File: GFramework.SourceGenerators.Tests/Rule/ContextAwareGeneratorSnapshotTests.cs:1-3
Timestamp: 2026-04-16T13:26:01.780Z
Learning: In the GeWuYou/GFramework repository, the test project `GFramework.SourceGenerators.Tests` provides `global using NUnit.Framework;` via its `GlobalUsings.cs`. Do not flag missing `using NUnit.Framework;` directives in test files within this project, as the global using already covers it. The project-wide GlobalUsings.cs pattern is used broadly to supply common namespaces (including NUnit, System.IO, etc.) without per-file imports.

Applied to files:

  • GFramework.Game/Setting/SettingsModel.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to ai-plan/public/*/traces/**/* : Contributors MUST maintain a matching execution trace under `ai-plan/public/<topic>/traces/` for complex work. The trace should record the current date, key decisions, validation milestones, and the immediate next step. Update traces at each meaningful milestone before pausing or handing work off.

Applied to files:

  • ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.Tests.cs : Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical. Public API changes must be covered by unit or integration tests.

Applied to files:

  • GFramework.Game.Tests/Setting/SettingsModelTests.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.Tests.cs : When a public API defines multiple contract branches, tests MUST cover the meaningful variants, including null, empty, default, and filtered inputs when those branches change behavior. Regression fixes should include a test that fails before the fix and passes after it.

Applied to files:

  • GFramework.Game.Tests/Setting/SettingsModelTests.cs
🪛 LanguageTool
ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md

[uncategorized] ~16-~16: 您的意思是“"不"强”?
Context: ...odel/SaveRepository` 共享迁移执行器收敛与契约补强 - 下一轮需要继续评估 codec / persistence pipe...

(BU)


[uncategorized] ~38-~38: 名词后要加"方位词"
Context: ...sistence pipeline 边界风险:压缩、加密、元数据与备份策略还散落在仓库与存储语义之间 - 缓解措施:下一轮先梳理现有 Serializer /...

(wa2)

ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md

[grammar] ~76-~76: Ensure spelling is correct
Context: ... 1. 进入 codec / persistence pipeline 边界评估 2. 重点查看压缩、加密、元数据、备份是否仍然跨越 Serializer / Storage / Repository...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🔇 Additional comments (17)
GFramework.Game/Serializer/JsonSerializer.cs (1)

7-12: 文档契约清晰,与当前实现一致。

这里明确了 JsonSerializerSettings / Converters 是活动引用、不会复制,并说明共享后不应继续修改配置,能降低并发使用时的误用风险。

Also applies to: 21-24, 33-36, 42-45

docs/zh-CN/game/setting.md (1)

184-190: 迁移规则说明补充到位。

这段把注册约束、目标版本选择、失败时不覆盖内存实例,以及与 SaveRepository<TSaveData> 的异常传播差异写清楚了。

GFramework.Game/README.md (1)

183-190: 最小接入示例与配置生命周期契约一致。

改成先创建具体 JsonSerializer,并提醒注册前完成 settings/converters 配置,和当前 serializer 的 live configuration 语义匹配。

GFramework.Game.Tests/Serializer/JsonSerializerTests.cs (1)

52-63: 契约测试覆盖准确。

这个测试用 Is.SameAs 锁定 settings/converters 的 live reference 行为,和本次文档说明形成了有效回归保护。

docs/zh-CN/game/data.md (1)

302-304: 存档迁移约束同步清楚。

同一 FromVersion 唯一、ToVersion 前进,以及返回数据版本必须匹配声明目标版本,都已明确写入迁移文档。

docs/zh-CN/game/serialization.md (1)

58-59: 序列化器配置生命周期说明完整。

这些更新把 live settings/converters、共享前配置、共享后只读、并发读前提都串起来了,能有效避免运行期动态修改配置导致的不稳定行为。

Also applies to: 85-88, 172-204, 503-507, 772-791

GFramework.Game.Tests/Setting/SettingsModelTests.cs (4)

50-66: 该缓存失效场景覆盖到位。

这里先把当前内存实例提升到目标版本,再注册迁移并重新初始化,可以验证迁移缓存被清掉后会重新使用新迁移链。


69-81: 重复迁移注册测试有效。

该用例直接覆盖同一 SettingsType + FromVersion 重复注册的失败契约,断言信息也和实现中的诊断前缀保持一致。


83-110: 不完整迁移链回归场景合理。

该测试验证缺少 2 -> 3 链路时不会把旧数据覆盖到当前实例,覆盖了本次迁移 runner 收敛后的关键失败语义。


157-200: 测试夹具与新增场景匹配。

TestLatestSettingsData 默认版本为 3,配合只注册 1 -> 2 的迁移器,能稳定触发缺链分支。

GFramework.Game/Data/SaveRepository.cs (1)

73-78: 注册校验复用得很好。

把 forward-only 校验集中到 VersionedMigrationRunner 后,存档与设置迁移的错误条件更一致。

ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md (1)

16-62: tracking 入口更新符合当前恢复点用途。

内容记录了完成项、当前事实、验证结果和下一步,并保持为可恢复入口,没有写入绝对路径或敏感环境信息。As per coding guidelines, “Tracking files are recovery entrypoints, not append-only changelogs, and MUST stay concise.”

GFramework.Game/Setting/SettingsModel.cs (2)

174-177: 目标版本来源调整正确。

传入当前内存实例后,迁移目标版本由运行时默认设置对象决定,符合“不替换实例、只回填数据”的初始化语义。


318-344: 迁移结果类型校验清晰。

这里先验证结果仍实现 ISettingsData,再验证与注册的设置类型兼容,可以避免错误迁移器把不兼容对象回填到当前设置实例。

GFramework.Game/Internal/VersionedMigrationRunner.cs (2)

16-48: 共享 runner 的职责边界描述清楚。

类级 remarks 明确只处理版本推进约束,不处理存储、日志、回写或吞异常策略,适合被 SettingsModel 和 SaveRepository 复用。


87-140: 迁移循环的安全检查完整。

缺链、返回 null、声明版本不一致、未前进和超目标版本都被显式拒绝,能防止迁移链静默停在错误状态。

ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md (1)

32-77: trace 记录完整且可恢复。

新增段落包含日期、阶段决策、验证结果和下一步,能和 tracking 文件一起作为后续交接入口。As per coding guidelines, “The trace should record the current date, key decisions, validation milestones, and the immediate next step.”

Comment thread GFramework.Game/Data/SaveRepository.cs
Comment thread GFramework.Game/Internal/VersionedMigrationRunner.cs
@GeWuYou GeWuYou changed the title Feat/data repository persistence Refactor migration chain execution with unified VersionedMigrationRunner Apr 20, 2026
- 修复 SaveRepository 迁移链并发读取,改为单次快照执行

- 补充 VersionedMigrationRunner 与 SettingsModel 的 XML 文档契约

- 更新 PersistenceTests、接入文档与 ai-plan 跟踪记录
@github-actions

Copy link
Copy Markdown

Summary

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Other ❓ Flaky 🍂 Duration ⏱️
2113    ↑6 2113    ↑6 0 0 0 0 36.2s    ↓308ms

Test Results

passed 2113 passed ↑6

Details

tests 2113 tests ↑6
clock 36.2s ↓308ms
tool nunit
build CI - Build & Test arrow-right build-and-test link #898
pull-request Refactor migration chain execution with unified VersionedMigrationRunner link #260

Insights

Average Tests per Run Total Flaky Tests Total Failed Slowest Test (p95)
1986 0 0 4.7s

build-and-test: Run #898

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Pending ⏳ Other ❓ Flaky 🍂 Duration ⏱️
2113 2113 0 0 0 0 0 36.2s

🎉 All tests passed!

Slowest Tests

Test 📝 Results 📊 Duration (avg) ⏱️ Duration (p95) ⏱️
SendRequestAsync_Should_ResolveCqrsRuntime_OnlyOnce_When_AccessedConcurrently 3 4.1s 4.7s
Does_Not_Report_When_FieldInjectedModel_Is_Registered 5 2.6s 3.5s
Generates_Scene_Behavior_Boilerplate 3 1.9s 2.0s
CleanupDuringAcquire_Should_NotCauseRaceCondition 5 1.1s 1.1s
Append_ShouldNotBlock 5 1.0s 1.0s
Context_Caching_Should_Improve_Performance 5 780ms 788ms
Snapshot_SchemaConfigGenerator 5 292ms 714ms
PendingCount_ShouldReflectQueuedEntries 5 501ms 501ms
Cleanup_Should_NotRemoveActiveLocks 5 406ms 411ms
Cleanup_Should_RemoveUnusedLocks 5 401ms 404ms

± Comparison with run #895 at ca459e7 | 🎉 No failed tests detected across all runs. | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 5 runs.

Github Test Reporter by CTRF 💚

coderabbitai[bot]
coderabbitai Bot previously requested changes Apr 20, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md`:
- Around line 36-37: 将第36-37行关于“PR `#260` 最新 review 仍要求补齐
`VersionedMigrationRunner` / `SettingsModel` 的 XML 异常契约,并确保 `SaveRepository<T>`
单次加载不会在并发注册期间读取到变化中的迁移链”这类描述从“仍要求”改为已完成状态;具体操作:把该句改为表明这些 follow-up
已完成并通过验证(与第17行和第61-63行的记录一致)、去掉或替换“active recovery”类可能误导接手者的措辞,并确保文中提到的符号
`VersionedMigrationRunner`、`SettingsModel` 和 `SaveRepository<T>`
的状态说明与文档其它位置一致以避免歧义。

In
`@ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md`:
- Around line 43-47: Multiple identical third-level headings "### 验证" and "###
下一步" cause markdownlint MD024 and anchor conflicts; update each repeated heading
to be unique by appending a stage qualifier (e.g., "### 验证 — Migration", "###
下一步 — Codec/Persistence") or a numeric suffix so the headings are distinct, and
apply this change to all occurrences referenced in the diff (the repeated "###
验证" / "### 下一步" blocks such as those around the sections that currently produce
MD024) to eliminate anchor collisions.

In `@GFramework.Game/Setting/SettingsModel.cs`:
- Around line 138-144: 当前注册逻辑在 RegisterMigration 中先向并发字典 _migrations 添加迁移再
TryRemove _migrationCache,会与并发的 MigrateIfNeeded 在 cache miss 时基于旧 _migrations
重建并写回 _migrationCache 导致新迁移被覆盖为 stale snapshot;为修复,在同一把锁(例如私有 readonly object
_migrationLock)上序列化 RegisterMigration 与 MigrateIfNeeded 中 cache miss
的重建/写回逻辑,确保添加迁移与缓存重建成为原子操作;如果设计上只允许启动前单线程注册,则在 RegisterMigration、_migrations 与
_migrationCache 的 XML 文档中明确生命周期约束并添加内联注释说明并发语义和为什么选择该策略,以满足并发、安全性和可维护性要求,并相应更新
318-325 处的类似代码。
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 636cdaac-9f23-4ac4-8fcd-a1f24d9bd004

📥 Commits

Reviewing files that changed from the base of the PR and between 88de123 and ec3de5b.

📒 Files selected for processing (7)
  • GFramework.Game.Tests/Data/PersistenceTests.cs
  • GFramework.Game/Data/SaveRepository.cs
  • GFramework.Game/Internal/VersionedMigrationRunner.cs
  • GFramework.Game/Setting/SettingsModel.cs
  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
  • ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md
  • docs/zh-CN/game/index.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/zh-CN/game/index.md
  • GFramework.Game.Tests/Data/PersistenceTests.cs
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Analyze (C#)
  • GitHub Check: Build and Test
  • GitHub Check: Code Quality & Security
🧰 Additional context used
📓 Path-based instructions (4)
**/*.cs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.cs: Apply [Log] attribute for automatic logging field and logging helper method generation
Apply [Priority] attribute for automatic priority comparison implementation generation
Apply [GenerateEnumExtensions] attribute to generate enumeration extension capabilities
Apply [ContextAware] attribute to automatically implement IContextAware boilerplate logic

**/*.cs: All public, protected, and internal types and members MUST include XML documentation comments (///) with <summary>, <param>, <returns>, <exception>, and <remarks> where applicable. Comments must explain intent, contract, and usage constraints instead of restating syntax.
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds or compatibility constraints, and registration order or lifecycle sequencing.
Core framework components such as Architecture, Module, System, Context, Registry, Service Module, and Lifecycle types MUST include high-level explanations of responsibilities, lifecycle, interaction with other components, why the abstraction exists, and when to use it instead of alternatives.
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling.
Follow the namespace pattern GFramework.{Module}.{Feature} with PascalCase segments.
Use the following C# naming conventions: Types, methods, properties, events, and constants use PascalCase; interfaces use I prefix; parameters and locals use camelCase; private fields use _camelCase.
Do not rely on implicit imports. Declare every required using explicitly. Keep using directives at the top of the file and sort them consistently.
Write null-safe code that respects nullable annotations instead of suppressing warnings by default. The project has Nullable enabled.
Use 4 spaces for indentation. Do not use tabs. Use Allman braces. Keep line length readable, around 120 characters as the preferred upper bound.
Prefer one primary type per...

Files:

  • GFramework.Game/Data/SaveRepository.cs
  • GFramework.Game/Internal/VersionedMigrationRunner.cs
  • GFramework.Game/Setting/SettingsModel.cs
ai-plan/public/**/*

📄 CodeRabbit inference engine (AGENTS.md)

ai-plan/public/**/*: Contributors MUST keep committed ai-plan/public/** content safe to publish in Git history. Never write secrets, tokens, credentials, private keys, machine usernames, home-directory paths, hostnames, IP addresses, proprietary URLs, or other sensitive environment details into any ai-plan/** file.
Never record absolute file-system paths in ai-plan/**; use repository-relative paths, branch names, PR numbers, or stable document identifiers instead.

Files:

  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
  • ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md
ai-plan/public/*/todos/**/*

📄 CodeRabbit inference engine (AGENTS.md)

When working from a tracked implementation plan, contributors MUST update the corresponding tracking document under ai-plan/public/<topic>/todos/ in the same change. Updates MUST reflect completed work, newly discovered issues, validation results, and the next recommended recovery point. Tracking files are recovery entrypoints, not append-only changelogs, and MUST stay concise.

Files:

  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
ai-plan/public/*/traces/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Contributors MUST maintain a matching execution trace under ai-plan/public/<topic>/traces/ for complex work. The trace should record the current date, key decisions, validation milestones, and the immediate next step. Update traces at each meaningful milestone before pausing or handing work off.

Files:

  • ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md
🧠 Learnings (7)
📚 Learning: 2026-04-06T12:45:43.921Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 190
File: GFramework.Game/Config/GameConfigBootstrap.cs:1-3
Timestamp: 2026-04-06T12:45:43.921Z
Learning: In the GeWuYou/GFramework repository, C# files may omit explicit `using System*` imports because the project-wide `GlobalUsings.cs` (referenced via manual global `using` directives) supplies common namespaces (e.g., `System`, `System.Threading`, `System.Threading.Tasks`). During code review, do not flag missing `using System...` directives in `.cs` files as long as `GlobalUsings.cs` is present/used to provide those namespaces.

Applied to files:

  • GFramework.Game/Data/SaveRepository.cs
  • GFramework.Game/Internal/VersionedMigrationRunner.cs
  • GFramework.Game/Setting/SettingsModel.cs
📚 Learning: 2026-04-16T13:26:01.780Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 235
File: GFramework.SourceGenerators.Tests/Rule/ContextAwareGeneratorSnapshotTests.cs:1-3
Timestamp: 2026-04-16T13:26:01.780Z
Learning: In the GeWuYou/GFramework repository, the test project `GFramework.SourceGenerators.Tests` provides `global using NUnit.Framework;` via its `GlobalUsings.cs`. Do not flag missing `using NUnit.Framework;` directives in test files within this project, as the global using already covers it. The project-wide GlobalUsings.cs pattern is used broadly to supply common namespaces (including NUnit, System.IO, etc.) without per-file imports.

Applied to files:

  • GFramework.Game/Setting/SettingsModel.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to ai-plan/public/*/todos/**/* : When working from a tracked implementation plan, contributors MUST update the corresponding tracking document under `ai-plan/public/<topic>/todos/` in the same change. Updates MUST reflect completed work, newly discovered issues, validation results, and the next recommended recovery point. Tracking files are recovery entrypoints, not append-only changelogs, and MUST stay concise.

Applied to files:

  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: For any multi-step refactor, migration, or cross-module task, contributors MUST create or adopt a dedicated recovery document under `ai-plan/public/<topic>/todos/` before making substantive code changes. Recovery documents MUST record the current phase, the active recovery point identifier, known risks, and the next recommended resume step so another contributor or subagent can continue the work safely.

Applied to files:

  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: When a task spans multiple commits or is likely to exceed a single agent context window, update both the recovery document and the trace at each meaningful milestone before pausing or handing work off.

Applied to files:

  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: When a stage inside an active topic is fully complete, move the finished artifacts into that topic's `archive/` directory instead of leaving every completed step in the default boot path. When completed and validated stages begin to accumulate, archive their detailed history out of the active `todos/` and `traces/` entry files. Keep only the current recovery point, active facts, active risks, immediate next step, and pointers to relevant archive files in the default boot path.

Applied to files:

  • ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to ai-plan/public/*/traces/**/* : Contributors MUST maintain a matching execution trace under `ai-plan/public/<topic>/traces/` for complex work. The trace should record the current date, key decisions, validation milestones, and the immediate next step. Update traces at each meaningful milestone before pausing or handing work off.

Applied to files:

  • ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md
🪛 LanguageTool
ai-plan/public/data-repository-persistence/todos/data-repository-persistence-tracking.md

[uncategorized] ~16-~16: 您的意思是“"不"强”?
Context: ...odel/SaveRepository共享迁移执行器收敛与契约补强 - 已完成 PR#260` 的 review follow-up:迁移链...

(BU)


[uncategorized] ~17-~17: 您的意思是“"不"齐”?
Context: ...60 的 review follow-up:迁移链快照一致性、XML docs 补齐与文档安全示例修正 - 下一轮需要继续评估 codec / persist...

(BU)


[uncategorized] ~36-~36: 您的意思是“"不"齐”?
Context: ...链式校验已与设置迁移共用同一实现 - PR #260 最新 review 仍要求补齐 VersionedMigrationRunner / `Settings...

(BU)


[uncategorized] ~43-~43: 名词后要加"方位词"
Context: ...sistence pipeline 边界风险:压缩、加密、元数据与备份策略还散落在仓库与存储语义之间 - 缓解措施:下一轮先梳理现有 Serializer /...

(wa2)

ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md

[grammar] ~76-~76: Ensure spelling is correct
Context: ... 1. 进入 codec / persistence pipeline 边界评估 2. 重点查看压缩、加密、元数据、备份是否仍然跨越 Serializer / Storage / Repository...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~108-~108: Ensure spelling is correct
Context: ... 1. 回到 codec / persistence pipeline 边界评估 2. 继续判断压缩、加密、元数据与备份策略是否需要新的 dedicated pipeline abstraction

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🪛 markdownlint-cli2 (0.22.0)
ai-plan/public/data-repository-persistence/traces/data-repository-persistence-trace.md

[warning] 74-74: Multiple headings with the same content

(MD024, no-duplicate-heading)


[warning] 101-101: Multiple headings with the same content

(MD024, no-duplicate-heading)


[warning] 106-106: Multiple headings with the same content

(MD024, no-duplicate-heading)

🔇 Additional comments (2)
GFramework.Game/Data/SaveRepository.cs (1)

231-247: 迁移表快照处理得当。

Line 231-247 在进入 runner 前一次性复制 _migrations,resolver 只读快照,避免单次 LoadAsync 在多步迁移中看到并发注册后的新链路;注释也清楚说明了并发语义。

GFramework.Game/Internal/VersionedMigrationRunner.cs (1)

16-24: 统一 runner 的职责边界和失败校验清晰。

文档明确限定 runner 只负责版本推进约束,MigrateToTargetVersion 也覆盖了缺链、null 返回、声明版本不一致、非递增与超目标版本等关键失败模式;这与 SaveRepository<T> / SettingsModel 的共享迁移路径匹配。

Also applies to: 50-148

Comment thread GFramework.Game/Setting/SettingsModel.cs Outdated
- 更新 tracking 文档中的 PR #260 review follow-up 状态描述

- 修复 trace 文档重复的三级标题,避免 Markdown 锚点冲突
@github-actions

Copy link
Copy Markdown

Summary

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Other ❓ Flaky 🍂 Duration ⏱️
2113 2113 0 0 0 0 37.5s    ↑1.3s

Test Results

passed 2113 passed

Details

tests 2113 tests
clock 37.5s ↑1.3s
tool nunit
build CI - Build & Test arrow-right build-and-test link #900
pull-request Refactor migration chain execution with unified VersionedMigrationRunner link #260

Insights

Average Tests per Run Total Flaky Tests Total Failed Slowest Test (p95)
2008 0 0 4.7s

build-and-test: Run #900

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Pending ⏳ Other ❓ Flaky 🍂 Duration ⏱️
2113 2113 0 0 0 0 0 37.5s

🎉 All tests passed!

Slowest Tests

Test 📝 Results 📊 Duration (avg) ⏱️ Duration (p95) ⏱️
SendRequestAsync_Should_ResolveCqrsRuntime_OnlyOnce_When_AccessedConcurrently 4 4.3s 4.7s
Does_Not_Report_When_FieldInjectedModel_Is_Registered 6 2.6s 3.5s
Generates_Scene_Behavior_Boilerplate 4 1.9s 2.0s
CleanupDuringAcquire_Should_NotCauseRaceCondition 6 1.1s 1.1s
Append_ShouldNotBlock 6 1.0s 1.0s
Context_Caching_Should_Improve_Performance 6 780ms 788ms
Snapshot_SchemaConfigGenerator 6 251ms 714ms
PendingCount_ShouldReflectQueuedEntries 6 501ms 501ms
Cleanup_Should_NotRemoveActiveLocks 6 405ms 411ms
Cleanup_Should_RemoveUnusedLocks 6 401ms 404ms

± Comparison with run #898 at bfc2bbc | 🎉 No failed tests detected across all runs. | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 6 runs.

Github Test Reporter by CTRF 💚

- 修复 SettingsModel 迁移注册与缓存重建的并发竞争

- 新增 SettingsModel 并发回归测试并更新 ai-plan 跟踪
@github-actions

Copy link
Copy Markdown

Summary

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Other ❓ Flaky 🍂 Duration ⏱️
2114    ↑1 2114    ↑1 0 0 0 0 36.1s    ↓1.5s

Test Results

passed 2114 passed ↑1

Details

tests 2114 tests ↑1
clock 36.1s ↓1.5s
tool nunit
build CI - Build & Test arrow-right build-and-test link #901
pull-request Refactor migration chain execution with unified VersionedMigrationRunner link #260

Insights

Average Tests per Run Total Flaky Tests Total Failed Slowest Test (p95)
2023 0 0 4.7s

build-and-test: Run #901

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Pending ⏳ Other ❓ Flaky 🍂 Duration ⏱️
2114 2114 0 0 0 0 0 36.1s

🎉 All tests passed!

Slowest Tests

Test 📝 Results 📊 Duration (avg) ⏱️ Duration (p95) ⏱️
SendRequestAsync_Should_ResolveCqrsRuntime_OnlyOnce_When_AccessedConcurrently 5 4.1s 4.7s
Does_Not_Report_When_FieldInjectedModel_Is_Registered 7 2.5s 3.5s
Generates_Scene_Behavior_Boilerplate 5 1.9s 2.0s
CleanupDuringAcquire_Should_NotCauseRaceCondition 7 1.1s 1.1s
Append_ShouldNotBlock 7 1.0s 1.0s
Context_Caching_Should_Improve_Performance 7 779ms 788ms
Snapshot_SchemaConfigGenerator 7 225ms 714ms
PendingCount_ShouldReflectQueuedEntries 7 501ms 501ms
Cleanup_Should_NotRemoveActiveLocks 7 405ms 411ms
Cleanup_Should_RemoveUnusedLocks 7 401ms 404ms

± Comparison with run #900 at f0b7d53 | 🎉 No failed tests detected across all runs. | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 7 runs.

Github Test Reporter by CTRF 💚

Repository owner deleted a comment from coderabbitai Bot Apr 20, 2026
@GeWuYou

GeWuYou commented Apr 20, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant