--- name: gstack-plan description: "Use before building a feature: challenge the scope, then review architecture, code quality, tests and performance, and end with a plan to build. (gstack)" --- # Plan a feature: scope first, then the engineering review Review a plan, not code: do not build the feature. If the user has not given you the plan or feature description, ask for it and stop. **Start here.** In the repository (`cd` into it first: the shell starts in the workspace root), run with `execute` to see what already exists: ```sh git log --oneline -10 ls ``` Then read the files the plan touches. Work through Scope Challenge, then Sections 1-4 in order; for each finding give severity, a recommendation and the alternative. Ask about decisions that change scope in one message and stop until the user answers. Finish with the revised plan: what to build, in order, the files it touches, the tests that prove it, and what is NOT in scope. ## My engineering preferences (use these to guide your recommendations): * **Shared code:** require common behavior and improved reliability or net savings; similar-looking code alone is insufficient. * **Tests:** every behavior tested; no test without a regression it would catch. * **Enough engineering:** avoid fragility and premature abstraction/complexity. * **Edge cases:** thorough handling over speed. * **Explicit over clever.** * **Right-sized diff:** smallest clear change; rewrite a broken foundation when necessary. * **Data model:** storage follows the data's queries, integrity rules and update boundaries, not file count: fields queried, constrained or updated on their own get explicit columns or tables; JSON suits a document read and written whole. ## A. Assess the target Complete these checks before proposing scope changes. Do not write findings into the plan yet. - **What already solves each sub-problem?** Inspect helpers, libraries, callers and reusable outputs: behavior and dependency/deployment boundaries. Cite authored sources; label proposed callers with their motivating plan requirement and assumptions. - **What minimum changes achieve the goal?** Flag work deferrable without blocking it; challenge scope creep. - **Complexity check:** Count the selected work, not files read only as evidence: for a plan, its proposed changed files and new classes/services; for a diff, changed files and classes/services introduced by that diff; for a file/directory, files in that selected scope and any explicitly proposed new classes/services. Count each once, label estimates, and seek fewer moving parts. At 8+ files or 2+ new classes or services, propose a smaller arrangement and ask before changing scope. - **Distribution check:** For new artifacts, verify build/publish CI/CD, target OS/architectures and download/install channels. Put deferrals in "NOT in scope". ## Review Sections (after scope is agreed) Evaluate Architecture → Code Quality → Tests → Performance. Never condense, abbreviate or skip a section, including strategy/spec/infra plans. ### 1. Architecture review Evaluate: * System/component boundaries, dependencies and coupling. * Data flow, bottlenecks, scaling and single points of failure. * Security: auth, data access and API boundaries. * Key flows needing ASCII diagrams in plans/code. * One realistic production failure per new path/integration; does the plan handle it? * **Distribution architecture:** New artifacts' build, publish and update paths; included or deferred CI/CD. ### Two shapes before one (design-twice) Run this only when the plan introduces a new module, service, package or boundary: a public surface other code calls; otherwise say so in one line. **Ask for the second shape.** A plan with one design usually took the first shape that compiled in the author's head. Before judging it, require a second shape that is structurally different: another owner for the state, another split of responsibilities, another place for the boundary. A point fix inside the first shape is not a second candidate. When the plan has none, sketch one in five lines (types and signatures, bodies `not implemented`) and record it as the alternative. Compare the candidates on interface depth: how much behavior and policy each hides behind how small a public surface, and prefer the one that hides more behind less. A deep module is not a deep call chain: the chain scatters the understanding across layers, the module concentrates it behind one interface. **The eight red flags.** Screen the chosen shape before Section 1 closes. Assume the next contributor is an agent that sees only the files it opened, copies the nearest example and takes the shortest path that compiles; prefer the design where a change that looks right from one file is right for the whole repo. A flag is a finding with a file or symbol and a reason to revise the shape, not a comment asking for care. 1. **Shallow module.** A large interface over little hidden complexity: several calls per operation, options that expose internal stages. 2. **Information leakage.** One decision (a representation, a policy, a wire or storage detail) lives in more than one module. Re-exported transport types are leakage; parse external data into domain types at the boundary. 3. **Temporal decomposition.** Modules split by execution order (load, validate, transform, save) instead of by the knowledge they own. 4. **Pass-through method.** A function that forwards the same arguments to another with the same shape; remove it or move the responsibility. 5. **Split ownership.** Two modules write the same state or each keep a copy; an agent editing one cannot see the other. One owner per piece of state. 6. **Two ways to do one task.** An agent copies whichever it meets first. Keep one; move the callers and delete the rest in the same change. 7. **Importable internals.** The shortest path that compiles imports them and they become interface. Make an outside import fail the build. 8. **Hand-synced list.** The same items in two places; an agent that sees one list edits one. Derive the second, or fail the build when they disagree. Record under Architecture: both shapes, the pick, the depth argument for it, and each flag with its fix. ### 2. Code quality review Evaluate: * Organization and module structure. * Shared-code opportunities in the target and related callers. No standalone history/PR sweep or quotas. Check proposed caller assumptions against existing interfaces. * Explicitly flag error handling gaps and missing edge cases. * Technical debt, fragility and needless complexity per engineering preferences. * Accuracy of touched files' ASCII diagrams. ### 3. Test review For shared-code changes, audit existing/missing shared-contract tests (behavior, errors, side effects, boundaries) and each migrated caller's integration/differences. Rejected extractions still need coverage for real duplicated-code defects. For every new codepath, branch and error path in the plan, name the test that proves it (unit, integration or end-to-end) and the regression it would catch. Missing tests become plan items. ### 4. Performance review Evaluate: * N+1 queries and database access patterns. * Memory usage. * Caching opportunities. * Slow or complex paths. On per-request or looped paths, check queries in loops, unbounded queries or caches, missing indexes, whole-file loads and blocking calls without timeouts. Give each finding's scale (rows, requests/s, bytes) or mark it unknown; never invent benchmarks.