diff --git a/docs/LESSONS.md b/docs/LESSONS.md index 4c625c7b..05e4e19f 100644 --- a/docs/LESSONS.md +++ b/docs/LESSONS.md @@ -2,30 +2,6 @@ Short review queue of reusable failure classes from merged workshop-rs work. Each entry names a prevention action and the evidence behind it. Promote an entry into a regression test, an owner contract, or an existing engineering standard, then remove it here. The procedure is the `wrightkit-retro` skill in `wrightkit/.agents`. -### Every declared catalog surface must parse back -- **Rule:** Spellings, aliases, and signatures declared in the catalog must all be exercised through the frontend, not sampled. When a declared surface does not parse, decide explicitly whether the declaration or the parser is wrong; do not leave the gap for consumers to find. -- **Why:** Consumers writing exactly what the catalog declares hit parse failures the suite never exercised: six colon-containing enum spellings rejected in bare position, index-action signatures omitting the surface player argument, and declared localized spellings failing to reparse in 11 of 15 locales. -- **Prevention home:** census/exhaustive catalog tests — the localization shard now enumerates every declared spelling ([#449](https://github.com/wrightkit/workshop-rs/pull/449)); signatures and colon-shape members need the same treatment. -- **Incidents:** [#443](https://github.com/wrightkit/workshop-rs/pull/443), [#445](https://github.com/wrightkit/workshop-rs/pull/445), [#449](https://github.com/wrightkit/workshop-rs/pull/449) — observed 2026-10-14 - -### Emitted text must reparse to the same program -- **Rule:** Round-trip coverage must feed emitter output back through the parser, including edge shapes — empty members, escapes, BOM, duplicate blocks, non-finite and out-of-range literals. Asserting the emitted string alone does not catch output the frontend reads differently. -- **Why:** Emit/parse drift shipped repeatedly: a trailing space on empty verbatim members, a bare `name:` member absorbing the next line (the existing round-trip test never re-parsed), and the audit fidelity batch — rule-name escapes, truncating declaration indexes, silently dropped duplicate `settings` blocks, opaque-string escapes, and a lexer that rejected BOM. -- **Prevention home:** existing policy — `wrightkit/.github` `docs/testing-policy.md` lists "parse -> emit -> parse preserving declared semantics"; repo roundtrip regression tests ([#437](https://github.com/wrightkit/workshop-rs/pull/437)). -- **Incidents:** [#398](https://github.com/wrightkit/workshop-rs/pull/398), [#404](https://github.com/wrightkit/workshop-rs/pull/404), [#433](https://github.com/wrightkit/workshop-rs/pull/433), [#436](https://github.com/wrightkit/workshop-rs/pull/436), [#437](https://github.com/wrightkit/workshop-rs/pull/437) — observed 2026-10-09, 2026-10-13 - -### Retained-source bookkeeping must audit only the attached file's spans -- **Rule:** When attaching source text to a registered file, prune only spans that belong to that file and fail to resolve in it. Provider maps legitimately record spans outside authored extents (macro expansion onto invocation sites); cover multi-file attach order in tests. -- **Why:** The first fix assumed every recorded provenance span resolves in the attached text; the follow-up found the audit ran against every file's spans, clearing earlier files' provenance when sources attach sequentially — multi-file provider programs lost all provenance. -- **Prevention home:** regression tests for multi-file attach ordering and non-resolving provider spans. -- **Incidents:** [#410](https://github.com/wrightkit/workshop-rs/pull/410), [#418](https://github.com/wrightkit/workshop-rs/pull/418) — observed 2026-10-09 - -### New syntax must fit the pinned MSRV, not just stable -- **Rule:** `rust-version = "1.85.0"` gates syntax, not only APIs — let-chains and newer constructs fail the 1.85 CI job even when stable accepts them. Check the MSRV toolchain (or enable `clippy::incompatible_msrv` against `rust-version`) before pushing. -- **Why:** Two PRs needed in-flight commits dropping let-chains after `Rust (1.85.0) / Quality` failed while `stable` passed — the same author-side mistake recurred within two days. -- **Prevention home:** lint/CI — enable `clippy::incompatible_msrv` so local `cargo clippy` catches post-MSRV syntax, or document the `+1.85.0` check in `AGENTS.md`. -- **Incidents:** [#433](https://github.com/wrightkit/workshop-rs/pull/433), [#449](https://github.com/wrightkit/workshop-rs/pull/449) — observed 2026-10-13, 2026-10-14 - ### A contract conflict is resolved by its owner, not by the PR that raises it - **Rule:** When a change conflicts with an architecture or public-contract rule, the deciding ADR must carry the owning role's approval before it is marked `Accepted`; the implementation PR cannot self-authorize the decision it needs. - **Why:** The residual-tolerant validation PR authored ADR-0018 as `Accepted` to resolve a contract conflict its own change created; the decision only landed after the owner reviewed and approved it, costing three review rounds.