Feat/config schema diagnostics#245
Conversation
- 新增 ConfigSchemaDiagnostics 类提供配置 schema 代码生成相关诊断 - 添加 JSON 解析错误、根对象检查、ID字段要求等12种诊断规则 - 实现配置验证逻辑，支持整数、数字、布尔值、邮箱等格式校验 - 添加日期时间、持续时间、URI、UUID 等字符串格式验证 - 实现 YAML 解析和注释提取功能 - 提供配置样本生成和批量编辑更新功能 - 添加模式匹配和格式验证的正则表达式支持
- 新增 AnalyzerReleases.Unshipped.md 包含 12 条配置模式分析器规则 - 添加游戏配置系统完整文档，涵盖 YAML 配置源文件和 JSON Schema 结构描述 - 提供推荐目录结构和 Schema 示例，支持一对象一文件的目录组织 - 介绍 Source Generator 生成配置类型、表包装和注册访问辅助功能 - 包含 VS Code 插件配置浏览、校验和表单编辑入口说明 - 提供 Godot 文本配置桥接和热重载模板示例 - 详细说明运行时校验行为和跨表引用机制 - 添加开发期热重载和生成器接入约定说明 - 包含 VS Code 工具能力和当前限制说明
|
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新增对 JSON Schema 对象级关键字 Changes
Sequence Diagram(s)sequenceDiagram
participant SchemaFile as Schema 文件
participant Generator as SchemaConfigGenerator
participant Analyzer as ConfigSchemaDiagnostics
participant Tool as Config Tool (JS)
participant Runtime as YamlConfigSchemaValidator/Loader
SchemaFile->>Generator: 解析 schema（包含 allOf）
Generator->>Analyzer: 校验 allOf 元数据 -> 可能生成 GF_ConfigSchema_012
Generator->>Tool: 将 allOf 文档化/嵌入约束说明
Tool->>Tool: 解析 allOf 为 inline schema 列表并纳入 UI hint 本地化
Runtime->>Runtime: 将 allOf 纳入 ObjectConstraints(AllOfSchemas)
Runtime->>Runtime: 对象匹配时逐条验证每个 allOf 条目（全部需匹配；首个失败抛出带索引的错误）
Runtime-->>Tool: 返回 localized allOfViolation 验证消息（若失败）
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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
docs/zh-CN/game/config-system.md (1)
793-796: 建议补一个最小allOf示例。这里已经说明了
allOf的 focused constraint block 语义，但还缺一个最小 schema/YAML 示例。由于这套语义和很多人对标准 JSON SchemaallOf的直觉不一样，最好直接示范“父对象先声明字段，再用allOf叠加 required/约束”的写法，并顺手注明它不会做属性合并。As per coding guidelines, integration-oriented features in
docs/zh-CN/MUST cover “minimal working usage example, and migration or compatibility notes when behavior changes”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/zh-CN/game/config-system.md` around lines 793 - 796, Add a minimal, runnable schema/YAML example demonstrating the documented focused constraint block behavior of allOf: show a parent object that first declares fields, then an allOf array containing an object-typed inline schema that imposes required/constraint (e.g., required on an existing field), and a short note that this does not merge/add properties from allOf entries but only layers constraints; also include a brief compatibility/migration note that this allOf differs from standard JSON Schema allOf merging semantics. Reference the allOf block and the parent object declaration in the example so readers can reproduce the pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.cs`:
- Around line 693-696: 修正 XML
注释的缩进不一致：在描述遍历行为的三行注释中，确保第二行（包含“该遍历覆盖对象属性、<c>dependentSchemas</c> / <c>allOf</c>
/ <c>not</c> 子 schema、”的那一行）前加上与上一行相同的一个前导空格，使其与同一 <summary>
注释块内的其他行对齐；在包含该注释的成员（即负责遍历 schema 树的生成器方法/类的 XML 注释块）中统一调整缩进以保持一致性。
In `@GFramework.Game/Config/YamlConfigSchemaValidator.cs`:
- Around line 1877-1945: ParseAllOfConstraints currently allows allOf entries to
introduce new object properties that aren't declared on the parent, producing
schemas that are impossible to satisfy; update ParseAllOfConstraints to validate
each allOf object's "properties" keys against the parent's declared property
whitelist (the same set used by ValidateObjectNode and logic around
dependentRequired) and reject (throw ConfigLoadExceptionFactory.Create with
SchemaUnsupported) any allOf entry that requires a property not present on the
parent, or alternately merge/normalize those properties into the parent schema
shape instead of silently allowing them; reference ParseAllOfConstraints,
ParseNode/YamlConfigSchemaNode (the parsed parent object), and the
dependentRequired handling to reuse the existing whitelist logic.
In `@GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs`:
- Around line 693-903: Add a generator unit test that covers the branch where an
allOf array contains an element that is not a JSON object value (e.g. null or a
literal), by creating a schema similar to existing tests but with "allOf": [
null ] (or [ 1 ] / [ true ]) under a property like "reward", running
SchemaGeneratorTestDriver.Run, and asserting the single diagnostic has Id
"GF_ConfigSchema_012", Severity Error, and a message that mentions the property
name (e.g. "reward") and a phrase like "Entry `#1` in 'allOf' must be an object
value"; name the test clearly (e.g.
Run_Should_Report_Diagnostic_When_AllOf_Entry_Is_Not_Object_Value) and place it
alongside the other allOf tests so TryValidateAllOfMetadata is covered.
In `@tools/gframework-config-tool/src/configValidation.js`:
- Line 1413: The generated nested path for allOf is using the wrong bracket
format `${displayPath}[allOf:${index}]`, causing mismatch with runtime
convention like `reward[allOf[0]]`; update the path construction in the call to
parseSchemaNode (where rawAllOfSchema and displayPath are used) to use the
runtime-compatible format `${displayPath}[allOf[${index}]]` so parsing errors
produce the same path as runtime/generator.
---
Nitpick comments:
In `@docs/zh-CN/game/config-system.md`:
- Around line 793-796: Add a minimal, runnable schema/YAML example demonstrating
the documented focused constraint block behavior of allOf: show a parent object
that first declares fields, then an allOf array containing an object-typed
inline schema that imposes required/constraint (e.g., required on an existing
field), and a short note that this does not merge/add properties from allOf
entries but only layers constraints; also include a brief
compatibility/migration note that this allOf differs from standard JSON Schema
allOf merging semantics. Reference the allOf block and the parent object
declaration in the example so readers can reproduce the pattern.
🪄 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: 42c33c2d-f1fd-4786-82ae-8a262813f845
📒 Files selected for processing (14)
GFramework.Game.SourceGenerators/AnalyzerReleases.Unshipped.mdGFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game.SourceGenerators/Diagnostics/ConfigSchemaDiagnostics.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.csGFramework.Game.Tests/Config/YamlConfigSchemaValidatorTests.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csdocs/zh-CN/game/config-system.mdtools/gframework-config-tool/src/configValidation.jstools/gframework-config-tool/src/extension.jstools/gframework-config-tool/src/localization.jstools/gframework-config-tool/src/localizationKeys.jstools/gframework-config-tool/test/configValidation.test.jstools/gframework-config-tool/test/localization.test.js
📜 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 (3)
**/*.cs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.cs: All public, protected, and internal types and members in C# MUST include XML documentation comments (///) using<summary>,<param>,<returns>,<exception>, and<remarks>tags 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 in XML documentation
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds and 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
Code should remain understandable without requiring external context. Prefer slightly more explanation over too little for framework code
Missing required documentation is a coding standards violation. Code that does not meet the documentation rules is considered incomplete
Do not rely on implicit imports. Declare every requiredusingexplicitly in C# files
Write null-safe code that respects nullable annotations instead of suppressing warnings by default
Use the namespace patternGFramework.{Module}.{Feature}with PascalCase segments
Follow standard C# naming: Types, methods, properties, events, and con...
Files:
GFramework.Game.SourceGenerators/Diagnostics/ConfigSchemaDiagnostics.csGFramework.Game.Tests/Config/YamlConfigSchemaValidatorTests.csGFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
{README.md,docs/**/*}
📄 CodeRabbit inference engine (AGENTS.md)
{README.md,docs/**/*}: Update the relevantREADME.mdordocs/page when behavior, setup steps, architecture guidance, or user-facing examples change
Keep code samples, package names, and command examples aligned with the current repository state in documentation
Prefer documenting behavior and design intent, not only API surface in documentation
If an existing documentation page no longer reflects the current implementation, fixing the code without fixing the documentation is considered incomplete work
Files:
docs/zh-CN/game/config-system.md
docs/zh-CN/**/*
📄 CodeRabbit inference engine (AGENTS.md)
docs/zh-CN/**/*: When a feature is added, removed, renamed, or substantially refactored, contributors MUST update or create the corresponding user-facing integration documentation indocs/zh-CN/in the same change
For 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 when behavior changes
Do not rely on "the code is self-explanatory" for framework features that consumers need to adopt; write the adoption path down so future users do not need to rediscover it from source
Files:
docs/zh-CN/game/config-system.md
🧠 Learnings (10)
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to docs/zh-CN/**/* : 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:
tools/gframework-config-tool/src/localization.jsdocs/zh-CN/game/config-system.md
📚 Learning: 2026-04-06T12:45:43.921Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 190
File: GFramework.Game/Config/GameConfigBootstrap.cs:1-3
Timestamp: 2026-04-06T12:45:43.921Z
Learning: In the GeWuYou/GFramework repository, C# files may omit explicit `using System*` imports because the project-wide `GlobalUsings.cs` (referenced via manual global `using` directives) supplies common namespaces (e.g., `System`, `System.Threading`, `System.Threading.Tasks`). During code review, do not flag missing `using System...` directives in `.cs` files as long as `GlobalUsings.cs` is present/used to provide those namespaces.
Applied to files:
GFramework.Game.SourceGenerators/Diagnostics/ConfigSchemaDiagnostics.csGFramework.Game.Tests/Config/YamlConfigSchemaValidatorTests.csGFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to docs/zh-CN/**/* : For 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 when behavior changes
Applied to files:
docs/zh-CN/game/config-system.md
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.SourceGenerators.Tests.cs : Source generator changes MUST be covered by generator tests
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.csproj : Public API changes must be covered by unit or integration tests
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.SourceGenerators.Tests.cs : Preserve snapshot-based verification patterns already used in the repository for source generator tests
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.csproj : Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.cs : Regression fixes should include a test that fails before the fix and passes after it
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
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.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.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 repo, treat `global using NUnit.Framework;` as provided project-wide by `GFramework.SourceGenerators.Tests/GlobalUsings.cs` (via GlobalUsings.cs). Do not require or flag missing per-file `using NUnit.Framework;` directives in test `.cs` files under `GFramework.SourceGenerators.Tests/`, since the project-wide global usings already include NUnit (along with other common namespaces).
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
🔇 Additional comments (16)
GFramework.Game.SourceGenerators/Diagnostics/ConfigSchemaDiagnostics.cs (1)
132-142: LGTM!新增的
InvalidAllOfMetadata诊断描述符遵循现有模式：ID 序列正确、消息格式与相似诊断（InvalidDependentRequiredMetadata、InvalidDependentSchemasMetadata）保持一致、严重级别为 Error 符合编译期校验的语义。GFramework.Game.SourceGenerators/AnalyzerReleases.Unshipped.md (1)
19-19: LGTM!新规则条目格式正确，与现有条目保持一致。
GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.cs (5)
163-170: LGTM!
allOf元数据的递归验证调用位置正确，遵循与dependentRequired和dependentSchemas相同的模式。
771-796: LGTM!
allOf遍历逻辑与dependentSchemas保持一致：仅在 object schema 上处理、递归访问子 schema、路径后缀格式清晰（[allOf:{index}]）。遍历时跳过非对象条目是正确的，因为结构校验由TryValidateAllOfMetadata负责。
1161-1297: LGTM!
allOf验证逻辑完整且与运行时验证器保持一致：
TryValidateAllOfDeclaration限制allOf仅用于 object schema（对应运行时ParseNode中的检查）TryValidateAllOfMetadata的三个校验点与YamlConfigSchemaValidator.ParseAllOfConstraints完全对齐：
allOf必须是数组- 每个条目必须是 object-valued schema
- 每个条目必须声明
type: "object"XML 文档说明了设计意图（focused constraint block 而非类型合并语义），有助于后续维护。
3627-3658: LGTM!
TryBuildAllOfDocumentation方法实现合理：
- 使用
[ ... ]格式表示数组（与dependentSchemas的{ ... }对象格式区分）- 复用
TryBuildInlineSchemaSummary保持文档生成逻辑一致- 正确处理空数组和非对象条目的边界情况
12-13: LGTM!类级摘要文档正确更新，明确说明
allOf已加入共享子集并写入生成代码文档。tools/gframework-config-tool/src/localizationKeys.js (1)
1-3: 新增键名与现有校验消息约定保持一致。这里的命名、冻结对象结构和导出方式都和现有
ValidationMessageKeys保持一致。tools/gframework-config-tool/src/extension.js (1)
1625-1733:allOf提示渲染实现合理且与现有提示体系一致这段变更把对象级
allOf约束纳入了表单提示链路，且复用了统一的 schema 摘要函数与 HTML 转义流程，方向正确、实现稳定。tools/gframework-config-tool/src/localization.js (2)
139-145: 英文本地化键新增完整，且占位符契约一致新增的
webview.hint.allOf与validation.allOfViolation文案结构清晰，参数占位符和调用侧契约匹配。
265-271: 中文本地化补充与英文语义保持对称
allOf提示与违规诊断文案都已补齐，语义与英文版本一致，适配当前本地化插值机制。tools/gframework-config-tool/test/localization.test.js (1)
150-176: 本地化回归测试覆盖点到位新增用例同时验证了
allOf提示与违规诊断在中英文下的插值输出，能有效防止后续 key 回归或文案漂移。GFramework.Game.Tests/Config/YamlConfigSchemaValidatorTests.cs (1)
129-182:allOf引用去重测试设计准确该用例精准覆盖了“同一 ref-table 字段在基础属性与
allOf子 schema 重复声明”这一关键场景，并通过计数与字段断言验证去重行为，测试价值很高。tools/gframework-config-tool/test/configValidation.test.js (2)
1795-1894:allOf解析层测试覆盖完整这组用例把
allOf的成功解析与三类非法声明都覆盖到了，能很好约束 schema 归一化阶段的输入契约。
2046-2125: 运行时allOf约束校验测试补得很扎实同时验证失败诊断（含条目序号）与多约束成功路径，基本闭合了
allOf在校验阶段的核心行为回归面。GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs (1)
67-301: 这组运行时回归测试覆盖得不错。成功路径、约束失败、以及几类
SchemaUnsupported分支都补到了，还顺手把失败后注册表保持为空一起锁住了，回归价值很高。
- 实现配置架构解析功能，支持对象、数组和标量类型的递归解析 - 添加YAML文件解析和注释提取功能 - 实现配置值的类型验证，包括整数、数字、布尔值和字符串格式验证 - 添加对日期、时间、持续时间、邮箱、URI和UUID等特殊格式的支持 - 实现表单更新应用功能，支持标量值和数组的批量编辑 - 添加配置架构枚举和常量值的处理逻辑 - 实现多语言本地化支持的验证消息系统 - 添加精确十进制计算功能，用于数值倍数约束验证 - 实现YAML标量格式化和反引用功能 - 添加配置架构模式规范化处理，包括正则表达式和格式验证
- 新增 YAML 配置源文件支持说明 - 添加 JSON Schema 结构描述功能介绍 - 提供一对象一文件目录组织方式 - 说明运行时只读查询机制 - 详细介绍 Source Generator 生成配置类型功能 - 添加 VS Code 插件配置浏览功能说明 - 提供推荐目录结构和 Schema 示例 - 说明 Godot 文本配置桥接方式 - 添加运行时读取模板和热重载模板 - 说明跨表引用和校验行为 - 提供开发期热重载功能说明 - 添加生成器接入约定说明 - 详细介绍 VS Code 工具功能特性
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.Game.SourceGenerators/Config/SchemaConfigGenerator.cs`:
- Around line 1360-1383: The code currently ignores an `allOf` entry when the
"properties" or "required" keyword exists but has the wrong JSON type; instead,
modify the logic around allOfSchema.TryGetProperty for "properties" and
"required" so that if the property exists but its ValueKind is incorrect you
create and return the appropriate diagnostic (use
ConfigSchemaDiagnostics.InvalidAllOfMetadata / GF_ConfigSchema_012 semantics),
set `diagnostic` via Diagnostic.Create with CreateFileLocation(filePath),
Path.GetFileName(filePath), and allOfEntryPath, and return false; preserve the
existing checks that continue when the keyword is absent, but do not return true
silently when the keyword is present with an invalid shape.
In `@GFramework.Game/Config/YamlConfigSchemaValidator.cs`:
- Around line 1997-2001: The current code silently returns when an allOf schema
contains a required property that is not an array; update the block that checks
allOfSchemaElement.TryGetProperty("required", out var allOfRequiredElement) &&
allOfRequiredElement.ValueKind != JsonValueKind.Array to emit an explicit
diagnostic (e.g., call the validator's diagnostic/reporting helper used
elsewhere in YamlConfigSchemaValidator such as
ReportDiagnostic/AddSchemaDiagnostic/AddError) describing "allOf.required must
be an array" and then stop processing that schema (return or skip further
validation), instead of silently returning; ensure you reference the
allOfSchemaElement and allOfRequiredElement in the diagnostic message so the
error can be located.
In `@GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs`:
- Around line 952-1003: Add a new generator test covering the alternate
GF_ConfigSchema_012 branch where an allOf entry introduces undeclared parent
properties: create a test in SchemaConfigGeneratorTests (mirror
Run_Should_Report_Diagnostic_When_AllOf_Entry_Targets_Undeclared_Parent_Property)
that feeds a schema where reward.allOf contains an object with a properties
section (e.g., allOf: [{ "type":"object", "properties": { "bonus": {
"type":"integer" } } }]) and run SchemaGeneratorTestDriver.Run; then assert the
single diagnostic is GF_ConfigSchema_012, severity Error, and its message
mentions the target slot (e.g., "reward[allOf[0]]") and that it
introduces/declares property 'bonus' in the parent object schema so
TryValidateAllOfEntryTargets' properties-branch is exercised.
🪄 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: e78fefa0-c585-4718-84c1-67eb03d23859
📒 Files selected for processing (7)
GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csdocs/zh-CN/game/config-system.mdtools/gframework-config-tool/src/configValidation.jstools/gframework-config-tool/test/configValidation.test.js
🚧 Files skipped from review as they are similar to previous changes (4)
- tools/gframework-config-tool/test/configValidation.test.js
- tools/gframework-config-tool/src/configValidation.js
- docs/zh-CN/game/config-system.md
- GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.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: Analyze (C#)
- GitHub Check: Code Quality & Security
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.cs: All public, protected, and internal types and members in C# MUST include XML documentation comments (///) using<summary>,<param>,<returns>,<exception>, and<remarks>tags 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 in XML documentation
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds and 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
Code should remain understandable without requiring external context. Prefer slightly more explanation over too little for framework code
Missing required documentation is a coding standards violation. Code that does not meet the documentation rules is considered incomplete
Do not rely on implicit imports. Declare every requiredusingexplicitly in C# files
Write null-safe code that respects nullable annotations instead of suppressing warnings by default
Use the namespace patternGFramework.{Module}.{Feature}with PascalCase segments
Follow standard C# naming: Types, methods, properties, events, and con...
Files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game/Config/YamlConfigSchemaValidator.cs
🧠 Learnings (9)
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.SourceGenerators.Tests.cs : Source generator changes MUST be covered by generator tests
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.csproj : Public API changes must be covered by unit or integration tests
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
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.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.csproj : Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.SourceGenerators.Tests.cs : Preserve snapshot-based verification patterns already used in the repository for source generator tests
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.cs : Regression fixes should include a test that fails before the fix and passes after it
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.cs : Keep tests focused on observable behavior, not implementation trivia
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.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.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game/Config/YamlConfigSchemaValidator.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 repo, treat `global using NUnit.Framework;` as provided project-wide by `GFramework.SourceGenerators.Tests/GlobalUsings.cs` (via GlobalUsings.cs). Do not require or flag missing per-file `using NUnit.Framework;` directives in test `.cs` files under `GFramework.SourceGenerators.Tests/`, since the project-wide global usings already include NUnit (along with other common namespaces).
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
🔇 Additional comments (1)
GFramework.Game/Config/YamlConfigSchemaValidator.cs (1)
1925-1933:allOf目标字段与父对象白名单对齐这点做得很好这段在解析期就拦截“
allOf引入父对象未声明字段”的不可满足 schema，能明显降低运行时歧义。
- 实现了基于JSON schema自动生成配置类型和配置表包装的功能 - 支持嵌套对象、对象数组、标量数组的数据结构生成 - 添加了default/enum/const/ref-table元数据映射功能 - 实现了查找索引生成功能，支持唯一键快速检索 - 集成了诊断报告系统，提供详细的错误提示信息 - 生成配置类、表类和绑定类三种类型的源代码 - 支持日期、时间、邮箱等字符串格式验证功能 - 实现了依赖关系验证，确保schema间的引用正确性
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@GFramework.Game/Config/YamlConfigSchemaValidator.cs`:
- Around line 1877-2042: The file has grown too large by accumulating
object-keyword logic; extract the object-focused parsing and validation (allOf +
dependentRequired + dependentSchemas) into a separate helper/partial to reduce
file size and improve cohesion: move ParseAllOfConstraints and
ValidateAllOfSchemaTargetsAgainstParentObject (and related functions that handle
object-focused parsing/whitelisting, runtime matching, and reference traversal)
into a new class or partial (e.g., ObjectKeywordValidator/ObjectKeywordParser)
and update callers (ParseNode and any reference traversal routines) to call the
new helper methods; ensure signatures and exception behavior remain identical so
existing callers (ParseNode, any callers of
ValidateAllOfSchemaTargetsAgainstParentObject) compile without behavior changes.
- Around line 2021-2033: The loop over allOfRequiredElement currently silently
skips invalid entries; change it to throw SchemaUnsupported for non-string or
empty required entries instead of continue: in the foreach over
allOfRequiredElement.EnumerateArray() replace the branches that check
requiredProperty.ValueKind != JsonValueKind.String and
string.IsNullOrWhiteSpace(requiredPropertyName) with a SchemaUnsupported throw
(including a clear message with requiredProperty or requiredPropertyName and
context such as the allOf-required array), while keeping the existing check that
continues when properties.ContainsKey(requiredPropertyName) if you still want to
ignore already-known properties; use the existing SchemaUnsupported exception
type so bad schemas like allOf.required: [1, ""] fail fast.
🪄 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: 637a404e-655f-432c-be5b-fd5de5aeb1aa
📒 Files selected for processing (4)
GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.cs
- GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
- GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.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: Analyze (C#)
- GitHub Check: Code Quality & Security
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.cs: All public, protected, and internal types and members in C# MUST include XML documentation comments (///) using<summary>,<param>,<returns>,<exception>, and<remarks>tags 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 in XML documentation
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds and 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
Code should remain understandable without requiring external context. Prefer slightly more explanation over too little for framework code
Missing required documentation is a coding standards violation. Code that does not meet the documentation rules is considered incomplete
Do not rely on implicit imports. Declare every requiredusingexplicitly in C# files
Write null-safe code that respects nullable annotations instead of suppressing warnings by default
Use the namespace patternGFramework.{Module}.{Feature}with PascalCase segments
Follow standard C# naming: Types, methods, properties, events, and con...
Files:
GFramework.Game/Config/YamlConfigSchemaValidator.cs
🧠 Learnings (1)
📚 Learning: 2026-04-06T12:45:43.921Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 190
File: GFramework.Game/Config/GameConfigBootstrap.cs:1-3
Timestamp: 2026-04-06T12:45:43.921Z
Learning: In the GeWuYou/GFramework repository, C# files may omit explicit `using System*` imports because the project-wide `GlobalUsings.cs` (referenced via manual global `using` directives) supplies common namespaces (e.g., `System`, `System.Threading`, `System.Threading.Tasks`). During code review, do not flag missing `using System...` directives in `.cs` files as long as `GlobalUsings.cs` is present/used to provide those namespaces.
Applied to files:
GFramework.Game/Config/YamlConfigSchemaValidator.cs
- 实现了基于JSON schema自动生成配置类型和配置表包装的功能 - 支持嵌套对象、对象数组、标量数组的数据结构生成 - 添加了default/enum/const/ref-table元数据的支持 - 实现了查找索引的自动生成和验证机制 - 集成了字符串格式验证包括date、datetime、email等格式 - 添加了dependentRequired和dependentSchemas的验证支持 - 实现了allOf组合约束的处理和验证 - 生成了配置目录类用于统一管理所有配置表 - 提供了完整的错误诊断和报告机制
There was a problem hiding this comment.
🧹 Nitpick comments (1)
GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs (1)
1-66: 测试脚手架设计合理。
- 使用
Guid.NewGuid()生成隔离的临时目录，避免测试间互相污染。SetUp和TearDown生命周期管理清晰。一个小建议：
TearDown中的Directory.Delete如果因文件被占用等原因失败会抛出异常。虽然对测试影响不大，但可以考虑捕获并记录异常以提高健壮性：🔧 可选的健壮性改进
[TearDown] public void TearDown() { if (!string.IsNullOrEmpty(_rootPath) && Directory.Exists(_rootPath)) { - Directory.Delete(_rootPath, true); + try + { + Directory.Delete(_rootPath, true); + } + catch (IOException) + { + // Ignore cleanup failures in test teardown + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs` around lines 1 - 66, TearDown currently calls Directory.Delete(_rootPath, true) directly which can throw (e.g., file locks); modify the TearDown method to wrap the Directory.Delete call in a try/catch, catching Exception, and when catching log the error (including _rootPath and exception details) via the test logger or TestContext.WriteLine so failures in cleanup don't abort the test run; keep the existing existence checks (_rootPath and Directory.Exists) and only attempt delete inside the try block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs`:
- Around line 1-66: TearDown currently calls Directory.Delete(_rootPath, true)
directly which can throw (e.g., file locks); modify the TearDown method to wrap
the Directory.Delete call in a try/catch, catching Exception, and when catching
log the error (including _rootPath and exception details) via the test logger or
TestContext.WriteLine so failures in cleanup don't abort the test run; keep the
existing existence checks (_rootPath and Directory.Exists) and only attempt
delete inside the try block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c5b82d56-98e6-4553-97df-2e48c581b3ac
📒 Files selected for processing (5)
GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.csGFramework.Game/Config/YamlConfigSchemaValidator.ObjectKeywords.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.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: Analyze (C#)
- GitHub Check: Code Quality & Security
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.cs: All public, protected, and internal types and members in C# MUST include XML documentation comments (///) using<summary>,<param>,<returns>,<exception>, and<remarks>tags 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 in XML documentation
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds and 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
Code should remain understandable without requiring external context. Prefer slightly more explanation over too little for framework code
Missing required documentation is a coding standards violation. Code that does not meet the documentation rules is considered incomplete
Do not rely on implicit imports. Declare every requiredusingexplicitly in C# files
Write null-safe code that respects nullable annotations instead of suppressing warnings by default
Use the namespace patternGFramework.{Module}.{Feature}with PascalCase segments
Follow standard C# naming: Types, methods, properties, events, and con...
Files:
GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game/Config/YamlConfigSchemaValidator.ObjectKeywords.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
🧠 Learnings (6)
📚 Learning: 2026-04-06T12:45:43.921Z
Learnt from: GeWuYou
Repo: GeWuYou/GFramework PR: 190
File: GFramework.Game/Config/GameConfigBootstrap.cs:1-3
Timestamp: 2026-04-06T12:45:43.921Z
Learning: In the GeWuYou/GFramework repository, C# files may omit explicit `using System*` imports because the project-wide `GlobalUsings.cs` (referenced via manual global `using` directives) supplies common namespaces (e.g., `System`, `System.Threading`, `System.Threading.Tasks`). During code review, do not flag missing `using System...` directives in `.cs` files as long as `GlobalUsings.cs` is present/used to provide those namespaces.
Applied to files:
GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game/Config/YamlConfigSchemaValidator.ObjectKeywords.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.cs : 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 stop and check whether responsibilities should be split before continuing
Applied to files:
GFramework.Game/Config/YamlConfigSchemaValidator.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
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.Game/Config/YamlConfigSchemaValidator.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.csproj : Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical
Applied to files:
GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
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/Config/YamlConfigLoaderAllOfTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.csproj : Public API changes must be covered by unit or integration tests
Applied to files:
GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
🔇 Additional comments (23)
GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.cs (7)
163-170: LGTM!
allOf元数据递归校验调用正确地插入到ParseSchema验证流程中，与dependentRequired/dependentSchemas的处理模式保持一致。
771-796: LGTM!
allOf遍历逻辑正确地集成到统一的 schema 树遍历器中。对非对象类型条目的静默跳过与现有dependentSchemas处理方式一致，且实际验证会在TryValidateAllOfMetadata中提前拒绝非法形状。
842-851: LGTM!
BuildAllOfEntryPath辅助方法使用[allOf[index]]格式生成路径，与运行时诊断路径格式保持一致，便于调试时关联编译期和运行时错误。
1172-1242: LGTM!
TryValidateAllOfDeclaration和TryValidateAllOfMetadataRecursively遵循与dependentSchemas相同的结构：先检查关键字是否只出现在 object 节点上，再委托到具体验证方法。
1360-1404: LGTM! 修复了先前的问题。现在当
properties或required关键字存在但类型不正确时，会正确生成GF_ConfigSchema_012诊断并返回失败，而不是静默跳过。这与先前审查意见中的建议一致。
3690-3695: LGTM!
allOf约束文档生成逻辑正确地集成到TryBuildConstraintDocumentation中，与dependentRequired/dependentSchemas的处理位置一致。
3777-3808: LGTM!
TryBuildAllOfDocumentation方法复用了TryBuildInlineSchemaSummary来生成紧凑的约束摘要，与TryBuildDependentSchemasDocumentation的实现模式保持一致。输出格式使用[ ... ]数组语法区分于dependentSchemas的{ ... }对象语法，语义清晰。GFramework.Game/Config/YamlConfigSchemaValidator.ObjectKeywords.cs (6)
1-11: LGTM!将对象级 schema 关键字解析逻辑拆分到独立的 partial 文件是一个很好的组织决策，符合编码指南中关于保持单文件在 800-1000 行以内的建议，有助于降低主校验文件的维护复杂度。
22-60: LGTM!
ParseObjectConstraints方法作为对象约束解析的入口点，正确地协调了各个约束解析器，并在minProperties > maxProperties时提前失败。返回值在所有约束都为空时返回null的逻辑也是正确的。
262-329: LGTM!
ParseAllOfConstraints方法正确实现了allOf数组解析，包括：
- 验证
allOf必须是数组- 验证每个条目必须是 object-valued schema
- 调用
ValidateAllOfSchemaTargetsAgainstParentObject确保只引用父对象已声明的字段- 通过
ParseNode递归解析每个条目并验证结果类型
343-430: LGTM!
ValidateAllOfSchemaTargetsAgainstParentObject方法的验证逻辑与生成器中TryValidateAllOfEntryTargets保持一致：
- 验证
properties如果存在必须是对象，且只能引用父对象已声明的字段- 验证
required如果存在必须是字符串数组，且只能引用父对象已声明的字段这确保了编译期诊断和运行时校验的行为对齐。
469-493: LGTM!辅助方法
TryParseObjectPropertyCountConstraint、DescribeObjectSchemaTarget和DescribeObjectSchemaTargetInClause提供了清晰的诊断消息构建逻辑，正确处理了根对象和嵌套属性的不同表述方式。
298-298: 路径格式已一致，无需调整。运行时通过
BuildNestedSchemaPath(propertyPath, $"allOf[{allOfIndex.ToString(CultureInfo.InvariantCulture)}]")生成的路径为propertyPath[allOf[X]]，生成器通过BuildAllOfEntryPath(displayPath, allOfIndex)生成的路径也是displayPath[allOf[X]]。两者格式一致，诊断路径可正常关联编译期和运行时错误消息。生成器的 XML 文档明确标注该方法为"与运行时一致的逻辑路径"，设计意图清晰。> Likely an incorrect or invalid review comment.GFramework.Game/Config/YamlConfigSchemaValidator.cs (5)
14-18: 类声明变更与文档更新看起来正确。将
YamlConfigSchemaValidator改为partial class以支持ObjectKeywords.cs分拆是合理的做法，符合将对象级关键字逻辑拆分以降低维护成本的目标。XML 文档也正确地补充了allOf的支持说明。
328-337: 非对象类型声明allOf的校验逻辑正确。在
ParseNode中提前拦截非 object 类型的allOf声明是正确的设计，与测试用例LoadAsync_Should_Throw_When_NonObject_Schema_Declares_AllOf的预期一致。错误信息清晰，指明了只有 object schemas 才能声明allOf。
926-1001:allOf运行时校验逻辑实现正确。几点观察：
- Lines 926-927: 对
DependentSchemas增加了Count > 0检查，避免空字典时的无效循环。- Lines 967-971: 同样对
AllOfSchemas做了 null 和 count 检查后提前返回。- Line 991:
allOfEntryNumber = index + 1提供 1-indexed 的条目编号，与测试期望的entry#1`` 格式一致。- 复用
TryMatchSchemaNode并设置allowUnknownObjectProperties: true，确保 allOf 按 focused constraint block 语义工作，与 dependentSchemas 行为一致。错误信息格式清晰，包含了具体的 allOf 条目索引，便于用户定位问题。
3373-3381: 跨表引用收集正确扩展到AllOfSchemas。与
DependentSchemas的引用遍历模式一致，确保allOf内联 schema 中声明的x-gframework-ref-table引用也被纳入依赖闭包计算，热重载时能正确推导受影响的表。
3975-4029:YamlConfigObjectConstraints扩展正确。新增的
AllOfSchemas参数和属性与现有的DependentSchemas风格保持一致，XML 文档也正确描述了其作为 "focused constraint block" 的语义。GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs (5)
67-131: 核心功能测试覆盖充分。
LoadAsync_Should_Throw_When_AllOf_Entry_Is_Not_Satisfied验证约束违反时的诊断输出，包括FailureKind、DisplayPath和消息内容中的allOf/entry#1``。LoadAsync_Should_Accept_When_All_AllOf_Entries_Are_Satisfied验证正常加载路径及反序列化结果。测试断言与
ValidateObjectConstraints中allOfEntryNumber = index + 1的实现一致，能正确验证 1-indexed 的条目编号格式。
133-261: Schema 结构校验错误场景覆盖全面。覆盖了以下错误场景：
allOf非数组 (Lines 136-171)- 非对象类型声明
allOf(Lines 173-226)allOf条目非对象值 (Lines 228-261)断言的
DisplayPath与Message内容期望与YamlConfigSchemaValidator.ObjectKeywords.cs中ParseAllOfConstraints的诊断输出一致。
263-506:allOf条目内部结构校验测试完整。覆盖了：
- 条目非 object-typed (Lines 263-301) - DisplayPath 期望
reward[allOf[0]]properties非对象映射 (Lines 303-341)required非数组 (Lines 343-381)required条目非字符串 (Lines 383-421)required条目为空白 (Lines 423-461)required引用未声明的父属性 (Lines 463-506)这些测试确保了之前 review 中提出的"显式拒绝非法
allOf.required条目"等问题已被正确实现和验证。
537-577: 辅助方法设计简洁。
BuildMonsterConfigYaml、BuildMonsterSchema和IndentLines提供了可复用的测试数据构建能力，减少了测试代码重复。raw string literal 的使用使 JSON/YAML 模板更易读。
604-656: 测试 Stub 类型设计恰当。
MonsterAllOfConfigStub、AllOfRewardConfigStub和MonsterTagConfigStub作为私有嵌套类型，仅服务于当前测试 fixture，符合测试隔离原则。XML 文档完整，符合编码规范。
- 在YamlConfigLoaderAllOfTests的清理方法中添加try-catch块 - 忽略测试拆解期间的目录删除失败异常 - 防止测试因权限或文件锁定问题而意外失败
There was a problem hiding this comment.
🧹 Nitpick comments (1)
GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs (1)
544-584: 建议为私有辅助方法添加简要 XML 文档。
BuildMonsterConfigYaml、BuildMonsterSchema和IndentLines这三个私有静态方法缺少 XML 文档。虽然编码规范主要针对 public/protected/internal 成员，但为保持与文件其他部分一致的高质量文档风格，建议补充简要说明。📝 建议添加的文档
+ /// <summary> + /// 构建带有指定奖励内容的怪物配置 YAML 文本。 + /// </summary> + /// <param name="rewardYaml">奖励对象的 YAML 片段。</param> + /// <returns>完整的怪物配置 YAML 文本。</returns> private static string BuildMonsterConfigYaml(string rewardYaml) { // ... } + /// <summary> + /// 构建带有指定奖励属性和 allOf 约束的怪物 schema JSON。 + /// </summary> + /// <param name="rewardPropertiesJson">奖励对象的 properties JSON 片段。</param> + /// <param name="allOfJson">allOf 约束的 JSON 数组片段。</param> + /// <returns>完整的 schema JSON 文本。</returns> private static string BuildMonsterSchema( string rewardPropertiesJson, string allOfJson) { // ... } + /// <summary> + /// 为多行文本的每一行添加指定数量的空格缩进。 + /// </summary> + /// <param name="text">原始文本。</param> + /// <param name="indentLevel">缩进空格数。</param> + /// <returns>添加缩进后的文本。</returns> private static string IndentLines(string text, int indentLevel) { // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs` around lines 544 - 584, 为私有静态辅助方法添加简短的 XML 文档注释：在 BuildMonsterConfigYaml、BuildMonsterSchema 和 IndentLines 方法上各添加一行或两行的 <summary> 描述，分别说明 BuildMonsterConfigYaml 用于构建怪物配置的 YAML 字符串（参数 rewardYaml 的作用），BuildMonsterSchema 用于构建怪物的 JSON schema（描述 rewardPropertiesJson 和 allOfJson 的用途），以及 IndentLines 用于按指定缩进级别缩进多行文本（说明 indentLevel 含义）；将注释放在方法声明上方，以保持与文件其余部分一致的文档风格。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs`:
- Around line 544-584: 为私有静态辅助方法添加简短的 XML 文档注释：在
BuildMonsterConfigYaml、BuildMonsterSchema 和 IndentLines 方法上各添加一行或两行的 <summary>
描述，分别说明 BuildMonsterConfigYaml 用于构建怪物配置的 YAML 字符串（参数 rewardYaml
的作用），BuildMonsterSchema 用于构建怪物的 JSON schema（描述 rewardPropertiesJson 和 allOfJson
的用途），以及 IndentLines 用于按指定缩进级别缩进多行文本（说明 indentLevel
含义）；将注释放在方法声明上方，以保持与文件其余部分一致的文档风格。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 97ee2279-d6d2-4bc0-9056-da055d929dea
📒 Files selected for processing (1)
GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.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: Analyze (C#)
- GitHub Check: Code Quality & Security
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.cs: All public, protected, and internal types and members in C# MUST include XML documentation comments (///) using<summary>,<param>,<returns>,<exception>, and<remarks>tags 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 in XML documentation
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds and 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
Code should remain understandable without requiring external context. Prefer slightly more explanation over too little for framework code
Missing required documentation is a coding standards violation. Code that does not meet the documentation rules is considered incomplete
Do not rely on implicit imports. Declare every requiredusingexplicitly in C# files
Write null-safe code that respects nullable annotations instead of suppressing warnings by default
Use the namespace patternGFramework.{Module}.{Feature}with PascalCase segments
Follow standard C# naming: Types, methods, properties, events, and con...
Files:
GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
🧠 Learnings (3)
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
Learning: Applies to **/*.Tests.csproj : Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical
Applied to files:
GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
📚 Learning: 2026-04-16T11:55:23.932Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-16T11:55:23.932Z
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/Config/YamlConfigLoaderAllOfTests.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.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs
🔇 Additional comments (6)
GFramework.Game.Tests/Config/YamlConfigLoaderAllOfTests.cs (6)
1-12: LGTM!类声明、命名空间和引用都符合规范，XML 文档清晰说明了测试目的。
42-72: LGTM!测试隔离机制设计合理：每个测试用例独立的临时目录 + GUID 避免冲突，TearDown 容忍清理失败以防掩盖实际测试错误。
74-138: LGTM!约束验证测试逻辑正确：
- 违反 allOf 时正确验证
FailureKind.ConstraintViolation、DisplayPath、错误消息内容- 满足约束时正确验证反序列化结果
测试用例与
YamlConfigSchemaValidator.cs中的验证逻辑（抛出 "entry #{allOfEntryNumber}"）完全对应。
140-233: LGTM!Schema 解析失败测试覆盖全面：
- 非数组
allOf声明 → 拒绝- 非对象类型声明
allOf→ 拒绝（与 context snippet 2 中的验证逻辑一致）allOf条目非对象值 → 拒绝每个测试都正确验证了
SchemaUnsupported失败类型和诊断路径。
270-513: LGTM!
allOf条目形状验证的边缘用例测试完善：
- 类型约束（必须是 object-typed）
properties必须是对象映射required必须是数组，元素必须是非空字符串required引用的属性必须在父对象 schema 中声明诊断路径
"reward[allOf[0]]"准确指向问题位置，符合预期。
611-662: LGTM!测试桩类设计简洁、文档完整：
- 属性有合理的默认值（
string.Empty、new()）避免空引用Stub后缀明确表示仅用于测试- XML 文档齐全，便于理解每个字段的用途
- 为 BuildMonsterConfigYaml 方法添加 XML 文档注释 - 为 BuildMonsterSchema 方法添加 XML 文档注释 - 为 IndentLines 方法添加 XML 文档注释 - 补充参数和返回值说明信息
1 participant
Summary by CodeRabbit
新功能
allOf的全面支持：对象级 inline 条目参与解析、运行时校验与引用收集。改进
allOf用法的校验与诊断（包含新增错误诊断），确保解析/生成/运行时路径一致且可定位。文档
allOf元数据提示。本地化
测试