docs(skills): record the measurement traps this investigation hit

Every wrong turn in this work came from trusting a number whose validity had
not been established: a throttled renderer reporting zero rendering work, a
trace category that was never enabled reporting zero long tasks, a scenario
that rendered nothing reporting a perfectly quiet profile, and an 'after'
measured without a matching 'before', which made a no-op change look like a
fix.

The performance skill now puts measurement validity ahead of measurement,
requires a baseline from the unchanged build through the identical scenario,
directs native work to the timeline trace rather than the sampler, requires
unvalidated changes to be reverted and recorded as rejected, and says when to
stop optimising a path that is already inside budget. It also points at the
repository's capture commands.

The sync skill gains the invariant behind the cache-thrash loop: an entry
acquired during render but protected only after commit is unprotected for the
whole render pass, capacity should be a soft target, eviction must not run on
the acquisition path, and raising a limit relocates a cliff instead of removing
it.
This commit is contained in:
Bohdan Triapitsyn
2026-08-03 17:05:56 +03:00
parent 262b1eb18a
commit d3576117da
2 changed files with 122 additions and 0 deletions
@@ -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.
@@ -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.