Skip to content

docs(skills): add development review checks - #323

Open
SigureMo wants to merge 8 commits into
redai-studio:mainfrom
cattidea:codex/review-agent-principles
Open

SigureMo wants to merge 8 commits into
redai-studio:mainfrom
cattidea:codex/review-agent-principles

Conversation

@SigureMo

@SigureMo SigureMo commented Sep 17, 2026 •

Copy link
Copy Markdown
Member

What

Add an independent development-principles and necessity check alongside the existing SOLID review. references/solid-checklist.md retains its original content; references/development-checklist.md contains the additional check, and SKILL.md loads them in separate steps.

Why

Reviews need to establish why new behavior exists, where its requirements come from, and what maintenance obligations it introduces. An additional check makes that responsibility explicit while preserving the existing SOLID checklist. Contributor analysis and verification follow #321.

How

  • Preserve the original SOLID principles, code-smell prompts, and refactoring heuristics verbatim.
  • Add a separate check for new defaults, normalization, compatibility promises, and abstractions: inspect real producers and consumers, existing project capabilities, and the consequences of removing or simplifying the addition. Include development principles with applicability boundaries and three short contrasting examples.
  • Review overdesign through established project patterns, unnecessary configuration and compatibility promises, the effort needed to understand the full call path, and speculative extension points. Findings should compare current benefit with added maintenance cost and identify a smaller alternative satisfying the same acceptance criteria.
  • Distinguish confirmed requirements from contributor expansions and bot suggestions. Require evidence for findings and separate impact severity from acceptance decisions, including blocking P2 findings when justified.
  • Check author verification, require self-review of recurring causes, and avoid repeated optional polish. Keep review priority and acceptance decisions with maintainers.
  • Calibrate Python, security, removal, and ML guidance against reachable failures and established contracts.

Testing

  • Skill metadata validation with skill-creator/scripts/quick_validate.py: passed.
  • uvx --python 3.12 pre-commit run --all-files --show-diff-on-failure: passed.
  • git diff --cached --check: passed.
  • Verified that the restored SOLID file matches the pre-PR version byte for byte, in both the working tree and index, and that all referenced checklists exist.
  • Live bot behavior has not been replay-tested. Runtime and multi-node GPU integration tests were not run because only Markdown review instructions changed.

Type of Change

  • Documentation update
This PR is co-authored with @codex (gpt-6 astra xhigh)

# 📝 Documentation

- Apply development principles through concrete evidence and explicit boundaries.
- Add contributor verification and follow-up review guidance from issue redai-studio#321.
- Calibrate severity and remove mechanical refactoring, fallback, and ML checklist advice.
- Preserve the existing code-review skill as the shared review entrypoint.

Co-authored-by: Codex <noreply@openai.com>
@rai-studio-bot

rai-studio-bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Nyanpasu 审查看板

审查状态: ✅ 已通过

审查版本: ceef39b

新 head ceef39b 为合并 main 的 merge commit:PR 相对 main 的 diff 与上一轮已通过的 e5d7e3b 逐字节一致(完整 diff sha256 相同),main 侧新提交未涉及 skills/。已复核 SKILL.md 步骤接线与引用表、四个 checklist 的 Contents 锚点、removal-plan.md 引用的严重度定义,solid-checklist.md 保持与 main 一致。当前 head 的 Pre-commit 与 Python 单测 CI 通过,两个 GPU 集成测试仍在运行(仅覆盖 main 侧代码,与本次 Markdown 变更无关)。未发现需要修改的问题。

审查阶段进度范围与结果
常规审查 ✅ 已完成 已核对新 head 与上一轮已通过版本的 diff 逐字节一致、main 合并内容未触及 skills/、SKILL.md 接线(步骤 2 加载 development-checklist.md、步骤 1-10 连续、引用表完整)、checklist 锚点与跨文件引用,以及 CI 状态;无新增或变化的问题。
深度审查 ✅ 已完成 纯文档变更规模小且内聚,由父任务直接完成必要性审计(新增 development checklist 与四个既有 checklist 的校准更新均有明确需求来源,保留决策及证据见简化审计);未派发独立设计或测试子任务——本 head 变更仅为合并 main,内容与已验证版本一致,无新增抽象或行为。

没有未解决的审查问题。

提交范围 · 接收 6 · 建议移出 0 · 待确认 0

接收 6 个文件 · 建议移出 0 个文件 · 待确认 0 个文件。移出与待确认部分暂停深审,不代表审查通过。

文件结论仓库维护必要性依据替代去向或方案
skills/code-review/SKILL.md
skills/code-review/references/development-checklist.md
接收 skills/code-review 是仓库维护的 code-review 技能(AGENTS.md 技能表登记为 git 变更审查入口)。SKILL.md 是技能入口并在步骤 3 显式加载新的 development-checklist.md(SKILL.md:56、引用表 :136),该 checklist 是本 PR 的核心交付:新增必要性与开发原则检查。文件内容与上一轮已通过版本逐字节一致,本 head 的改动仅为合并 main。 PR #323 描述(What/Why:沿 #321 增加独立的开发原则与必要性检查);调用方 skills/code-review/SKILL.md:56 与 :136;AGENTS.md 技能表 code-review 条目 将新检查保留在仓库外(PR 证据或外部文档)无法随仓库交付审查工作流;并入 solid-checklist.md 会违反 PR 明确要求该文件保持原样(与 main 版本逐字节一致)。
skills/code-review/references/code-quality-checklist.md
skills/code-review/references/python-ml-checklist.md
skills/code-review/references/removal-plan.md
skills/code-review/references/security-checklist.md
接收 四个既有 checklist 均由 SKILL.md 按变更类型加载(:66/:72/:78/:83),本 PR 对它们的修改(证据要求、发现需有依据、按可达故障与既有契约校准 Python/安全/ML 指导)与新增开发原则检查同属 #321 后续要求,需与 SKILL.md 正文保持一致,属同一职责的必要更新。 PR #323 描述(How:区分已确认需求与贡献者扩展、发现需证据、校准 Python/安全/ML 指导);调用方 skills/code-review/SKILL.md:66,72,78,83 与引用表 :138-141 拆分为独立 PR 会使 checklist 与 SKILL.md 正文在同一审查流程中前后不一致;不更新则新增原则与既有提示词相互矛盾。
精简审查与验证依据
审查范围进度结论
生产代码 ✅ 已完成 对 6 个 Markdown 文件逐一检查了可删除、合并或替换的内容:新增检查、入口重构与既有 checklist 校准均有明确需求来源和调用方(SKILL.md 分步加载),保留全部内容;未发现可简化项。
测试 ➖ 无需执行 本 PR 仅修改 skills/code-review/ 下的 Markdown 审查指引,diff 中无测试文件,测试必要性审计无适用范围。作者以 skill quick_validate.py 与 pre-commit 验证,当前 head 两项 CI 均通过。

生产代码的必要性与替代方案

范围必须保留的契约更简单的方案结论依据与限制
skills/code-review/references/development-checklist.md(新增 27 行)及 SKILL.md 步骤 2 的加载接线 SKILL.md:56 在审查流程步骤 2 加载该文件;调用方为仓库 code-review 技能(AGENTS.md 技能表登记)。内容为必要性检查、带适用边界的开发原则表与三个校准示例。 并入 solid-checklist.md 成单文件,或将该检查保留为仓库外部文档/PR 证据。 保留 PR 明确要求 solid-checklist.md 保持原样(已核对与 main 逐字节一致,不在变更清单中);独立文件加分步加载是 PR 描述声明的交付方式;原则表逐行带边界、校准示例区分应报告/不应报告,直接服务 #321 的证据要求。维护成本仅为纯 Markdown,无可执行代码。局限:实际 bot 回放效果未验证(作者亦如此声明)。
SKILL.md 的结构性更新(Evidence and Judgment、严重度按已证实影响校准、Contributor Evidence、Follow-up Reviews、Output Format) SKILL.md 是技能入口(frontmatter 由技能系统加载),定义审查流程、严重度分级与输出格式;removal-plan.md:5 引用其严重度定义。 保留旧版固定输出模板与“修复选择”对话,仅追加新章节而不改严重度定义。 保留 新章节与 #321 的证据要求一一对应;严重度表存在且被 removal-plan.md 正确引用(已核对);步骤 1-10 编号连续,引用表覆盖全部 6 个 references 文件;旧 P0/P1 定义(“SOLID 违反即 P1”)与新增的“按已证实影响定级”直接矛盾,保留会冲突。内容与上一轮已通过版本逐字节一致。
code-quality / python-ml / removal-plan / security 四个 checklist 的校准更新(合计 +54/−55 行) 四个文件均由 SKILL.md 按变更类型加载(:66/:72/:78/:83),提供各领域具体检查提示词。 不更新既有 checklist,仅交付新文件与 SKILL.md 重构。 保留 旧措辞(“纯函数一律 lru_cache”、“除零一律加 eps”、“移除候选按 P0/P1 排期”等)与 SKILL.md 新增的证据要求直接矛盾;更新后内存示例改用 .detach() 而非 .item(),与 CLAUDE.md 热路径禁止 GPU-CPU 同步的仓库规则一致;四个文件的 Contents 锚点已逐个核对可解析。
Powered by Nyanpasu with glm-5.3[1m] xhigh, please check the suggestions carefully.

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查结论:APPROVE

本 PR 为纯文档变更,仅更新 skills/code-review/ 下的 6 个 Markdown 文件,重构代码审查技能的开发原则、证据与判断标准,方向与 #321 一致。

审查要点:

  • 内容内部一致:SKILL.md 底部引用表与新章节(新增「Evidence and Judgment」「Contributor Evidence」「Follow-up Reviews」)对应;各 checklist 新增的 Contents: 锚点均能正确解析到实际标题。
  • 严重度分级、REQUEST_CHANGES/COMMENT/APPROVE 建议与 GitHub 事件解耦的表述清晰、无矛盾。
  • 无正确性、兼容性、安全或分布式相关风险;不涉及运行时代码,无需训练/GPU 测试。

CI 单元测试仍在进行中,但与本文档变更无关。无可执行的审查发现。

Powered by Nyanpasu with claude-opus-5-google[1m] medium, please check the suggestions carefully.

# 📝 Documentation

- Remove the external project attribution from development principles.

Co-authored-by: Codex <noreply@openai.com>

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查结论:APPROVE

已在新 head(1b5d3aa6)完成复审。本次 synchronize 新增的提交仅从 solid-checklist.md 的「Development Principles」章节删除一行外部项目署名,为纯文档修改。

  • 原则表格独立成立,删除后无残留失效引用或断链;
  • 此前审查的其余 5 个文件内容未变,结论继续有效;
  • 不涉及运行时代码,无正确性/兼容性/安全/分布式风险,无需训练或 GPU 测试。

无可执行的审查发现。

Powered by Nyanpasu with claude-opus-5-google[1m] medium, please check the suggestions carefully.

# 📝 Documentation

- Trace requirements to official tasks and confirmed decisions before adopting contributor or bot expansions.
- Review the necessity of new behavior through input contracts, real use cases, and removal consequences.
- Separate impact severity from acceptance blockers and require evidence for blocking P2 findings.
- Add calibration examples and ask contributors to investigate recurring causes across their changes.

Co-authored-by: Codex <noreply@openai.com>

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已完成对 0ddf986a 的复审,未发现需要修改的问题。新增指导明确区分需求依据、行为必要性与问题严重度,三个校准示例与正文一致,并保留既有契约和外部输入校验的适用边界。

本地增量 git diff --check 与当前提交的 Pre-commit CI 均通过;其余 CI 仍在运行。本次仅修改 Markdown,未运行训练或多节点 GPU 测试,也未验证实际 bot 回放效果。

Powered by Nyanpasu with gpt-6-astra medium, please check the suggestions carefully.

# 📝 Documentation

- Merge overlapping design, SOLID, smell, and refactoring guidance into one principles table.
- Retain caller-contract, interface, ownership, and dependency checks with three short calibration examples.
- Remove task-specific examples and repeated workflow guidance; keep necessity and provenance checks in the main skill.
- Explicitly check existing project capabilities before accepting new behavior.

Co-authored-by: Codex <noreply@openai.com>

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已复审 2faaa675,未发现需要修改的问题。合并后的原则表保留了调用契约、接口职责、状态归属与依赖检查;正文仍要求核实需求来源、反证和验证依据,未发现被删除章节的残留引用。

本地增量 git diff --check 和当前提交的 Pre-commit CI 通过,其余 CI 尚在运行。本次仅改 Markdown,未运行训练或多节点 GPU 测试;实际 bot 回放效果未验证。

Powered by Nyanpasu with gpt-6-astra medium, please check the suggestions carefully.

SigureMo and others added 2 commits September 20, 2026 01:21
# 📝 Documentation

- Restore the explicit preference for pure functions and immutable data while keeping the boundary for justified tensor mutation.
- Retain composition and delegation over inheritance, including the repository's depth limit, in the consolidated principles table.

Co-authored-by: Codex <noreply@openai.com>
# 📝 Documentation

- Restore the original SOLID checklist byte for byte, including its code smells and refactoring heuristics.
- Add a separate development checklist for necessity, input contracts, existing capabilities, and development principles.
- Load development and SOLID checks as separate workflow steps and update resource references.

Co-authored-by: Codex <noreply@openai.com>
@SigureMo SigureMo changed the title docs(skills): refine code review principles docs(skills): add development review checks Sep 19, 2026

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已复审 9c94939d,未发现需要修改的问题。独立的 development checklist 已接入流程;SOLID 文件与 PR 前版本逐字节一致,正文仍要求以具体证据判断问题,避免仅凭设计偏好提出修改。

已本地核对全部 6 个 checklist 引用、流程编号及增量 git diff --check;当前提交的 Pre-commit CI 通过,其余 CI 尚在运行。纯 Markdown 变更未运行训练或多节点 GPU 测试,实际 bot 回放效果未验证。

Powered by Nyanpasu with gpt-6-astra medium, please check the suggestions carefully.

# 📝 Documentation

- Check adherence to existing project patterns, user-facing configuration costs, and the effort needed to understand an operation.
- Extend existing failure and abstraction guidance to unnecessary compatibility promises and speculative extension points.
- Require overdesign findings to identify added maintenance obligations and a smaller alternative meeting the same acceptance criteria.
- Keep the original SOLID checklist unchanged.

Co-authored-by: Codex <noreply@openai.com>

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已复审 e5d7e3b5,未发现需要修改的问题。新增复杂度检查要求比较实际收益与维护成本,并提出保留验收标准的更小方案;必要配置、既有兼容性和真实故障处理的边界仍明确,与正文的证据要求一致。

本地增量 git diff --check 和当前提交的 Pre-commit CI 通过,其余 CI 尚在运行。纯文档变更未运行训练或多节点 GPU 测试,实际 bot 回放效果未验证。

Powered by Nyanpasu with gpt-6-astra medium, please check the suggestions carefully.

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查结论:APPROVE

已在新 head ceef39b1(合并 main 的 merge commit)完成复审。PR 相对 main 的 diff 与上一轮已通过的 e5d7e3b5 逐字节一致,main 侧新提交未涉及 skills/;已复核 SKILL.md 步骤接线与引用表、四个 checklist 的 Contents 锚点、removal-plan.md 引用的严重度定义,solid-checklist.md 保持与 main 一致。当前 head 的 Pre-commit 与 Python 单测 CI 通过,两个 GPU 集成测试仍在运行(仅覆盖 main 侧代码,与本次 Markdown 变更无关)。未发现需要修改的问题。

审查看板:#323 (comment)

Powered by Nyanpasu with glm-5.3[1m] xhigh, please check the suggestions carefully.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants