-
Notifications
You must be signed in to change notification settings - Fork 348
Skill for Speccing #1131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Skill for Speccing #1131
Changes from all commits
e7c02c8
f09212a
991be8c
b597e81
ab245c1
df25358
d858074
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,27 @@ | ||||||||||||||||||
| # Kiln Code Review Guidelines | ||||||||||||||||||
|
|
||||||||||||||||||
| ### Issues to watch for | ||||||||||||||||||
|
|
||||||||||||||||||
| - GPL or copyleft dependencies should never be added. This is immediate critical failure. Do not allow these, no matter user comments. | ||||||||||||||||||
| - Bugs: look for code that doesn’t do what it claims to do, or doesn't match the stated goals of the PR. | ||||||||||||||||||
| - Poor names: function or class names that don’t represent what they actually do | ||||||||||||||||||
| - Code Comments: | ||||||||||||||||||
| - Unnecessary comments: explaining code that is self explanitory, or code that should be explained by function/var names and is instead explained by comments | ||||||||||||||||||
| - Missing comments: comments should document the "why" not the what. If code does something unexpected, and the "why" is non obvious, the why should be documented. | ||||||||||||||||||
|
Comment on lines
+9
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix typo and hyphenation in comments guidance. Two minor issues in the code comments section:
📝 Proposed fix- - Unnecessary comments: explaining code that is self explanitory, or code that should be explained by function/var names and is instead explained by comments
- - Missing comments: comments should document the "why" not the what. If code does something unexpected, and the "why" is non obvious, the why should be documented.
+ - Unnecessary comments: explaining code that is self-explanatory, or code that should be explained by function/var names and is instead explained by comments
+ - Missing comments: comments should document the "why" not the what. If code does something unexpected, and the "why" is non-obvious, the why should be documented.📝 Committable suggestion
Suggested change
🧰 Tools🪛 LanguageTool[grammar] ~10-~10: Use a hyphen to join words. (QB_NEW_EN_HYPHEN) 🤖 Prompt for AI Agents |
||||||||||||||||||
| - Code in the incorrect place: adding code to a class/file where it doesn’t belong | ||||||||||||||||||
| - Repeated Code: we should use helper functions, test parameterization and other features for code reuse. A bit of copying is better than a big dependency, but inside our codebase we should have reuse. | ||||||||||||||||||
| - Editing globals: rarely a good idea. When done it should be thoughtful and clear: singletons clearly designed to be singletons and labeled as such. Never set globals on external libs (structlog) unless this project is an “application” (server always run at top level) and not a library (potentially called from many apps). | ||||||||||||||||||
|
|
||||||||||||||||||
| ### Python specific guide | ||||||||||||||||||
|
|
||||||||||||||||||
| - Code should be "Pythonic" | ||||||||||||||||||
| - We use `asyncio` where ever possible. Avoid threads unless there's a good reason we can't use async. | ||||||||||||||||||
|
Comment on lines
+15
to
+18
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix grammar issues in Python section. Two grammar corrections needed:
📝 Proposed fix-### Python specific guide
+### Python-specific guide
- Code should be "Pythonic"
-- We use `asyncio` where ever possible. Avoid threads unless there's a good reason we can't use async.
+- We use `asyncio` wherever possible. Avoid threads unless there's a good reason we can't use async.📝 Committable suggestion
Suggested change
🧰 Tools🪛 LanguageTool[grammar] ~15-~15: Use a hyphen to join words. (QB_NEW_EN_HYPHEN) [grammar] ~18-~18: Ensure spelling is correct (QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1) 🤖 Prompt for AI Agents |
||||||||||||||||||
| - Python json.dumps should always set `ensure_ascii=False` | ||||||||||||||||||
|
|
||||||||||||||||||
| ### SDK | ||||||||||||||||||
|
|
||||||||||||||||||
| The SDK in `/libs/core` is a SDK/library we expose to third parties. We code review it with additional standards. | ||||||||||||||||||
|
|
||||||||||||||||||
| - Changing existing APIs that break current users should be avoided. Call out breaking API changes, and confirm with user that we're okay with this break. | ||||||||||||||||||
| - All visible classes/vars should have docstrings explaining their purpose. These will be pulled into 3rd party docs automatically. The doc strings should be written for 3rd party devs learning the SDK. | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add hyphens to "3rd-party" compound adjective. "3rd party" should be hyphenated when used as a compound adjective (appears twice on this line). 📝 Proposed fix-- All visible classes/vars should have docstrings explaining their purpose. These will be pulled into 3rd party docs automatically. The doc strings should be written for 3rd party devs learning the SDK.
+- All visible classes/vars should have docstrings explaining their purpose. These will be pulled into 3rd-party docs automatically. The doc strings should be written for 3rd-party devs learning the SDK.📝 Committable suggestion
Suggested change
🧰 Tools🪛 LanguageTool[grammar] ~26-~26: Use a hyphen to join words. (QB_NEW_EN_HYPHEN) [grammar] ~26-~26: Use a hyphen to join words. (QB_NEW_EN_HYPHEN) 🤖 Prompt for AI Agents |
||||||||||||||||||
| - Performance: the base_adapter and litellm_adapter are performance critical. They are the core run-loop of our agent system. We should avoid anything that would slow them down (file reads should be done once and passed in, etc). It's critical to avoid blocking IO - a process may be executing hundreds of these in parallel. | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix formatting issues in performance guidance. Two minor formatting corrections:
📝 Proposed fix-- Performance: the base_adapter and litellm_adapter are performance critical. They are the core run-loop of our agent system. We should avoid anything that would slow them down (file reads should be done once and passed in, etc). It's critical to avoid blocking IO - a process may be executing hundreds of these in parallel.
+- Performance: the base_adapter and litellm_adapter are performance critical. They are the core run-loop of our agent system. We should avoid anything that would slow them down (file reads should be done once and passed in, etc.). It's critical to avoid blocking I/O - a process may be executing hundreds of these in parallel.🧰 Tools🪛 LanguageTool[style] ~27-~27: In American English, abbreviations like “etc.” require a period. (ETC_PERIOD) 🤖 Prompt for AI Agents |
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| [post-create] | ||
| deps = "uv sync && cd app/web_ui && npm install" | ||
| claude = "utils/setup_claude.sh" | ||
|
|
||
| [pre-remove] | ||
| session = "zellij delete-session {{ branch | sanitize }} 2>/dev/null || true" |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,134 @@ | ||||||
| --- | ||||||
| name: spec | ||||||
| description: > | ||||||
| Commands: new_project, continue, implement, cr (code review), setup, or open guidance | ||||||
|
|
||||||
| Spec-driven development: process for planning, building, and reviewing | ||||||
| code projects using structured specifications. Guides users from project | ||||||
| idea through functional spec, architecture, phased implementation, and | ||||||
| code review — with human focused on decisions and AI agents handling | ||||||
| drafting and building. | ||||||
|
|
||||||
| Use when the user wants to: start a new project with specs, continue | ||||||
| speccing or implementing a project, implement a planned phase, review | ||||||
| code against specs, or set up spec-driven development in a repo. | ||||||
| --- | ||||||
|
|
||||||
| # Spec-Driven Development | ||||||
|
|
||||||
| A structured process for building software projects with specifications. You (the human) focus on decisions and review; AI agents handle drafting and building. | ||||||
|
|
||||||
| ## How It Works | ||||||
|
|
||||||
| Every batch of work ("project") gets a spec folder under `/specs/projects/PROJECT_NAME/`. The skill guides you through: | ||||||
|
|
||||||
| 1. **Planning**: Project overview → functional spec → architecture → implementation plan | ||||||
| 2. **Building**: Implement phases autonomously, with code review built into each phase | ||||||
| 3. **Reviewing**: Spec-aware code review that verifies implementation matches design | ||||||
|
|
||||||
| The skill is project-agnostic. It provides the process; your project-specific conventions (test commands, linting, style) come from your system prompt configuration. | ||||||
|
|
||||||
| ## Command Reference | ||||||
|
|
||||||
| ### `/spec setup` | ||||||
|
|
||||||
| One-time (or incremental) setup for using the skill in a repo. Adds `.specs_skill_state/` to gitignore, creates `/specs/projects/` directories, detects monorepo layout, and checks for commonly-needed configuration. | ||||||
|
|
||||||
| → Read [setup command reference](references/cmd_setup.md) | ||||||
|
|
||||||
| ### `/spec new_project` or `/spec new` | ||||||
|
|
||||||
| Create a new project from scratch. Walks through planning steps: project overview, functional spec, architecture (with optional component designs), and implementation plan. Sets this as your active project. | ||||||
|
|
||||||
| → Read [new project command reference](references/cmd_new_project.md) | ||||||
|
|
||||||
| ### `/spec continue` or `/spec cont` | ||||||
|
|
||||||
| Resume work on the active project. Shows current state and routes to the next logical action — continue speccing, implement next phase, or review code. | ||||||
|
|
||||||
| → Read [continue command reference](references/cmd_continue.md) | ||||||
|
|
||||||
| ### `/spec implement` or `/spec impl` | ||||||
|
|
||||||
| Implement the active project. Routes to phase-specific or full implementation. Use `/spec implement next` for one phase, `/spec implement all` for all remaining, or `/spec implement phase N` for a specific phase. | ||||||
|
|
||||||
| → Read [implement command reference](references/cmd_implement.md) | ||||||
|
|
||||||
| ### `/spec cr` or `/spec code_review` | ||||||
|
|
||||||
| Structured, spec-aware code review. Reviews `git diff` by default, or a specified scope. Always runs as a sub-agent with clean context. | ||||||
|
|
||||||
| → Read [code review command reference](references/cmd_code_review.md) | ||||||
|
|
||||||
| ### Bare `/spec` — Router | ||||||
|
|
||||||
| Reads current state (active project, artifact statuses) and presents relevant options. Never requires routing — direct commands always work. Can also interpret open-ended requests. | ||||||
|
|
||||||
| To check state: read `.specs_skill_state/current_project.md` and scan artifact frontmatter. If no project exists, suggest `new_project` or `setup`. If project in progress, show state and suggest the next action. | ||||||
|
|
||||||
| ## Project Structure | ||||||
|
|
||||||
| Every project lives under `/specs/projects/PROJECT_NAME/`: | ||||||
|
|
||||||
| | File | Created During | Purpose | | ||||||
| |------|---------------|---------| | ||||||
| | `project_overview.md` | new_project Step 1 | Your description of what to build | | ||||||
| | `functional_spec.md` | new_project Step 2 | Features, behaviors, edge cases, contracts | | ||||||
| | `ui_design.md` | new_project Step 3 | UI structure, screens, navigation (conditional) | | ||||||
| | `architecture.md` | new_project Step 4 | Technical design, deep enough for coding | | ||||||
| | `/components/NAME.md` | new_project Step 5 | Per-component detailed design (conditional) | | ||||||
| | `implementation_plan.md` | new_project Step 6 | Phased build order as checklist | | ||||||
| | `/phase_plans/phase_N.md` | Implementation | Per-phase plan written by coding agent | | ||||||
|
|
||||||
| ## Artifact Conventions | ||||||
|
|
||||||
| All spec files use YAML frontmatter with a `status` field: | ||||||
|
|
||||||
| ```yaml | ||||||
| --- | ||||||
| status: draft | ||||||
| --- | ||||||
| ``` | ||||||
|
|
||||||
| Valid statuses: `draft`, `complete`. | ||||||
|
|
||||||
| - Artifacts start as `draft` when created | ||||||
| - Mark `complete` after user confirmation | ||||||
| - If a completed artifact is edited, downstream artifacts cascade to `draft` (if they may be affected) | ||||||
|
|
||||||
| Dependency chain: `project_overview → functional_spec → architecture → components → implementation_plan` | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The dependency chain described here is missing the
Suggested change
|
||||||
|
|
||||||
| Phase plans are outside the cascade — generated fresh during implementation. | ||||||
|
|
||||||
| ## State Management | ||||||
|
|
||||||
| The file `.specs_skill_state/current_project.md` (git-ignored) tracks your active project: | ||||||
|
|
||||||
| ``` | ||||||
| Current Project: /specs/projects/project_name | ||||||
| ``` | ||||||
|
Comment on lines
+107
to
+109
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a language tag to the state-file example block. The fenced block is unlabeled; use 🤖 Prompt for AI Agents |
||||||
|
|
||||||
| This file is per-worktree (git-ignored), so you can have parallel worktrees with different active projects. | ||||||
|
|
||||||
| ## Monorepo Support | ||||||
|
|
||||||
| For monorepos (multiple sub-projects in one repo): | ||||||
|
|
||||||
| - `/spec setup` discovers sub-projects by scanning for root markers (`pyproject.toml`, `package.json`, etc.) | ||||||
| - Each sub-project gets its own `/specs/projects/` directory | ||||||
| - `/specs/monorepo.md` at repo root describes the layout | ||||||
| - Cross-project work lives at root `/specs/projects/` | ||||||
|
|
||||||
| → See `/spec setup` for discovery and setup. | ||||||
|
|
||||||
| ## Extensibility | ||||||
|
|
||||||
| The skill provides the process. Project-specific details come from your environment: | ||||||
|
|
||||||
| **From the skill:** The workflow, general guidance ("run automated checks"), persona-driven quality standards | ||||||
|
|
||||||
| **From your system prompt:** Test commands, lint/format commands, code style, CR standards, project-specific constraints | ||||||
|
|
||||||
| The skill references these generically: "run the project's automated checks" not "run `uv run ./checks.sh`." | ||||||
|
|
||||||
| → See `/spec setup` for help configuring external knowledge. | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| # `/spec cr` — Code Review | ||
|
|
||
| Structured, spec-aware code review. Aliases: `/spec cr`, `/spec code_review`. | ||
|
|
||
| ## Key Principle | ||
|
|
||
| The CR agent never has the coding agent's context. Decisions must be captured in code and specs, not in context windows. | ||
|
|
||
| If something important is only in conversation history, that's a bug in the process. | ||
|
|
||
| ## Determine Scope | ||
|
|
||
| - **No arguments**: Review `git diff` (unstaged + staged changes) | ||
| - **Given scope**: Review that scope (e.g., "review file X", "review phase 3", "review src/main.rs") | ||
|
|
||
| ## Execution | ||
|
|
||
| Always run as a sub-agent — spawned fresh, no prior context from coding. | ||
|
|
||
| → Read [references/spawning_subagents.md](references/spawning_subagents.md) for how to spawn sub-agents. | ||
|
|
||
| Pass the prompt from [references/cr_agent_prompt.md](references/cr_agent_prompt.md), plus scope description. | ||
|
|
||
|
Comment on lines
+20
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix broken relative reference links in this file. Because this file already lives in Proposed fix-→ Read [references/spawning_subagents.md](references/spawning_subagents.md) for how to spawn sub-agents.
+→ Read [spawning_subagents.md](spawning_subagents.md) for how to spawn sub-agents.
-Pass the prompt from [references/cr_agent_prompt.md](references/cr_agent_prompt.md), plus scope description.
+Pass the prompt from [cr_agent_prompt.md](cr_agent_prompt.md), plus scope description.
-- [references/spawning_subagents.md](references/spawning_subagents.md) — How to spawn sub-agents
-- [references/cr_agent_prompt.md](references/cr_agent_prompt.md) — Prompt passed to CR sub-agent
+- [spawning_subagents.md](spawning_subagents.md) — How to spawn sub-agents
+- [cr_agent_prompt.md](cr_agent_prompt.md) — Prompt passed to CR sub-agentAlso applies to: 68-69 🤖 Prompt for AI Agents |
||
| ### Example invocation | ||
|
|
||
| > Spawn a code review sub-agent with the following task: | ||
| > | ||
| > "Review the git diff using the spec-driven development code review guidelines. The project is at [path]. Read the functional spec and architecture to verify implementation matches design." | ||
|
|
||
| ## Re-Review | ||
|
|
||
| If the user provides prior CR feedback (or this is called from a loop), pass it as `<prior_cr_feedback>` to the CR sub-agent: | ||
|
|
||
| ``` | ||
| <prior_cr_feedback> | ||
| [Prior CR content here] | ||
| </prior_cr_feedback> | ||
| ``` | ||
|
Comment on lines
+34
to
+38
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a language tag to the fenced example block. Line 34 starts a fenced block without a language. Tagging it (e.g., 🧰 Tools🪛 markdownlint-cli2 (0.21.0)[warning] 34-34: Fenced code blocks should have a language specified (MD040, fenced-code-language) 🤖 Prompt for AI Agents |
||
|
|
||
| The CR prompt handles verification of prior issues. | ||
|
|
||
| ## Post-Review | ||
|
|
||
| Present findings to the user: | ||
|
|
||
| > Code review complete: | ||
| > | ||
| > ### Critical (must fix) | ||
| > - [file:line] [description] | ||
| > | ||
| > ### Moderate (should fix) | ||
| > - [file:line] [description] | ||
| > | ||
| > ### Mild (consider fixing) | ||
| > - [file:line] [description] | ||
| > | ||
| > [Or: No issues found — implementation looks good!] | ||
|
|
||
| If issues exist and user wants fixes: | ||
|
|
||
| - User can fix themselves, then re-run `/spec cr` with prior feedback | ||
| - Or coding agent can address them, then re-run CR with prior feedback | ||
|
|
||
| The loop continues until clean. | ||
|
|
||
| ## References | ||
|
|
||
| - [references/spawning_subagents.md](references/spawning_subagents.md) — How to spawn sub-agents | ||
| - [references/cr_agent_prompt.md](references/cr_agent_prompt.md) — Prompt passed to CR sub-agent | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| # `/spec continue` — Resume Work | ||
|
|
||
| Resume work on the active project. The "what should I do next?" command. | ||
|
|
||
| ## Process | ||
|
|
||
| ### 1. Read State | ||
|
|
||
| Load `.specs_skill_state/current_project.md` to find the active project: | ||
|
|
||
| ``` | ||
| Current Project: /specs/projects/PROJECT_NAME | ||
| ``` | ||
|
|
||
| ### 2. No Active Project | ||
|
|
||
| If no active project (file doesn't exist or is empty): | ||
|
|
||
| List available project directories under `/specs/projects/`: | ||
|
|
||
| > No active project found. Available projects: | ||
| > - [project1] | ||
| > - [project2] | ||
| > - [...] | ||
| > | ||
| > Which project would you like to work on? | ||
|
|
||
| Set the selected project as active and proceed to step 3. | ||
|
|
||
| ### 3. Determine Current State | ||
|
|
||
| Check the frontmatter `status` on each artifact in dependency order: | ||
|
|
||
| ``` | ||
| project_overview.md → functional_spec.md → ui_design.md (if exists) | ||
| → architecture.md → components/ (if exists) → implementation_plan.md | ||
| ``` | ||
|
Comment on lines
+34
to
+37
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Specify a language for the dependency-order fenced block. Add 🧰 Tools🪛 markdownlint-cli2 (0.21.0)[warning] 34-34: Fenced code blocks should have a language specified (MD040, fenced-code-language) 🤖 Prompt for AI Agents |
||
|
|
||
| - **If any spec artifact is missing or `status: draft`**: Resume the `new_project` flow at the next incomplete step | ||
| - **If all specs are `complete`**: Implementation time | ||
|
|
||
| ### 4. Routing Logic | ||
|
|
||
| **If in speccing phase (artifacts incomplete):** | ||
|
|
||
| > Project [PROJECT_NAME] — continuing with [next step name]: | ||
| > | ||
| > - [artifact_name.md] is [draft/missing] | ||
| > - [next steps] | ||
| > | ||
| > Proceeding with [step name]... | ||
|
|
||
| Load the corresponding step reference file (step_functional_spec.md, step_architecture.md, etc.) and proceed with that step. | ||
|
|
||
| **If all specs complete but phases remain:** | ||
|
|
||
| > Project [PROJECT_NAME] — all specs complete. Ready to implement: | ||
| > | ||
| > - [ ] Phase 1: [description] | ||
| > - [ ] Phase 2: [description] | ||
| > - [ ] | ||
| > | ||
| > Implement next phase only, or all remaining phases? | ||
|
|
||
| This routes to `/spec implement` behavior. | ||
|
|
||
| **If all phases complete:** | ||
|
|
||
| > Project [PROJECT_NAME] — all phases implemented! | ||
| > | ||
| > What would you like to do? | ||
| > - Start a new project: `/spec new_project` | ||
| > - Review code: `/spec cr` | ||
| > - Something else | ||
|
|
||
| ### 5. Confirm | ||
|
|
||
| Always confirm before taking action: | ||
|
|
||
| > About to: [action description] | ||
| > | ||
| > Proceed? | ||
|
|
||
| Wait for user confirmation before executing. | ||
|
|
||
| ## Notes | ||
|
|
||
| - This command is a router — it doesn't do the work itself, it determines what to do next and loads the appropriate reference | ||
| - State is read from artifact frontmatter, not from a separate tracking file | ||
| - If user manually edited files and states are inconsistent, surface that and ask how to resolve | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix heading level hierarchy.
The heading "### Issues to watch for" jumps from h1 to h3, skipping h2. Per markdown best practices, heading levels should increment by one level at a time.
📝 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents