vault backup: 2026-01-05 14:26:17
This commit is contained in:
@@ -0,0 +1,526 @@
|
||||
|
||||
review
|
||||
|
||||
---
|
||||
|
||||
# Code Review Prompt (improved)
|
||||
|
||||
**Goal:** Provide a rigorous, actionable review that balances correctness, security, and maintainability for the following code.
|
||||
|
||||
## Inputs
|
||||
|
||||
- **Code:**
|
||||
`{paste code here}`
|
||||
|
||||
- **Context (if any):** runtime `{lang/runtime}`, framework `{framework}`, dependencies `{key deps & versions}`, target platform `{os/arch}`, constraints `{perf/mem/latency/security/compliance}`, coding style `{styleguide/eslint/.editorconfig}`, known requirements `{tickets/PRD refs}`.
|
||||
|
||||
|
||||
## Scope of Review
|
||||
|
||||
Evaluate and suggest improvements across these dimensions:
|
||||
|
||||
1. **Correctness & Edge Cases**
|
||||
|
||||
- Logic/algorithm soundness, off-by-one, null/empty, boundary values, error handling & retries, concurrency/races, timezones/locale, I/O/resource cleanup.
|
||||
|
||||
2. **Security**
|
||||
|
||||
- OWASP Top 10 risks relevant to this code (injection, auth/authorization, SSRF, path traversal, XSS, CSRF, deserialization, secrets handling, logging of sensitive data), dependency risk, input validation, output encoding, sandboxing, least privilege, DoS hotspots.
|
||||
|
||||
3. **Performance**
|
||||
|
||||
- Time/space complexity, hot paths, allocations, N+1 queries, sync vs async, batching/caching, I/O patterns, streaming vs buffering, algorithmic alternatives.
|
||||
|
||||
4. **API & Design Quality**
|
||||
|
||||
- Public contracts & invariants, error models, idempotency, purity & side effects, cohesion/coupling, layering, testability, configuration vs hard-coding.
|
||||
|
||||
5. **Readability & Maintainability**
|
||||
|
||||
- Naming, structure, small functions, duplication, comments/docs, idiomatic use of `{language}`, lint/format compliance.
|
||||
|
||||
|
||||
## Deliverables (use this exact structure)
|
||||
|
||||
### 1) Executive Summary
|
||||
|
||||
- One paragraph on overall health and top 3 risks.
|
||||
|
||||
|
||||
### 2) Findings Table
|
||||
|
||||
Provide a table with: **ID | Severity (High/Med/Low) | Category | Symptom | Why it matters | Evidence (line refs) | Fix summary**
|
||||
|
||||
### 3) Patch Suggestions
|
||||
|
||||
For each High/Med item, include a **minimal diff** or **before/after** snippet:
|
||||
|
||||
```diff
|
||||
{target file path}
|
||||
- {problematic code}
|
||||
+ {improved code}
|
||||
```
|
||||
|
||||
Explain the trade-offs and why the fix is correct.
|
||||
|
||||
### 4) Tests to Add
|
||||
|
||||
List concrete test cases (names + intent). Include edge values and failure paths.
|
||||
|
||||
- Unit: `{TestName_Should...}`
|
||||
|
||||
- Integration: `{Scenario_When..._Then...}`
|
||||
|
||||
- Property/Fuzz (if applicable): input domains & invariants.
|
||||
|
||||
|
||||
### 5) Performance Notes
|
||||
|
||||
- Estimated complexity and bottlenecks.
|
||||
|
||||
- Quick wins (e.g., cache/batch/stream) and expected impact.
|
||||
|
||||
|
||||
### 6) Security Checklist
|
||||
|
||||
- Inputs validated? Output encoded? Secrets sourced from vault? Least privilege? Safe defaults? Rate limiting? Logging PII redaction?
|
||||
|
||||
|
||||
### 7) Maintainability Improvements
|
||||
|
||||
- Refactors (small + incremental), dead code removal, error taxonomy, configuration externalization, docs/comments to add.
|
||||
|
||||
|
||||
### 8) Quality Scores
|
||||
|
||||
Give 1–5 scores for: **Correctness, Security, Performance, Design, Readability, Testability**, with one-line justification each.
|
||||
|
||||
## Constraints
|
||||
|
||||
- Prefer **minimal, targeted changes** over large rewrites.
|
||||
|
||||
- Match existing project style and patterns.
|
||||
|
||||
- If context is missing, **state assumptions explicitly** and proceed.
|
||||
|
||||
- Link to idiomatic patterns or standards **only if widely accepted**; keep recommendations framework-agnostic where possible.
|
||||
|
||||
|
||||
## Output Format
|
||||
|
||||
Return **only** the sections 1–8 above in Markdown. Keep code blocks self-contained and compilable where possible.
|
||||
|
||||
---
|
||||
|
||||
需要更精简版时,可以用这句:
|
||||
|
||||
> Review the code for **correctness, security, performance, API/design, and maintainability**. Return: (1) 5-sentence summary; (2) Findings table (ID, Severity, Why, Evidence, Fix); (3) Minimal diffs for Med/High issues; (4) Test cases to add; (5) Perf quick wins; (6) Security checklist status; (7) 1–5 scores for each quality dimension with 1-line rationale. Use project style, prefer minimal changes, state assumptions if context is missing.
|
||||
|
||||
|
||||
|
||||
|
||||
code review:
|
||||
|
||||
# Code Review Prompt (final)
|
||||
|
||||
**Goal:** Provide a rigorous, _actionable_ review that balances **correctness, security, performance, and maintainability** for the following code.
|
||||
|
||||
---
|
||||
|
||||
## Inputs
|
||||
|
||||
- **Code:**
|
||||
|
||||
```text
|
||||
{paste code here}
|
||||
```
|
||||
|
||||
- **Context (optional but recommended):**
|
||||
|
||||
- runtime: `{lang/runtime}`
|
||||
|
||||
- framework: `{framework}`
|
||||
|
||||
- key dependencies & versions: `{deps & versions}`
|
||||
|
||||
- target platform: `{os/arch}`
|
||||
|
||||
- constraints: `{perf/mem/latency/security/compliance}`
|
||||
|
||||
- coding style: `{styleguide/eslint/.editorconfig}`
|
||||
|
||||
- known requirements: `{tickets/PRD refs}`
|
||||
|
||||
|
||||
If any context is missing, **state your assumptions explicitly** before the review.
|
||||
|
||||
---
|
||||
|
||||
## Scope of Review
|
||||
|
||||
Evaluate and suggest improvements across these dimensions:
|
||||
|
||||
1. **Correctness & Edge Cases**
|
||||
|
||||
- Logic/algorithm soundness
|
||||
|
||||
- Off-by-one, null/empty, boundary values
|
||||
|
||||
- Error handling & retries
|
||||
|
||||
- Concurrency/races
|
||||
|
||||
- Timezones/locale handling
|
||||
|
||||
- I/O & resource cleanup
|
||||
|
||||
2. **Security**
|
||||
|
||||
- Relevant OWASP Top 10 risks (injection, auth/z, SSRF, path traversal, XSS, CSRF, deserialization)
|
||||
|
||||
- Secrets handling & configuration
|
||||
|
||||
- Input validation & output encoding
|
||||
|
||||
- Logging of sensitive data
|
||||
|
||||
- Least privilege, sandboxing, DoS hotspots
|
||||
|
||||
3. **Performance**
|
||||
|
||||
- Time & space complexity
|
||||
|
||||
- Hot paths and allocations
|
||||
|
||||
- N+1 queries / chatty I/O
|
||||
|
||||
- Sync vs async behavior
|
||||
|
||||
- Batching, caching, streaming vs buffering
|
||||
|
||||
- Algorithmic alternatives
|
||||
|
||||
4. **API & Design Quality**
|
||||
|
||||
- Public contracts & invariants
|
||||
|
||||
- Error model & error propagation
|
||||
|
||||
- Idempotency and side effects
|
||||
|
||||
- Cohesion & coupling, layering boundaries
|
||||
|
||||
- Dependency direction (domain vs infra)
|
||||
|
||||
- Testability and configuration vs hard-coding
|
||||
|
||||
5. **Readability & Maintainability**
|
||||
|
||||
- Naming and intent clarity
|
||||
|
||||
- Function/module size and structure
|
||||
|
||||
- Duplication vs reuse
|
||||
|
||||
- Comments/docs (where needed)
|
||||
|
||||
- Idiomatic use of `{language}`
|
||||
|
||||
- Lint/format compliance
|
||||
|
||||
|
||||
---
|
||||
|
||||
## Deliverables (use this exact structure)
|
||||
|
||||
### 1) Executive Summary
|
||||
|
||||
- One short paragraph on overall health.
|
||||
|
||||
- List the **top 3 risks or opportunities** (bullets).
|
||||
|
||||
|
||||
### 2) Findings Table
|
||||
|
||||
Provide a table with:
|
||||
|
||||
- **ID** – short stable identifier (e.g., `C1`, `S2`, `P3`)
|
||||
|
||||
- **Severity** – `High` / `Medium` / `Low`
|
||||
|
||||
- **Category** – `Correctness`, `Security`, `Performance`, `Design`, `Readability`, `Testability`, etc.
|
||||
|
||||
- **Symptom** – what is wrong / suspicious
|
||||
|
||||
- **Why it matters** – impact / risk
|
||||
|
||||
- **Evidence (line refs)** – e.g., `file.go:42-57`
|
||||
|
||||
- **Fix summary** – 1–2 line suggested direction
|
||||
|
||||
|
||||
Example:
|
||||
|
||||
|ID|Severity|Category|Symptom|Why it matters|Evidence|Fix summary|
|
||||
|---|---|---|---|---|---|---|
|
||||
|C1|High|Correctness|Possible nil deref on error path|Can cause runtime panic in production|`handler.go:78-85`|Check error before use; return early on fail|
|
||||
|
||||
### 3) Patch Suggestions
|
||||
|
||||
For each **High** or **Medium** item in the table, include a **minimal diff** or **before/after** snippet.
|
||||
|
||||
```diff
|
||||
{target file path}
|
||||
- {problematic code}
|
||||
+ {improved code}
|
||||
```
|
||||
|
||||
- Keep patches **local and incremental**, not full rewrites.
|
||||
|
||||
- Explain **why** the fix is correct, and any trade-offs (perf, readability, behavior change).
|
||||
|
||||
|
||||
### 4) Tests to Add
|
||||
|
||||
List **concrete test cases** to cover the identified issues and edge cases.
|
||||
|
||||
- Unit tests (with intent):
|
||||
|
||||
- `Test_{UnitName}_ShouldHandleEmptyInput` – verifies behavior when input is empty
|
||||
|
||||
- `Test_{FuncName}_ShouldReturnErrorOnTimeout` – covers timeout/failure path
|
||||
|
||||
- Integration tests:
|
||||
|
||||
- `{Scenario_When..._Then...}` – describe full flows: external calls, DB, queues, etc.
|
||||
|
||||
- Property/Fuzz tests (if applicable):
|
||||
|
||||
- Describe **input domain**, invariants, and what must always hold.
|
||||
|
||||
|
||||
Where possible, map tests back to **Finding IDs** (e.g. “C1, S2”).
|
||||
|
||||
### 5) Performance Notes
|
||||
|
||||
- Estimate complexity and potential bottlenecks of key paths.
|
||||
|
||||
- Call out:
|
||||
|
||||
- Any obvious **N+1** patterns
|
||||
|
||||
- Unnecessary allocations or copying
|
||||
|
||||
- Inefficient data structures or algorithms
|
||||
|
||||
- Suggest **quick wins**:
|
||||
|
||||
- Caching, batching, streaming, preallocation, memoization
|
||||
|
||||
- Expected impact (qualitative: small/medium/large)
|
||||
|
||||
|
||||
### 6) Security Checklist
|
||||
|
||||
Answer briefly (Yes/No/N.A. + short note):
|
||||
|
||||
- Inputs validated at boundaries?
|
||||
|
||||
- Outputs properly encoded for their sinks (HTML/SQL/OS/etc.)?
|
||||
|
||||
- Auth & authorization checks present and correctly ordered?
|
||||
|
||||
- Secrets kept out of code (config, env, vault)?
|
||||
|
||||
- Least privilege for external resources (DB, queues, files)?
|
||||
|
||||
- Safe defaults (e.g., secure TLS, secure cookies, strict modes)?
|
||||
|
||||
- Rate limiting / throttling for expensive or exposed endpoints?
|
||||
|
||||
- Logs avoid PII/credential leakage; sensitive data redacted or omitted?
|
||||
|
||||
|
||||
Highlight any **High** severity gaps and link them to Findings IDs.
|
||||
|
||||
### 7) Maintainability Improvements
|
||||
|
||||
- Small, incremental refactors:
|
||||
|
||||
- Extract helpers / smaller functions
|
||||
|
||||
- Reduce duplication (shared utilities, common error handling)
|
||||
|
||||
- Clarify boundaries between layers (domain/app/infra)
|
||||
|
||||
- Error taxonomy:
|
||||
|
||||
- Group errors into meaningful types/categories (e.g., validation vs system vs external)
|
||||
|
||||
- Standardize error wrapping and messages
|
||||
|
||||
- Configuration:
|
||||
|
||||
- Externalize magic numbers/strings
|
||||
|
||||
- Centralize feature flags or switches
|
||||
|
||||
- Documentation:
|
||||
|
||||
- Add or update docstrings for non-obvious logic
|
||||
|
||||
- Brief README/ADR notes if design is non-trivial
|
||||
|
||||
|
||||
### 8) Quality Scores
|
||||
|
||||
Give **1–5** scores (5 = excellent, 1 = poor) with a **one-line justification** each:
|
||||
|
||||
- **Correctness:** `X/5` – `{short reason}`
|
||||
|
||||
- **Security:** `X/5` – `{short reason}`
|
||||
|
||||
- **Performance:** `X/5` – `{short reason}`
|
||||
|
||||
- **Design:** `X/5` – `{short reason}`
|
||||
|
||||
- **Readability:** `X/5` – `{short reason}`
|
||||
|
||||
- **Testability:** `X/5` – `{short reason}`
|
||||
|
||||
|
||||
---
|
||||
|
||||
## Constraints
|
||||
|
||||
- Prefer **minimal, targeted changes** over big-bang rewrites.
|
||||
|
||||
- Match **existing project style and patterns** where visible.
|
||||
|
||||
- If context is missing, **state assumptions explicitly** and proceed.
|
||||
|
||||
- Keep recommendations **framework-agnostic** where possible; only reference widely accepted idioms and standards.
|
||||
|
||||
- When in doubt, **prioritize clarity and safety** over micro-optimizations.
|
||||
|
||||
|
||||
---
|
||||
|
||||
## Output Format
|
||||
|
||||
Return **only** sections **1–8** above in Markdown when performing an actual review.
|
||||
Keep all code blocks self-contained and compilable where possible.
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
----
|
||||
|
||||
# 可观测性 —— **4.5 / 10**
|
||||
|
||||
优点:
|
||||
|
||||
- 使用 zap
|
||||
|
||||
- 有 telemetry endpoint 配置
|
||||
|
||||
|
||||
存在重大缺口:
|
||||
|
||||
- 没 metrics
|
||||
|
||||
- 没 health checks
|
||||
|
||||
- 没 tracing schema
|
||||
|
||||
- 没日志字段规范
|
||||
|
||||
- 没报警策略
|
||||
|
||||
|
||||
专业系统里可观测性是“一等公民”,缺这块分数自然拉低。
|
||||
|
||||
---
|
||||
|
||||
# 4️⃣ 可靠性(Reliability & Fault Handling)—— **5.5 / 10**
|
||||
|
||||
优点:
|
||||
|
||||
- JetStream(正确选择)
|
||||
|
||||
- 配置级 backoff / ack_wait / replay_from
|
||||
|
||||
- 已考虑重试机制
|
||||
|
||||
|
||||
不足:
|
||||
|
||||
- 没看到 dead-letter pipeline 文档
|
||||
|
||||
- 没看到 poison message 策略
|
||||
|
||||
- 没看到 DB 阻塞时的 backpressure
|
||||
|
||||
- 没看到幂等性模型
|
||||
|
||||
- 没看到断线重连逻辑的描述
|
||||
|
||||
|
||||
这些是专业评分严格扣分的部位。
|
||||
|
||||
|
||||
|
||||
### 严格评审的缺失
|
||||
|
||||
- 没看到“dead-letter pipeline”定义
|
||||
|
||||
- 没看到“poison message”策略
|
||||
|
||||
- 没看到“持久化失败策略”
|
||||
|
||||
- 没看到“DB 降级”逻辑
|
||||
|
||||
- 没看到“幂等性策略”(特别关键)
|
||||
|
||||
- 没看到“重平衡策略”(consumer scaling)
|
||||
|
||||
- 没看到“高可用拓扑”(replicas 仅是 JetStream 层,服务自身无说明)
|
||||
|
||||
|
||||
按专业级评分,就是 **4/10**。
|
||||
|
||||
|
||||
这个维度是最严格的(专业评分里非常重要)。
|
||||
|
||||
### ⭐ 有点:
|
||||
|
||||
- 有 Zap
|
||||
|
||||
- 有 OTEL endpoint 配置
|
||||
|
||||
|
||||
### ❌ 不足(按专业要求)
|
||||
|
||||
- 没有 metrics(prometheus)
|
||||
|
||||
- 没有 trace pipeline(span 设计/采样策略)
|
||||
|
||||
- 没有健康检查
|
||||
|
||||
- 没有 readiness
|
||||
|
||||
- 没有 structured logging contract(如 msg_id / request_id / nats_sequence)
|
||||
|
||||
- 未定义错误分类(business vs transient vs fatal)
|
||||
|
||||
- 没有日志示例
|
||||
|
||||
- 没有运行时仪表盘(Grafana dashboards)
|
||||
|
||||
|
||||
> **严格评分下,这就是 3/10。**
|
||||
>
|
||||
|
||||
----
|
||||
Reference in New Issue
Block a user