defect-matrix

# Skill: defect-matrix Used to systematically audit a feature for hidden defects, cross-reference with proven OSS implementations, and produce a prioritized improvement matrix — **before** implementing any changes. ## When to trigger - User explicitly asks: "결함 찾아", "audit", "what could go wrong", "find more issues", "improvement proposals", "비교해서 결함 도출" - User has a working feature but suspects edge cases - User wants to validate UX patterns against industry conventions - Just before signing off on a feature ("is this done?") Do NOT trigger when: - User just wants something built (no audit asked) - The change is a one-line bug fix (audit overhead > value) - The user explicitly says "just implement it" ## Process ### Step 1 — Scope the audit Identify the feature boundary explicitly. Examples: - "Read state + count badge on swimlane chips" - "WebSocket reconnection logic" - "Form validation in NewSessionForm" State the scope back to the user in one line. Don't audit beyond it. ### Step 2 — Enumerate failure modes Walk through these axes deliberately. Skip if not applicable, but DON'T forget to consider: - **Hooks order / state management** - useState lazy init runs once per mount (frozen across prop changes) - useEffect dep array misses (missing items → stale closure; extra items → over-fire) - early return BEFORE all hooks → React #310 - **Race conditions / async boundaries** - markX-on-effect vs markX-on-handler timing - in-flight setState → next handler reads old value - cleanup handlers running after unmount - **Off-by-one in cursors / indices** - `>` vs `>=` on cursor comparison - inclusive vs exclusive bounds on slice/filter - **Empty / loading / error states** - 0 items → does it crash, return null, or render placeholder? - First render before data → flash of wrong content? - Network error after partial data → stale state? - **Data invariants** - Append-only (transcript.jsonl) → can't retro-fix old records - Monotonic (cursor, ULID) → strictly forward-only? - Idempotent ops → multiple invocations safe? - **Cross-component state sync** - Two components reading same store → both subscribed? - One component writes, others read → stale until re-render? - **Persistence / storage** - localStorage 5MB quota - sessionStorage cleared on tab close - cleanup of stale entries - **Multi-tab / multi-instance coherence** - storage event listener present? - BroadcastChannel for richer signal? - Conflict resolution (last-write-wins vs CRDT)? - **Keyboard / accessibility** - Focus trap in modal - Tab navigation order - Esc / Enter / Arrow key handlers - aria-label, role, aria-selected - **Performance** - O(n²) per render - CSS animation on element with high paint cost - useMemo dep instability (new object each render) - **UX consistency with proven OSS** - Does behavior match user's mental model from Slack / GitHub / Linear / Sentry? #### Runtime / dataflow / multi-process axes (backend) Add these when auditing dispatchers, reducers, transcript writers, env-stamping paths, or anything that crosses a process boundary. Crumb / LangGraph / Temporal-shaped systems live or die on these. - **Multi-callsite divergence** — when N call sites compute the same logical value (a binding tuple, a metadata stamp, an enum-to-string mapping), each site drifts independently as PRs bolt on patches. Symptom: PR-X fixes site 1, PR-X+3 fixes site 2 the same way, PR-X+7 fixes site 3 still the same way, all citing the same root cause. Cure: extract one helper, prove each callsite is byte-equivalent, replace. CLAUDE.md "≥ 2 real call sites" rule applies in *reverse* — duplicated logic across 5 sites is the duplication that justified extraction since iteration 2. - **Comment-vs-code divergence** — a comment claims invariant X holds ("tool.call carries adapter_substituted") but the code below doesn't honor it. Comments rot; PRs amend behavior on one path and skip another. Grep the claim against the code; flag the mismatch as a defect, not a doc fix — the gap is operational. - **Schema field claimed but never consumed** — `metadata.intended_adapter` is emitted but no reader (Studio / OTel / scorer / verifier) reads it. Dead schema accumulates: every event pays the byte cost, replay re-derives nothing useful, schema grows brittler. Cure: prove ≥ 1 consumer or delete the emit. - **Stamping path symmetry** — when there are two paths to set the same metadata (dispatcher direct emit vs subprocess env → `stampEnvMetadata`), the two paths must stamp identical fields. Asymmetry causes "Studio chip flickers as latest event alternates between dispatcher and actor sources" (PR-SubstitutionEnv 2026-05-06 fault). Diff the two stamp call sites field-by-field. - **Append-only / replay determinism** — every helper introduced into a reducer or pre-effect path MUST be referentially transparent: no `Date.now()`, no `crypto.randomUUID()`, no fs reads of mutable state. The `crumb replay` invariant means a helper that uses ambient time once will produce divergent transcripts on second replay. Cure: pass time/random as explicit parameters, mock in tests. - **D1-D6 source-of-truth violations (Crumb-specific)** — D2/D6 must come from `qa-check-effect`, never an LLM. D3/D5 must combine `verifier-llm` + `reducer-auto` halves in `combineDimScore()`, never just one. Any new code path that touches `scores.D*` must declare its source explicitly. AGENTS.md invariant #4-5. - **Anti-deception forgery surfaces** — `from=system` and `kind=qa.result` are dispatcher-only. Any new `writer.append()` callsite that an LLM-driven actor can reach via subprocess MUST go through `applyEventFirewall` (`src/transcript/firewall.ts`). Adding a 6th writer.append in cli.ts/dispatcher without the firewall is a fresh forgery surface. - **Cross_provider metadata wiring** — `kind=judge.score` must stamp `metadata.cross_provider = (verifier_provider !== builder_provider)`. AGENTS.md §136. Any change to the verifier spawn path that drops the `builderProvider` env or `readLatestBuildProvider()` lookup silently breaks Rule 4 self-bias detection. - **Wall-clock / token / spawn-count budget interaction** — `state.done` triggered by budget cap (`budget_exhausted=true`) must NOT block verifier-tail events (`judge.score / verify.result / handoff.rollback`); pre-PR-Task61b this caused "verifier said FAIL but builder never got the retry" limbo. Any new `if (state.done)` gate in the reducer or coordinator must add the `isBudgetDrain` exemption or document why not. - **Single-writer lease / multi-coordinator races** — two coordinators on the same session double-spawn from one handoff event. Any new write path that bypasses `acquireLease()` (background script, debug helper, mcp tool) is a fresh race surface. - **Prompt cache carry-over (forward-compat)** — `metadata.adapter_session_id` + `cache_carry_over` exist but no adapter consumes them yet. Adding a feature that depends on them without first wiring the adapter is a footgun. ### Step 3 — Cross-reference with OSS Pick 3-5 mature OSS projects with similar functionality. Skim their actual code. For chat / read-state / inbox patterns: - **Mattermost** (Go + React) — server-authoritative `last_viewed_at`, channel-scoped - **Element / matrix-react-sdk** (TS + React) — `m.read` receipts, room-scoped, server-side - **Zulip** (Python + JS) — per-message read tracking, `unread_message_ids` Set - **Rocket.Chat** (Meteor + React) — DB `lr` field, room-scoped - **Linear** (closed but observable via DevTools) — `markIssueRead` mutation, IntersectionObserver-driven - **GitHub Notifications** (closed) — thread-scoped cursor, `is:unread` query filter - **Sentry** (Python + React) — fingerprint grouping, per-user `GroupSeen` boolean - **Mastodon** (Ruby + React) — server `last_read_id` + localStorage cache - **Discord** (closed) — `read_state` per channel, last_message_id cursor - **Slack** (closed) — `last_read_ts` per channel, marker-line UX For other domains, swap in equivalents (e.g. CodeMirror / Monaco for editor, react-flow / dagre for graph layout). For multi-agent dispatcher / event-sourced backends (Crumb-shape): - **LangGraph** (Python + JS) — `StateGraph` + reducer + `Command` effect; checkpointer for replay. Source: github.com/langchain-ai/langgraph - **Temporal** (Go) — workflow event history, deterministic replay, side-effect via `activity`. Source: github.com/temporalio/temporal - **AutoGen v0.4** (Python) — `RoutedAgent` + `MessageContext` + topic-based dispatch. Source: github.com/microsoft/autogen - **Magentic-One** (Python) — orchestrator → specialist with private context windows + ledger pattern. Source: github.com/microsoft/autogen/tree/main/python/packages/autogen-magentic-one - **Anthropic Claude Code** (closed but public docs) — `Task` tool spawn depth=1, sandwich injection via `--append-system-prompt`, sub-agent context isolation. Source: docs.claude.com/en/docs/claude-code - **OpenAI Swarm / Agents SDK** (Python) — handoff abstraction, agent context passing. Source: github.com/openai/swarm For each backend reference, note: - State shape (event log / DB rows / in-memory map) - Effect dispatch (sync / async / queued) - Replay strategy (full / from-checkpoint / none) - Cross-process coordination (lock / lease / consensus / none) - The specific file/function that handles the relevant logic, if known For each OSS reference, note: - Storage location (server / localStorage / IndexedDB / memory) - Data structure (cursor / Set / boolean / timestamp) - Mark-as-read trigger (auto on view / explicit click / scroll / dwell) - Decrement granularity (batch / per-item) - Cross-device sync strategy - The specific file/function that handles the relevant logic, if known ### Step 4 — Build the matrix table ```markdown ## 🔴 High — immediately user-visible wrong signal | # | Defect | Location | Symptom | OSS comparison | Improvement | |---|---|---|---|---|---| | D1 | <one-line> | <file:line> | <user sees> | <OSS does X> | <our fix> | ## 🟡 Medium — accumulates as friction | # | ... | ... | ## 🟢 Low — polish | # | ... | ## 🔵 Info — checked but not a defect - <list non-defects with brief justification> ``` ### Step 5 — Prioritize + propose Group defects into: - **Immediate** (impact × low-effort): suggest applying first - **Next-PR candidates**: separate scope, clear value - **Defer**: low impact or high effort Suggest a sequence with explicit build-and-verify checkpoints between groups. ### Step 6 — Get user confirmation Present the matrix + priority list. Wait for user pick. **Do not implement until they confirm.** If user says "go" / "진행해", apply in priority order with build + restart between groups. ### Step 7 — Sequential application For each group: 1. Apply changes 2. Typecheck (`npx tsc -p tsconfig.json --noEmit`) 3. Build (`npm run build`) 4. Restart studio / re-run tests 5. Brief verification (curl / read smoke test / key path) 6. Report new bundle hash so user can hard-refresh 7. Move to next group only after current is verified ## Output style - Use the matrix table format (it scales — small audits use 1 group, big ones use 4) - Locations should be `file:line` (clickable in most editors) - "OSS comparison" should cite the actual project + behavior, not generic "industry standard" - Be terse in cells but precise — if a defect needs more than one line, link to a separate section below ## Example invocation that triggered codification (2026-05-04) User asked to audit Crumb Studio's read-state + count-badge feature. Process produced 11 defects + 3 priority groups + 4 OSS references (Matrix `m.read`, Sentry `GroupSeen`, Linear `markIssueRead`, Slack marker-line). Two specific defects directly traced to mismatched assumptions vs OSS: 1. `useState(() => init)` snapshot frozen across prop changes → matrix-react-sdk uses `componentDidUpdate(prevProps)` to re-snapshot 2. `markRead` on useEffect[cur] auto-fired on mount → Zulip / Sentry / Linear all only advance on explicit gesture Both fixes were single-component changes < 20 LOC each, but neither would have surfaced without the OSS cross-reference step. ## Backend extension example (2026-05-06) User asked to audit `src/dispatcher/live.ts` 1500+ LoC `case 'spawn'` for hidden defects while extracting a god-object. The runtime axes above surfaced: 1. **Multi-callsite divergence** — binding-tuple resolution `adapterSubstituted ? fallback : (binding ?? fallback)` was duplicated in 5 sites (pre-spawn note / agent.wake / adapter.spawn args / tool.call onProgress / agent.stop). PR-LeakAudit-O1/O2/O4/O6/O7/O8 + PR-SubstitutionStamp + PR-SubstitutionEnv each patched 4 of 5 sites; sub-PR cycle was inevitable. Cure: extract `resolveSpawnBinding()` once. 2. **Comment-vs-code divergence** — `_shared.ts:121` claimed tool.call carried `adapter_substituted` flag; grep confirmed tool.call's metadata block omitted it. Studio's NodeInspector chip flickered when latest event alternated between wake/stop (correct) and tool.call (missing). 3. **Schema field claimed but never consumed** — `metadata.intended_adapter` emitted only in pre-spawn note's `data` field; zero consumers across `src/`, `packages/studio/`. Dead schema candidate (kept in body string for human readability instead). Both #1 and #2 were structural defects the LongCodeBench-aligned 18-case-switch principle did NOT cover — the principle says "single file good," not "single helper per N callsites." The two skills compose: keep the file, extract the truly-duplicated tuple. ## Anti-patterns - ❌ Audit while implementing — always finish enumeration first, then pick what to do - ❌ Rely solely on memory of "industry standards" without citing actual projects - ❌ Treat lint/type warnings as defects (they're hygiene; this skill is for behavioral correctness) - ❌ Skip the priority pass — a flat list of 20 defects loses the "do these 3 first" signal - ❌ Get permission once, then snowball into out-of-scope rewrites