메뉴
Use this skill when reviewing a pull request to ForgeLM, whether your own (self-review before requesting review) or someone else's. Applies the code-review standard, catches anti-patterns, and produces actionable feedback. Triggered by requests like "review this PR", "check if PR
Codex 또는 Claude로 설치 이 Prompt를 복사해 Codex, Claude 또는 다른 어시스턴트에 붙여 넣으면 Skill 페이지를 검토하고 설치를 진행할 수 있습니다.
SOC 직업 분류 기준
| name | review-pr |
| description | Use this skill when reviewing a pull request to ForgeLM, whether your own (self-review before requesting review) or someone else's. Applies the code-review standard, catches anti-patterns, and produces actionable feedback. Triggered by requests like "review this PR", "check if PR |
Following docs/standards/code-review.md. This skill is equally useful for self-review (run it before requesting review) and peer review.
Do not use for:
Go through these in order. If any answer is "no" or "unclear," block the PR. Question 7 fires only when a regex changes; the other six apply to every PR.
Check against docs/standards/architecture.md:
config.py doesn't do training, trainer.py doesn't parse args)pyproject.toml?try: import X except: X = None)?Check against docs/standards/coding.md:
ruff check + ruff format --check pass (CI catches this — confirm CI green)Optional[X] not X | None (codebase consistency)__init__.py without reasonCheck against docs/standards/error-handling.md:
except handlersys.exit() only in cli.py, not in library modulesexcept:, no except Exception: passCheck against docs/standards/logging-observability.md:
logger = logging.getLogger("forgelm.X"))print() outside cli.py JSON-output blockssys.exit(!=0) preceded by logger.error(...)Check against docs/standards/testing.md:
pytest.skip without pytest.mark.xfail(reason=..., ...) + issue linkpragma: no cover only for documented exempt patternsCheck against docs/standards/documentation.md + localization.md:
docs/reference/configuration.md + -tr.md updateddocs/reference/usage.md + -tr.md updateddocs/guides/[Unreleased] in the right categoryconfig_template.yaml updated if new required config fieldTrigger: any change to re.compile, re.match, re.sub, re.findall, re.split. Phase 11/11.5/12 review cycles burned ~10 iterations on regex correctness — this skill exists to spend zero on the next PR.
Run git diff --unified=0 origin/main..HEAD -- '*.py' | grep -E '^\+.*re\.(compile|match|search|sub|findall|split|fullmatch)' to surface the deltas, then for each:
[A-Za-z0-9_] (Sonar python:S6353); use \w.[ ], [\.], [\\] (Sonar python:S6328); use the bare character.{1,6} for ATX heading depth, not +).* / + / *? / +? competing for the same character class. This is the #1 ReDoS shape we keep hitting ([ \t]+(.+?)[ \t]*$ → 100ms at n=2000). Anchor on \S at body boundaries: [ \t]+(\S(?:[^\n]*\S)?)[ \t]*$..*? + back-reference + re.DOTALL (Sonar python:S5852); replace with a state machine — see forgelm/data_audit.py::_strip_code_fences and forgelm/ingestion.py::_markdown_sections.\s under re.MULTILINE → prefer [ \t] (no newline ambiguity).FAKE_AWS_KEY / FAKE_GH_TOKEN in tests/test_data_audit_phase12.py).r"-----" + r"BEGIN " + r"...") so repo-wide secret scanners don't false-positive.If the regex runs on operator-controlled input, paste the pathological-input benchmark output into the PR description so reviewers see the linearity proof.
Beyond the six questions:
TODO / FIXME without owner + issue link..DS_Store, editor files — flag for removal.For reviewers (including Claude as reviewer):
❌ "Docstrings need improvement"
✅ "trainer.py:142 — the docstring says 'trains the model' but the function also saves a checkpoint; please state the full contract"
Use GitHub's suggestion blocks:
```suggestion
def train_with_revert(config: ForgeConfig, ...) -> TrainResult:
"""Train and conditionally revert on safety regression.
Args:
config: Validated run configuration.
Returns:
TrainResult with metrics and revert status.
"""
```
Nit:, Style:, or FYI:.Q: why is this Optional — can the caller guarantee non-None?Blocking: this needs to use Optional[int] to match codebase conventionBefore clicking "Create PR":
Read the diff in GitHub's web UI (not just git diff) — rendering reveals issues the CLI hides.
Grep for new TODO / FIXME / XXX — add owner + issue link or remove.
Grep for print( in non-CLI code — remove or justify.
Click through each test file — does each new test actually assert something?
Click the doc diffs — renders correctly? Links work?
Run the one-liner:
ruff format . && ruff check . && pytest tests/ && \
forgelm --config config_template.yaml --dry-run
Read your PR description — does it state the one concern clearly?
When you as reviewer disagree with the author:
Default stance: a blocked PR is better than a bad merge.
add-config-field, add-trainer-feature, add-test, sync-bilingual-docs — the skills whose output this reviewscut-release — what happens to merged PRs eventuallyUse this skill when preparing a ForgeLM release. Walks through version bump, changelog finalization, tag, publish, and post-release sync per docs/standards/release.md. Triggered by requests like "cut v0.4.0", "release ForgeLM", "prepare the next version".
Use this skill when editing any docs/reference/* or other bilingual documentation pairs in ForgeLM. Keeps the .md and -tr.md mirrors structurally aligned per docs/standards/localization.md. Triggered by requests like "update the configuration docs", "add a section about X to the usage guide", "translate the new feature docs".
Use this skill when adding a new YAML configuration field to ForgeLM. Handles Pydantic model update, cross-field validation, config_template.yaml sync, bilingual docs, and tests. Triggered by requests like "add a new config option for X", "expose Y as a YAML field", "make Z configurable".
Use this skill when writing new tests for ForgeLM. Applies conventions from docs/standards/testing.md — fixture factory patterns, mock boundaries, no-GPU-no-network discipline, coverage floor. Triggered by requests like "add a test for X", "cover function Y", "write a test that triggers error Z".
Use this skill when adding a substantial end-to-end feature to ForgeLM's training pipeline — a new alignment method, a new evaluation gate, a new quantization backend, a new distributed scheme. Differs from add-config-field by touching multiple modules and requiring integration tests. Triggered by requests like "add support for algorithm X", "integrate library Y for training", "implement feature from roadmap Phase N".
Use this skill when reviewing a pull request to ForgeLM, whether your own (self-review before requesting review) or someone else's. Applies the code-review standard, catches anti-patterns, and produces actionable feedback. Triggered by requests like "review this PR", "check if PR