# Runtime Pattern Checklist (RP1-RP37)

This file contains the canonical list of recurring production bug patterns that code review and unit tests consistently miss. The REVIEW stage references this file as a BLOCKING check -- every applicable pattern must be explicitly verified or marked N/A.

## Patterns

| # | Pattern | Trigger (when to check) | What to verify | Example bug |
|---|---------|------------------------|----------------|-------------|
| RP1 | **subprocess timeout orphan** | `create_subprocess_exec` or `subprocess.Popen` | `asyncio.wait_for` timeout path has `proc.kill()` + `await proc.wait()` BEFORE re-raise | ffmpeg runs forever after 30s timeout (run_c2881d2f) |
| RP2 | **subprocess missing binary** | `create_subprocess_exec("some_binary", ...)` | `FileNotFoundError` caught -- user-friendly error message | ffmpeg not installed -- raw 500 instead of "install ffmpeg" (run_c2881d2f) |
| RP3 | **React hook cleanup** | new `use*` hook with refs or subscriptions | `useEffect` return releases all resources (streams, timers, listeners) | Mic never released on tab close (run_c2881d2f) |
| RP4 | **stale closure** | callback passed to hook/async uses component state | use `useRef` to hold latest value, read `.current` in callback | transcribed text overwrites user's typing (run_c2881d2f) |
| RP5 | **FormData Content-Type** | `new FormData()` sent via axios/fetch | NO explicit `Content-Type` header -- browser must add boundary | Multipart broken, backend gets 400 (run_c2881d2f) |
| RP6 | **setTimeout leak** | `setTimeout` in component/hook | timer ID stored in ref, cleared in cleanup effect | setState on unmounted component (run_c2881d2f) |
| RP7 | **error msg / constant mismatch** | error messages containing numeric values | message matches the actual threshold/constant in code | "need 0.5s" but constant is 1.0s (run_c2881d2f) |
| RP8 | **hardcoded env assumption** | region, URL, port, path as string literal | configurable via env var or parameter with sensible default | us-east-1 hardcoded, user in us-west-2 (run_c2881d2f) |
| RP9 | **API boundary naming** | new endpoint returns JSON consumed by frontend | backend snake_case fields have camelCase frontend interface + conversion function | `duration_ms` passed raw to TS instead of `durationMs` (Kiro review) |
| RP10 | **barrel export** | new hook, component, or service file created | exported from the directory's `index.ts` | `useVoiceRecorder` missing from `hooks/index.ts` (Kiro review) |
| RP11 | **SDK handler reassignment** | `self._handler = new_handler` or similar | old handler `.close()` called BEFORE reassignment; reset to `None` before restart after crash | Old SocketModeHandler leaked on reconnect (Kiro review) |
| RP12 | **unstable callback refs** | inline arrow functions passed as props to hooks | wrap in `useCallback` with correct deps; use ref for values that change every render | `onTranscript`/`onError` recreated every render (Kiro review) |
| RP13 | **state machine completeness** | type/enum declares states + design doc shows transitions | every declared state has >=1 code path that enters it AND exits it; every transition has a trigger | Voice `interrupted` state declared but no code ever sets it; VAD never calls `stopRecording()` -- `listening->processing` transition unreachable (2026-04-25 E2E review) |
| RP14 | **cross-service parameter mismatch** | function A calls service B with parameters mapped from A's inputs | parameter names/semantics match B's API contract; override values don't conflict with defaults | `voice_id="Zhiyu"` + `language="en-US"` -- Polly rejects because Zhiyu requires `zh-CN` (2026-04-25 E2E review) |
| RP15 | **setTimeout for state propagation** | `setTimeout(() => fn(), 0)` to sequence React state + side effects | verify the ref/state the deferred fn reads is set BEFORE the timeout, not by a concurrent React render | Voice send `setTimeout(0)` reads `inputValueRef` which could be overwritten by React re-render before timeout fires (2026-04-25 E2E review) |
| RP16 | **concurrent async without ordering** | N async calls fired in a loop with `.then()` enqueue | responses may resolve out of order; use sequential `await` or `Promise.all` + indexed insert | 3 concurrent Polly calls -- sentence 2 (short) resolves before sentence 1 (long) -- audio plays out of order (2026-04-25 E2E review) |
| RP17 | **unsanitized string in structured format** | user/dynamic text embedded in HTML, JSON template, SQL, or shell | escape for target format: `html.escape()`, `json.dumps()[1:-1]`, parameterized query, `shlex.quote()` | `<pre>{message}</pre>` with raw message -- XSS in email; `payload_template.replace("{title}", title)` -- JSON parse failure on quotes (2026-04-25 E2E review) |
| RP18 | **timezone-sensitive date boundary** | SQL uses `date('now')`, `datetime('now')`, or `strftime(...,'now')` for filtering (e.g. "today's records") | write and query use the same timezone; desktop apps should use local time (`datetime.now()`), not SQLite UTC `date('now')`. Tests always pass because test runner and DB share a timezone -- the bug only appears when user is in a different TZ from UTC. | `WHERE date(timestamp) = date('now')` but timestamps written in UTC -- "today" boundary is 8h off for UTC+8 user, tokens after midnight local show as yesterday (run_6823b0d4 E2E review) |
| RP19 | **deprecated stdlib API** | `asyncio.get_event_loop()`, `datetime.utcnow()`, `logging.warn()`, `imp.reload()` | use Python 3.12+ recommended replacements: `get_running_loop()` / `get_event_loop_policy().get_event_loop()`, `datetime.now(UTC)`, `logging.warning()`. Deprecated APIs emit warnings and may raise `RuntimeError` in future Python versions. | `asyncio.get_event_loop().create_task()` in async generator -- DeprecationWarning in 3.12, potential RuntimeError in 3.13+ (run_6823b0d4 E2E review) |
| RP20 | **nested clickable event propagation** | `<button>` or `onClick` handler inside another element with `onClick` or `<li onClick>` wrapping action buttons | inner handler calls `e.stopPropagation()` -- or event WILL bubble to outer handler, firing both actions on one click | Todo ✅ button inside `<li onClick={onItemClick}>` -- clicking "mark done" also populated ChatInput with the todo (Briefing Hub v2 review) |
| RP21 | **popover toggle + click-outside race** | popover with `mousedown` click-outside handler + a toggle button that opens/closes it | toggle uses `onMouseDown` + `e.stopPropagation()` (fires before click-outside), or click-outside handler excludes the toggle button via ref. Using `onClick` for toggle + `mousedown` for click-outside = race: close fires first, toggle reopens. | History search popover flickers on re-click -- click-outside closes it, then toggle button `onClick` reopens it in the same event cycle (Briefing Hub v2 review) |
| RP22 | **React ref/state desync on programmatic input** | code calls `setState(value)` for a value that is ALSO read via `someRef.current` in a synchronous path (e.g., send handler reads `inputValueRef.current`) | ref is synced alongside state: `ref.current = value; setState(value)`. If only state is set, any synchronous consumer within the same microtask reads the stale ref. React state updates are async; refs are immediate. | `handleItemClick` called `setInputValue(text)` but not `inputValueRef.current = text` -- fast ⌘Enter within the same render frame sent the previous message (Briefing Hub v2 PE review) |
| RP23 | **conditional layout with empty children** | CSS grid/flex container with N columns/rows where children are conditionally rendered (`{hasX && <Card/>}`) | when one side renders 0 children, the container still allocates space (empty column in `grid-cols-2`, empty flex child). Either: collapse to single-column when one side is empty, or use `display: contents` wrapper. | 2-col Welcome Screen -- only Signals on right, left column empty -- 50% of screen is blank (Briefing Hub v2 review) |
| RP24 | **cross-language serialization format assumption** | Language A produces JSON/text consumed by Language B (e.g., Python FastAPI → Rust parser, Node → Python, Go → TS) | verify the **exact byte-level format** each side produces/expects. Python `json.dumps()` uses `": "` (space after colon); compact JSON has `":"`. Rust/Go manual string matching, regex, or `split(":")` will fail on the spacing variant. Test with the **actual serializer output**, not a hand-crafted string. | Rust `get_daemon_version()` searched for `"version":"` (no space), FastAPI returned `"version": "1.8.4"` (with space) — version always `None` — daemon restarted every launch (run_91a6fb7e E2E review) |
| RP25 | **blast radius — system lifecycle not traced** | changeset touches infra, release, deploy, CI, or cross-service config | after diff review, list ALL system-level flows this changeset participates in (build→package→deploy→update→run). For EACH flow, trace the complete chain and verify every link is compatible. Specifically check: (1) existing code that CONSUMES what you built (SSM update script, CI workflow, provisioner) — even if you didn't modify it. (2) config propagation — does a new file/format reach all consumers? (3) cleanup/reload — if config format changed, does the runtime pick up the new config without restart? | Unified release pipeline (run_19129544): added Hive tar.gz packaging but didn't check SSM update script — it didn't reload Caddy after config changes. Added CI job but made it depend on unrelated macOS build (15 min wasted). Added rsync excludes but missed .DS_Store. All 5 bugs outside the diff, inside the system. (2026-04-29 DevOps audit) |
| RP26 | **deprecated API from current docs** | changeset uses framework-specific patterns (React hooks, FastAPI decorators, boto3 calls, Pydantic models, asyncio patterns) | for each non-trivial framework pattern, verify against **current official documentation** (not training data). Detect version from pyproject.toml/package.json. Check: (1) is this the currently recommended API? (2) are there deprecation warnings in the docs? (3) does the docs show a different/newer pattern? Source hierarchy: official docs > official blog > MDN/web.dev. Never trust Stack Overflow, tutorials, or training data as primary source. | `asyncio.get_event_loop()` deprecated in 3.12+ — passed pipeline because no check verified against current docs. `date('now')` UTC mismatch — SQLite docs clearly state this returns UTC, but training data habit used it for local time. Both from run_6823b0d4 E2E review. (2026-04-29 Agent Skills Adoption) |
| RP27 | **non-deterministic output ordering** | function returns list, dict, or set that callers display or compare | output ordering is either (1) explicitly sorted, (2) insertion-ordered (dict/list), or (3) documented as unordered. `set()` iteration, `dict.keys()` from hash-dependent sources (DB rows, JSON parse), or `os.listdir()` produce unstable order across runs. Callers that `assertEqual` or display to user will flake or confuse. | Audio sentences from 3 concurrent Polly `synthesize_speech()` calls — responses resolved in network-latency order, not sentence order — playback scrambled (C011, 2026-04-25). Inspired by Kiro PE review "Determinism" category. |
| RP28 | **schema migration without rollback path** | changeset adds/modifies DB table, column, index, or runs `ALTER TABLE` | migration has a documented rollback: either (1) `ALTER TABLE ... DROP COLUMN` reverse step, (2) migration is additive-only (new nullable column, new table), or (3) blue-green/shadow-write strategy for destructive changes. `NOT NULL` on existing column without backfill = data loss on rollback. Check: can we deploy the previous version after this migration runs? | No real bug yet — preventive pattern from Kiro PE review "Schema Evolution" category + industry best practice. SQLite `ALTER TABLE` is limited (no DROP COLUMN before 3.35.0) — verify SQLite version supports the operation. |
| RP29 | **YAGNI — unnecessary abstraction layer** | new interface/protocol/ABC with ≤1 concrete implementation, or new config/flag for a behavior that could be hardcoded | count concrete implementations. 0 = dead interface. 1 = premature abstraction (delete interface, use concrete type). New config key with only 1 possible value = unnecessary indirection. Ask: "if I delete this abstraction, does the code get simpler?" If yes, delete it. | Pytest safety hook v1: 130 lines with mutex, auto-rewrite, and 3 abstraction layers. After 4 rounds of user pushback, final version was 55 lines with zero abstractions (C009, 2026-04-12). Memory intelligence: 515-line 5-dimension weighted scoring, reduced to 60-line frequency gate after design review (KD07, 2026-04-22). |
| RP30 | **hook no-op path scaling** | code added to a per-session/per-request hook, cron, or recurring callback | analyze the **no-op path** (hook fires, finds nothing to do, exits): (1) what's the cost when it does NOTHING? (2) does that cost scale with data history or just recent data? (3) if O(total_history): add a time/size bound (mtime filter, index file, cursor/watermark). The no-op path is 99% of invocations — optimizing the action path while ignoring no-op cost is a structural blind spot. | Auto-cultivation hook scanned ALL 141 run.json files every session to find 0 uncultivated runs. Pipeline REVIEW + adversarial both missed it because they only analyzed the action path (correctness of cultivation). PE review caught the O(n) steady-state cost. Fix: 30-day mtime filter bounds scan to O(recent). (run_d73239fe, 2026-05-14) |
| RP31 | **synthetic-only test data** | feature processes data from an external source (DB, JSONL, API, user input) | at least one test uses **production-representative data** — not data crafted by the developer to match the implementation. Synthetic inputs validate the code path; real inputs validate real-world utility. RED flag: all test inputs were written by the person who wrote the code AND match the internal heuristic perfectly. | DDD T2 keyword classifier: 29 tests pass with synthetic corrections containing magic words ("daemon", "nc -z"). All 5 real corrections from production JSONL (narrative, behavioral) returned None — 100% false negative. Feature "worked" but delivered zero value. (run_76273219, 2026-05-15 PE review) |
| RP32 | **re-filtering curated input** | function receives input from a curated source (LLM extraction, human editor, upstream validator) AND applies its own relevance filter | if upstream already curated (existence = relevance), downstream should **route** (where does it go), not **re-filter** (should it go at all). A weaker heuristic downstream silently drops valid inputs that passed a stronger upstream gate. Anti-pattern: keyword regex gate on top of LLM extraction gate = double-filtering that removes signal. | `cultivate_from_corrections()` applied `_classify_lesson()` keyword gate to corrections already curated by DailyActivity extraction LLM. Corrections without technical keywords (100% of real corrections) silently rejected. Fix: corrections bypass keyword minimum, fallback to IMPROVEMENT.md "What Failed". (run_76273219, 2026-05-15 PE review) |

| RP33 | **multi-shape function return (branching code paths)** | function has `try/except`, `if feature_flag`, or fallback branch that produces a **different output dict shape** per path | ALL consumer tests must cover ALL return shapes. The function's docstring or type annotation must describe the union of shapes. If shape A has `{"docs": {"PRODUCT.md": {"exists": bool}}}` and shape B has `{"docs": {"PRODUCT.md": {"sections": {...}}}}`, tests using shape A will pass locally while CI (which has the import for shape B) hits KeyError. Specifically: (1) list every branch that produces output, (2) for each consumer test, verify which branch it exercises, (3) any untested branch = finding. | `_collect_ddd_health()` has `try: import compute_section_health` (5-dimensional output, no `exists` key per doc) / `except ImportError:` fallback (staleness-only, with `exists` key). Test created minimal fixture → CI imported the real module → dict shape mismatch → `KeyError: 'PRODUCT.md'`. 1735 tests pass, 1 fails — and it's the shape-mismatch branch. (v1.13.0 release CI, 2026-05-16) |
| RP34 | **shell variable scope across Bash tool calls** | a multi-stage script/skill sets a variable (e.g., `NEW_VERSION=...`) in one Bash call that a later Bash call references via `${VAR}` | each `Bash` tool call is an **independent shell** — variables from prior calls do NOT exist. Every Bash block that uses a variable must either: (1) re-derive it (`NEW_VERSION=$(cat VERSION)`), (2) read it from a file/env, or (3) receive it as a function argument. Specifically: grep for `${...}` or `$VAR` in the script block, trace where that variable is SET — if the set is in a DIFFERENT Bash call, it's a guaranteed bug. | `s_swarm-release` Stage 5 DMG check used `${NEW_VERSION}` which was set in Stage 2's Bash call. Stage 5 runs in a separate invocation → variable undefined → check always fails or silently expands to empty string → wrong DMG path. Fix: `NEW_VERSION=$(cat VERSION)` at top of Stage 5 block. (v1.13.0 release PE review, 2026-05-16) |
| RP35 | **shared executor/pool contention** | code adds `asyncio.to_thread()`, `run_in_executor(None, ...)`, or any blocking call (>1s) dispatched to a thread pool | enumerate ALL consumers of the same pool. If the new consumer can block >5s (subprocess wait, network call, file lock) AND the pool has latency-sensitive consumers (health endpoints, DB queries, SSE keepalives), the new consumer MUST use a **dedicated** `ThreadPoolExecutor`. Default asyncio pool is SHARED — one 480s block starves everything. Specifically: (1) list all `to_thread`/`run_in_executor(None)` calls in the module + imports, (2) find the longest-blocking one, (3) if max_block > 5s AND pool has <1s consumers → finding. | Job scheduler's `subprocess.run(timeout=480s)` ran on default asyncio pool. Health endpoint's `aiosqlite` also uses default pool. 4 concurrent jobs filled 8 default workers → health endpoint hung → Tauri showed "not responding". Fix: `_job_executor = ThreadPoolExecutor(4)` dedicated pool. (commit 8d9895c1, 2026-05-20) |
| RP36 | **fix-removes-failure-mode regression** | changeset fixes a bug that previously caused early failure (circuit breaker, bad path, missing config, wrong state) on a code path | ask: "Now that this path SUCCEEDS, what new load/behavior does it introduce? What shared resources does the newly-successful path consume?" A path that previously failed at line 3 now runs to line 300 — everything between line 3 and 300 just became reachable for the first time. Check: memory, thread pool slots, file handles, DB connections, network sockets consumed by the now-successful path. | b812e9e2 fixed job paths + reset circuit breakers → jobs successfully spawn 480s CLI subprocesses for first time → default thread pool exhausted → health endpoint hang. The fix was correct; the regression was what the fix ENABLED. Same class: removing a `return` guard, lowering a threshold, resetting a circuit breaker. (2026-05-20, LL01) |
| RP37 | **process tree lifetime mismatch** | code spawns a subprocess (build, test, long computation) from within a session/request that has a shorter lifetime | if the spawner can be evicted/killed (session timeout, tab close, app restart) while the child is still running, the child dies with it (SIGKILL propagates down process tree). Verify: (1) is the child's expected runtime > spawner's minimum lifetime? (2) if yes, child must either detach (`setsid`/`nohup`) or be spawned by a longer-lived process (daemon, systemd). The right fix is usually architectural: long-running ops belong on the daemon/job system, not the session. | PyInstaller build (120-300s) spawned as session child process. Session evicted after 60s idle → build receives SIGKILL (exit 137). Looked like OOM for weeks. Correct fix: spawn from daemon TaskRunner (not session), or detach from process group. (C031, 2026-05-20) |

## Output Format

For each applicable pattern, one line:
```
RP1: pass -- proc.kill() in timeout handler + finally (voice_transcribe.py:92,112)
RP3: pass -- useEffect cleanup releases stream + recorder (useVoiceRecorder.ts:58-68)
RP5: N/A -- no FormData in this changeset
```

## Maintenance

The REFLECT stage is responsible for maintaining this file. When post-pipeline review (E2E, external, or user feedback) finds bugs the pipeline missed:

1. If the bug fits an existing RP pattern -- investigate why the checklist missed it and tighten the verification criteria above.
2. If the bug is a new pattern -- **append a new row** (RP30, RP31, etc.) to the table above. Include: trigger condition, what to verify, and the real bug as the example.
3. Update the pattern count in `backend/skills/s_autonomous-pipeline/stages/review.md` check #6 if the total changes.

This ensures the pipeline learns from every review cycle. Without this step, lessons live in IMPROVEMENT.md but never reach the checklist that would prevent recurrence.
