Skip to content

feat: add lightweight verification review#55

Open
ddddddddwp wants to merge 2 commits into
rpamis:masterfrom
ddddddddwp:codex/lightweight-verify-review
Open

feat: add lightweight verification review#55
ddddddddwp wants to merge 2 commits into
rpamis:masterfrom
ddddddddwp:codex/lightweight-verify-review

Conversation

@ddddddddwp
Copy link
Copy Markdown

Summary

  • Require lightweight verification to include a scoped code review.
  • Limit the review to correctness, security, and edge cases.
  • Add workflow safeguard coverage and a changelog entry.
  • Address the prior review nit by capitalizing the English lightweight review checklist item.

Test Plan

  • npx vitest run test/ts/skills.test.ts
  • git diff --check origin/master..HEAD

Notes

  • Full npx vitest run currently fails in test/ts/init-e2e.test.ts because the clean-directory init fixture cannot find .claude/skills in the temporary Lingma Superpowers staging directory, which also leaves .codex/skills/comet/SKILL.md absent.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a lightweight verification review step to the verification process, requiring a scoped code review focused on correctness, security, and edge cases without running full spec or design drift checks. This change is documented in both English and Chinese skill files, and corresponding test assertions have been added. The reviewer feedback suggests localizing the English word "correctness" to "正确性" in the Chinese skill file and updating the corresponding test assertions to maintain consistency.

Comment thread assets/skills-zh/comet-verify/SKILL.md Outdated
3. 编译通过(执行项目对应的构建命令,如 `npm run build`、`mvn compile`、`cargo build` 等)
4. 相关测试通过
5. 无明显安全问题(无硬编码密钥、无新增 unsafe 操作)
6. 简化代码审查通过:派发 code-reviewer 子 agent,只检查 correctness、安全、边界条件
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

In the Chinese version of the skill, the English word "correctness" is used in the checklist item, whereas "正确性" is used in the subsequent paragraph (line 96). To maintain language consistency and proper localization, "correctness" should be translated to "正确性".

Suggested change
6. 简化代码审查通过:派发 code-reviewer 子 agent,只检查 correctness、安全、边界条件
6. 简化代码审查通过:派发 code-reviewer 子 agent,只检查正确性、安全、边界条件

Comment thread assets/skills-zh/comet-verify/SKILL.md Outdated
- spec scenario 覆盖率
- design doc 一致性深度比对
- code pattern consistency 建议
- 不影响 correctness、安全、边界条件的 code pattern consistency 建议
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Similarly, "correctness" in the skipped items list should be translated to "正确性" for consistency with the rest of the Chinese document.

Suggested change
- 不影响 correctness、安全、边界条件的 code pattern consistency 建议
- 不影响正确性、安全、边界条件的 code pattern consistency 建议

Comment thread test/ts/skills.test.ts Outdated
expect(zhVerify).toContain('CRITICAL 失败项必须修复');
expect(zhVerify).toContain('不允许跳过修复直接全部接受');
expect(zhVerify).toContain('简化代码审查');
expect(zhVerify).toContain('只检查 correctness、安全、边界条件');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since we are translating "correctness" to "正确性" in the Chinese skill file, the corresponding test assertion needs to be updated to match the localized string.

Suggested change
expect(zhVerify).toContain('只检查 correctness、安全、边界条件');
expect(zhVerify).toContain('只检查正确性、安全、边界条件');

@benym
Copy link
Copy Markdown
Contributor

benym commented May 30, 2026

Please fix the issues in the Gemini review first.

@ddddddddwp
Copy link
Copy Markdown
Author

已经解决Gemini评论中的问题。

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