diff --git a/docs/method/backlog/bad-code/PLATFORM_wesley-gen-test-crate-builder-duplication.md b/docs/method/backlog/bad-code/PLATFORM_wesley-gen-test-crate-builder-duplication.md new file mode 100644 index 00000000..ce631ab7 --- /dev/null +++ b/docs/method/backlog/bad-code/PLATFORM_wesley-gen-test-crate-builder-duplication.md @@ -0,0 +1,73 @@ + + + +# `wesley-gen` test consumer-crate builders are ~40-line copy-paste + +Legend: `PLATFORM` + +## The smell + +`crates/echo-wesley-gen/tests/generation.rs` has four consumer-crate +builder functions: + +- `write_basic_generated_crate` +- `write_consumer_smoke_crate` +- `write_contract_host_smoke_crate` +- `write_optic_binding_smoke_crate` + +Each one writes a `Cargo.toml` + `src/lib.rs` + `src/generated.rs` and +uses essentially the same boilerplate: + +- the same `[workspace]` isolation block +- the same `path = "{}"` dep declaration pattern for + `echo-wasm-abi` and `echo-registry-api` +- the same `default-features = false, features = ["alloc"]` switching + on a `no_std` flag +- the same `serde` dep with no_std/std variants +- the same `#![no_std] / extern crate alloc;` vs `mod generated;` + lib.rs branching + +All four were also keyed off `std::process::id()` for the crate dir +until the `chore/test-loop-speedup` PR landed; the PID segment is now +inert (the shared `CARGO_TARGET_DIR` made it moot) but still litters +the disk during a test run. + +## Why this matters + +- A new generated-output test path means a fifth copy of the same + boilerplate. Easy to drift one config knob (a feature flag, a + serde version, a path) silently. +- The PID-segment leftovers create + remove directories every test + for no benefit now that the target dir is shared elsewhere. +- The PR #383 speedup work touched all four sites identically when + adding `.env("CARGO_TARGET_DIR", ...)` — every future change to + the harness will pay the same cost. + +## Fix shape + +Extract a single `ConsumerCrateBuilder` (or a free function with an +options struct) that owns: + +- the `[workspace]` boilerplate +- the dep declarations (no_std vs std switching) +- the lib.rs shim selection +- the crate-dir layout (dropping the PID segment changes path + semantics and is safe only if deterministic unique labels or + explicit tempdir guarantees preserve parallel-test isolation) + +Each call site then declares only what's unique to it: the label, +the no_std flag, the generated source, and any extra deps. + +This is a structural cleanup whose path-layout change is +behavior-preserving only when uniqueness and isolation invariants +are retained. Dropping the PID segment can otherwise change +collision behavior under parallel runs or label reuse. After the +extraction, adding a fifth generated-output test path should be a +~5-line call, not a ~40-line copy of the previous one. + +## Out of scope here + +- Sharing `CARGO_TARGET_DIR` (already landed in PR #383). +- The `[workspace]` isolation itself, which is required because the + generated crates must not pull in the parent workspace's + `Cargo.lock`. Keep that constraint. diff --git a/docs/method/backlog/cool-ideas/PLATFORM_prettier-before-cargo-hook-ordering.md b/docs/method/backlog/cool-ideas/PLATFORM_prettier-before-cargo-hook-ordering.md new file mode 100644 index 00000000..f380607b --- /dev/null +++ b/docs/method/backlog/cool-ideas/PLATFORM_prettier-before-cargo-hook-ordering.md @@ -0,0 +1,75 @@ + + + +# Pre-commit hook should run prettier before cargo verify + +Legend: `PLATFORM` + +## What hurts + +The PR #383 auto-stage fix already eliminated the most painful case +(prettier rewrites → abort → manual restage → cargo verify again from +scratch). But the underlying ordering is still backwards. + +Current hook order: + +1. cargo fmt check (auto-stage if rewrites) +2. toolchain pin check +3. PRNG / task list / lockfile guards +4. **`scripts/verify-local.sh pre-commit`** ← can take minutes cold +5. SPDX guard +6. **markdown prettier + lint** ← cheap, runs LAST + +The asymmetry: the expensive cargo step runs first, and the cheap +prettier-and-markdownlint step runs last. If a commit only touches +markdown but a previous touch left the cargo cache cold, you pay the +full cargo verify cost for nothing. If your commit touches both Rust +and markdown and prettier rewrites the markdown, the auto-stage path +now means you only pay cargo once — but that one pay is still cold +because cargo ran before prettier had a chance to decide whether the +hook can short-circuit. + +## The flip + +Run the cheap checks first, expensive last: + +1. toolchain / PRNG / lockfile guards (already cheap) +2. **prettier auto-format + markdownlint** ← move up +3. cargo fmt auto-stage +4. `scripts/verify-local.sh pre-commit` +5. SPDX guard + +This costs nothing when cargo would have run anyway (the user is +committing Rust changes and the cache happens to be cold). It saves +the entire cargo cost when: + +- the user is committing only markdown changes +- the user is committing markdown + Rust changes and prettier is + going to fail in a way the user wants to see before they wait for + cargo + +It also makes the hook's failure modes more user-friendly: prettier +errors arrive in seconds instead of after a multi-minute cargo build. + +## Risk + +- The current order may be intentional if cargo fmt sometimes + rewrites markdown-adjacent files (e.g. `include_str!` doctests). + Worth a quick audit of which files cargo fmt touches before + flipping. +- If prettier auto-stages markdown that's then expected by a + downstream cargo check (unlikely, but possible if a markdown file + is included via `include_str!`), the flip needs care. + +## Expected delta + +For markdown-only commits on a cold cargo cache: minutes → seconds. +For mixed Rust + markdown commits where prettier rewrites: same wall +time as today, but earlier visibility of formatting errors. For +Rust-only commits: no change. + +## Related + +- `docs/method/backlog/cool-ideas/PLATFORM_wesley-gen-test-loop-speedup.md` + (the original speedup backlog, win #5 — this is the spiritual + successor now that win #5 partially landed in PR #383).