feat(config): 支持对象约束型条件 schema#262
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 52 minutes and 13 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthrough本PR在配置系统中为对象级 JSON Schema 风格的 if/then/else 引入解析、结构化校验、运行时匹配、生成器文档化与诊断(GF_ConfigSchema_013),并同步更新工具链与多语言提示与测试覆盖。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Generator as Schema Generator
participant Parser as Schema Parser
participant Validator as Conditional Validator
participant Doc as Doc Generator
participant Diag as Diagnostics
User->>Generator: 提交包含 if/then/else 的 schema
Generator->>Parser: ParseSchema()
Parser->>Validator: TryValidateConditionalSchemasMetadataRecursively()
Validator->>Validator: 校验结构(对象类型、if/then/else 关系、父字段引用)
alt 非法条件元数据
Validator->>Diag: Emit GF_ConfigSchema_013
Diag-->>Generator: 返回诊断,解析失败
Generator-->>User: 报错并中止
else 合法
Validator-->>Parser: 验证通过
Parser->>Generator: 完整解析
Parser->>Doc: TryBuildConditionalDocumentation()
Doc->>Generator: 生成 if/then/else 文档摘要
Generator-->>User: 生成代码与文档成功
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Test ResultsDetails
Insights
build-and-test: Run #902
🎉 All tests passed!Slowest Tests
± Comparison with run #886 at defffbe | 🎉 No failed tests detected across all runs. | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 4 runs. Github Test Reporter by CTRF 💚 |
✅
|
| Descriptor | Linter | Files | Fixed | Errors | Warnings | Elapsed time |
|---|---|---|---|---|---|---|
| dotnet-format | yes | 1 | no | 4.88s | ||
| ✅ REPOSITORY | gitleaks | yes | no | no | 6.13s | |
| ✅ REPOSITORY | trufflehog | yes | no | no | 6.13s |
Detailed Issues
⚠️ CSHARP / dotnet-format - 1 error
Welcome to .NET 9.0!
---------------------
SDK Version: 9.0.114
----------------
Installed an ASP.NET Core HTTPS development certificate.
To trust the certificate, run 'dotnet dev-certs https --trust'
Learn about HTTPS: https://aka.ms/dotnet-https
----------------
Write your first app: https://aka.ms/dotnet-hello-world
Find out what's new: https://aka.ms/dotnet-whats-new
Explore documentation: https://aka.ms/dotnet-docs
Report issues and find source on GitHub: https://github.com/dotnet/core
Use 'dotnet --help' to see available commands or visit: https://aka.ms/dotnet-cli
--------------------------------------------------------------------------------------
Unhandled exception: System.Exception: Restore operation failed.
at Microsoft.CodeAnalysis.Tools.CodeFormatter.OpenMSBuildWorkspaceAsync(String solutionOrProjectPath, WorkspaceType workspaceType, Boolean noRestore, Boolean requiresSemantics, String binaryLogPath, Boolean logWorkspaceWarnings, ILogger logger, CancellationToken cancellationToken)
at Microsoft.CodeAnalysis.Tools.CodeFormatter.FormatWorkspaceAsync(FormatOptions formatOptions, ILogger logger, CancellationToken cancellationToken, String binaryLogPath)
at Microsoft.CodeAnalysis.Tools.FormatCommandCommon.FormatAsync(FormatOptions formatOptions, ILogger`1 logger, CancellationToken cancellationToken)
at Microsoft.CodeAnalysis.Tools.Commands.RootFormatCommand.FormatCommandDefaultHandler.InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
at System.CommandLine.Invocation.InvocationPipeline.InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
See detailed reports in MegaLinter artifacts
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

Show us your support by starring ⭐ the repository
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
tools/gframework-config-tool/src/localization.js (1)
140-141: 建议在ifElse提示中同时显式condition。
webview.hint.ifElse模板为"Otherwise: satisfy {schema}"(中英一致),不包含{condition}。当ifThen未渲染(例如仅存在else而没有then)时,用户只能看到 “Otherwise:…” 而无法直接看到“何种条件不成立”的上下文,可读性略差。可以考虑在
ifElse模板中加入{condition},并让extension.js的renderFieldHint同时传入condition参数(当前仅传schema,参见 Line 1747-1749)。非阻塞。Also applies to: 270-271
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gframework-config-tool/src/localization.js` around lines 140 - 141, Update the localization template "webview.hint.ifElse" to include the {condition} placeholder (e.g., "Otherwise (when {condition}): satisfy {schema}") and modify the renderer to pass the condition value when rendering else-only hints: in the renderFieldHint function (extension.js) ensure it supplies both schema and condition to the template (currently only schema is passed around at the if/else rendering logic), and apply the same change for the other occurrence referenced (the other localization pair at lines ~270-271) so both places render condition + schema consistently.tools/gframework-config-tool/src/extension.js (1)
1625-1625: JSDoc 类型字面量过长,可考虑抽取别名。
propertySchema的 JSDoc 类型自从引入dependentSchemas/allOf以来已相当庞大,本次再补ifSchema/thenSchema/elseSchema后单行长度远超仓库推荐的 ~120 列上限,维护时容易出错。建议抽取一个
@typedef InlineObjectSchemaHint(对应那套重复的{type?, required?, enumValues?, constValue?, constDisplayValue?, pattern?, refTable?})并在本处引用,dependentSchemas/allOf/ifSchema/thenSchema/elseSchema都可复用。非阻塞。As per coding guidelines: "Keep line length readable, around 120 characters as the preferred upper bound."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gframework-config-tool/src/extension.js` at line 1625, The JSDoc for propertySchema is excessively long due to repeated inline object shapes (used in dependentSchemas, allOf, ifSchema, thenSchema, elseSchema); extract a reusable typedef (e.g., `@typedef` InlineObjectSchemaHint) that defines {type?, required?, enumValues?, constValue?, constDisplayValue?, pattern?, refTable?} and then replace the repeated inline object literals in the propertySchema JSDoc with references to InlineObjectSchemaHint (use InlineObjectSchemaHint, Array.<InlineObjectSchemaHint>, or Record<string,InlineObjectSchemaHint> as appropriate for dependentSchemas/allOf/ifSchema/thenSchema/elseSchema) so the comment is much shorter and reuses the single alias.GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs (1)
1344-1520: 建议补齐对称分支的测试覆盖。当前新增的四个用例已覆盖
then孤立声明以及then内部 invalid format 的诊断路径,但以下与运行时校验(YamlConfigSchemaValidator.ObjectKeywords.cs第 365-383 行、ParseConditionalObjectSchema)对称的分支在生成器层未被直接触发测试:
else单独声明而缺少if(与Run_Should_Report_Diagnostic_When_Then_Is_Declared_Without_If对称)- 只声明了
if却没有then/else任一分支(对应运行时"must declare at least one of 'then' or 'else' when using 'if'")else分支内部声明 invalidformat(与Run_Should_Report_Diagnostic_With_Runtime_Aligned_Path_When_Then_Inner_Schema_Is_Invalid对称,用于验证reward[else].xxx的诊断路径)由于这些分支在运行时各自独立生成诊断,补齐后可以降低未来生成器/运行时诊断文案分叉时产生回归而测试无感知的风险。
基于学习内容:
Applies to **/*.Tests.cs : When a public API defines multiple contract branches, tests MUST cover the meaningful variants...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs` around lines 1344 - 1520, Add symmetric generator-layer tests in SchemaConfigGeneratorTests to mirror the existing conditional-branch coverage: create three new test methods (e.g., Run_Should_Report_Diagnostic_When_Else_Is_Declared_Without_If, Run_Should_Report_Diagnostic_When_If_Declared_Without_ThenOrElse, Run_Should_Report_Diagnostic_With_Runtime_Aligned_Path_When_Else_Inner_Schema_Is_Invalid) that follow the pattern of the existing tests (use SchemaGeneratorTestDriver.Run with a similar source stub and schema tuples), assert the corresponding diagnostics (IDs GF_ConfigSchema_013 for missing if, GF_ConfigSchema_013 or the appropriate ID for missing then/else, and GF_ConfigSchema_009 for invalid format) and check the message contains the runtime-aligned paths like "reward[else]" or "must declare at least one of 'then' or 'else' when using 'if'"; place these tests in the same test class (SchemaConfigGeneratorTests) alongside the existing Run_Should_* methods so generator-layer behavior matches ParseConditionalObjectSchema/runtime expectations.tools/gframework-config-tool/src/configValidation.js (1)
1506-1542:validateConditionalSchemaTargets与validateAllOfEntryTargets可考虑抽取共用。这两个函数的
properties键校验、required字符串校验结构几乎完全相同,只是错误消息中的定位片段不同('${keywordName}'vs'allOf' entry #${index + 1}')。建议抽取一个共用辅助函数,例如:♻️ 建议的抽取方式
function validateDeclaredTargetReferences(rawSchema, displayPath, contextLabel, properties) { if (!rawSchema || typeof rawSchema !== "object" || Array.isArray(rawSchema)) { return; } if (rawSchema.properties && typeof rawSchema.properties === "object" && !Array.isArray(rawSchema.properties)) { for (const propertyName of Object.keys(rawSchema.properties)) { if (!Object.prototype.hasOwnProperty.call(properties, propertyName)) { throw new Error( `Schema property '${displayPath}' declares property '${propertyName}' in ${contextLabel}, ` + "but that property is not declared in the parent object schema."); } } } if (!Array.isArray(rawSchema.required)) { return; } for (const requiredProperty of rawSchema.required) { if (typeof requiredProperty !== "string" || requiredProperty.trim().length === 0) { continue; } if (!Object.prototype.hasOwnProperty.call(properties, requiredProperty)) { throw new Error( `Schema property '${displayPath}' requires property '${requiredProperty}' in ${contextLabel}, ` + "but that property is not declared in the parent object schema."); } } }调用方传入
`'${keywordName}'`或`'allOf' entry #${index + 1}`即可。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gframework-config-tool/src/configValidation.js` around lines 1506 - 1542, The validateConditionalSchemaTargets implementation duplicates logic shared with validateAllOfEntryTargets; extract that common logic into a helper named validateDeclaredTargetReferences(rawSchema, displayPath, contextLabel, properties) which performs the same properties and required checks and throws the same error text but uses contextLabel in place of hardcoded keyword text. Replace the bodies of validateConditionalSchemaTargets and validateAllOfEntryTargets to call validateDeclaredTargetReferences(rawSchema, displayPath, `'${keywordName}'` ) and validateDeclaredTargetReferences(rawSchema, displayPath, `'allOf' entry #${index + 1}` ) respectively (passing the existing properties argument), ensuring all original validation behavior and error messages are preserved.GFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs (1)
68-83: TearDown 中吞掉所有异常可以接受但建议略作收窄。测试清理临时目录时采用
catch (Exception)是合理的防御,能避免清理失败干扰其他用例。如果希望更精确,可以只捕获IOException/UnauthorizedAccessException,让例如OperationCanceledException这类意外仍然冒出。非阻塞性建议。♻️ 可选收窄
try { Directory.Delete(_rootPath, true); } - catch (Exception) + catch (IOException) + { + // Ignore cleanup failures in test teardown + } + catch (UnauthorizedAccessException) { // 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/YamlConfigLoaderIfThenElseTests.cs` around lines 68 - 83, The TearDown method currently swallows all exceptions when deleting the temp directory; change the broad catch in TearDown to only catch relevant cleanup exceptions (e.g., IOException and UnauthorizedAccessException) around the Directory.Delete(_rootPath, true) call so unexpected exceptions like OperationCanceledException still bubble up; locate the TearDown() method and replace the catch (Exception) block with specific catches (or a combined catch clause) targeting IOException and UnauthorizedAccessException and keep the existing comment/ignore behavior for those cases.
🤖 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 1586-1595: 当前在处理条件分支时已经计算了 branchPath(via
BuildConditionalSchemaPath(displayPath, keywordName)),但当 schemaElement.ValueKind
不是对象时创建的诊断仍使用父 displayPath,导致错误位置不精确;请在创建 InvalidConditionalSchemaMetadata
诊断(Diagnostic.Create 调用)时把显示路径参数由 displayPath 改为 branchPath(保留
CreateFileLocation(filePath)、Path.GetFileName(filePath)、keywordName
等其它参数不变),以便非对象分支(如 then: [])的错误定位到具体分支路径;保持原有的返回 false 行为不变并在相同函数/上下文中(例如在
SchemaConfigGenerator.cs 的相关方法中)修改。
- Around line 4217-4240: The conditional-branch summaries currently only include
object-level info (required/etc.) via TryBuildInlineSchemaSummary, losing
per-property constraints (enum/const/range); update the logic that builds the
"if/then/else" parts to call a new helper (e.g.
TryBuildConditionalBranchSummary) which combines the existing
TryBuildInlineSchemaSummary with a properties-level summary produced by a new
TryBuildInlineObjectPropertiesSummary helper that iterates
schemaElement["properties"] and calls TryBuildInlineSchemaSummary for each
property and formats "name: summary"; add these two helpers
(TryBuildConditionalBranchSummary and TryBuildInlineObjectPropertiesSummary)
near the existing schema-summary methods and replace the current then/else/if
summary calls with the new helper so branches include both object and
per-property constraint details.
---
Nitpick comments:
In `@GFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs`:
- Around line 68-83: The TearDown method currently swallows all exceptions when
deleting the temp directory; change the broad catch in TearDown to only catch
relevant cleanup exceptions (e.g., IOException and UnauthorizedAccessException)
around the Directory.Delete(_rootPath, true) call so unexpected exceptions like
OperationCanceledException still bubble up; locate the TearDown() method and
replace the catch (Exception) block with specific catches (or a combined catch
clause) targeting IOException and UnauthorizedAccessException and keep the
existing comment/ignore behavior for those cases.
In `@GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs`:
- Around line 1344-1520: Add symmetric generator-layer tests in
SchemaConfigGeneratorTests to mirror the existing conditional-branch coverage:
create three new test methods (e.g.,
Run_Should_Report_Diagnostic_When_Else_Is_Declared_Without_If,
Run_Should_Report_Diagnostic_When_If_Declared_Without_ThenOrElse,
Run_Should_Report_Diagnostic_With_Runtime_Aligned_Path_When_Else_Inner_Schema_Is_Invalid)
that follow the pattern of the existing tests (use SchemaGeneratorTestDriver.Run
with a similar source stub and schema tuples), assert the corresponding
diagnostics (IDs GF_ConfigSchema_013 for missing if, GF_ConfigSchema_013 or the
appropriate ID for missing then/else, and GF_ConfigSchema_009 for invalid
format) and check the message contains the runtime-aligned paths like
"reward[else]" or "must declare at least one of 'then' or 'else' when using
'if'"; place these tests in the same test class (SchemaConfigGeneratorTests)
alongside the existing Run_Should_* methods so generator-layer behavior matches
ParseConditionalObjectSchema/runtime expectations.
In `@tools/gframework-config-tool/src/configValidation.js`:
- Around line 1506-1542: The validateConditionalSchemaTargets implementation
duplicates logic shared with validateAllOfEntryTargets; extract that common
logic into a helper named validateDeclaredTargetReferences(rawSchema,
displayPath, contextLabel, properties) which performs the same properties and
required checks and throws the same error text but uses contextLabel in place of
hardcoded keyword text. Replace the bodies of validateConditionalSchemaTargets
and validateAllOfEntryTargets to call
validateDeclaredTargetReferences(rawSchema, displayPath, `'${keywordName}'` )
and validateDeclaredTargetReferences(rawSchema, displayPath, `'allOf' entry
#${index + 1}` ) respectively (passing the existing properties argument),
ensuring all original validation behavior and error messages are preserved.
In `@tools/gframework-config-tool/src/extension.js`:
- Line 1625: The JSDoc for propertySchema is excessively long due to repeated
inline object shapes (used in dependentSchemas, allOf, ifSchema, thenSchema,
elseSchema); extract a reusable typedef (e.g., `@typedef` InlineObjectSchemaHint)
that defines {type?, required?, enumValues?, constValue?, constDisplayValue?,
pattern?, refTable?} and then replace the repeated inline object literals in the
propertySchema JSDoc with references to InlineObjectSchemaHint (use
InlineObjectSchemaHint, Array.<InlineObjectSchemaHint>, or
Record<string,InlineObjectSchemaHint> as appropriate for
dependentSchemas/allOf/ifSchema/thenSchema/elseSchema) so the comment is much
shorter and reuses the single alias.
In `@tools/gframework-config-tool/src/localization.js`:
- Around line 140-141: Update the localization template "webview.hint.ifElse" to
include the {condition} placeholder (e.g., "Otherwise (when {condition}):
satisfy {schema}") and modify the renderer to pass the condition value when
rendering else-only hints: in the renderFieldHint function (extension.js) ensure
it supplies both schema and condition to the template (currently only schema is
passed around at the if/else rendering logic), and apply the same change for the
other occurrence referenced (the other localization pair at lines ~270-271) so
both places render condition + schema consistently.
🪄 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: d1bb635f-1b93-4df0-8791-1877e5c3dfad
📒 Files selected for processing (16)
GFramework.Game.SourceGenerators/AnalyzerReleases.Unshipped.mdGFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game.SourceGenerators/Diagnostics/ConfigSchemaDiagnostics.csGFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.csGFramework.Game/Config/YamlConfigSchemaValidator.ObjectKeywords.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.mdai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.mddocs/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.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.cs: Apply [Log] attribute for automatic logging field and logging helper method generation
Apply [Priority] attribute for automatic priority comparison implementation generation
Apply [GenerateEnumExtensions] attribute to generate enumeration extension capabilities
Apply [ContextAware] attribute to automatically implement IContextAware boilerplate logic
**/*.cs: All public, protected, and internal types and members MUST include XML documentation comments (///) with<summary>,<param>,<returns>,<exception>, and<remarks>where applicable. Comments must explain intent, contract, and usage constraints instead of restating syntax.
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds or compatibility constraints, and registration order or lifecycle sequencing.
Core framework components such as Architecture, Module, System, Context, Registry, Service Module, and Lifecycle types MUST include high-level explanations of responsibilities, lifecycle, interaction with other components, why the abstraction exists, and when to use it instead of alternatives.
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling.
Follow the namespace patternGFramework.{Module}.{Feature}with PascalCase segments.
Use the following C# naming conventions: Types, methods, properties, events, and constants use PascalCase; interfaces useIprefix; parameters and locals use camelCase; private fields use_camelCase.
Do not rely on implicit imports. Declare every requiredusingexplicitly. Keepusingdirectives at the top of the file and sort them consistently.
Write null-safe code that respects nullable annotations instead of suppressing warnings by default. The project hasNullableenabled.
Use 4 spaces for indentation. Do not use tabs. Use Allman braces. Keep line length readable, around 120 characters as the preferred upper bound.
Prefer one primary type per...
Files:
GFramework.Game.SourceGenerators/Diagnostics/ConfigSchemaDiagnostics.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game/Config/YamlConfigSchemaValidator.ObjectKeywords.csGFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs
ai-plan/public/**/*
📄 CodeRabbit inference engine (AGENTS.md)
ai-plan/public/**/*: Contributors MUST keep committedai-plan/public/**content safe to publish in Git history. Never write secrets, tokens, credentials, private keys, machine usernames, home-directory paths, hostnames, IP addresses, proprietary URLs, or other sensitive environment details into anyai-plan/**file.
Never record absolute file-system paths inai-plan/**; use repository-relative paths, branch names, PR numbers, or stable document identifiers instead.
Files:
ai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.md
ai-plan/public/*/traces/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Contributors MUST maintain a matching execution trace under
ai-plan/public/<topic>/traces/for complex work. The trace should record the current date, key decisions, validation milestones, and the immediate next step. Update traces at each meaningful milestone before pausing or handing work off.
Files:
ai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.md
ai-plan/public/*/todos/**/*
📄 CodeRabbit inference engine (AGENTS.md)
When working from a tracked implementation plan, contributors MUST update the corresponding tracking document under
ai-plan/public/<topic>/todos/in the same change. Updates MUST reflect completed work, newly discovered issues, validation results, and the next recommended recovery point. Tracking files are recovery entrypoints, not append-only changelogs, and MUST stay concise.
Files:
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.md
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/config-system.md
docs/zh-CN/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/zh-CN/**/*.md: When a feature is added, removed, renamed, or substantially refactored, contributors MUST update or create the corresponding user-facing integration documentation indocs/zh-CN/in the same change. For integration-oriented features, documentation MUST cover project directory layout, file conventions, required project/package wiring, minimal working usage example, and migration/compatibility notes.
Code samples, package names, and command examples in documentation MUST be aligned with the current repository state. When an existing documentation page no longer reflects the current implementation, fix the code without fixing the documentation is considered incomplete work.
Prefer documenting behavior and design intent, not only API surface. Do not rely on 'the code is self-explanatory' for framework features that consumers need to adopt; write the adoption path down.
Files:
docs/zh-CN/game/config-system.md
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
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-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.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game/Config/YamlConfigSchemaValidator.ObjectKeywords.csGFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to ai-plan/public/*/traces/**/* : Contributors MUST maintain a matching execution trace under `ai-plan/public/<topic>/traces/` for complex work. The trace should record the current date, key decisions, validation milestones, and the immediate next step. Update traces at each meaningful milestone before pausing or handing work off.
Applied to files:
ai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.g.cs : Source generator changes MUST be covered by generator tests. Preserve snapshot-based verification patterns already used in the repository. When generator behavior changes intentionally, update snapshots together with the implementation.
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.mdGFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.Tests.cs : When a public API defines multiple contract branches, tests MUST cover the meaningful variants, including null, empty, default, and filtered inputs when those branches change behavior. Regression fixes should include a test that fails before the fix and passes after it.
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.Tests.cs : Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical. Public API changes must be covered by unit or integration tests.
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.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
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to ai-plan/public/*/todos/**/* : When working from a tracked implementation plan, contributors MUST update the corresponding tracking document under `ai-plan/public/<topic>/todos/` in the same change. Updates MUST reflect completed work, newly discovered issues, validation results, and the next recommended recovery point. Tracking files are recovery entrypoints, not append-only changelogs, and MUST stay concise.
Applied to files:
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: For any multi-step refactor, migration, or cross-module task, contributors MUST create or adopt a dedicated recovery document under `ai-plan/public/<topic>/todos/` before making substantive code changes. Recovery documents MUST record the current phase, the active recovery point identifier, known risks, and the next recommended resume step so another contributor or subagent can continue the work safely.
Applied to files:
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: 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:
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to docs/zh-CN/**/*.md : When a feature is added, removed, renamed, or substantially refactored, contributors MUST update or create the corresponding user-facing integration documentation in `docs/zh-CN/` in the same change. For integration-oriented features, documentation MUST cover project directory layout, file conventions, required project/package wiring, minimal working usage example, and migration/compatibility notes.
Applied to files:
docs/zh-CN/game/config-system.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.cs : 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.
Applied to files:
GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.cs
🪛 LanguageTool
ai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.md
[uncategorized] ~48-~48: 您的意思是“"不"齐”?
Context: ...坏形状的条件元数据,并把条件摘要写入 XML 文档 - VS Code 工具同步补齐 schema 解析、校验消息、本地化文本与表单 hint 元数据显示 ##...
(BU)
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.md
[uncategorized] ~40-~40: 您的意思是“"不"齐”?
Context: .../ Generator / Tooling 三端都能稳定解释的关键字 - 已补齐:enum(当前覆盖标量、对象、数组节点,以及标量数组元素)、`const...
(BU)
[uncategorized] ~78-~78: 您的意思是“"不"齐”?
Context: ...object-focused if / then / else 也已补齐;下一步转到下一批仍不改变生成形状的组合关键字评估,或继续推进 VS Code...
(BU)
[uncategorized] ~103-~103: 您的意思是“"不"齐”?
Context: ...- 工具链非阻塞风险:将 VS Code 功能标为非阻塞后,可能导致 C# 主线补齐新关键字时缺少工具侧同步验证 - 组合关键字范围风险:allOf 与 ...
(BU)
🔇 Additional comments (15)
GFramework.Game.SourceGenerators/AnalyzerReleases.Unshipped.md (1)
20-20: 新增诊断条目规范。
GF_ConfigSchema_013的类别、严重性、备注与既有条目保持一致,与ConfigSchemaDiagnostics.cs中新增的InvalidConditionalSchemaMetadata描述符对齐。tools/gframework-config-tool/src/localizationKeys.js (1)
5-5: 键位添加合理。新增
elseViolation与thenViolation两个 freeze 常量,与本地化字典及configValidation.js中的条件分支违规报告一致。Also applies to: 22-22
GFramework.Game.SourceGenerators/Diagnostics/ConfigSchemaDiagnostics.cs (1)
143-153: 诊断描述符定义与其他条目风格一致。ID、类别、模板占位符、严重性均与同文件其他
InvalidXxxMetadata条目对齐;XML 摘要也明确表达了意图。tools/gframework-config-tool/test/configValidation.test.js (1)
2778-2986: 测试覆盖面充分。新增用例同时覆盖:
parseSchemaContent正向采集ifSchema/thenSchema/elseSchema(含分支required);- 无
if单独声明then的反向拒绝路径;- 运行时
then/else分支违规的诊断输出;then与else同时满足时无诊断。行为与
configValidation.js的新增实现对齐。tools/gframework-config-tool/src/extension.js (1)
1735-1750: 提示渲染分支独立,行为符合语义。
ifThen与ifElse作为两条相互独立的 hint 追加,允许仅声明if + then或if + else的 schema 仍能渲染恰当提示;与configValidation.js的解析规则(if必填 + 至少一个分支)一致。唯一可打磨点:
webview.hint.ifElse未回显condition,详见localization.js的评论。ai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.md (1)
10-76: 跟踪文档更新符合恢复点规范。恢复点编号滚动到
AI-FIRST-CONFIG-RP-003,当前状态、已知风险、验证说明和下一步都已与本 PR 的实际落地(Runtime / Generator / Tooling 三端if/then/else支持)同步;未出现敏感环境信息或绝对路径。Based on learnings: "Tracking files are recovery entrypoints, not append-only changelogs, and MUST stay concise."
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.md (1)
40-41: Backlog 状态与主跟踪文档保持一致。恢复点编号、已知风险描述(组合关键字形状风险 / 非阻塞风险 / 组合范围风险)、最近验证时间(2026-04-20)及下一步动作均与
ai-first-config-system-tracking.md对齐;allOf与 object-focusedif/then/else的 P1 条目勾选一致。Also applies to: 78-78, 89-112
tools/gframework-config-tool/src/configValidation.js (4)
1132-1148: 非对象类型的if/then/else拒绝逻辑正确。在
type !== "object"时直接抛错,与上方allOf的保护一致;条件分支的进一步校验也只在对象分支中调用parseConditionalSchemaMetadata,层次清晰。
1444-1495: 条件 schema 解析流程正确且边界处理齐全。关键不变量都覆盖到了:
- 三者皆缺 → 返回
undefined(不产出ifSchema)。- 仅
then/else而无if→ 抛错(1452-1454),与 JSON Schema 语义一致。- 有
if但then/else全缺 → 抛错(1456-1458),避免生成只带ifSchema的无意义约束。parseConditionalObjectSchema对null、数组、非对象先通过 1484 行拒绝,然后解析后再次通过conditionalSchema.type !== "object"兜底,冗余但安全。下游
validateObjectNode/matchesSchemaNodeInternal在schemaNode.ifSchema缺失时不会生成elseViolation,整个链路自洽。
2010-2050:if/then/else运行时校验逻辑正确,去重模型与allOf/dependentSchemas保持一致。几个关键点都已处理好:
ifMatched在缺失ifSchema时固定为false,配合!ifMatched && schemaNode.ifSchema守护,避免在无条件约束时误报elseViolation。- 只有
then而无else、或只有else而无then的场景均能正确 no-op。- 继续使用
reportedMessages去重,与本文件中其他对象级约束的行为一致。与
matchesSchemaNodeInternal(2193-2207)的判定语义一致,不会出现校验通过但匹配失败(或相反)的割裂情况。
2193-2207: 匹配路径对if/then/else的处理与校验路径对齐。对
ifSchema使用allowUnknownObjectProperties=true进行匹配是正确的 JSON Schema 语义(分支 schema 不应因为对象上存在未声明属性而被判定为不匹配)。thenSchema/elseSchema的守护条件也足够严谨,避免了ifSchema缺失时错误地进入else分支。GFramework.Game/Config/YamlConfigSchemaValidator.cs (3)
969-1062: 对象级 if/then/else 与 allOf 的实现看起来没问题。
allOf循环在首个失败条目抛出,并通过index + 1保留 1-based 的条目序号,诊断信息清晰。if/then/else沿用与dependentSchemas/allOf相同的allowUnknownObjectProperties: truefocused-matcher 语义,控制流覆盖ThenSchema或ElseSchema为null的组合(ParseConditionalSchemasConstraints已保证至少一个 then/else + 必须有 if)。- 引用收集顺序也正确:
if命中时其引用通过TryMatchSchemaNode合并回 outer,之后then/else命中分支的引用再追加合并,CollectReferencedTableNames在 schema-load 阶段同时遍历三个分支,保证热重载依赖闭包与运行时收集闭包一致。
3596-3621:TryGetObjectOnlyKeywordName的优先级顺序符合预期。顺序为
allOf→if→then→else,在非 object 类型上同时声明if+then时会返回"if",与LoadAsync_Should_Throw_When_NonObject_Schema_Declares_If测试断言的诊断消息"can only declare 'if' on object schemas"对应。实现简洁且保持确定性。
4143-4181:YamlConfigConditionalSchemasDTO 建模合理。对必填的
IfSchema做了ArgumentNullException.ThrowIfNull,ThenSchema/ElseSchema保持 nullable,与解析器“if 必须存在 + 至少一个 then/else”的约束匹配。XML 文档齐全,字段语义解释到位。GFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs (1)
85-358: 条件分支回归测试覆盖度良好。7 个用例覆盖了运行时与 schema 解析两个阶段的关键分支:
if命中 +then满足 / 不满足(含默认值itemCount = 0/bonus = 0的断言);if未命中 +else满足 / 不满足;- 非对象字段声明
if(期望DisplayPath == "tag"与关键字专属消息);- 孤立
then(期望must declare 'if' when using 'then' or 'else');- 条件分支引用父对象未声明属性(期望
DisplayPath == "reward[if]",与解析器BuildNestedSchemaPath输出形状一致)。每个失败用例都同时断言了
FailureKind、DisplayPath、消息关键字和registry.Count == 0,对 schema 路径格式与注册回滚行为都形成锚点。
- 新增 Runtime、Source Generator 与 VS Code Tooling 对 object-focused if/then/else 的一致支持 - 补充运行时、生成器与工具链回归测试覆盖 - 更新配置系统文档与 ai-plan 恢复入口
- 修复 then/else 非 object 分支诊断定位到具体条件路径 - 优化 if/then/else 文档摘要,补充 properties 内约束说明 - 补充生成器回归测试,覆盖分支路径与文档输出
- 修复 gframework-pr-review 对 latest review body folded nitpick comments 的解析遗漏,并输出 declared / parsed 数量 - 优化 config tool 的条件提示与共享校验 helper - 补充 generator/runtime/tooling 回归测试并更新 ai-plan 跟踪
afc8b5e to
8a39f0a
Compare
Summary
Test ResultsDetails
Insights
build-and-test: Run #903
🎉 All tests passed!Slowest Tests
± Comparison with run #902 at 797e36f | 🎉 No failed tests detected across all runs. | 🍂 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/gframework-config-tool/src/configValidation.js (1)
1483-1567:⚠️ Potential issue | 🟠 Major收紧 tooling 对条件分支形状的拒绝规则。
parseSchemaNode会把缺省type当作"object",所以这里会接受缺少type: "object"的if/then/else;同时properties: []、required: "x"、空白 required 等坏形状会被跳过而不是报错。源生成器和运行时都会拒绝这些 schema,VS Code tooling 现在会提前放行无效 schema,造成三端不一致。建议修改
function parseConditionalObjectSchema(rawSchema, displayPath, keywordName, properties) { if (!rawSchema || typeof rawSchema !== "object" || Array.isArray(rawSchema)) { throw new Error(`Schema property '${displayPath}' must declare '${keywordName}' as an object-valued schema.`); } + if (rawSchema.type !== "object") { + throw new Error(`Schema property '${displayPath}' must declare an object-typed '${keywordName}' schema.`); + } + validateConditionalSchemaTargets(rawSchema, displayPath, keywordName, properties); const conditionalSchema = parseSchemaNode(rawSchema, `${displayPath}[${keywordName}]`); if (conditionalSchema.type !== "object") { throw new Error(`Schema property '${displayPath}' must declare an object-typed '${keywordName}' schema.`); } @@ function validateDeclaredTargetReferences(rawSchema, displayPath, contextLabel, properties) { if (!rawSchema || typeof rawSchema !== "object" || Array.isArray(rawSchema)) { return; } - if (rawSchema.properties && - typeof rawSchema.properties === "object" && - !Array.isArray(rawSchema.properties)) { + if (rawSchema.properties !== undefined) { + if (!rawSchema.properties || + typeof rawSchema.properties !== "object" || + Array.isArray(rawSchema.properties)) { + throw new Error( + `Schema property '${displayPath}' must declare 'properties' in ${contextLabel} as an object-valued map.`); + } + for (const propertyName of Object.keys(rawSchema.properties)) { if (Object.prototype.hasOwnProperty.call(properties, propertyName)) { continue; } @@ } } - if (!Array.isArray(rawSchema.required)) { + if (rawSchema.required === undefined) { return; } + if (!Array.isArray(rawSchema.required)) { + throw new Error( + `Schema property '${displayPath}' must declare 'required' in ${contextLabel} as an array of property names.`); + } + for (const requiredProperty of rawSchema.required) { - if (typeof requiredProperty !== "string" || requiredProperty.trim().length === 0) { - continue; + if (typeof requiredProperty !== "string") { + throw new Error( + `Schema property '${displayPath}' must declare 'required' entries in ${contextLabel} as property-name strings.`); + } + + if (requiredProperty.trim().length === 0) { + throw new Error( + `Schema property '${displayPath}' cannot declare blank property names in 'required' for ${contextLabel}.`); } if (Object.prototype.hasOwnProperty.call(properties, requiredProperty)) { continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gframework-config-tool/src/configValidation.js` around lines 1483 - 1567, The conditional-branch validation is too permissive: parseConditionalObjectSchema relies on parseSchemaNode defaulting missing type to "object" and validateDeclaredTargetReferences silently skips malformed shapes (properties as array, required not an array, or required entries that are non-strings/empty). Update parseConditionalObjectSchema to require rawSchema.type === "object" (reject when type is missing or different) and harden validateDeclaredTargetReferences to explicitly error on invalid shapes—reject when rawSchema.properties exists but is not a plain object (including arrays), reject when rawSchema.required exists but is not an array, and reject when any required entry is not a non-empty string; continue to check that each referenced property exists in properties and throw descriptive Errors when violations occur (referencing parseConditionalObjectSchema and validateDeclaredTargetReferences to locate changes).
🧹 Nitpick comments (2)
.codex/skills/gframework-pr-review/scripts/fetch_current_pr_review.py (1)
278-292: 微小可选优化:避免对每个 marker 重复调用find。
end_positions列表推导式里对每个 marker 都调用了两次remaining_body.find(marker)(一次在if条件里、一次在输出里)。虽然这些字符串不算长,对性能影响可忽略,但写法上略显冗余,顺手可改成一次性求值:♻️ 建议写法
- end_positions = [remaining_body.find(marker) for marker in end_markers if remaining_body.find(marker) >= 0] - end_index = min(end_positions) if end_positions else len(remaining_body) + end_positions = [position for position in (remaining_body.find(marker) for marker in end_markers) if position >= 0] + end_index = min(end_positions) if end_positions else len(remaining_body)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.codex/skills/gframework-pr-review/scripts/fetch_current_pr_review.py around lines 278 - 292, The code repeatedly calls remaining_body.find(marker) twice in the end_positions comprehension; replace that with a single evaluation per marker by iterating over end_markers, computing pos = remaining_body.find(marker) once, appending pos to end_positions only if pos >= 0, then compute end_index = min(end_positions) if any, and proceed to build comment_block and return as before (references: remaining_body, end_markers, end_positions, end_index, section_match, parse_comment_cards).GFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs (1)
279-314: 建议补齐else缺失if的运行时回归。生成器侧的
SchemaConfigGeneratorTests对then与else两条"缺失if"的诊断分支都做了覆盖(Run_Should_Report_Diagnostic_When_Then_Is_Declared_Without_If与Run_Should_Report_Diagnostic_When_Else_Is_Declared_Without_If),但运行时YamlConfigSchemaValidator是另一条独立代码路径,这里仅覆盖了then的情况。建议补一个else缺失if的同形用例,避免未来仅有一侧分支在运行时被正确拒绝造成运行时/生成器行为漂移。🧪 建议新增测试(示意)
/// <summary> /// 验证缺少 <c>if</c> 却声明 <c>else</c> 时,会在 schema 解析阶段被拒绝。 /// </summary> [Test] public void LoadAsync_Should_Throw_When_Else_Is_Declared_Without_If() { CreateConfigFile( "monster/slime.yaml", BuildMonsterConfigYaml( """ bonus: 1 """)); CreateSchemaFile( "schemas/monster.schema.json", BuildMonsterSchema( DefaultRewardPropertiesJson, """ "else": { "type": "object", "required": ["bonus"], "properties": { "bonus": { "type": "integer" } } } """)); var loader = CreateMonsterRewardLoader(); var registry = CreateRegistry(); var exception = Assert.ThrowsAsync<ConfigLoadException>(async () => await loader.LoadAsync(registry)); Assert.Multiple(() => { Assert.That(exception, Is.Not.Null); Assert.That(exception!.Diagnostic.FailureKind, Is.EqualTo(ConfigLoadFailureKind.SchemaUnsupported)); Assert.That(exception.Diagnostic.DisplayPath, Is.EqualTo("reward")); Assert.That(exception.Message, Does.Contain("must declare 'if' when using 'then' or 'else'")); Assert.That(registry.Count, Is.EqualTo(0)); }); }Based on learnings: "When a public API defines multiple contract branches, tests MUST cover the meaningful variants ... Regression fixes should include a test that fails before the fix and passes after it."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs` around lines 279 - 314, Add a symmetric test to cover the "else without if" runtime validation by creating a new test method named LoadAsync_Should_Throw_When_Else_Is_Declared_Without_If that mirrors LoadAsync_Should_Throw_When_Then_Is_Declared_Without_If: create a config file (e.g., bonus property), create a schema via BuildMonsterSchema that includes an "else" object block (instead of "then"), instantiate CreateMonsterRewardLoader and registry, call loader.LoadAsync(registry) and Assert.ThrowsAsync<ConfigLoadException>, then assert exception!.Diagnostic.FailureKind == ConfigLoadFailureKind.SchemaUnsupported, Diagnostic.DisplayPath == "reward", exception.Message contains "must declare 'if' when using 'then' or 'else'", and registry.Count == 0 so the YamlConfigSchemaValidator runtime branch for "else" is exercised and prevented from regressing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.md`:
- Line 81: There are duplicate "### 验证" headings causing MD024; locate the
second occurrence of the header text "### 验证" and rename it to a more specific
title (e.g., "### 验证 — PR follow-up" or "### 验证 (follow-up)") so anchors become
unique, and update any internal links/anchors that referenced the old heading if
present.
In `@GFramework.Game/Config/YamlConfigSchemaValidator.ObjectKeywords.cs`:
- Around line 439-447: The validator throws a schema error using the parent
propertyPath and diagnostic path instead of the branch-specific
conditionalSchemaPath; update the exception construction in the block that
checks conditionalSchemaElement.ValueKind to use the conditional branch path and
diagnostics (replace DescribeObjectSchemaTarget(propertyPath) and
GetDiagnosticPath(propertyPath) with
DescribeObjectSchemaTarget(conditionalSchemaPath) and
GetDiagnosticPath(conditionalSchemaPath) or otherwise pass conditionalSchemaPath
into ConfigLoadExceptionFactory.Create) so the reported location points to the
specific [if]/[then]/[else] branch (keep keywordName, schemaPath and tableName
as-is).
---
Outside diff comments:
In `@tools/gframework-config-tool/src/configValidation.js`:
- Around line 1483-1567: The conditional-branch validation is too permissive:
parseConditionalObjectSchema relies on parseSchemaNode defaulting missing type
to "object" and validateDeclaredTargetReferences silently skips malformed shapes
(properties as array, required not an array, or required entries that are
non-strings/empty). Update parseConditionalObjectSchema to require
rawSchema.type === "object" (reject when type is missing or different) and
harden validateDeclaredTargetReferences to explicitly error on invalid
shapes—reject when rawSchema.properties exists but is not a plain object
(including arrays), reject when rawSchema.required exists but is not an array,
and reject when any required entry is not a non-empty string; continue to check
that each referenced property exists in properties and throw descriptive Errors
when violations occur (referencing parseConditionalObjectSchema and
validateDeclaredTargetReferences to locate changes).
---
Nitpick comments:
In @.codex/skills/gframework-pr-review/scripts/fetch_current_pr_review.py:
- Around line 278-292: The code repeatedly calls remaining_body.find(marker)
twice in the end_positions comprehension; replace that with a single evaluation
per marker by iterating over end_markers, computing pos =
remaining_body.find(marker) once, appending pos to end_positions only if pos >=
0, then compute end_index = min(end_positions) if any, and proceed to build
comment_block and return as before (references: remaining_body, end_markers,
end_positions, end_index, section_match, parse_comment_cards).
In `@GFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs`:
- Around line 279-314: Add a symmetric test to cover the "else without if"
runtime validation by creating a new test method named
LoadAsync_Should_Throw_When_Else_Is_Declared_Without_If that mirrors
LoadAsync_Should_Throw_When_Then_Is_Declared_Without_If: create a config file
(e.g., bonus property), create a schema via BuildMonsterSchema that includes an
"else" object block (instead of "then"), instantiate CreateMonsterRewardLoader
and registry, call loader.LoadAsync(registry) and
Assert.ThrowsAsync<ConfigLoadException>, then assert
exception!.Diagnostic.FailureKind == ConfigLoadFailureKind.SchemaUnsupported,
Diagnostic.DisplayPath == "reward", exception.Message contains "must declare
'if' when using 'then' or 'else'", and registry.Count == 0 so the
YamlConfigSchemaValidator runtime branch for "else" is exercised and prevented
from regressing.
🪄 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: 6abecf7b-6b40-4033-9370-312232e3d1a1
📒 Files selected for processing (19)
.codex/skills/gframework-pr-review/SKILL.md.codex/skills/gframework-pr-review/scripts/fetch_current_pr_review.pyGFramework.Game.SourceGenerators/AnalyzerReleases.Unshipped.mdGFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.csGFramework.Game.SourceGenerators/Diagnostics/ConfigSchemaDiagnostics.csGFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.csGFramework.Game/Config/YamlConfigSchemaValidator.ObjectKeywords.csGFramework.Game/Config/YamlConfigSchemaValidator.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.mdai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.mddocs/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
✅ Files skipped from review due to trivial changes (6)
- GFramework.Game.SourceGenerators/AnalyzerReleases.Unshipped.md
- tools/gframework-config-tool/test/localization.test.js
- tools/gframework-config-tool/test/configValidation.test.js
- tools/gframework-config-tool/src/localizationKeys.js
- GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.cs
- GFramework.Game/Config/YamlConfigSchemaValidator.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- tools/gframework-config-tool/src/localization.js
- docs/zh-CN/game/config-system.md
📜 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 (4)
**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.cs: Apply [Log] attribute for automatic logging field and logging helper method generation
Apply [Priority] attribute for automatic priority comparison implementation generation
Apply [GenerateEnumExtensions] attribute to generate enumeration extension capabilities
Apply [ContextAware] attribute to automatically implement IContextAware boilerplate logic
**/*.cs: All public, protected, and internal types and members MUST include XML documentation comments (///) with<summary>,<param>,<returns>,<exception>, and<remarks>where applicable. Comments must explain intent, contract, and usage constraints instead of restating syntax.
Add inline comments for non-trivial logic, concurrency or threading behavior, performance-sensitive paths, workarounds or compatibility constraints, and registration order or lifecycle sequencing.
Core framework components such as Architecture, Module, System, Context, Registry, Service Module, and Lifecycle types MUST include high-level explanations of responsibilities, lifecycle, interaction with other components, why the abstraction exists, and when to use it instead of alternatives.
Methods with non-trivial logic MUST document the core idea, key decisions, and edge case handling.
Follow the namespace patternGFramework.{Module}.{Feature}with PascalCase segments.
Use the following C# naming conventions: Types, methods, properties, events, and constants use PascalCase; interfaces useIprefix; parameters and locals use camelCase; private fields use_camelCase.
Do not rely on implicit imports. Declare every requiredusingexplicitly. Keepusingdirectives at the top of the file and sort them consistently.
Write null-safe code that respects nullable annotations instead of suppressing warnings by default. The project hasNullableenabled.
Use 4 spaces for indentation. Do not use tabs. Use Allman braces. Keep line length readable, around 120 characters as the preferred upper bound.
Prefer one primary type per...
Files:
GFramework.Game.SourceGenerators/Diagnostics/ConfigSchemaDiagnostics.csGFramework.Game/Config/YamlConfigSchemaValidator.ObjectKeywords.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs
ai-plan/public/**/*
📄 CodeRabbit inference engine (AGENTS.md)
ai-plan/public/**/*: Contributors MUST keep committedai-plan/public/**content safe to publish in Git history. Never write secrets, tokens, credentials, private keys, machine usernames, home-directory paths, hostnames, IP addresses, proprietary URLs, or other sensitive environment details into anyai-plan/**file.
Never record absolute file-system paths inai-plan/**; use repository-relative paths, branch names, PR numbers, or stable document identifiers instead.
Files:
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.mdai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.md
ai-plan/public/*/todos/**/*
📄 CodeRabbit inference engine (AGENTS.md)
When working from a tracked implementation plan, contributors MUST update the corresponding tracking document under
ai-plan/public/<topic>/todos/in the same change. Updates MUST reflect completed work, newly discovered issues, validation results, and the next recommended recovery point. Tracking files are recovery entrypoints, not append-only changelogs, and MUST stay concise.
Files:
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.md
ai-plan/public/*/traces/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Contributors MUST maintain a matching execution trace under
ai-plan/public/<topic>/traces/for complex work. The trace should record the current date, key decisions, validation milestones, and the immediate next step. Update traces at each meaningful milestone before pausing or handing work off.
Files:
ai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.md
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
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-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: 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:
.codex/skills/gframework-pr-review/SKILL.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.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/Config/YamlConfigSchemaValidator.ObjectKeywords.csGFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to ai-plan/public/*/todos/**/* : When working from a tracked implementation plan, contributors MUST update the corresponding tracking document under `ai-plan/public/<topic>/todos/` in the same change. Updates MUST reflect completed work, newly discovered issues, validation results, and the next recommended recovery point. Tracking files are recovery entrypoints, not append-only changelogs, and MUST stay concise.
Applied to files:
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: For any multi-step refactor, migration, or cross-module task, contributors MUST create or adopt a dedicated recovery document under `ai-plan/public/<topic>/todos/` before making substantive code changes. Recovery documents MUST record the current phase, the active recovery point identifier, known risks, and the next recommended resume step so another contributor or subagent can continue the work safely.
Applied to files:
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.mdai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.g.cs : Source generator changes MUST be covered by generator tests. Preserve snapshot-based verification patterns already used in the repository. When generator behavior changes intentionally, update snapshots together with the implementation.
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.Tests.cs : When a public API defines multiple contract branches, tests MUST cover the meaningful variants, including null, empty, default, and filtered inputs when those branches change behavior. Regression fixes should include a test that fails before the fix and passes after it.
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to **/*.Tests.cs : Every non-trivial feature, bug fix, or behavior change MUST include tests or an explicit justification for why a test is not practical. Public API changes must be covered by unit or integration tests.
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.csGFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Run targeted tests for the code you changed whenever possible. Run broader solution-level validation for changes that touch shared abstractions, lifecycle behavior, source generators, or dependency wiring. Do not claim completion if required tests were skipped; state what was not run and why.
Applied to files:
GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.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
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: Applies to ai-plan/public/*/traces/**/* : Contributors MUST maintain a matching execution trace under `ai-plan/public/<topic>/traces/` for complex work. The trace should record the current date, key decisions, validation milestones, and the immediate next step. Update traces at each meaningful milestone before pausing or handing work off.
Applied to files:
ai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.md
📚 Learning: 2026-04-19T07:58:40.583Z
Learnt from: CR
Repo: GeWuYou/GFramework PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-19T07:58:40.583Z
Learning: If a task depends on observations from `ai-libs/**`, record the referenced path and conclusion in the active plan or trace when the work is multi-step or complex, or when an active tracking document already exists, rather than editing the third-party reference copy.
Applied to files:
ai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.md
🪛 LanguageTool
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.md
[grammar] ~53-~53: Ensure spelling is correct
Context: ...状态: - feat/ai-first-config 已 rebase 到 origin/feat/ai-first-config - 当前已解决“ahead / behind 同时存在”的分支差异,不再 behind 远端 - 当前最细...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.md
[uncategorized] ~40-~40: 您的意思是“"不"齐”?
Context: .../ Generator / Tooling 三端都能稳定解释的关键字 - 已补齐:enum(当前覆盖标量、对象、数组节点,以及标量数组元素)、`const...
(BU)
[uncategorized] ~78-~78: 您的意思是“"不"齐”?
Context: ...object-focused if / then / else 也已补齐;下一步转到下一批仍不改变生成形状的组合关键字评估,或继续推进 VS Code...
(BU)
[uncategorized] ~103-~103: 您的意思是“"不"齐”?
Context: ...- 工具链非阻塞风险:将 VS Code 功能标为非阻塞后,可能导致 C# 主线补齐新关键字时缺少工具侧同步验证 - 组合关键字范围风险:allOf 与 ...
(BU)
ai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.md
[uncategorized] ~48-~48: 您的意思是“"不"齐”?
Context: ...坏形状的条件元数据,并把条件摘要写入 XML 文档 - VS Code 工具同步补齐 schema 解析、校验消息、本地化文本与表单 hint 元数据显示 ##...
(BU)
[uncategorized] ~74-~74: 您的意思是“"不"齐”?
Context: ...e 校验 helper - Source Generator tests 已补齐对称分支覆盖 - Runtime test cleanup 已从 `cat...
(BU)
🪛 markdownlint-cli2 (0.22.0)
ai-plan/public/ai-first-config-system/traces/ai-first-config-system-trace.md
[warning] 81-81: Multiple headings with the same content
(MD024, no-duplicate-heading)
🔇 Additional comments (8)
GFramework.Game.SourceGenerators/Diagnostics/ConfigSchemaDiagnostics.cs (1)
144-153: 条件 schema 诊断描述符补齐一致。
GF_ConfigSchema_013的 ID、严重级别和消息参数顺序都与生成器校验调用保持一致,可以清晰覆盖if/then/else元数据坏形状。.codex/skills/gframework-pr-review/SKILL.md (1)
20-20: PR review skill 的输入来源说明补齐到位。最新 review body 与 folded nitpick section 被明确纳入解析范围,能避免后续只看 issue comments 时漏掉建议。
Also applies to: 43-43, 59-59
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-tracking.md (1)
10-87: 恢复点记录可接手。当前焦点、风险、验证命令和下一步都已同步到
AI-FIRST-CONFIG-RP-003,并且没有引入敏感环境信息。tools/gframework-config-tool/src/extension.js (1)
1579-1647: 条件 schema 表单 hint 接入清晰。新增 typedef 覆盖了
ifSchema/thenSchema/elseSchema,渲染时也同时展示条件与分支摘要,并在输出 HTML 前统一转义。Also applies to: 1801-1817
ai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.md (1)
40-41: 下一阶段 backlog 与本次能力边界同步。已把 object-focused
if/then/else纳入已完成子集,并继续保留“不改变生成类型形状”的下一步筛选线。Also applies to: 78-112
tools/gframework-config-tool/src/configValidation.js (1)
1989-2029: 条件分支运行时校验路径保持对称。
then与else的匹配分支、无诊断 matcher 结果,以及中英文 fallback 文案都已同步补齐,整体语义清晰。Also applies to: 2172-2187, 2605-2696
GFramework.Game/Config/YamlConfigSchemaValidator.ObjectKeywords.cs (1)
458-463: 不存在语义漂移,当前代码已正确处理。
ParseNode方法在行 316-321 中明确要求type属性存在且为字符串类型,若缺失则直接抛出异常。因此conditionalSchemaElement必须包含显式的type声明,否则ParseNode会拒绝。这与源生成器的验证要求(SchemaConfigGenerator.cs 行 1598-1600)保持一致,不存在运行时接受生成器拒绝的场景。GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs (1)
1270-1753: 新增条件 schema 的生成器测试覆盖全面,LGTM。八个新用例在成功路径(
if/then/else合成 XML 文档)、诊断路径(GF_ConfigSchema_013:孤立then/ 孤立else/ 仅有if/then非对象、分支引用父对象未声明字段)以及嵌套then/else内层走GF_ConfigSchema_009的运行时对齐路径(reward[then].itemCount、reward[else].bonus)三个维度上形成了完整覆盖,且命名、结构与本文件既有的allOf/dependentSchemas用例风格完全一致。诊断消息断言(如 "must also declare 'if'"、"must also declare at least one of 'then' or 'else'"、"must be an object-valued schema")也与 PR 中生成器实现新增的提示相吻合。
- 修复 Runtime 条件分支 schema 坏形状的诊断路径,改为指向具体 if/then/else 分支 - 新增 else 缺失 if 的运行时回归测试,保持与 Generator 覆盖对称 - 更新 ai-plan 跟踪与 trace,记录 PR #262 follow-up 验证并消除重复标题
Summary
Test ResultsDetails
Insights
build-and-test: Run #904
🎉 All tests passed!Slowest Tests
± Comparison with run #903 at 39346ba | 🎉 No failed tests detected across all runs. | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 6 runs. Github Test Reporter by CTRF 💚 |
- 修复 gframework-pr-review 对 outside-diff 评论与 Python nitpick 卡片的解析,并补充结构化输出 - 优化 review section 解析边界,避免 latest review body 区块串读并消化 marker 查找 nitpick - 收紧 config tool 对条件分支 schema 坏形状的拒绝规则,并新增 JS 回归测试 - 更新 ai-plan 跟踪与 trace,记录本轮 PR #262 follow-up 验证结果
Summary
Test ResultsDetails
Insights
build-and-test: Run #906
🎉 All tests passed!Slowest Tests
± Comparison with run #904 at a1beebf | 🎉 No failed tests detected across all runs. | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 7 runs. Github Test Reporter by CTRF 💚 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary by CodeRabbit
发布说明