Omar SobhandClaude Opus 5
4358964c05
55 of 85 role skill bindings pointed at skills that were never authored,
so 10 of 11 team templates bound a smaller context bundle than their role
prompts assumed. Three roles bound nothing at all (gpu.bench_engineer,
threejs.shader_author, threejs.perf_engineer) while their prompts described
procedures they had no way to read.
The loader comment at team_template_loader.rs:167 already diagnosed this —
snake_case slugs in TOML against kebab-case skill files — and it was
half-fixed: the kebab names were corrected, the snake_case ones left.
It was invisible because both existing tests assert authored ⊆ referenced
(30/30, green) and the second explicitly declines to check the other
direction. So the failing half was the half nobody asserted.
Resolved every name by one of three explicit choices:
- 23 skills authored where the role genuinely needed the procedure
(gpu, threejs, research, analysis, frontend, mobile, backend, platform)
- renames onto authored skills where one existed in substance, including
the four-near-duplicate cases that collapse onto one real skill
- 22 aspirational references deleted — a binding an agent cannot read is
a promise, not a capability
Two tests now hold it. The unit test checks referenced ⊆ authored against
the files. The new integration test runs both loaders in boot order and
asserts the bindings survive the trip through the database, which is a
different question: resolution goes through skills_catalog rows, so a skill
file that exists but fails to ingest still leaves the role empty.
Negative controls: the unit test failed naming all 55; the integration test
fails naming the exact role when one name is reverted.
threejs.shader_author and .perf_engineer gained a second and third skill
after the collapse — pin_in_context pins idx < 2, so a role left with one
skill silently pins less than the policy intends.
Co-Authored-By: Claude Opus 5 <[email protected]>
56 lines
2.2 KiB
Markdown
56 lines
2.2 KiB
Markdown
---
|
||
name: criterion-benchmarking
|
||
description: Writing benchmarks whose numbers mean something — warmup, distributions, and the changes that are noise.
|
||
when_to_use: You are asked to benchmark a change, or to show a performance claim is real.
|
||
tags: [gpu, rust, benchmarking]
|
||
---
|
||
|
||
# A benchmark is an experiment
|
||
|
||
Most performance claims fail not because the code is slow but because the
|
||
measurement cannot support the claim.
|
||
|
||
## What criterion does for you
|
||
|
||
`criterion` runs the routine many times, discards warmup, and reports a
|
||
confidence interval rather than a single number. Take that seriously: if the
|
||
intervals for before and after overlap, **you have not measured an improvement**,
|
||
whatever the point estimates say.
|
||
|
||
```rust
|
||
fn bench_search(c: &mut Criterion) {
|
||
let index = build_index(10_000); // setup OUTSIDE the timed closure
|
||
c.bench_function("search/10k", |b| {
|
||
b.iter(|| index.search(black_box(&query), 10))
|
||
});
|
||
}
|
||
```
|
||
|
||
Two mistakes this shape avoids:
|
||
- **Setup inside `iter`** measures the setup. If the setup must be per-iteration,
|
||
use `iter_batched` so it is excluded.
|
||
- **A missing `black_box`** lets the optimiser delete the work entirely. A
|
||
benchmark that got 400× faster after a refactor usually got deleted, not
|
||
optimised.
|
||
|
||
## Report the shape, not the headline
|
||
|
||
"34.7% faster" invites a follow-up question the number cannot answer. Give the
|
||
distribution, the input size, and the machine. A change that is 30% faster at
|
||
10k elements and 5% slower at 10M is a trade-off, and only the sweep shows it.
|
||
|
||
## What is noise
|
||
|
||
On a laptop, expect 5-10% run-to-run variance from thermal state and other
|
||
processes alone. Treat anything under that as unmeasured. If a change is
|
||
genuinely small but real, prove it by increasing iterations rather than by
|
||
asserting it — and if that is too expensive, say the change was too small to
|
||
measure rather than reporting the point estimate as fact.
|
||
|
||
## Benchmarks are also regression tests
|
||
|
||
The value is in the baseline. Record it (`--save-baseline`), compare against it
|
||
(`--baseline`), and keep the numbers with the commit that produced them.
|
||
`benchmark_snapshots` exists for exactly this — a benchmark whose history is
|
||
lost measures nothing the next time.
|