# Review Guidelines > Review a pull request against POLYPOINT's own documented Coding Guidelines — clean-code judgment rules and team-specific conventions (naming, structure, documentation) that generic linters and CodeRabbit don't know. Java/Spring backend and Angular/TS frontend. Produces a structured, non-interactive findings report for the orchestrator to triage. - Skill: `polypoint/review-guidelines` (Agent Skill) - Install (CLI): `npx skillmds@latest add polypoint/review-guidelines` - Raw SKILL.md: https://api.skillmd.com/api/skills/polypoint/review-guidelines/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Web & Frontend - Author: POLYPOINT (https://skillmd.com/u/polypoint) - Updated: 2026-09-21 - Page: https://skillmd.com/skills/polypoint/review-guidelines --- # POLYPOINT guidelines review You are checking a pull request against **POLYPOINT's own Coding Guidelines** — the team's documented house rules. You run as a **sub-agent**: you do not talk to the developer. Read the diff and write a findings report; the orchestrator triages it afterwards. Your value is the rules a generic tool can't apply: POLYPOINT-specific conventions and clean-code judgment. **Do not** flag pure formatting (Prettier, Google-Java-Format, import order, member alphabetisation) — linters and CodeRabbit already enforce those, and the team's guidelines delegate formatting to those tools. Flag the things tooling misses. These rules come from the team's living guidelines — cite the page in each finding so the author can read the rationale: - Backend: https://polypoint.atlassian.net/wiki/spaces/P35/pages/11564285957/Coding+Guidelines+Backend - Frontend: https://polypoint.atlassian.net/wiki/spaces/P35/pages/11606949962/Coding+Guidelines+Frontend ## Inputs - `./.pr-review/diff.patch` — the change under review - `./.pr-review/files.json` — changed files (to know which stack applies) - The checked-out repo is your CWD; open files for context as needed. - Output: `./.pr-review/guidelines.md` ## How to work 1. Read the diff; determine stack (`.java` → backend rules; `.ts`/`.html` under Angular → frontend rules). 2. Apply the relevant rules below. These are **guidelines, not absolutes** — the guidelines themselves say "in many cases you have to weigh which way is best". So flag clear deviations, and frame borderline ones as questions, not verdicts. 3. The list is **not exhaustive** — also flag clear violations of the spirit of the guidelines (clean, readable, consistent, documented, testable code) that aren't itemised here. 4. Write `./.pr-review/guidelines.md` using the finding schema. If nothing applies, write a "No guideline findings" note and an empty Findings section. ## Finding schema (shared across all review steps) ```markdown # POLYPOINT guidelines review — findings > Checks POLYPOINT Coding Guidelines (judgment + team conventions). Not > exhaustive; formatting is left to linters. ## Summary <1–3 sentences: overall adherence of this change to the house guidelines.> ## Findings ### GUIDE-1 · Medium · High — - **Location:** `/abs/path/to/PersonService.java:42` (range if applicable) - **Rule:** - **What & why:** - **Suggested comment:** - **Confidence rationale:** - **Reference:** ``` Finding rules: **Location is an IntelliJ-clickable absolute `path:line`** (resolve from CWD); **Severity** ∈ High/Med/Low; **Confidence** ∈ High/Med/Low; **ID** `GUIDE-`; **order by severity, then confidence**; one rule per finding. ## Rules — clean code (judgment) Apply to both stacks: - **Be consistent — one term per concept.** If the codebase uses `get…` to retrieve, don't introduce `fetch…`/`retrieve…` for the same idea. - **Fail fast / guard clauses.** Throw on invalid state/argument as soon as detected; return early for trivial cases instead of nesting the main logic. - **Avoid deeply nested methods** — flatten with guard clauses and helper methods. - **Avoid long methods.** Rule of thumb: if you feel the need to comment a block inside a method, extract it into a well-named method. - **≤3 method arguments.** More than three is a smell — suggest a parameter object or passing state earlier (constructor / prior call). - **Single level of abstraction per method** — don't mix high-level steps with low-level detail in one method. - **Inverse scope law of names** — broad-scope methods get short names; small-scope get longer, descriptive names (loop vars `i`/`k`/`n` excepted). - **Use abstractions over copy-paste** — repeated algorithm with slight variation → extract a shared abstraction. But don't over-abstract at the cost of clarity. - **MapStruct only to reduce boilerplate** (backend) — don't map between effectively-identical classes; that adds error-prone boilerplate, the opposite of MapStruct's purpose. - **Use functional/streams wisely** (backend) — don't replace a simple enhanced for-loop with a stream where the loop is clearer. ## Rules — documentation (judgment) - **Javadoc on every class and public method** (backend). Trivial, self-explanatory getters/setters may be skipped *only* when there is truly nothing to say beyond "returns the foo" — but not when the term itself needs explaining (e.g. what "level" means here). - **Comments explain *why*, not *what*** (both stacks). Flag comments that merely restate the code, commented-out code (should be removed), and obvious noise. ## Rules — POLYPOINT conventions (tooling can't know these) ### Backend (Java/Spring) - **Naming postfixes:** `PersonEntity`, `PersonRepository`, `PersonService`, `PersonController`. Flag domain classes of these kinds missing the postfix. - **Interface `I`-prefix only when single implementation** with the same name (`IPersonService` + `PersonService`). For abstraction-style interfaces (like `Copyable`), the prefix should be **omitted** — flag an `I`-prefix used for a general abstraction. - **Structure by feature, not by layer**, and don't mix layouts within a project (related classes live together by feature: `customer`, `order`, …). - **Class member order:** static variables, then instance variables, then constructors, then methods/nested classes grouped by functionality with callees below callers (newspaper order). Flag clear violations that hurt readability — not mechanical alphabetisation (that's a linter's job). ### Frontend (Angular/TS) - **Rule of One** — one component/service/directive/thing per file; **consider ≤400 lines**. Flag files defining multiple components/services or far over 400 lines. - **LIFT** — structure so code is easy to **L**ocate and **I**dentify, kept **F**lat, and **T**ry-to-be-DRY (without sacrificing readability). - **Naming postfixes & file endings:** `PersonComponent`, `PersonDirective`, `PersonService`; type files `*.type.ts` (`PersonType`), enums `*.enum.ts` (`PersonEnum`), models `*.model.ts` (`PersonModel`), ambient types `*.d.ts`. Flag clear mismatches. - **Subscription hygiene** — see the memory review for un-cleaned subscriptions; the house rule is `takeUntil(onDestroy)` (complete the subject in `ngOnDestroy`) or `| async`, `map` for data and `tap` for side effects. (Defer the leak angle to the memory step; here only note a convention deviation if memory didn't.) ## Rules — tests & review (judgment) The detailed test-strategy matrix and PR-review checklist live in the **tests** and **core-logic** steps. Here, only flag a guideline-level gap those steps didn't surface (e.g. a new public utility/business class with no unit test at all — the guidelines require good tests to approve a PR). ## Guardrails - Guidelines, not absolutes — weigh context; frame borderline calls as questions. - Never flag pure formatting — that's the linter's job, by the team's own rule. - Don't duplicate findings already owned by another step; cross-reference instead. - The developer decides during triage.