# slop > Invoke only when the user explicitly asks to review code through the "single level of abstraction / layered error handling" lens — a function does only its own layer's business logic while errors are handled above or below. The agent reports detections, raw-count measurements, and move directions. Apply only when the user explicitly requests this lens. - Skill: `pymodel/slop` (Agent Skill) - Install (CLI): `npx skillmds add pymodel/slop` - Raw SKILL.md: https://api.skillmd.com/api/skills/pymodel/slop/raw - Safety review: pending (external: skill-scanner PASS, skillspector PASS) - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: AI & ML - Author: PyModel (https://skillmd.com/u/pymodel) - Updated: 2026-08-19 - Page: https://skillmd.com/skills/pymodel/slop --- # Single Level of Abstraction & Layered Error Handling North star: **a function should read as a straight-line description of what its own layer does. Anything that is not that — input validation, error handling, error-to-response translation, logging, retries, low-level mechanics — belongs to a layer above or below, not inline.** This is a review dimension, not a hard rule. See "Exemption checklist" at the end. ## Scope of this skill — detect and measure The agent applying this lens is a **sensor**. Its one job is to report *whether* a function mixes levels and *by how much*; deciding *how serious* it is belongs downstream. Severity labels (`Block` / `Request changes` / `Nit`) compress a continuous quantity into an uncalibrated three-point scale and are the main source of review-to-review variance, so they are produced downstream — by a deterministic rubric, anchored examples, or a human — from the facts the agent reports. The agent's output is exactly these four things: - **Detection (yes/no):** does this statement / block / function violate a rule of the lens? - **Measurement (raw factual counts only):** mechanically countable quantities — body size, control-flow keywords, named syntactic shapes (see "Quantify"). Anything that first requires classifying a line (core/foreign, happy/error, high/low level) is recorded under detection, not here. - **Direction (where it moves):** for each foreign concern, the destination layer — push **down** into a value / parser / infra helper, or push **up** into the edge handler. - **Exemption flags:** which items, if any, hit the exemption checklist — recorded, not weighed. Severity grades, merge/block verdicts, and "is splitting worth it" calls live downstream, derived from the four items above. ## When to use Apply this lens only when the user asks for it explicitly (for example "用单一抽象层次审视一下", "check whether this function does too much", "errors should be handled above/below, right?"). Leave general reviews and refactors to other lenses unless the user names this one. ## The principle One function, one level of abstraction, one responsibility. Three mutually reinforcing rules: 1. **Single Level of Abstraction (SLAP).** Every statement inside a function sits at the same conceptual level. High-level intent ("reserve inventory, charge payment, create the order") must not be interleaved with low-level mechanics (building headers, escaping strings, opening sockets, parsing bytes). If some lines read as "what" and others as "how", they belong in different functions. 2. **Error handling is its own concern (Clean Code).** A function either does the work or handles the error — not both. Business logic describes the happy path and *signals* failure (throw or return a result); the catch, mapping, logging, and recovery live in a dedicated handler, usually one layer up. Prefer exceptions / result types over threaded check-and-return ladders that interrupt the main flow. 3. **Separation of concerns by layer.** Each layer owns exactly one kind of knowledge: low-level code knows formats and protocols; mid-level code knows business rules; edge code knows the outside world (HTTP / CLI / UI). A function that knows two of these at once is leaking a layer. The combined test: **could you explain this function to someone without using the word "and"?** If the explanation is "it reserves stock AND validates the email format AND maps the error to a status code AND logs to metrics", it is doing more than its layer's job. Concerns that usually do **not** belong in a business function: - Format / range / null validation that a lower value or parser could guarantee once. - Mapping domain failures to an external protocol (status code, exit code, UI message) — that is the edge layer's job. - Catch-and-swallow, retry loops, backoff, timeout, circuit breaking around a single call — infrastructure, push down. - Cross-cutting telemetry / log / metric noise woven through every step — extract or push to a wrapper. - Check-and-return ladders that occupy more space than the business core — replace with signal + a handler above. ## Methodology — fixing a function that violates it Work top-down. Never start by shuffling lines. 1. **Name the level.** In one sentence, write what this function is for at its own layer. If you cannot, the function has no clear level — split before polishing. 2. **Classify every statement.** Tag each line or block as: **core** (this layer's business), **down** (a detail a lower abstraction should own), **up** (a concern an upper / edge layer should own), or **cross-cutting** (log / metric / retry). Unlabeled lines are where the mess hides — do not "just leave them". 3. **Decide down vs. up for each foreign item.** - Push **down** when it is a guarantee a lower building block can provide: a value that can only be constructed valid, a parser that returns a typed result, an infra helper that already retries. The business function then assumes validity and stays clean. - Push **up** when it is about translating or reacting to failure for the outside world: status codes, messages, exit codes, aggregation of many errors. The edge layer catches once and maps; business code just signals. - Rule of thumb: if removing it would change what the business rule says, it is core and stays; if removing it only changes how a failure is reported or a detail is computed, it moves. 4. **Extract, do not interleave.** Pull each foreign concern into its own named function or layer. Keep the original function as a readable sequence of same-level calls. For error handling specifically, separate the work body from the recovery body into distinct functions so neither clutters the other. 5. **Signal, do not handle, in the middle.** Mid-layer business functions throw / return and let the right layer react. Do not catch-and-log-and-continue in business code unless continuing is itself the business rule. 6. **Re-read for level.** After the moves, every remaining line should be explainable at the same altitude. If not, repeat from step 1. Keep the change minimal: move the smallest thing that restores the level. Do not invent abstractions, frameworks, or generic "handler" machinery beyond what the function actually needs. Three straight-line, same-level calls beat a premature pipeline. ## Review method — applying the lens to a diff Read each changed or touched function and, for each check, record only: **the hit (yes/no) plus evidence (`file:line`)**, and — where the check points at a construct — a raw factual count from "Quantify". 1. **Altitude check.** Are all lines at the same level of abstraction? Record each place where a "what" line is immediately followed by a "how" block (or vice versa) inside the same function, with `file:line`. 2. **Happy-path check.** Can you read the business intent top to bottom without stepping through error branches? Record whether error handling sits inline between business steps (yes/no + `file:line`), supported by raw counts from "Quantify" (e.g. number of `catch` clauses, `continue` statements). 3. **Ownership check.** For each validation, catch, mapping, log, retry: is this layer the rightful owner, or is it borrowed from above / below? Record each borrowed item with `file:line` and its destination (down / up), using the rules from the methodology. 4. **Layer-leak check.** Does a business function mention an external protocol (status code, exit code, UI text, wire field)? Does an edge function contain a business rule? Record each leak candidate with `file:line` and whether it names an *external* protocol or an *internal* domain shape. 5. **Explanation test.** Describe the function in one sentence with no "and". Record whether "and" was needed; if so, list the proposed split as candidate moves (down / up). ### Quantify — report only raw factual counts Report only quantities that can be counted **mechanically from the text**. Anything that first requires classifying a line (core vs foreign, happy-path vs error-handling, high-level vs low-level) is recorded under detection (the five checks above) as evidence, not as a number here. Report, per function: - **Body size** — lines and/or statements of the function body; state the basis (e.g. "statements, excluding lone braces"). - **Control-flow keywords (raw counts)** — `if`, `continue`, early `return`, `throw`, `try` / `catch` / `finally`, `await`, loops (`for` / `while` / `.forEach`). - **Named syntactic shapes a check points at** — when a check cites a construct, count it verbatim and name the exact token: e.g. number of object literals, string literals, `.trim()` calls, `.length` reads, `origin.` property reads, spread `[...x]` operations. - **Recovery presence (raw)** — number of `catch` clauses, and number of log / metric calls inside them. Quantities that embed a prior classification — out-of-level vs core counts, guard-to-core ratios, happy-path vs error-handling volume, "repeated boundary checks a lower layer could guarantee once", "low-level literals in a high-level flow" — are captured as evidence under the relevant check (`file:line` + the verbatim tokens). A downstream rubric derives any ratio from those raw facts. ### Red flags Record each as evidence (yes/no + `file:line`); these are candidates, not verdicts: - A body that is mostly check-and-return / check-and-throw ladders around a thin core. - A recovery block that logs, maps, and returns inline, sitting next to business steps. - A function that both computes a value and decides how that value's failure is shown to the user. - Low-level literals (byte offsets, header strings, format codes) inside a high-level workflow. - A name that needs "And" / "Or" / "With" to be honest, or a name so vague ("handle", "process", "do") that it hides multiple levels. - Catch-and-swallow that hides a failure the caller needed to see. - Defensive null / format checks repeated at every call site instead of guaranteed once at the boundary. ### Severity grading belongs downstream The agent's facts (detections, raw counts, directions, exemptions) feed a downstream grade; the agent reports those facts and stops there. Grades compress a continuous quantity into an uncalibrated three-point scale and are exactly where identical evidence gets labeled differently across runs. Grading happens above the agent: - A **deterministic rubric** — a versioned threshold table over the raw counts from "Quantify"; or - **Anchored examples** — the reviewer judges relative to repo-known reference functions rather than against an absolute adjective like "materially"; or - A **human**, for items that land near a threshold boundary. If a downstream consumer still asks the agent for a grade, the agent returns the underlying facts and the threshold band it would fall under, with `confidence: low` on boundary cases; the grade itself is produced downstream. ### How to report findings Report **evidence + direction**. Lead with the location and the level, then the proposed move. Prefer "this block is one level lower than the rest of the function (`file:line`) — move it **down** into X" over "this is ugly" or "this is a request-changes". The destination layer (down into a value / parser / infra helper, or up into the edge handler) is the actionable output and the deliverable. Attach the "Quantify" numbers and any exemption flags to each finding. ## Exemption checklist This is a lens, not a law. For each foreign concern, check whether any exemption below applies and **record the hit (yes/no) plus the reason**. The agent records exemptions as facts; a recorded exemption is then used downstream to cap the grade (e.g. to `Nit`) deterministically. - **Tiny function:** the function is small enough that splitting would add indirection with no reader benefit. - **Foreign concern is the single job:** the "foreign" concern is in fact the function's one purpose — a dedicated error mapper, a validator, an infra wrapper, or an index-bookkeeping helper whose low-level arithmetic *is* its level. - **Atomicity / correctness / performance:** the steps genuinely must stay together (e.g. a re-check after an `await` to guard state that may have changed). - **Edge-translator role:** an edge / handler function whose job is to translate an external event into internal indices; naming the wire fields is its job. Keep a split that would make the code harder to read as a recorded candidate for downstream review. When the evidence lands on an exemption boundary, record both sides and set `confidence: low`. ## Output contract Return, per function, items 1–5 only: 1. **Level statement** — one sentence: what the function is for at its own layer. 2. **Per-check results** — for each of the five review checks: `hit: yes/no`, evidence `file:line`, and (only where the check points at a construct) a raw factual count. 3. **Measurements** — the raw factual counts from "Quantify". 4. **Exemptions** — checklist hits (yes/no + reason). 5. **Proposed moves** — for each foreign concern: `file:line` → destination (down into X / up into Y). This is the actionable deliverable. Severity grades, block/merge verdicts, and "worth splitting" calls live downstream, derived from items 1–4. When a consumer asks for a label, hand back items 1–4 and the threshold band, with `confidence: low` on boundary cases.