Add Kiro skills — 18 SKILL.md files (3/4)#811
Merged
affaan-m merged 1 commit intoaffaan-m:mainfrom Mar 23, 2026
Merged
Conversation
Co-Authored-By: Sungmin Hong <hsungmin@amazon.com>
Contributor
|
Contributor
Analysis Failed
Troubleshooting
Retry: |
Contributor
📝 WalkthroughWalkthroughThis PR adds 18 new skill documentation files (.kiro/skills/*) defining best practices, patterns, and workflows across engineering domains including AI-assisted development, API design, backend/frontend patterns, testing, database migrations, deployment, security, and tool-specific guidance (Go, Python, PostgreSQL, Docker). Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR is part 3 of 4 of the Kiro IDE integration, adding 18 Key observations:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Kiro IDE /menu] --> B[.kiro/skills/ loader]
B --> C1[agentic-engineering]
B --> C2[api-design]
B --> C3[backend-patterns]
B --> C4[coding-standards]
B --> C5[database-migrations]
B --> C6[deployment-patterns]
B --> C7[docker-patterns]
B --> C8[e2e-testing]
B --> C9[frontend-patterns]
B --> C10[golang-patterns]
B --> C11[golang-testing]
B --> C12[postgres-patterns]
B --> C13[python-patterns]
B --> C14[python-testing]
B --> C15[search-first]
B --> C16[security-review]
B --> C17[tdd-workflow]
B --> C18[verification-loop]
D[skills/ - existing ECC skills] -.->|copied & enhanced| B
style C16 fill:#f99,stroke:#c33
style C10 fill:#f99,stroke:#c33
style D fill:#ffe,stroke:#aa0
Reviews (1): Last reviewed commit: "Add Kiro skills (18 SKILL.md files)" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+88
to
+103
| ```go | ||
| func workerPool(jobs <-chan Job, results chan<- Result, workers int) { | ||
| var wg sync.WaitGroup | ||
| for i := 0; i < workers; i++ { | ||
| wg.Add(1) | ||
| go func() { | ||
| defer wg.Done() | ||
| for job := range jobs { | ||
| results <- processJob(job) | ||
| } | ||
| }() | ||
| } | ||
| wg.Wait() | ||
| close(results) | ||
| } | ||
| ``` |
Contributor
There was a problem hiding this comment.
The workerPool function blocks until all workers finish (wg.Wait()) before returning, but workers block on results <- processJob(job) until someone reads from results. If the caller calls workerPool(...) synchronously and then tries to range over results afterward, neither goroutine can make progress — a classic Go deadlock.
Typical safe usage requires running workerPool in its own goroutine:
go workerPool(jobs, results, workers) // must be in a goroutine
for result := range results {
// consume results concurrently
}Without this context, a reader copying the pattern may write:
workerPool(jobs, results, workers) // deadlocks if results is unbuffered
for result := range results { ... }Recommend adding a usage example showing the goroutine call and the concurrent consumer, or using a sufficiently-buffered results channel.
Comment on lines
+1
to
+10
| --- | ||
| name: agentic-engineering | ||
| description: > | ||
| Operate as an agentic engineer using eval-first execution, decomposition, | ||
| and cost-aware model routing. Use when AI agents perform most implementation | ||
| work and humans enforce quality and risk controls. | ||
| metadata: | ||
| origin: ECC | ||
| --- | ||
|
|
Contributor
There was a problem hiding this comment.
skills/
Every skill added under .kiro/skills/ is a copy of a file that already exists under skills/ — e.g. skills/agentic-engineering/SKILL.md, skills/security-review/SKILL.md, etc. The differences are minor (mostly reformatted YAML frontmatter with a metadata: wrapper, a few extra example blocks, and capitalisation fixes).
This creates two sources of truth for the same content. Any future improvement to a skill must be applied in both skills/ and .kiro/skills/, and the copies will inevitably drift.
A few approaches to consider:
- Symlinks: Have
.kiro/skills/<name>/SKILL.mdsymlink toskills/<name>/SKILL.md. - Build step: Generate
.kiro/skills/fromskills/automatically. - Document the intent: If Kiro requires its own copies for loader compatibility, add a comment in the top-level README explaining the relationship and the chosen sync strategy so contributors know which file to edit.
This same pattern applies to all 18 files in this PR: api-design, backend-patterns, coding-standards, database-migrations, deployment-patterns, docker-patterns, e2e-testing, frontend-patterns, golang-patterns, golang-testing, postgres-patterns, python-patterns, python-testing, search-first, security-review, tdd-workflow, and verification-loop.
Comment on lines
+213
to
+228
| ```typescript | ||
| // next.config.js | ||
| const securityHeaders = [ | ||
| { | ||
| key: 'Content-Security-Policy', | ||
| value: ` | ||
| default-src 'self'; | ||
| script-src 'self' 'unsafe-eval' 'unsafe-inline'; | ||
| style-src 'self' 'unsafe-inline'; | ||
| img-src 'self' data: https:; | ||
| font-src 'self'; | ||
| connect-src 'self' https://api.example.com; | ||
| `.replace(/\s{2,}/g, ' ').trim() | ||
| } | ||
| ] | ||
| ``` |
Contributor
There was a problem hiding this comment.
The CSP configuration example includes 'unsafe-eval' and 'unsafe-inline' in script-src without any warning. These two directives fundamentally undermine the XSS protection that CSP is designed to provide:
'unsafe-inline'permits attacker-injected inline<script>tags and event-handler attributes — the most common XSS vector.'unsafe-eval'permitseval(),Function(), andsetTimeout(string), which are frequently used to escalate XSS payloads.
Because this block appears inside a Security Review Skill without any caveat, a reader is likely to copy it verbatim and believe their app is protected. The recommended baseline script-src for new projects is 'self' only, with nonces or hashes for any required inline scripts.
If the application genuinely requires 'unsafe-eval' (e.g. an older bundler), the example should explicitly call this out as a known weakness to accept, not silently include it as a safe default.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (14)
.kiro/skills/golang-patterns/SKILL.md (2)
70-83: Align constructor example with stated dependency-validation rule.The pattern says “Validate dependencies in constructor” (Line 82), but the example constructor does not validate inputs.
🧩 Suggested doc/code example update
func NewUserService(repo UserRepository, logger Logger) *UserService { + if repo == nil { + panic("repo must not be nil") + } + if logger == nil { + panic("logger must not be nil") + } return &UserService{ repo: repo, logger: logger, } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/golang-patterns/SKILL.md around lines 70 - 83, The NewUserService constructor lacks dependency validation; update the function signature to return (*UserService, error) and add nil checks for repo (UserRepository) and logger (Logger) inside NewUserService, returning a descriptive error (e.g., "nil repo" or "nil logger") when a dependency is invalid, otherwise return the constructed *UserService and nil error; update callers to handle the error accordingly.
13-227: Add an explicitHow It Workssection.This skill includes “When to Use” and examples, but it is missing a dedicated “How It Works” section expected for skill docs.
📄 Suggested doc patch
## Go Patterns > This skill provides comprehensive Go patterns extending common design principles with Go-specific idioms. + +## How It Works + +This skill is organized by practical Go concerns: +- API design and constructors (functional options, dependency injection) +- abstraction boundaries (small interfaces, accept interfaces/return structs) +- concurrency and cancellation patterns (worker pools, context) +- robust error handling and testing idioms + +Apply the relevant section while implementing or refactoring, then validate with tests.Based on learnings: Skills must be formatted as Markdown files with clear sections including "When to Use", "How It Works", and "Examples".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/golang-patterns/SKILL.md around lines 13 - 227, Add a new "How It Works" section to SKILL.md (placed near the existing "When to Use" section) that concisely explains the mechanics and intent of the documented patterns: summarize how Functional Options, Small Interfaces, Dependency Injection, Concurrency Patterns (Worker Pool, Context Propagation), Error Handling (wrapping, custom, sentinel), Package Organization, and Testing Patterns operate in practice, what problems they solve, and quick guidance on when to apply each; reference those section names in the text so readers can jump to examples and keep the section to 3–6 short sentences focusing on actionable mechanics rather than examples..kiro/skills/frontend-patterns/SKILL.md (3)
174-196: StabilizeuseQuerydependencies to prevent accidental refetch churn.
optionsin bothuseCallbackanduseEffectdeps can trigger extra refetches when callers pass inline objects. Prefer destructuring stable fields (or memoized options) in the example.Proposed doc fix
export function useQuery<T>( key: string, fetcher: () => Promise<T>, options?: UseQueryOptions<T> ) { + const enabled = options?.enabled !== false + const onSuccess = options?.onSuccess + const onError = options?.onError + const [data, setData] = useState<T | null>(null) const [error, setError] = useState<Error | null>(null) const [loading, setLoading] = useState(false) const refetch = useCallback(async () => { setLoading(true) setError(null) try { const result = await fetcher() setData(result) - options?.onSuccess?.(result) + onSuccess?.(result) } catch (err) { const error = err as Error setError(error) - options?.onError?.(error) + onError?.(error) } finally { setLoading(false) } - }, [fetcher, options]) + }, [fetcher, onSuccess, onError]) useEffect(() => { - if (options?.enabled !== false) { - refetch() - } - }, [key, refetch, options?.enabled]) + if (enabled) refetch() + }, [key, enabled, refetch])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/frontend-patterns/SKILL.md around lines 174 - 196, The example causes unnecessary refetch churn because the broad options object is used in useCallback and useEffect deps; update the refetch useCallback and the effect to depend on stable primitives instead of the entire options object — destructure the options into options?.enabled, options?.onSuccess, options?.onError (or require callers to memoize options) and change the dependency arrays from [fetcher, options] and [key, refetch, options?.enabled] to something like [fetcher, options?.enabled, options?.onSuccess, options?.onError] for useCallback (and [key, refetch, options?.enabled] or equivalent stable fields for useEffect) so that refetch, useCallback, useEffect, options, enabled, onSuccess, onError, fetcher and key are referenced explicitly and avoid refetch churn.
13-23: Standardize skill section headers for loader/discoverability consistency.This doc has rich content, but it does not expose explicit
When to Use,How It Works, andExamplesheadings. Please add these canonical headings (you can keep current content under them) to align with the expected skill format.Based on learnings: "Applies to skills/**/*.md : Skills must be formatted as Markdown files with clear sections including 'When to Use', 'How It Works', and 'Examples'."
Also applies to: 137-644
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/frontend-patterns/SKILL.md around lines 13 - 23, The current SKILL.md uses a "When to Activate" section but lacks the canonical headings; update the document to include explicit top-level headings "When to Use", "How It Works", and "Examples" (you can relocate the existing "When to Activate" content under "When to Use"), ensure "Component Patterns" or similar content is placed under "How It Works" or "Examples" as appropriate, and keep all existing content but reorganize it under these three headings to match the required skill format.
116-122: Add request cancellation to the fetch pattern.This pattern can update state after unmount or after URL changes race. Include
AbortControllercleanup so the example models safe async effects.Proposed doc fix
useEffect(() => { - fetch(url) - .then(res => res.json()) - .then(setData) - .catch(setError) - .finally(() => setLoading(false)) + const controller = new AbortController() + fetch(url, { signal: controller.signal }) + .then(res => res.json()) + .then(setData) + .catch(err => { + if (err.name !== 'AbortError') setError(err as Error) + }) + .finally(() => setLoading(false)) + return () => controller.abort() }, [url])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/frontend-patterns/SKILL.md around lines 116 - 122, The effect should create an AbortController and pass its signal to fetch to allow cancellation; update the chain for useEffect so fetch(url, { signal: controller.signal }) is used, and before calling setData, setError, or setLoading check controller.signal.aborted (or ignore errors with err.name === 'AbortError' in the catch) to avoid updating state after unmount or URL changes; finally return a cleanup function that calls controller.abort(). Reference: useEffect, fetch, setData, setError, setLoading, and AbortController..kiro/skills/coding-standards/SKILL.md (2)
78-92: Consider stricter immutability type safety.The immutability pattern shows spread operators but doesn't demonstrate the
Readonly<T>wrapper thatrules/typescript/coding-style.md(lines 116-139) prescribes for parameter types. The existing rules useReadonly<User>to enforce immutability at the type level, providing stronger guarantees than spread operators alone.♻️ Enhanced immutability example
### Immutability Pattern (CRITICAL) ```typescript +interface User { + id: string + name: string +} + // ✅ ALWAYS use spread operator +function updateUser(user: Readonly<User>, name: string): User { + return { + ...user, + name + } +} + const updatedUser = { ...user, name: 'New Name' }Based on learnings: As per coding guidelines, immutability patterns should use
Readonly<T>types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/coding-standards/SKILL.md around lines 78 - 92, Update the Immutability Pattern section to show type-level immutability by adding an example that defines an interface User and a function updateUser(user: Readonly<User>, name: string): User which uses the spread operator to return a new User; reference the existing rule in rules/typescript/coding-style.md (lines 116-139) to justify using Readonly<T> for parameter types so readers see both runtime spread usage and compile-time immutability enforcement.
259-283: Consider consolidation with existing rules.The
ApiResponse<T>interface and API response format defined here is identical to the one inrules/typescript/patterns.md(lines 233-247). Similarly, the Zod validation pattern (lines 287-314) duplicatesrules/typescript/coding-style.md(lines 178-193).While this duplication may be intentional (skills for LLM vs rules for other tools), consider whether these could reference a single source of truth to avoid drift.
Based on learnings: As per coding guidelines, these patterns follow established conventions but exist in multiple locations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/coding-standards/SKILL.md around lines 259 - 283, The ApiResponse<T> interface and the Zod validation pattern are duplicated; extract the canonical definitions into a single shared reference and update the duplicated occurrences to import or link to that single source instead of repeating them—specifically consolidate the ApiResponse<T> interface and the Zod schema pattern (the examples around the ApiResponse response objects and the Zod validation block) into one authoritative rule/skill entry and replace the copies with references to that entry so edits only need to be made in one place..kiro/skills/verification-loop/SKILL.md (1)
125-128: Potential redundancy with existing hooks.The verification phases overlap with existing postToolUse hooks:
quality-gateruns lint/format checks after edits,post:edit:typecheckruns type checks, andpost:edit:console-warnchecks for console.log statements (seehooks/hooks.jsonlines 158-198). Running this verification skill immediately after file edits may cause redundant checks.Consider documenting when to use this comprehensive verification (e.g., before PRs, after major refactors) vs relying on automatic hooks for incremental checks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/verification-loop/SKILL.md around lines 125 - 128, The verification-loop skill duplicates checks already performed by existing postToolUse hooks (notably quality-gate, post:edit:typecheck, and post:edit:console-warn), so update SKILL.md to clarify when to run this comprehensive verification versus relying on automatic hooks: state that verification-loop is intended for full-project validation before PRs or after major refactors, while incremental postToolUse hooks handle per-edit lint/format/type/console checks; reference the hook names (quality-gate, post:edit:typecheck, post:edit:console-warn) and hooks/hooks.json so readers can see the overlap and avoid redundant runs..kiro/skills/golang-testing/SKILL.md (1)
157-160: Coverage threshold check is fragile.The grep-based coverage enforcement on line 159 works but is brittle—it assumes a specific output format and would break if Go changes its coverage output. For production use, consider recommending dedicated tools like
gocovor CI-specific solutions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/golang-testing/SKILL.md around lines 157 - 160, The current grep-based coverage check using the command starting with "go test -cover ./..." and matching 'coverage: [0-7][0-9]\.[0-9]%' is brittle; replace it with a deterministic coverage-profile approach (e.g., run "go test -coverprofile=profile.out ./...", then use "go tool cover -func=profile.out" to read the "total:" line and parse the numeric coverage) or recommend using a dedicated tool like gocov or a CI-native coverage gate; update SKILL.md to show the robust sequence (generate profile, extract total percent programmatically, compare against 80) and/or add a short note suggesting gocov/CI solutions as alternatives..kiro/skills/api-design/SKILL.md (1)
120-198: Consider adding success indicator field to align with API response conventions.The current response format examples use HTTP status codes to distinguish success from errors, which is valid. However, the envelope lacks an explicit
successboolean field. Many API conventions include this field for easier client-side handling.Current format:
{ "data": {...} }Alternative envelope format (more explicit):
{ "success": true, "data": {...} }This is a recommended enhancement rather than a blocker, as HTTP status codes already provide success/error semantics. Based on learnings, consistent API response format with success indicator, data payload, error message, and pagination metadata is preferred in this codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/api-design/SKILL.md around lines 120 - 198, Add an explicit success boolean to the API envelope examples and TypeScript interfaces so clients can rely on a consistent field rather than only HTTP status codes: update the example JSON responses to include "success": true/false, modify the ApiResponse<T> interface to include success: boolean, and ensure ApiError or error payloads include success: false; update the collection/pagination examples and Error Response section to show the success flag for both success and error variants..kiro/skills/docker-patterns/SKILL.md (2)
370-376: Remove duplicate "When to Use This Skill" section.This section duplicates the "When to Activate" section (lines 15-21) with nearly identical content. Keep only one to avoid redundancy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/docker-patterns/SKILL.md around lines 370 - 376, The "When to Use This Skill" block duplicates the earlier "When to Activate" section; remove the redundant "When to Use This Skill" header and its bullet list (the block beginning with "## When to Use This Skill") and keep the original "When to Activate" content instead, ensuring any internal references or table-of-contents entries still point to the single remaining section.
119-119: Align HEALTHCHECK parameters with deployment-patterns for consistency.The HEALTHCHECK instruction here uses
--interval=30s --timeout=3s, while the deployment-patterns skill (lines 118-119) includes additional parameters:--start-period=5s --retries=3. Consider adding these for consistency across skills and to provide a startup grace period before health checks begin.Current:
HEALTHCHECK --interval=30s --timeout=3s CMD wget -qO- http://localhost:3000/health || exit 1Suggested (aligned with deployment-patterns):
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ CMD wget -qO- http://localhost:3000/health || exit 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/docker-patterns/SKILL.md at line 119, Update the HEALTHCHECK instruction to match deployment-patterns by adding the startup grace and retry options; specifically modify the existing HEALTHCHECK line (the HEALTHCHECK --interval=30s --timeout=3s CMD ... entry) to include --start-period=5s and --retries=3 so it becomes HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 CMD wget -qO- http://localhost:3000/health || exit 1 (you can optionally wrap the CMD on the next line for readability)..kiro/skills/deployment-patterns/SKILL.md (1)
433-440: Remove duplicate "When to Use This Skill" section.This section duplicates the "When to Activate" section (lines 15-22) with nearly identical content. Keep only one to avoid redundancy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/deployment-patterns/SKILL.md around lines 433 - 440, The file contains a duplicate "When to Use This Skill" section that repeats the "When to Activate" content; remove the duplicate heading and bullet list (the "When to Use This Skill" block) and keep the original "When to Activate" section (or vice versa), ensuring only one of those headings with its bullet points remains to eliminate redundancy..kiro/skills/postgres-patterns/SKILL.md (1)
150-157: Remove duplicate "When to Use This Skill" section.This section duplicates the "When to Activate" section (lines 16-22) with nearly identical content. Keep only one to avoid redundancy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/skills/postgres-patterns/SKILL.md around lines 150 - 157, The document contains a duplicated section header "When to Use This Skill" with the same bullet list as the existing "When to Activate" section; remove the redundant "When to Use This Skill" header and its bullet list so only the original "When to Activate" content remains, and verify any internal links/TOC still point to the single remaining section (look for the headers "When to Use This Skill" and "When to Activate" in SKILL.md to locate and delete the duplicate block).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.kiro/skills/backend-patterns/SKILL.md:
- Around line 187-206: The PL/pgSQL function create_market_with_position uses
incorrect single-dollar delimiters; change the function body delimiters from AS
$ ... $; to AS $$ ... $$; so the function declaration and ending dollar-quote
match (i.e., use $$ before the BEGIN and $$; after the END) ensuring the
PostgreSQL parser accepts the block for the create_market_with_position
function.
In @.kiro/skills/e2e-testing/SKILL.md:
- Around line 1-11: Add a "When to Activate" section immediately after the
existing "E2E Testing Patterns" header (i.e., below the top description) titled
"## When to Activate" and include the suggested bullet points (Writing
end-to-end tests for critical user flows; Setting up Playwright test
infrastructure; Debugging flaky E2E tests; Integrating E2E tests into CI/CD
pipelines; Testing Web3/wallet integration flows; Validating financial or
payment workflows), and ensure the skill also contains the required 'When to
Use', 'How It Works', and 'Examples' sections elsewhere in the document so it
matches the other skills' format.
In @.kiro/skills/frontend-patterns/SKILL.md:
- Around line 299-302: The useMemo example mutates the original markets array by
calling markets.sort(), so update the useMemo block (the sortedMarkets constant
defined via useMemo) to sort a shallow copy instead of markets directly; copy
markets (e.g., using spread or slice) before calling sort so the original
markets prop/state is not mutated and the memoization remains safe.
In @.kiro/skills/python-patterns/SKILL.md:
- Around line 13-421: The SKILL.md is missing explicit "How It Works" and
"Examples" headings required by skill format; add a "How It Works" section
explaining the core patterns (Protocol, Dataclasses, Context Managers,
Generators, Decorators, Async/Await, Type Hints, Dependency Injection, Package
Organization, Error Handling, Property Decorators, Functional Programming) and
an "Examples" section that contains short runnable snippets or references to the
existing examples (e.g., the Protocol example, dataclass DTOs, context manager
examples, generator/ decorator/async examples) — place these headings
prominently after the summary/introduction under the main "Python Patterns"
title (near the existing examples and before or adjacent to "When to Use This
Skill") so loaders and contributors can find them.
---
Nitpick comments:
In @.kiro/skills/api-design/SKILL.md:
- Around line 120-198: Add an explicit success boolean to the API envelope
examples and TypeScript interfaces so clients can rely on a consistent field
rather than only HTTP status codes: update the example JSON responses to include
"success": true/false, modify the ApiResponse<T> interface to include success:
boolean, and ensure ApiError or error payloads include success: false; update
the collection/pagination examples and Error Response section to show the
success flag for both success and error variants.
In @.kiro/skills/coding-standards/SKILL.md:
- Around line 78-92: Update the Immutability Pattern section to show type-level
immutability by adding an example that defines an interface User and a function
updateUser(user: Readonly<User>, name: string): User which uses the spread
operator to return a new User; reference the existing rule in
rules/typescript/coding-style.md (lines 116-139) to justify using Readonly<T>
for parameter types so readers see both runtime spread usage and compile-time
immutability enforcement.
- Around line 259-283: The ApiResponse<T> interface and the Zod validation
pattern are duplicated; extract the canonical definitions into a single shared
reference and update the duplicated occurrences to import or link to that single
source instead of repeating them—specifically consolidate the ApiResponse<T>
interface and the Zod schema pattern (the examples around the ApiResponse
response objects and the Zod validation block) into one authoritative rule/skill
entry and replace the copies with references to that entry so edits only need to
be made in one place.
In @.kiro/skills/deployment-patterns/SKILL.md:
- Around line 433-440: The file contains a duplicate "When to Use This Skill"
section that repeats the "When to Activate" content; remove the duplicate
heading and bullet list (the "When to Use This Skill" block) and keep the
original "When to Activate" section (or vice versa), ensuring only one of those
headings with its bullet points remains to eliminate redundancy.
In @.kiro/skills/docker-patterns/SKILL.md:
- Around line 370-376: The "When to Use This Skill" block duplicates the earlier
"When to Activate" section; remove the redundant "When to Use This Skill" header
and its bullet list (the block beginning with "## When to Use This Skill") and
keep the original "When to Activate" content instead, ensuring any internal
references or table-of-contents entries still point to the single remaining
section.
- Line 119: Update the HEALTHCHECK instruction to match deployment-patterns by
adding the startup grace and retry options; specifically modify the existing
HEALTHCHECK line (the HEALTHCHECK --interval=30s --timeout=3s CMD ... entry) to
include --start-period=5s and --retries=3 so it becomes HEALTHCHECK
--interval=30s --timeout=3s --start-period=5s --retries=3 CMD wget -qO-
http://localhost:3000/health || exit 1 (you can optionally wrap the CMD on the
next line for readability).
In @.kiro/skills/frontend-patterns/SKILL.md:
- Around line 174-196: The example causes unnecessary refetch churn because the
broad options object is used in useCallback and useEffect deps; update the
refetch useCallback and the effect to depend on stable primitives instead of the
entire options object — destructure the options into options?.enabled,
options?.onSuccess, options?.onError (or require callers to memoize options) and
change the dependency arrays from [fetcher, options] and [key, refetch,
options?.enabled] to something like [fetcher, options?.enabled,
options?.onSuccess, options?.onError] for useCallback (and [key, refetch,
options?.enabled] or equivalent stable fields for useEffect) so that refetch,
useCallback, useEffect, options, enabled, onSuccess, onError, fetcher and key
are referenced explicitly and avoid refetch churn.
- Around line 13-23: The current SKILL.md uses a "When to Activate" section but
lacks the canonical headings; update the document to include explicit top-level
headings "When to Use", "How It Works", and "Examples" (you can relocate the
existing "When to Activate" content under "When to Use"), ensure "Component
Patterns" or similar content is placed under "How It Works" or "Examples" as
appropriate, and keep all existing content but reorganize it under these three
headings to match the required skill format.
- Around line 116-122: The effect should create an AbortController and pass its
signal to fetch to allow cancellation; update the chain for useEffect so
fetch(url, { signal: controller.signal }) is used, and before calling setData,
setError, or setLoading check controller.signal.aborted (or ignore errors with
err.name === 'AbortError' in the catch) to avoid updating state after unmount or
URL changes; finally return a cleanup function that calls controller.abort().
Reference: useEffect, fetch, setData, setError, setLoading, and AbortController.
In @.kiro/skills/golang-patterns/SKILL.md:
- Around line 70-83: The NewUserService constructor lacks dependency validation;
update the function signature to return (*UserService, error) and add nil checks
for repo (UserRepository) and logger (Logger) inside NewUserService, returning a
descriptive error (e.g., "nil repo" or "nil logger") when a dependency is
invalid, otherwise return the constructed *UserService and nil error; update
callers to handle the error accordingly.
- Around line 13-227: Add a new "How It Works" section to SKILL.md (placed near
the existing "When to Use" section) that concisely explains the mechanics and
intent of the documented patterns: summarize how Functional Options, Small
Interfaces, Dependency Injection, Concurrency Patterns (Worker Pool, Context
Propagation), Error Handling (wrapping, custom, sentinel), Package Organization,
and Testing Patterns operate in practice, what problems they solve, and quick
guidance on when to apply each; reference those section names in the text so
readers can jump to examples and keep the section to 3–6 short sentences
focusing on actionable mechanics rather than examples.
In @.kiro/skills/golang-testing/SKILL.md:
- Around line 157-160: The current grep-based coverage check using the command
starting with "go test -cover ./..." and matching 'coverage: [0-7][0-9]\.[0-9]%'
is brittle; replace it with a deterministic coverage-profile approach (e.g., run
"go test -coverprofile=profile.out ./...", then use "go tool cover
-func=profile.out" to read the "total:" line and parse the numeric coverage) or
recommend using a dedicated tool like gocov or a CI-native coverage gate; update
SKILL.md to show the robust sequence (generate profile, extract total percent
programmatically, compare against 80) and/or add a short note suggesting
gocov/CI solutions as alternatives.
In @.kiro/skills/postgres-patterns/SKILL.md:
- Around line 150-157: The document contains a duplicated section header "When
to Use This Skill" with the same bullet list as the existing "When to Activate"
section; remove the redundant "When to Use This Skill" header and its bullet
list so only the original "When to Activate" content remains, and verify any
internal links/TOC still point to the single remaining section (look for the
headers "When to Use This Skill" and "When to Activate" in SKILL.md to locate
and delete the duplicate block).
In @.kiro/skills/verification-loop/SKILL.md:
- Around line 125-128: The verification-loop skill duplicates checks already
performed by existing postToolUse hooks (notably quality-gate,
post:edit:typecheck, and post:edit:console-warn), so update SKILL.md to clarify
when to run this comprehensive verification versus relying on automatic hooks:
state that verification-loop is intended for full-project validation before PRs
or after major refactors, while incremental postToolUse hooks handle per-edit
lint/format/type/console checks; reference the hook names (quality-gate,
post:edit:typecheck, post:edit:console-warn) and hooks/hooks.json so readers can
see the overlap and avoid redundant runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 28704a2f-f794-43d8-a7a8-4fd653fe9a62
📒 Files selected for processing (18)
.kiro/skills/agentic-engineering/SKILL.md.kiro/skills/api-design/SKILL.md.kiro/skills/backend-patterns/SKILL.md.kiro/skills/coding-standards/SKILL.md.kiro/skills/database-migrations/SKILL.md.kiro/skills/deployment-patterns/SKILL.md.kiro/skills/docker-patterns/SKILL.md.kiro/skills/e2e-testing/SKILL.md.kiro/skills/frontend-patterns/SKILL.md.kiro/skills/golang-patterns/SKILL.md.kiro/skills/golang-testing/SKILL.md.kiro/skills/postgres-patterns/SKILL.md.kiro/skills/python-patterns/SKILL.md.kiro/skills/python-testing/SKILL.md.kiro/skills/search-first/SKILL.md.kiro/skills/security-review/SKILL.md.kiro/skills/tdd-workflow/SKILL.md.kiro/skills/verification-loop/SKILL.md
Comment on lines
+187
to
+206
| // SQL function in Supabase | ||
| CREATE OR REPLACE FUNCTION create_market_with_position( | ||
| market_data jsonb, | ||
| position_data jsonb | ||
| ) | ||
| RETURNS jsonb | ||
| LANGUAGE plpgsql | ||
| AS $ | ||
| BEGIN | ||
| -- Start transaction automatically | ||
| INSERT INTO markets VALUES (market_data); | ||
| INSERT INTO positions VALUES (position_data); | ||
| RETURN jsonb_build_object('success', true); | ||
| EXCEPTION | ||
| WHEN OTHERS THEN | ||
| -- Rollback happens automatically | ||
| RETURN jsonb_build_object('success', false, 'error', SQLERRM); | ||
| END; | ||
| $; | ||
| ``` |
Contributor
There was a problem hiding this comment.
Fix SQL function delimiter syntax.
The SQL function has incorrect delimiters. PostgreSQL requires $$ for function body delimiters, not $.
🐛 Proposed fix for SQL syntax
RETURNS jsonb
LANGUAGE plpgsql
-AS $
+AS $$
BEGIN
-- Start transaction automatically
INSERT INTO markets VALUES (market_data);
INSERT INTO positions VALUES (position_data);
RETURN jsonb_build_object('success', true);
EXCEPTION
WHEN OTHERS THEN
-- Rollback happens automatically
RETURN jsonb_build_object('success', false, 'error', SQLERRM);
END;
-$;
+$$;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // SQL function in Supabase | |
| CREATE OR REPLACE FUNCTION create_market_with_position( | |
| market_data jsonb, | |
| position_data jsonb | |
| ) | |
| RETURNS jsonb | |
| LANGUAGE plpgsql | |
| AS $ | |
| BEGIN | |
| -- Start transaction automatically | |
| INSERT INTO markets VALUES (market_data); | |
| INSERT INTO positions VALUES (position_data); | |
| RETURN jsonb_build_object('success', true); | |
| EXCEPTION | |
| WHEN OTHERS THEN | |
| -- Rollback happens automatically | |
| RETURN jsonb_build_object('success', false, 'error', SQLERRM); | |
| END; | |
| $; | |
| ``` | |
| // SQL function in Supabase | |
| CREATE OR REPLACE FUNCTION create_market_with_position( | |
| market_data jsonb, | |
| position_data jsonb | |
| ) | |
| RETURNS jsonb | |
| LANGUAGE plpgsql | |
| AS $$ | |
| BEGIN | |
| -- Start transaction automatically | |
| INSERT INTO markets VALUES (market_data); | |
| INSERT INTO positions VALUES (position_data); | |
| RETURN jsonb_build_object('success', true); | |
| EXCEPTION | |
| WHEN OTHERS THEN | |
| -- Rollback happens automatically | |
| RETURN jsonb_build_object('success', false, 'error', SQLERRM); | |
| END; | |
| $$; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.kiro/skills/backend-patterns/SKILL.md around lines 187 - 206, The PL/pgSQL
function create_market_with_position uses incorrect single-dollar delimiters;
change the function body delimiters from AS $ ... $; to AS $$ ... $$; so the
function declaration and ending dollar-quote match (i.e., use $$ before the
BEGIN and $$; after the END) ensuring the PostgreSQL parser accepts the block
for the create_market_with_position function.
Comment on lines
+1
to
+11
| --- | ||
| name: e2e-testing | ||
| description: > | ||
| Playwright E2E testing patterns, Page Object Model, configuration, CI/CD integration, artifact management, and flaky test strategies. | ||
| metadata: | ||
| origin: ECC | ||
| --- | ||
|
|
||
| # E2E Testing Patterns | ||
|
|
||
| Comprehensive Playwright patterns for building stable, fast, and maintainable E2E test suites. |
Contributor
There was a problem hiding this comment.
Add missing "When to Activate" section.
According to skill format conventions, all skills should include a "When to Activate" or "When to Use" section near the top to clarify activation criteria. Other skills in this PR consistently include this section (e.g., lines 15-22 in deployment-patterns, lines 16-22 in postgres-patterns).
Suggested addition after line 11:
## When to Activate
- Writing end-to-end tests for critical user flows
- Setting up Playwright test infrastructure
- Debugging flaky E2E tests
- Integrating E2E tests into CI/CD pipelines
- Testing Web3/wallet integration flows
- Validating financial or payment workflowsBased on learnings, skills must include 'When to Use', 'How It Works', and 'Examples' sections.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.kiro/skills/e2e-testing/SKILL.md around lines 1 - 11, Add a "When to
Activate" section immediately after the existing "E2E Testing Patterns" header
(i.e., below the top description) titled "## When to Activate" and include the
suggested bullet points (Writing end-to-end tests for critical user flows;
Setting up Playwright test infrastructure; Debugging flaky E2E tests;
Integrating E2E tests into CI/CD pipelines; Testing Web3/wallet integration
flows; Validating financial or payment workflows), and ensure the skill also
contains the required 'When to Use', 'How It Works', and 'Examples' sections
elsewhere in the document so it matches the other skills' format.
Comment on lines
+299
to
+302
| // ✅ useMemo for expensive computations | ||
| const sortedMarkets = useMemo(() => { | ||
| return markets.sort((a, b) => b.volume - a.volume) | ||
| }, [markets]) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "SKILL.md" -path "*frontend-patterns*" | head -20Repository: affaan-m/everything-claude-code
Length of output: 384
🏁 Script executed:
cat -n ./.kiro/skills/frontend-patterns/SKILL.md | sed -n '295,310p'Repository: affaan-m/everything-claude-code
Length of output: 610
Avoid in-place mutation in the memoization example.
Line 301 mutates markets via sort(), violating immutability principles. This pattern can cause bugs if markets comes from state/props. Use a shallow copy before sorting:
Proposed fix
const sortedMarkets = useMemo(() => {
- return markets.sort((a, b) => b.volume - a.volume)
+ return [...markets].sort((a, b) => b.volume - a.volume)
}, [markets])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // ✅ useMemo for expensive computations | |
| const sortedMarkets = useMemo(() => { | |
| return markets.sort((a, b) => b.volume - a.volume) | |
| }, [markets]) | |
| // ✅ useMemo for expensive computations | |
| const sortedMarkets = useMemo(() => { | |
| return [...markets].sort((a, b) => b.volume - a.volume) | |
| }, [markets]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.kiro/skills/frontend-patterns/SKILL.md around lines 299 - 302, The useMemo
example mutates the original markets array by calling markets.sort(), so update
the useMemo block (the sortedMarkets constant defined via useMemo) to sort a
shallow copy instead of markets directly; copy markets (e.g., using spread or
slice) before calling sort so the original markets prop/state is not mutated and
the memoization remains safe.
Comment on lines
+13
to
+421
| # Python Patterns | ||
|
|
||
| > This skill provides comprehensive Python patterns extending common design principles with Python-specific idioms. | ||
|
|
||
| ## Protocol (Duck Typing) | ||
|
|
||
| Use `Protocol` for structural subtyping (duck typing with type hints): | ||
|
|
||
| ```python | ||
| from typing import Protocol | ||
|
|
||
| class Repository(Protocol): | ||
| def find_by_id(self, id: str) -> dict | None: ... | ||
| def save(self, entity: dict) -> dict: ... | ||
|
|
||
| # Any class with these methods satisfies the protocol | ||
| class UserRepository: | ||
| def find_by_id(self, id: str) -> dict | None: | ||
| # implementation | ||
| pass | ||
|
|
||
| def save(self, entity: dict) -> dict: | ||
| # implementation | ||
| pass | ||
|
|
||
| def process_entity(repo: Repository, id: str) -> None: | ||
| entity = repo.find_by_id(id) | ||
| # ... process | ||
| ``` | ||
|
|
||
| **Benefits:** | ||
| - Type safety without inheritance | ||
| - Flexible, loosely coupled code | ||
| - Easy testing and mocking | ||
|
|
||
| ## Dataclasses as DTOs | ||
|
|
||
| Use `dataclass` for data transfer objects and value objects: | ||
|
|
||
| ```python | ||
| from dataclasses import dataclass, field | ||
| from typing import Optional | ||
|
|
||
| @dataclass | ||
| class CreateUserRequest: | ||
| name: str | ||
| email: str | ||
| age: Optional[int] = None | ||
| tags: list[str] = field(default_factory=list) | ||
|
|
||
| @dataclass(frozen=True) | ||
| class User: | ||
| """Immutable user entity""" | ||
| id: str | ||
| name: str | ||
| email: str | ||
| ``` | ||
|
|
||
| **Features:** | ||
| - Auto-generated `__init__`, `__repr__`, `__eq__` | ||
| - `frozen=True` for immutability | ||
| - `field()` for complex defaults | ||
| - Type hints for validation | ||
|
|
||
| ## Context Managers | ||
|
|
||
| Use context managers (`with` statement) for resource management: | ||
|
|
||
| ```python | ||
| from contextlib import contextmanager | ||
| from typing import Generator | ||
|
|
||
| @contextmanager | ||
| def database_transaction(db) -> Generator[None, None, None]: | ||
| """Context manager for database transactions""" | ||
| try: | ||
| yield | ||
| db.commit() | ||
| except Exception: | ||
| db.rollback() | ||
| raise | ||
|
|
||
| # Usage | ||
| with database_transaction(db): | ||
| db.execute("INSERT INTO users ...") | ||
| ``` | ||
|
|
||
| **Class-based context manager:** | ||
|
|
||
| ```python | ||
| class FileProcessor: | ||
| def __init__(self, filename: str): | ||
| self.filename = filename | ||
| self.file = None | ||
|
|
||
| def __enter__(self): | ||
| self.file = open(self.filename, 'r') | ||
| return self.file | ||
|
|
||
| def __exit__(self, exc_type, exc_val, exc_tb): | ||
| if self.file: | ||
| self.file.close() | ||
| return False # Don't suppress exceptions | ||
| ``` | ||
|
|
||
| ## Generators | ||
|
|
||
| Use generators for lazy evaluation and memory-efficient iteration: | ||
|
|
||
| ```python | ||
| def read_large_file(filename: str): | ||
| """Generator for reading large files line by line""" | ||
| with open(filename, 'r') as f: | ||
| for line in f: | ||
| yield line.strip() | ||
|
|
||
| # Memory-efficient processing | ||
| for line in read_large_file('huge.txt'): | ||
| process(line) | ||
| ``` | ||
|
|
||
| **Generator expressions:** | ||
|
|
||
| ```python | ||
| # Instead of list comprehension | ||
| squares = (x**2 for x in range(1000000)) # Lazy evaluation | ||
|
|
||
| # Pipeline pattern | ||
| numbers = (x for x in range(100)) | ||
| evens = (x for x in numbers if x % 2 == 0) | ||
| squares = (x**2 for x in evens) | ||
| ``` | ||
|
|
||
| ## Decorators | ||
|
|
||
| ### Function Decorators | ||
|
|
||
| ```python | ||
| from functools import wraps | ||
| import time | ||
|
|
||
| def timing(func): | ||
| """Decorator to measure execution time""" | ||
| @wraps(func) | ||
| def wrapper(*args, **kwargs): | ||
| start = time.time() | ||
| result = func(*args, **kwargs) | ||
| end = time.time() | ||
| print(f"{func.__name__} took {end - start:.2f}s") | ||
| return result | ||
| return wrapper | ||
|
|
||
| @timing | ||
| def slow_function(): | ||
| time.sleep(1) | ||
| ``` | ||
|
|
||
| ### Class Decorators | ||
|
|
||
| ```python | ||
| def singleton(cls): | ||
| """Decorator to make a class a singleton""" | ||
| instances = {} | ||
|
|
||
| @wraps(cls) | ||
| def get_instance(*args, **kwargs): | ||
| if cls not in instances: | ||
| instances[cls] = cls(*args, **kwargs) | ||
| return instances[cls] | ||
|
|
||
| return get_instance | ||
|
|
||
| @singleton | ||
| class Config: | ||
| pass | ||
| ``` | ||
|
|
||
| ## Async/Await | ||
|
|
||
| ### Async Functions | ||
|
|
||
| ```python | ||
| import asyncio | ||
| from typing import List | ||
|
|
||
| async def fetch_user(user_id: str) -> dict: | ||
| """Async function for I/O-bound operations""" | ||
| await asyncio.sleep(0.1) # Simulate network call | ||
| return {"id": user_id, "name": "Alice"} | ||
|
|
||
| async def fetch_all_users(user_ids: List[str]) -> List[dict]: | ||
| """Concurrent execution with asyncio.gather""" | ||
| tasks = [fetch_user(uid) for uid in user_ids] | ||
| return await asyncio.gather(*tasks) | ||
|
|
||
| # Run async code | ||
| asyncio.run(fetch_all_users(["1", "2", "3"])) | ||
| ``` | ||
|
|
||
| ### Async Context Managers | ||
|
|
||
| ```python | ||
| class AsyncDatabase: | ||
| async def __aenter__(self): | ||
| await self.connect() | ||
| return self | ||
|
|
||
| async def __aexit__(self, exc_type, exc_val, exc_tb): | ||
| await self.disconnect() | ||
|
|
||
| async with AsyncDatabase() as db: | ||
| await db.query("SELECT * FROM users") | ||
| ``` | ||
|
|
||
| ## Type Hints | ||
|
|
||
| ### Advanced Type Hints | ||
|
|
||
| ```python | ||
| from typing import TypeVar, Generic, Callable, ParamSpec, Concatenate | ||
|
|
||
| T = TypeVar('T') | ||
| P = ParamSpec('P') | ||
|
|
||
| class Repository(Generic[T]): | ||
| """Generic repository pattern""" | ||
| def __init__(self, entity_type: type[T]): | ||
| self.entity_type = entity_type | ||
|
|
||
| def find_by_id(self, id: str) -> T | None: | ||
| # implementation | ||
| pass | ||
|
|
||
| # Type-safe decorator | ||
| def log_call(func: Callable[P, T]) -> Callable[P, T]: | ||
| @wraps(func) | ||
| def wrapper(*args: P.args, **kwargs: P.kwargs) -> T: | ||
| print(f"Calling {func.__name__}") | ||
| return func(*args, **kwargs) | ||
| return wrapper | ||
| ``` | ||
|
|
||
| ### Union Types (Python 3.10+) | ||
|
|
||
| ```python | ||
| def process(value: str | int | None) -> str: | ||
| match value: | ||
| case str(): | ||
| return value.upper() | ||
| case int(): | ||
| return str(value) | ||
| case None: | ||
| return "empty" | ||
| ``` | ||
|
|
||
| ## Dependency Injection | ||
|
|
||
| ### Constructor Injection | ||
|
|
||
| ```python | ||
| class UserService: | ||
| def __init__( | ||
| self, | ||
| repository: Repository, | ||
| logger: Logger, | ||
| cache: Cache | None = None | ||
| ): | ||
| self.repository = repository | ||
| self.logger = logger | ||
| self.cache = cache | ||
|
|
||
| def get_user(self, user_id: str) -> User | None: | ||
| if self.cache: | ||
| cached = self.cache.get(user_id) | ||
| if cached: | ||
| return cached | ||
|
|
||
| user = self.repository.find_by_id(user_id) | ||
| if user and self.cache: | ||
| self.cache.set(user_id, user) | ||
|
|
||
| return user | ||
| ``` | ||
|
|
||
| ## Package Organization | ||
|
|
||
| ### Project Structure | ||
|
|
||
| ``` | ||
| project/ | ||
| ├── src/ | ||
| │ └── mypackage/ | ||
| │ ├── __init__.py | ||
| │ ├── domain/ # Business logic | ||
| │ │ ├── __init__.py | ||
| │ │ └── models.py | ||
| │ ├── services/ # Application services | ||
| │ │ ├── __init__.py | ||
| │ │ └── user_service.py | ||
| │ └── infrastructure/ # External dependencies | ||
| │ ├── __init__.py | ||
| │ └── database.py | ||
| ├── tests/ | ||
| │ ├── unit/ | ||
| │ └── integration/ | ||
| ├── pyproject.toml | ||
| └── README.md | ||
| ``` | ||
|
|
||
| ### Module Exports | ||
|
|
||
| ```python | ||
| # __init__.py | ||
| from .models import User, Product | ||
| from .services import UserService | ||
|
|
||
| __all__ = ['User', 'Product', 'UserService'] | ||
| ``` | ||
|
|
||
| ## Error Handling | ||
|
|
||
| ### Custom Exceptions | ||
|
|
||
| ```python | ||
| class DomainError(Exception): | ||
| """Base exception for domain errors""" | ||
| pass | ||
|
|
||
| class UserNotFoundError(DomainError): | ||
| """Raised when user is not found""" | ||
| def __init__(self, user_id: str): | ||
| self.user_id = user_id | ||
| super().__init__(f"User {user_id} not found") | ||
|
|
||
| class ValidationError(DomainError): | ||
| """Raised when validation fails""" | ||
| def __init__(self, field: str, message: str): | ||
| self.field = field | ||
| self.message = message | ||
| super().__init__(f"{field}: {message}") | ||
| ``` | ||
|
|
||
| ### Exception Groups (Python 3.11+) | ||
|
|
||
| ```python | ||
| try: | ||
| # Multiple operations | ||
| pass | ||
| except* ValueError as eg: | ||
| # Handle all ValueError instances | ||
| for exc in eg.exceptions: | ||
| print(f"ValueError: {exc}") | ||
| except* TypeError as eg: | ||
| # Handle all TypeError instances | ||
| for exc in eg.exceptions: | ||
| print(f"TypeError: {exc}") | ||
| ``` | ||
|
|
||
| ## Property Decorators | ||
|
|
||
| ```python | ||
| class User: | ||
| def __init__(self, name: str): | ||
| self._name = name | ||
| self._email = None | ||
|
|
||
| @property | ||
| def name(self) -> str: | ||
| """Read-only property""" | ||
| return self._name | ||
|
|
||
| @property | ||
| def email(self) -> str | None: | ||
| return self._email | ||
|
|
||
| @email.setter | ||
| def email(self, value: str) -> None: | ||
| if '@' not in value: | ||
| raise ValueError("Invalid email") | ||
| self._email = value | ||
| ``` | ||
|
|
||
| ## Functional Programming | ||
|
|
||
| ### Higher-Order Functions | ||
|
|
||
| ```python | ||
| from functools import reduce | ||
| from typing import Callable, TypeVar | ||
|
|
||
| T = TypeVar('T') | ||
| U = TypeVar('U') | ||
|
|
||
| def pipe(*functions: Callable) -> Callable: | ||
| """Compose functions left to right""" | ||
| def inner(arg): | ||
| return reduce(lambda x, f: f(x), functions, arg) | ||
| return inner | ||
|
|
||
| # Usage | ||
| process = pipe( | ||
| str.strip, | ||
| str.lower, | ||
| lambda s: s.replace(' ', '_') | ||
| ) | ||
| result = process(" Hello World ") # "hello_world" | ||
| ``` | ||
|
|
||
| ## When to Use This Skill |
Contributor
There was a problem hiding this comment.
Add explicit How It Works and Examples headings to meet skill format requirements.
The document has strong content, but it currently lacks explicit sections named “How It Works” and “Examples” (Line 13 onward). Please add those headings so the skill format is unambiguous for contributors and loaders.
Suggested structural patch
# Python Patterns
> This skill provides comprehensive Python patterns extending common design principles with Python-specific idioms.
+## How It Works
+
+This skill presents Python-first implementation patterns grouped by concern
+(typing, modeling, resource management, async, packaging, and errors), with
+copyable snippets and practical guidance.
+
## Protocol (Duck Typing)
@@
-## Protocol (Duck Typing)
+## Examples
+
+### Protocol (Duck Typing)
@@
-## Dataclasses as DTOs
+### Dataclasses as DTOs
@@
-## Context Managers
+### Context Managers
@@
-## Generators
+### Generators
@@
-## Decorators
+### Decorators
@@
-## Async/Await
+### Async/Await
@@
-## Type Hints
+### Type Hints
@@
-## Dependency Injection
+### Dependency Injection
@@
-## Package Organization
+### Package Organization
@@
-## Error Handling
+### Error Handling
@@
-## Property Decorators
+### Property Decorators
@@
-## Functional Programming
+### Functional ProgrammingBased on learnings: Skills must be formatted as Markdown files with clear sections including 'When to Use', 'How It Works', and 'Examples'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.kiro/skills/python-patterns/SKILL.md around lines 13 - 421, The SKILL.md is
missing explicit "How It Works" and "Examples" headings required by skill
format; add a "How It Works" section explaining the core patterns (Protocol,
Dataclasses, Context Managers, Generators, Decorators, Async/Await, Type Hints,
Dependency Injection, Package Organization, Error Handling, Property Decorators,
Functional Programming) and an "Examples" section that contains short runnable
snippets or references to the existing examples (e.g., the Protocol example,
dataclass DTOs, context manager examples, generator/ decorator/async examples) —
place these headings prominently after the summary/introduction under the main
"Python Patterns" title (near the existing examples and before or adjacent to
"When to Use This Skill") so loaders and contributors can find them.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
37 issues found across 18 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".kiro/skills/coding-standards/SKILL.md">
<violation number="1" location=".kiro/skills/coding-standards/SKILL.md:380">
P2: Nested triple-backtick fence inside a triple-backtick block can break markdown rendering; use a longer outer fence for this section.</violation>
<violation number="2" location=".kiro/skills/coding-standards/SKILL.md:402">
P2: The “GOOD” React memoization example mutates the input array with `sort`, which violates immutability and can cause prop/state side effects.</violation>
</file>
<file name=".kiro/skills/security-review/SKILL.md">
<violation number="1" location=".kiro/skills/security-review/SKILL.md:161">
P2: Authorization example dereferences a potentially null requester before access control check, which can cause runtime errors instead of a controlled 403 deny response.</violation>
<violation number="2" location=".kiro/skills/security-review/SKILL.md:220">
P2: The CSP example marked as a security best practice allows 'unsafe-eval' and 'unsafe-inline' for scripts, which weakens CSP protections and can enable XSS. A security guide should avoid recommending these directives unless absolutely necessary.</violation>
<violation number="3" location=".kiro/skills/security-review/SKILL.md:344">
P1: Solana wallet verification example uses incorrect API/encoding assumptions (`verify` import + base64 key/signature), likely producing non-working ownership checks.</violation>
</file>
<file name=".kiro/skills/backend-patterns/SKILL.md">
<violation number="1" location=".kiro/skills/backend-patterns/SKILL.md:87">
P2: Service example calls `findByIds`, but the documented `MarketRepository` interface does not define it, creating an inconsistent contract that won’t type-check.</violation>
<violation number="2" location=".kiro/skills/backend-patterns/SKILL.md:183">
P2: Transaction helper only checks RPC error and ignores business-failure payloads (`success: false`) returned by the SQL exception handler.</violation>
<violation number="3" location=".kiro/skills/backend-patterns/SKILL.md:194">
P2: The SQL function example uses invalid PL/pgSQL dollar-quote delimiters (`AS $ ... $;`), making the documented function non-executable.</violation>
<violation number="4" location=".kiro/skills/backend-patterns/SKILL.md:341">
P2: `fetchWithRetry` can throw `undefined` when `maxRetries <= 0` because `lastError` is only assigned inside the loop but unconditionally thrown afterward.</violation>
<violation number="5" location=".kiro/skills/backend-patterns/SKILL.md:382">
P2: Auth and RBAC examples use incompatible user models (`JWTPayload` vs `User`), so `requirePermission` passes the wrong shape into permission checks and handlers.</violation>
</file>
<file name=".kiro/skills/golang-patterns/SKILL.md">
<violation number="1" location=".kiro/skills/golang-patterns/SKILL.md:100">
P2: This example can deadlock with an unbuffered `results` channel because it waits synchronously for workers to finish before returning. Close `results` from a separate goroutine so callers can start consuming after `workerPool(...)` returns.</violation>
<violation number="2" location=".kiro/skills/golang-patterns/SKILL.md:196">
P2: Table-driven subtest example captures loop variable without rebinding, which can produce incorrect subtest inputs on pre-1.22 Go.</violation>
</file>
<file name=".kiro/skills/deployment-patterns/SKILL.md">
<violation number="1" location=".kiro/skills/deployment-patterns/SKILL.md:224">
P2: CI/CD template pushes to GHCR with `GITHUB_TOKEN` but omits explicit `packages: write` permission, which can break image pushes in repos with restricted default token permissions.</violation>
<violation number="2" location=".kiro/skills/deployment-patterns/SKILL.md:279">
P2: Detailed health endpoint example is marked internal but exposed without access controls while returning sensitive operational details.</violation>
</file>
<file name=".kiro/skills/frontend-patterns/SKILL.md">
<violation number="1" location=".kiro/skills/frontend-patterns/SKILL.md:189">
P2: `useQuery` example couples `useEffect` to unstable `options`/`fetcher` identities, which can cause repeated refetching when used with inline arguments (as shown).</violation>
<violation number="2" location=".kiro/skills/frontend-patterns/SKILL.md:301">
P2: The `useMemo` example mutates `markets` in place via `sort()`, teaching a React state/props mutation anti-pattern.</violation>
<violation number="3" location=".kiro/skills/frontend-patterns/SKILL.md:588">
P2: Dropdown Enter handler can pass `undefined` to `onSelect` when `options` is empty.</violation>
</file>
<file name=".kiro/skills/tdd-workflow/SKILL.md">
<violation number="1" location=".kiro/skills/tdd-workflow/SKILL.md:199">
P2: E2E guidance uses a fixed Playwright sleep (`waitForTimeout`), which promotes flaky and slower tests; prefer condition-based waiting/assertions.</violation>
<violation number="2" location=".kiro/skills/tdd-workflow/SKILL.md:307">
P2: Jest coverage example uses `coverageThresholds` (plural), which can prevent the documented 80% coverage gate from being enforced.</violation>
</file>
<file name=".kiro/skills/python-testing/SKILL.md">
<violation number="1" location=".kiro/skills/python-testing/SKILL.md:186">
P2: The pytest marker example uses `sys.version_info` without importing `sys`, making the snippet invalid when copied.</violation>
<violation number="2" location=".kiro/skills/python-testing/SKILL.md:222">
P2: Patched-class mock assertion targets the class mock instead of the instance (`return_value`).</violation>
<violation number="3" location=".kiro/skills/python-testing/SKILL.md:257">
P2: Coverage config example incorrectly states it works for `pytest.ini` and `pyproject.toml` while using TOML-only syntax, which can lead to invalid `pytest.ini` configuration.</violation>
</file>
<file name=".kiro/skills/docker-patterns/SKILL.md">
<violation number="1" location=".kiro/skills/docker-patterns/SKILL.md:263">
P2: Secret Management YAML example defines duplicate top-level `services` keys, producing an invalid/unsafe Compose example that can override prior service definitions or fail parsing.</violation>
</file>
<file name=".kiro/skills/golang-testing/SKILL.md">
<violation number="1" location=".kiro/skills/golang-testing/SKILL.md:48">
P2: Table-driven subtest example closes over loop variable `tt` without rebinding, which can produce incorrect subtest behavior on pre-Go-1.22 (especially with parallel subtests).</violation>
<violation number="2" location=".kiro/skills/golang-testing/SKILL.md:159">
P2: Coverage gate regex misses single-digit percentages (<10%), allowing very low coverage to bypass the documented 80% threshold check.</violation>
</file>
<file name=".kiro/skills/python-patterns/SKILL.md">
<violation number="1" location=".kiro/skills/python-patterns/SKILL.md:156">
P2: The type-hints decorator example uses `@wraps(func)` without importing `wraps`, so the snippet fails when run independently.</violation>
<violation number="2" location=".kiro/skills/python-patterns/SKILL.md:173">
P2: The singleton class-decorator example replaces the decorated class with a function, breaking class/type semantics (`isinstance`, subclass/introspection expectations).</violation>
<violation number="3" location=".kiro/skills/python-patterns/SKILL.md:223">
P2: Async context manager usage is shown at module scope; it should be inside an async function and run via an event loop.</violation>
</file>
<file name=".kiro/skills/e2e-testing/SKILL.md">
<violation number="1" location=".kiro/skills/e2e-testing/SKILL.md:90">
P2: Static screenshot artifact paths in examples are unsafe with parallel/multi-project Playwright runs and can overwrite artifacts.</violation>
</file>
<file name=".kiro/skills/search-first/SKILL.md">
<violation number="1" location=".kiro/skills/search-first/SKILL.md:60">
P2: The skill’s decision model is internally inconsistent: the workflow omits `Compose` while the decision matrix includes it, creating ambiguous guidance for agents.</violation>
</file>
<file name=".kiro/skills/verification-loop/SKILL.md">
<violation number="1" location=".kiro/skills/verification-loop/SKILL.md:26">
P1: Verification commands pipe into `head`/`tail` without `pipefail`, which can mask failing checks and produce false PASS results.</violation>
<violation number="2" location=".kiro/skills/verification-loop/SKILL.md:71">
P2: Security verification is overly narrow (few patterns and JS/TS-only scope), which can produce false PASS results and under-report risk.</violation>
<violation number="3" location=".kiro/skills/verification-loop/SKILL.md:82">
P2: Diff review command is scoped to the last commit, so multi-commit PR changes can be missed.</violation>
<violation number="4" location=".kiro/skills/verification-loop/SKILL.md:105">
P2: The skill marks work as "READY for PR" without checking CI/check-run status or merge-conflict status, so it can incorrectly signal readiness under team-blocking conditions.</violation>
</file>
<file name=".kiro/skills/postgres-patterns/SKILL.md">
<violation number="1" location=".kiro/skills/postgres-patterns/SKILL.md:71">
P2: Supabase-specific `auth.uid()` is documented as a general PostgreSQL RLS pattern, which is not portable and can fail in standard PostgreSQL.</violation>
<violation number="2" location=".kiro/skills/postgres-patterns/SKILL.md:84">
P2: Cursor pagination example uses non-PostgreSQL named `$last_id` placeholder, which can fail when executed as SQL.</violation>
<violation number="3" location=".kiro/skills/postgres-patterns/SKILL.md:133">
P2: The config template uses global `ALTER SYSTEM` defaults with fixed values, which is unsafe guidance for copy/paste and can cause operational regressions.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| #### Wallet Verification | ||
| ```typescript | ||
| import { verify } from '@solana/web3.js' |
Contributor
There was a problem hiding this comment.
P1: Solana wallet verification example uses incorrect API/encoding assumptions (verify import + base64 key/signature), likely producing non-working ownership checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .kiro/skills/security-review/SKILL.md, line 344:
<comment>Solana wallet verification example uses incorrect API/encoding assumptions (`verify` import + base64 key/signature), likely producing non-working ownership checks.</comment>
<file context>
@@ -0,0 +1,497 @@
+
+#### Wallet Verification
+```typescript
+import { verify } from '@solana/web3.js'
+
+async function verifyWalletOwnership(
</file context>
| ### Phase 1: Build Verification | ||
| ```bash | ||
| # Check if project builds | ||
| npm run build 2>&1 | tail -20 |
Contributor
There was a problem hiding this comment.
P1: Verification commands pipe into head/tail without pipefail, which can mask failing checks and produce false PASS results.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .kiro/skills/verification-loop/SKILL.md, line 26:
<comment>Verification commands pipe into `head`/`tail` without `pipefail`, which can mask failing checks and produce false PASS results.</comment>
<file context>
@@ -0,0 +1,128 @@
+### Phase 1: Build Verification
+```bash
+# Check if project builds
+npm run build 2>&1 | tail -20
+# OR
+pnpm build 2>&1 | tail -20
</file context>
|
|
||
| // ✅ GOOD: Memoize expensive computations | ||
| const sortedMarkets = useMemo(() => { | ||
| return markets.sort((a, b) => b.volume - a.volume) |
Contributor
There was a problem hiding this comment.
P2: The “GOOD” React memoization example mutates the input array with sort, which violates immutability and can cause prop/state side effects.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .kiro/skills/coding-standards/SKILL.md, line 402:
<comment>The “GOOD” React memoization example mutates the input array with `sort`, which violates immutability and can cause prop/state side effects.</comment>
<file context>
@@ -0,0 +1,532 @@
+
+// ✅ GOOD: Memoize expensive computations
+const sortedMarkets = useMemo(() => {
+ return markets.sort((a, b) => b.volume - a.volume)
+}, [markets])
+
</file context>
| * @throws {Error} If OpenAI API fails or Redis unavailable | ||
| * | ||
| * @example | ||
| * ```typescript |
Contributor
There was a problem hiding this comment.
P2: Nested triple-backtick fence inside a triple-backtick block can break markdown rendering; use a longer outer fence for this section.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .kiro/skills/coding-standards/SKILL.md, line 380:
<comment>Nested triple-backtick fence inside a triple-backtick block can break markdown rendering; use a longer outer fence for this section.</comment>
<file context>
@@ -0,0 +1,532 @@
+ * @throws {Error} If OpenAI API fails or Redis unavailable
+ *
+ * @example
+ * ```typescript
+ * const results = await searchMarkets('election', 5)
+ * console.log(results[0].name) // "Trump vs Biden"
</file context>
| where: { id: requesterId } | ||
| }) | ||
|
|
||
| if (requester.role !== 'admin') { |
Contributor
There was a problem hiding this comment.
P2: Authorization example dereferences a potentially null requester before access control check, which can cause runtime errors instead of a controlled 403 deny response.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .kiro/skills/security-review/SKILL.md, line 161:
<comment>Authorization example dereferences a potentially null requester before access control check, which can cause runtime errors instead of a controlled 403 deny response.</comment>
<file context>
@@ -0,0 +1,497 @@
+ where: { id: requesterId }
+ })
+
+ if (requester.role !== 'admin') {
+ return NextResponse.json(
+ { error: 'Unauthorized' },
</file context>
| ### Phase 5: Security Scan | ||
| ```bash | ||
| # Check for secrets | ||
| grep -rn "sk-" --include="*.ts" --include="*.js" . 2>/dev/null | head -10 |
Contributor
There was a problem hiding this comment.
P2: Security verification is overly narrow (few patterns and JS/TS-only scope), which can produce false PASS results and under-report risk.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .kiro/skills/verification-loop/SKILL.md, line 71:
<comment>Security verification is overly narrow (few patterns and JS/TS-only scope), which can produce false PASS results and under-report risk.</comment>
<file context>
@@ -0,0 +1,128 @@
+### Phase 5: Security Scan
+```bash
+# Check for secrets
+grep -rn "sk-" --include="*.ts" --include="*.js" . 2>/dev/null | head -10
+grep -rn "api_key" --include="*.ts" --include="*.js" . 2>/dev/null | head -10
+
</file context>
| } | ||
| }() | ||
| } | ||
| wg.Wait() |
Contributor
There was a problem hiding this comment.
P2: This example can deadlock with an unbuffered results channel because it waits synchronously for workers to finish before returning. Close results from a separate goroutine so callers can start consuming after workerPool(...) returns.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .kiro/skills/golang-patterns/SKILL.md, line 100:
<comment>This example can deadlock with an unbuffered `results` channel because it waits synchronously for workers to finish before returning. Close `results` from a separate goroutine so callers can start consuming after `workerPool(...)` returns.</comment>
<file context>
@@ -0,0 +1,227 @@
+ }
+ }()
+ }
+ wg.Wait()
+ close(results)
+}
</file context>
|
|
||
| **Cursor Pagination:** | ||
| ```sql | ||
| SELECT * FROM products WHERE id > $last_id ORDER BY id LIMIT 20; |
Contributor
There was a problem hiding this comment.
P2: Cursor pagination example uses non-PostgreSQL named $last_id placeholder, which can fail when executed as SQL.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .kiro/skills/postgres-patterns/SKILL.md, line 84:
<comment>Cursor pagination example uses non-PostgreSQL named `$last_id` placeholder, which can fail when executed as SQL.</comment>
<file context>
@@ -0,0 +1,161 @@
+
+**Cursor Pagination:**
+```sql
+SELECT * FROM products WHERE id > $last_id ORDER BY id LIMIT 20;
+-- O(1) vs OFFSET which is O(n)
+```
</file context>
| **RLS Policy (Optimized):** | ||
| ```sql | ||
| CREATE POLICY policy ON orders | ||
| USING ((SELECT auth.uid()) = user_id); -- Wrap in SELECT! |
Contributor
There was a problem hiding this comment.
P2: Supabase-specific auth.uid() is documented as a general PostgreSQL RLS pattern, which is not portable and can fail in standard PostgreSQL.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .kiro/skills/postgres-patterns/SKILL.md, line 71:
<comment>Supabase-specific `auth.uid()` is documented as a general PostgreSQL RLS pattern, which is not portable and can fail in standard PostgreSQL.</comment>
<file context>
@@ -0,0 +1,161 @@
+**RLS Policy (Optimized):**
+```sql
+CREATE POLICY policy ON orders
+ USING ((SELECT auth.uid()) = user_id); -- Wrap in SELECT!
+```
+
</file context>
|
|
||
| -- Timeouts | ||
| ALTER SYSTEM SET idle_in_transaction_session_timeout = '30s'; | ||
| ALTER SYSTEM SET statement_timeout = '30s'; |
Contributor
There was a problem hiding this comment.
P2: The config template uses global ALTER SYSTEM defaults with fixed values, which is unsafe guidance for copy/paste and can cause operational regressions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .kiro/skills/postgres-patterns/SKILL.md, line 133:
<comment>The config template uses global `ALTER SYSTEM` defaults with fixed values, which is unsafe guidance for copy/paste and can cause operational regressions.</comment>
<file context>
@@ -0,0 +1,161 @@
+
+-- Timeouts
+ALTER SYSTEM SET idle_in_transaction_session_timeout = '30s';
+ALTER SYSTEM SET statement_timeout = '30s';
+
+-- Monitoring
</file context>
mecemis
pushed a commit
to mecemis/everything-claude-code
that referenced
this pull request
Mar 25, 2026
Co-authored-by: Sungmin Hong <hsungmin@amazon.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part 3 of 4 — Kiro IDE support broken into smaller PRs as requested in #548.
Adds 18 skills invocable via Kiro's
/menu:tdd-workflow, coding-standards, security-review, verification-loop, api-design, frontend-patterns, backend-patterns, e2e-testing, golang-patterns, golang-testing, python-patterns, python-testing, database-migrations, postgres-patterns, docker-patterns, deployment-patterns, search-first, agentic-engineering.
Contributors
Testing
Summary by cubic
Adds 18 new Kiro IDE skills accessible from the
/menu to guide design, testing, security, and engineering workflows. Part 3 of 4 for Kiro support, enabling consistent, stack-aware guidance across the codebase.SKILL.mdskills:SKILL.mdschema (Core Concepts, Examples, Best Practices, When to Use); language skills includeglobsfor context..kiro/skillsand are invocable via the/menu.Written for commit bfabe56. Summary will update on new commits.
Summary by CodeRabbit