mirror of
https://github.com/GeWuYou/GFramework.git
synced 2026-05-07 00:39:00 +08:00
fix(ai-first-config): 修复 PR review nitpick 解析与跟进收口
- 修复 gframework-pr-review 对 latest review body folded nitpick comments 的解析遗漏，并输出 declared / parsed 数量 - 优化 config tool 的条件提示与共享校验 helper - 补充 generator/runtime/tooling 回归测试并更新 ai-plan 跟踪
This commit is contained in:
GeWuYou
parent
0da15f6ffd
commit
8a39f0a932
@ -17,6 +17,7 @@ Shortcut: `$gframework-pr-review`
|
||||
- locate the PR for the current branch through the GitHub PR API
|
||||
- fetch PR metadata, issue comments, reviews, and review comments through the GitHub API
|
||||
- extract `Summary by CodeRabbit`、GitHub Actions bot comments such as `MegaLinter analysis: Success with warnings`、and CTRF test reports from issue comments
|
||||
- parse the latest CodeRabbit review body itself, including folded sections such as `🧹 Nitpick comments (N)` and the overall AI-agent prompt
|
||||
- fetch the latest head commit review threads from the GitHub PR API
|
||||
- prefer unresolved review threads on the latest head commit over older summary-only signals
|
||||
- extract failed checks, MegaLinter detailed issues, and test-report signals such as `Failed Tests` or `No failed tests in this run`
|
||||
@ -39,6 +40,7 @@ The script should produce:
|
||||
|
||||
- PR metadata: number, title, state, branch, URL
|
||||
- CodeRabbit summary block from issue comments when available
|
||||
- Folded latest-review sections such as `Nitpick comments (N)` when CodeRabbit puts them in the review body instead of issue comments
|
||||
- Parsed latest head-review threads, with unresolved threads clearly separated
|
||||
- Latest head commit review metadata and review threads
|
||||
- Unresolved latest-commit review threads after reply-thread folding
|
||||
@ -54,6 +56,7 @@ The script should produce:
|
||||
- Prefer GitHub API results over PR HTML. The PR HTML page is now a fallback/debugging source, not the primary source of truth.
|
||||
- If the summary block and the latest head review threads disagree, trust the latest unresolved head-review threads and treat older summary findings as stale until re-verified locally.
|
||||
- Treat GitHub Actions comments with `Success with warnings` as actionable review input when they include concrete linter diagnostics such as `MegaLinter` detailed issues; do not skip them just because the parent check is green.
|
||||
- Do not assume all CodeRabbit findings live in issue comments. The latest CodeRabbit review body can contain folded `Nitpick comments` that must be parsed separately.
|
||||
|
||||
## Example Triggers
|
||||
|
||||
|
||||
@ -210,19 +210,39 @@ def parse_actionable_comments(actionable_block: str) -> dict[str, Any]:
|
||||
comment_count_match = re.search(r"Actionable comments posted:\s*(\d+)", actionable_block)
|
||||
count = int(comment_count_match.group(1)) if comment_count_match else 0
|
||||
|
||||
comments: list[dict[str, str]] = []
|
||||
primary_block = actionable_block.split(
|
||||
"<details>\n<summary>🤖 Prompt for all review comments with AI agents</summary>",
|
||||
1,
|
||||
)[0]
|
||||
comments = parse_comment_cards(primary_block)
|
||||
|
||||
prompt_match = re.search(
|
||||
r"<summary>🤖 Prompt for all review comments with AI agents</summary>\s*```(.*?)```",
|
||||
actionable_block,
|
||||
re.S,
|
||||
)
|
||||
|
||||
return {
|
||||
"count": count or len(comments),
|
||||
"comments": comments,
|
||||
"all_comments_prompt": prompt_match.group(1).strip() if prompt_match else "",
|
||||
"raw": actionable_block.strip(),
|
||||
}
|
||||
|
||||
|
||||
def parse_comment_cards(comment_block: str) -> list[dict[str, str]]:
|
||||
comments: list[dict[str, str]] = []
|
||||
pattern = re.compile(
|
||||
r"<summary>"
|
||||
r"((?:[^<\n]+/)*[^<\n]+\.(?:cs|md|csproj|yaml|yml|json|txt|props|targets)|AGENTS\.md|CLAUDE\.md|README\.md|\.gitignore)"
|
||||
# CodeRabbit can fold nitpick cards for repo tooling files such as .js/.ts.
|
||||
# Keep the matcher broad enough for common source/config files while still
|
||||
# requiring a path-like summary header instead of arbitrary review text.
|
||||
r"((?:[^<\n]+/)*[^<\n]+\.(?:cs|md|csproj|yaml|yml|json|txt|props|targets|js|jsx|mjs|cjs|ts|tsx)|AGENTS\.md|CLAUDE\.md|README\.md|\.gitignore)"
|
||||
r" \((\d+)\)</summary><blockquote>\s*(.*?)\s*(?:(?:</blockquote></details>)|(?:</blockquote>))",
|
||||
re.S,
|
||||
)
|
||||
|
||||
for path, _, body in pattern.findall(primary_block):
|
||||
for path, _, body in pattern.findall(comment_block):
|
||||
finding_match = re.search(r"`([^`]+)`: \*\*(.*?)\*\*", body, re.S)
|
||||
prompt_match = re.search(r"<summary>🤖 Prompt for AI Agents</summary>\s*```(.*?)```", body, re.S)
|
||||
suggestion_match = re.search(r"<summary>✏️ 建议文案调整</summary>\s*```diff(.*?)```", body, re.S)
|
||||
@ -243,17 +263,49 @@ def parse_actionable_comments(actionable_block: str) -> dict[str, Any]:
|
||||
}
|
||||
)
|
||||
|
||||
prompt_match = re.search(
|
||||
r"<summary>🤖 Prompt for all review comments with AI agents</summary>\s*```(.*?)```",
|
||||
actionable_block,
|
||||
return comments
|
||||
|
||||
|
||||
def parse_review_comment_group(review_body: str, section_name: str) -> dict[str, Any]:
|
||||
section_match = re.search(
|
||||
rf"<summary>[^<]*{re.escape(section_name)} \((?P<count>\d+)\)</summary><blockquote>\s*",
|
||||
review_body,
|
||||
re.S,
|
||||
)
|
||||
if section_match is None:
|
||||
return {"count": 0, "comments": [], "raw": ""}
|
||||
|
||||
remaining_body = review_body[section_match.end() :]
|
||||
end_markers = [
|
||||
"\n</blockquote></details>\n\n<details>\n<summary>🤖 Prompt for all review comments with AI agents</summary>",
|
||||
"\n</blockquote></details>\n\n<details>\n<summary>🪄 Autofix (Beta)</summary>",
|
||||
"\n</blockquote></details>\n\n<details>\n<summary>ℹ️ Review info</summary>",
|
||||
"\n</blockquote></details>\n\n---",
|
||||
]
|
||||
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)
|
||||
comment_block = remaining_body[:end_index].strip()
|
||||
return {
|
||||
"count": count,
|
||||
"comments": comments,
|
||||
"count": int(section_match.group("count")),
|
||||
"comments": parse_comment_cards(comment_block),
|
||||
"raw": comment_block,
|
||||
}
|
||||
|
||||
|
||||
def parse_latest_review_body(review_body: str) -> dict[str, Any]:
|
||||
actionable_count_match = re.search(r"\*\*Actionable comments posted:\s*(\d+)\*\*", review_body)
|
||||
prompt_match = re.search(
|
||||
r"<summary>🤖 Prompt for all review comments with AI agents</summary>\s*```(.*?)```",
|
||||
review_body,
|
||||
re.S,
|
||||
)
|
||||
nitpick_group = parse_review_comment_group(review_body, "Nitpick comments")
|
||||
return {
|
||||
"actionable_count": int(actionable_count_match.group(1)) if actionable_count_match else 0,
|
||||
"nitpick_count": nitpick_group["count"],
|
||||
"nitpick_comments": nitpick_group["comments"],
|
||||
"all_comments_prompt": prompt_match.group(1).strip() if prompt_match else "",
|
||||
"raw": actionable_block.strip(),
|
||||
"raw": review_body.strip(),
|
||||
}
|
||||
|
||||
|
||||
@ -548,12 +600,30 @@ def build_result(pr_number: int, branch: str) -> dict[str, Any]:
|
||||
warnings.append("MegaLinter report block was not found in issue comments.")
|
||||
|
||||
latest_commit_review: dict[str, Any] = {}
|
||||
coderabbit_review: dict[str, Any] = {}
|
||||
try:
|
||||
latest_commit_review = fetch_latest_commit_review(pr_number)
|
||||
latest_review = latest_commit_review.get("latest_review", {})
|
||||
latest_review_body = str(latest_review.get("body") or "")
|
||||
if latest_review.get("user") == CODERABBIT_LOGIN and latest_review_body:
|
||||
coderabbit_review = parse_latest_review_body(latest_review_body)
|
||||
nitpick_count = int(coderabbit_review.get("nitpick_count") or 0)
|
||||
parsed_nitpick_count = len(coderabbit_review.get("nitpick_comments", []))
|
||||
if "Nitpick comments" in latest_review_body and not parsed_nitpick_count:
|
||||
warnings.append("CodeRabbit nitpick comments block could not be parsed from the latest review body.")
|
||||
elif nitpick_count and parsed_nitpick_count != nitpick_count:
|
||||
warnings.append(
|
||||
"CodeRabbit nitpick comments were only partially parsed from the latest review body: "
|
||||
f"declared={nitpick_count}, parsed={parsed_nitpick_count}."
|
||||
)
|
||||
except Exception as error: # noqa: BLE001
|
||||
warnings.append(f"Latest commit review comments could not be fetched: {error}")
|
||||
|
||||
if not actionable_block and not latest_commit_review.get("threads"):
|
||||
if (
|
||||
not actionable_block
|
||||
and not latest_commit_review.get("threads")
|
||||
and not coderabbit_review.get("nitpick_comments")
|
||||
):
|
||||
warnings.append("CodeRabbit actionable comments block was not found in issue comments.")
|
||||
|
||||
return {
|
||||
@ -571,6 +641,7 @@ def build_result(pr_number: int, branch: str) -> dict[str, Any]:
|
||||
"raw": summary_block,
|
||||
},
|
||||
"coderabbit_comments": parse_actionable_comments(actionable_block) if actionable_block else {},
|
||||
"coderabbit_review": coderabbit_review,
|
||||
"latest_commit_review": latest_commit_review,
|
||||
"megalinter_report": parse_megalinter_comment(megalinter_block) if megalinter_block else {},
|
||||
"test_reports": [parse_test_report(block) for block in test_blocks],
|
||||
@ -594,15 +665,31 @@ def format_text(result: dict[str, Any]) -> str:
|
||||
lines.append(f" Explanation: {check['explanation']}")
|
||||
lines.append(f" Resolution: {check['resolution']}")
|
||||
|
||||
comments = result.get("coderabbit_comments", {}).get("comments", [])
|
||||
coderabbit_comments = result.get("coderabbit_comments", {})
|
||||
review_feedback = result.get("coderabbit_review", {})
|
||||
comments = coderabbit_comments.get("comments", [])
|
||||
actionable_count = review_feedback.get("actionable_count") or coderabbit_comments.get("count") or len(comments)
|
||||
lines.append("")
|
||||
lines.append(f"CodeRabbit actionable comments: {len(comments)}")
|
||||
lines.append(f"CodeRabbit actionable comments: {actionable_count}")
|
||||
for comment in comments:
|
||||
lines.append(f"- {comment['path']} {comment['range']}".rstrip())
|
||||
if comment["title"]:
|
||||
lines.append(f" Title: {comment['title']}")
|
||||
if comment["description"]:
|
||||
lines.append(f" Description: {comment['description']}")
|
||||
if actionable_count and not comments:
|
||||
lines.append(" Details: see latest-commit review threads below.")
|
||||
|
||||
nitpick_comments = review_feedback.get("nitpick_comments", [])
|
||||
nitpick_count = review_feedback.get("nitpick_count") or len(nitpick_comments)
|
||||
lines.append("")
|
||||
lines.append(f"CodeRabbit nitpick comments: {nitpick_count} declared, {len(nitpick_comments)} parsed")
|
||||
for comment in nitpick_comments:
|
||||
lines.append(f"- {comment['path']} {comment['range']}".rstrip())
|
||||
if comment["title"]:
|
||||
lines.append(f" Title: {comment['title']}")
|
||||
if comment["description"]:
|
||||
lines.append(f" Description: {comment['description']}")
|
||||
|
||||
latest_commit_review = result.get("latest_commit_review", {})
|
||||
latest_commit = latest_commit_review.get("latest_commit", {})
|
||||
|
||||
@ -75,7 +75,11 @@ public sealed class YamlConfigLoaderIfThenElseTests
|
||||
{
|
||||
Directory.Delete(_rootPath, true);
|
||||
}
|
||||
catch (Exception)
|
||||
catch (IOException)
|
||||
{
|
||||
// Ignore cleanup failures in test teardown
|
||||
}
|
||||
catch (UnauthorizedAccessException)
|
||||
{
|
||||
// Ignore cleanup failures in test teardown
|
||||
}
|
||||
|
||||
@ -1397,6 +1397,114 @@ public class SchemaConfigGeneratorTests
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 验证缺少 <c>if</c> 时生成器也会拒绝孤立的 <c>else</c>。
|
||||
/// </summary>
|
||||
[Test]
|
||||
public void Run_Should_Report_Diagnostic_When_Else_Is_Declared_Without_If()
|
||||
{
|
||||
const string source = """
|
||||
namespace TestApp
|
||||
{
|
||||
public sealed class Dummy
|
||||
{
|
||||
}
|
||||
}
|
||||
""";
|
||||
|
||||
const string schema = """
|
||||
{
|
||||
"type": "object",
|
||||
"required": ["id", "reward"],
|
||||
"properties": {
|
||||
"id": { "type": "integer" },
|
||||
"reward": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"bonus": { "type": "integer" }
|
||||
},
|
||||
"else": {
|
||||
"type": "object",
|
||||
"required": ["bonus"],
|
||||
"properties": {
|
||||
"bonus": { "type": "integer" }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
""";
|
||||
|
||||
var result = SchemaGeneratorTestDriver.Run(
|
||||
source,
|
||||
("monster.schema.json", schema));
|
||||
|
||||
var diagnostic = result.Results.Single().Diagnostics.Single();
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(diagnostic.Id, Is.EqualTo("GF_ConfigSchema_013"));
|
||||
Assert.That(diagnostic.Severity, Is.EqualTo(DiagnosticSeverity.Error));
|
||||
Assert.That(diagnostic.GetMessage(), Does.Contain("reward"));
|
||||
Assert.That(diagnostic.GetMessage(), Does.Contain("must also declare 'if'"));
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 验证只声明 <c>if</c> 而没有分支时，生成器会给出对齐运行时的诊断。
|
||||
/// </summary>
|
||||
[Test]
|
||||
public void Run_Should_Report_Diagnostic_When_If_Is_Declared_Without_Then_Or_Else()
|
||||
{
|
||||
const string source = """
|
||||
namespace TestApp
|
||||
{
|
||||
public sealed class Dummy
|
||||
{
|
||||
}
|
||||
}
|
||||
""";
|
||||
|
||||
const string schema = """
|
||||
{
|
||||
"type": "object",
|
||||
"required": ["id", "reward"],
|
||||
"properties": {
|
||||
"id": { "type": "integer" },
|
||||
"reward": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"itemId": { "type": "string" }
|
||||
},
|
||||
"if": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"itemId": {
|
||||
"type": "string",
|
||||
"const": "potion"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
""";
|
||||
|
||||
var result = SchemaGeneratorTestDriver.Run(
|
||||
source,
|
||||
("monster.schema.json", schema));
|
||||
|
||||
var diagnostic = result.Results.Single().Diagnostics.Single();
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(diagnostic.Id, Is.EqualTo("GF_ConfigSchema_013"));
|
||||
Assert.That(diagnostic.Severity, Is.EqualTo(DiagnosticSeverity.Error));
|
||||
Assert.That(diagnostic.GetMessage(), Does.Contain("reward"));
|
||||
Assert.That(diagnostic.GetMessage(), Does.Contain("must also declare at least one of 'then' or 'else'"));
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 验证条件分支不是 object schema 时，诊断路径会定位到具体分支而不是父对象。
|
||||
/// </summary>
|
||||
@ -1579,6 +1687,71 @@ public class SchemaConfigGeneratorTests
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 验证 <c>else</c> 子 schema 内的非法 <c>format</c> 也会在生成阶段直接给出诊断。
|
||||
/// </summary>
|
||||
[Test]
|
||||
public void Run_Should_Report_Diagnostic_With_Runtime_Aligned_Path_When_Else_Inner_Schema_Is_Invalid()
|
||||
{
|
||||
const string source = """
|
||||
namespace TestApp
|
||||
{
|
||||
public sealed class Dummy
|
||||
{
|
||||
}
|
||||
}
|
||||
""";
|
||||
|
||||
const string schema = """
|
||||
{
|
||||
"type": "object",
|
||||
"required": ["id", "reward"],
|
||||
"properties": {
|
||||
"id": { "type": "integer" },
|
||||
"reward": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"itemId": { "type": "string" },
|
||||
"bonus": { "type": "integer" }
|
||||
},
|
||||
"if": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"itemId": {
|
||||
"type": "string",
|
||||
"const": "potion"
|
||||
}
|
||||
}
|
||||
},
|
||||
"else": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"bonus": {
|
||||
"type": "integer",
|
||||
"format": "uuid"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
""";
|
||||
|
||||
var result = SchemaGeneratorTestDriver.Run(
|
||||
source,
|
||||
("monster.schema.json", schema));
|
||||
|
||||
var diagnostic = result.Results.Single().Diagnostics.Single();
|
||||
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(diagnostic.Id, Is.EqualTo("GF_ConfigSchema_009"));
|
||||
Assert.That(diagnostic.Severity, Is.EqualTo(DiagnosticSeverity.Error));
|
||||
Assert.That(diagnostic.GetMessage(), Does.Contain("reward[else].bonus"));
|
||||
Assert.That(diagnostic.GetMessage(), Does.Contain("Only 'string' properties can declare 'format'."));
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 验证深层不支持的数组嵌套会带着完整字段路径产生命名明确的诊断。
|
||||
/// </summary>
|
||||
|
||||
@ -11,6 +11,7 @@
|
||||
- 当前阶段：`C# Runtime + Source Generator + Consumer DX`
|
||||
- 当前焦点：
|
||||
- 已完成 object-focused `if` / `then` / `else`，继续评估下一批仍不改变生成类型形状的共享关键字
|
||||
- 已完成 PR #262 的 CodeRabbit follow-up，补齐 latest review body 中 folded `Nitpick comments` 的 skill 解析并按建议收口 Tooling / Tests
|
||||
- 先以 Runtime / Generator / Tooling 三端一致语义为前提筛选下一项，而不是盲目扩全量 JSON Schema
|
||||
- 继续把 VS Code 工具能力视为非阻塞项，不让复杂 UI 编辑器需求反过来拖慢 C# 主线
|
||||
|
||||
@ -20,6 +21,8 @@
|
||||
- 缓解措施：延续 object-focused / focused matcher 约束，只接受三端都能稳定解释且不需要属性合并的子集
|
||||
- 工具链验证风险：VS Code 与 CI / 发布管道验证覆盖不足
|
||||
- 缓解措施：继续为新增共享关键字补齐三端测试覆盖，优先保证 C# Runtime 与 Generator 回归通过，并记录 JS 测试与构建验证
|
||||
- PR review 信号漂移风险：CodeRabbit 可能把建议折叠在 latest review body，而不是 issue comments
|
||||
- 缓解措施：`gframework-pr-review` 现已同时解析 latest review body，并输出 declared / parsed 数量以便快速识别解析缺口
|
||||
- 非阻塞项回退风险：将 VS Code 功能标为非阻塞但导致主线回退的风险
|
||||
- 缓解措施：C# 主线补齐新关键字时仍需在 `configValidation.js` 与 `extension.js` 中同步落地，只是不让复杂表单控件阻塞发布
|
||||
|
||||
@ -42,6 +45,13 @@
|
||||
- Generator：`GFramework.Game.SourceGenerators/Config/SchemaConfigGenerator.cs`
|
||||
- Tooling：`tools/gframework-config-tool/src/configValidation.js`、`tools/gframework-config-tool/src/extension.js`
|
||||
- Tests：`GFramework.Game.Tests/Config/YamlConfigLoaderIfThenElseTests.cs`、`GFramework.SourceGenerators.Tests/Config/SchemaConfigGeneratorTests.cs`、`tools/gframework-config-tool/test/configValidation.test.js`
|
||||
- PR review follow-up 收口：
|
||||
- `gframework-pr-review` 现已解析 latest CodeRabbit review body 中 folded `Nitpick comments`
|
||||
- text 输出会显示 `CodeRabbit nitpick comments: X declared, Y parsed`，避免再次静默遗漏
|
||||
- 已按 5 条 nitpick 更新 VS Code tool hints、shared validation helper，以及对称分支测试覆盖
|
||||
- 分支同步状态：
|
||||
- `feat/ai-first-config` 已 rebase 到 `origin/feat/ai-first-config`
|
||||
- 当前已解决“ahead / behind 同时存在”的分支差异，不再 behind 远端
|
||||
- 当前最细粒度的下一阶段 backlog 保留在独立文件：
|
||||
- `ai-plan/public/ai-first-config-system/todos/ai-first-config-system-csharp-experience-next.md`
|
||||
|
||||
@ -64,6 +74,7 @@
|
||||
- `2026-04-17` 之前的详细实现记录与定向验证命令已归档到历史 tracking / trace
|
||||
- active 跟踪文件只保留当前恢复点、当前状态和下一步，不再重复堆积已完成阶段的完整历史
|
||||
- `2026-04-20` 当前恢复点验证：
|
||||
- `python3 .codex/skills/gframework-pr-review/scripts/fetch_current_pr_review.py`：通过（`CodeRabbit nitpick comments: 5 declared, 5 parsed`）
|
||||
- `bun run test`（`tools/gframework-config-tool`）：通过
|
||||
- `dotnet test GFramework.SourceGenerators.Tests/GFramework.SourceGenerators.Tests.csproj -c Release --filter "FullyQualifiedName~SchemaConfigGeneratorTests"`：通过
|
||||
- `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~YamlConfigLoaderIfThenElseTests"`：通过
|
||||
|
||||
@ -60,6 +60,39 @@
|
||||
- 结果：通过
|
||||
- 备注：解决方案构建成功；输出包含仓库既有 analyzer warning，但无新增错误
|
||||
|
||||
### 阶段：PR #262 review follow-up 与分支同步
|
||||
|
||||
- 已使用 `gframework-pr-review` 复核 PR #262，并确认 latest CodeRabbit review body 的第一行下方存在 folded `🧹 Nitpick comments (5)`
|
||||
- 已修复 `fetch_current_pr_review.py` 的 follow-up 盲区：
|
||||
- 不再只依赖 issue comments，而会解析 latest review body 中的 folded nitpick cards
|
||||
- `parse_comment_cards` 现已覆盖 `.js/.ts` 等工具文件路径
|
||||
- text 输出会同时显示 declared / parsed 数量，避免 future drift 时静默少报
|
||||
- 已按 5 条 nitpick 收口代码：
|
||||
- VS Code tooling 的 `ifElse` hint 现会显示 `condition`
|
||||
- `extension.js` 已抽出可复用的 `InlineObjectSchemaHint` typedef
|
||||
- `configValidation.js` 已抽取共享 target reference 校验 helper
|
||||
- Source Generator tests 已补齐对称分支覆盖
|
||||
- Runtime test cleanup 已从 `catch (Exception)` 收窄到 IO / 权限异常
|
||||
- 已处理本地分支与远端分支差异：
|
||||
- 本地 `feat/ai-first-config` 已 rebase 到 `origin/feat/ai-first-config`
|
||||
- rebase 过程中 Git 跳过了远端已具备的 commit `76488dc`
|
||||
- 当前分支已不再 behind 远端，仅保留本地领先提交
|
||||
|
||||
### 验证
|
||||
|
||||
- 2026-04-20：`python3 .codex/skills/gframework-pr-review/scripts/fetch_current_pr_review.py`
|
||||
- 结果：通过
|
||||
- 备注：输出 `CodeRabbit nitpick comments: 5 declared, 5 parsed`
|
||||
- 2026-04-20：`bun run test`（`tools/gframework-config-tool`）
|
||||
- 结果：通过（118 tests）
|
||||
- 2026-04-20：`dotnet test GFramework.SourceGenerators.Tests/GFramework.SourceGenerators.Tests.csproj -c Release --filter "FullyQualifiedName~SchemaConfigGeneratorTests"`
|
||||
- 结果：通过（46 tests）
|
||||
- 2026-04-20：`dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~YamlConfigLoaderIfThenElseTests"`
|
||||
- 结果：通过（7 tests）
|
||||
- 2026-04-20：`dotnet build GFramework.sln -c Release`
|
||||
- 结果：通过
|
||||
- 备注：存在仓库既有 analyzer warning，但无新增错误
|
||||
|
||||
### 下一步
|
||||
|
||||
1. 评估 `oneOf` / `anyOf` 是否值得继续沿用 object-focused 子集；若仍会造成生成形状漂移，就直接跳过
|
||||
|
||||
@ -1504,6 +1504,32 @@ function parseConditionalObjectSchema(rawSchema, displayPath, keywordName, prope
|
||||
* @param {Record<string, SchemaNode>} properties Declared parent properties.
|
||||
*/
|
||||
function validateConditionalSchemaTargets(rawSchema, displayPath, keywordName, properties) {
|
||||
validateDeclaredTargetReferences(rawSchema, displayPath, `'${keywordName}'`, properties);
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensure one object-focused `allOf` entry only constrains properties that the
|
||||
* parent object schema already declared.
|
||||
*
|
||||
* @param {unknown} rawAllOfSchema Raw allOf entry.
|
||||
* @param {string} displayPath Parent schema path.
|
||||
* @param {number} index Zero-based allOf entry index.
|
||||
* @param {Record<string, SchemaNode>} properties Declared parent properties.
|
||||
*/
|
||||
function validateAllOfEntryTargets(rawAllOfSchema, displayPath, index, properties) {
|
||||
validateDeclaredTargetReferences(rawAllOfSchema, displayPath, `'allOf' entry #${index + 1}`, properties);
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensure one focused object schema only references properties that the parent
|
||||
* object schema already declared.
|
||||
*
|
||||
* @param {unknown} rawSchema Raw object-focused schema.
|
||||
* @param {string} displayPath Parent schema path.
|
||||
* @param {string} contextLabel Human-readable constraint origin label.
|
||||
* @param {Record<string, SchemaNode>} properties Declared parent properties.
|
||||
*/
|
||||
function validateDeclaredTargetReferences(rawSchema, displayPath, contextLabel, properties) {
|
||||
if (!rawSchema || typeof rawSchema !== "object" || Array.isArray(rawSchema)) {
|
||||
return;
|
||||
}
|
||||
@ -1517,7 +1543,7 @@ function validateConditionalSchemaTargets(rawSchema, displayPath, keywordName, p
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
`Schema property '${displayPath}' declares property '${propertyName}' in '${keywordName}', ` +
|
||||
`Schema property '${displayPath}' declares property '${propertyName}' in ${contextLabel}, ` +
|
||||
"but that property is not declared in the parent object schema.");
|
||||
}
|
||||
}
|
||||
@ -1536,54 +1562,7 @@ function validateConditionalSchemaTargets(rawSchema, displayPath, keywordName, p
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
`Schema property '${displayPath}' requires property '${requiredProperty}' in '${keywordName}', ` +
|
||||
"but that property is not declared in the parent object schema.");
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensure one object-focused `allOf` entry only constrains properties that the
|
||||
* parent object schema already declared.
|
||||
*
|
||||
* @param {unknown} rawAllOfSchema Raw allOf entry.
|
||||
* @param {string} displayPath Parent schema path.
|
||||
* @param {number} index Zero-based allOf entry index.
|
||||
* @param {Record<string, SchemaNode>} properties Declared parent properties.
|
||||
*/
|
||||
function validateAllOfEntryTargets(rawAllOfSchema, displayPath, index, properties) {
|
||||
if (!rawAllOfSchema || typeof rawAllOfSchema !== "object" || Array.isArray(rawAllOfSchema)) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (rawAllOfSchema.properties &&
|
||||
typeof rawAllOfSchema.properties === "object" &&
|
||||
!Array.isArray(rawAllOfSchema.properties)) {
|
||||
for (const propertyName of Object.keys(rawAllOfSchema.properties)) {
|
||||
if (Object.prototype.hasOwnProperty.call(properties, propertyName)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
`Schema property '${displayPath}' declares property '${propertyName}' in 'allOf' entry #${index + 1}, ` +
|
||||
"but that property is not declared in the parent object schema.");
|
||||
}
|
||||
}
|
||||
|
||||
if (!Array.isArray(rawAllOfSchema.required)) {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const requiredProperty of rawAllOfSchema.required) {
|
||||
if (typeof requiredProperty !== "string" || requiredProperty.trim().length === 0) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (Object.prototype.hasOwnProperty.call(properties, requiredProperty)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
`Schema property '${displayPath}' requires property '${requiredProperty}' in 'allOf' entry #${index + 1}, ` +
|
||||
`Schema property '${displayPath}' requires property '${requiredProperty}' in ${contextLabel}, ` +
|
||||
"but that property is not declared in the parent object schema.");
|
||||
}
|
||||
}
|
||||
|
||||
@ -1576,9 +1576,75 @@ function getScalarArrayValue(yamlNode) {
|
||||
}
|
||||
|
||||
/**
|
||||
* @typedef {object} InlineObjectSchemaHint
|
||||
* @property {string=} type Inline schema type.
|
||||
* @property {string[]=} required Required properties.
|
||||
* @property {string[]=} enumValues Allowed enum values.
|
||||
* @property {string=} constValue Raw const value.
|
||||
* @property {string=} constDisplayValue Human-readable const value.
|
||||
* @property {string=} pattern String pattern metadata.
|
||||
* @property {string=} refTable Referenced table name.
|
||||
*
|
||||
* @typedef {object} ContainsSchemaHint
|
||||
* @property {string=} type Inline schema type.
|
||||
* @property {string[]=} enumValues Allowed enum values.
|
||||
* @property {string=} constValue Raw const value.
|
||||
* @property {string=} constDisplayValue Human-readable const value.
|
||||
* @property {string=} pattern String pattern metadata.
|
||||
* @property {string=} format String format metadata.
|
||||
* @property {string=} refTable Referenced table name.
|
||||
*
|
||||
* @typedef {object} ScalarArrayItemHint
|
||||
* @property {string[]=} enumValues Allowed enum values.
|
||||
* @property {string=} constValue Raw const value.
|
||||
* @property {string=} constDisplayValue Human-readable const value.
|
||||
* @property {number=} minimum Inclusive minimum.
|
||||
* @property {number=} exclusiveMinimum Exclusive minimum.
|
||||
* @property {number=} maximum Inclusive maximum.
|
||||
* @property {number=} exclusiveMaximum Exclusive maximum.
|
||||
* @property {number=} multipleOf Numeric multiple constraint.
|
||||
* @property {number=} minLength Minimum length.
|
||||
* @property {number=} maxLength Maximum length.
|
||||
* @property {string=} pattern String pattern metadata.
|
||||
* @property {string=} format String format metadata.
|
||||
*
|
||||
* @typedef {object} PropertySchemaHint
|
||||
* @property {string=} type Schema type.
|
||||
* @property {string=} description Human-facing description.
|
||||
* @property {string=} defaultValue Default value text.
|
||||
* @property {string=} constValue Raw const value.
|
||||
* @property {string=} constDisplayValue Human-readable const value.
|
||||
* @property {number=} minimum Inclusive minimum.
|
||||
* @property {number=} exclusiveMinimum Exclusive minimum.
|
||||
* @property {number=} maximum Inclusive maximum.
|
||||
* @property {number=} exclusiveMaximum Exclusive maximum.
|
||||
* @property {number=} multipleOf Numeric multiple constraint.
|
||||
* @property {number=} minLength Minimum length.
|
||||
* @property {number=} maxLength Maximum length.
|
||||
* @property {string=} pattern String pattern metadata.
|
||||
* @property {string=} format String format metadata.
|
||||
* @property {number=} minItems Minimum array item count.
|
||||
* @property {number=} maxItems Maximum array item count.
|
||||
* @property {number=} minContains Minimum contains matches.
|
||||
* @property {number=} maxContains Maximum contains matches.
|
||||
* @property {number=} minProperties Minimum property count.
|
||||
* @property {number=} maxProperties Maximum property count.
|
||||
* @property {string[]=} required Required properties.
|
||||
* @property {Record<string, string[]>=} dependentRequired dependentRequired metadata.
|
||||
* @property {Record<string, InlineObjectSchemaHint>=} dependentSchemas dependentSchemas metadata.
|
||||
* @property {Array<InlineObjectSchemaHint>=} allOf allOf metadata.
|
||||
* @property {InlineObjectSchemaHint=} ifSchema if metadata.
|
||||
* @property {InlineObjectSchemaHint=} thenSchema then metadata.
|
||||
* @property {InlineObjectSchemaHint=} elseSchema else metadata.
|
||||
* @property {boolean=} uniqueItems uniqueItems metadata.
|
||||
* @property {string[]=} enumValues Allowed enum values.
|
||||
* @property {ContainsSchemaHint=} contains contains metadata.
|
||||
* @property {ScalarArrayItemHint=} items Array item metadata.
|
||||
* @property {string=} refTable Referenced table name.
|
||||
*
|
||||
* Render one compact inline-schema summary for form hints.
|
||||
*
|
||||
* @param {{type?: string, required?: string[], enumValues?: string[], constValue?: string, constDisplayValue?: string, pattern?: string, refTable?: string}} schema Parsed inline schema metadata.
|
||||
* @param {InlineObjectSchemaHint} schema Parsed inline schema metadata.
|
||||
* @param {boolean} includeRequiredProperties Whether object `required` members should be surfaced.
|
||||
* @returns {string} Localized summary.
|
||||
*/
|
||||
@ -1622,7 +1688,7 @@ function describeInlineSchemaForHint(schema, includeRequiredProperties = false)
|
||||
/**
|
||||
* Render human-facing metadata hints for one schema field.
|
||||
*
|
||||
* @param {{type?: string, description?: string, defaultValue?: string, constValue?: string, constDisplayValue?: string, minimum?: number, exclusiveMinimum?: number, maximum?: number, exclusiveMaximum?: number, multipleOf?: number, minLength?: number, maxLength?: number, pattern?: string, format?: string, minItems?: number, maxItems?: number, minContains?: number, maxContains?: number, minProperties?: number, maxProperties?: number, required?: string[], dependentRequired?: Record<string, string[]>, dependentSchemas?: Record<string, {type?: string, required?: string[], enumValues?: string[], constValue?: string, constDisplayValue?: string, pattern?: string, refTable?: string}>, allOf?: Array<{type?: string, required?: string[], enumValues?: string[], constValue?: string, constDisplayValue?: string, pattern?: string, refTable?: string}>, ifSchema?: {type?: string, required?: string[], enumValues?: string[], constValue?: string, constDisplayValue?: string, pattern?: string, refTable?: string}, thenSchema?: {type?: string, required?: string[], enumValues?: string[], constValue?: string, constDisplayValue?: string, pattern?: string, refTable?: string}, elseSchema?: {type?: string, required?: string[], enumValues?: string[], constValue?: string, constDisplayValue?: string, pattern?: string, refTable?: string}, uniqueItems?: boolean, enumValues?: string[], contains?: {type?: string, enumValues?: string[], constValue?: string, constDisplayValue?: string, pattern?: string, format?: string, refTable?: string}, items?: {enumValues?: string[], constValue?: string, constDisplayValue?: string, minimum?: number, exclusiveMinimum?: number, maximum?: number, exclusiveMaximum?: number, multipleOf?: number, minLength?: number, maxLength?: number, pattern?: string, format?: string}, refTable?: string}} propertySchema Property schema metadata.
|
||||
* @param {PropertySchemaHint} propertySchema Property schema metadata.
|
||||
* @param {boolean} isArrayField Whether the field is an array.
|
||||
* @param {boolean} includeDescription Whether description text should be included in the hint output.
|
||||
* @returns {string} HTML fragment.
|
||||
@ -1745,6 +1811,7 @@ function renderFieldHint(propertySchema, isArrayField, includeDescription = true
|
||||
propertySchema.ifSchema &&
|
||||
propertySchema.elseSchema) {
|
||||
hints.push(escapeHtml(localizer.t("webview.hint.ifElse", {
|
||||
condition: describeInlineSchemaForHint(propertySchema.ifSchema, true),
|
||||
schema: describeInlineSchemaForHint(propertySchema.elseSchema, true)
|
||||
})));
|
||||
}
|
||||
|
||||
@ -138,7 +138,7 @@ const enMessages = {
|
||||
"webview.hint.dependentSchemas": "When {trigger} is set: satisfy {schema}",
|
||||
"webview.hint.allOf": "Also satisfy: {schema}",
|
||||
"webview.hint.ifThen": "When {condition}: satisfy {schema}",
|
||||
"webview.hint.ifElse": "Otherwise: satisfy {schema}",
|
||||
"webview.hint.ifElse": "Otherwise (when {condition} does not match): satisfy {schema}",
|
||||
"webview.hint.refTable": "Ref table: {refTable}",
|
||||
"webview.unsupported.array": "Unsupported array shapes are currently raw-YAML-only in the form preview.",
|
||||
"webview.unsupported.type": "{type} fields are currently raw-YAML-only.",
|
||||
@ -268,7 +268,7 @@ const zhCnMessages = {
|
||||
"webview.hint.dependentSchemas": "当 {trigger} 出现时：还必须满足 {schema}",
|
||||
"webview.hint.allOf": "还必须满足：{schema}",
|
||||
"webview.hint.ifThen": "当满足 {condition} 时：还必须满足 {schema}",
|
||||
"webview.hint.ifElse": "否则：还必须满足 {schema}",
|
||||
"webview.hint.ifElse": "否则（当 {condition} 不匹配时）：还必须满足 {schema}",
|
||||
"webview.hint.refTable": "引用表：{refTable}",
|
||||
"webview.unsupported.array": "当前表单预览暂不支持这种数组结构，请改用原始 YAML。",
|
||||
"webview.unsupported.type": "当前表单预览暂不支持 {type} 字段，请改用原始 YAML。",
|
||||
|
||||
@ -174,3 +174,21 @@ test("createLocalizer should expose allOf validation keys", () => {
|
||||
}),
|
||||
"对象“reward”必须满足全部 `allOf` schema，第 1 项未匹配。");
|
||||
});
|
||||
|
||||
test("createLocalizer should expose ifElse hints with the condition context", () => {
|
||||
const englishLocalizer = createLocalizer("en");
|
||||
const chineseLocalizer = createLocalizer("zh-cn");
|
||||
|
||||
assert.equal(
|
||||
englishLocalizer.t("webview.hint.ifElse", {
|
||||
condition: "object, Required: itemId",
|
||||
schema: "object, Required: bonus"
|
||||
}),
|
||||
"Otherwise (when object, Required: itemId does not match): satisfy object, Required: bonus");
|
||||
assert.equal(
|
||||
chineseLocalizer.t("webview.hint.ifElse", {
|
||||
condition: "object，必填字段：itemId",
|
||||
schema: "object，必填字段：bonus"
|
||||
}),
|
||||
"否则（当 object，必填字段：itemId 不匹配时）：还必须满足 object，必填字段：bonus");
|
||||
});
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user