diff --git a/.devflow/docs/design/v040-wave1/20260812_0046/pr5-deprecate-apply-fixes-plan.md b/.devflow/docs/design/v040-wave1/20260812_0046/pr5-deprecate-apply-fixes-plan.md new file mode 100644 index 0000000..3015ef7 --- /dev/null +++ b/.devflow/docs/design/v040-wave1/20260812_0046/pr5-deprecate-apply-fixes-plan.md @@ -0,0 +1,424 @@ +# chore(mds-core)!: deprecate `apply_fixes` in favor of `apply_fixes_incremental` [#209] + +Issues: #209 + +## Implementation Plan + +# PR5 — Deprecate `apply_fixes` (#209) — CHALLENGED AND AMENDED + +**Size:** XS as scoped, S/M if OD-209-A or OD-209-E resolve toward doing the real work now. +**Wave position:** independent. Touches `crates/mds-core/src/lint/fix.rs`, `crates/mds-core/tests/api_surface.rs`, `CHANGELOG.md`, `.devflow/features/mds-lint/KNOWLEDGE.md`. Only the `CHANGELOG.md` `[Unreleased]` block is plausibly shared with the other five PRs. + +> **STOP — do not implement until OD-209-A is answered.** The issue's stated blocker is falsified by the tree (see §0.1). Everything below assumes OD-209-A resolves to option C (deprecate). If it resolves to A, B, or D, §3-§5 change materially. + +--- + +## 0. State verified against 113f472 + +Every anchor in the original plan was re-checked against the working tree. **All of them are accurate** — including the ten test fn-decl/call-site pairs, which matched exactly. The original plan's correction of the issue's stale `fileRefs` (407-465 → 663-762) is also correct. + +| Claim | Verified | Evidence | +|---|---|---| +| `apply_fixes` defined at fix.rs:691 | YES | `pub fn apply_fixes(source: &str, plan: FixPlan, original: &LintResult, reverify: F) -> FixOutcome`; rustdoc 663-689; `#[must_use = "a dropped FixOutcome silently discards the fix result"]` at 690 | +| Zero production callers | YES | Only call sites are inside fix.rs's own `#[cfg(test)] mod tests` (mod starts 1007). mds-cli/src/lint.rs uses `apply_fixes_incremental` at 451 and 571. Zero references in mds-napi, mds-wasm, mds-python, packages/*, README.md, or any crate README | +| Public via `mds::fix` | YES | lint/mod.rs:31 `pub mod fix;`; lib.rs:63-67 `pub use lint::{fix, ...}` | +| Issue `fileRefs` 407-465 is stale | YES | That range is `plan_fixes_with_options` territory. Real anchor 663-762 | +| Workspace version is 0.3.0, not 0.4.0 | YES | Cargo.toml:6. `bump-version.mjs` rewrites manifests + package.json + CHANGELOG only, never a `.rs` file | +| MSRV supports `#[expect]` | YES | Cargo.toml:8 `rust-version = "1.88"`; ci.yml MSRV job pinned to `dtolnay/rust-toolchain@1.88`; `#[expect]` stable since 1.81 | +| No workspace/crate lint table | YES | No `[workspace.lints]`, no `[lints]` in mds-core, no clippy.toml, no crate-level `#![deny]`/`#![warn]` | +| KNOWLEDGE.md is git-tracked | YES | `git ls-files` returns it; `.gitignore:64-70` is the un-ignore block. **Checked from the repo root working tree, not a worktree (PF-016).** | +| No `AD-` convention exists yet | YES | `grep -rn 'AD-[0-9]' --include='*.rs' crates/` returns nothing | +| No `#[expect]` or `#[deprecated]` exists yet | YES | Both greps return nothing across `crates/`. This PR introduces both | +| CHANGELOG anchors | YES | `[Unreleased]` 8, Security 89, Added 427, Changed 627, Fixed 764 | +| api_surface.rs F-API-1 | YES | 1374-1418; `use mds::fix::{...}` inside the fn body at 1382; next test at 1420 | +| WASM guard | YES | ci.yml:87-118 loops over `pkg/mds_wasm_bg.wasm` and `pkg-web/mds_wasm_bg.wasm`, emits `::notice::`, fails at `raw > 850000` | + +### 0.1 NEW — the issue's premise does not hold + +`git show v0.3.0:crates/mds-core/src/lint/fix.rs` → **does not exist in v0.3.0**. The file was added by 5a227dc (mds lint #61, PR #171). `git tag --list 'v*'` → newest tag is `v0.3.0`. The v0.4.0 tag is not cut. + +**`mds::fix::apply_fixes` has never been published to crates.io.** The issue asserts it 'cannot be deleted without a semver-breaking change since it is a public mds-core export.' That is false at this commit: deletion or `pub(crate)` demotion costs nothing, because no downstream consumer exists or can exist. ADR-010 recorded the counter-argument verbatim for exactly this window: *'the pre-publish window is the last moment the break is free and every unmarked public type is a permanent semver trap.'* → **OD-209-A.** + +### 0.2 NEW — the v0.5.0 coverage cliff + +Six ID-tagged regression guards assert only through `apply_fixes` and have no `apply_fixes_incremental` counterpart: + +| fix.rs line | Test | Behavior it is the sole pin for | +|---|---|---| +| 1392 | `a4_partial_overlap_still_rejected_after_dedup` | A4: partial overlap survives dedup and still rejects | +| 1681 | `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` | A5 rejection-message contract | +| 1869 | `reverify_preexisting_untargeted_survives_and_fix_applies` | **AC-F-23**: pre-existing untargeted diagnostic must not refuse the fix | +| 1890 | `reverify_new_untargeted_diagnostic_is_rejected` | A genuinely new untargeted diagnostic IS a regression | +| 1957 | `tier_b_unused_function_standalone_apply_succeeds` | **I-13**: end-to-end Tier B with a real reverify closure | +| 2024 | `l_fix_rev1_output_delta_causes_rejection` | **L-FIX-REV1**: output delta must reject | + +The incremental suite is INC-1..INC-8 (2075, 2094, 2113, 2150, 2204, 2236, 2278, 2404), `pf005_unsorted_edits_rejected_in_incremental` (2336), `incremental_rejection_reason_escapes_embedded_error_display` (1767). It covers none of the six. + +Two consequences. First, deleting `apply_fixes` at v0.5.0 silently deletes the only coverage of five ADR-004 reverify-gate behaviors. Second — and this is true today, before any deprecation — those safety behaviors are pinned against a function the shipped CLI never calls. → **OD-209-E.** + +### 0.3 NEW — ADR mis-citation inside the block being edited + +fix.rs:585 reads ``# `_unchecked` suffix — ADR-001`` and 586-588 say the function 'bypasses the ADR-001 reverify gate (compile-equivalence check)'. Per the ledger, ADR-001 is the **mds fmt** gate; ADR-004 states that gate is 'inapplicable BY CONSTRUCTION' to lint --fix. Rewriting 589-591 while leaving 585-588 ships one paragraph citing two mutually exclusive ADRs. Same defect at KNOWLEDGE.md:180. → **OD-209-D.** + +--- + +## 1. Approach + +Six moves, strictly ordered: + +1. Attach `#[deprecated]` to `apply_fixes`; write the migration semantics into the rustdoc as an `AD-209-1` record. +2. Run the **positive control** (clippy must report exactly 10 deprecation errors) before touching a single suppression. +3. Add `#[expect(deprecated)]` per test function — never at module scope, never `#[allow]`. +4. Correct the misdirecting rustdoc at 238, 590, 599, **and the ADR-001 mis-citation at 585-588**. +5. CHANGELOG `### Deprecated`; KNOWLEDGE.md:462. +6. Add `F-API-3` to `tests/api_surface.rs`, then run the **reverse mutation control** (delete the attribute, confirm 11 `unfulfilled_lint_expectations`, restore). + +No new module, no wrapper, no forwarder. The body (695-762) is byte-identical. Codegen delta is exactly zero. + +--- + +## 2. Affected files and anchors + +### `crates/mds-core/src/lint/fix.rs` (2488 lines) + +| Anchor | Current | Change | +|---|---|---| +| 663-689 | rustdoc for `apply_fixes` | Prepend `# Deprecated (AD-209-1)` (see §3 D2). **No compiled doctest** — ```text or ```ignore fences only | +| 690 | `#[must_use = "..."]` | Unchanged; insert `#[deprecated(...)]` after it, directly above 691 | +| 691-762 | signature and body | **Untouched** | +| 236-240 | `FixPlan` docs | Rewrite 238 to name `apply_fixes_incremental` only. **Preserve 239-240 verbatim** (the `FixPlan::default()` / ADR-010 sentence) | +| 585-588 | ``# `_unchecked` suffix — ADR-001`` | **NEW** — correct to ADR-004 (§0.3) | +| 589-591 | 'must use [`apply_fixes`] instead' | Rewrite to `apply_fixes_incremental`. Load-bearing safety guidance | +| 598-600 | 'use [`apply_fixes`] which checks this' | Rewrite to `apply_fixes_incremental` | +| 620-626 | comment naming both functions | **Leave** — factually true of both | +| 800-802 | `Unlike [`apply_fixes`] which requires `F: FnOnce`` | **Leave** — this is the migration caveat readers need | +| 1006-1008 | `#[cfg(test)]` / `mod tests {` / `use super::*;` | **No module-level suppression.** Do not convert the glob to explicit imports (a glob of a deprecated item does not fire the lint; an explicit `use` would) | + +**Ten `#[expect(deprecated)]` insertion points** (fn-decl line → call line, all verified): + +| # | Function | fn | call | +|---|---|---|---| +| 1 | `a4_partial_overlap_still_rejected_after_dedup` | 1392 | 1448 | +| 2 | `l_fix_rev1_reverify_failure_rejects_fix` | 1654 | 1661 | +| 3 | `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` | 1681 | 1687 | +| 4 | `apply_fixes_rejection_reason_escapes_embedded_error_display` | 1739 | 1749 | +| 5 | `reverify_success_returns_fixed` | 1833 | 1850 | +| 6 | `reverify_preexisting_untargeted_survives_and_fix_applies` | 1869 | 1878 | +| 7 | `reverify_new_untargeted_diagnostic_is_rejected` | 1890 | 1898 | +| 8 | `tier_b_unused_function_standalone_apply_succeeds` | 1957 | 1995 | +| 9 | `l_fix_rev1_output_delta_causes_rejection` | 2024 | 2050 | +| 10 | `pf005_unsorted_edits_rejected_in_apply_fixes` | 2371 | 2392 | + +All ten bind the result (`let outcome = apply_fixes(...)`), so `unused_must_use` will not fire and the 'exactly 10' control is not polluted. + +### `crates/mds-core/tests/api_surface.rs` +Insert `F-API-3` after 1418, before the `STRING_SOURCE_MAP_LABEL` test at 1420. Mirror `fix_api_incremental_exists` (1381-1418): `use mds::fix::{...}` **inside** the fn body so the `#[expect(deprecated)]` covers both import and call; construct via `LintResult::new` / `plan_fixes` / `ByteEdit::deletion`, never struct literals (applies ADR-010). + +### `CHANGELOG.md` +Insert `### Deprecated` immediately before the existing `### Fixed` at 764, matching the file's own Added(427) → Changed(627) → Fixed(764) run. **Do not justify this by Keep a Changelog ordering** — `[Unreleased]` already places `### Security` at 89, ahead of Added, so the file does not follow it. + +### `.devflow/features/mds-lint/KNOWLEDGE.md` (tracked) +- **462** — the only line that changes: drop `apply_fixes()` from the 'MUST use' anti-pattern bullet. +- **Do not touch:** line 4 (frontmatter keyword blob), 180 (unless OD-209-D says so), 188 (factual FixOutcome statement), 543 (key-files list), 571 (ADR-004 linkage), or `.devflow/features/index.md:4`. None name `apply_fixes` in a way that misdirects. + +--- + +## 3. Design decisions + +**D1 — `since` handling is now an open question (OD-209-B), not settled.** Hardcoding `"0.4.0"` compiles clean today (rustc's `deprecated_semver` only checks parseability for third-party crates) but leaves a drift risk that `bump-version.mjs` provably cannot fix. Omitting `since` eliminates the risk at zero cost. Do not implement until answered. + +**D2 — The `note` carries the blocking caveat; the rustdoc carries the full record. `note` MUST be ≤ 160 characters.** rustc renders `note` verbatim, inline, in every downstream warning; a 230-character note wraps badly in terminals and CI logs. Three verified migration deltas belong in the `AD-209-1` rustdoc section: +1. **Closure bound:** `apply_fixes` takes `F: FnOnce` (693); `apply_fixes_incremental` requires `F: Fn` (811). A move-once closure cannot migrate mechanically. Already documented at 800-802. +2. **New reachable outcome:** `PartiallyFixed` (docs 794-799, construction 966+) is returned only by the incremental path. `FixOutcome` is `#[non_exhaustive]` (265, applies ADR-010), so external matches already carry a wildcard — and a wildcard that swallows `PartiallyFixed` drops partial results on the floor. +3. **Cost:** 1 reverify call vs. up to N+1, capped by `FALLBACK_MAX_EDITS = 50` (774). + +Shape (note trimmed, full record in the rustdoc above it): + +```rust +#[must_use = "a dropped FixOutcome silently discards the fix result"] +#[deprecated( + note = "use `apply_fixes_incremental`; not a drop-in swap, the reverify closure must be `Fn`, not `FnOnce`. See the item docs." +)] +pub fn apply_fixes(...) +``` + +**No placeholder token may reach a commit.** OD-209-A/B/C must resolve first. + +**D3 — `#[expect(deprecated)]` per test fn, never `#[allow]`, never at module scope.** `#[expect]` suppresses the diagnostic AND fires `unfulfilled_lint_expectations` if the diagnostic stops being produced, so the suppression doubles as a live assertion. **Amendment:** state plainly that this assertion has exactly ONE enforcement point — `cargo clippy --workspace --all-targets -- -D warnings` (ci.yml:38). `cargo test --workspace`, `cargo nextest run`, and the MSRV job (`cargo check` with no `--all-targets`, so tests are never compiled) all let it pass as a mere warning. Module scope is wrong regardless: a `#![allow]` at 1007 would blanket ~1480 lines and mask future accidental use of any other deprecated item. + +**D4 — Do NOT also deprecate `apply_plan_unchecked`.** Verified: its only reference outside fix.rs is a *comment* at mds-cli/src/lint.rs:845; its three live callers are fix.rs:720, 846, 895. Deprecating it would force suppressions onto three production call sites inside `apply_fixes_incremental` itself, violating #209's own AC. It is a live internal primitive with a deliberately scary name and an unconditional PF-005 sortedness assert (627-630). Leave it public and undeprecated; only correct its rustdoc (585-600, per §0.3). + +**D5 — AD-series traceability.** `AD-209-1` on `apply_fixes` (why deprecated, `applies ADR-004`, the three deltas, why the body was not deleted **given fix.rs is absent at tag v0.3.0**), `AD-209-2` on F-API-3 (why `#[expect]` over `trybuild`). No leading `#` (avoids PF-010). Whether to establish this repo-wide convention here is OD-209-F. + +**D6 — ADR-004 linkage.** `apply_fixes` implements the ADR-004 gate as an all-or-nothing batch verify; `apply_fixes_incremental` implements the same safety contract with a batch attempt plus a bounded per-edit fallback, salvaging the safe subset rather than refusing wholesale. That is *why* the deprecation is correct rather than arbitrary, and it belongs in code. + +**D7 — No new abstraction.** A `pub(crate) fn apply_fixes_impl` with a deprecated forwarder is rejected: the tests would stop exercising the deprecated public path, which is the only path an external user can reach. + +**D8 (NEW) — No compiled doctest, and this PR must ADD no new `allow(deprecated)` under `crates/*/src/`.** `cargo clippy --all-targets` does not compile doctests, so a doctest calling `apply_fixes` would emit a permanently ungated warning in every downstream `cargo test`. And because the AC-209-04 audit is a lexical grep over `*.rs`, an `#[allow(deprecated)]` written inside a doc-comment code fence in `src/` would trip it. Migration examples use ```text or ```ignore. **This is an "add none" rule, not an absolute-absence rule (avoids PF-015):** `crates/mds-core/src/lint/config.rs` lines 287 and 289 already carry `/// #[allow(deprecated)]` inside the compiled doctest for the earlier `LintConfig::from_rules` deprecation, where they are load-bearing — removing them would make that doctest emit a deprecation warning. They are pre-existing and whitelisted by AC-209-04 group (c). Do not delete them in service of this design decision. + +--- + +## 4. Implementation sequence + +0. **Resolve OD-209-A.** If it lands on A, B, or D, discard §3 D1-D3 and re-plan; this sequence assumes C. +1. **Resolve OD-209-B and OD-209-C** so no placeholder is ever committed. +2. **Attach `#[deprecated]` + write the `AD-209-1` rustdoc.** Run `cargo clippy -p mds-core --all-targets -- -D warnings` and capture the failure list. **Positive control (applies ADR-009, avoids PF-013): expect exactly 10 errors at 1448, 1661, 1687, 1749, 1850, 1878, 1898, 1995, 2050, 2392.** Fewer than 10 is a failure signal, not success — stop and diagnose. +3. **Add `#[expect(deprecated)]` to each of the ten fn-decl lines**, each with a one-line rationale comment. Re-run → clean. +4. **Correct rustdoc at 585-588, 589-591, 598-600, and 238** (preserving 239-240 verbatim). Leave 620-626 and 800-802. +5. **CHANGELOG `### Deprecated`** after `### Added` and before the first `### **BREAKING**` section (deliberately positioned in the visible upper portion of `[Unreleased]`; see AC-209-13). Public-facing copy: no em/en dashes, no placeholders. +6. **`tests/api_surface.rs` F-API-3** after 1418, with the `AD-209-2` docstring. +7. **Reverse mutation control (NEW, mandatory):** delete only the `#[deprecated(...)]` attribute, run `cargo clippy --workspace --all-targets -- -D warnings`, then also run `cargo clippy -p mds-core --test api_surface -- -D warnings`; the UNION of the two runs must be **exactly 11** `unfulfilled_lint_expectations` — 10 in fix.rs and 1 in api_surface.rs (applies ADR-009) — then restore and confirm both runs clean. How the 11 split across the two commands is build-scheduling dependent, not a property of the code: removing the attribute leaves the lib rlib compiling clean, so the lib-test and `api_surface` targets become ready simultaneously. In the run verified for this PR the workspace command alone reported all 11; a serialized scheduler may instead abort after the lib-test target, in which case the targeted command supplies the 11th. Assert the union, never a per-command count. Without this, AC-209-05 is an absence-only claim. +8. **`.devflow/features/mds-lint/KNOWLEDGE.md:462`**, confirmed tracked from the **repo root working tree** (avoids PF-016). +9. **OD-209-E work**, if it resolved to 'migrate now'. +10. **Full gate (§6).** + +--- + +## 5. Risks + +| ID | Risk | Likelihood | Mitigation | +|---|---|---|---| +| R1 | `-D warnings` breaks on 10 in-crate uses the moment the attribute lands | Certain | By design; steps 2-3. Sites pre-enumerated with exact lines | +| R2 | `since = "0.4.0"` drifts; `bump-version.mjs` provably cannot fix it | Low but silent | Escalated to **OD-209-B**. 'Omit `since`' eliminates it; a runbook grep only defers it to a human | +| R3 | A future PR deletes an `apply_fixes` call but leaves the `#[expect]` → build error | Low | Intended feedback loop. Note it in the PR body so a later reviewer is not confused | +| R4 | CHANGELOG `[Unreleased]` conflicts with the other five wave PRs | Medium | PR5 is the only wave PR creating `### Deprecated`, and it inserts at a section boundary. Land early in the squash order | +| R5 | WASM size guard (820,305 / 850,000; 3.5% headroom) | Effectively zero — **but verify, do not assume** | Attributes and doc comments emit no codegen; the body is byte-identical. **Assert delta == 0 bytes on BOTH `pkg/mds_wasm_bg.wasm` and `pkg-web/mds_wasm_bg.wasm` against the wave base, not merely '≤ 850,000'** (ci.yml:87-118 checks both). A budget-only pass would absorb an unrelated regression | +| R6 | Someone converts `use super::*;` (1008) to explicit imports later; an explicit `use` fires the lint where a glob does not | Low | Called out in §2. Documented, not defended against | +| R7 | Deprecating the function that `apply_plan_unchecked`'s safety doc names as the required alternative | Certain if step 4 is skipped | Step 4 is not optional. Now also covers the ADR-001 mis-citation at 585-588 | +| **R8 (NEW)** | A rewritten intra-doc link silently degrades to literal text — **CI has no `cargo doc` step and no `RUSTDOCFLAGS`** | Medium | `RUSTDOCFLAGS="-D warnings" cargo doc -p mds-core --no-deps` is a blocking AC (AC-209-11), plus visual confirmation of three resolved anchors. Whether to add the CI job is **OD-209-G** | +| **R9 (NEW)** | A doctest in the new section emits a permanently ungated deprecation warning downstream (`clippy --all-targets` does not compile doctests; CI's `cargo test --workspace` runs them but does not fail on warnings) | Medium | D8: no compiled doctest; `cargo test --doc -p mds-core` must log zero `deprecated` warnings | +| **R10 (NEW)** | v0.5.0 deletion silently drops the only coverage of AC-F-23, I-13, L-FIX-REV1, A5, and A4-after-dedup | **High if unaddressed** | AC-209-15 forces either migration now or verbatim enumeration in the removal tracker. **OD-209-E** | + +--- + +## 6. Verification + +```bash +# Rust — nextest SKIPS doctests, so the --doc run is mandatory +cargo nextest run --workspace && cargo test --doc +cargo fmt --all --check +cargo clippy --workspace --all-targets -- -D warnings # HARD STOP on any warning +RUSTDOCFLAGS="-D warnings" cargo doc -p mds-core --no-deps # NEW — CI has no rustdoc gate + +# JS surfaces +npm ci && npm run build -w @mdscript/mds-wasm && npm run build --workspaces --if-present +npm test --workspaces --if-present +node scripts/verify-versions.mjs +``` + +Use the repo-local `.cargo/config.toml` workaround (`rustc-wrapper=""`, `jobs=2`) for local Rust runs; plain `cargo test --workspace` stalls ~20 min on fresh binaries. **Never commit that file.** `mds lint` exiting 2 on `examples/` is by design. + +PR-specific checks: + +1. **Forward positive control** — before any suppression, exactly 10 deprecation errors at the ten enumerated lines (§4 step 2). +2. **Reverse mutation control** — attribute deleted → exactly 11 `unfulfilled_lint_expectations`; restored → clean (§4 step 7). +3. **Suppression audit with a planted positive (shape-tolerant grep required)** — the audit grep MUST be shape-tolerant because rustfmt wraps every `#[expect(deprecated, reason = ...)]` across multiple lines; a single-line `expect(deprecated)` pattern is blind to all ten suppressions in the tree (applies ADR-009, avoids PF-013). Command: `grep -rn -A2 -e '#\[expect(' -e '#\[allow(' crates/ --include='*.rs' | grep deprecated`. Positive control: temporarily insert the multi-line attribute block — `#[expect(` on one line, ` deprecated,` on the next — in `crates/mds-core/src/lexer.rs` (e.g., inside `#[cfg(never)] fn _audit() { todo!() }`), confirm the grep reports a `deprecated,` context line in lexer.rs, remove the block, re-run. The planted form MUST be the multi-line attribute shape, NOT `// allow(deprecated)`; a comment only proves the old single-line pattern works. Clean run must hit only (a) fix.rs below line 1006, (b) `tests/api_surface.rs`, and (c) config.rs:287,289 (doc-comment prose). Read every hit individually; `-A2` produces multiple output lines per suppression, so a bare count is not evidence. +4. **Stale-guidance sweep** — `grep -n 'apply_fixes\b' crates/mds-core/src/lint/fix.rs`; every surviving hit must be the deprecated item's own docs, a factual comparison (624, 801), or a test identifier. Zero sentences may *instruct* a caller to use it. +5. **ADR sweep** — `grep -n 'ADR-001' crates/mds-core/src/lint/fix.rs` returns nothing in any lint-fix reverify-gate paragraph. +6. **Rustdoc** — zero broken intra-doc links; three resolved `apply_fixes_incremental` anchors on the FixPlan / apply_plan_unchecked / apply_fixes pages. +7. **Doctest hygiene** — `cargo test --doc -p mds-core` logs zero `deprecated` warnings. +8. **WASM byte identity** — build both artifacts at base and at head with the identical toolchain (Binaryen v129+ locally); raw sizes must be equal and ≤ 850,000. Compare the two `::notice::WASM ...` lines across CI runs as a cross-check. +9. **Diff shape** — `git diff -U0` on fix.rs shows only attribute, `///`, and `//` lines. +10. **Python surface** — no `pytest` run strictly required (zero `apply_fixes` references in `crates/mds-python`), but AC-209-17 asks for it as the cheap proof that nothing leaked. + +--- + +## 7. Acceptance criteria + +See the `acceptanceCriteria` array: AC-209-01 through AC-209-17. They cover API contract (01, 02, 17), functionality and lint-gate behavior (03, 04, 05, 06, 09), documentation correctness (07, 08, 11, 12, 14), release artifacts (13, 16), coverage protection (15), and performance (10 — explicit zero-byte codegen-delta threshold plus an explicit 'no runtime performance requirement'). Six are stated negatively (03, 04, 08, 09, 12, 17). + +--- + +## 8. Plan self-review — what the original plan got right, and what it missed + +**Got right (all re-verified):** every line anchor including all ten fn/call pairs; the stale-`fileRefs` correction; the zero-callers claim across all three binding crates and packages/*; that `since` is informational for third-party crates; that in-crate uses do warn; that a glob import does not fire the lint; that MSRV clears `#[expect]`; that D4's rejection of deprecating `apply_plan_unchecked` is technically correct (mds-cli:845 is a comment, not a call); PF-010-clean local IDs; the PF-016 caution on KNOWLEDGE.md. + +**Missed:** +1. The issue's premise is falsified — `fix.rs` is absent at tag v0.3.0, so `apply_fixes` was never published and deletion is free right now. **OD-209-A.** +2. Six ADR-004 regression guards exist only on the deprecated path; the incremental suite covers none of them. **OD-209-E.** +3. fix.rs:585-588 attributes the lint --fix reverify gate to ADR-001, contradicting ADR-004 and the plan's own D6, inside the block the plan edits. **OD-209-D.** +4. CI has no `cargo doc` step, so the three intra-doc-link rewrites land unguarded. **OD-209-G.** +5. Doctests escape `-D warnings`, and a doc-fence `#[allow(deprecated)]` would trip the plan's own AC-209-2 grep. **D8.** +6. The positive control only proved the forward direction; the removal direction that AC-209-6 actually claims was never tested (PF-013 shape). **§4 step 7.** +7. The `#[expect]`-as-assertion mechanism has exactly one enforcement point, unstated. +8. `since` had a zero-cost alternative (omit it) that was never weighed. **OD-209-B.** +9. The CHANGELOG placement was justified by a Keep a Changelog rule the file itself violates. +10. The `note` draft was ~230 chars and contained a live `` placeholder. +11. R5 downgraded the WASM re-measure to 'confirmation only' when the wave rule requires a re-measure, and 'under budget' is the wrong assertion for a provably codegen-neutral change. +12. Three first-of-kind repo conventions (`#[deprecated]`, `#[expect]`, `AD-`) ride an XS PR without being flagged as a governance choice. **OD-209-F.** + + +## Improvements and Gaps Identified + +- VERIFICATION RESULT — every line anchor in the plan checked out against 113f472. Confirmed accurate: fix.rs:690 `#[must_use]`, 691 `pub fn apply_fixes`, rustdoc 663-689, FixPlan doc 238, apply_plan_unchecked doc 590 and 599, comment 624, comparison 801, FALLBACK_MAX_EDITS 774, FixOutcome `#[non_exhaustive]` 265, `#[cfg(test)]` 1006 / `mod tests` 1007 / `use super::*;` 1008, lib.rs:63-67 `pub use lint::{fix, ...}`, lint/mod.rs:31 `pub mod fix;`, api_surface.rs F-API-1 1374-1418, CHANGELOG `[Unreleased]` 8 / Security 89 / Added 427 / Changed 627 / Fixed 764, KNOWLEDGE.md 188 / 462 / 543 / 571. ALL TEN test fn-decl→call pairs verified exactly as tabled (1392→1448, 1654→1661, 1681→1687, 1739→1749, 1833→1850, 1869→1878, 1890→1898, 1957→1995, 2024→2050, 2371→2392). The plan's rejection of the issue's stale `fileRefs` (407-465) is correct. Zero unconfirmable claims. This is a well-verified plan; the gaps below are things it did not look for, not things it got wrong. +- BLOCKER-CLASS GAP 1 — the issue's core premise is falsified by the tree, and the plan accepted it without re-checking. `git show v0.3.0:crates/mds-core/src/lint/fix.rs` returns 'does not exist in v0.3.0'; the file was ADDED by 5a227dc (feat: mds lint #61 / PR #171) and `git tag --list 'v*'` shows the newest tag is v0.3.0. `mds::fix::apply_fixes` HAS NEVER BEEN PUBLISHED to crates.io, and per RELEASE CONTEXT the v0.4.0 tag is NOT cut. The issue asserts it 'cannot be deleted without a semver-breaking change since it is a public mds-core export' — that is false at this commit. Deleting it, or demoting it to `pub(crate)`, is FREE right now: zero downstream consumers exist and none can. Deprecating instead ships a brand-new public function that is born deprecated, plus 11 lint suppressions, plus a permanent v0.5.0 removal chore, plus a tracker issue, to preserve compatibility with nobody. ADR-010's own recorded rationale points the other way verbatim: 'the pre-publish window is the last moment the break is free and every unmarked public type is a permanent semver trap.' This is OD-209-A and it must be settled before a line is written. +- BLOCKER-CLASS GAP 2 — v0.5.0 test-coverage cliff, entirely unnoticed by the plan. Six ID-tagged regression guards live ONLY on the `apply_fixes` path and have no `apply_fixes_incremental` counterpart: (a) fix.rs:1869 `reverify_preexisting_untargeted_survives_and_fix_applies` — the AC-F-23 guard; (b) 1890 `reverify_new_untargeted_diagnostic_is_rejected`; (c) 1957 `tier_b_unused_function_standalone_apply_succeeds` — the I-13 end-to-end Tier B guard, whose own docstring says it 'closes the coverage gap identified in I-13'; (d) 2024 `l_fix_rev1_output_delta_causes_rejection` — L-FIX-REV1; (e) 1681 `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` — the A5 message contract; (f) 1392 `a4_partial_overlap_still_rejected_after_dedup` — A4 overlap-after-dedup. The incremental suite is INC-1..INC-8 (2075, 2094, 2113, 2150, 2204, 2236, 2278, 2404), `pf005_unsorted_edits_rejected_in_incremental` (2336) and `incremental_rejection_reason_escapes_embedded_error_display` (1767) — it covers NONE of (a)-(f). Two consequences: deleting `apply_fixes` at v0.5.0 silently deletes the only coverage of five ADR-004 reverify-gate behaviors; and, worse, TODAY those safety-critical behaviors are pinned against a function the production CLI no longer calls (mds-cli/src/lint.rs:451 and :571 both use `apply_fixes_incremental`). The deprecation makes the future deletion look free precisely because nobody has counted what it takes with it. +- GAP 3 — an ADR mis-citation sits inside the exact rustdoc block the plan edits. fix.rs:585 reads '# `_unchecked` suffix — ADR-001' and 586-588 read 'This function bypasses the ADR-001 reverify gate (compile-equivalence check)'. Per the ledger, ADR-001 is the *mds fmt* compile-equivalence gate, and ADR-004 states that gate 'is inapplicable BY CONSTRUCTION' to lint --fix. The plan rewrites 589-591 and asserts `applies ADR-004` in D6 — landing that without touching 585-588 ships one paragraph citing two mutually exclusive ADRs. Same defect at KNOWLEDGE.md:180 ('char-boundary guard (fail-closed, ADR-001)'). Correcting 585-588 is adjacent-breakage-in-the-same-block, not scope creep; KNOWLEDGE.md:180 is a scope call (OD-209-D). +- GAP 4 — no CI rustdoc gate exists, so the plan's three intra-doc-link rewrites are unguarded. `.github/workflows/ci.yml` has no `cargo doc` step and no `RUSTDOCFLAGS` anywhere; the rust job (lines 22-41) is exactly fmt + clippy + `cargo test --workspace`. A typo'd `[`apply_fixes_incremental`]` renders as literal text and no gate notices. The plan lists `cargo doc -p mds-core --no-deps` as a local check only — promote it to a blocking AC run with `RUSTDOCFLAGS="-D warnings"`, and decide whether to add the CI step (OD-209-G). +- GAP 5 — doctests are the one hole in the `-D warnings` gate, and the plan's own D2 walks into it. `cargo clippy --all-targets` does NOT compile doctests; CI's `cargo test --workspace` DOES run them but does not fail on warnings. So a migration example added to the new `# Deprecated (AD-209-1)` section that calls `apply_fixes` would emit an ungated `deprecated` warning in every downstream `cargo test` forever. And because AC-209-2's audit is a lexical grep over `crates/**/*.rs`, writing `#[allow(deprecated)]` inside a doc-comment code fence in `src/` would ALSO trip that grep. The plan never notices this interaction between D2 (write the migration guidance) and its own AC-209-2. Rule to state explicitly: the `# Deprecated` section may contain a ```text or ```ignore block only; no compiled doctest, and the literal string `allow(deprecated)` must not appear anywhere under `crates/*/src/`. +- GAP 6 — the plan's positive control only proves one direction. Step 2 proves `#[deprecated]` FIRES (expect exactly 10 clippy errors). It never proves the claim AC-209-6 actually makes — that removing the attribute BREAKS the build. Without a reversible mutation run, AC-209-6 is an absence-only assertion, which is the PF-013 shape the ledger already flags. Required second control: with all suppressions in place, temporarily delete the `#[deprecated(...)]` attribute and confirm `cargo clippy --workspace --all-targets -- -D warnings` fails with exactly 11 `unfulfilled_lint_expectations` diagnostics (10 in fix.rs + 1 in api_surface.rs), then restore. Verified as sound: all 10 call sites bind the result (`let outcome = apply_fixes(...)`), so `must_use` never fires and the count is not polluted. +- GAP 7 — the `#[expect]`-as-assertion mechanism has exactly one enforcement point and the plan does not say so. `unfulfilled_lint_expectations` is warn-by-default; only `cargo clippy --workspace --all-targets -- -D warnings` (ci.yml:38) promotes it to an error. `cargo test --workspace`, `cargo nextest run`, and the MSRV job (`cargo check -p mds-core -p mds-cli -p mds-python` — no `--all-targets`, so tests are never compiled) all let it pass as a warning. The 'build breaks loudly' claim in D3 is true of exactly one command. State it, so nobody later assumes a green `cargo build` means the attribute survived. +- GAP 8 — R2 (`since` drift) has a zero-cost fix the plan never considers: omit `since` entirely. `#[deprecated(note = "...")]` is legal with no `since` field. Verified `scripts/bump-version.mjs` rewrites only Cargo.toml `[workspace.package] version`, the four crate manifests, eight package.json files, and the CHANGELOG heading — never a `.rs` file — so the plan's own risk analysis is correct, but its mitigation (a manual grep line in RELEASING.md) is the weakest of three available options. Ranked: omit `since` (risk eliminated) > extend bump-version.mjs to rewrite `since = "..."` (risk automated away) > hardcode `0.4.0` + runbook grep (risk survives as a human step). See OD-209-B. +- GAP 9 — the CHANGELOG placement rationale cites a rule the file violates. The plan justifies inserting at line 764 with 'Keep a Changelog orders Added → Changed → Deprecated → Removed → Fixed → Security', but `[Unreleased]` in this repo puts `### Security` at line 89, ahead of `### Added` at 427. The file does not follow KaC ordering. The insertion point is still right; justify it as 'immediately before the existing `### Fixed` at 764, matching the file's own Added(427) → Changed(627) → Fixed(764) run', and drop the KaC appeal. +- GAP 10 — KNOWLEDGE.md collateral. `git ls-files` confirms `.devflow/features/mds-lint/KNOWLEDGE.md` is tracked, and `.gitignore:64-70` is the un-ignore block exactly as claimed (PF-016 check satisfied from the repo root, not a worktree). Beyond line 462, the plan should state what NOT to touch: KNOWLEDGE.md:4 (frontmatter `description:` keyword blob) and `.devflow/features/index.md:4` both list `apply_fixes_incremental` and do NOT list `apply_fixes` — correct as-is, leave them. Line 188 is a factual FixOutcome statement — leave. Line 543 (key-files list) and 571 (ADR-004 linkage) name only `apply_fixes_incremental` — leave. Only 462 changes. Saying this explicitly prevents a Coder from 'helpfully' adding the deprecated name to the keyword index. +- GAP 11 — the WASM re-measure must assert byte identity, not budget compliance. ci.yml:87-118 loops over BOTH `crates/mds-wasm/pkg/mds_wasm_bg.wasm` and `crates/mds-wasm/pkg-web/mds_wasm_bg.wasm`, emits `::notice::WASM