diff options
| -rw-r--r-- | .ai/workflows/INDEX.org | 7 | ||||
| -rw-r--r-- | .ai/workflows/spec-response.org | 138 | ||||
| -rw-r--r-- | .ai/workflows/spec-review.org | 208 | ||||
| -rw-r--r-- | claude-templates/.ai/workflows/INDEX.org | 7 | ||||
| -rw-r--r-- | claude-templates/.ai/workflows/spec-response.org | 138 | ||||
| -rw-r--r-- | claude-templates/.ai/workflows/spec-review.org | 208 |
6 files changed, 706 insertions, 0 deletions
diff --git a/.ai/workflows/INDEX.org b/.ai/workflows/INDEX.org index 756c6a9..bad2a48 100644 --- a/.ai/workflows/INDEX.org +++ b/.ai/workflows/INDEX.org @@ -66,6 +66,13 @@ This index must list every =.org= file in =.ai/workflows/= except this one. Star - =email-assembly.org= — gather documents into an email package. - Triggers: "assemble an email", "email assembly workflow", "gather documents for an email", "I need to send [person] some documents" +** Specs and design + +- =spec-review.org= — review a design/feature spec for implementation-readiness: run the readiness gate, read the code first, evaluate across dimensions, assign a rubric (Ready / Ready-with-caveats / Not-ready / Needs-research), and write a =<spec>-review.org= file when not ready. The *reviewer* side; its output feeds =spec-response.org=. + - Triggers: "review the spec", "is this spec implementation-ready?", "spec-review workflow", "review the design" +- =spec-response.org= — fold an external spec review back in: decide accept / modify / reject for every recommendation, weave accepts into the spec body, document modifies and rejects in a "Review dispositions" section, reconcile cross-spec tensions, iterate to implementation-ready. The *author* side; consumes the =<spec>-review.org= file =spec-review.org= produces. + - Triggers: "respond to the review", "process the spec reviews", "spec-response workflow", "fold in the review" + ** Tools and meta - =process-meeting-transcript.org= — record → transcript → labeled archive. diff --git a/.ai/workflows/spec-response.org b/.ai/workflows/spec-response.org new file mode 100644 index 0000000..2986d0d --- /dev/null +++ b/.ai/workflows/spec-response.org @@ -0,0 +1,138 @@ +#+TITLE: Spec-Response Workflow +#+AUTHOR: Craig Jennings +#+DATE: 2026-05-23 +#+STARTUP: showall + +* Overview + +The spec-response workflow processes external reviews of a design spec and folds them into the spec until it is implementation-ready. A reviewer (human or another agent) leaves a review file next to the spec — typically produced by its counterpart, the *spec-review* workflow, which writes =<spec-basename>-review.org= and assigns a readiness rubric. Claude works through every recommendation, deciding accept / modify / reject for each, updates the spec for the accepted ones, documents the modified and rejected ones with reasons, then deletes the review file. Repeat for each spec under review. + +The output is a spec a reader could implement from, plus a durable record — inside the spec — of why any recommendation was changed or declined, so the reviewer can find the reasoning later without re-litigating. + +This workflow was first run on 2026-05-23 against the linear-emacs =issue-query-spec.org= and =issue-representation-spec.org= reviews, and written up from that run. + +* Problem We're Solving + +A review is only useful if every point in it gets a decision. Without a defined process: + +- *Recommendations get dropped silently.* A skim picks up the obvious ones and loses the rest; the reviewer can't tell whether a missing change was rejected or forgotten. +- *Rejections leave no trace.* When a recommendation is declined for a good reason, that reason lives only in chat and evaporates. The next reviewer re-raises it. +- *Reviews get rubber-stamped.* Accepting everything uncritically is as bad as ignoring it — the point of the review is judgment, including the judgment to say no. +- *Cross-spec conflicts go unreconciled.* When two related specs are reviewed and the reviews pull in opposite directions, nobody resolves the tension and it surfaces during implementation. +- *The spec never converges.* Without an explicit "implementation-ready" bar, review cycles drift. + +*Impact:* slow convergence, lost rationale, re-litigated decisions, and specs that look reviewed but aren't decided. + +* Exit Criteria + +A spec's review is fully processed when: + +1. *Every recommendation has an explicit disposition* — accepted, modified, or rejected. None dropped. +2. *Accepted recommendations are woven into the spec body* — the spec reads as if they were always there, not appended as a changelog. +3. *Modified and rejected recommendations are documented in a bottom section* (e.g. "Review dispositions") with a one-paragraph reason each, so the reviewer can find the reasoning. +4. *Pre-agreed decisions are baked in as decisions*, moved out of "open questions." +5. *Cross-spec tensions are reconciled in writing* when related specs were reviewed together. +6. *The review file is deleted* once 1-5 hold. +7. *Tracking is updated* — the spec's VERIFY/task body notes "review incorporated" and whether it's implementation-ready. + +The whole run is done when no =*-review.org= files remain and each spec is judged implementation-ready (or its remaining blockers are named). + +*Measurable validation:* a reader scanning the review against the revised spec can find, for every review point, either the change in the body or its disposition at the bottom. Nothing is unaccounted for. + +* When to Use This Workflow + +Trigger when: + +- A reviewer drops a review file alongside a spec — convention: same basename with a =-review.org= suffix (=foo.org= → =foo-review.org=). +- Craig says "respond to the review" / "let's run the spec-response workflow" / "process the spec reviews." +- Any time a spec needs to absorb structured external feedback and converge to implementation-ready. + +Not for: drafting a spec from scratch (that's a design conversation), or informal inline comments (handle those conversationally / via the cj-comment flow). + +* Approach: How We Work Together + +** Phase 0: Orient + +1. List the review files (=ls docs/*-review.org= or wherever they live). Process them one at a time in whatever order; the user may name an order. +2. Re-read the *current* spec, not your memory of it — it may have changed since you wrote it (the user or a linter may have edited it, and pre-agreed decisions may already be encoded in the tracking file). +3. Note any *pre-agreed decisions* the reviewer or user has already settled — in the review's own "Agreed decisions" section, or in the spec's tracking task. These are settled inputs. Don't reopen them; bake them in. + +** Phase 1: Read the whole review before touching the spec + +Read the entire review first. Recommendations interact — an early "medium" finding may be subsumed by a "high" one, or two findings may point at the same edit. Decide dispositions with the whole picture in view, not finding-by-finding as you scroll. + +** Phase 2: Decide a disposition for every recommendation + +For each recommendation, choose one: + +- *Accept* — the recommendation is right as written. Plan the edit. +- *Modify* — the recommendation is right in spirit but wrong in detail or scope. Adjust it, and record what you changed and why. +- *Reject* — the recommendation doesn't fit. Record why. + +*Engage critically.* Rubber-stamping is a failure mode. On a strong review most points are accepts, but actively look for the genuine modify/reject cases — they are where your judgment earns its place. Examples from the first run: + +- *Modify:* the review proposed an automatic cache-TTL defcustom. Accepted the goal (fresh data) but deferred TTL to vNext because for a single-user tool an explicit force-refresh + clear-cache command covers it without invalidation complexity. +- *Reject:* the review floated a separate =default-issue-filter= defcustom. Rejected as redundant — a fixed default command plus a default-view preference already covered it. +- *Modify (scope):* "split representation from network into modules" — adopted the logical boundaries but kept a single file, because the package is a single-file MELPA target and a file split is a larger restructuring. + +*Honor pre-agreed decisions.* If the reviewer marked a decision "agreed" (or the user pre-encoded it), treat it as accepted and settled — move it from the spec's open questions into a decisions section. + +** Phase 3: Reconcile cross-spec tensions + +When related specs were reviewed together, two reviews can recommend opposite things. Don't pick one and silently drop the other — find the framing in which both hold. Example from the first run: one review said "switching views *replaces* the active file"; the other said "refresh by *merge*, not wholesale rewrite." Reconciliation: switching *source* replaces; refreshing the *same source* merges by stable ID with a per-item conflict check. Write the reconciliation into the spec. + +** Phase 4: Update the spec + +1. *Weave accepted recommendations into the body.* The spec should read naturally — a new "Selector semantics" section, a revised phase plan, an added test-strategy section — not a list of "review said X so I did Y." The body reflects the decisions; it doesn't narrate them. +2. *Add a bottom "Review dispositions" section* listing only the *modified* and *rejected* recommendations, each with a short reason. Close it with a one-line "everything else accepted as written" so the reader knows the omissions from this section are accepts, not gaps. +3. *Move settled questions out of "Open decisions"* into an "Agreed decisions" (or equivalent) section. Open decisions should contain only what genuinely still blocks. +4. *Raise the spec to implementation-ready:* consolidate decisions up front, add any implementation prerequisites the review surfaced (e.g. a schema-verification checklist), a consolidated test strategy, and a phased plan ordered so dependencies (like an output model everything depends on) come early. +5. *Update the status line* to note "review incorporated (<reviewer>, <date>)." + +** Phase 5: Close out and iterate + +1. *Delete the review file* — only after every recommendation has a disposition. Its deletion is the signal the review is fully processed. +2. *Update tracking* — the spec's VERIFY/task body gets a line noting review incorporated, what changed at a high level, which recommendations were modified (pointing at Review dispositions), and whether it's now implementation-ready pending final go. +3. *Update the session log* (state changed this turn). +4. *Move to the next review file.* Repeat Phases 1-5 until none remain. +5. *Report* what was accepted-wholesale, what was modified/rejected and why, any cross-spec reconciliations, and the implementation-ready verdict per spec. + +* Principles to Follow + +** Every recommendation gets a decision +Accept, modify, or reject — but never silently drop. The reviewer must be able to account for every point. + +** Document the no's, not the yes's +Accepted recommendations live in the spec body (the change *is* the record). Modified and rejected ones need an explicit written reason at the bottom, because the change is invisible and the reasoning would otherwise be lost. The asymmetry is deliberate. + +** Critique, don't rubber-stamp +A review you accept entirely without finding a single thing to push on probably wasn't read critically. Your judgment — including a well-reasoned no — is the value you add. + +** Pre-agreed decisions are settled inputs +Don't re-open what the reviewer or user already agreed. Bake it in and move on. + +** Reconcile, don't choose-and-drop +When reviews conflict, find the framing where both are right. Silently honoring one and ignoring the other reintroduces the conflict at implementation time. + +** A reject goes back to the reviewer, not just into the file +Recording a reasoned reject is the floor, not the close. Communicate the rejection and its reason to the reviewer — a reject is a two-party event, not a unilateral call. If the reviewer disagrees, that's a discussion: weigh the counter, and if you still can't agree, escalate to whoever owns the decision rather than letting the author's "no" stand by default. "I'm not doing that" with no reason the reviewer can engage is the failure mode. (For a tight solo author-reviewer loop this is lightweight; for a team it's the difference between a review and a rubber-stamp-in-reverse.) + +** The spec reads forward, the dispositions read backward +The body is written for the implementer (no review archaeology). The dispositions section is written for the reviewer (the reasoning trail). Keep the two audiences separate. + +** Re-read before editing +The spec may have changed since you last saw it. Edit the current file, reconcile against the latest tracking state. + +** Converge to implementation-ready +The bar is "a reader could implement from this." Decisions consolidated, prerequisites named, tests strategized, phases ordered by dependency. The loop terminates when the spec-review rubric reaches =Ready= (or =Ready with caveats= the author accepts) and every blocking, high-priority finding has a disposition — not when you run out of energy. Iterate toward that bar, not in circles. + +* Living Document + +Update this workflow as we learn what works. Capture new disposition patterns, better reconciliation examples, and refinements to the implementation-ready bar. + +** Updates and Learnings + +*** 2026-05-23: First run — two coupled specs, one reviewer +*Learning:* The reviews were high quality and mostly accepts, which made the genuine modify/reject cases the high-value work — and the cross-spec reconciliation (replace-on-switch vs merge-on-refresh) was the single most useful synthesis, something neither review could do alone. The "Review dispositions" section closing with "everything else accepted as written" proved important: without it, a reader can't tell whether an unlisted recommendation was accepted or missed. + +*Open question for next run:* whether to keep dispositions inside each spec (current approach) or in a shared response doc when many specs are reviewed at once. Inside-the-spec kept the reasoning next to the affected text, which read well for two specs. diff --git a/.ai/workflows/spec-review.org b/.ai/workflows/spec-review.org new file mode 100644 index 0000000..2bdcf73 --- /dev/null +++ b/.ai/workflows/spec-review.org @@ -0,0 +1,208 @@ +#+TITLE: Spec-Review Workflow +#+AUTHOR: Craig Jennings +#+DATE: 2026-05-23 +#+STARTUP: showall + +* Overview + +The spec-review workflow evaluates a feature/specification document before implementation and decides one thing: can an engineer implement it confidently, test it thoroughly, and ship behavior that matches the user's mental model? If yes, say so and stop. If no, write a review file next to the spec naming every blocking gap and the concrete change that closes it. + +This is the *reviewer* side of a pair. Its counterpart is the spec-response workflow, which the spec's author runs to fold a review back in. The contract between them is the review file: =<spec-basename>-review.org= (e.g. =docs/issue-query-spec.org= → =docs/issue-query-spec-review.org=). spec-review produces it; spec-response consumes it. + +The goal is not to prove the spec is clever. It is to leave the implementer with *fewer* hidden decisions, not more prose. + +* Problem We're Solving + +Without a disciplined review before implementation: + +- *Specs look finished but force implementers to invent product behavior.* Ownership, conflict, empty-state, and error semantics get discovered (and decided ad hoc) mid-implementation, where they're expensive to change. +- *API assumptions ship unverified.* A spec quietly depends on a field/mutation/enum shape that turns out wrong, and the work stalls at integration. +- *Reviews are vague or sprawling.* "Add tests," "consider edge cases," or a full rewrite of the spec — none of which an implementer can act on. +- *Deferred decisions live only in chat.* The next session re-litigates them. + +*Impact:* viability and consistency problems that should surface in review instead surface during implementation, when they cost the most. + +* Exit Criteria + +A review is complete when: + +1. *The implementation-readiness gate has been evaluated* and a rubric label assigned (=Ready= / =Ready with caveats= / =Not ready= / =Needs research=). +2. *If ready:* the user is told plainly ("This spec is implementation-ready. I have no further blocking review notes."), and the review stops — no churn for its own sake. +3. *If not ready:* a =<spec>-review.org= file is written next to the spec, in the standard structure, with every finding specific and actionable (current behavior named, risk explained, change recommended, blocking-or-not stated). +4. *Deferred work is logged* to =todo.org= (v1 = =[#B]=, vNext/someday = =[#D]=), not left only in chat. + +*Measurable validation:* after reading the review, an implementer knows exactly what to decide or change before starting, and an author running spec-response can give every finding an accept/modify/reject without guessing what it means. + +* When to Use This Workflow + +Trigger when: + +- A spec is up for implementation and someone needs to know if it's ready. +- A spec changes materially (significant design conclusions changed → fresh review round). +- Craig says "review the spec" / "is this spec implementation-ready?" / "let's run the spec-review workflow." + +Run it *early* — design review exists to catch viability problems and costly mistakes before implementation, not after. + +* Approach: How We Work Together + +** Phase 1: First-pass readiness gate + +This is a fast triage on the spec's face — can it be rejected as obviously not ready before you invest in the full read? It is not the final word: several items below (architecture fit, API verification, integration points) can only be judged after Phase 2's code read, so Phase 3 re-runs this gate as the authoritative pass. Here, decide what you already can, and flag the rest to confirm after reading. + +Mark the spec implementation-ready only if *all* of these hold: + +- The user problem and target behavior are clear. +- Scope is explicit: v1, out-of-scope, and vNext are separated. +- All prior review questions/recommendations are answered or intentionally deferred. +- The expected UX is concrete enough to implement without guessing. +- The data model, state ownership, persistence, and sync behavior are defined. +- Error, conflict, empty-state, and partial-failure behavior are defined. +- Security and privacy behavior is defined wherever the feature touches credentials, personal or shared data, or external services — or there is nothing sensitive to define and that's stated. +- Observability is defined: how the user (or operator) sees what the feature is doing and how failures surface — or the feature is simple enough not to need it, and that's stated. +- Performance expectations and likely bottlenecks are considered. +- The architecture fits the existing codebase and names its integration points. +- The test strategy covers unit, integration/fixture, regression, and user-flow as appropriate. +- The plan can be phased without shipping broken intermediate states. +- External API assumptions are verified or explicitly listed as prerequisites. + +If all true → tell the user it's ready and stop unless they ask for more. If any false → continue and write the review file. A "ready" at this phase is provisional; confirm it at Phase 3 after the code read. + +** Phase 2: Required reading order + +Never review a spec in isolation. + +1. *Read the existing implementation first.* The code paths the spec would touch: public commands and entry points, internal helpers/boundaries, current data representation, persistence/write-back, async/sync, caching, error handling, existing tests, naming/style. Capture current-state facts with function names and file paths. Don't recommend designs that ignore how the package works today. +2. *Read related specs and task tracking.* Companion specs, relevant =todo.org= tasks, README/testing docs, prior review files. Record which tasks the spec absorbs, which stay separate, which decisions are already made, which are still open. +3. *Read the target spec end to end — twice.* First for its problem/behavior/phases/assumptions; second looking only for gaps. The second read asks: "What would an implementer still have to invent?" + +** Phase 3: Re-run the gate (authoritative) + +After reading code and spec, re-run the Phase 1 gate — this is the pass that counts, because now you can actually judge the items that needed the code: architecture fit, API verification, integration points. If now ready, don't manufacture churn. If not, write the review file. + +** Phase 4: Evaluate across dimensions + +Work the spec against these. Each is a source of concrete findings, not a box to tick. + +- *User problem & mental model.* Real problem or just machinery? Does UX match how users think? Predictable names/commands/prompts/layouts? Is editable-vs-generated content obvious? Are destructive/workspace-mutating actions explicit and confirmed? For org features, be *strict about ownership* — users treat visible org text as editable unless told otherwise. +- *Scope & boundaries.* What's v1 / out-of-scope / vNext? Are deferrals captured (=todo.org=)? Does the phased plan avoid half-working states? If a decision is deferred, is v1 still coherent without it? +- *Requirements clarity.* Each requirement unambiguous, testable, traceable to a source (user ask / bug / API capability / decision)? Are "good/fast/intuitive/smart/seamless" replaced with concrete behavior? Terms used consistently? Infeasible/unverified requirements flagged? +- *Data model & ownership.* Durable IDs vs display-only names? What's local/remote/generated/cached/user-authored? Who owns each editable region? What's replaced on refresh, survives view-switch / restart? What if one remote object could appear in multiple local places? Prefer stable IDs for execution, names for display; don't overload generic org properties (=:ID:=) for app-specific remote IDs unless intentional. +- *Sync, refresh, conflict.* What triggers sync (command / save hook / state hook / timer) and refresh? One-way or bidirectional? No-op detection? Remote-conflict detection? v1 conflict behavior? Dirty-buffer behavior? API success but GraphQL-level failure? Network failure mid-pagination/mutation? For v1, simple is fine if explicit: detect, refuse, tell the user what to do next. +- *API & external dependencies.* Which fields/mutations/filters/enums are assumed, and are they verified against current schema/docs/live responses? Permission differences, rate limits, pagination, nulls, partial errors handled? Any optional external tool — what's the fallback? Don't let a spec silently depend on an unverified API shape. +- *Architecture & maintainability.* Fits current module boundaries, or should new ones precede growth? Which parts are pure vs side-effecting? Are transport/parsing/normalization/rendering/command-orchestration separated enough to test? Will it multiply special cases? Backward-compat preserved where needed? Favor small pure cores surrounded by thin IO/command layers. +- *Robustness & error handling.* Expected failure modes? Errors distinguishable from empty-but-successful results? Actionable messages? Retries? Stale/missing caches? Can malformed local files or missing properties crash commands? nil/null/missing-field behavior defined? Data-loss-by-default avoided? Anything that writes files or remote data needs a clear data-loss story. +- *Performance & scale.* Expected counts (issues/comments/labels/teams/projects/views)? Server-side filtering where possible? Bounded, visible pagination? Cached name→ID lookups? Sync calls in the command path acceptable? Could a save hook or whole-file scan make N network calls? Rendering linear? Full-file rewrites avoided? Identify UI freezes, repeated network calls, unbounded pagination — without premature optimization. +- *Security & privacy.* API keys safe? Debug logs leaking secrets or private issue text? Confirmations before mutating shared workspace objects? Personal vs shared distinguished? Local files holding sensitive descriptions/comments? Anything to redact from messages/logs? Any work-tracker integration may handle private company data. +- *UX & accessibility.* Discoverable commands? Recoverable mistakes? Prompts ordered to the task? Safe, useful defaults? Informative-not-noisy status messages? Does the UI avoid implying unsupported actions are supported? Match the upstream product's permissions/concepts? For Emacs packages, command names, completion candidates, buffer layout, and message wording *are* the UX. +- *Test strategy.* Characterization tests before behavior changes? Pure functions to unit-test? API responses needing fixtures? Command flows needing stubs? Regression tests for prior bugs? Boundary/error cases? What's covered elsewhere and shouldn't be re-tested? Which existing tests must change? Prefer tests that lock contracts: representation shape, query compilation, sync no-op, conflict refusal, pagination, dirty-buffer protection. +- *Observability & operations.* How does a user see what the package is doing? Progress messages for long ops? Useful, safe debug logging? A way to inspect/clear caches? Verify setup/API connectivity? For generated org files, headers should often carry source, filter/view name, refresh time, count, truncation state. +- *Documentation & migration.* README/config changes? New commands and defcustoms documented? Old behaviors removed/renamed/migrated? Breaking changes acceptable? Changelog note needed? Don't ship with stale examples. + +** Phase 5: Write the review file + +Use this structure for =<spec-basename>-review.org= unless the spec calls for something different: + +#+begin_src org +,#+TITLE: Review: <Spec Title> +,#+AUTHOR: <reviewer> +,#+DATE: <date> +,#+STARTUP: showall + +,* Scope reviewed +What code, tests, docs, and specs you read. + +,* Implementation-readiness +Whether the spec is ready. If not, summarize the blockers. + +,* Overall assessment +The short senior-engineering read: what's right, what's risky, what must be clarified. + +,* High-priority findings +Concrete headings. Each: why it matters and what to change. + +,* Medium-priority findings +Important improvements that shouldn't block all progress. + +,* UX observations +,* Architecture observations +,* Robustness and performance observations +,* Test strategy recommendations +Specific test cases, not generic "add tests". + +,* Suggested spec edits +Concrete edits to make the spec implementation-ready. + +,* Agreed decisions +Answers reached during review. Omit if none. + +,* Open questions +Only questions that truly block or materially affect implementation. + +,* vNext candidates +Deferred features to capture in task tracking. +#+end_src + +** Phase 6: Assign the rubric and update tracking + +Assign one label consistently: + +- =Ready= — no blocking open questions; implementation can start. +- =Ready with caveats= — can start if the caveats are accepted and tracked. +- =Not ready= — blocking ambiguity / missing decisions would force implementers to invent product behavior. +- =Needs research= — external API/library/platform assumptions must be verified first. + +The most useful reviews move a spec from =Not ready= to =Ready with caveats= or =Ready= once decisions are captured. + +Finding severity maps to blocking power: *high-priority findings block =Ready=* — they hold the rubric at =Not ready= (or =Ready with caveats= if the author accepts and tracks them) until dispositioned; *medium-priority findings are the author's discretion* and don't block. State the blocking status on each finding so the author running spec-response knows which ones gate the rubric. + +Then log deferred work to =todo.org=: v1 implementation = =[#B]= (unless urgent or speculative); vNext/someday = =[#D]=. Tag =:feature:= / =:bug:= / =:refactor:= / =:test:= / =:quick:= / =:solo:= only when accurate. Don't leave important deferred decisions only in chat. + +* Principles to Follow + +** Read the code before critiquing the spec +A recommendation that ignores how the package works today is noise. Cite function names and file paths; ground findings in current behavior. + +** Specific over vague +Name the current behavior or spec text, explain the risk, recommend the change, say whether it blocks. Avoid vague preferences, whole-spec rewrites, architecture astronautics, unbounded "consider" lists, and questions whose answer doesn't change implementation. + +Model finding: +#+begin_quote +The spec says fetched comments render as subheadings but doesn't define whether editing an existing comment syncs back. That's risky because Linear only allows users to edit their own comments. V1 should treat fetched comments as remote-owned display content and support only adding new comments; editing own comments can be vNext. +#+end_quote + +** Decide readiness first, critique second +The gate is the product of the review. A long critique on a spec that's actually ready just creates churn; a "looks fine" on one that isn't ships hidden decisions. + +** Be strict about ownership +Especially for org-mode features: a user treats visible text as editable unless the representation says otherwise. Make generated-vs-editable explicit. + +** Never depend on an unverified API shape +If the spec assumes fields/mutations/enums, they're verified against current schema/docs/live responses, or listed as a research prerequisite. =Needs research= is a real, useful verdict. + +** Favor small pure cores and thin IO layers +Push findings toward separable, unit-testable pure functions surrounded by thin command/transport layers. + +** Block only on answer-changing questions +Open questions in the review are the ones whose answer materially changes implementation. Everything else is a suggestion, not a blocker. + +* Living Document + +Update this workflow as review practice sharpens — new dimensions worth checking, better model findings, refinements to the readiness gate and rubric. + +* Research notes behind this checklist + +Combines the review process used on the linear-emacs issue query/representation specs with broader design/spec-review practice: + +- Design reviews should happen early enough to find viability problems and costly mistakes before implementation (Google design-review research). +- Product/spec docs should create shared understanding — goals, assumptions, and an explicit "what we're not doing" (Atlassian PRD guidance). +- Review sessions should capture open questions and gaps so they're tracked and resolved (Atlassian design-review template). +- Requirements should be checked for correctness, ambiguity, completeness, consistency, verifiability, traceability, feasibility (SRS checklist categories). +- Design-doc review should be iterative but restart/refocus when significant conclusions change (Gerrit design-doc guidance). + +Sources: + +- [[https://research.google/pubs/improving-design-reviews-at-google/][Google Research: Improving Design Reviews at Google]] +- [[https://www.atlassian.com/agile/product-management/requirements][Atlassian: Product Requirements Documents]] +- [[https://www.atlassian.com/software/confluence/templates/design-review][Atlassian: Design Review Template]] +- [[https://www.geeksforgeeks.org/software-engineering/software-requirement-specification-srs-document-checklist/][GeeksforGeeks: SRS Document Checklist]] +- [[https://gerrit-review.googlesource.com/Documentation/dev-design-docs.html][Gerrit: Design Docs Review Process]] diff --git a/claude-templates/.ai/workflows/INDEX.org b/claude-templates/.ai/workflows/INDEX.org index 756c6a9..bad2a48 100644 --- a/claude-templates/.ai/workflows/INDEX.org +++ b/claude-templates/.ai/workflows/INDEX.org @@ -66,6 +66,13 @@ This index must list every =.org= file in =.ai/workflows/= except this one. Star - =email-assembly.org= — gather documents into an email package. - Triggers: "assemble an email", "email assembly workflow", "gather documents for an email", "I need to send [person] some documents" +** Specs and design + +- =spec-review.org= — review a design/feature spec for implementation-readiness: run the readiness gate, read the code first, evaluate across dimensions, assign a rubric (Ready / Ready-with-caveats / Not-ready / Needs-research), and write a =<spec>-review.org= file when not ready. The *reviewer* side; its output feeds =spec-response.org=. + - Triggers: "review the spec", "is this spec implementation-ready?", "spec-review workflow", "review the design" +- =spec-response.org= — fold an external spec review back in: decide accept / modify / reject for every recommendation, weave accepts into the spec body, document modifies and rejects in a "Review dispositions" section, reconcile cross-spec tensions, iterate to implementation-ready. The *author* side; consumes the =<spec>-review.org= file =spec-review.org= produces. + - Triggers: "respond to the review", "process the spec reviews", "spec-response workflow", "fold in the review" + ** Tools and meta - =process-meeting-transcript.org= — record → transcript → labeled archive. diff --git a/claude-templates/.ai/workflows/spec-response.org b/claude-templates/.ai/workflows/spec-response.org new file mode 100644 index 0000000..2986d0d --- /dev/null +++ b/claude-templates/.ai/workflows/spec-response.org @@ -0,0 +1,138 @@ +#+TITLE: Spec-Response Workflow +#+AUTHOR: Craig Jennings +#+DATE: 2026-05-23 +#+STARTUP: showall + +* Overview + +The spec-response workflow processes external reviews of a design spec and folds them into the spec until it is implementation-ready. A reviewer (human or another agent) leaves a review file next to the spec — typically produced by its counterpart, the *spec-review* workflow, which writes =<spec-basename>-review.org= and assigns a readiness rubric. Claude works through every recommendation, deciding accept / modify / reject for each, updates the spec for the accepted ones, documents the modified and rejected ones with reasons, then deletes the review file. Repeat for each spec under review. + +The output is a spec a reader could implement from, plus a durable record — inside the spec — of why any recommendation was changed or declined, so the reviewer can find the reasoning later without re-litigating. + +This workflow was first run on 2026-05-23 against the linear-emacs =issue-query-spec.org= and =issue-representation-spec.org= reviews, and written up from that run. + +* Problem We're Solving + +A review is only useful if every point in it gets a decision. Without a defined process: + +- *Recommendations get dropped silently.* A skim picks up the obvious ones and loses the rest; the reviewer can't tell whether a missing change was rejected or forgotten. +- *Rejections leave no trace.* When a recommendation is declined for a good reason, that reason lives only in chat and evaporates. The next reviewer re-raises it. +- *Reviews get rubber-stamped.* Accepting everything uncritically is as bad as ignoring it — the point of the review is judgment, including the judgment to say no. +- *Cross-spec conflicts go unreconciled.* When two related specs are reviewed and the reviews pull in opposite directions, nobody resolves the tension and it surfaces during implementation. +- *The spec never converges.* Without an explicit "implementation-ready" bar, review cycles drift. + +*Impact:* slow convergence, lost rationale, re-litigated decisions, and specs that look reviewed but aren't decided. + +* Exit Criteria + +A spec's review is fully processed when: + +1. *Every recommendation has an explicit disposition* — accepted, modified, or rejected. None dropped. +2. *Accepted recommendations are woven into the spec body* — the spec reads as if they were always there, not appended as a changelog. +3. *Modified and rejected recommendations are documented in a bottom section* (e.g. "Review dispositions") with a one-paragraph reason each, so the reviewer can find the reasoning. +4. *Pre-agreed decisions are baked in as decisions*, moved out of "open questions." +5. *Cross-spec tensions are reconciled in writing* when related specs were reviewed together. +6. *The review file is deleted* once 1-5 hold. +7. *Tracking is updated* — the spec's VERIFY/task body notes "review incorporated" and whether it's implementation-ready. + +The whole run is done when no =*-review.org= files remain and each spec is judged implementation-ready (or its remaining blockers are named). + +*Measurable validation:* a reader scanning the review against the revised spec can find, for every review point, either the change in the body or its disposition at the bottom. Nothing is unaccounted for. + +* When to Use This Workflow + +Trigger when: + +- A reviewer drops a review file alongside a spec — convention: same basename with a =-review.org= suffix (=foo.org= → =foo-review.org=). +- Craig says "respond to the review" / "let's run the spec-response workflow" / "process the spec reviews." +- Any time a spec needs to absorb structured external feedback and converge to implementation-ready. + +Not for: drafting a spec from scratch (that's a design conversation), or informal inline comments (handle those conversationally / via the cj-comment flow). + +* Approach: How We Work Together + +** Phase 0: Orient + +1. List the review files (=ls docs/*-review.org= or wherever they live). Process them one at a time in whatever order; the user may name an order. +2. Re-read the *current* spec, not your memory of it — it may have changed since you wrote it (the user or a linter may have edited it, and pre-agreed decisions may already be encoded in the tracking file). +3. Note any *pre-agreed decisions* the reviewer or user has already settled — in the review's own "Agreed decisions" section, or in the spec's tracking task. These are settled inputs. Don't reopen them; bake them in. + +** Phase 1: Read the whole review before touching the spec + +Read the entire review first. Recommendations interact — an early "medium" finding may be subsumed by a "high" one, or two findings may point at the same edit. Decide dispositions with the whole picture in view, not finding-by-finding as you scroll. + +** Phase 2: Decide a disposition for every recommendation + +For each recommendation, choose one: + +- *Accept* — the recommendation is right as written. Plan the edit. +- *Modify* — the recommendation is right in spirit but wrong in detail or scope. Adjust it, and record what you changed and why. +- *Reject* — the recommendation doesn't fit. Record why. + +*Engage critically.* Rubber-stamping is a failure mode. On a strong review most points are accepts, but actively look for the genuine modify/reject cases — they are where your judgment earns its place. Examples from the first run: + +- *Modify:* the review proposed an automatic cache-TTL defcustom. Accepted the goal (fresh data) but deferred TTL to vNext because for a single-user tool an explicit force-refresh + clear-cache command covers it without invalidation complexity. +- *Reject:* the review floated a separate =default-issue-filter= defcustom. Rejected as redundant — a fixed default command plus a default-view preference already covered it. +- *Modify (scope):* "split representation from network into modules" — adopted the logical boundaries but kept a single file, because the package is a single-file MELPA target and a file split is a larger restructuring. + +*Honor pre-agreed decisions.* If the reviewer marked a decision "agreed" (or the user pre-encoded it), treat it as accepted and settled — move it from the spec's open questions into a decisions section. + +** Phase 3: Reconcile cross-spec tensions + +When related specs were reviewed together, two reviews can recommend opposite things. Don't pick one and silently drop the other — find the framing in which both hold. Example from the first run: one review said "switching views *replaces* the active file"; the other said "refresh by *merge*, not wholesale rewrite." Reconciliation: switching *source* replaces; refreshing the *same source* merges by stable ID with a per-item conflict check. Write the reconciliation into the spec. + +** Phase 4: Update the spec + +1. *Weave accepted recommendations into the body.* The spec should read naturally — a new "Selector semantics" section, a revised phase plan, an added test-strategy section — not a list of "review said X so I did Y." The body reflects the decisions; it doesn't narrate them. +2. *Add a bottom "Review dispositions" section* listing only the *modified* and *rejected* recommendations, each with a short reason. Close it with a one-line "everything else accepted as written" so the reader knows the omissions from this section are accepts, not gaps. +3. *Move settled questions out of "Open decisions"* into an "Agreed decisions" (or equivalent) section. Open decisions should contain only what genuinely still blocks. +4. *Raise the spec to implementation-ready:* consolidate decisions up front, add any implementation prerequisites the review surfaced (e.g. a schema-verification checklist), a consolidated test strategy, and a phased plan ordered so dependencies (like an output model everything depends on) come early. +5. *Update the status line* to note "review incorporated (<reviewer>, <date>)." + +** Phase 5: Close out and iterate + +1. *Delete the review file* — only after every recommendation has a disposition. Its deletion is the signal the review is fully processed. +2. *Update tracking* — the spec's VERIFY/task body gets a line noting review incorporated, what changed at a high level, which recommendations were modified (pointing at Review dispositions), and whether it's now implementation-ready pending final go. +3. *Update the session log* (state changed this turn). +4. *Move to the next review file.* Repeat Phases 1-5 until none remain. +5. *Report* what was accepted-wholesale, what was modified/rejected and why, any cross-spec reconciliations, and the implementation-ready verdict per spec. + +* Principles to Follow + +** Every recommendation gets a decision +Accept, modify, or reject — but never silently drop. The reviewer must be able to account for every point. + +** Document the no's, not the yes's +Accepted recommendations live in the spec body (the change *is* the record). Modified and rejected ones need an explicit written reason at the bottom, because the change is invisible and the reasoning would otherwise be lost. The asymmetry is deliberate. + +** Critique, don't rubber-stamp +A review you accept entirely without finding a single thing to push on probably wasn't read critically. Your judgment — including a well-reasoned no — is the value you add. + +** Pre-agreed decisions are settled inputs +Don't re-open what the reviewer or user already agreed. Bake it in and move on. + +** Reconcile, don't choose-and-drop +When reviews conflict, find the framing where both are right. Silently honoring one and ignoring the other reintroduces the conflict at implementation time. + +** A reject goes back to the reviewer, not just into the file +Recording a reasoned reject is the floor, not the close. Communicate the rejection and its reason to the reviewer — a reject is a two-party event, not a unilateral call. If the reviewer disagrees, that's a discussion: weigh the counter, and if you still can't agree, escalate to whoever owns the decision rather than letting the author's "no" stand by default. "I'm not doing that" with no reason the reviewer can engage is the failure mode. (For a tight solo author-reviewer loop this is lightweight; for a team it's the difference between a review and a rubber-stamp-in-reverse.) + +** The spec reads forward, the dispositions read backward +The body is written for the implementer (no review archaeology). The dispositions section is written for the reviewer (the reasoning trail). Keep the two audiences separate. + +** Re-read before editing +The spec may have changed since you last saw it. Edit the current file, reconcile against the latest tracking state. + +** Converge to implementation-ready +The bar is "a reader could implement from this." Decisions consolidated, prerequisites named, tests strategized, phases ordered by dependency. The loop terminates when the spec-review rubric reaches =Ready= (or =Ready with caveats= the author accepts) and every blocking, high-priority finding has a disposition — not when you run out of energy. Iterate toward that bar, not in circles. + +* Living Document + +Update this workflow as we learn what works. Capture new disposition patterns, better reconciliation examples, and refinements to the implementation-ready bar. + +** Updates and Learnings + +*** 2026-05-23: First run — two coupled specs, one reviewer +*Learning:* The reviews were high quality and mostly accepts, which made the genuine modify/reject cases the high-value work — and the cross-spec reconciliation (replace-on-switch vs merge-on-refresh) was the single most useful synthesis, something neither review could do alone. The "Review dispositions" section closing with "everything else accepted as written" proved important: without it, a reader can't tell whether an unlisted recommendation was accepted or missed. + +*Open question for next run:* whether to keep dispositions inside each spec (current approach) or in a shared response doc when many specs are reviewed at once. Inside-the-spec kept the reasoning next to the affected text, which read well for two specs. diff --git a/claude-templates/.ai/workflows/spec-review.org b/claude-templates/.ai/workflows/spec-review.org new file mode 100644 index 0000000..2bdcf73 --- /dev/null +++ b/claude-templates/.ai/workflows/spec-review.org @@ -0,0 +1,208 @@ +#+TITLE: Spec-Review Workflow +#+AUTHOR: Craig Jennings +#+DATE: 2026-05-23 +#+STARTUP: showall + +* Overview + +The spec-review workflow evaluates a feature/specification document before implementation and decides one thing: can an engineer implement it confidently, test it thoroughly, and ship behavior that matches the user's mental model? If yes, say so and stop. If no, write a review file next to the spec naming every blocking gap and the concrete change that closes it. + +This is the *reviewer* side of a pair. Its counterpart is the spec-response workflow, which the spec's author runs to fold a review back in. The contract between them is the review file: =<spec-basename>-review.org= (e.g. =docs/issue-query-spec.org= → =docs/issue-query-spec-review.org=). spec-review produces it; spec-response consumes it. + +The goal is not to prove the spec is clever. It is to leave the implementer with *fewer* hidden decisions, not more prose. + +* Problem We're Solving + +Without a disciplined review before implementation: + +- *Specs look finished but force implementers to invent product behavior.* Ownership, conflict, empty-state, and error semantics get discovered (and decided ad hoc) mid-implementation, where they're expensive to change. +- *API assumptions ship unverified.* A spec quietly depends on a field/mutation/enum shape that turns out wrong, and the work stalls at integration. +- *Reviews are vague or sprawling.* "Add tests," "consider edge cases," or a full rewrite of the spec — none of which an implementer can act on. +- *Deferred decisions live only in chat.* The next session re-litigates them. + +*Impact:* viability and consistency problems that should surface in review instead surface during implementation, when they cost the most. + +* Exit Criteria + +A review is complete when: + +1. *The implementation-readiness gate has been evaluated* and a rubric label assigned (=Ready= / =Ready with caveats= / =Not ready= / =Needs research=). +2. *If ready:* the user is told plainly ("This spec is implementation-ready. I have no further blocking review notes."), and the review stops — no churn for its own sake. +3. *If not ready:* a =<spec>-review.org= file is written next to the spec, in the standard structure, with every finding specific and actionable (current behavior named, risk explained, change recommended, blocking-or-not stated). +4. *Deferred work is logged* to =todo.org= (v1 = =[#B]=, vNext/someday = =[#D]=), not left only in chat. + +*Measurable validation:* after reading the review, an implementer knows exactly what to decide or change before starting, and an author running spec-response can give every finding an accept/modify/reject without guessing what it means. + +* When to Use This Workflow + +Trigger when: + +- A spec is up for implementation and someone needs to know if it's ready. +- A spec changes materially (significant design conclusions changed → fresh review round). +- Craig says "review the spec" / "is this spec implementation-ready?" / "let's run the spec-review workflow." + +Run it *early* — design review exists to catch viability problems and costly mistakes before implementation, not after. + +* Approach: How We Work Together + +** Phase 1: First-pass readiness gate + +This is a fast triage on the spec's face — can it be rejected as obviously not ready before you invest in the full read? It is not the final word: several items below (architecture fit, API verification, integration points) can only be judged after Phase 2's code read, so Phase 3 re-runs this gate as the authoritative pass. Here, decide what you already can, and flag the rest to confirm after reading. + +Mark the spec implementation-ready only if *all* of these hold: + +- The user problem and target behavior are clear. +- Scope is explicit: v1, out-of-scope, and vNext are separated. +- All prior review questions/recommendations are answered or intentionally deferred. +- The expected UX is concrete enough to implement without guessing. +- The data model, state ownership, persistence, and sync behavior are defined. +- Error, conflict, empty-state, and partial-failure behavior are defined. +- Security and privacy behavior is defined wherever the feature touches credentials, personal or shared data, or external services — or there is nothing sensitive to define and that's stated. +- Observability is defined: how the user (or operator) sees what the feature is doing and how failures surface — or the feature is simple enough not to need it, and that's stated. +- Performance expectations and likely bottlenecks are considered. +- The architecture fits the existing codebase and names its integration points. +- The test strategy covers unit, integration/fixture, regression, and user-flow as appropriate. +- The plan can be phased without shipping broken intermediate states. +- External API assumptions are verified or explicitly listed as prerequisites. + +If all true → tell the user it's ready and stop unless they ask for more. If any false → continue and write the review file. A "ready" at this phase is provisional; confirm it at Phase 3 after the code read. + +** Phase 2: Required reading order + +Never review a spec in isolation. + +1. *Read the existing implementation first.* The code paths the spec would touch: public commands and entry points, internal helpers/boundaries, current data representation, persistence/write-back, async/sync, caching, error handling, existing tests, naming/style. Capture current-state facts with function names and file paths. Don't recommend designs that ignore how the package works today. +2. *Read related specs and task tracking.* Companion specs, relevant =todo.org= tasks, README/testing docs, prior review files. Record which tasks the spec absorbs, which stay separate, which decisions are already made, which are still open. +3. *Read the target spec end to end — twice.* First for its problem/behavior/phases/assumptions; second looking only for gaps. The second read asks: "What would an implementer still have to invent?" + +** Phase 3: Re-run the gate (authoritative) + +After reading code and spec, re-run the Phase 1 gate — this is the pass that counts, because now you can actually judge the items that needed the code: architecture fit, API verification, integration points. If now ready, don't manufacture churn. If not, write the review file. + +** Phase 4: Evaluate across dimensions + +Work the spec against these. Each is a source of concrete findings, not a box to tick. + +- *User problem & mental model.* Real problem or just machinery? Does UX match how users think? Predictable names/commands/prompts/layouts? Is editable-vs-generated content obvious? Are destructive/workspace-mutating actions explicit and confirmed? For org features, be *strict about ownership* — users treat visible org text as editable unless told otherwise. +- *Scope & boundaries.* What's v1 / out-of-scope / vNext? Are deferrals captured (=todo.org=)? Does the phased plan avoid half-working states? If a decision is deferred, is v1 still coherent without it? +- *Requirements clarity.* Each requirement unambiguous, testable, traceable to a source (user ask / bug / API capability / decision)? Are "good/fast/intuitive/smart/seamless" replaced with concrete behavior? Terms used consistently? Infeasible/unverified requirements flagged? +- *Data model & ownership.* Durable IDs vs display-only names? What's local/remote/generated/cached/user-authored? Who owns each editable region? What's replaced on refresh, survives view-switch / restart? What if one remote object could appear in multiple local places? Prefer stable IDs for execution, names for display; don't overload generic org properties (=:ID:=) for app-specific remote IDs unless intentional. +- *Sync, refresh, conflict.* What triggers sync (command / save hook / state hook / timer) and refresh? One-way or bidirectional? No-op detection? Remote-conflict detection? v1 conflict behavior? Dirty-buffer behavior? API success but GraphQL-level failure? Network failure mid-pagination/mutation? For v1, simple is fine if explicit: detect, refuse, tell the user what to do next. +- *API & external dependencies.* Which fields/mutations/filters/enums are assumed, and are they verified against current schema/docs/live responses? Permission differences, rate limits, pagination, nulls, partial errors handled? Any optional external tool — what's the fallback? Don't let a spec silently depend on an unverified API shape. +- *Architecture & maintainability.* Fits current module boundaries, or should new ones precede growth? Which parts are pure vs side-effecting? Are transport/parsing/normalization/rendering/command-orchestration separated enough to test? Will it multiply special cases? Backward-compat preserved where needed? Favor small pure cores surrounded by thin IO/command layers. +- *Robustness & error handling.* Expected failure modes? Errors distinguishable from empty-but-successful results? Actionable messages? Retries? Stale/missing caches? Can malformed local files or missing properties crash commands? nil/null/missing-field behavior defined? Data-loss-by-default avoided? Anything that writes files or remote data needs a clear data-loss story. +- *Performance & scale.* Expected counts (issues/comments/labels/teams/projects/views)? Server-side filtering where possible? Bounded, visible pagination? Cached name→ID lookups? Sync calls in the command path acceptable? Could a save hook or whole-file scan make N network calls? Rendering linear? Full-file rewrites avoided? Identify UI freezes, repeated network calls, unbounded pagination — without premature optimization. +- *Security & privacy.* API keys safe? Debug logs leaking secrets or private issue text? Confirmations before mutating shared workspace objects? Personal vs shared distinguished? Local files holding sensitive descriptions/comments? Anything to redact from messages/logs? Any work-tracker integration may handle private company data. +- *UX & accessibility.* Discoverable commands? Recoverable mistakes? Prompts ordered to the task? Safe, useful defaults? Informative-not-noisy status messages? Does the UI avoid implying unsupported actions are supported? Match the upstream product's permissions/concepts? For Emacs packages, command names, completion candidates, buffer layout, and message wording *are* the UX. +- *Test strategy.* Characterization tests before behavior changes? Pure functions to unit-test? API responses needing fixtures? Command flows needing stubs? Regression tests for prior bugs? Boundary/error cases? What's covered elsewhere and shouldn't be re-tested? Which existing tests must change? Prefer tests that lock contracts: representation shape, query compilation, sync no-op, conflict refusal, pagination, dirty-buffer protection. +- *Observability & operations.* How does a user see what the package is doing? Progress messages for long ops? Useful, safe debug logging? A way to inspect/clear caches? Verify setup/API connectivity? For generated org files, headers should often carry source, filter/view name, refresh time, count, truncation state. +- *Documentation & migration.* README/config changes? New commands and defcustoms documented? Old behaviors removed/renamed/migrated? Breaking changes acceptable? Changelog note needed? Don't ship with stale examples. + +** Phase 5: Write the review file + +Use this structure for =<spec-basename>-review.org= unless the spec calls for something different: + +#+begin_src org +,#+TITLE: Review: <Spec Title> +,#+AUTHOR: <reviewer> +,#+DATE: <date> +,#+STARTUP: showall + +,* Scope reviewed +What code, tests, docs, and specs you read. + +,* Implementation-readiness +Whether the spec is ready. If not, summarize the blockers. + +,* Overall assessment +The short senior-engineering read: what's right, what's risky, what must be clarified. + +,* High-priority findings +Concrete headings. Each: why it matters and what to change. + +,* Medium-priority findings +Important improvements that shouldn't block all progress. + +,* UX observations +,* Architecture observations +,* Robustness and performance observations +,* Test strategy recommendations +Specific test cases, not generic "add tests". + +,* Suggested spec edits +Concrete edits to make the spec implementation-ready. + +,* Agreed decisions +Answers reached during review. Omit if none. + +,* Open questions +Only questions that truly block or materially affect implementation. + +,* vNext candidates +Deferred features to capture in task tracking. +#+end_src + +** Phase 6: Assign the rubric and update tracking + +Assign one label consistently: + +- =Ready= — no blocking open questions; implementation can start. +- =Ready with caveats= — can start if the caveats are accepted and tracked. +- =Not ready= — blocking ambiguity / missing decisions would force implementers to invent product behavior. +- =Needs research= — external API/library/platform assumptions must be verified first. + +The most useful reviews move a spec from =Not ready= to =Ready with caveats= or =Ready= once decisions are captured. + +Finding severity maps to blocking power: *high-priority findings block =Ready=* — they hold the rubric at =Not ready= (or =Ready with caveats= if the author accepts and tracks them) until dispositioned; *medium-priority findings are the author's discretion* and don't block. State the blocking status on each finding so the author running spec-response knows which ones gate the rubric. + +Then log deferred work to =todo.org=: v1 implementation = =[#B]= (unless urgent or speculative); vNext/someday = =[#D]=. Tag =:feature:= / =:bug:= / =:refactor:= / =:test:= / =:quick:= / =:solo:= only when accurate. Don't leave important deferred decisions only in chat. + +* Principles to Follow + +** Read the code before critiquing the spec +A recommendation that ignores how the package works today is noise. Cite function names and file paths; ground findings in current behavior. + +** Specific over vague +Name the current behavior or spec text, explain the risk, recommend the change, say whether it blocks. Avoid vague preferences, whole-spec rewrites, architecture astronautics, unbounded "consider" lists, and questions whose answer doesn't change implementation. + +Model finding: +#+begin_quote +The spec says fetched comments render as subheadings but doesn't define whether editing an existing comment syncs back. That's risky because Linear only allows users to edit their own comments. V1 should treat fetched comments as remote-owned display content and support only adding new comments; editing own comments can be vNext. +#+end_quote + +** Decide readiness first, critique second +The gate is the product of the review. A long critique on a spec that's actually ready just creates churn; a "looks fine" on one that isn't ships hidden decisions. + +** Be strict about ownership +Especially for org-mode features: a user treats visible text as editable unless the representation says otherwise. Make generated-vs-editable explicit. + +** Never depend on an unverified API shape +If the spec assumes fields/mutations/enums, they're verified against current schema/docs/live responses, or listed as a research prerequisite. =Needs research= is a real, useful verdict. + +** Favor small pure cores and thin IO layers +Push findings toward separable, unit-testable pure functions surrounded by thin command/transport layers. + +** Block only on answer-changing questions +Open questions in the review are the ones whose answer materially changes implementation. Everything else is a suggestion, not a blocker. + +* Living Document + +Update this workflow as review practice sharpens — new dimensions worth checking, better model findings, refinements to the readiness gate and rubric. + +* Research notes behind this checklist + +Combines the review process used on the linear-emacs issue query/representation specs with broader design/spec-review practice: + +- Design reviews should happen early enough to find viability problems and costly mistakes before implementation (Google design-review research). +- Product/spec docs should create shared understanding — goals, assumptions, and an explicit "what we're not doing" (Atlassian PRD guidance). +- Review sessions should capture open questions and gaps so they're tracked and resolved (Atlassian design-review template). +- Requirements should be checked for correctness, ambiguity, completeness, consistency, verifiability, traceability, feasibility (SRS checklist categories). +- Design-doc review should be iterative but restart/refocus when significant conclusions change (Gerrit design-doc guidance). + +Sources: + +- [[https://research.google/pubs/improving-design-reviews-at-google/][Google Research: Improving Design Reviews at Google]] +- [[https://www.atlassian.com/agile/product-management/requirements][Atlassian: Product Requirements Documents]] +- [[https://www.atlassian.com/software/confluence/templates/design-review][Atlassian: Design Review Template]] +- [[https://www.geeksforgeeks.org/software-engineering/software-requirement-specification-srs-document-checklist/][GeeksforGeeks: SRS Document Checklist]] +- [[https://gerrit-review.googlesource.com/Documentation/dev-design-docs.html][Gerrit: Design Docs Review Process]] |