diff --git a/.agents/skills/performance-engineering/SKILL.md b/.agents/skills/performance-engineering/SKILL.md index 6bcd7123..eeef8359 100644 --- a/.agents/skills/performance-engineering/SKILL.md +++ b/.agents/skills/performance-engineering/SKILL.md @@ -27,6 +27,34 @@ Do not optimize against a toy fixture when the report provides production scale. ## Workflow +### 0. Trust The Measurement Before Trusting The Number + +A measurement setup that is wrong produces clean, confident, wrong numbers, and +a clean number ends an investigation. Establish validity first. + +**Prove the environment is not throttled.** Chrome stops producing frames and +throttles timers for windows it considers backgrounded or occluded, headless or +not. A capture taken that way reports near-zero rendering work no matter what +the page does. Disable background/occlusion throttling at launch and measure +frame liveness inside the capture. The same applies to any environment that +idles when unobserved. + +**Prove zero is a measurement.** A metric reading zero, absent, or perfectly +quiet is a claim that requires evidence, because a disabled instrument reports +exactly the same thing. `RunTask` only appears under the disabled-by-default +timeline category; a scenario opened for the wrong directory renders nothing at +all. Before believing a quiet result, confirm the instrument fired and the +workload actually ran: assert on an independent signal, such as DOM growth +alongside the application's own render counters. + +**Prove the workload is comparable.** When the stimulus varies in size between +runs, per-second and total figures are not comparable. Normalise by units of +work delivered, and check run-to-run spread on an unchanged build before +attributing any difference to a change. + +Do not report a number whose validity you have not established. State which +validity checks ran. + ### 1. Reproduce And Measure - Reproduce the exact interaction, not a nearby helper in isolation. @@ -37,6 +65,24 @@ Do not optimize against a toy fixture when the report provides production scale. Do not infer a bottleneck from code appearance when a trace or counter can identify it. +**Never accept an "after" without a "before" on the identical scenario and +build.** Measuring a fixed build against a remembered number, a different +scenario, or a nearby baseline proves nothing: the mechanism you changed may +not even execute in the path you measured. Re-run the unchanged build through +the same scenario, however inconvenient the rebuild. Expect to discover that a +plausible fix changes nothing. + +**A sampling profiler cannot explain native work.** Self time attributed to +`(program)` says only that the time was not in interpreted JavaScript. Use the +timeline trace, which names parsing, style recalculation, layout, layerization, +paint, and raster, and reserve the sampler for attributing application code. + +**Reproduction may require production scale you do not have.** A threshold +effect is invisible below its threshold, and a development workspace is usually +below it. When a report will not reproduce, compare the reporter's scale +against yours on the specific dimension the code keys on before concluding the +bug is absent. + Profiling identifies where time is spent; it does not prove behavioral equivalence. Separately verify the applicable state, identity, layout, and lifecycle transitions for every structural optimization. ### 2. Write The Cost Equation @@ -149,6 +195,24 @@ Add a cache only when all are explicit: A cache inside an `O(consumers × entities × candidates)` loop is a mitigation, not automatically a complete fix. +## Repository Tooling + +Three unattended capture commands exist; prefer them over ad-hoc timing code, +and extend them when a scenario is missing rather than measuring by hand. + +| Command | Answers | +|---|---| +| `bun run profile:idle` | What the app does while nobody interacts with it. Supports `--session`, `--tab`, `--then-tab`, `--panel`, `--expand-projects` to reach a specific mounted state, plus `--baseline` and `--budget-*` for regression gating. | +| `bun run profile:session` | What a streaming assistant response costs. Creates a session, dispatches a prompt through the `openchamber session` CLI, and records until the session reports idle. Reports the long-task distribution, a timeline-trace breakdown, running animations, and output-normalised metrics. | +| `bun run profile:browser` | A manually driven capture when the interaction cannot be scripted. | + +Both automated commands fail loudly rather than reporting a clean result when +the renderer was throttled, the trace collected no tasks, or the scenario never +rendered. Keep that property when extending them. + +Measure a production build. A development build's render and bundle behaviour +does not represent what users run. + ## Verification Require both correctness and performance guards: @@ -169,6 +233,32 @@ Require both correctness and performance guards: State what was not measured. Never claim a freeze is fixed from type-check and unit tests alone. +## Revert What You Cannot Measure + +A change that does not move its target metric is not a small win, a safety +improvement, or a cleanup. It is unvalidated complexity, and shipping it under +a performance rationale makes the next investigation harder by implying the +path was already optimised. Revert it and record the hypothesis as rejected. + +This applies to a change whose benefit appears only in reasoning, one measured +against the wrong baseline, and one whose measured scenario turns out to behave +identically without it. + +Report negative results explicitly. "Disabling this removed 40% of the +layerization, and the fix that preserved the visuals did not" is a finding, and +the next person needs it. + +## Know When To Stop + +Compare the remaining cost against the user-facing budget, not against zero. +When the interaction already sits far inside budget, further optimisation of +that path trades real regression risk for an invisible gain, and it displaces +work on the path the user actually reported. Say so and move on. + +Cost that comes from intentional, user-visible behaviour is not waste. Removing +it is a product decision, not a performance fix, and it needs the owner's +agreement rather than a quiet commit. + ## Hotfix Policy Ship a bounded cache-only or local mitigation under deadline pressure only when: @@ -192,9 +282,16 @@ If the interaction remains above budget, do not call the mitigation the complete | "Move it to a worker" | Moving waste changes responsiveness, not total cost or data correctness. | | "Empty means nothing exists" | Empty after failure or partial loading is not authoritative absence. | | "We can optimize later" | Add a scale regression now or the multiplier will return. | +| "The profile is clean" | Prove the instrument fired and the renderer was not throttled. A disabled instrument looks identical to a fast app. | +| "It is much faster now" | Against which baseline, on which build, in which scenario? Re-run the unchanged build. | +| "Most of the time is `(program)`" | The sampler cannot see native work. Read the timeline trace. | +| "It does not reproduce here" | Compare your scale to the reporter's on the dimension the code keys on. | +| "It cannot hurt to keep the change" | An unmeasured change is unvalidated complexity that hides the path from the next investigation. | ## Exit Checklist +- [ ] Measurement validity established: no throttling, instruments confirmed firing, workload comparable. +- [ ] Baseline captured from the unchanged build through the identical scenario. - [ ] Exact interaction and production scale reproduced. - [ ] Cost equation written and dominant multipliers removed. - [ ] Sources of truth, completeness, and invalidation explicit. @@ -205,4 +302,6 @@ If the interaction remains above budget, do not call the mitigation the complete - [ ] Operation-count or repeated-event regression test prevents recurrence. - [ ] Structural optimizations have transition-focused correctness coverage independent of performance measurements. - [ ] When mount topology or activation boundaries change, instrumentation distinguishes those transitions from steady state. +- [ ] Every change retained is justified by a measured difference; unvalidated ones reverted and recorded as rejected. +- [ ] Remaining cost compared against the budget, and stopping justified when inside it. - [ ] Correctness, type, lint, and relevant runtime validations pass. diff --git a/.agents/skills/sync-state-invariants/SKILL.md b/.agents/skills/sync-state-invariants/SKILL.md index 9aa3fec0..c6cd720a 100644 --- a/.agents/skills/sync-state-invariants/SKILL.md +++ b/.agents/skills/sync-state-invariants/SKILL.md @@ -97,6 +97,28 @@ For streaming-frequency work, also load `performance-engineering`. - Key runtime-scoped caches by runtime identity when IDs or paths can collide. - Clean optimistic and local cache state after partial failures. +### Never Evict What Is In Use + +An entry acquired during render but protected only after commit is unprotected +for the whole render pass. Eviction that runs on acquisition therefore disposes +entries that are actively mounting; the next render recreates them in a loading +state, which issues another fetch, which repeats forever. The symptom is an +endless request loop and sawtoothing listeners, heap, and CPU, and it appears +only once live entries outnumber the limit, so it never reproduces on a small +workspace. + +- Define what protects an entry from eviction, and prove that protection is in + place before eviction can observe the entry, not one commit later. +- Treat capacity as a soft target. Overflowing briefly is always cheaper than + evict/recreate cycles; bound the cache with idle-time eviction instead. +- Never run an eviction scan on the acquisition path. Coalesce it into one + deferred pass so a render mounting many entries scans once, not once per + entry. +- Keep explicit lifecycle edges, such as the last consumer releasing an entry, + synchronous. Deferring those changes an observable contract. +- Raising a limit is a workaround, not a fix. It relocates the cliff and hides + the loop from everyone whose workload is smaller than the new number. + ## Persisted Snapshot Ordering When state exists in memory and one or more persistent stores, define an explicit authority and ordering protocol: @@ -136,5 +158,6 @@ Cover the relevant lifecycle, not only static state: - New session lookup assumes SSE already indexed it. - Optimistic data has no shadow entry or rollback. - Snapshot-difference cleanup treats its first startup snapshot as a disappearance event. +- Eviction runs on the acquisition path, or a cache limit is raised in response to a request loop. - Missing or malformed persistence becomes authoritative empty state. - Debounced writes are canceled on owner/lifecycle change without completing against the captured owner or an explicit durability/data-loss contract.