fix(analyzers): 降低 Core、Cqrs、Godot 与生成器的构建警告#252
Conversation
- 清理 GFramework.Core 与 GFramework.Cqrs 中的大量低风险 Meziantou 警告 - 修复 GFramework.Godot 运行时中的 ConfigureAwait、StringComparison 与参数校验告警 - 调整 Core SourceGenerators 中的字符串比较、文件命名与局部长方法问题 - 拆分部分配置与缓存辅助类型文件以消除 file/type mismatch 告警 - 更新 warning reduction 跟踪与执行记录,保留下一批结构性告警的恢复点
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough本次变更在代码库范围内统一为序数(ordinal)字符串比较与显式字典比较器(StringComparer.Ordinal),大量 await 点添加 Changes
Sequence Diagram(s)(跳过) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Summary
Test ResultsDetails
Failed TestsLocalizationIntegrationTests Insights
Fail Rate
build-and-test: Run #871
❌ Some tests failed!
Slowest Tests
🍂 No flaky tests in this run. Github Test Reporter by CTRF 💚 |
✅
|
| Descriptor | Linter | Files | Fixed | Errors | Warnings | Elapsed time |
|---|---|---|---|---|---|---|
| dotnet-format | yes | 1 | no | 0.64s | ||
| ✅ REPOSITORY | gitleaks | yes | no | no | 3.41s | |
| ✅ REPOSITORY | trufflehog | yes | no | no | 3.85s |
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.IO.FileNotFoundException: Both a MSBuild project file and solution file found in ''. Specify which to use with the <workspace> argument.
at Microsoft.CodeAnalysis.Tools.Workspaces.MSBuildWorkspaceFinder.FindWorkspace(String searchDirectory, String workspacePath)
at Microsoft.CodeAnalysis.Tools.Workspaces.MSBuildWorkspaceFinder.FindWorkspace(String searchDirectory, String workspacePath)
at Microsoft.CodeAnalysis.Tools.FormatCommandCommon.ParseWorkspaceOptions(ParseResult parseResult, FormatOptions formatOptions)
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

Show us your support by starring ⭐ the repository
- 实现 ResultExtensions 扩展类,提供 Combine、Map、Bind 等函数式操作 - 添加 MicrosoftDiContainer 依赖注入容器,支持单例、瞬态、作用域服务注册 - 实现 CQRS 管道行为和处理器注册功能 - 添加本地化字符串实现和相关工具方法 - 提供线程安全的多读单写锁保护机制 - 实现服务实例的优先级排序和批量获取功能
Summary
Test ResultsDetails
Insights
Fail Rate
build-and-test: Run #872
🎉 All tests passed!Slowest Tests
± Comparison with run #871 at ada707b | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 2 runs. Github Test Reporter by CTRF 💚 |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
GFramework.Godot/UI/UiPageBehaviorFactory.cs (1)
25-33:⚠️ Potential issue | 🟡 Minor请为已抛出的异常补充 XML
<exception>文档。
Create<T>在Line 43明确抛出ArgumentException,但当前注释缺少对应<exception>说明,公共 API 契约不完整。✍️ 建议补丁
/// <param name="key">UI 标识键</param> /// <param name="layer">目标层级</param> /// <returns>对应层级的 IUiPageBehavior 实例</returns> + /// <exception cref="ArgumentException">当 <paramref name="layer"/> 不是受支持的 UI 层级时抛出。</exception> public static IUiPageBehavior Create<T>(T owner, string key, UiLayer layer) where T : CanvasItemAs per coding guidelines "All public, protected, and internal types and members MUST include XML documentation comments (
///) with<summary>,<param>,<returns>,<exception>, and<remarks>where applicable".Also applies to: 43-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot/UI/UiPageBehaviorFactory.cs` around lines 25 - 33, The public generic method Create<T>(T owner, string key, UiLayer layer) throws ArgumentException (see the throw at the body) but its XML docblock lacks an <exception> tag; add an <exception cref="System.ArgumentException"> entry describing when and why the ArgumentException is thrown (e.g., invalid or unsupported layer/key or owner type) in the method's XML comments alongside existing <summary>, <param>, and <returns> tags so the API contract is complete for Create<T>.GFramework.Core/Extensions/NumericExtensions.cs (1)
26-34:⚠️ Potential issue | 🟠 Major空值契约与实际行为不一致,存在 NRE 风险。
Between<T>文档声明会抛ArgumentNullException,但当前实现未做空值校验;当value或min为null时会在比较处触发NullReferenceException。建议显式校验以匹配契约。🛠️ 建议补丁
public static bool Between<T>(this T value, T min, T max, bool inclusive = true) where T : IComparable<T> { + ArgumentNullException.ThrowIfNull(value); + ArgumentNullException.ThrowIfNull(min); + ArgumentNullException.ThrowIfNull(max); + if (min.CompareTo(max) > 0) throw new ArgumentException($"最小值 ({min}) 不能大于最大值 ({max})", nameof(min));As per coding guidelines "Write null-safe code that respects nullable annotations instead of suppressing warnings by default".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Core/Extensions/NumericExtensions.cs` around lines 26 - 34, The Between<T> extension currently lacks null checks and can throw NREs; update Between<T>(this T value, T min, T max, bool inclusive = true) to explicitly validate value, min, and max for null and throw ArgumentNullException (e.g. for value, nameof(value); for min, nameof(min); for max, nameof(max)) before any CompareTo calls, then retain the existing min>max ArgumentException and the inclusive/exclusive comparisons so the behavior matches the documented contract and nullable annotations.GFramework.Core/Coroutine/Instructions/WaitForTask.cs (1)
5-10:⚠️ Potential issue | 🟡 Minor修复重复的 XML 文档注释。
类型声明前存在两个完全相同的
<summary>块(第 5-7 行和第 8-10 行),这是多余的。应删除其中一个以符合编码标准。🔧 建议的修复
/// <summary> /// 等待Task完成的等待指令 /// </summary> -/// <summary> -/// 等待Task完成的等待指令 -/// </summary> public sealed class WaitForTask : IYieldInstruction🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Core/Coroutine/Instructions/WaitForTask.cs` around lines 5 - 10, 在类型声明 WaitForTask 前存在两个完全相同的 XML <summary> 注释块(重复的第 5-7 行和第 8-10 行),请删除多余的一个保留单个 <summary> 描述以消除重复并符合编码标准;检查 WaitForTask 类/struct/文件头部以确保仅保留一套 XML 文档注释并保持其内容准确无改动。GFramework.Core/Resource/ResourceManager.cs (1)
84-90: 🛠️ Refactor suggestion | 🟠 Major补齐被修改公开异步方法的 XML 契约文档(含线程语义)
Line [87] 与 Line [232] 对应的方法已被本次 PR 修改,但当前注释仍缺少
<param> / <returns> / <exception> / <remarks>,且未明确ConfigureAwait(false)带来的线程延续语义,和仓库规范不一致。建议补丁
/// <summary> -/// 异步加载资源 +/// 异步加载指定路径的资源,并在缓存中进行并发去重。 /// </summary> +/// <param name="path">资源路径,不能为空或空白。</param> +/// <returns>加载成功返回资源实例;加载失败返回 <see langword="null"/>。</returns> +/// <exception cref="ArgumentException">当 <paramref name="path"/> 为空或空白时抛出。</exception> +/// <exception cref="InvalidOperationException">当未注册对应资源加载器时抛出。</exception> +/// <remarks>内部使用 <c>ConfigureAwait(false)</c>,后续延续不保证回到调用线程。</remarks> public async Task<T?> LoadAsync<T>(string path) where T : class ... /// <summary> -/// 预加载资源 +/// 预加载资源到缓存中。 /// </summary> +/// <param name="path">资源路径,不能为空或空白。</param> +/// <returns>表示预加载流程完成的任务。</returns> +/// <exception cref="ArgumentException">当 <paramref name="path"/> 为空或空白时抛出。</exception> +/// <exception cref="InvalidOperationException">当未注册对应资源加载器时抛出。</exception> +/// <remarks>内部委托给 <see cref="LoadAsync{T}(string)"/>,同样不捕获同步上下文。</remarks> public async Task PreloadAsync<T>(string path) where T : classAs per coding guidelines “All public, protected, and internal types and members MUST include XML documentation comments (
///) with<summary>,<param>,<returns>,<exception>, and<remarks>where applicable” and “If a member participates in lifecycle, threading … document that behavior explicitly”.Also applies to: 229-235
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Core/Resource/ResourceManager.cs` around lines 84 - 90, Update the XML documentation for the public async method LoadAsync<T>(string path) (and the other modified public async method around lines 229–235) to include a complete <summary>, <param name="path">, <returns>, and <exception> tags (e.g., ArgumentException for null/whitespace path), and add a <remarks> section that explicitly documents the threading/continuation semantics introduced by ConfigureAwait(false) (stating that continuations may run on a different thread/context and that callers should not assume original SynchronizationContext), following the repository's XML doc guidelines for public/protected/internal members.GFramework.Core/Functional/Result.cs (1)
120-129:⚠️ Potential issue | 🔴 Critical
default(Result)在Equals和GetHashCode中触发空引用异常
Result作为readonly struct可通过default(Result)构造,此时_isSuccess == false且_exception == null。当前代码用!压制编译器警告而非真正检查空值,导致Equals、GetHashCode及ToString在该路径执行时抛出NullReferenceException。需在解引用前验证_exception非空。建议修复
public bool Equals(Result other) { if (_isSuccess != other._isSuccess) return false; if (_isSuccess) return true; + if (_exception is null || other._exception is null) + return _exception is null && other._exception is null; + - return _exception!.GetType() == other._exception!.GetType() && + return _exception.GetType() == other._exception.GetType() && string.Equals(_exception.Message, other._exception.Message, StringComparison.Ordinal); } public override int GetHashCode() { - return _isSuccess ? 1 : HashCode.Combine(_exception!.GetType(), _exception.Message); + if (_isSuccess) + return 1; + + return _exception is null + ? 0 + : HashCode.Combine(_exception.GetType(), _exception.Message); } public override string ToString() => - _isSuccess ? "Success" : $"Fail({_exception!.Message})"; + _isSuccess ? "Success" : (_exception != null ? $"Fail({_exception.Message})" : "Fail(null)");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Core/Functional/Result.cs` around lines 120 - 129, The Equals, GetHashCode and ToString implementations dereference _exception using null-forgiving (!) without handling default(Result) where _exception can be null; update Result (readonly struct) to explicitly check _exception for null before accessing its members: in Equals(Result other) first handle cases where both _exception are null (treat as equal failure states) and where one is null (not equal), otherwise compare exception types and messages; in GetHashCode return a stable hash when _exception is null (e.g., based on _isSuccess and a constant) and combine _exception.GetType()/Message only when non-null; in ToString guard against null _exception and produce a sensible string for the default failure case. Ensure all checks reference the Result struct's _isSuccess and _exception fields and avoid using ! to suppress nullability.GFramework.Godot/Scene/SceneBehaviorBase.cs (1)
147-153:⚠️ Potential issue | 🔴 Critical在 Godot 异步操作后调用 Node API 前移除
ConfigureAwait(false),防止线程亲和性违反Lines 147、168、201 的
ConfigureAwait(false)导致续体可能在线程池线程上恢复,而随后的Owner.SetProcess*和QueueFreeX()调用必须在主线程执行。根据 Godot 4.1+ 的线程安全策略,这会触发运行时错误。移除这些 await 点上的ConfigureAwait(false),让续体按 Godot 的 SynchronizationContext 回到主线程。建议修复
- await _scene.OnPauseAsync().ConfigureAwait(false); + await _scene.OnPauseAsync(); - await _scene.OnResumeAsync().ConfigureAwait(false); + await _scene.OnResumeAsync(); - await _scene.OnUnloadAsync().ConfigureAwait(false); + await _scene.OnUnloadAsync();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot/Scene/SceneBehaviorBase.cs` around lines 147 - 153, Remove ConfigureAwait(false) from the awaits that precede calls into Godot Node APIs so continuations run on Godot's SynchronizationContext; specifically, change awaits like await _scene.OnPauseAsync().ConfigureAwait(false), await _scene.OnResumeAsync().ConfigureAwait(false), and await _scene.OnExitAsync().ConfigureAwait(false) to plain awaits so subsequent calls to Owner.SetProcess / Owner.SetPhysicsProcess / Owner.SetProcessInput and QueueFreeX() execute on the main thread.GFramework.Godot/Architectures/AbstractArchitecture.cs (1)
109-115:⚠️ Potential issue | 🔴 Critical在
await _anchor.WaitUntilReadyAsync().ConfigureAwait(false)后直接操作 Godot 对象存在主线程风险Line 109 的
ConfigureAwait(false)会破坏 Godot C# 同步上下文,导致后续代码在线程池线程上执行,而非主线程。虽然CallDeferred正确地将AddChild操作排队到主线程,但紧接着的module.OnAttach(this)被直接调用(在非主线程上)。如果模块实现的OnAttach访问节点属性或执行任何场景树操作,将违反 Godot 的线程安全约束。建议移除
ConfigureAwait(false),或将module.OnAttach(this)也用CallDeferred包装,或明确文档说明OnAttach实现不能依赖主线程。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot/Architectures/AbstractArchitecture.cs` around lines 109 - 115, The await on _anchor.WaitUntilReadyAsync() uses ConfigureAwait(false), which can resume on a thread-pool thread and causes module.OnAttach(this) to run off the Godot main thread; either remove ConfigureAwait(false) from the await so execution continues on Godot's synchronization context, or defer the callback by invoking module.OnAttach(this) via the anchor's CallDeferred (e.g., queue the OnAttach invocation using _anchor.CallDeferred) so OnAttach always runs on the main thread; update the code around _anchor.WaitUntilReadyAsync(), CallDeferred and module.OnAttach(this) accordingly.
🧹 Nitpick comments (5)
GFramework.Core/Configuration/ConfigurationManager.cs (1)
204-204:SaveToJson这里有一次可避免的中间字典分配。当前
ToDictionary会在每次序列化时创建完整副本;如果该路径调用频繁,建议考虑直接序列化现有快照/结构,减少分配压力。属于可延后优化。可选优化示例
- var dict = _configs.ToDictionary(kvp => kvp.Key, kvp => kvp.Value, StringComparer.Ordinal); - return JsonSerializer.Serialize(dict, new JsonSerializerOptions + return JsonSerializer.Serialize(_configs, new JsonSerializerOptions { WriteIndented = true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Core/Configuration/ConfigurationManager.cs` at line 204, The SaveToJson method currently creates an unnecessary intermediate copy via _configs.ToDictionary(...) (assigned to dict); instead avoid that allocation by serializing the existing snapshot/collection directly (e.g., pass _configs or a shallow read-only view/enumerable of _configs to the serializer) or by creating a lightweight array of KeyValuePairs only when needed—replace the ToDictionary call in SaveToJson with direct serialization of _configs (or _configs.ToArray()/AsEnumerable()) to eliminate the extra dict allocation while keeping thread-safety semantics.GFramework.Core/Resource/ResourceCache.cs (1)
55-58: 可选优化:考虑使用更细粒度的锁策略。当前使用单一
_lock对象保护所有条目的属性访问。由于ResourceCacheEntry的ReferenceCount和LastAccessTime是普通属性,可考虑:
- 将
ReferenceCount改为Interlocked操作(无锁原子递增/递减)- 或在
ResourceCacheEntry内部封装同步逻辑这不是本次 PR 范围内的问题,但如果未来此缓存成为高并发热点路径,值得重构。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Core/Resource/ResourceCache.cs` around lines 55 - 58, The current code uses a coarse-grained lock (_lock) to update entry.LastAccessTime which can become a contention point; to improve concurrency, move synchronization closer to the ResourceCacheEntry by either making ReferenceCount updates use Interlocked (e.g., Interlocked.Increment/Decrement) and update LastAccessTime without the global lock, or add a lightweight sync inside ResourceCacheEntry (e.g., a private object or volatile timestamp plus Interlocked for atomic updates) and change callers to call entry.UpdateLastAccessTime() and entry.Increment/DecrementRef() instead of locking _lock externally; modify methods referencing ReferenceCount and LastAccessTime to use these new atomic/internal-sync operations (symbols: ResourceCacheEntry, ReferenceCount, LastAccessTime, _lock, UpdateLastAccessTime, Increment/DecrementRef).GFramework.Godot/Setting/GodotGraphicsSettings.cs (1)
16-17: 建议更新 XML 文档以反映同步完成行为。虽然方法返回
Task,但实际上是同步完成的(直接返回Task.CompletedTask)。当前的文档注释<returns>异步任务</returns>可能会误导调用者认为方法会执行异步操作。建议在文档中明确说明该方法同步应用设置。📝 建议的文档改进
/// <summary> /// 应用图形设置到Godot引擎 /// </summary> - /// <returns>异步任务</returns> + /// <returns>已完成的任务(方法同步执行)</returns> public Task Apply()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot/Setting/GodotGraphicsSettings.cs` around lines 16 - 17, 更新 GodotGraphicsSettings.cs 中 Apply 方法的 XML 文档以反映其同步完成行为:在 Apply 的 <returns> 注释里说明方法立即完成并返回 Task.CompletedTask(即不是异步执行),并在方法摘要或备注中注明设置是同步应用的。定位符:Apply 方法(类 GodotGraphicsSettings)并替换或补充原有的 <returns> 注释以明确“同步完成”语义。GFramework.Core/Logging/AppenderConfiguration.cs (1)
1-1: 未使用的 using 指令。
GFramework.Core.Abstractions.Logging在此文件中未被直接使用。FilterConfiguration位于同一命名空间GFramework.Core.Logging,无需额外导入。♻️ 建议移除未使用的 using
-using GFramework.Core.Abstractions.Logging; - namespace GFramework.Core.Logging;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Core/Logging/AppenderConfiguration.cs` at line 1, Remove the unused using directive "using GFramework.Core.Abstractions.Logging" from AppenderConfiguration.cs because types like FilterConfiguration are in the same namespace GFramework.Core.Logging and the extra import is redundant; locate the top of the file where the using directive appears and delete that single line so the file compiles cleanly without unused usings.GFramework.Core/Logging/ConfigurableLoggerFactory.cs (1)
28-44: Dispose 方法非线程安全。并发调用
Dispose()时,_disposed检查与赋值之间存在竞态条件,可能导致 appenders 被多次释放。对于
internal sealed类型,如果确定调用方不会并发调用Dispose,这可以接受。否则建议添加同步机制。🔒 线程安全的 Dispose 实现
+ private int _disposed; public void Dispose() { - if (_disposed) - { - return; - } + if (Interlocked.Exchange(ref _disposed, 1) != 0) + return; foreach (var appender in _appenders) { if (appender is IDisposable disposable) { disposable.Dispose(); } } - - _disposed = true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Core/Logging/ConfigurableLoggerFactory.cs` around lines 28 - 44, The Dispose implementation in ConfigurableLoggerFactory is not thread-safe: concurrent calls can race between the _disposed check and assignment causing appenders to be disposed multiple times; make Dispose atomic by guarding the check-and-set with synchronization or an atomic operation (e.g., use a private readonly object _disposeLock and lock it around the existing _disposed check/assignment and the foreach over _appenders, or use Interlocked.Exchange to set a flag and null/replace _appenders before disposing). Ensure you reference Dispose, _disposed and _appenders when applying the fix so concurrent calls cannot enter the disposal loop twice.
🤖 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.Core/Functional/ResultExtensions.cs`:
- Around line 177-181: The ArgumentException created in ResultExtensions.Ensure
uses nameof(errorMessage) which incorrectly attributes the failure to the
errorMessage parameter; update the ArgumentException's paramName to a meaningful
identifier (e.g., nameof(result)) in the Result<T>.Fail(...) call inside Ensure
(the succ branch that returns Result<T>.Fail(new ArgumentException(...))). This
ensures the thrown ArgumentException points to the result/value context instead
of errorMessage.
In `@GFramework.Core/Ioc/MicrosoftDiContainer.cs`:
- Around line 395-398: The null-element check inside MicrosoftDiContainer (the
branch that tests "if (assembly is null)") currently throws ArgumentException
but the XML documentation promises ArgumentNullException; change the thrown
exception to ArgumentNullException for the null "assembly" element to preserve
the public API contract, and update any unit tests or the XML doc if you
intentionally want a different behavior instead.
In `@GFramework.Core/Logging/ConfigurableLoggerFactory.cs`:
- Around line 56-64: The current loop over _config.LoggerLevels can yield
nondeterministic prefix matches; change the matching logic in
ConfigurableLoggerFactory (where variables name and effectiveLevel are set) to
perform a longest-prefix match: iterate or search _config.LoggerLevels.Keys and
pick the key that either equals name or is a prefix of name (name.StartsWith(key
+ ".", StringComparison.Ordinal)) with the greatest key.Length, then set
effectiveLevel to that key's value; this ensures deterministic selection when
multiple namespace prefixes (e.g., "GFramework" and "GFramework.Core") could
match.
In `@GFramework.Core/Logging/Filters/SamplingFilter.cs`:
- Line 72: The single-line if statement in SamplingFilter that checks
string.Equals(kvp.Key, "*", StringComparison.Ordinal) should be converted to
Allman-brace style: replace the inline `if (...) continue;` with a full block
using braces so the continue is inside a braced block; locate the check in the
SamplingFilter class (the kvp.Key == "*" guard) and apply the Allman braces
convention to satisfy the repository C# style.
In `@GFramework.Cqrs/Internal/WeakKeyCache.cs`:
- Around line 47-48: The XML docs for the two overloads that call valueFactory
must be updated to reflect the actual exception thrown: change the existing
<exception> entries that say ArgumentNullException to instead document
InvalidOperationException for the case where valueFactory returns null (the code
path that throws new InvalidOperationException("The value factory returned
null.")). Update both overloads' <exception> text entries so they name
InvalidOperationException and describe that it is thrown when the provided
valueFactory returns null (reference the valueFactory usage in the WeakKeyCache
methods that create createdValue).
In `@GFramework.Cqrs/Internal/WeakTypePairCache.cs`:
- Around line 24-27: The XML docs for GetOrAdd currently only list
ArgumentNullException but not the InvalidOperationException that can be thrown
when the provided valueFactory returns null via WeakKeyCache.GetOrAdd; update
the <exception> section for GetOrAdd to also document InvalidOperationException
(thrown when valueFactory yields null) and mention the conditions (e.g.,
valueFactory returns null) and include the existing ArgumentNullException note
for primaryType/secondaryType/valueFactory being null; reference the GetOrAdd
method and the WeakKeyCache.GetOrAdd call in the comment to make the contract
explicit.
- Around line 3-11: Add thread-safety and failure-mode notes to the XML docs for
the WeakTypePairCache class: state that WeakTypePairCache is safe for concurrent
readers and writers for its public methods (GetOrAdd, TryGetValue, Clear),
describe atomicity guarantees (e.g., GetOrAdd provides a single insertion per
key-pair even under races), and clarify failure modes that entries can be
collected by GC or removed by Clear causing subsequent calls to miss and rebuild
values; also mention that stored TValue instances are not protected from
external mutation and consumers must handle synchronization if needed.
In `@GFramework.Godot/Architectures/AbstractArchitecture.cs`:
- Line 88: The fire-and-forget call bound via _anchor.Bind(() => _ =
DestroyAsync().AsTask()) can swallow exceptions; modify the binding so
DestroyAsync() exceptions are observed and logged (either by awaiting the Task
or attaching a continuation with try/catch and logging) instead of discarding
with `_ =`; alternatively add a top-level try/catch inside DestroyAsync()
(similar to Initialize()/InitializeAsync()) to log and surface errors via
GD.PushError or the existing logger; ensure this prevents silent failures during
_ExitTree() cleanup by observing the Task and recording any exception
information.
In `@GFramework.Godot/Config/GodotYamlConfigEnvironment.cs`:
- Around line 157-162: The Godot branch in ReadAllBytesCore currently calls
FileAccess.GetFileAsBytes but doesn't respect the documented contract to throw
on failure; after calling FileAccess.GetFileAsBytes(path) check
FileAccess.GetOpenError() and if it indicates an error, throw an exception
(e.g., propagate a FileNotFoundException/IOException containing the error
information) so the loader can preserve the failure reason; otherwise return the
byte[] as before — ensure behavior matches the File.ReadAllBytes path.
In `@GFramework.Godot/Setting/GodotGraphicsSettings.cs`:
- Line 17: Rename the asynchronous API on the interface and all implementations:
change the interface method IApplyAbleSettings.Apply() to Task ApplyAsync(),
then update GodotGraphicsSettings.Apply() to GodotGraphicsSettings.ApplyAsync(),
and likewise rename and refactor every class implementing IApplyAbleSettings to
implement ApplyAsync() instead of Apply(); ensure all callers and overrides are
updated to call ApplyAsync(), preserve behavior and signatures (returning Task),
and update any XML docs/comments to reflect the new ApplyAsync name.
---
Outside diff comments:
In `@GFramework.Core/Coroutine/Instructions/WaitForTask.cs`:
- Around line 5-10: 在类型声明 WaitForTask 前存在两个完全相同的 XML <summary> 注释块(重复的第 5-7 行和第
8-10 行),请删除多余的一个保留单个 <summary> 描述以消除重复并符合编码标准;检查 WaitForTask
类/struct/文件头部以确保仅保留一套 XML 文档注释并保持其内容准确无改动。
In `@GFramework.Core/Extensions/NumericExtensions.cs`:
- Around line 26-34: The Between<T> extension currently lacks null checks and
can throw NREs; update Between<T>(this T value, T min, T max, bool inclusive =
true) to explicitly validate value, min, and max for null and throw
ArgumentNullException (e.g. for value, nameof(value); for min, nameof(min); for
max, nameof(max)) before any CompareTo calls, then retain the existing min>max
ArgumentException and the inclusive/exclusive comparisons so the behavior
matches the documented contract and nullable annotations.
In `@GFramework.Core/Functional/Result.cs`:
- Around line 120-129: The Equals, GetHashCode and ToString implementations
dereference _exception using null-forgiving (!) without handling default(Result)
where _exception can be null; update Result (readonly struct) to explicitly
check _exception for null before accessing its members: in Equals(Result other)
first handle cases where both _exception are null (treat as equal failure
states) and where one is null (not equal), otherwise compare exception types and
messages; in GetHashCode return a stable hash when _exception is null (e.g.,
based on _isSuccess and a constant) and combine _exception.GetType()/Message
only when non-null; in ToString guard against null _exception and produce a
sensible string for the default failure case. Ensure all checks reference the
Result struct's _isSuccess and _exception fields and avoid using ! to suppress
nullability.
In `@GFramework.Core/Resource/ResourceManager.cs`:
- Around line 84-90: Update the XML documentation for the public async method
LoadAsync<T>(string path) (and the other modified public async method around
lines 229–235) to include a complete <summary>, <param name="path">, <returns>,
and <exception> tags (e.g., ArgumentException for null/whitespace path), and add
a <remarks> section that explicitly documents the threading/continuation
semantics introduced by ConfigureAwait(false) (stating that continuations may
run on a different thread/context and that callers should not assume original
SynchronizationContext), following the repository's XML doc guidelines for
public/protected/internal members.
In `@GFramework.Godot/Architectures/AbstractArchitecture.cs`:
- Around line 109-115: The await on _anchor.WaitUntilReadyAsync() uses
ConfigureAwait(false), which can resume on a thread-pool thread and causes
module.OnAttach(this) to run off the Godot main thread; either remove
ConfigureAwait(false) from the await so execution continues on Godot's
synchronization context, or defer the callback by invoking module.OnAttach(this)
via the anchor's CallDeferred (e.g., queue the OnAttach invocation using
_anchor.CallDeferred) so OnAttach always runs on the main thread; update the
code around _anchor.WaitUntilReadyAsync(), CallDeferred and
module.OnAttach(this) accordingly.
In `@GFramework.Godot/Scene/SceneBehaviorBase.cs`:
- Around line 147-153: Remove ConfigureAwait(false) from the awaits that precede
calls into Godot Node APIs so continuations run on Godot's
SynchronizationContext; specifically, change awaits like await
_scene.OnPauseAsync().ConfigureAwait(false), await
_scene.OnResumeAsync().ConfigureAwait(false), and await
_scene.OnExitAsync().ConfigureAwait(false) to plain awaits so subsequent calls
to Owner.SetProcess / Owner.SetPhysicsProcess / Owner.SetProcessInput and
QueueFreeX() execute on the main thread.
In `@GFramework.Godot/UI/UiPageBehaviorFactory.cs`:
- Around line 25-33: The public generic method Create<T>(T owner, string key,
UiLayer layer) throws ArgumentException (see the throw at the body) but its XML
docblock lacks an <exception> tag; add an <exception
cref="System.ArgumentException"> entry describing when and why the
ArgumentException is thrown (e.g., invalid or unsupported layer/key or owner
type) in the method's XML comments alongside existing <summary>, <param>, and
<returns> tags so the API contract is complete for Create<T>.
---
Nitpick comments:
In `@GFramework.Core/Configuration/ConfigurationManager.cs`:
- Line 204: The SaveToJson method currently creates an unnecessary intermediate
copy via _configs.ToDictionary(...) (assigned to dict); instead avoid that
allocation by serializing the existing snapshot/collection directly (e.g., pass
_configs or a shallow read-only view/enumerable of _configs to the serializer)
or by creating a lightweight array of KeyValuePairs only when needed—replace the
ToDictionary call in SaveToJson with direct serialization of _configs (or
_configs.ToArray()/AsEnumerable()) to eliminate the extra dict allocation while
keeping thread-safety semantics.
In `@GFramework.Core/Logging/AppenderConfiguration.cs`:
- Line 1: Remove the unused using directive "using
GFramework.Core.Abstractions.Logging" from AppenderConfiguration.cs because
types like FilterConfiguration are in the same namespace GFramework.Core.Logging
and the extra import is redundant; locate the top of the file where the using
directive appears and delete that single line so the file compiles cleanly
without unused usings.
In `@GFramework.Core/Logging/ConfigurableLoggerFactory.cs`:
- Around line 28-44: The Dispose implementation in ConfigurableLoggerFactory is
not thread-safe: concurrent calls can race between the _disposed check and
assignment causing appenders to be disposed multiple times; make Dispose atomic
by guarding the check-and-set with synchronization or an atomic operation (e.g.,
use a private readonly object _disposeLock and lock it around the existing
_disposed check/assignment and the foreach over _appenders, or use
Interlocked.Exchange to set a flag and null/replace _appenders before
disposing). Ensure you reference Dispose, _disposed and _appenders when applying
the fix so concurrent calls cannot enter the disposal loop twice.
In `@GFramework.Core/Resource/ResourceCache.cs`:
- Around line 55-58: The current code uses a coarse-grained lock (_lock) to
update entry.LastAccessTime which can become a contention point; to improve
concurrency, move synchronization closer to the ResourceCacheEntry by either
making ReferenceCount updates use Interlocked (e.g.,
Interlocked.Increment/Decrement) and update LastAccessTime without the global
lock, or add a lightweight sync inside ResourceCacheEntry (e.g., a private
object or volatile timestamp plus Interlocked for atomic updates) and change
callers to call entry.UpdateLastAccessTime() and entry.Increment/DecrementRef()
instead of locking _lock externally; modify methods referencing ReferenceCount
and LastAccessTime to use these new atomic/internal-sync operations (symbols:
ResourceCacheEntry, ReferenceCount, LastAccessTime, _lock, UpdateLastAccessTime,
Increment/DecrementRef).
In `@GFramework.Godot/Setting/GodotGraphicsSettings.cs`:
- Around line 16-17: 更新 GodotGraphicsSettings.cs 中 Apply 方法的 XML 文档以反映其同步完成行为:在
Apply 的 <returns> 注释里说明方法立即完成并返回
Task.CompletedTask(即不是异步执行),并在方法摘要或备注中注明设置是同步应用的。定位符:Apply 方法(类
GodotGraphicsSettings)并替换或补充原有的 <returns> 注释以明确“同步完成”语义。
🪄 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: 3d2ee7b7-99f2-4523-944b-5818b0d57d40
📒 Files selected for processing (74)
GFramework.Core.SourceGenerators/Analyzers/ContextRegistrationAnalyzer.csGFramework.Core.SourceGenerators/Analyzers/PriorityUsageAnalyzer.csGFramework.Core.SourceGenerators/Architectures/AutoRegisterModuleGenerator.csGFramework.Core.SourceGenerators/Diagnostics/LoggerDiagnostics.csGFramework.Core.SourceGenerators/Enums/EnumExtensionsGenerator.csGFramework.Core.Tests/State/StateMachineSystemTests.csGFramework.Core/Architectures/Architecture.csGFramework.Core/Architectures/ArchitectureBootstrapper.csGFramework.Core/Architectures/ArchitectureContext.csGFramework.Core/Architectures/ArchitectureDisposer.csGFramework.Core/Architectures/ArchitectureLifecycle.csGFramework.Core/Command/AbstractAsyncCommand.csGFramework.Core/Command/AbstractAsyncCommandWithInput.csGFramework.Core/Command/AbstractAsyncCommandWithResult.csGFramework.Core/Concurrency/AsyncKeyLockManager.csGFramework.Core/Configuration/ConfigurationManager.csGFramework.Core/Coroutine/CoroutineHandle.csGFramework.Core/Coroutine/CoroutineScheduler.csGFramework.Core/Coroutine/CoroutineStatistics.csGFramework.Core/Coroutine/Instructions/WaitForTask.T.csGFramework.Core/Coroutine/Instructions/WaitForTask.csGFramework.Core/Environment/EnvironmentBase.csGFramework.Core/Events/EventStatistics.csGFramework.Core/Extensions/AsyncExtensions.csGFramework.Core/Extensions/ContextAwareCommandExtensions.csGFramework.Core/Extensions/ContextAwareQueryExtensions.csGFramework.Core/Extensions/NumericExtensions.csGFramework.Core/Functional/Async/AsyncFunctionalExtensions.csGFramework.Core/Functional/Result.T.csGFramework.Core/Functional/Result.csGFramework.Core/Functional/ResultExtensions.csGFramework.Core/Ioc/MicrosoftDiContainer.csGFramework.Core/Localization/Formatters/CompactNumberLocalizationFormatter.csGFramework.Core/Localization/LocalizationManager.csGFramework.Core/Localization/LocalizationString.csGFramework.Core/Localization/LocalizationTable.csGFramework.Core/Logging/AppenderConfiguration.csGFramework.Core/Logging/Appenders/AsyncLogAppender.csGFramework.Core/Logging/Appenders/StatisticsAppender.csGFramework.Core/Logging/CachedLoggerFactory.csGFramework.Core/Logging/CompositeLogger.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Core/Logging/ConsoleLogger.csGFramework.Core/Logging/FilterConfiguration.csGFramework.Core/Logging/Filters/SamplingFilter.csGFramework.Core/Logging/Formatters/DefaultLogFormatter.csGFramework.Core/Logging/Formatters/JsonLogFormatter.csGFramework.Core/Logging/LoggingConfiguration.csGFramework.Core/Logging/LoggingConfigurationLoader.csGFramework.Core/Pool/AbstractObjectPoolSystem.csGFramework.Core/Resource/ResourceCache.csGFramework.Core/Resource/ResourceCacheEntry.csGFramework.Core/Resource/ResourceManager.csGFramework.Core/Services/ServiceModuleManager.csGFramework.Core/State/StateMachine.csGFramework.Core/State/StateMachineSystem.csGFramework.Cqrs/Cqrs/Behaviors/LoggingBehavior.csGFramework.Cqrs/Cqrs/Behaviors/PerformanceBehavior.csGFramework.Cqrs/Internal/CqrsDispatcher.csGFramework.Cqrs/Internal/WeakKeyCache.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Godot/Config/GodotYamlConfigDirectoryEntry.csGFramework.Godot/Config/GodotYamlConfigEnvironment.csGFramework.Godot/Config/GodotYamlConfigLoader.csGFramework.Godot/Data/GodotResourceRepository.csGFramework.Godot/Extensions/GodotPathExtensions.csGFramework.Godot/Extensions/NodeExtensions.csGFramework.Godot/Logging/GodotLogger.csGFramework.Godot/Scene/SceneBehaviorBase.csGFramework.Godot/Setting/Data/LocalizationMap.csGFramework.Godot/Setting/GodotGraphicsSettings.csGFramework.Godot/Storage/GodotFileStorage.csGFramework.Godot/UI/UiPageBehaviorFactory.cs
💤 Files with no reviewable changes (1)
- GFramework.Core/Logging/LoggingConfigurationLoader.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). (2)
- GitHub Check: Code Quality & Security
- GitHub Check: Analyze (C#)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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
XML documentation comments must explain intent, contract, and usage constraints instead of restating syntax
If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds, compatibility constraints or edge cases, and registration order or lifecycle sequencing
Avoid obvious comments such as// increment i
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
Generated logic and generator pipelines MUST explain what is generated, why it is generated, the semantic assumptions the generator relies on, and any diagnostics or fallback behavior
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling if any
Comments MUST NOT be trivial, redundant, or misleading; prefer explainingwhyandwhen, not justwhat
Prefer slightly more explanation over too little for framework code
Do not rely on implicit imports; declare every requiredusingexplicitly
Write null-safe code that respects nullable annotations instead of suppressing warnings by default
Use the namespac...
Files:
GFramework.Core/Command/AbstractAsyncCommand.csGFramework.Core/Extensions/AsyncExtensions.csGFramework.Core/Functional/Result.csGFramework.Cqrs/Cqrs/Behaviors/PerformanceBehavior.csGFramework.Core/Extensions/NumericExtensions.csGFramework.Core/Architectures/ArchitectureDisposer.csGFramework.Core/Logging/Appenders/AsyncLogAppender.csGFramework.Core/Coroutine/Instructions/WaitForTask.T.csGFramework.Cqrs/Cqrs/Behaviors/LoggingBehavior.csGFramework.Core/Coroutine/CoroutineHandle.csGFramework.Core/Command/AbstractAsyncCommandWithInput.csGFramework.Core.SourceGenerators/Analyzers/ContextRegistrationAnalyzer.csGFramework.Godot/UI/UiPageBehaviorFactory.csGFramework.Core/Logging/CachedLoggerFactory.csGFramework.Core/Logging/Formatters/DefaultLogFormatter.csGFramework.Core/Logging/CompositeLogger.csGFramework.Core/Architectures/Architecture.csGFramework.Core/Coroutine/CoroutineScheduler.csGFramework.Godot/Scene/SceneBehaviorBase.csGFramework.Godot/Extensions/GodotPathExtensions.csGFramework.Godot/Extensions/NodeExtensions.csGFramework.Core/Localization/Formatters/CompactNumberLocalizationFormatter.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Core/Logging/ConsoleLogger.csGFramework.Core/Events/EventStatistics.csGFramework.Core/Coroutine/Instructions/WaitForTask.csGFramework.Godot/Data/GodotResourceRepository.csGFramework.Core/Extensions/ContextAwareCommandExtensions.csGFramework.Core/Ioc/MicrosoftDiContainer.csGFramework.Core/Configuration/ConfigurationManager.csGFramework.Core/Logging/Formatters/JsonLogFormatter.csGFramework.Core/Architectures/ArchitectureBootstrapper.csGFramework.Core/Architectures/ArchitectureLifecycle.csGFramework.Core/Resource/ResourceManager.csGFramework.Godot/Setting/GodotGraphicsSettings.csGFramework.Core/State/StateMachineSystem.csGFramework.Core/Command/AbstractAsyncCommandWithResult.csGFramework.Core/Functional/Result.T.csGFramework.Core/Functional/Async/AsyncFunctionalExtensions.csGFramework.Core/Localization/LocalizationString.csGFramework.Core/Pool/AbstractObjectPoolSystem.csGFramework.Godot/Logging/GodotLogger.csGFramework.Core/Coroutine/CoroutineStatistics.csGFramework.Core/Resource/ResourceCache.csGFramework.Cqrs/Internal/CqrsDispatcher.csGFramework.Core.SourceGenerators/Architectures/AutoRegisterModuleGenerator.csGFramework.Core/Services/ServiceModuleManager.csGFramework.Core.SourceGenerators/Enums/EnumExtensionsGenerator.csGFramework.Core/Architectures/ArchitectureContext.csGFramework.Cqrs/Internal/WeakKeyCache.csGFramework.Godot/Config/GodotYamlConfigLoader.csGFramework.Core/Logging/LoggingConfiguration.csGFramework.Core/Logging/Appenders/StatisticsAppender.csGFramework.Godot/Config/GodotYamlConfigDirectoryEntry.csGFramework.Core/Resource/ResourceCacheEntry.csGFramework.Godot/Setting/Data/LocalizationMap.csGFramework.Core/Localization/LocalizationManager.csGFramework.Core/State/StateMachine.csGFramework.Core/Logging/AppenderConfiguration.csGFramework.Core/Extensions/ContextAwareQueryExtensions.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Core/Logging/Filters/SamplingFilter.csGFramework.Core.Tests/State/StateMachineSystemTests.csGFramework.Core/Functional/ResultExtensions.csGFramework.Core.SourceGenerators/Analyzers/PriorityUsageAnalyzer.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Core/Environment/EnvironmentBase.csGFramework.Core/Logging/FilterConfiguration.csGFramework.Core/Concurrency/AsyncKeyLockManager.csGFramework.Godot/Config/GodotYamlConfigEnvironment.csGFramework.Godot/Storage/GodotFileStorage.csGFramework.Core/Localization/LocalizationTable.cs
**/*.{cs,csproj,xml,yaml,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation; do not use tabs
Files:
GFramework.Core/Command/AbstractAsyncCommand.csGFramework.Core/Extensions/AsyncExtensions.csGFramework.Core/Functional/Result.csGFramework.Cqrs/Cqrs/Behaviors/PerformanceBehavior.csGFramework.Core/Extensions/NumericExtensions.csGFramework.Core/Architectures/ArchitectureDisposer.csGFramework.Core/Logging/Appenders/AsyncLogAppender.csGFramework.Core/Coroutine/Instructions/WaitForTask.T.csGFramework.Cqrs/Cqrs/Behaviors/LoggingBehavior.csGFramework.Core/Coroutine/CoroutineHandle.csGFramework.Core/Command/AbstractAsyncCommandWithInput.csGFramework.Core.SourceGenerators/Analyzers/ContextRegistrationAnalyzer.csGFramework.Godot/UI/UiPageBehaviorFactory.csGFramework.Core/Logging/CachedLoggerFactory.csGFramework.Core/Logging/Formatters/DefaultLogFormatter.csGFramework.Core/Logging/CompositeLogger.csGFramework.Core/Architectures/Architecture.csGFramework.Core/Coroutine/CoroutineScheduler.csGFramework.Godot/Scene/SceneBehaviorBase.csGFramework.Godot/Extensions/GodotPathExtensions.csGFramework.Godot/Extensions/NodeExtensions.csGFramework.Core/Localization/Formatters/CompactNumberLocalizationFormatter.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Core/Logging/ConsoleLogger.csGFramework.Core/Events/EventStatistics.csGFramework.Core/Coroutine/Instructions/WaitForTask.csGFramework.Godot/Data/GodotResourceRepository.csGFramework.Core/Extensions/ContextAwareCommandExtensions.csGFramework.Core/Ioc/MicrosoftDiContainer.csGFramework.Core/Configuration/ConfigurationManager.csGFramework.Core/Logging/Formatters/JsonLogFormatter.csGFramework.Core/Architectures/ArchitectureBootstrapper.csGFramework.Core/Architectures/ArchitectureLifecycle.csGFramework.Core/Resource/ResourceManager.csGFramework.Godot/Setting/GodotGraphicsSettings.csGFramework.Core/State/StateMachineSystem.csGFramework.Core/Command/AbstractAsyncCommandWithResult.csGFramework.Core/Functional/Result.T.csGFramework.Core/Functional/Async/AsyncFunctionalExtensions.csGFramework.Core/Localization/LocalizationString.csGFramework.Core/Pool/AbstractObjectPoolSystem.csGFramework.Godot/Logging/GodotLogger.csGFramework.Core/Coroutine/CoroutineStatistics.csGFramework.Core/Resource/ResourceCache.csGFramework.Cqrs/Internal/CqrsDispatcher.csGFramework.Core.SourceGenerators/Architectures/AutoRegisterModuleGenerator.csGFramework.Core/Services/ServiceModuleManager.csGFramework.Core.SourceGenerators/Enums/EnumExtensionsGenerator.csGFramework.Core/Architectures/ArchitectureContext.csGFramework.Cqrs/Internal/WeakKeyCache.csGFramework.Godot/Config/GodotYamlConfigLoader.csGFramework.Core/Logging/LoggingConfiguration.csGFramework.Core/Logging/Appenders/StatisticsAppender.csGFramework.Godot/Config/GodotYamlConfigDirectoryEntry.csGFramework.Core/Resource/ResourceCacheEntry.csGFramework.Godot/Setting/Data/LocalizationMap.csGFramework.Core/Localization/LocalizationManager.csGFramework.Core/State/StateMachine.csGFramework.Core/Logging/AppenderConfiguration.csGFramework.Core/Extensions/ContextAwareQueryExtensions.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Core/Logging/Filters/SamplingFilter.csGFramework.Core.Tests/State/StateMachineSystemTests.csGFramework.Core/Functional/ResultExtensions.csGFramework.Core.SourceGenerators/Analyzers/PriorityUsageAnalyzer.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Core/Environment/EnvironmentBase.csGFramework.Core/Logging/FilterConfiguration.csGFramework.Core/Concurrency/AsyncKeyLockManager.csGFramework.Godot/Config/GodotYamlConfigEnvironment.csGFramework.Godot/Storage/GodotFileStorage.csGFramework.Core/Localization/LocalizationTable.cs
**/Cqrs/**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
Use CQRS (Command Query Responsibility Segregation) pattern with the Cqrs naming entry point instead of the historical Mediator alias
Files:
GFramework.Cqrs/Cqrs/Behaviors/PerformanceBehavior.csGFramework.Cqrs/Cqrs/Behaviors/LoggingBehavior.cs
**/*SourceGenerators/**/*.cs
📄 CodeRabbit inference engine (AGENTS.md)
Keep source generators deterministic and free of hidden environment or network dependencies
Files:
GFramework.Core.SourceGenerators/Analyzers/ContextRegistrationAnalyzer.csGFramework.Core.SourceGenerators/Architectures/AutoRegisterModuleGenerator.csGFramework.Core.SourceGenerators/Enums/EnumExtensionsGenerator.csGFramework.Core.SourceGenerators/Analyzers/PriorityUsageAnalyzer.cs
**/*{Startup,Init,Register,Setup,Configure}*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
CQRS handler registration should use generated products from CqrsHandlerRegistryGenerator at runtime, falling back to reflection scanning when generation cannot cover cases; explicitly register handlers from non-default assemblies using RegisterCqrsHandlersFromAssembly(...) or RegisterCqrsHandlersFromAssemblies(...)
Files:
GFramework.Core.SourceGenerators/Architectures/AutoRegisterModuleGenerator.cs
🧠 Learnings (18)
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Match existing async patterns and naming conventions; use `Async` suffix for asynchronous methods
Applied to files:
GFramework.Core/Command/AbstractAsyncCommand.csGFramework.Core/Extensions/AsyncExtensions.csGFramework.Core/Logging/Appenders/AsyncLogAppender.csGFramework.Godot/Scene/SceneBehaviorBase.csGFramework.Godot/Extensions/NodeExtensions.csGFramework.Core/Resource/ResourceManager.csGFramework.Godot/Setting/GodotGraphicsSettings.csGFramework.Core/Functional/Result.T.csGFramework.Core/Functional/Async/AsyncFunctionalExtensions.cs
📚 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.Core/Command/AbstractAsyncCommand.csGFramework.Core/Extensions/AsyncExtensions.csGFramework.Core/Functional/Result.csGFramework.Cqrs/Cqrs/Behaviors/PerformanceBehavior.csGFramework.Core/Extensions/NumericExtensions.csGFramework.Core/Architectures/ArchitectureDisposer.csGFramework.Core/Logging/Appenders/AsyncLogAppender.csGFramework.Core/Coroutine/Instructions/WaitForTask.T.csGFramework.Cqrs/Cqrs/Behaviors/LoggingBehavior.csGFramework.Core/Coroutine/CoroutineHandle.csGFramework.Core/Command/AbstractAsyncCommandWithInput.csGFramework.Core.SourceGenerators/Analyzers/ContextRegistrationAnalyzer.csGFramework.Godot/UI/UiPageBehaviorFactory.csGFramework.Core/Logging/CachedLoggerFactory.csGFramework.Core/Logging/Formatters/DefaultLogFormatter.csGFramework.Core/Logging/CompositeLogger.csGFramework.Core/Architectures/Architecture.csGFramework.Core/Coroutine/CoroutineScheduler.csGFramework.Godot/Scene/SceneBehaviorBase.csGFramework.Godot/Extensions/GodotPathExtensions.csGFramework.Godot/Extensions/NodeExtensions.csGFramework.Core/Localization/Formatters/CompactNumberLocalizationFormatter.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Core/Logging/ConsoleLogger.csGFramework.Core/Events/EventStatistics.csGFramework.Core/Coroutine/Instructions/WaitForTask.csGFramework.Godot/Data/GodotResourceRepository.csGFramework.Core/Extensions/ContextAwareCommandExtensions.csGFramework.Core/Ioc/MicrosoftDiContainer.csGFramework.Core/Configuration/ConfigurationManager.csGFramework.Core/Logging/Formatters/JsonLogFormatter.csGFramework.Core/Architectures/ArchitectureBootstrapper.csGFramework.Core/Architectures/ArchitectureLifecycle.csGFramework.Core/Resource/ResourceManager.csGFramework.Godot/Setting/GodotGraphicsSettings.csGFramework.Core/State/StateMachineSystem.csGFramework.Core/Command/AbstractAsyncCommandWithResult.csGFramework.Core/Functional/Result.T.csGFramework.Core/Functional/Async/AsyncFunctionalExtensions.csGFramework.Core/Localization/LocalizationString.csGFramework.Core/Pool/AbstractObjectPoolSystem.csGFramework.Godot/Logging/GodotLogger.csGFramework.Core/Coroutine/CoroutineStatistics.csGFramework.Core/Resource/ResourceCache.csGFramework.Cqrs/Internal/CqrsDispatcher.csGFramework.Core.SourceGenerators/Architectures/AutoRegisterModuleGenerator.csGFramework.Core/Services/ServiceModuleManager.csGFramework.Core.SourceGenerators/Enums/EnumExtensionsGenerator.csGFramework.Core/Architectures/ArchitectureContext.csGFramework.Cqrs/Internal/WeakKeyCache.csGFramework.Godot/Config/GodotYamlConfigLoader.csGFramework.Core/Logging/LoggingConfiguration.csGFramework.Core/Logging/Appenders/StatisticsAppender.csGFramework.Godot/Config/GodotYamlConfigDirectoryEntry.csGFramework.Core/Resource/ResourceCacheEntry.csGFramework.Godot/Setting/Data/LocalizationMap.csGFramework.Core/Localization/LocalizationManager.csGFramework.Core/State/StateMachine.csGFramework.Core/Logging/AppenderConfiguration.csGFramework.Core/Extensions/ContextAwareQueryExtensions.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Core/Logging/Filters/SamplingFilter.csGFramework.Core.Tests/State/StateMachineSystemTests.csGFramework.Core/Functional/ResultExtensions.csGFramework.Core.SourceGenerators/Analyzers/PriorityUsageAnalyzer.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Core/Environment/EnvironmentBase.csGFramework.Core/Logging/FilterConfiguration.csGFramework.Core/Concurrency/AsyncKeyLockManager.csGFramework.Godot/Config/GodotYamlConfigEnvironment.csGFramework.Godot/Storage/GodotFileStorage.csGFramework.Core/Localization/LocalizationTable.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.Tests.cs : Reuse existing architecture test infrastructure when relevant: `ArchitectureTestsBase<T>`, `SyncTestArchitecture`, `AsyncTestArchitecture`
Applied to files:
GFramework.Core/Architectures/ArchitectureDisposer.csGFramework.Core/Architectures/Architecture.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Core/Architectures/ArchitectureLifecycle.csGFramework.Core/Architectures/ArchitectureContext.cs
📚 Learning: 2026-04-17T11:35:08.735Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-17T11:35:08.735Z
Learning: Lifecycle management in Architecture implementations should handle Init, Ready, and Destroy phases with fine-grained initialization and destruction stages to maintain order consistency across Utility, Model, System, service modules, and hooks
Applied to files:
GFramework.Core/Architectures/ArchitectureDisposer.csGFramework.Core/Architectures/Architecture.csGFramework.Core/Architectures/ArchitectureLifecycle.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Avoid hidden side effects in property getters, constructors, and registration helpers
Applied to files:
GFramework.Core.SourceGenerators/Analyzers/ContextRegistrationAnalyzer.cs
📚 Learning: 2026-04-17T11:35:08.735Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-17T11:35:08.735Z
Learning: Applies to **/*{Startup,Init,Register,Setup,Configure}*.cs : CQRS handler registration should use generated products from CqrsHandlerRegistryGenerator at runtime, falling back to reflection scanning when generation cannot cover cases; explicitly register handlers from non-default assemblies using RegisterCqrsHandlersFromAssembly(...) or RegisterCqrsHandlersFromAssemblies(...)
Applied to files:
GFramework.Core.SourceGenerators/Analyzers/ContextRegistrationAnalyzer.csGFramework.Core/Ioc/MicrosoftDiContainer.csGFramework.Core.SourceGenerators/Architectures/AutoRegisterModuleGenerator.cs
📚 Learning: 2026-04-16T13:25:59.124Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 235
File: GFramework.SourceGenerators.Tests/Rule/ContextAwareGeneratorSnapshotTests.cs:1-3
Timestamp: 2026-04-16T13:25:59.124Z
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.Core/Logging/Formatters/DefaultLogFormatter.csGFramework.Core/Localization/Formatters/CompactNumberLocalizationFormatter.csGFramework.Core/Logging/ConsoleLogger.csGFramework.Core/Configuration/ConfigurationManager.csGFramework.Godot/Logging/GodotLogger.csGFramework.Core/Coroutine/CoroutineStatistics.csGFramework.Core/Logging/Appenders/StatisticsAppender.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Use the namespace pattern `GFramework.{Module}.{Feature}` with PascalCase segments
Applied to files:
GFramework.Core/Logging/ConsoleLogger.cs
📚 Learning: 2026-04-17T11:35:08.735Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-17T11:35:08.735Z
Learning: Applies to **/Cqrs/**/*.cs : Use CQRS (Command Query Responsibility Segregation) pattern with the Cqrs naming entry point instead of the historical Mediator alias
Applied to files:
GFramework.Core/Ioc/MicrosoftDiContainer.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : 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
Applied to files:
GFramework.Core/Architectures/ArchitectureLifecycle.cs
📚 Learning: 2026-04-17T11:35:08.735Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-17T11:35:08.735Z
Learning: Coroutine implementations should be frame-driven based on IYieldInstruction abstraction and scheduler abstraction to support common patterns like waiting for time, events, and task completion
Applied to files:
GFramework.Core/Coroutine/CoroutineStatistics.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Any change to public API, lifecycle semantics, module behavior, or extension points MUST update the related XML docs
Applied to files:
GFramework.Core/Resource/ResourceCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*SourceGenerators/**/*.cs : Keep source generators deterministic and free of hidden environment or network dependencies
Applied to files:
GFramework.Core.SourceGenerators/Architectures/AutoRegisterModuleGenerator.csGFramework.Core.SourceGenerators/Enums/EnumExtensionsGenerator.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*SourceGenerators*.Tests/**/*.cs : Source generator changes MUST be covered by generator tests
Applied to files:
GFramework.Core.SourceGenerators/Architectures/AutoRegisterModuleGenerator.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*SourceGenerators*.Tests/**/*.cs : When generator behavior changes intentionally, update snapshots together with the implementation
Applied to files:
GFramework.Core.SourceGenerators/Architectures/AutoRegisterModuleGenerator.cs
📚 Learning: 2026-04-17T11:35:08.735Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-17T11:35:08.735Z
Learning: Applies to **/*.cs : Apply [GenerateEnumExtensions] attribute to generate enumeration extension capabilities
Applied to files:
GFramework.Core.SourceGenerators/Enums/EnumExtensionsGenerator.cs
📚 Learning: 2026-04-17T11:35:08.735Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-17T11:35:08.735Z
Learning: Applies to **/*.cs : Apply [Priority] attribute for automatic priority comparison implementation generation
Applied to files:
GFramework.Core.SourceGenerators/Analyzers/PriorityUsageAnalyzer.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : When a method approaches analyzer complexity limits, prefer extracting named helper methods by semantic phase (parsing, normalization, validation, diagnostics) instead of silencing the warning or doing cosmetic reshuffles
Applied to files:
GFramework.Core.SourceGenerators/Analyzers/PriorityUsageAnalyzer.cs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
GFramework.Core/Functional/ResultExtensions.cs (1)
95-110:⚠️ Potential issue | 🟠 Major请为本次触达的公开方法补齐 XML 文档合同标签
这些公开扩展方法当前仅有
<summary>,缺少<param>、<returns>,并且在可抛异常场景下缺少<exception>(必要时还应补<remarks>)。建议在本 PR 一并补齐,避免公共 API 的契约说明不完整。📌 建议补齐示例(以
BindAsync为例)/// <summary> /// 异步绑定操作,将结果链式转换为异步结果 /// </summary> +/// <param name="result">输入结果;仅在成功态时执行 <paramref name="binder"/>。</param> +/// <param name="binder">将成功值映射为异步 <see cref="Result{TResult}"/> 的绑定委托。</param> +/// <returns> +/// 若 <paramref name="result"/> 为成功态,则返回 <paramref name="binder"/> 的执行结果; +/// 否则透传失败结果。 +/// </returns> +/// <exception cref="ArgumentNullException"> +/// <paramref name="binder"/> 为 <see langword="null"/> 时抛出。 +/// </exception> public static async Task<Result<TResult>> BindAsync<T, TResult>(As per coding guidelines "All public, protected, and internal types and members MUST include XML documentation comments (
///) with<summary>,<param>,<returns>,<exception>, and<remarks>where applicable".Also applies to: 224-233, 257-273
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Core/Functional/ResultExtensions.cs` around lines 95 - 110, The public extension method BindAsync (signature: public static async Task<Result<TResult>> BindAsync<T, TResult>(this Result<T> result, Func<T, Task<Result<TResult>>> binder)) lacks full XML doc tags; add <param name="result"> describing the input result, <param name="binder"> describing the binder function and its contract, a <returns> describing the returned Task<Result<TResult>>, and include <exception> tags for any exceptions thrown (e.g., ArgumentNullException if result or binder can be null) and a <remarks> if there are important behavioral notes (such as enumeration, threading, or short-circuit behavior). Apply the same pattern to the other public methods noted (around lines 224-233 and 257-273): ensure each has <summary>, <param> for each parameter, <returns>, and <exception>/<remarks> where applicable to satisfy the XML documentation contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@GFramework.Core/Functional/ResultExtensions.cs`:
- Around line 95-110: The public extension method BindAsync (signature: public
static async Task<Result<TResult>> BindAsync<T, TResult>(this Result<T> result,
Func<T, Task<Result<TResult>>> binder)) lacks full XML doc tags; add <param
name="result"> describing the input result, <param name="binder"> describing the
binder function and its contract, a <returns> describing the returned
Task<Result<TResult>>, and include <exception> tags for any exceptions thrown
(e.g., ArgumentNullException if result or binder can be null) and a <remarks> if
there are important behavioral notes (such as enumeration, threading, or
short-circuit behavior). Apply the same pattern to the other public methods
noted (around lines 224-233 and 257-273): ensure each has <summary>, <param> for
each parameter, <returns>, and <exception>/<remarks> where applicable to satisfy
the XML documentation contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d2b44d17-668e-4174-951c-d8ec135beaeb
📒 Files selected for processing (3)
GFramework.Core/Functional/ResultExtensions.csGFramework.Core/Ioc/MicrosoftDiContainer.csGFramework.Core/Localization/LocalizationString.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- GFramework.Core/Ioc/MicrosoftDiContainer.cs
- GFramework.Core/Localization/LocalizationString.cs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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
XML documentation comments must explain intent, contract, and usage constraints instead of restating syntax
If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds, compatibility constraints or edge cases, and registration order or lifecycle sequencing
Avoid obvious comments such as// increment i
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
Generated logic and generator pipelines MUST explain what is generated, why it is generated, the semantic assumptions the generator relies on, and any diagnostics or fallback behavior
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling if any
Comments MUST NOT be trivial, redundant, or misleading; prefer explainingwhyandwhen, not justwhat
Prefer slightly more explanation over too little for framework code
Do not rely on implicit imports; declare every requiredusingexplicitly
Write null-safe code that respects nullable annotations instead of suppressing warnings by default
Use the namespac...
Files:
GFramework.Core/Functional/ResultExtensions.cs
**/*.{cs,csproj,xml,yaml,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation; do not use tabs
Files:
GFramework.Core/Functional/ResultExtensions.cs
🧠 Learnings (2)
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Match existing async patterns and naming conventions; use `Async` suffix for asynchronous methods
Applied to files:
GFramework.Core/Functional/ResultExtensions.cs
📚 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.Core/Functional/ResultExtensions.cs
🔇 Additional comments (3)
GFramework.Core/Functional/ResultExtensions.cs (3)
278-278: 仅文件结尾换行,无需进一步处理。
107-115: 异步上下文处理一致,变更方向正确
BindAsync与TryAsync增加.ConfigureAwait(false)后,与仓库现有异步 Result 模式保持一致,降低上下文捕获风险。Also applies to: 232-238
260-266: 空值转换异常构造更清晰
ArgumentNullException(nameof(value), errorMessage)同时保留参数名与业务错误信息,调用侧排障信息更完整。Also applies to: 270-276
- 修复 Core、Cqrs、Godot 与 Game 模块中的异常契约、空值校验和线程亲和性问题 - 更新 Settings API 为 ApplyAsync 并同步实现、调用点、测试与中文文档 - 补充 AGENTS.md 中的构建校验、自动提交与分支创建规则 - 整理 Logging、WeakCache 与 Resource 相关实现的行为与文档一致性
Summary
Test ResultsDetails
Insights
Fail Rate
build-and-test: Run #873
🎉 All tests passed!Slowest Tests
± Comparison with run #872 at 6c0225f | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 3 runs. Github Test Reporter by CTRF 💚 |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
GFramework.Godot/Architectures/AbstractArchitecture.cs (1)
105-119:⚠️ Potential issue | 🟠 Major先校验
_anchor,再执行module.Install(this)。现在 Line 109 先安装模块,Line 112-113 才校验锚点。若当前尚未绑定到场景树,这里会先产生模块侧副作用,再抛出
InvalidOperationException,留下半初始化状态,后续重试也很难保证幂等。建议先拿到已校验的本地anchor,再开始安装流程。💡 建议修改
protected async Task InstallGodotModule<TModule>(TModule module) where TModule : IGodotModule { ArgumentNullException.ThrowIfNull(module); - - module.Install(this); - - // 检查锚点是否已初始化,未初始化则抛出异常 - if (_anchor == null) - throw new InvalidOperationException("Anchor not initialized"); + var anchor = _anchor ?? throw new InvalidOperationException("Anchor not initialized"); + + module.Install(this); // 等待锚点准备就绪,并保持 Godot 同步上下文,以便后续附加逻辑安全访问节点 API。 - await _anchor.WaitUntilReadyAsync(); + await anchor.WaitUntilReadyAsync(); // 延迟调用将扩展节点添加为锚点的子节点 - _anchor.CallDeferred(Node.MethodName.AddChild, module.Node); + anchor.CallDeferred(Node.MethodName.AddChild, module.Node);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot/Architectures/AbstractArchitecture.cs` around lines 105 - 119, The InstallGodotModule<TModule> method validates the anchor too late (module.Install(this) runs before checking _anchor), potentially leaving the module partially initialized on error; fix it by capturing and validating a local anchor reference first (throw if null), await anchor.WaitUntilReadyAsync() while preserving the Godot sync context, then call module.Install(this) and finally anchor.CallDeferred(Node.MethodName.AddChild, module.Node); this ensures no module side-effects occur before anchor readiness and avoids races with the _anchor field.
🧹 Nitpick comments (2)
GFramework.Core/Logging/ConfigurableLoggerFactory.cs (1)
49-54:minLevel参数与实现语义不一致Line [49] 的参数说明写明是“调用方传入的默认最小级别”,但 Line [53] 完全忽略了
minLevel。建议二选一:要么把minLevel纳入计算,要么更新 XML 文档明确其被配置覆盖,避免 API 误导。♻️ 若希望保留“调用方默认级别”语义,可直接合并两者
- var effectiveLevel = _config.MinLevel; + var effectiveLevel = _config.MinLevel > minLevel ? _config.MinLevel : minLevel;As per coding guidelines: "Any change to public API, lifecycle semantics, module behavior, or extension points MUST update the related XML docs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Core/Logging/ConfigurableLoggerFactory.cs` around lines 49 - 54, The XML doc for GetLogger says minLevel is the caller's default but the method ignores it and uses _config.MinLevel; update GetLogger to honor minLevel by merging it with _config.MinLevel (e.g., compute effectiveLevel as the stricter/higher severity of minLevel and _config.MinLevel) so callers' defaults are respected, and keep references to the method and parameter names (GetLogger, minLevel, _config.MinLevel) in your change; alternatively, if you choose to have configuration always override caller defaults, update the XML doc comment for the minLevel parameter to state it will be overridden by configuration instead of changing behavior.GFramework.Godot/Config/GodotYamlConfigEnvironment.cs (1)
129-147: 建议用try/finally保障ListDirEnd()一定执行。
Line 146当前只在正常路径调用ListDirEnd();若后续循环体扩展引入异常,会跳过结束调用。建议现在就收敛成finally,降低维护期资源状态风险。建议修改
var entries = new List<GodotYamlConfigDirectoryEntry>(); var listDirectoryError = directory.ListDirBegin(); if (listDirectoryError != Error.Ok) { return null; } - while (true) + try { - var name = directory.GetNext(); - if (string.IsNullOrEmpty(name)) + while (true) { - break; - } + var name = directory.GetNext(); + if (string.IsNullOrEmpty(name)) + { + break; + } - entries.Add(new GodotYamlConfigDirectoryEntry(name, directory.CurrentIsDir())); + entries.Add(new GodotYamlConfigDirectoryEntry(name, directory.CurrentIsDir())); + } + } + finally + { + directory.ListDirEnd(); } - - directory.ListDirEnd(); return entries; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot/Config/GodotYamlConfigEnvironment.cs` around lines 129 - 147, The directory listing must always call directory.ListDirEnd() even if an exception is thrown; wrap the listing logic in a try/finally: after verifying directory.ListDirBegin() == Error.Ok, enter a try block that runs the while loop (using directory.GetNext(), directory.CurrentIsDir(), adding GodotYamlConfigDirectoryEntry to entries) and put directory.ListDirEnd() in the finally block so it always executes; ensure you still return entries (or null) appropriately and do not swallow the original error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/zh-CN/godot/setting.md`:
- Around line 385-386: The SetMasterVolume methods are declared "private void"
but call "await audioSettings.ApplyAsync()", causing a C# compile error; update
each SetMasterVolume signature to be async (e.g., change to "private async Task
SetMasterVolume(...)" or "private async void SetMasterVolume(...)" if it's an
event handler) so awaiting ApplyAsync is valid, and ensure you add "using
System.Threading.Tasks;" if you choose Task.
In `@GFramework.Core/Logging/ConfigurableLoggerFactory.cs`:
- Around line 21-23: ConfigurableLoggerFactory's constructor assumes
config.Appenders and config.LoggerLevels are non-null which can cause
NullReferenceException during deserialization; update the constructor to
defensively normalize those collections before use (e.g., treat config.Appenders
and config.LoggerLevels as empty sequences when null), then continue mapping via
LoggingConfigurationLoader.CreateAppender for appenders and the existing
logger-level mapping logic for logger levels so assignments to _appenders and
the logger-level field cannot throw on null input.
- Around line 35-40: ConfigurableLoggerFactory currently only disposes appenders
implementing IDisposable in the _appenders loop, which skips AsyncLogAppender
because it exposes a public Dispose() but does not implement IDisposable; update
the disposal loop in ConfigurableLoggerFactory to additionally detect appenders
of type GFramework.Core.Logging.Appenders.AsyncLogAppender (or any type exposing
a public Dispose() method) and call their Dispose() (or stop method) so the
async channel/background task and inner appender are cleaned up; keep the
existing IDisposable branch but add an else-if that casts to AsyncLogAppender
and invokes its Dispose(), and add a comment suggesting AsyncLogAppender should
implement IDisposable in the future.
In `@GFramework.Cqrs/Internal/WeakTypePairCache.cs`:
- Around line 78-90: Add an <exception> XML doc entry to
GetValueOrDefaultForTesting indicating it throws ArgumentNullException when
primaryType or secondaryType is null; reference the method name
GetValueOrDefaultForTesting and the internal call to TryGetValue to explain the
contract, update the XML comments to include <exception
cref="System.ArgumentNullException">Thrown when primaryType or secondaryType is
null.</exception> so the public/test helper API documents the null-argument
behavior.
In `@GFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.cs`:
- Around line 16-57: Add XML documentation comments (///) to the three public
test methods ApplyAsync_ShouldSyncEnglishToGodotLocaleAndFrameworkLanguage,
ApplyAsync_ShouldSyncChineseToGodotLocaleAndFrameworkLanguage, and
ApplyAsync_ShouldFallbackUnknownLanguageToEnglish: include a <summary>
describing what the test verifies and a <returns> noting that it returns a Task;
add <remarks> if you want to mention side-effects (e.g. that it sets Godot
locale and calls ILocalizationManager.SetLanguage) so the public async test
methods satisfy the repository's XML doc requirement.
In `@GFramework.Godot/Config/GodotYamlConfigEnvironment.cs`:
- Around line 109-119: The non-Godot branch in GodotYamlConfigEnvironment (the
block using Directory.Exists(path) and Directory.EnumerateFileSystemEntries) can
throw on permission/IO errors but currently propagates exceptions; change it to
mirror the Godot path branch by wrapping the EnumerateFileSystemEntries and
subsequent Select/ToArray logic in a try-catch that catches IOException,
UnauthorizedAccessException, PathTooLongException and other relevant exceptions
(or a general Exception as the Godot branch does) and returns null on any
failure, so the method returns null for inaccessible directories as documented;
keep the creation of GodotYamlConfigDirectoryEntry and use
Path.GetFileName(entryPath) and Directory.Exists(entryPath) inside the try
block.
---
Outside diff comments:
In `@GFramework.Godot/Architectures/AbstractArchitecture.cs`:
- Around line 105-119: The InstallGodotModule<TModule> method validates the
anchor too late (module.Install(this) runs before checking _anchor), potentially
leaving the module partially initialized on error; fix it by capturing and
validating a local anchor reference first (throw if null), await
anchor.WaitUntilReadyAsync() while preserving the Godot sync context, then call
module.Install(this) and finally anchor.CallDeferred(Node.MethodName.AddChild,
module.Node); this ensures no module side-effects occur before anchor readiness
and avoids races with the _anchor field.
---
Nitpick comments:
In `@GFramework.Core/Logging/ConfigurableLoggerFactory.cs`:
- Around line 49-54: The XML doc for GetLogger says minLevel is the caller's
default but the method ignores it and uses _config.MinLevel; update GetLogger to
honor minLevel by merging it with _config.MinLevel (e.g., compute effectiveLevel
as the stricter/higher severity of minLevel and _config.MinLevel) so callers'
defaults are respected, and keep references to the method and parameter names
(GetLogger, minLevel, _config.MinLevel) in your change; alternatively, if you
choose to have configuration always override caller defaults, update the XML doc
comment for the minLevel parameter to state it will be overridden by
configuration instead of changing behavior.
In `@GFramework.Godot/Config/GodotYamlConfigEnvironment.cs`:
- Around line 129-147: The directory listing must always call
directory.ListDirEnd() even if an exception is thrown; wrap the listing logic in
a try/finally: after verifying directory.ListDirBegin() == Error.Ok, enter a try
block that runs the while loop (using directory.GetNext(),
directory.CurrentIsDir(), adding GodotYamlConfigDirectoryEntry to entries) and
put directory.ListDirEnd() in the finally block so it always executes; ensure
you still return entries (or null) appropriately and do not swallow the original
error.
🪄 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: 728c8da9-d89a-40a6-bc80-b52da4eefae2
📒 Files selected for processing (30)
AGENTS.mdGFramework.Core.Abstractions/Resource/IResourceManager.csGFramework.Core.Tests/Extensions/NumericExtensionsTests.csGFramework.Core.Tests/Extensions/ResultExtensionsTests.csGFramework.Core.Tests/Functional/ResultTests.csGFramework.Core.Tests/Logging/LoggingConfigurationTests.csGFramework.Core/Coroutine/Instructions/WaitForTask.csGFramework.Core/Extensions/NumericExtensions.csGFramework.Core/Functional/Result.csGFramework.Core/Functional/ResultExtensions.csGFramework.Core/Logging/AppenderConfiguration.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Core/Logging/Filters/SamplingFilter.csGFramework.Core/Resource/ResourceManager.csGFramework.Cqrs/Internal/WeakKeyCache.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Game.Abstractions/Setting/IApplyAbleSettings.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Game.Tests/Setting/SettingsSystemTests.csGFramework.Game/Setting/SettingsModel.csGFramework.Game/Setting/SettingsSystem.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Godot/Config/GodotYamlConfigEnvironment.csGFramework.Godot/Scene/SceneBehaviorBase.csGFramework.Godot/Setting/GodotAudioSettings.csGFramework.Godot/Setting/GodotGraphicsSettings.csGFramework.Godot/Setting/GodotLocalizationSettings.csGFramework.Godot/UI/UiPageBehaviorFactory.csdocs/zh-CN/game/setting.mddocs/zh-CN/godot/setting.md
✅ Files skipped from review due to trivial changes (7)
- GFramework.Core.Tests/Extensions/NumericExtensionsTests.cs
- GFramework.Core.Tests/Extensions/ResultExtensionsTests.cs
- GFramework.Core/Logging/Filters/SamplingFilter.cs
- AGENTS.md
- GFramework.Core/Coroutine/Instructions/WaitForTask.cs
- GFramework.Core.Abstractions/Resource/IResourceManager.cs
- GFramework.Core/Logging/AppenderConfiguration.cs
🚧 Files skipped from review as they are similar to previous changes (5)
- GFramework.Godot/UI/UiPageBehaviorFactory.cs
- GFramework.Core/Functional/Result.cs
- GFramework.Godot/Setting/GodotGraphicsSettings.cs
- GFramework.Core/Functional/ResultExtensions.cs
- GFramework.Core/Extensions/NumericExtensions.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). (1)
- GitHub Check: Analyze (C#)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.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
Files:
GFramework.Godot/Scene/SceneBehaviorBase.csGFramework.Game/Setting/SettingsModel.csGFramework.Core.Tests/Logging/LoggingConfigurationTests.csGFramework.Core.Tests/Functional/ResultTests.csGFramework.Game.Tests/Setting/SettingsSystemTests.csGFramework.Godot/Setting/GodotLocalizationSettings.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Cqrs/Internal/WeakKeyCache.csGFramework.Game.Abstractions/Setting/IApplyAbleSettings.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Core/Resource/ResourceManager.csGFramework.Game/Setting/SettingsSystem.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Godot/Config/GodotYamlConfigEnvironment.csGFramework.Godot/Setting/GodotAudioSettings.cs
**/*.{cs,csx}
📄 CodeRabbit inference engine (AGENTS.md)
All public, protected, and internal types and members in C# MUST include XML documentation comments (
///) with<summary>,<param>,<returns>,<exception>, and<remarks>where applicableAdd inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds, compatibility constraints, edge cases, and registration order or lifecycle sequencing
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling
Do not rely on implicit imports. Declare every required
usingexplicitlyWrite null-safe code that respects nullable annotations instead of suppressing warnings by default
Use the namespace pattern
GFramework.{Module}.{Feature}with PascalCase segmentsFollow standard C# naming: Types, methods, properties, events, and constants use PascalCase; Interfaces use
Iprefix; Parameters and locals use camelCase; Private fields use_camelCaseUse Allman braces style
Keep
usingdirectives at the top of the file and sort them consistentlyPrefer one primary type per file unless the surrounding project already uses a different local pattern
Unless there is a clear and documented reason to keep a file large, keep a single source file under roughly 800-1000 lines. If a file grows beyond that range, contributors MUST check whether responsibilities should be split
Prefer explicit, readable code over clever shorthand in framework internals
Match existing async patterns and naming conventions with
Asyncsuffix for asynchronous methodsAvoid hidden side effects in property getters, constructors, and registration helpers
Preserve deterministic behavior in registries, lifecycle orchestration, and generated outputs
When adding analyzers or suppressions, keep them minimal and justify them in code comments if the reason is not obvious
Treat
Meziantou.Analyzerfeedback as part of the coding standardTreat SonarQube maintainability rules as part of the coding standard,...
Files:
GFramework.Godot/Scene/SceneBehaviorBase.csGFramework.Game/Setting/SettingsModel.csGFramework.Core.Tests/Logging/LoggingConfigurationTests.csGFramework.Core.Tests/Functional/ResultTests.csGFramework.Game.Tests/Setting/SettingsSystemTests.csGFramework.Godot/Setting/GodotLocalizationSettings.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Cqrs/Internal/WeakKeyCache.csGFramework.Game.Abstractions/Setting/IApplyAbleSettings.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Core/Resource/ResourceManager.csGFramework.Game/Setting/SettingsSystem.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Godot/Config/GodotYamlConfigEnvironment.csGFramework.Godot/Setting/GodotAudioSettings.cs
**/*.{cs,csx,csproj,xml,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation. Do not use tabs
Files:
GFramework.Godot/Scene/SceneBehaviorBase.csGFramework.Game/Setting/SettingsModel.csGFramework.Core.Tests/Logging/LoggingConfigurationTests.csGFramework.Core.Tests/Functional/ResultTests.csGFramework.Game.Tests/Setting/SettingsSystemTests.csGFramework.Godot/Setting/GodotLocalizationSettings.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Cqrs/Internal/WeakKeyCache.csGFramework.Game.Abstractions/Setting/IApplyAbleSettings.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Core/Resource/ResourceManager.csGFramework.Game/Setting/SettingsSystem.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Godot/Config/GodotYamlConfigEnvironment.csGFramework.Godot/Setting/GodotAudioSettings.cs
**/*.{cs,csx,csproj,xml,yml,yaml,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep line length readable. Around 120 characters is the preferred upper bound
Files:
GFramework.Godot/Scene/SceneBehaviorBase.csGFramework.Game/Setting/SettingsModel.csGFramework.Core.Tests/Logging/LoggingConfigurationTests.csGFramework.Core.Tests/Functional/ResultTests.csdocs/zh-CN/game/setting.mdGFramework.Game.Tests/Setting/SettingsSystemTests.csGFramework.Godot/Setting/GodotLocalizationSettings.csdocs/zh-CN/godot/setting.mdGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Cqrs/Internal/WeakKeyCache.csGFramework.Game.Abstractions/Setting/IApplyAbleSettings.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Core/Resource/ResourceManager.csGFramework.Game/Setting/SettingsSystem.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Godot/Config/GodotYamlConfigEnvironment.csGFramework.Godot/Setting/GodotAudioSettings.cs
**/*.Tests/**/*.{cs,csx}
📄 CodeRabbit inference engine (AGENTS.md)
Mirror the source structure in test projects whenever practical
Reuse existing architecture test infrastructure when relevant:
ArchitectureTestsBase<T>,SyncTestArchitecture,AsyncTestArchitectureKeep tests focused on observable behavior, not implementation trivia
Files:
GFramework.Core.Tests/Logging/LoggingConfigurationTests.csGFramework.Core.Tests/Functional/ResultTests.csGFramework.Game.Tests/Setting/SettingsSystemTests.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.cs
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/setting.mddocs/zh-CN/godot/setting.md
docs/zh-CN/**/*.md
📄 CodeRabbit inference engine (AGENTS.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 changeFor integration-oriented features such as the AI-First config system, documentation MUST cover project directory layout and file conventions, required project or package wiring, minimal working usage example, and migration or compatibility notes
Files:
docs/zh-CN/game/setting.mddocs/zh-CN/godot/setting.md
docs/**/*.{md,ts,js,json}
📄 CodeRabbit inference engine (AGENTS.md)
If a docs category appears in VitePress navigation or sidebar, it MUST have a real landing page or be removed from navigation in the same change
Files:
docs/zh-CN/game/setting.mddocs/zh-CN/godot/setting.md
**/*Abstractions/**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
Abstractions projects should only contain interfaces and contract definitions without any runtime implementation logic
Files:
GFramework.Game.Abstractions/Setting/IApplyAbleSettings.cs
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T11:38:58.263Z
Learning: Before choosing runtimes or CLI tools, read `@.ai/environment/tools.ai.yaml`
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T11:38:58.263Z
Learning: Every completed task MUST pass at least one build validation before it is considered done
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T11:38:58.263Z
Learning: Commit messages MUST use Conventional Commits format: `<type>(<scope>): <summary>` with summary in simplified Chinese and body using unordered list items starting with verbs like `新增`, `修复`, `优化`, `更新`, `补充`, `重构`
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T11:38:58.263Z
Learning: If a new task starts while the current branch is `main`, contributors MUST first try to update local `main` from the remote, then create and switch to a dedicated branch before making substantive changes
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T11:38:58.263Z
Learning: The branch naming rule for a new task branch is `<type>/<topic-or-scope>`, where `<type>` should match the intended Conventional Commit category
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T11:38:58.263Z
Learning: Use subagents only when the task is complex, the context is likely to grow too large, or the work can be split into independent parallel subtasks
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T11:38:58.263Z
Learning: Keep abstractions projects free of implementation details and engine-specific dependencies
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T11:38:58.263Z
Learning: Preserve existing module boundaries. Do not introduce new cross-module dependencies without clear architectural need
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T11:38:58.263Z
Learning: Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T11:38:58.263Z
Learning: Every user-facing package or module directory that contains a `*.csproj` intended for direct consumption MUST have a sibling `README.md`
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Match existing async patterns and naming conventions; use `Async` suffix for asynchronous methods
Applied to files:
GFramework.Godot/Scene/SceneBehaviorBase.csGFramework.Godot/Setting/GodotLocalizationSettings.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Game.Abstractions/Setting/IApplyAbleSettings.csGFramework.Core/Resource/ResourceManager.csGFramework.Godot/Setting/GodotAudioSettings.cs
📚 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.Godot/Scene/SceneBehaviorBase.csGFramework.Game/Setting/SettingsModel.csGFramework.Core.Tests/Logging/LoggingConfigurationTests.csGFramework.Core.Tests/Functional/ResultTests.csGFramework.Game.Tests/Setting/SettingsSystemTests.csGFramework.Godot/Setting/GodotLocalizationSettings.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Cqrs/Internal/WeakKeyCache.csGFramework.Game.Abstractions/Setting/IApplyAbleSettings.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Core/Resource/ResourceManager.csGFramework.Game/Setting/SettingsSystem.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Godot/Config/GodotYamlConfigEnvironment.csGFramework.Godot/Setting/GodotAudioSettings.cs
📚 Learning: 2026-04-16T13:25:59.124Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 235
File: GFramework.SourceGenerators.Tests/Rule/ContextAwareGeneratorSnapshotTests.cs:1-3
Timestamp: 2026-04-16T13:25:59.124Z
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.Core.Tests/Logging/LoggingConfigurationTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*SourceGenerators*.Tests/**/*.cs : Preserve snapshot-based verification patterns already used in the repository for generator tests
Applied to files:
GFramework.Core.Tests/Logging/LoggingConfigurationTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.Tests.cs : Regression fixes should include a test that fails before the fix and passes after it
Applied to files:
GFramework.Core.Tests/Logging/LoggingConfigurationTests.csGFramework.Core.Tests/Functional/ResultTests.csGFramework.Game.Tests/Setting/SettingsSystemTests.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
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
Applied to files:
GFramework.Core.Tests/Functional/ResultTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
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
Applied to files:
docs/zh-CN/game/setting.md
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Any change to public API, lifecycle semantics, module behavior, or extension points MUST update the related XML docs
Applied to files:
docs/zh-CN/game/setting.mdGFramework.Cqrs/Internal/WeakKeyCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.Tests.cs : Reuse existing architecture test infrastructure when relevant: `ArchitectureTestsBase<T>`, `SyncTestArchitecture`, `AsyncTestArchitecture`
Applied to files:
GFramework.Game.Tests/Setting/SettingsSystemTests.csGFramework.Godot/Architectures/AbstractArchitecture.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Public API changes must be covered by unit or integration tests
Applied to files:
GFramework.Game.Tests/Setting/SettingsSystemTests.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*SourceGenerators*.Tests/**/*.cs : When generator behavior changes intentionally, update snapshots together with the implementation
Applied to files:
GFramework.Game.Tests/Setting/SettingsSystemTests.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : When adding caching, pooling, or shared mutable state, document thread-safety assumptions and failure modes
Applied to files:
GFramework.Cqrs/Internal/WeakKeyCache.csGFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds, compatibility constraints or edge cases, and registration order or lifecycle sequencing
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
🔇 Additional comments (15)
GFramework.Core.Tests/Functional/ResultTests.cs (1)
433-447: 新增的default(Result)回归测试覆盖很到位。该用例把默认值分支下的相等性、哈希和字符串表示都锁定住了,能有效防止后续重构引入空引用回归。
Based on learnings: 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.
GFramework.Cqrs/Internal/WeakKeyCache.cs (1)
28-29: 异常契约与实现已对齐,改动合理。两个
GetOrAdd重载现在都明确区分了参数空值与工厂返回空值的异常类型,行为一致且可预测。Also applies to: 45-46, 60-61, 77-78
GFramework.Cqrs/Internal/WeakTypePairCache.cs (1)
8-13: 两层弱键缓存实现清晰,线程语义说明到位。类级注释已覆盖并发访问与失效模式,
GetOrAdd的两段键设计与调用路径匹配。Also applies to: 31-42
GFramework.Core/Resource/ResourceManager.cs (2)
85-93: 文档契约补充完整,方向正确。这两段 XML 注释把参数约束、异常和 continuation 语义写清楚了,和本次行为调整是对齐的。
Also applies to: 236-244
113-113: 本评论所提及的关注点已在代码中完全解决。
ConfigureAwait(false)的引入确实改变了执行上下文,但该实现已通过以下方式妥善处理了这一问题:
文档已明确说明(第 92 行):XML 注释明确记载
ConfigureAwait(false)的行为和对调用线程的影响。线程安全已正确保护(第 114-126 行):
LoadAsync的后续延续(包括loader.Unload()和缓存写入)完全在lock (_loadLock)保护下执行,消除了竞态条件。并发测试已覆盖:
Concurrent_LoadAsync_Should_Be_Thread_Safe()测试已验证并发场景下的正确性。不存在线程亲和性依赖:仓库中未发现
IResourceLoader<T>的实现有线程亲和性需求(主线程、同步上下文、Dispatcher 等)。无需进一步行动。
> Likely an incorrect or invalid review comment.GFramework.Core.Tests/Logging/LoggingConfigurationTests.cs (1)
146-173: 最长前缀匹配的回归测试覆盖到位这个用例准确命中了“重叠命名空间前缀”场景,并通过
IsTraceEnabled/IsDebugEnabled做行为断言,能稳定防止该问题回归。Based on learnings: Applies to **/*.Tests.cs : Regression fixes should include a test that fails before the fix and passes after it.
GFramework.Game.Abstractions/Setting/IApplyAbleSettings.cs (1)
9-15: 接口异步契约迁移清晰且一致。
ApplyAsync()与注释语义匹配,且与本次设置系统的异步调用链保持一致。GFramework.Godot/Setting/GodotAudioSettings.cs (1)
19-20: 音频设置应用器签名更新正确。
ApplyAsync()与Task.CompletedTask的实现语义一致,迁移稳定。GFramework.Godot/Setting/GodotLocalizationSettings.cs (1)
80-81: 本地化应用器的异步接口迁移完成度良好。方法签名与接口统一,且不改变既有语言同步流程。
GFramework.Game/Setting/SettingsModel.cs (1)
201-201: 模型层对新异步应用契约的适配正确。调用点已切换为
ApplyAsync(),并保留了原有容错行为。GFramework.Game/Setting/SettingsSystem.cs (1)
90-90: 系统层异步调用切换与事件编排保持一致。
TryApplyAsync的行为语义未漂移,迁移风险低。GFramework.Game.Tests/Setting/SettingsSystemTests.cs (1)
317-317: 测试桩方法签名同步到ApplyAsync,覆盖面保持完整。这能有效验证系统在新异步契约下的成功/失败事件行为。
docs/zh-CN/game/setting.md (1)
75-76: 文档已正确同步ApplyAsync()语义与示例。这次更新能减少用户按旧 API 编写代码的误用风险。
Also applies to: 142-147
docs/zh-CN/godot/setting.md (1)
47-47: Godot 设置文档主流程已完成ApplyAsync()迁移。术语、流程图与多数调用示例保持一致,迁移方向正确。
Also applies to: 109-109, 134-134, 158-158, 173-173, 190-190, 208-208, 216-216, 240-240, 269-269, 281-281, 295-295, 525-525
GFramework.Godot/Config/GodotYamlConfigEnvironment.cs (1)
40-52: 构造函数的委托参数防御性校验实现正确。
Line 47-51对全部注入委托做了空值检查,失败时立即抛ArgumentNullException,这里实现清晰且稳健。
- 修复 AbstractArchitecture 在锚点未初始化时先执行模块安装的顺序问题,并收紧 GodotYamlConfigEnvironment 的目录枚举异常处理 - 修复 ConfigurableLoggerFactory 对 null 集合、调用方 minLevel 与 AsyncLogAppender 释放路径的处理 - 补充 WeakTypePairCache 与 GodotLocalizationSettingsTests 的 XML 文档,并新增日志工厂回归测试
Summary
Test ResultsDetails
Insights
Fail Rate
build-and-test: Run #874
🎉 All tests passed!Slowest Tests
± Comparison with run #873 at 568eef8 | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 4 runs. Github Test Reporter by CTRF 💚 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
GFramework.Godot/Architectures/AbstractArchitecture.cs (1)
112-124:⚠️ Potential issue | 🟠 Major把模块加入
_extensions的时机提前到可能失败的附加流程之前这里只有
module.OnAttach(this)成功后才会记录到_extensions。如果WaitUntilReadyAsync()、延迟挂载后的访问,或OnAttach()抛异常,模块已经产生安装副作用,但后续DestroyAsync()不会再走到它的OnDetach(),会留下半安装状态。🛠️ 最小修正示例
module.Install(this); + _extensions.Add(module); // 等待锚点准备就绪,并保持 Godot 同步上下文,以便后续附加逻辑安全访问节点 API。 await anchor.WaitUntilReadyAsync(); // 延迟调用将扩展节点添加为锚点的子节点 anchor.CallDeferred(Node.MethodName.AddChild, module.Node); // 调用扩展的附加回调方法 module.OnAttach(this); - - // 将扩展添加到扩展集合中 - _extensions.Add(module);如需更稳妥,可以再用
try/catch做失败回滚。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot/Architectures/AbstractArchitecture.cs` around lines 112 - 124, Add the extension to the internal collection before any operations that can fail so partial installs are tracked: call _extensions.Add(module) immediately after module.Install(this) (before awaiting anchor.WaitUntilReadyAsync(), before anchor.CallDeferred(..., module.Node) and before module.OnAttach(this)), and wrap the subsequent attach steps (WaitUntilReadyAsync, CallDeferred, OnAttach) in a try/catch that calls module.OnDetach(this) (or invokes DestroyAsync behavior) and removes the module from _extensions on failure to ensure proper rollback; reference module.Install, anchor.WaitUntilReadyAsync, anchor.CallDeferred(Node.MethodName.AddChild, module.Node), module.OnAttach, _extensions.Add, module.OnDetach/DestroyAsync.
🧹 Nitpick comments (2)
GFramework.Cqrs/Internal/WeakTypePairCache.cs (1)
37-41: 建议为两级GetOrAdd热路径补充行内注释。这段是非平凡且偏性能敏感逻辑(双层缓存 +
staticlambda + state 传递),建议补 1-2 条注释说明设计意图,避免后续误改。✍️ 建议补丁
- var secondaryEntries = _entries.GetOrAdd(primaryType, static _ => new WeakKeyCache<Type, TValue>()); + // 第一层按 primaryType 定位/创建二级缓存,避免重复构造容器。 + var secondaryEntries = _entries.GetOrAdd(primaryType, static _ => new WeakKeyCache<Type, TValue>()); return secondaryEntries.GetOrAdd( secondaryType, (PrimaryType: primaryType, Factory: valueFactory), + // 使用 static lambda + state 传参,避免闭包捕获与额外分配。 static (cachedSecondaryType, state) => state.Factory(state.PrimaryType, cachedSecondaryType));As per coding guidelines, “Add inline comments for non-trivial logic, concurrency/threading behavior, performance-sensitive paths...”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Cqrs/Internal/WeakTypePairCache.cs` around lines 37 - 41, Add 1-2 concise inline comments to the hot path in WeakTypePairCache: explain the two-level cache pattern using _entries.GetOrAdd(primaryType, ...) which returns a WeakKeyCache<Type, TValue> and the nested secondaryEntries.GetOrAdd(secondaryType, (PrimaryType: primaryType, Factory: valueFactory), ...) call; clarify why a static lambda is used with a captured state tuple (to avoid closure allocations) and note concurrency expectations (thread-safe semantics of GetOrAdd and that values are created via Factory(primaryType, cachedSecondaryType)).GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs (1)
105-116: 建议避免通过反射绑定私有字段名_appenders的断言方式。这会让测试对内部重构(改字段名/结构)过于敏感。更推荐断言可观察行为(例如释放后写日志触发已释放语义),降低脆弱性。
♻️ 可选重构示例(行为断言替代反射)
- var asyncAppender = GetSingleAsyncAppender(factory); - logger.Info("dispose-path"); ((IDisposable)factory).Dispose(); - - Assert.That(asyncAppender.IsCompleted, Is.True); - } - - private static AsyncLogAppender GetSingleAsyncAppender(ILoggerFactory factory) - { - var appendersField = factory.GetType().GetField("_appenders", BindingFlags.Instance | BindingFlags.NonPublic); - Assert.That(appendersField, Is.Not.Null); - - var appenders = appendersField!.GetValue(factory) as ILogAppender[]; - Assert.That(appenders, Is.Not.Null); - Assert.That(appenders, Has.Length.EqualTo(1)); - Assert.That(appenders![0], Is.TypeOf<AsyncLogAppender>()); - - return (AsyncLogAppender)appenders[0]; + Assert.Throws<ObjectDisposedException>(() => logger.Info("after-dispose")); } }As per coding guidelines: "Keep tests focused on observable behavior, not implementation trivia".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs` around lines 105 - 116, The test currently reaches into factory internals by reflecting the private field "_appenders" in GetSingleAsyncAppender — stop asserting implementation details and instead assert observable behavior: remove the reflection-based GetSingleAsyncAppender and either (a) use public APIs like ILoggerFactory.CreateLogger and produce log entries, then assert logs are processed/ flushed exactly once (e.g., by writing N messages and verifying the appender output/queue size or flush/Dispose semantics) to prove a single AsyncLogAppender is active, or (b) if direct type access is required for testing, add a test-only internal property or friend accessor on the ConfigurableLoggerFactory to expose the appenders array for tests and use that instead of reflection; reference GetSingleAsyncAppender, ILoggerFactory/ConfigurableLoggerFactory, and AsyncLogAppender when making the change.
🤖 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.Core/Logging/ConfigurableLoggerFactory.cs`:
- Around line 57-62: The XML doc for the minLevel parameter is misleading:
update the documentation in ConfigurableLoggerFactory (the XML comments around
the minLevel param and the remarks) to state that although callers provide a
requested minimum level via the minLevel parameter, namespace-specific overrides
in LoggerLevels take precedence and the effective logger level may be lower than
the caller’s minLevel; reference the LoggerLevels override behavior and the
factory method that returns the logger instance so callers know the contract and
precedence rules.
- Around line 63-77: GetLogger currently calls name.StartsWith(...) which will
throw a NullReferenceException if the caller passes null; add an explicit null
check at the start of GetLogger (validate the parameter name) and throw
ArgumentNullException(nameof(name)) for invalid input before any use (this
affects the GetLogger method and prevents NRE when iterating
_config.LoggerLevels and calling name.StartsWith or string.Equals).
In `@GFramework.Godot/Architectures/AbstractArchitecture.cs`:
- Line 88: 补充针对 Godot 架构生命周期的回归测试:为 InstallGodotModule<TModule> 添加一个测试用例(使用
ArchitectureTestsBase<T> / SyncTestArchitecture 或 AsyncTestArchitecture)验证当
_anchor 未初始化时 InstallGodotModule 不会调用 module.Install()(即无副作用);另外为
_anchor.Bind(ObserveDestroyAsync) 的行为添加测试,模拟退出树/销毁流程并断言 ObserveDestroyAsync
中抛出的异常被捕获并记录(而不是被吞掉),可通过替换/注入可触发异常的模块或钩子并断言日志记录器收到预期错误信息来实现。
---
Outside diff comments:
In `@GFramework.Godot/Architectures/AbstractArchitecture.cs`:
- Around line 112-124: Add the extension to the internal collection before any
operations that can fail so partial installs are tracked: call
_extensions.Add(module) immediately after module.Install(this) (before awaiting
anchor.WaitUntilReadyAsync(), before anchor.CallDeferred(..., module.Node) and
before module.OnAttach(this)), and wrap the subsequent attach steps
(WaitUntilReadyAsync, CallDeferred, OnAttach) in a try/catch that calls
module.OnDetach(this) (or invokes DestroyAsync behavior) and removes the module
from _extensions on failure to ensure proper rollback; reference module.Install,
anchor.WaitUntilReadyAsync, anchor.CallDeferred(Node.MethodName.AddChild,
module.Node), module.OnAttach, _extensions.Add, module.OnDetach/DestroyAsync.
---
Nitpick comments:
In `@GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs`:
- Around line 105-116: The test currently reaches into factory internals by
reflecting the private field "_appenders" in GetSingleAsyncAppender — stop
asserting implementation details and instead assert observable behavior: remove
the reflection-based GetSingleAsyncAppender and either (a) use public APIs like
ILoggerFactory.CreateLogger and produce log entries, then assert logs are
processed/ flushed exactly once (e.g., by writing N messages and verifying the
appender output/queue size or flush/Dispose semantics) to prove a single
AsyncLogAppender is active, or (b) if direct type access is required for
testing, add a test-only internal property or friend accessor on the
ConfigurableLoggerFactory to expose the appenders array for tests and use that
instead of reflection; reference GetSingleAsyncAppender,
ILoggerFactory/ConfigurableLoggerFactory, and AsyncLogAppender when making the
change.
In `@GFramework.Cqrs/Internal/WeakTypePairCache.cs`:
- Around line 37-41: Add 1-2 concise inline comments to the hot path in
WeakTypePairCache: explain the two-level cache pattern using
_entries.GetOrAdd(primaryType, ...) which returns a WeakKeyCache<Type, TValue>
and the nested secondaryEntries.GetOrAdd(secondaryType, (PrimaryType:
primaryType, Factory: valueFactory), ...) call; clarify why a static lambda is
used with a captured state tuple (to avoid closure allocations) and note
concurrency expectations (thread-safe semantics of GetOrAdd and that values are
created via Factory(primaryType, cachedSecondaryType)).
🪄 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: c213ef10-5624-43a0-9f1a-82079f28ed2a
📒 Files selected for processing (6)
GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Godot/Config/GodotYamlConfigEnvironment.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- GFramework.Godot/Config/GodotYamlConfigEnvironment.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). (2)
- GitHub Check: Code Quality & Security
- GitHub Check: Analyze (C#)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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 logicAll public, protected, and internal types and members in C# code MUST include XML documentation comments (
///) with<summary>,<param>,<returns>,<exception>, and<remarks>where applicableXML documentation comments must explain intent, contract, and usage constraints instead of restating syntax
Add inline comments for non-trivial logic, concurrency/threading behavior, performance-sensitive paths, workarounds/compatibility constraints/edge cases, and registration order/lifecycle sequencing/generated code assumptions
Core framework components such as Architecture, Module, System, Context, Registry, Service Module, and Lifecycle types MUST include high-level explanations of responsibilities, lifecycle, interactions, why the abstraction exists, and when to use it instead of alternatives
Generated logic and generator pipelines MUST explain what is generated, why it is generated, semantic assumptions the generator relies on, and any diagnostics or fallback behavior
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling if any
Do not rely on implicit imports. Declare every required
usingexplicitlyWrite null-safe code that respects nullable annotations instead of suppressing warnings by default
Use the namespace pattern
GFramework.{Module}.{Feature}with PascalCase segmentsFollow standard C# naming: Types, methods, properties, events, and constants use PascalCase; Interfaces use
Iprefix; Parameters and locals use camelCase; Private fields use_camelCaseUse Allman braces in C# code
Keep
usingdirectives at the top of the file and sort ...
Files:
GFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
**/*.Tests/**/*.cs
📄 CodeRabbit inference engine (AGENTS.md)
Mirror the source structure in test projects whenever practical
Reuse existing architecture test infrastructure such as
ArchitectureTestsBase<T>,SyncTestArchitecture, andAsyncTestArchitectureKeep tests focused on observable behavior, not implementation trivia
Files:
GFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:27:33.303Z
Learning: Before choosing runtimes or CLI tools, read `@.ai/environment/tools.ai.yaml` and prefer project-relevant tools listed there instead of assuming every installed system tool is fair game
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:27:33.303Z
Learning: Every completed task MUST pass at least one build validation before it is considered done
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:27:33.303Z
Learning: Commit messages MUST use Conventional Commits format: `<type>(<scope>): <summary>`, with simplified Chinese summary and unordered list items in body starting with verbs such as `新增`、`修复`、`优化`、`更新`、`补充`、`重构`
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:27:33.303Z
Learning: The branch naming rule for a new task branch is `<type>/<topic-or-scope>`, where `<type>` should match the intended Conventional Commit category
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:27:33.303Z
Learning: Use subagents only when the task is complex, context is likely to grow too large, or the work can be split into independent parallel subtasks. The main agent MUST identify the critical path first and MUST specify concrete objective, expected output format, file/subsystem ownership, and constraints
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:27:33.303Z
Learning: Use 4 spaces for indentation. Do not use tabs
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:27:33.303Z
Learning: Separate logical blocks with blank lines when it improves readability
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:27:33.303Z
Learning: Keep line length readable. Around 120 characters is the preferred upper bound
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:27:33.303Z
Learning: Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:27:33.303Z
Learning: Public API changes must be covered by unit or integration tests
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:27:33.303Z
Learning: Every user-facing package or module directory that contains a `*.csproj` intended for direct consumption MUST have a sibling `README.md`
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : When adding caching, pooling, or shared mutable state, document thread-safety assumptions and failure modes
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds, compatibility constraints or edge cases, and registration order or lifecycle sequencing
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 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.Cqrs/Internal/WeakTypePairCache.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Match existing async patterns and naming conventions; use `Async` suffix for asynchronous methods
Applied to files:
GFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Public API changes must be covered by unit or integration tests
Applied to files:
GFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.Tests.cs : Regression fixes should include a test that fails before the fix and passes after it
Applied to files:
GFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.Tests.cs : Reuse existing architecture test infrastructure when relevant: `ArchitectureTestsBase<T>`, `SyncTestArchitecture`, `AsyncTestArchitecture`
Applied to files:
GFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Godot/Architectures/AbstractArchitecture.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*SourceGenerators*.Tests/**/*.cs : When generator behavior changes intentionally, update snapshots together with the implementation
Applied to files:
GFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*SourceGenerators*.Tests/**/*.cs : Preserve snapshot-based verification patterns already used in the repository for generator tests
Applied to files:
GFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Before considering work complete, confirm: required comments and XML docs are present, code follows repository style and naming rules, relevant tests were added or updated, sensitive or unsafe behavior was not introduced, user-facing documentation is updated when needed, and feature adoption docs under `docs/zh-CN/` were added or updated when functionality was added, removed, or refactored
Applied to files:
GFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
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
Applied to files:
GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-16T13:25:59.124Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 235
File: GFramework.SourceGenerators.Tests/Rule/ContextAwareGeneratorSnapshotTests.cs:1-3
Timestamp: 2026-04-16T13:25:59.124Z
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.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*SourceGenerators*.Tests/**/*.cs : Source generator changes MUST be covered by generator tests
Applied to files:
GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
🔇 Additional comments (7)
GFramework.Game.Tests/Setting/GodotLocalizationSettingsTests.cs (3)
86-86: 文件末尾换行变更无需额外处理该变更不影响行为或可维护性。
15-20: 异步命名与 XML 文档补齐到位3 个公开异步测试方法的命名与
ApplyAsync迁移一致,且已补齐<summary>/<returns>,这次改动解决了之前的文档规范问题。
As per coding guidelinesAll public, protected, and internal types and members in C# code MUST include XML documentation comments (///) with <summary>, <param>, <returns>, <exception>, and <remarks> where applicable.Also applies to: 34-40, 53-59
28-31: 行为断言保持聚焦,回归覆盖有效三组用例都在调用
ApplyAsync()后验证了外部可观察结果(Godot locale +ILocalizationManager.SetLanguage),回归测试目标清晰。
Based on learningsApplies to **/*.Tests.cs : Regression fixes should include a test that fails before the fix and passes after it.Also applies to: 47-50, 66-69
GFramework.Cqrs/Internal/WeakTypePairCache.cs (2)
3-13: 类级缓存契约说明完整,做得很好。弱键回收语义、并发访问能力和
Clear后重建路径都已明确,维护成本明显降低。Based on learnings, “Applies to **/*.cs : When adding caching, pooling, or shared mutable state, document thread-safety assumptions and failure modes”.
84-93: 测试辅助入口的文档契约清晰。参数异常、返回语义和“仅用于测试观察”的边界说明都已到位。
As per coding guidelines, “All public, protected, and internal types and members in C# code MUST include XML documentation comments (
///) with<summary>,<param>,<returns>,<exception>, and<remarks>where applicable”.GFramework.Core/Logging/ConfigurableLoggerFactory.cs (1)
23-50: 归一化与释放路径实现完整,方向正确。
null集合归一化、Dispose幂等控制,以及AsyncLogAppender的兼容释放都已覆盖到位。GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs (1)
18-103: 测试覆盖本次修复的关键契约分支,整体很好。已覆盖:
null配置归一化、调用方minLevel合并规则、AsyncLogAppender释放路径回归。
- 修复 Godot 模块在附加流程失败时的登记时机,确保后续销毁仍可感知半安装模块 - 更新 ConfigurableLoggerFactory 的 name 空值校验与 minLevel XML 契约,并用可观察行为替换脆弱的反射测试 - 补充 WeakTypePairCache 热路径注释,并新增 Godot 模块安装顺序回归测试
Summary
Test ResultsDetails
Insights
Fail Rate
build-and-test: Run #875
🎉 All tests passed!Slowest Tests
± Comparison with run #874 at 77c4474 | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 5 runs. Github Test Reporter by CTRF 💚 |
There was a problem hiding this comment.
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 `@GFramework.Core/Logging/ConfigurableLoggerFactory.cs`:
- Around line 15-19: The XML docs for the public constructor
ConfigurableLoggerFactory(LoggingConfiguration config) and the public
GetLogger(...) method are missing <exception> tags; update their XML comments to
declare the exceptions they throw (e.g., <exception
cref="System.ArgumentNullException">when config or name is
null/empty</exception>) and any other explicit exception paths the methods
surface (e.g., ArgumentException/InvalidOperationException if applicable),
including concise conditions under which each is thrown so the contract is
complete for callers.
- Around line 24-26: The current constructor in ConfigurableLoggerFactory passes
_config.Appenders elements straight into
LoggingConfigurationLoader.CreateAppender which will throw obscure errors if the
configuration contains null entries; validate _config.Appenders for null items
before mapping (e.g., check for _config.Appenders.Any(a => a == null)) and throw
a clear ArgumentException or ConfigurationException indicating "appenders
contains null entry" (include the index or context if available), then proceed
to call LoggingConfigurationLoader.CreateAppender for only non-null entries and
assign to _appenders; reference _config.Appenders, _appenders,
ConfigurableLoggerFactory constructor, and
LoggingConfigurationLoader.CreateAppender when locating the code to change.
In `@GFramework.Godot/Architectures/AbstractArchitecture.cs`:
- Around line 164-173: In ObserveDestroyCoreAsync change the await of
DestroyAsync() to use ConfigureAwait(false) to match the pattern used elsewhere
(e.g., the earlier async call) and avoid capturing the synchronization context;
specifically update the await in method ObserveDestroyCoreAsync so it awaits
DestroyAsync().ConfigureAwait(false) while keeping the existing try/catch and
GD.PushError logging intact.
🪄 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: 9d241e2b-44ca-4fcc-a7b5-0d62e9022f3c
📒 Files selected for processing (5)
GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.csGFramework.Godot/Architectures/AbstractArchitecture.cs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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 logicWhen working in WSL against this repository's Windows-backed worktree, prefer Windows Git from WSL (e.g.,
git.exe) instead of the Linuxgitbinary, and treat Windows Git as the repository-default Git path if conflicts ariseAll public, protected, and internal types and members MUST include XML documentation comments (
///) with<summary>,<param>,<returns>,<exception>, and<remarks>where applicable, explaining intent, contract, and usage constraints instead of restating syntaxAdd inline comments for non-trivial logic, concurrency/threading behavior, performance-sensitive paths, workarounds, compatibility constraints, edge cases, registration order, lifecycle sequencing, or generated code assumptions; avoid obvious comments such as
// increment iCore 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
Generated logic and source generator pipelines MUST explain what is generated, why it is generated, the semantic assumptions the generator relies on, and any diagnostics or fallback behavior
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling, if any
Comments MUST NOT be trivial, redundant, or misleading; prefer explaining
whyandwhen, not justwhat; code should remain understandable without requiring external context; prefer slightly more explanation over too little for framework codeMissing requir...
Files:
GFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
**/*.{cs,csproj}
📄 CodeRabbit inference engine (AGENTS.md)
Before choosing runtimes or CLI tools, read
@.ai/environment/tools.ai.yamland prefer project-relevant tools listed there instead of assuming every installed system tool is fair gameIf the task changes multiple projects or shared abstractions, prefer a solution-level or affected-project
dotnet build ... -c Release; otherwise use the smallest build command that still proves the result compiles
Files:
GFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:48:23.280Z
Learning: Every completed task MUST pass at least one build validation before it is considered done
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:48:23.280Z
Learning: If the required build passes and there are task-related staged or unstaged changes, contributors MUST create a Git commit automatically instead of leaving the task uncommitted, unless the user explicitly says not to commit
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:48:23.280Z
Learning: Commit messages MUST use Conventional Commits format: `<type>(<scope>): <summary>` with summary in simplified Chinese, and body using unordered list items starting with action verbs such as `新增`, `修复`, `优化`, `更新`, `补充`, `重构`
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:48:23.280Z
Learning: Keep technical terms in English when they are established project terms, such as `API`, `Model`, `System` in commit messages and documentation
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:48:23.280Z
Learning: If a new task starts while the current branch is `main`, contributors MUST first try to update local `main` from the remote, then create and switch to a dedicated branch before making substantive changes
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:48:23.280Z
Learning: The branch naming rule for a new task branch is `<type>/<topic-or-scope>`, where `<type>` should match the intended Conventional Commit category as closely as practical
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:48:23.280Z
Learning: Use subagents only when the task is complex, the context is likely to grow too large, or the work can be split into independent parallel subtasks, and always specify concrete objective, expected output format, file/module ownership, and constraints
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:48:23.280Z
Learning: Subagents are not allowed to revert or overwrite unrelated changes from the user or other agents; they must adapt to concurrent work instead of assuming exclusive ownership
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:48:23.280Z
Learning: If existing documentation conflicts with code or tests, update the documentation to match the implementation instead of preserving outdated wording
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:48:23.280Z
Learning: Completing code changes without updating the active tracking document is considered incomplete work
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T12:48:23.280Z
Learning: Before considering work complete, confirm: required comments and XML docs are present, code follows repository style and naming rules, relevant tests were added or updated, sensitive or unsafe behavior was not introduced, user-facing documentation is updated when needed, and feature adoption docs under `docs/zh-CN/` were added or updated when functionality was added, removed, or refactored
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.Tests.cs : Reuse existing architecture test infrastructure when relevant: `ArchitectureTestsBase<T>`, `SyncTestArchitecture`, `AsyncTestArchitecture`
Applied to files:
GFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Public API changes must be covered by unit or integration tests
Applied to files:
GFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.Tests.cs : Regression fixes should include a test that fails before the fix and passes after it
Applied to files:
GFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 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.Godot/Architectures/AbstractArchitecture.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : When adding caching, pooling, or shared mutable state, document thread-safety assumptions and failure modes
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds, compatibility constraints or edge cases, and registration order or lifecycle sequencing
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-16T13:25:59.124Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 235
File: GFramework.SourceGenerators.Tests/Rule/ContextAwareGeneratorSnapshotTests.cs:1-3
Timestamp: 2026-04-16T13:25:59.124Z
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.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
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
Applied to files:
GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*SourceGenerators*.Tests/**/*.cs : Source generator changes MUST be covered by generator tests
Applied to files:
GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*SourceGenerators*.Tests/**/*.cs : Preserve snapshot-based verification patterns already used in the repository for generator tests
Applied to files:
GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
🔇 Additional comments (10)
GFramework.Cqrs/Internal/WeakTypePairCache.cs (4)
3-13: 类级缓存语义说明到位这里已经清楚说明了弱键回收、并发访问与
Clear/GC 后的未命中重建行为,契约完整且可维护。
Based on learnings, "Applies to **/*.cs : When adding caching, pooling, or shared mutable state, document thread-safety assumptions and failure modes".
26-31:GetOrAdd的契约与热路径实现都很扎实参数/异常文档与实现一致,
static lambda + state的无捕获写法也有明确注释,性能意图清晰。
As per coding guidelines, “Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling, if any”.Also applies to: 37-43
46-67: 只读查询路径语义清晰
TryGetValue明确保证“不触发创建”,并在未命中时稳定返回false + null,调用方易于正确处理。
69-75: 测试辅助入口的文档与行为一致
Clear的用途边界和GetValueOrDefaultForTesting的异常/返回契约都写清楚了,当前实现可读性与可测性都不错。
As per coding guidelines, “All public, protected, and internal types and members MUST include XML documentation comments (///) with<summary>,<param>,<returns>,<exception>, and<remarks>where applicable”.Also applies to: 80-95
GFramework.Godot/Architectures/AbstractArchitecture.cs (2)
88-88: 异常观察模式设计合理将
_anchor.Bind改为绑定ObserveDestroyAsync后,通过ObserveDestroyCoreAsync内部的 try-catch 捕获销毁失败并写入GD.PushError,有效解决了先前 fire-and-forget 静默吞异常的问题。Also applies to: 148-158
107-121: 模块安装流程的防御性改进良好
ArgumentNullException.ThrowIfNull前置校验参数;- 提前捕获
_anchor并在null时抛出InvalidOperationException,确保module.Install()不会在锚点未就绪时执行;- 将
_extensions.Add(module)移至WaitUntilReadyAsync之前,保证后续步骤失败时模块仍可参与销毁阶段清理。整体逻辑清晰,符合"失败尽早暴露、资源仍可清理"的设计意图。
GFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.cs (2)
16-31: 测试覆盖了锚点未初始化时的早期失败路径测试正确验证了:
- 当
_anchor未初始化时抛出InvalidOperationException;- 异常消息为
"Anchor not initialized";module.Install()未被调用(InstallCalled为false)。此测试符合"回归修复应包含修复前失败、修复后通过的测试"原则。Based on learnings: "Regression fixes should include a test that fails before the fix and passes after it"。
45-63:RecordingGodotModule作为测试桩足以验证当前场景
Node => null!在此测试中可接受,因为测试验证的是锚点缺失时的早期失败路径,Node属性不会被访问。若后续需要覆盖更多场景(如成功安装流程),可能需要提供有效的Node实例。GFramework.Core/Logging/ConfigurableLoggerFactory.cs (1)
67-85: 命名空间级别解析逻辑实现得当。这里的“精确匹配优先 + 最长前缀回退”避免了前缀重叠时的不确定性,行为清晰。
GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs (1)
16-144: 测试集对本次行为修复覆盖充分。用例已覆盖配置归一化、级别决议优先级、空参数防御与异步 appender 释放回归,和当前工厂契约保持一致。
- 修复 ConfigurableLoggerFactory 对 null Appender 配置项的显式校验与 XML 契约 - 补充日志工厂针对 appenders 空项输入的回归测试 - 更新 Godot setting 文档中的 async 示例签名以匹配 ApplyAsync 用法 - 修正 AbstractArchitecture 中 ObserveDestroyCoreAsync 方法里调用await DestroyAsync() 未配置ConfigureAwait(false)的问题
Summary
Test ResultsDetails
Insights
Fail Rate
build-and-test: Run #876
🎉 All tests passed!Slowest Tests
± Comparison with run #875 at 5ccfeae | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 6 runs. Github Test Reporter by CTRF 💚 |
- 为 ConfigurableLoggerFactory.GetLogger 方法添加 ArgumentNullException 异常说明 - 补充了当 name 参数为 null 时的异常抛出情况 - 完善了方法的 XML 注释文档
Summary
Test ResultsDetails
Insights
Fail Rate
build-and-test: Run #877
🎉 All tests passed!Slowest Tests
± Comparison with run #876 at 64f7329 | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 7 runs. Github Test Reporter by CTRF 💚 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
GFramework.Godot/Architectures/AbstractArchitecture.cs (1)
126-131:⚠️ Potential issue | 🟡 Minor请为
DestroyAsync补齐 XML 返回值与生命周期约束说明Line [131] 是公开异步生命周期成员,但当前注释缺少
<returns>,且对“幂等销毁/由退出树触发的异步观察调用”约束描述不够完整,建议补齐以满足仓库文档规范。♻️ 建议修复
/// <summary> /// 销毁架构及其相关资源。 /// 调用所有已安装扩展的OnDetach方法,并清空扩展列表。 /// 若已被销毁则直接返回。 /// </summary> +/// <returns>表示销毁流程完成的 <see cref="ValueTask" />。</returns> +/// <remarks> +/// 该方法是幂等的:重复调用会被直接忽略。 +/// 当由 Godot 生命周期退出回调触发时,会通过观察器异步调用并记录异常,避免未观测任务异常。 +/// </remarks> public override async ValueTask DestroyAsync()As per coding guidelines:
All public, protected, and internal types and members in C# code MUST include XML documentation comments (...),以及If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly in XML documentation。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot/Architectures/AbstractArchitecture.cs` around lines 126 - 131, Add XML documentation for the public async lifecycle method DestroyAsync: include a <returns> tag describing that it returns a ValueTask which completes when teardown is finished, and explicitly document lifecycle constraints—that DestroyAsync is asynchronous, may be invoked from the engine "exit tree" path or other observers, is safe to call multiple times (idempotent) and will return immediately if already destroyed, and describe thread/context expectations and that it calls installed extensions' OnDetach and clears the extension list; reference the method name DestroyAsync and the extension callback OnDetach in the text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@GFramework.Godot/Architectures/AbstractArchitecture.cs`:
- Around line 126-131: Add XML documentation for the public async lifecycle
method DestroyAsync: include a <returns> tag describing that it returns a
ValueTask which completes when teardown is finished, and explicitly document
lifecycle constraints—that DestroyAsync is asynchronous, may be invoked from the
engine "exit tree" path or other observers, is safe to call multiple times
(idempotent) and will return immediately if already destroyed, and describe
thread/context expectations and that it calls installed extensions' OnDetach and
clears the extension list; reference the method name DestroyAsync and the
extension callback OnDetach in the text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 233278ee-308a-4cc5-addf-1c6ce99794f6
📒 Files selected for processing (6)
GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.csGFramework.Core/Logging/ConfigurableLoggerFactory.csGFramework.Cqrs/Internal/WeakTypePairCache.csGFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.csGFramework.Godot/Architectures/AbstractArchitecture.csdocs/zh-CN/godot/setting.md
✅ Files skipped from review due to trivial changes (1)
- GFramework.Godot.Tests/Architectures/AbstractArchitectureModuleInstallationTests.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/zh-CN/godot/setting.md
- GFramework.Core/Logging/ConfigurableLoggerFactory.cs
- GFramework.Core.Tests/Logging/ConfigurableLoggerFactoryTests.cs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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 logicAll public, protected, and internal types and members in C# code MUST include XML documentation comments (
///) with<summary>,<param>,<returns>,<exception>, and<remarks>where applicableXML documentation comments must explain intent, contract, and usage constraints instead of restating syntax
If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly in XML documentation
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds, compatibility constraints, edge cases, registration order, lifecycle sequencing, or generated code assumptions
Avoid obvious comments such as
// increment iin C# codeCore 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
Generated logic and generator pipelines MUST explain what is generated, why it is generated, the semantic assumptions the generator relies on, and any diagnostics or fallback behavior
Methods with non-trivial logic in C# code MUST document the core idea, key decisions, and edge case handling if any
Comments in C# code MUST NOT be trivial, redundant, or misleading; prefer explaining
whyandwhen, not justwhatC# code should remain understandable without requiring external context; prefer slightly more explanation over too little for framework code
Missing required documentation...
Files:
GFramework.Godot/Architectures/AbstractArchitecture.csGFramework.Cqrs/Internal/WeakTypePairCache.cs
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Read `@.ai/environment/tools.ai.yaml` before choosing runtimes or CLI tools
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Use `@.ai/environment/tools.raw.yaml` only when you need the full collected facts behind the AI-facing hints
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Prefer project-relevant tools listed in the environment inventory instead of assuming every installed system tool is fair game
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: When working in WSL against this repository's Windows-backed worktree, prefer Windows Git from WSL (for example `git.exe`) instead of the Linux `git` binary
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Every completed task MUST pass at least one build validation before it is considered done
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: If the task changes multiple projects or shared abstractions, prefer a solution-level or affected-project `dotnet build ... -c Release`; otherwise use the smallest build command that still proves the result compiles
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: If the required build passes and there are task-related staged or unstaged changes, contributors MUST create a Git commit automatically instead of leaving the task uncommitted, unless the user explicitly says not to commit
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Commit messages MUST use Conventional Commits format: `<type>(<scope>): <summary>`
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: The commit `summary` MUST use simplified Chinese and briefly describe the main change
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: The commit `body` MUST use unordered list items, and each item MUST start with a verb such as `新增`、`修复`、`优化`、`更新`、`补充`、`重构`
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Keep technical terms in English when they are established project terms, such as `API`、`Model`、`System` in commit messages
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: If a new task starts while the current branch is `main`, contributors MUST first try to update local `main` from the remote, then create and switch to a dedicated branch before making substantive changes
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: The branch naming rule for a new task branch is `<type>/<topic-or-scope>`, where `<type>` should match the intended Conventional Commit category
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Use subagents only when the task is complex, the context is likely to grow too large, or the work can be split into independent parallel subtasks
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: The main agent MUST identify the critical path first. Do not delegate the immediate blocking task if the next local step depends on that result
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Use `explorer` subagents for read-only discovery, comparison, tracing, and narrow codebase questions
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Use `worker` subagents only for bounded implementation tasks with an explicit file or module ownership boundary
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Every delegation MUST specify the concrete objective, the expected output format, the files or subsystem the subagent owns, and any constraints about tests, diagnostics, or compatibility
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Subagents are not allowed to revert or overwrite unrelated changes from the user or other agents. They must adapt to concurrent work instead of assuming exclusive ownership of the repository
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Prefer lightweight models such as `gpt-5.1-codex-mini` for narrow exploration, indexing, and comparison tasks
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Prefer stronger models such as `gpt-5.4` for cross-module design work, non-trivial refactors, and tasks that require higher confidence reasoning
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: The main agent remains responsible for reviewing and integrating subagent output. Unreviewed subagent conclusions do not count as final results
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Keep abstractions projects free of implementation details and engine-specific dependencies
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Preserve existing module boundaries. Do not introduce new cross-module dependencies without clear architectural need
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Preserve deterministic behavior in registries, lifecycle orchestration, and generated outputs in C# code
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Every non-trivial feature, bug fix, or behavior change in code MUST include tests or an explicit justification for why a test is not practical
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Minimize new package dependencies. Add them only when necessary and keep scope narrow
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Treat source code, `*.csproj`, tests, generated snapshots, and packaging metadata as the primary evidence for documentation updates
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Treat `CoreGrid` as a secondary evidence source for real project adoption patterns, directory layouts, and end-to-end usage examples
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Treat existing `README.md` files and `docs/zh-CN/` pages as editable outputs, not authoritative truth
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Adding a new top-level module directory without a `README.md` is considered incomplete work
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: The main documentation site lives under `docs/`, with Chinese content under `docs/zh-CN/`
Learnt from: CR
Repo: GeWuYou/GFramework
Timestamp: 2026-04-18T13:52:44.468Z
Learning: Completing code changes without updating the active tracking document is considered incomplete work
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.Tests.cs : Reuse existing architecture test infrastructure when relevant: `ArchitectureTestsBase<T>`, `SyncTestArchitecture`, `AsyncTestArchitecture`
Applied to files:
GFramework.Godot/Architectures/AbstractArchitecture.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Public API changes must be covered by unit or integration tests
Applied to files:
GFramework.Godot/Architectures/AbstractArchitecture.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.Tests.cs : Regression fixes should include a test that fails before the fix and passes after it
Applied to files:
GFramework.Godot/Architectures/AbstractArchitecture.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Before considering work complete, confirm: required comments and XML docs are present, code follows repository style and naming rules, relevant tests were added or updated, sensitive or unsafe behavior was not introduced, user-facing documentation is updated when needed, and feature adoption docs under `docs/zh-CN/` were added or updated when functionality was added, removed, or refactored
Applied to files:
GFramework.Godot/Architectures/AbstractArchitecture.cs
📚 Learning: 2026-04-16T13:25:59.124Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 235
File: GFramework.SourceGenerators.Tests/Rule/ContextAwareGeneratorSnapshotTests.cs:1-3
Timestamp: 2026-04-16T13:25:59.124Z
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.Godot/Architectures/AbstractArchitecture.csGFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 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.Godot/Architectures/AbstractArchitecture.csGFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : When adding caching, pooling, or shared mutable state, document thread-safety assumptions and failure modes
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds, compatibility constraints or edge cases, and registration order or lifecycle sequencing
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : If a member participates in lifecycle, threading, registration, or disposal behavior, document that behavior explicitly
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Prefer slightly more explanation over too little for framework code
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.cs : Do not rely on implicit imports; declare every required `using` explicitly
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
📚 Learning: 2026-04-18T02:29:44.423Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-18T02:29:44.423Z
Learning: Applies to **/*.csproj : Follow repository defaults: `ImplicitUsings` disabled, `Nullable` enabled, `GenerateDocumentationFile` enabled for shipped libraries, `LangVersion` generally `preview` in main libraries and abstractions
Applied to files:
GFramework.Cqrs/Internal/WeakTypePairCache.cs
🔇 Additional comments (6)
GFramework.Cqrs/Internal/WeakTypePairCache.cs (4)
3-13: 类级缓存契约说明清晰完整。弱键回收语义、并发访问保证、以及 GC/Clear 后的未命中重建行为都已明确,便于调用方正确理解失效与恢复路径。
31-44:GetOrAdd实现稳健且考虑了热路径分配。参数校验完整,且通过
staticlambda + state 传参避免闭包捕获,性能与可读性都不错。
56-67:TryGetValue命中/未命中语义一致。未命中时统一返回
false并置value = null,与方法契约保持一致,行为可预测。
75-95: 清理与测试辅助入口边界定义合理。
Clear的测试隔离用途和GetValueOrDefaultForTesting的非运行时定位都已在文档中说明,接口意图明确。GFramework.Godot/Architectures/AbstractArchitecture.cs (2)
98-123:InstallGodotModule的防御性顺序调整很到位这里的
module空值校验、_anchor前置校验,以及“先登记再等待附加完成”的处理,能更好覆盖半安装场景下的清理路径,方向正确。
86-86: 异步销毁观察链路实现清晰,异常可观测性已增强
Bind(ObserveDestroyAsync)+ObserveDestroyCoreAsync的组合把退出树中的 fire-and-forget 销毁异常转成可见错误输出,这一修复有效降低了静默失败风险。Also applies to: 146-171
清理 GFramework.Core 与 GFramework.Cqrs 中的大量低风险 Meziantou 警告
修复 GFramework.Godot 运行时中的 ConfigureAwait、StringComparison 与参数校验告警
调整 Core SourceGenerators 中的字符串比较、文件命名与局部长方法问题
拆分部分配置与缓存辅助类型文件以消除 file/type mismatch 告警
更新 warning reduction 跟踪与执行记录,保留下一批结构性告警的恢复点
Summary by CodeRabbit
性能优化
可靠性提升
日志系统增强
小幅 API 更新
测试