pr-review-human

Compare original and translation side by side

🇺🇸

Original

English

🇨🇳

Translation

Chinese

PR Review — Human Reviewer Patterns

PR评审——人工评审参考范式

Overview

概述

This skill runs a self-review checklist based on patterns observed in actual PR reviews on NestJS/TypeScript backend repos. It helps catch issues a thorough human reviewer consistently flags before submitting for review.

本技能基于NestJS/TypeScript后端仓库中实际PR评审的经验，提供一份自检清单，帮助开发者在提交PR前，提前发现资深人工评审会重点标记的问题。

Required Workflow

必走流程

Follow these steps in order.

请按以下顺序执行步骤。

Step 1: Understand the diff

步骤1：理解代码差异

Run

git diff main...HEAD

or read the list of modified files to understand the scope of changes.

运行

git diff main...HEAD

命令，或查看修改文件列表，了解本次变更的范围。

Step 2: Apply the Checklist

步骤2：应用自检清单

Work through each category below. Flag every issue found with the file path and line number.

逐一检查以下每个类别，标记发现的所有问题，并注明文件路径和行号。

Checklist

自检清单

1. Method Complexity & Single Responsibility

1. 方法复杂度与单一职责

Flag methods that grow too large or handle too many concerns.

"This is massively increasing complexity of this already complex method. Would be a good idea to extract this logic to its own method or a few." "Would be a good idea to extract this to its own method or a few." "Could extract all this to its own method or a few."

Check:

Are there methods longer than ~40 lines containing multiple distinct concerns?
Is there logic that should be a private helper method?
Are there deeply nested
```
if/else
```
blocks that could be extracted?

标记过于庞大或承担过多职责的方法。

“这会大幅增加本已复杂的方法的复杂度。建议将这部分逻辑提取为独立方法，或拆分为多个小方法。” “建议将这部分提取为独立方法。” “可以把这些逻辑都提取为独立方法。”

检查要点：

是否存在超过40行且包含多个独立逻辑的方法？
是否有逻辑应封装为私有辅助方法？
是否存在可提取的深层嵌套
```
if/else
```
代码块？

2. Named Constants vs Magic Strings/Values

2. 命名常量 vs 魔法字符串/值

Always ask whether magic strings have constants defined elsewhere.

"Do we have constants for these states?" "Same as above — do we have constants for these states?"

Check:

Are string literals used inline that should be named constants?
Are status/state values hardcoded instead of referencing existing constants?
Search for existing constant files before adding new inline strings.

始终确认魔法字符串是否已有对应的命名常量。

“我们有没有定义这些状态的常量？” “同上——我们有没有这些状态的常量？”

检查要点：

是否存在应使用命名常量却直接内联的字符串字面量？
是否存在硬编码的状态/状态值，而非引用已有的常量？
添加新的内联字符串前，先搜索是否已有对应的常量文件。

3. Naming Clarity & Specificity

3. 命名清晰度与特异性

Flag vague or misleading names — methods, variables, files, and test descriptions.

"Could change to more specific name and could add some documentation." "Could be a more specific name." "This is a ServiceA helper — how come we have something with ServiceB?" (naming mismatch)

Check:

Are method/variable names specific enough to describe what they do without reading the body?
Are there naming mismatches — e.g., a helper named for X that contains logic for Y?
Would a new reader understand the name without context?

标记模糊或具有误导性的命名——包括方法、变量、文件和测试描述。

“可以改为更具体的名称，并添加一些文档说明。” “可以用更具体的名称。” “这是ServiceA的辅助方法——为什么会包含ServiceB的逻辑？”（命名不匹配）

检查要点：

方法/变量名称是否足够具体，无需查看实现就能理解其用途？
是否存在命名不匹配的情况——比如为X设计的辅助方法却包含Y的逻辑？
新读者无需上下文是否能理解名称的含义？

4. Documentation

4. 文档完善度

Request documentation whenever logic is non-obvious, especially for helpers and similar-looking methods.

"Could use some documentation to explain what's the difference from
generateSignature
." "This could use some documentation." "Please update state machine documentation."

Check:

Are there new methods similar to existing ones without explaining the difference?
Are new state transitions documented in relevant state machine or architecture docs?
Do helpers have JSDoc/inline comments explaining purpose and usage?
Did this change make any existing docs stale?

当逻辑不直观时，尤其是辅助方法和相似方法，要求补充文档。

“可以添加一些文档，说明与
generateSignature
的区别。” “这里需要补充文档说明。” “请更新状态机相关文档。”

检查要点：

是否存在与现有方法相似的新方法，但未说明差异？
新的状态转换是否在相关状态机或架构文档中更新？
辅助方法是否有JSDoc/内联注释说明用途和使用方式？
本次变更是否导致现有文档过时？

5. Test Data Realism

5. 测试数据真实性

Scrutinise mock data types — mocks should use semantically correct values, not generic faker strings.

"Price can be alphanumeric?" → fix:
faker.number.float(...).toString()
"What format is currency that is 10 chars in length?" → fix:
faker.finance.currencyCode()
"How come dimensions are alphanumeric?" → fix:
faker.number.float(...).toString()

Check:

Are numeric fields (price, quantity, dimensions, weight) using number generators, not
```
faker.string.alphanumeric()
```
?
Are currency fields using valid ISO codes (3 chars), not random strings?
Are URL fields using dummy but realistic shapes (e.g.,
```
https://example.com/...
```
)?
Are date fields using date generators?

仔细检查模拟数据类型——模拟数据应使用语义正确的值，而非通用的faker字符串。

“价格可以是字母数字混合？”→修复方案：
faker.number.float(...).toString()
“长度为10位的货币格式是什么？”→修复方案：
faker.finance.currencyCode()
“尺寸为什么是字母数字混合？”→修复方案：
faker.number.float(...).toString()

检查要点：

数值字段（价格、数量、尺寸、重量）是否使用数值生成器，而非
```
faker.string.alphanumeric()
```
？
货币字段是否使用有效的ISO代码（3位），而非随机字符串？
URL字段是否使用符合真实格式的虚拟地址（如
```
https://example.com/...
```
）？
日期字段是否使用日期生成器？

6. Test Naming Clarity

6. 测试命名清晰度

Read every test name and flag grammar issues and vague phrasing.

"What does 'completely' mean here?" "Sorry, what's the 'product edge'?" "Did you mean 'successfully'?" "Sorry, what does 'fetching calling' mean?"

Check:

Is every
```
it()
```
/
```
test()
```
description grammatically correct?
Do test names precisely describe the scenario and expected outcome?
Avoid vague words: "completely", "properly", "correctly", "edge" without specifics.
Preferred pattern:
```
should <do something> when <condition>
```

通读所有测试名称，标记语法问题和模糊表述。

“这里的‘completely’是什么意思？” “抱歉，‘product edge’具体指什么？” 你是不是想表达‘successfully’？” “抱歉，‘fetching calling’是什么意思？”

检查要点：

每个
```
it()
```
/
```
test()
```
的描述是否语法正确？
测试名称是否准确描述场景和预期结果？
避免使用模糊词汇：“completely”“properly”“correctly”“edge”等无具体含义的词。
推荐格式：
```
should <执行操作> when <条件>
```

7. No Real URLs or Credentials in Tests or Service Defaults

7. 测试或服务默认值中禁止使用真实URL或凭证

Flag real-looking URLs and credentials in unit tests — and also in service-level defaults that could accidentally point to production.

"is this a dummy URL? If not — let's make it such" "Let's change this to a dummy URL, if it's not." "is this production URL set by default? If so — please change to some dummy one to prevent incidents 🙏"

Check:

Are all URLs in test files clearly dummy (e.g.,
```
https://example.com
```
,
```
https://test.invalid
```
)?
Are API keys, tokens, secrets using obviously fake placeholder values?
No real partner IDs, shop IDs, or credentials from real systems.
Are there production-looking base URLs hardcoded as defaults in service constructors or constants? These should use
```
ConfigService
```
only — no hardcoded fallback to a live URL.

标记单元测试中看似真实的URL和凭证——同时也要检查服务级默认值，防止其意外指向生产环境。

“这是虚拟URL吗？如果不是，请改为虚拟地址” “请将其改为虚拟URL（如果还不是的话）。” “这是默认设置的生产环境URL吗？如果是，请改为虚拟地址，避免引发事故🙏”

检查要点：

测试文件中的所有URL是否明确为虚拟地址（如
```
https://example.com
```
、
```
https://test.invalid
```
）？
API密钥、令牌、机密是否使用明显的占位符？
禁止使用真实的合作方ID、店铺ID或真实系统中的凭证。
服务构造函数或常量中是否硬编码了看似生产环境的基础URL作为默认值？这些应仅使用
```
ConfigService
```
获取——不能硬编码回退到真实环境URL。

8. Test Assertion Completeness

8. 测试断言完整性

Check that tests actually validate critical behaviour, including negative assertions on guard/skip paths, and that multi-step flows are covered for all failure branches.

"Is there a way to actually validate that the signature has a correct value?" Guard/skip tests should assert
expect(service.processRecord).not.toHaveBeenCalled()
"What about the scenario when the request fails even after refreshing the token?"

Check:

For guard/skip paths: does the test assert that downstream methods were NOT called?
For cryptographic operations: is the output value validated, not just that the function ran?
Are all critical side effects (e.g.,
```
markAsFinalised
```
) asserted in happy-path tests?
For retry/token-refresh flows: is there a test for the case where the retry itself also fails (not just the first failure)?

检查测试是否真正验证了关键行为，包括对守卫/跳过路径的否定断言，以及多步骤流程是否覆盖了所有失败分支。

“有没有办法验证签名的具体值是否正确？” 守卫/跳过测试应断言
expect(service.processRecord).not.toHaveBeenCalled()
“如果刷新令牌后请求仍然失败的场景，有没有覆盖到？”

检查要点：

对于守卫/跳过路径：测试是否断言下游方法未被调用？
对于加密操作：是否验证了输出值，而非仅确认函数执行？
正常流程测试中，是否断言了所有关键副作用（如
```
markAsFinalised
```
）？
对于重试/令牌刷新流程：是否有测试覆盖了重试本身也失败的场景（而非仅首次失败）？

.env.example

Completeness

.env.example

文件完整性

Check that new env vars appear in

.env.example

"Should it also have
SERVICE_SANDBOX_MODE
?"

Check:

If new env vars were added, are they in
```
.env.example
```
with a descriptive placeholder?

检查新增的环境变量是否已添加到

.env.example

中。

“是不是也应该添加
SERVICE_SANDBOX_MODE
？”

检查要点：

如果新增了环境变量，是否已添加到
```
.env.example
```
中，并附带描述性占位符？

10. Type Precision

10. 类型精度

Favour precise types over loose patterns.

Suggested moving from
||
chain constants to
as const
arrays or union literal types. Optional fields that are always provided should be required. "Could be just a boolean if we'd change it to something like
isAvailable
. Expiration could just set it to
false
in the future if needed."

Check:

Are constants defined with
```
||
```
chains that should be
```
as const
```
tuples or union types?
Are optional fields (
```
field?: T
```
) used where the field is always present and should be required?
When a field only needs to represent on/off, prefer
```
boolean
```
with a descriptive
```
is_
```
name over a nullable date or status enum — it's simpler and future logic can just set it to
```
false
```
.

优先使用精确类型，而非松散类型。

建议将
||
链式常量改为
as const
数组或联合字面量类型。始终存在的可选字段应改为必填字段。 “如果改为类似
isAvailable
的布尔类型会更简单。如果未来需要过期逻辑，只需将其设置为
false
即可。”

检查要点：

是否存在用
```
||
```
链定义的常量，应改为
```
as const
```
元组或联合类型？
是否在字段始终存在的情况下，使用了可选字段（
```
field?: T
```
）？
当字段仅需表示开关状态时，优先使用带
```
is_
```
前缀的布尔类型，而非可空日期或状态枚举——这样更简单，未来逻辑只需将其设为
```
false
```
即可。

11. Logical Grouping of Types & Constants

11. 类型与常量的逻辑分组

Notice when types or constants are placed in the wrong logical section.

"Nit: Shouldn't these be under the same domain section as well? =).."

Check:

Are new types/constants placed in the section that matches their domain?
Are auth types under auth? Payment types under payment? etc.

注意类型或常量是否被放置在错误的逻辑模块中。

“小建议：这些是不是也应该放在对应的领域模块下？=)..”

检查要点：

新增的类型/常量是否放置在与其领域匹配的模块中？
认证类型是否在认证模块下？支付类型是否在支付模块下？以此类推。

12. Idempotency of External Calls in Retry Contexts

12. 重试场景下外部调用的幂等性

Flag places where a call to an external system is made in a cron or retry context without considering what happens if the call succeeds externally but fails to record locally — causing the next retry to create a duplicate.

"Just pointing out that if this call fails — we'll try to create the record again in the next cron run, but it would have already been created in the external system."

Check:

Are there cron-driven or retry-driven calls to external APIs where a partial failure (succeeded remotely, failed to record locally) would lead to duplicate creation on retry?
Does the external API support idempotency keys or duplicate detection?
If not, consider checking whether the resource already exists before creating it.

标记在定时任务或重试场景中调用外部系统，但未考虑以下情况的代码：外部调用成功，但本地记录失败，导致下一次重试创建重复数据。

“需要注意的是，如果这个调用失败——下一次定时任务会尝试再次创建记录，但此时外部系统中已经创建了该记录。”

检查要点：

是否存在定时任务或重试驱动的外部API调用，当出现部分失败（远程调用成功，本地记录失败）时，重试会导致重复创建？
外部API是否支持幂等键或重复检测？
如果不支持，是否考虑在创建前检查资源是否已存在？

13. Log Level for Security and Validation Failures

13. 安全与验证失败的日志级别

Flag when a security-relevant failure (e.g., failed HMAC signature validation, missing auth token) is logged at

warn

instead of

error

"I think this deserves to be an error." (HMAC guard logging warning on invalid signature)

Check:

Are security failures (invalid signatures, unauthorized access, failed auth) logged at
```
error
```
level, not
```
warn
```
?
Warnings imply "something unusual but possibly ok" — security failures are always errors.

标记将安全相关失败（如HMAC签名验证失败、缺少认证令牌）记录为

warn

级别而非

error

级别的情况。

“我认为这应该记录为error级别。”（HMAC守卫在签名无效时记录警告日志）

检查要点：

安全失败（无效签名、未授权访问、认证失败）是否记录为
```
error
```
级别，而非
```
warn
```
？
警告意味着“异常但可能可接受”——安全失败始终属于错误。

14. Use Existing Helpers Consistently

14. 一致性使用现有辅助方法

Flag when code manually constructs a value (key, path, string) that already has a dedicated helper function elsewhere.

"Let's also use
getAccessTokenCacheKey
to be safe?"

Check:

Before constructing cache keys, identifiers, or formatted strings inline, search for an existing helper that does the same.
If a helper exists for building a value, use it everywhere — don't mix inline construction with helper usage in the same codebase.
Inconsistent construction means a future change to the format only gets applied in some places.

标记手动构造已有专用辅助方法的数值（键、路径、字符串）的代码。

“我们是不是也应该使用
getAccessTokenCacheKey
来确保一致性？”

检查要点：

在手动构造缓存键、标识符或格式化字符串前，先搜索是否已有对应的辅助方法。
如果存在构造该值的辅助方法，应在所有地方统一使用——不要在代码库中混合使用手动构造和辅助方法。
不一致的构造方式意味着未来格式变更时，只有部分代码会被更新。

15. YAGNI — Avoid Implementing Unbuilt Features

15. YAGNI原则——避免实现未规划的功能

Flag when constants, fields, or logic are added for a feature that does not exist yet.

"We don't have expiration logic yet, hence not sure this is needed =)..."

Check:

Are there constants, entity fields, or validation rules for a feature (e.g., expiration, quotas, tiers) that has not been built yet?
Defer those additions until the feature is actually being implemented.
The comment "we'll need this later" is not enough justification — add it when "later" arrives.

标记为尚未开发的功能添加常量、字段或逻辑的情况。

“我们目前还没有过期逻辑，所以这个可能不需要=)...”

检查要点：

是否为尚未开发的功能（如过期、配额、层级）添加了常量、实体字段或验证规则？
这些添加应推迟到功能实际开发时再进行。
“我们以后会需要这个”不足以成为理由——等“以后”到来时再添加。

16. Database Schema & Migration Quality

16. 数据库Schema与迁移质量

Review entity column types and migrations for precision — flag loose types, missing constraints, wrong defaults, and mismatched indexes.

"This is DateTime, right?" (entity field missing timestamptz decorator) "Let's make it a big integer. No need for decimal points in this case." "Could be a small int, not sure if ORM allows to specify." "is it possible to specify the length?" (migration column missing length) "Default should be
null
, right? =)..." (column default mismatch) "We are going to be querying by
hash
, right?
status
column index does not make sense." "You might want to add UNIQUE index on the
hash
column."

Check:

Entity field decorators: does
```
@Column()
```
specify the right type (
```
timestamptz
```
,
```
bigint
```
,
```
smallint
```
) to match the actual data?
Numeric columns: use
```
int
```
/
```
bigint
```
/
```
smallint
```
for whole numbers, not
```
float
```
/
```
decimal
```
.
Migration columns: specify explicit lengths for
```
varchar
```
columns where the domain has a known maximum (hash strings, codes, slugs). Ask "is it possible to specify the length?" for unbound varchars.
Column defaults: nullable columns should default to
```
null
```
; non-nullable columns should have an explicit default.
Index design: indexes must match the actual query patterns.
- Add a UNIQUE index on lookup columns (e.g.,
```
hash
```
  ,
```
code
```
  ,
```
token
```
  ).
- Don't add indexes on columns that are never used as a query filter.

评审实体列类型和迁移的精度——标记松散类型、缺失约束、错误默认值和不匹配的索引。

“这应该是DateTime类型，对吗？”（实体字段缺少timestamptz装饰器） “我们把它设为大整数吧，这里不需要小数。” “可以设为小整数吗？不确定ORM是否支持。” “能不能指定长度？”（迁移列未设置长度） “默认值应该是
null
，对吗？=)...”（列默认值不匹配） “我们会按
hash
查询，对吗？
status
列的索引没有意义。” “你可能需要为
hash
列添加唯一索引。”

检查要点：

实体字段装饰器：
```
@Column()
```
是否指定了与实际数据匹配的正确类型（
```
timestamptz
```
、
```
bigint
```
、
```
smallint
```
）？
数值列：整数使用
```
int
```
/
```
bigint
```
/
```
smallint
```
，而非
```
float
```
/
```
decimal
```
。
迁移列：对于有已知最大长度的
```
varchar
```
列（哈希字符串、编码、短链接），应指定明确长度。对于无限制的varchar列，询问“能不能指定长度？”。
列默认值：可空列默认值应为
```
null
```
；非空列应有明确的默认值。
索引设计：索引必须与实际查询模式匹配。
- 为查询列（如
```
hash
```
  、
```
code
```
  、
```
token
```
  ）添加唯一索引。
- 不要为从未作为查询条件的列添加索引。

Step 3: Report Findings

步骤3：报告发现的问题

For each issue found, output:

[CATEGORY] file/path:line
Issue: <description>
Fix: <suggestion>

If no issues in a category, skip it.

对于每个发现的问题，按以下格式输出：

[类别] 文件路径:行号
问题: <描述>
修复建议: <建议>

如果某个类别没有发现问题，可跳过。

Step 4: Summary

步骤4：总结

End with:

Total issues found (blocking / nit)
Files most affected
Verdict: ready for review / needs changes before submitting

最后添加：

发现的问题总数（阻塞性/小问题）
受影响最严重的文件
结论：可提交评审 / 需要修改后再提交

pr-review-human

Original

Translation

PR Review — Human Reviewer Patterns

PR评审——人工评审参考范式

Overview

概述

Required Workflow

必走流程

Step 1: Understand the diff

步骤1：理解代码差异

Step 2: Apply the Checklist

步骤2：应用自检清单

Checklist

自检清单

1. Method Complexity & Single Responsibility

1. 方法复杂度与单一职责

2. Named Constants vs Magic Strings/Values

2. 命名常量 vs 魔法字符串/值

3. Naming Clarity & Specificity

3. 命名清晰度与特异性

4. Documentation

4. 文档完善度

5. Test Data Realism

5. 测试数据真实性

6. Test Naming Clarity

6. 测试命名清晰度

7. No Real URLs or Credentials in Tests or Service Defaults

7. 测试或服务默认值中禁止使用真实URL或凭证

8. Test Assertion Completeness

8. 测试断言完整性

9. .env.example Completeness

9. .env.example文件完整性

10. Type Precision

10. 类型精度

11. Logical Grouping of Types & Constants

11. 类型与常量的逻辑分组

12. Idempotency of External Calls in Retry Contexts

12. 重试场景下外部调用的幂等性

13. Log Level for Security and Validation Failures

13. 安全与验证失败的日志级别

14. Use Existing Helpers Consistently

14. 一致性使用现有辅助方法

15. YAGNI — Avoid Implementing Unbuilt Features

15. YAGNI原则——避免实现未规划的功能

16. Database Schema & Migration Quality

16. 数据库Schema与迁移质量

Step 3: Report Findings

步骤3：报告发现的问题

Step 4: Summary

步骤4：总结

9.
`.env.example`
Completeness

9.
`.env.example`
文件完整性