From 779c44767037d37053c4f782abf96cd7f77b323b Mon Sep 17 00:00:00 2001 From: jilinchen Date: Wed, 7 Jan 2026 09:59:18 +0800 Subject: [PATCH] fix: address code review issues from Claude and GPT-5.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - report/SKILL.md: add AskUserQuestion to allowed-tools - validate_json.py: remove unused imports (List, Any) - validate_json.py: sort output lists for deterministic order - validate_json.py: limit recursion to category level only - validate_json.py: add list-of-dict handling - EN validate_json.py: remove Chinese from CATEGORY_MAPPING - EN deep/SKILL.md: change output language from Chinese to English - deep/SKILL.md: add slug handling for filenames - report/SKILL.md: fix uncertain_fields -> uncertain naming - README: add PyYAML dependency note 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- README.md | 3 ++ README.zh.md | 3 ++ skills/research-en/deep/SKILL.md | 6 +-- skills/research-en/report/SKILL.md | 4 +- skills/research-en/validate_json.py | 72 ++++++++++++++++++----------- skills/research-zh/deep/SKILL.md | 2 +- skills/research-zh/report/SKILL.md | 4 +- skills/research-zh/validate_json.py | 54 ++++++++++++++-------- 8 files changed, 93 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index ec8652a..22b71a4 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,9 @@ cp -r skills/research-zh ~/.claude/skills/research # Required: Install agent cp agents/web-search-agent.md ~/.claude/agents/ + +# Required: Install Python dependency +pip install pyyaml ``` ## Commands diff --git a/README.zh.md b/README.zh.md index 9eded9f..64c3043 100644 --- a/README.zh.md +++ b/README.zh.md @@ -24,6 +24,9 @@ cp -r skills/research-en ~/.claude/skills/research # 必需：安装agent cp agents/web-search-agent.md ~/.claude/agents/ + +# 必需：安装Python依赖 +pip install pyyaml ``` ## 命令 diff --git a/skills/research-en/deep/SKILL.md b/skills/research-en/deep/SKILL.md index 287a3ae..6ac8b7b 100644 --- a/skills/research-en/deep/SKILL.md +++ b/skills/research-en/deep/SKILL.md @@ -28,7 +28,7 @@ Find `*/outline.yaml` file in current working directory, read items list, execut - `{item_related_info}`: item's complete yaml content (name + category + description etc.) - `{output_dir}`: execution.output_dir from outline.yaml (default: ./results) - `{fields_path}`: absolute path to {topic}/fields.yaml -- `{output_path}`: absolute path to {output_dir}/{item_name}.json +- `{output_path}`: absolute path to {output_dir}/{item_name_slug}.json (slugify item_name: replace spaces with _, remove special chars) **Hard Constraint**: The following prompt must be strictly reproduced, only replacing variables in {xxx}, do not modify structure or wording. @@ -44,7 +44,7 @@ Read {fields_path} to get all field definitions 1. Output JSON according to fields defined in fields.yaml 2. Mark uncertain field values with [uncertain] 3. Add uncertain array at the end of JSON, listing all uncertain field names -4. All field values must be in Chinese (research can be in English, but final JSON values in Chinese) +4. All field values must be in English ## Output Path {output_path} @@ -70,7 +70,7 @@ Read /home/weizhena/AIcoding/aicoding-history/fields.yaml to get all field defin 1. Output JSON according to fields defined in fields.yaml 2. Mark uncertain field values with [uncertain] 3. Add uncertain array at the end of JSON, listing all uncertain field names -4. All field values must be in Chinese (research can be in English, but final JSON values in Chinese) +4. All field values must be in English ## Output Path /home/weizhena/AIcoding/aicoding-history/results/GitHub_Copilot.json diff --git a/skills/research-en/report/SKILL.md b/skills/research-en/report/SKILL.md index 6b63933..dd0d081 100644 --- a/skills/research-en/report/SKILL.md +++ b/skills/research-en/report/SKILL.md @@ -1,6 +1,6 @@ --- description: Summarize deep research results into markdown report, cover all fields, skip uncertain values. -allowed-tools: Read, Write, Glob, Bash +allowed-tools: Read, Write, Glob, Bash, AskUserQuestion --- # Research Report - Summary Report @@ -75,7 +75,7 @@ CATEGORY_MAPPING = { Collect fields that exist in JSON but not defined in fields.yaml, put in "Other Info" category. Note to filter: - Internal fields: `_source_file`, `uncertain` - Nested structure top-level keys: `basic_info`, `technical_features` etc. -- `uncertain_fields` list: Display each field name on separate line, don't compress into one line +- `uncertain` array: Display each field name on separate line, don't compress into one line **5. Uncertain Value Skipping** Skip conditions: diff --git a/skills/research-en/validate_json.py b/skills/research-en/validate_json.py index c2e4ed0..37d69e2 100644 --- a/skills/research-en/validate_json.py +++ b/skills/research-en/validate_json.py @@ -9,18 +9,18 @@ import json import yaml import sys from pathlib import Path -from typing import Dict, List, Set, Any, Tuple +from typing import Dict, Set, Tuple -# Category mapping (supports both Chinese and English keys) +# Category mapping (English keys only) CATEGORY_MAPPING = { - "basic_info": ["basic_info", "基本信息"], - "technical_features": ["technical_features", "technical_characteristics", "技术特性"], - "performance_metrics": ["performance_metrics", "performance", "性能指标"], - "milestone_significance": ["milestone_significance", "milestones", "里程碑意义"], - "business_info": ["business_info", "commercial_info", "商业信息"], - "competition_ecosystem": ["competition_ecosystem", "competition", "竞争与生态"], - "history": ["history", "历史沿革"], - "market_positioning": ["market_positioning", "market", "市场定位"], + "basic_info": ["basic_info", "Basic Info"], + "technical_features": ["technical_features", "technical_characteristics", "Technical Features"], + "performance_metrics": ["performance_metrics", "performance", "Performance Metrics"], + "milestone_significance": ["milestone_significance", "milestones", "Milestone Significance"], + "business_info": ["business_info", "commercial_info", "Business Info"], + "competition_ecosystem": ["competition_ecosystem", "competition", "Competition Ecosystem"], + "history": ["history", "History"], + "market_positioning": ["market_positioning", "market", "Market Positioning"], } @@ -53,30 +53,41 @@ def load_fields_yaml(fields_path: Path) -> Tuple[Set[str], Set[str], Dict[str, s def extract_json_fields(data: Dict, category_mapping: Dict = None) -> Set[str]: """ Extract all field names from JSON (supports both flat and nested structures) + Only extracts field names at category level, not nested dict/list values """ if category_mapping is None: category_mapping = CATEGORY_MAPPING - # Get all possible nested keys + # Get all possible nested keys (category containers) nested_keys = set() for keys in category_mapping.values(): nested_keys.update(keys) fields = set() - def collect_fields(d: Dict, is_top_level: bool = True): - for k, v in d.items(): - # Skip internal fields - if k in {"_source_file", "uncertain"}: - continue - # If it's a top-level nested key, recurse into it - if is_top_level and k in nested_keys: - if isinstance(v, dict): - collect_fields(v, is_top_level=False) - else: - fields.add(k) - if isinstance(v, dict): - collect_fields(v, is_top_level=False) + def collect_fields(d, is_category_level: bool = True): + """ + Collect fields from dict or list structures + is_category_level: True if we're at top level or inside a category container + """ + if isinstance(d, dict): + for k, v in d.items(): + # Skip internal fields + if k in {"_source_file", "uncertain"}: + continue + # If it's a category container key, recurse into it + if is_category_level and k in nested_keys: + if isinstance(v, dict): + collect_fields(v, is_category_level=True) + else: + # This is a field name, add it + fields.add(k) + # Don't recurse into field values (avoid counting nested keys as fields) + elif isinstance(d, list): + # Handle list-of-dict structures at category level + for item in d: + if isinstance(item, dict): + collect_fields(item, is_category_level=is_category_level) collect_fields(data) return fields @@ -110,6 +121,10 @@ def validate_json(json_path: Path, all_fields: Set[str], required_fields: Set[st missing_by_category[cat] = [] missing_by_category[cat].append(field) + # Sort lists within categories for deterministic output + for cat in missing_by_category: + missing_by_category[cat].sort() + return { "file": json_path.name, "total_defined": len(all_fields), @@ -117,10 +132,10 @@ def validate_json(json_path: Path, all_fields: Set[str], required_fields: Set[st "missing": len(missing), "extra": len(extra), "coverage_rate": len(covered) / len(all_fields) * 100 if all_fields else 100, - "missing_required": list(missing_required), - "missing_optional": list(missing_optional), + "missing_required": sorted(missing_required), + "missing_optional": sorted(missing_optional), "missing_by_category": missing_by_category, - "extra_fields": list(extra), + "extra_fields": sorted(extra), "valid": len(missing_required) == 0, # Valid if all required fields are covered } @@ -140,7 +155,8 @@ def print_result(result: Dict, verbose: bool = True): if verbose and result["missing_optional"]: print(f"\n[WARN] Missing optional fields ({len(result['missing_optional'])}):") - for cat, fields in result["missing_by_category"].items(): + for cat in sorted(result["missing_by_category"].keys()): + fields = result["missing_by_category"][cat] optional_fields = [f for f in fields if f not in result["missing_required"]] if optional_fields: print(f" [{cat}]: {', '.join(optional_fields)}") diff --git a/skills/research-zh/deep/SKILL.md b/skills/research-zh/deep/SKILL.md index 56035fe..77616e3 100644 --- a/skills/research-zh/deep/SKILL.md +++ b/skills/research-zh/deep/SKILL.md @@ -28,7 +28,7 @@ allowed-tools: Bash, Read, Write, Glob, WebSearch, Task - `{item_related_info}`: item的完整yaml内容（name + category + description等） - `{output_dir}`: outline.yaml中execution.output_dir（默认./results） - `{fields_path}`: {topic}/fields.yaml的绝对路径 -- `{output_path}`: {output_dir}/{item_name}.json的绝对路径 +- `{output_path}`: {output_dir}/{item_name_slug}.json的绝对路径（slugify处理item_name：空格替换为_，移除特殊字符） **硬约束**：以下prompt必须严格复述，仅替换{xxx}中的变量，禁止改写结构或措辞。 diff --git a/skills/research-zh/report/SKILL.md b/skills/research-zh/report/SKILL.md index a614e27..5365a9f 100644 --- a/skills/research-zh/report/SKILL.md +++ b/skills/research-zh/report/SKILL.md @@ -1,6 +1,6 @@ --- description: 将deep调研结果汇总为markdown报告，覆盖所有字段，跳过不确定值。 -allowed-tools: Read, Write, Glob, Bash +allowed-tools: Read, Write, Glob, Bash, AskUserQuestion --- # Research Report - 汇总报告 @@ -75,7 +75,7 @@ CATEGORY_MAPPING = { 收集JSON中有但fields.yaml中没定义的字段，放入"其他信息"分类。注意过滤： - 内部字段：`_source_file`, `uncertain` - 嵌套结构顶级key：`basic_info`, `technical_features`等 -- `uncertain_fields`列表：需要逐行显示每个字段名，不要压缩成一行 +- `uncertain`数组：需要逐行显示每个字段名，不要压缩成一行 **5. 不确定值跳过** 跳过条件： diff --git a/skills/research-zh/validate_json.py b/skills/research-zh/validate_json.py index 2fe69c0..7354a2e 100644 --- a/skills/research-zh/validate_json.py +++ b/skills/research-zh/validate_json.py @@ -9,7 +9,7 @@ import json import yaml import sys from pathlib import Path -from typing import Dict, List, Set, Any, Tuple +from typing import Dict, Set, Tuple # Category中英文映射 CATEGORY_MAPPING = { @@ -53,30 +53,41 @@ def load_fields_yaml(fields_path: Path) -> Tuple[Set[str], Set[str], Dict[str, s def extract_json_fields(data: Dict, category_mapping: Dict = None) -> Set[str]: """ 从JSON中提取所有字段名（支持扁平和嵌套结构） + 只提取category级别的字段名，不递归到字段值的dict/list中 """ if category_mapping is None: category_mapping = CATEGORY_MAPPING - # 获取所有可能的嵌套key + # 获取所有可能的嵌套key（category容器） nested_keys = set() for keys in category_mapping.values(): nested_keys.update(keys) fields = set() - def collect_fields(d: Dict, is_top_level: bool = True): - for k, v in d.items(): - # 跳过内部字段 - if k in {"_source_file", "uncertain"}: - continue - # 如果是嵌套结构的顶级key，递归进入 - if is_top_level and k in nested_keys: - if isinstance(v, dict): - collect_fields(v, is_top_level=False) - else: - fields.add(k) - if isinstance(v, dict): - collect_fields(v, is_top_level=False) + def collect_fields(d, is_category_level: bool = True): + """ + 从dict或list结构中收集字段 + is_category_level: True表示在顶层或category容器内部 + """ + if isinstance(d, dict): + for k, v in d.items(): + # 跳过内部字段 + if k in {"_source_file", "uncertain"}: + continue + # 如果是category容器key，递归进入 + if is_category_level and k in nested_keys: + if isinstance(v, dict): + collect_fields(v, is_category_level=True) + else: + # 这是一个字段名，添加它 + fields.add(k) + # 不递归到字段值中（避免将嵌套key误计为字段） + elif isinstance(d, list): + # 处理category级别的list-of-dict结构 + for item in d: + if isinstance(item, dict): + collect_fields(item, is_category_level=is_category_level) collect_fields(data) return fields @@ -110,6 +121,10 @@ def validate_json(json_path: Path, all_fields: Set[str], required_fields: Set[st missing_by_category[cat] = [] missing_by_category[cat].append(field) + # 对category内的列表排序，确保输出确定性 + for cat in missing_by_category: + missing_by_category[cat].sort() + return { "file": json_path.name, "total_defined": len(all_fields), @@ -117,10 +132,10 @@ def validate_json(json_path: Path, all_fields: Set[str], required_fields: Set[st "missing": len(missing), "extra": len(extra), "coverage_rate": len(covered) / len(all_fields) * 100 if all_fields else 100, - "missing_required": list(missing_required), - "missing_optional": list(missing_optional), + "missing_required": sorted(missing_required), + "missing_optional": sorted(missing_optional), "missing_by_category": missing_by_category, - "extra_fields": list(extra), + "extra_fields": sorted(extra), "valid": len(missing_required) == 0, # required字段全覆盖则valid } @@ -140,7 +155,8 @@ def print_result(result: Dict, verbose: bool = True): if verbose and result["missing_optional"]: print(f"\n[WARN] 缺失可选字段 ({len(result['missing_optional'])}):") - for cat, fields in result["missing_by_category"].items(): + for cat in sorted(result["missing_by_category"].keys()): + fields = result["missing_by_category"][cat] optional_fields = [f for f in fields if f not in result["missing_required"]] if optional_fields: print(f" [{cat}]: {', '.join(optional_fields)}")