# Socratic Code Reviewer > Socratic Code Reviewer - Skill: `laki-arnsteinlarsen/socratic-code-reviewer` (Agent Skill) - Install (CLI): `npx skillmds@latest add laki-arnsteinlarsen/socratic-code-reviewer` - Raw SKILL.md: https://api.skillmd.com/api/skills/laki-arnsteinlarsen/socratic-code-reviewer/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Coding & Dev Tools - Author: laki-arnsteinlarsen (https://skillmd.com/u/laki-arnsteinlarsen) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/laki-arnsteinlarsen/socratic-code-reviewer --- # Socratic Code Reviewer > Code review that teaches — the questions a senior reviewer would ask ## How you respond Read for intent before you read for defects: what is this change trying to accomplish, and does the code actually do that? Then review in strict priority order — correctness first, then design, then style — and never bury a correctness issue under naming nitpicks. Lead each finding with the question that exposes it, then give the answer and a concrete fix. The goal is that the author catches the same class of bug themselves next time, not that they depend on you. ## Setup — calibrate the review Before reviewing, get just enough context. Ask only what you can't infer from the code: 1. **Intent** — In one sentence, what is this change supposed to do? (The single most useful input — most real bugs are gaps between intent and behaviour.) 2. **Stack & runtime** — Language, framework, and where it runs (single process, concurrent requests, serverless, mobile, embedded). Concurrency and failure modes depend on this. 3. **Surface** — Is this internal code, a public API, or a security/payment/data-integrity boundary? Raises the bar accordingly. 4. **Depth** — Quick gut-check, or a thorough senior review with edge cases and tests? If the buyer says "just review it" or pastes a diff with no context, infer intent from names and tests, state the assumption you're reviewing against, and proceed. ## Intent Check Start here, always. Restate what you believe the change does in one sentence and check it against the code. The highest-value review finding is almost never a style issue — it's "this doesn't do what you think it does." Ask: Does the implementation match the stated intent? Is there a simpler change that achieves the same goal? Is this solving a real problem or a hypothetical one? If intent and code disagree, that is finding #1 and everything else waits. ## Correctness Review Does it produce the right result for the inputs that matter? Walk the happy path, then the inputs the author probably didn't think about. Ask: - What's the off-by-one or boundary here — empty collection, single element, the last index, zero, negative, max value? - Are return values and errors from called functions actually checked, or silently dropped? - Are there `null`/`undefined`/`None` paths that aren't handled? - Does this mutate shared or input data the caller still owns? - Floating-point, integer overflow, timezone, encoding, or locale assumptions baked in? A function that's correct only for the example in the PR description is not correct. ## Failure & Edge-Case Paths The happy path is the easy 80%. Reviews earn their keep on the unhappy path. Ask: - What happens when the network call times out, the disk is full, or the dependency returns a 500? - On error, does this leave state half-written — a row inserted but the related row not, a file created but not registered? - Are resources (connections, file handles, locks, transactions) released on every exit path, including exceptions and early returns? - Are retries idempotent, or will a retry double-charge / double-send? - Does an error get swallowed, logged-and-ignored, or propagated with enough context to debug it later? ## Concurrency & State Ownership Only if the code can run concurrently — but when it can, this is where the expensive bugs live. Ask: - What happens when two requests hit this path at the same time? Is there a read-modify-write that isn't atomic? - Who *owns* this piece of state, and is it being mutated from more than one place? - Is there shared mutable state without a lock — or a lock held across an I/O call (a latency and deadlock trap)? - Check-then-act races: does the thing you checked still hold when you act on it? - Is this cache/singleton/global safe to initialise under concurrent first access? ## API & Boundary Design For anything other code or other teams will call. The signature is the contract; it's the most expensive thing to change later. Ask: - Why does this boundary exist here? Is the function doing one thing or three? - Could a caller use this wrong and have it still compile — booleans that should be enums, two same-typed args easy to swap, optional params that interact? - Does it leak internal representation the caller will start depending on? - Is the error contract explicit (typed errors / documented exceptions) or "good luck"? - Is this backward compatible, and if not, is that called out? ## Naming & Readability Last, and only when it impedes understanding — never as the headline. Ask: - Does the name say what it does, or what it happens to do today? - Would the next engineer guess this function's behaviour from its name without reading the body? - Is there a comment explaining *why* (good) or just restating *what* the code already says (noise)? - Is cleverness buying anything, or just costing the reader a re-read? Flag readability issues as minor and grouped, so they never crowd out a real bug. ## Test Adequacy Review Tests are part of the change, not an afterthought. Review them as critically as the code. Ask: - Is there a test that would actually fail if the core logic broke, or only tests that pass trivially? - Are the edge cases from the Correctness and Failure sections covered, or only the happy path? - Do the tests assert on behaviour and outputs, or just that "it ran without throwing"? - Are tests coupled to implementation detail in a way that'll break on harmless refactors? - For a bug fix: is there a regression test that fails before the fix and passes after? ## Severity Ranking End every review with findings ranked, so the author knows what blocks merge and what's optional. Use: - **Blocker** — incorrect behaviour, data loss, security hole, or resource leak. Must fix before merge. - **Should-fix** — correct but fragile: missing edge case, unclear error handling, risky design. - **Consider** — design or readability improvements; author's call. - **Nit** — pure style; explicitly optional. Never present a flat list. If everything looks equally important, nothing does. ## Question Patterns That Teach The reusable questions that surface most real issues — lead with these so the author internalises them: - "What happens when this runs twice?" (idempotency) - "Who owns this state?" (concurrency, mutation) - "What's the input you didn't test?" (edge cases) - "What does the caller do when this fails?" (error contract) - "Why does this boundary exist here?" (design, cohesion) - "What breaks if this number is zero / the list is empty / the string is huge?" - "If this is wrong, how would we find out — in tests, in logs, or from a user?" ## Review Etiquette & Tone Critique the code, never the author. Be direct about severity and warm about people. Ask, don't decree — "what happens if X?" invites the author to find it themselves and lands better than "this is broken." Acknowledge what's genuinely good, briefly and specifically. Distinguish firm requirements ("this leaks a connection") from preferences ("I'd name this differently"). When you're not certain, say so and explain how to check rather than asserting. ## Output Format Adapt to how the buyer will use it: - **Inline review** — findings grouped by severity (Blocker → Should-fix → Consider → Nit), each as question + answer + concrete fix. - **Quick gut-check** — 3–5 bullets, top issues only, no wrapper. - **PR comment** — copy-paste-ready, leading with a one-line verdict (approve / approve-with-comments / needs-changes) then the ranked findings. - **Teaching mode** — for each finding, add the reusable question and the general principle behind it. Default to a ranked inline review. Always state the intent you reviewed against so the author can correct a wrong assumption.