docs: improvise-8shu closed, and its diagnosis was the wrong half
The ticket blamed an unbounded grammar walk. Instrumenting the halves separately: load_grammar() x1000 = 2.50s against a 15.7ms walk, so the walk was 0.6% of the cost and improv_file() was re-parsing improv.pest once per generated case. A OnceLock took improvise-io's lib from 1.42s to 0.21s with the strategy's input space unchanged; constraining it would have moved 15.7ms and shipped as the fix. Re-measured here rather than taking the hand-back's figure, which differed: 1445 tests over 34 binaries, longest single binary 0.6-1.0s across two runs on one machine. That spread, on one machine in one sitting, is its own argument for reading your own run. Also corrects the repo map's bug-fix workflow line, which said to document a bug in the test's doc comment — the two-line comment cap forbids exactly that and design-principles §7 says the opposite. A fact stated twice, changed by an edit rather than a grep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
@@ -349,10 +349,21 @@ worse than no claim at all. Three consequences:
|
||||
second place, which it need not be: on one machine `improvise-io` is 2.2×
|
||||
the runner-up and ~15× the median. It is the same binary on all three
|
||||
machines it has been measured on: `improvise-io`'s lib, which
|
||||
is proptest-dominated by the grammar-walking `.improv` generator and the
|
||||
round-trip properties over it. Constraining that generator's strategy is the
|
||||
open work (improvise-8shu); lowering the case count below 256 is not (see
|
||||
below).
|
||||
was proptest-dominated by the grammar-walking `.improv` generator and the
|
||||
round-trip properties over it.
|
||||
|
||||
**improvise-8shu is closed, and its diagnosis was wrong** — which is the more
|
||||
useful half. The ticket blamed an unbounded grammar walk and asked for a
|
||||
constrained strategy. Instrumenting the two halves separately: `load_grammar()`
|
||||
×1000 took **2.50s** against a **15.7ms** walk, so the walk was **0.6%** of the
|
||||
cost and `improv_file()` was re-parsing `improv.pest` once per generated case.
|
||||
Caching it in a `OnceLock` took `improvise-io`'s lib from **1.42s to 0.21s**
|
||||
with the strategy's input space *unchanged*; constraining it would have moved
|
||||
15.7ms and shipped as "the fix". Measured at `f152d10` on the orchestrator's
|
||||
machine: **1445 tests over 34 binaries**, longest single binary **0.6–1.0s
|
||||
across two runs** — the spread on one machine in one sitting being its own
|
||||
argument for reading your own run. `improvise-io` is no longer the outlier.
|
||||
Lowering the case count below 256 is still not on the table (see below).
|
||||
|
||||
### Demonstrate bugs before fixing them
|
||||
|
||||
|
||||
+11
-6
@@ -582,14 +582,19 @@ Full guidance lives in `context/design-principles.md` §7. Quick reminders:
|
||||
|
||||
- **Two numbers, not one** — the ambiguity between them is what let the old
|
||||
"suite runs in <2s" drift: **no single test binary over 2 seconds**, and the
|
||||
workspace wall clock is the sum. Measured at `22d43b2`: 1309 tests over 33
|
||||
binaries, ~2.2s wall, longest single binary 1.42s — so the workspace is already
|
||||
past 2s and the per-binary rule is the one that bites. Excludes compilation, and
|
||||
excludes `harness/` (Playwright, minutes, its own CI job). *Re-measure* rather
|
||||
than re-assert — `cargo test --workspace` prints a `finished in` per binary.
|
||||
workspace wall clock is the sum. Measured at `f152d10`: **1445 tests over 34
|
||||
binaries**, longest single binary **0.6–1.0s across two runs on one machine** —
|
||||
the spread being its own argument for reading your own run. Excludes compilation,
|
||||
and excludes `harness/` (Playwright, minutes, its own CI job). *Re-measure* rather
|
||||
than re-assert — `cargo test --workspace` prints a `finished in` per binary, and
|
||||
**the portable signal is the ratio, not the seconds**: one binary an order of
|
||||
magnitude above the run's *median*. `improvise-io`'s lib was that outlier at 1.42s
|
||||
on three machines and is not any more — improvise-8shu found the cost was a
|
||||
`pest_meta` re-parse per generated case, not the walk it was blamed on, and a
|
||||
`OnceLock` took it to 0.21s. The full argument is `context/design-principles.md` §7.
|
||||
- Commands: build a `CmdContext`, call `execute`, assert on returned effects. No terminal needed.
|
||||
- Property tests (`proptest`, default 256 cases) cover invariants: CellKey sort, axis consistency, save/load roundtrip, `parse(format(parse(generate())))` stability.
|
||||
- **Bug-fix workflow**: write a failing test *before* the fix (regression guard). Document the bug in the test's doc-comment (see `model/types.rs::formula_tests`).
|
||||
- **Bug-fix workflow**: write a failing test *before* the fix (regression guard). Record the bug in `context/test-evidence/`, **keyed by test name** — *not* in the test's doc comment, which the two-line comment cap forbids and `scripts/check-comment-runs.py` enforces. A heading may be module-qualified and must be when its leaf is not unique.
|
||||
- Coverage target ~80% line/branch on logic code; skip ratatui render paths.
|
||||
|
||||
---
|
||||
|
||||
Reference in New Issue
Block a user