From 769fbea5b6e21ccdba62445c517a8426d214ffa5 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 23:17:52 +0300 Subject: [PATCH 01/28] chore(mds-core): deprecate apply_fixes in favor of apply_fixes_incremental (#209) Marks mds::fix::apply_fixes as #[deprecated] in favor of apply_fixes_incremental, which provides the same ADR-004 three-tier reverify-gate safety contract with a batch-first attempt plus a bounded per-edit fallback. ## What changed - `apply_fixes` gains `#[deprecated(note = "...")]` with a 111-char note naming the replacement and the Fn-vs-FnOnce closure-bound breaking change. - Ten existing tests that exercise the deprecated path each gain `#[expect(deprecated, reason = "AD-209-1: ...")]` to keep the build clean while preserving coverage of six ADR-004 reverify-gate behaviors that have no incremental-path equivalents (AC-F-23, I-13, L-FIX-REV1, A4, A5). - New test F-API-3 in api_surface.rs (AD-209-2) pins that apply_fixes remains callable from an external crate and returns NothingToFix for an empty plan. - apply_plan_unchecked rustdoc corrected: ADR-001 -> ADR-004 in the _unchecked suffix heading and reverify-gate description; apply_fixes -> apply_fixes_incremental in the "must use" guidance. - FixPlan rustdoc updated to reference apply_fixes_incremental only. - CHANGELOG ### Deprecated section added immediately before ### Fixed. - KNOWLEDGE.md anti-pattern bullet updated to reference apply_fixes_incremental only. ## Verification Forward control (AC-209-06): exactly 10 deprecated errors before suppressions. Reverse control (AC-209-05): removing #[deprecated] fires 10 unfulfilled_lint_ expectations in fix.rs + 1 in api_surface.rs (discovered via separate target runs due to cargo dependency ordering). cargo clippy --workspace --all-targets -- -D warnings: EXIT=0 (AC-209-03). cargo nextest run -p mds-core: EXIT=0. cargo test --doc -p mds-core: EXIT=0, zero deprecated warnings (AC-209-12). cargo fmt --all --check: EXIT=0. node scripts/verify-no-control-bytes.mjs: EXIT=0. node scripts/verify-versions.mjs: EXIT=0. ## Notes - `since` omitted from #[deprecated] per OD-209-B to avoid drift risk; bump-version.mjs cannot rewrite .rs files. - Pre-existing rustdoc -D warnings failures (21 errors in fs.rs, config.rs, diagnostic.rs) are unrelated to this PR; AC-209-11 is satisfied for my changes (no new broken links). - OD-209-E resolved to "defer": six ADR-004 coverage tests enumerated in AD-209-1 rustdoc and issue #209 cited as the v0.5.0 removal tracker. applies ADR-004, avoids PF-009, avoids PF-013, avoids PF-015 Co-Authored-By: Claude --- .devflow/features/mds-lint/KNOWLEDGE.md | 2 +- CHANGELOG.md | 21 +++++ crates/mds-core/src/lint/fix.rs | 116 +++++++++++++++++++++--- crates/mds-core/tests/api_surface.rs | 41 +++++++++ 4 files changed, 165 insertions(+), 15 deletions(-) diff --git a/.devflow/features/mds-lint/KNOWLEDGE.md b/.devflow/features/mds-lint/KNOWLEDGE.md index c0f53e1..939f8fa 100644 --- a/.devflow/features/mds-lint/KNOWLEDGE.md +++ b/.devflow/features/mds-lint/KNOWLEDGE.md @@ -464,7 +464,7 @@ LintDiagnostic.fix_removals (FixLineSpan) OR .fix_edits (TextEdit) - **Resting a security invariant on `debug_assert!`** (PF-005): `debug_assert!` is compiled out of release. The old `SanitizedReport` returned `None` for `source()`/`related()` behind a `debug_assert!` that no CLI error populates the aux graph — real in tests, absent in the shipped binary. Enforce invariants with data transformation, not assertions. -- **Calling `apply_plan_unchecked()` on a production write path**: Production code that writes back to disk MUST use `apply_fixes()` or `apply_fixes_incremental()`. The `_unchecked` suffix makes the bypass explicit at every call site. +- **Calling `apply_plan_unchecked()` on a production write path**: Production code that writes back to disk MUST use `apply_fixes_incremental()`. The `_unchecked` suffix makes the bypass explicit at every call site. - **Adding a ModuleCache "optimization"**: Per-file fresh resolve is intentional. A shared cache would be unsafe because runtime vars are per-call. diff --git a/CHANGELOG.md b/CHANGELOG.md index a0ab0e8..3c5a12e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1136,6 +1136,27 @@ diagnostic messages must update to check for the `\uXXXX` literal form instead. for awareness — it clears as a side effect of applying other fixes (e.g. removing a duplicate import that was also the unused one). +### Deprecated + +- **`mds::fix::apply_fixes` is deprecated in favor of `apply_fixes_incremental` (#209).** + The replacement applies the same ADR-004 three-tier reverify-gate safety contract with a + batch-first attempt plus a bounded per-edit fallback, salvaging the safe subset of fixes + when some edits fail the reverify gate rather than refusing the whole batch. + + Two behavioral differences require manual migration: + + - **Closure bound**: `apply_fixes` takes `F: FnOnce`; `apply_fixes_incremental` requires + `F: Fn` because the reverify closure may be called more than once. A move-once closure + cannot be migrated mechanically. + - **New reachable outcome**: `apply_fixes_incremental` can return + `FixOutcome::PartiallyFixed` when some edits are accepted and some are refused. + `apply_fixes` never returns `PartiallyFixed`. Because `FixOutcome` is + `#[non_exhaustive]`, existing wildcard arms compile unchanged, but a wildcard that + swallows `PartiallyFixed` silently discards partial results. + + Scheduled for removal in v0.5.0. The six ADR-004 regression tests that are pinned only + against `apply_fixes` must be ported or explicitly tracked before removal (issue #209). + ### Fixed - **`mds build -o build/out.md` with sources in `src/` again emits map-relative diff --git a/crates/mds-core/src/lint/fix.rs b/crates/mds-core/src/lint/fix.rs index 545bc78..dd9896f 100644 --- a/crates/mds-core/src/lint/fix.rs +++ b/crates/mds-core/src/lint/fix.rs @@ -235,7 +235,7 @@ fn reverify_failure_reason(err: &MdsError) -> String { /// A plan of fix edits for a single file's source. /// /// Obtain via [`plan_fixes`] or [`plan_fixes_with_options`], then pass to -/// [`apply_fixes`] or [`apply_fixes_incremental`]. External crates that need an +/// [`apply_fixes_incremental`]. External crates that need an /// empty plan can use `FixPlan::default()`; its fields are `pub`, so they remain /// directly readable and writable. /// @@ -582,22 +582,22 @@ fn regressed_rules( /// single pass, so earlier edits' offsets remain valid after later edits are /// applied. /// -/// # `_unchecked` suffix — ADR-001 +/// # `_unchecked` suffix — ADR-004 /// -/// This function bypasses the ADR-001 reverify gate (compile-equivalence -/// check) — it applies edits without recompiling or verifying that the fixed -/// source produces identical compiled output. Production code that writes back -/// to disk **must** use [`apply_fixes`] instead, which gates on the reverify -/// callback before returning `FixOutcome::Fixed`. `apply_plan_unchecked` is -/// provided for the `--fix --diff` / `--fix --check` diff-preview path (which -/// computes the delta without writing it) and for unit tests. Calling it on a -/// write path without a subsequent reverify is an anti-pattern — the reverify -/// gate is the only guard against a fix that accidentally changes compiled -/// semantics. +/// This function bypasses the ADR-004 reverify gate — it applies edits without +/// recompiling or verifying that the fixed source produces identical compiled +/// output. Production code that writes back to disk **must** use +/// [`apply_fixes_incremental`] instead, which gates on the reverify callback +/// before returning `FixOutcome::Fixed` or `FixOutcome::PartiallyFixed`. +/// `apply_plan_unchecked` is provided for the `--fix --diff` / `--fix --check` +/// diff-preview path (which computes the delta without writing it) and for unit +/// tests. Calling it on a write path without a subsequent reverify is an +/// anti-pattern -- the reverify gate is the only guard against a fix that +/// accidentally changes compiled semantics. /// /// The caller must pass `plan` with `overlap_rejected == false`; if true, -/// calling this function is a logic error (use [`apply_fixes`] which checks -/// this). +/// calling this function is a logic error (use [`apply_fixes_incremental`] which +/// checks this). /// /// # Panics /// @@ -660,6 +660,51 @@ pub fn apply_plan_unchecked(source: &str, plan: &FixPlan) -> String { result } +/// # Deprecated (AD-209-1) +/// +/// Use [`apply_fixes_incremental`] instead. (applies ADR-004) +/// +/// `apply_fixes` implements the ADR-004 three-tier reverify gate as a single +/// all-or-nothing call: `reverify` runs once and either the whole batch is +/// accepted or the whole batch is refused. `apply_fixes_incremental` provides +/// the same safety contract with a batch-first attempt plus a bounded per-edit +/// fallback (capped at `FALLBACK_MAX_EDITS = 50`), which salvages the safe +/// subset rather than refusing wholesale. That is why the deprecation is +/// correct rather than arbitrary (applies ADR-004). +/// +/// ## Migration deltas (not a drop-in replacement) +/// +/// 1. **Closure bound change**: `apply_fixes` takes `F: FnOnce`; +/// `apply_fixes_incremental` requires `F: Fn` because `reverify` may be +/// called more than once. A move-once closure cannot migrate mechanically. +/// +/// 2. **New reachable outcome**: `apply_fixes_incremental` can return +/// `FixOutcome::PartiallyFixed` (some edits accepted, some refused). +/// `apply_fixes` never returns `PartiallyFixed`. `FixOutcome` is +/// `#[non_exhaustive]`, so existing wildcard arms still compile, but a +/// wildcard that swallows `PartiallyFixed` silently discards partial +/// results. +/// +/// 3. **Reverify call count**: `apply_fixes` calls `reverify` exactly once; +/// `apply_fixes_incremental` calls it up to `plan.edits.len() + 1` times. +/// +/// ## Why the body was not deleted +/// +/// `crates/mds-core/src/lint/fix.rs` does not exist at tag `v0.3.0` (the +/// newest published tag at this commit), so `apply_fixes` has never been +/// published to crates.io. However, deleting it would silently drop coverage +/// of six ADR-004 reverify-gate behaviors that are pinned only through this +/// function, with no equivalent on the `apply_fixes_incremental` path. These +/// tests must be ported or explicitly tracked before removal at v0.5.0 +/// (see issue #209): +/// +/// - `a4_partial_overlap_still_rejected_after_dedup` (fix.rs:1392) -- A4 +/// - `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` (fix.rs:1681) -- A5 +/// - `reverify_preexisting_untargeted_survives_and_fix_applies` (fix.rs:1869) -- AC-F-23 +/// - `reverify_new_untargeted_diagnostic_is_rejected` (fix.rs:1890) +/// - `tier_b_unused_function_standalone_apply_succeeds` (fix.rs:1957) -- I-13 +/// - `l_fix_rev1_output_delta_causes_rejection` (fix.rs:2024) -- L-FIX-REV1 +/// /// Apply a `FixPlan` with a reverify callback. /// /// The `reverify` callback is called with the fixed source and must return: @@ -688,6 +733,9 @@ pub fn apply_plan_unchecked(source: &str, plan: &FixPlan) -> String { /// /// Returns `FixOutcome::Fixed`, `FixOutcome::Rejected`, or `FixOutcome::NothingToFix`. #[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(source: &str, plan: FixPlan, original: &LintResult, reverify: F) -> FixOutcome where F: FnOnce(&str) -> Result, @@ -1388,6 +1436,10 @@ mod tests { /// `dedup_contained_or_identical`: B.end=18 > max_end(12) → B is not contained, /// both edits are retained. /// `has_overlapping_edits`: A.end=12 > B.start=6 → partial overlap → rejected. + #[expect( + deprecated, + reason = "AD-209-1: exercises deprecated apply_fixes path pending v0.5.0 removal" + )] #[test] fn a4_partial_overlap_still_rejected_after_dedup() { let source = "line0\nline1\nline2\n"; @@ -1650,6 +1702,10 @@ mod tests { // ── L-FIX-REV1: Reverify gate ──────────────────────────────────────────── + #[expect( + deprecated, + reason = "AD-209-1: exercises deprecated apply_fixes path pending v0.5.0 removal" + )] #[test] fn l_fix_rev1_reverify_failure_rejects_fix() { let source = "@import \"./a.mds\" as a\n@import \"./a.mds\" as b\n"; @@ -1677,6 +1733,10 @@ mod tests { /// /// The embedded `{err}` is asserted non-empty, confirming that a real error /// was propagated rather than a blank placeholder. + #[expect( + deprecated, + reason = "AD-209-1: exercises deprecated apply_fixes path pending v0.5.0 removal" + )] #[test] fn l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix() { let source = "@import \"./a.mds\" as a\n@import \"./a.mds\" as b\n"; @@ -1735,6 +1795,10 @@ mod tests { /// T-REASON-1 [security-11 / CWE-117 / PF-013 / #176]: the reverify failure reason /// produced by `apply_fixes` escapes the embedded `MdsError` Display. + #[expect( + deprecated, + reason = "AD-209-1: exercises deprecated apply_fixes path pending v0.5.0 removal" + )] #[test] fn apply_fixes_rejection_reason_escapes_embedded_error_display() { let source = "@import \"./a.mds\" as a\n@import \"./a.mds\" as b\n"; @@ -1829,6 +1893,10 @@ mod tests { /// The source has the second `@import` starting at byte 23 /// (`"@import \"./a.mds\" as a\n"` = 23 bytes), which is a valid char /// boundary, so `diag_to_edit` succeeds and the plan is non-empty. + #[expect( + deprecated, + reason = "AD-209-1: exercises deprecated apply_fixes path pending v0.5.0 removal" + )] #[test] fn reverify_success_returns_fixed() { let source = "@import \"./a.mds\" as a\n@import \"./a.mds\" as b\nHello!\n"; @@ -1865,6 +1933,10 @@ mod tests { /// `unused-variable` that coexists with a fixable `duplicate-import`) survives the /// reverify but must NOT cause the fix to be refused — residual findings are /// expected to remain and determine the exit code. + #[expect( + deprecated, + reason = "AD-209-1: exercises deprecated apply_fixes path pending v0.5.0 removal" + )] #[test] fn reverify_preexisting_untargeted_survives_and_fix_applies() { let source = "@import \"./a.mds\" as a\n@import \"./a.mds\" as b\nHello!\n"; @@ -1886,6 +1958,10 @@ mod tests { /// A genuinely NEW untargeted diagnostic introduced by the edit IS a regression /// and must refuse the fix. + #[expect( + deprecated, + reason = "AD-209-1: exercises deprecated apply_fixes path pending v0.5.0 removal" + )] #[test] fn reverify_new_untargeted_diagnostic_is_rejected() { let source = "@import \"./a.mds\" as a\n@import \"./a.mds\" as b\nHello!\n"; @@ -1953,6 +2029,10 @@ mod tests { /// so `unused-import` cannot fire on a standalone file. `unused-function` fires /// when `has_explicit_exports && !exported && !called` — achieved here with an /// explicit `@export greet` plus an unexported, uncalled `@define dead():`. + #[expect( + deprecated, + reason = "AD-209-1: exercises deprecated apply_fixes path pending v0.5.0 removal" + )] #[test] fn tier_b_unused_function_standalone_apply_succeeds() { // Standalone source: no @import, no @extends, has explicit @export. @@ -2020,6 +2100,10 @@ mod tests { /// /// This verifies the mechanism the CLI relies on: when the reverify closure /// returns `Err` due to an output delta, the entire fix batch is refused. + #[expect( + deprecated, + reason = "AD-209-1: exercises deprecated apply_fixes path pending v0.5.0 removal" + )] #[test] fn l_fix_rev1_output_delta_causes_rejection() { let source = "Hello World!\n"; @@ -2367,6 +2451,10 @@ mod tests { /// PF-005 regression: `apply_fixes` with unsorted edits must return /// `FixOutcome::Rejected`, NOT silently corrupt the source. + #[expect( + deprecated, + reason = "AD-209-1: exercises deprecated apply_fixes path pending v0.5.0 removal" + )] #[test] fn pf005_unsorted_edits_rejected_in_apply_fixes() { let source = "LineA\nLineB\n"; diff --git a/crates/mds-core/tests/api_surface.rs b/crates/mds-core/tests/api_surface.rs index c1888e8..243cdc2 100644 --- a/crates/mds-core/tests/api_surface.rs +++ b/crates/mds-core/tests/api_surface.rs @@ -1649,6 +1649,47 @@ fn fix_api_incremental_exists() { ); } +/// F-API-3: `apply_fixes` remains reachable on the public API surface and behaviorally +/// unchanged while deprecated. +/// +/// AD-209-2: `#[expect(deprecated)]` was chosen over a `trybuild` compile-fail fixture +/// because: (a) trybuild only asserts that the deprecation warning fires; it does not +/// verify the function's signature or return value; (b) this test also asserts the +/// runtime behavior (NothingToFix for an empty plan), giving a stronger pin; and +/// (c) `#[expect(deprecated)]` fires `unfulfilled_lint_expectations` under +/// `cargo clippy --workspace --all-targets -- -D warnings` whenever the +/// `#[deprecated]` attribute is removed from `apply_fixes`, providing the same +/// "must not compile cleanly without the attribute" guarantee as trybuild without +/// a separate test-driver crate. +/// +/// All values constructed via named constructors, never struct literals (applies ADR-010). +// AD-209-1: apply_fixes is deprecated; this pin is the F-API-3 compile-and-runtime +// guard for the deprecated public path. Remove at v0.5.0 with the function. +#[expect( + deprecated, + reason = "AD-209-2: F-API-3 pins the deprecated apply_fixes public API surface; see fix.rs rustdoc" +)] +#[test] +fn fix_api_apply_fixes_exists() { + use mds::fix::{apply_fixes, plan_fixes, FixOutcome}; + + // Construct via named constructors; never struct literals (applies ADR-010). + let source = "Hello!\n"; + let original = LintResult::new(vec![]); + let plan = plan_fixes(&original, source); + let outcome = apply_fixes( + source, + plan, + &original, + |_s| -> Result { Ok(LintResult::new(vec![])) }, + ); + // Empty source with no diagnostics must return NothingToFix (no reverify called). + assert!( + matches!(outcome, FixOutcome::NothingToFix), + "trivial source with no diagnostics must return NothingToFix; got: {outcome:?}" + ); +} + /// Regression gate (issue #9): `STRING_SOURCE_MAP_LABEL` must be reachable from /// the public `mds` API so every surface can import it rather than redeclaring /// the literal (avoids PF-007 per-surface re-declaration defeating cross-surface From 7dbb9708e46d27944f681e817ab0b3e868bdebc7 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 23:28:36 +0300 Subject: [PATCH 02/28] chore(mds-core): simplify PR5 deprecation docs (style + stale line numbers) - fix.rs: restore em dash in apply_plan_unchecked doc (was regressed to double-hyphen; file uses em dashes throughout) - fix.rs: update six stale test line numbers in AD-209-1 deprecated section (PR5's own additions shifted them 52-84 lines) - api_surface.rs: fold // AD-209-1 removal note into the /// doc block; remove misplaced code comment between /// and #[expect] --- crates/mds-core/src/lint/fix.rs | 14 +++++++------- crates/mds-core/tests/api_surface.rs | 4 +--- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/crates/mds-core/src/lint/fix.rs b/crates/mds-core/src/lint/fix.rs index dd9896f..8aaacc7 100644 --- a/crates/mds-core/src/lint/fix.rs +++ b/crates/mds-core/src/lint/fix.rs @@ -592,7 +592,7 @@ fn regressed_rules( /// `apply_plan_unchecked` is provided for the `--fix --diff` / `--fix --check` /// diff-preview path (which computes the delta without writing it) and for unit /// tests. Calling it on a write path without a subsequent reverify is an -/// anti-pattern -- the reverify gate is the only guard against a fix that +/// anti-pattern — the reverify gate is the only guard against a fix that /// accidentally changes compiled semantics. /// /// The caller must pass `plan` with `overlap_rejected == false`; if true, @@ -698,12 +698,12 @@ pub fn apply_plan_unchecked(source: &str, plan: &FixPlan) -> String { /// tests must be ported or explicitly tracked before removal at v0.5.0 /// (see issue #209): /// -/// - `a4_partial_overlap_still_rejected_after_dedup` (fix.rs:1392) -- A4 -/// - `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` (fix.rs:1681) -- A5 -/// - `reverify_preexisting_untargeted_survives_and_fix_applies` (fix.rs:1869) -- AC-F-23 -/// - `reverify_new_untargeted_diagnostic_is_rejected` (fix.rs:1890) -/// - `tier_b_unused_function_standalone_apply_succeeds` (fix.rs:1957) -- I-13 -/// - `l_fix_rev1_output_delta_causes_rejection` (fix.rs:2024) -- L-FIX-REV1 +/// - `a4_partial_overlap_still_rejected_after_dedup` (fix.rs:1444) -- A4 +/// - `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` (fix.rs:1741) -- A5 +/// - `reverify_preexisting_untargeted_survives_and_fix_applies` (fix.rs:1941) -- AC-F-23 +/// - `reverify_new_untargeted_diagnostic_is_rejected` (fix.rs:1966) +/// - `tier_b_unused_function_standalone_apply_succeeds` (fix.rs:2037) -- I-13 +/// - `l_fix_rev1_output_delta_causes_rejection` (fix.rs:2108) -- L-FIX-REV1 /// /// Apply a `FixPlan` with a reverify callback. /// diff --git a/crates/mds-core/tests/api_surface.rs b/crates/mds-core/tests/api_surface.rs index 243cdc2..f34f131 100644 --- a/crates/mds-core/tests/api_surface.rs +++ b/crates/mds-core/tests/api_surface.rs @@ -1650,7 +1650,7 @@ fn fix_api_incremental_exists() { } /// F-API-3: `apply_fixes` remains reachable on the public API surface and behaviorally -/// unchanged while deprecated. +/// unchanged while deprecated. Remove at v0.5.0 with the function (AD-209-1). /// /// AD-209-2: `#[expect(deprecated)]` was chosen over a `trybuild` compile-fail fixture /// because: (a) trybuild only asserts that the deprecation warning fires; it does not @@ -1663,8 +1663,6 @@ fn fix_api_incremental_exists() { /// a separate test-driver crate. /// /// All values constructed via named constructors, never struct literals (applies ADR-010). -// AD-209-1: apply_fixes is deprecated; this pin is the F-API-3 compile-and-runtime -// guard for the deprecated public path. Remove at v0.5.0 with the function. #[expect( deprecated, reason = "AD-209-2: F-API-3 pins the deprecated apply_fixes public API surface; see fix.rs rustdoc" From 55ff5049aee857cca81d78d2cd001dea5f0754ee Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 23:38:58 +0300 Subject: [PATCH 03/28] fix: address self-review issues [#209] Three defects found by the 9-pillar self-review of the PR5 deprecation. 1. Rustdoc summary regression (Documentation). Prepending `# Deprecated (AD-209-1)` above the summary sentence made rustdoc render `apply_fixes`'s one-line description in the `mds::fix` module index as literally "Deprecated (AD-209-1)" instead of "Apply a `FixPlan` with a reverify callback." (confirmed in the generated index.html). The summary sentence is restored to the top and a `# Behavior` heading now separates the post-deprecation prose so it is not visually nested under the Deprecated section. Rustdoc already renders its own Deprecated badge and banner, so the notice loses no prominence. 2. Deprecation note did not name the removal version (AC-209-01). The note now reads "... Removed in v0.5.0; see the item docs." at 137 chars, inside the 160-char budget, and the rendered rustdoc banner carries it. 3. F-API-3's `F: FnOnce` pin was vacuous (AC-209-02). The old closure was an `Fn` closure, which satisfies both bounds, so tightening `apply_fixes` to `F: Fn` would have compiled clean. The reverify closure now moves out of a captured `String`, making it `FnOnce`-only. Positive control: tightening the bound to `F: Fn` fails with E0507 at api_surface.rs:1688 (avoids PF-013). Also refreshed the six cited test line numbers in the AD-209-1 rustdoc, which drifted by +2 from change 1, cross-checked against the fn declarations. Verified: cargo fmt --all --check; cargo clippy --workspace --all-targets -D warnings (0); cargo nextest run -p mds-core -p mds-cli (2087 passed); cargo test --doc (52 passed, zero deprecated warnings); F-API-1 and F-API-3 pass; source-hygiene gate exit 0; verify-versions exit 0. Reverse mutation control re-run: exactly 11 unfulfilled lint expectations (10 in fix.rs, 1 at api_surface.rs:1667), clean after restore. --- crates/mds-core/src/lint/fix.rs | 18 ++++++++++-------- crates/mds-core/tests/api_surface.rs | 10 +++++++++- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/crates/mds-core/src/lint/fix.rs b/crates/mds-core/src/lint/fix.rs index 8aaacc7..7c78171 100644 --- a/crates/mds-core/src/lint/fix.rs +++ b/crates/mds-core/src/lint/fix.rs @@ -660,6 +660,8 @@ pub fn apply_plan_unchecked(source: &str, plan: &FixPlan) -> String { result } +/// Apply a `FixPlan` with a reverify callback. +/// /// # Deprecated (AD-209-1) /// /// Use [`apply_fixes_incremental`] instead. (applies ADR-004) @@ -698,14 +700,14 @@ pub fn apply_plan_unchecked(source: &str, plan: &FixPlan) -> String { /// tests must be ported or explicitly tracked before removal at v0.5.0 /// (see issue #209): /// -/// - `a4_partial_overlap_still_rejected_after_dedup` (fix.rs:1444) -- A4 -/// - `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` (fix.rs:1741) -- A5 -/// - `reverify_preexisting_untargeted_survives_and_fix_applies` (fix.rs:1941) -- AC-F-23 -/// - `reverify_new_untargeted_diagnostic_is_rejected` (fix.rs:1966) -/// - `tier_b_unused_function_standalone_apply_succeeds` (fix.rs:2037) -- I-13 -/// - `l_fix_rev1_output_delta_causes_rejection` (fix.rs:2108) -- L-FIX-REV1 +/// - `a4_partial_overlap_still_rejected_after_dedup` (fix.rs:1446) -- A4 +/// - `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` (fix.rs:1743) -- A5 +/// - `reverify_preexisting_untargeted_survives_and_fix_applies` (fix.rs:1943) -- AC-F-23 +/// - `reverify_new_untargeted_diagnostic_is_rejected` (fix.rs:1968) +/// - `tier_b_unused_function_standalone_apply_succeeds` (fix.rs:2039) -- I-13 +/// - `l_fix_rev1_output_delta_causes_rejection` (fix.rs:2110) -- L-FIX-REV1 /// -/// Apply a `FixPlan` with a reverify callback. +/// # Behavior /// /// The `reverify` callback is called with the fixed source and must return: /// - `Ok(LintResult)`: the lint result of the fixed source (may be empty). @@ -734,7 +736,7 @@ pub fn apply_plan_unchecked(source: &str, plan: &FixPlan) -> String { /// Returns `FixOutcome::Fixed`, `FixOutcome::Rejected`, or `FixOutcome::NothingToFix`. #[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." + note = "use `apply_fixes_incremental`; not a drop-in swap, the reverify closure must be `Fn`, not `FnOnce`. Removed in v0.5.0; see the item docs." )] pub fn apply_fixes(source: &str, plan: FixPlan, original: &LintResult, reverify: F) -> FixOutcome where diff --git a/crates/mds-core/tests/api_surface.rs b/crates/mds-core/tests/api_surface.rs index f34f131..9b7e976 100644 --- a/crates/mds-core/tests/api_surface.rs +++ b/crates/mds-core/tests/api_surface.rs @@ -1675,11 +1675,19 @@ fn fix_api_apply_fixes_exists() { let source = "Hello!\n"; let original = LintResult::new(vec![]); let plan = plan_fixes(&original, source); + // The closure moves out of a captured `String`, so it implements `FnOnce` but + // NOT `Fn`/`FnMut`. That makes this a real compile-time pin on the `F: FnOnce` + // bound: tightening `apply_fixes` to `F: Fn` (the `apply_fixes_incremental` + // bound) would break this test's compilation rather than pass silently. + let move_once = String::from("consumed-by-value"); let outcome = apply_fixes( source, plan, &original, - |_s| -> Result { Ok(LintResult::new(vec![])) }, + move |_s| -> Result { + drop(move_once); + Ok(LintResult::new(vec![])) + }, ); // Empty source with no diagnostics must return NothingToFix (no reverify called). assert!( From 7dfa6ddfddc59782e382a4dbb6c545f6274aa8a5 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 16 Aug 2026 23:59:23 +0300 Subject: [PATCH 04/28] fix(mds-core): address Evaluator alignment findings [#209] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1/F2 (Orchestrator item (a) + AC-209-16): add `since = "0.4.0"` to the `apply_fixes` #[deprecated] attribute, matching config.rs:296 (LintConfig:: from_rules) which established the convention earlier in this wave. Both siblings now carry identical attribute shapes. grep -rn 'since = ' crates/ --include='*.rs' returns both hits; no zero-hit branch remains. F4 (AC-209-15, self-defeating enumeration): the six ADR-004 reverify-gate test names were inside apply_fixes' own doc block — a self-deleting location that disappears when the function is removed at v0.5.0. Moved the canonical list to .devflow/features/mds-lint/KNOWLEDGE.md under "v0.5.0 Removal Tracker: apply_fixes" (a tracked, persistent file). Updated fix.rs:700-704 to reference KNOWLEDGE.md instead of "issue #209" (which would close on merge, creating a dead reference). Updated CHANGELOG.md to reference KNOWLEDGE.md. "Closes #209" removed from PR body so issue #209 stays open as the tracker. F3 (AC-209-11) and remaining AC-209-15 (no removal-tracker issue): documented explicitly in PR body — 20 rustdoc errors are pre-existing (git diff --name-only confirms none in this PR's changed files); substantive intra-doc link resolution verified independently. A GitHub removal-tracker issue must still be opened by a human before the v0.5.0 tag. P1/P2 (plan-text defects): corrected in PR body — reverse-mutation count (10 under -D warnings / 11 across two targets), note length (137 chars), WASM AC label (AC-209-10 not AC-209-13), since field description updated. No executable statements changed. Source hygiene gate: exit 0. cargo check -p mds-core: clean. cargo clippy -p mds-core --all-targets -- -D warnings: zero warnings. --- .devflow/features/mds-lint/KNOWLEDGE.md | 22 ++++++++++++++++++++++ CHANGELOG.md | 4 +++- crates/mds-core/src/lint/fix.rs | 15 ++++++--------- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/.devflow/features/mds-lint/KNOWLEDGE.md b/.devflow/features/mds-lint/KNOWLEDGE.md index 939f8fa..22eaba5 100644 --- a/.devflow/features/mds-lint/KNOWLEDGE.md +++ b/.devflow/features/mds-lint/KNOWLEDGE.md @@ -582,3 +582,25 @@ LintDiagnostic.fix_removals (FixLineSpan) OR .fix_edits (TextEdit) - `crates/mds-core/tests/api_surface.rs` — pins the public lint API signatures. - `.devflow/features/mds-fmt/KNOWLEDGE.md` — `mds fmt` knowledge base; `atomic_write_file` is shared between both subcommands via `output.rs`. - `.devflow/features/source-map-security/KNOWLEDGE.md` — source map path-containment choke-point. + +## v0.5.0 Removal Tracker: apply_fixes + +`mds::fix::apply_fixes` is deprecated as of v0.4.0 and scheduled for removal at v0.5.0. +Before the v0.5.0 tag, the following six ADR-004 reverify-gate behaviors must either +gain equivalent coverage on the `apply_fixes_incremental` path or be carried in a +GitHub removal-tracker issue (to be opened before the v0.5.0 branch cuts). +This list is kept here — in a tracked, persistent file — so it survives the function's +own deletion. + +| Test name | Line (v0.4.0 HEAD) | Behavior pinned | +|---|---|---| +| `a4_partial_overlap_still_rejected_after_dedup` | fix.rs:1446 | A4: overlapping edits refused after dedup | +| `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` | fix.rs:1743 | A5: rejection message stability | +| `reverify_preexisting_untargeted_survives_and_fix_applies` | fix.rs:1943 | AC-F-23: pre-existing untargeted diagnostic survives | +| `reverify_new_untargeted_diagnostic_is_rejected` | fix.rs:1968 | reverify gate rejects new untargeted diagnostics | +| `tier_b_unused_function_standalone_apply_succeeds` | fix.rs:2039 | I-13: Tier B fix applies on standalone file | +| `l_fix_rev1_output_delta_causes_rejection` | fix.rs:2110 | L-FIX-REV1: output delta causes rejection | + +When a removal-tracker issue is opened, update `crates/mds-core/src/lint/fix.rs` (the +`apply_fixes` doc block) and `CHANGELOG.md` to reference the issue number, then remove +this table once the six behaviors are covered by `apply_fixes_incremental` tests. diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c5a12e..bcebc3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1155,7 +1155,9 @@ diagnostic messages must update to check for the `\uXXXX` literal form instead. swallows `PartiallyFixed` silently discards partial results. Scheduled for removal in v0.5.0. The six ADR-004 regression tests that are pinned only - against `apply_fixes` must be ported or explicitly tracked before removal (issue #209). + against `apply_fixes` must be ported or tracked in a GitHub removal-tracker issue before + the v0.5.0 tag; the canonical list is in `.devflow/features/mds-lint/KNOWLEDGE.md` + under "v0.5.0 Removal Tracker: apply_fixes". ### Fixed diff --git a/crates/mds-core/src/lint/fix.rs b/crates/mds-core/src/lint/fix.rs index 7c78171..4dd3a88 100644 --- a/crates/mds-core/src/lint/fix.rs +++ b/crates/mds-core/src/lint/fix.rs @@ -697,15 +697,11 @@ pub fn apply_plan_unchecked(source: &str, plan: &FixPlan) -> String { /// published to crates.io. However, deleting it would silently drop coverage /// of six ADR-004 reverify-gate behaviors that are pinned only through this /// function, with no equivalent on the `apply_fixes_incremental` path. These -/// tests must be ported or explicitly tracked before removal at v0.5.0 -/// (see issue #209): -/// -/// - `a4_partial_overlap_still_rejected_after_dedup` (fix.rs:1446) -- A4 -/// - `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` (fix.rs:1743) -- A5 -/// - `reverify_preexisting_untargeted_survives_and_fix_applies` (fix.rs:1943) -- AC-F-23 -/// - `reverify_new_untargeted_diagnostic_is_rejected` (fix.rs:1968) -/// - `tier_b_unused_function_standalone_apply_succeeds` (fix.rs:2039) -- I-13 -/// - `l_fix_rev1_output_delta_causes_rejection` (fix.rs:2110) -- L-FIX-REV1 +/// tests must be ported or explicitly tracked before removal at v0.5.0. +/// The canonical list is kept in `.devflow/features/mds-lint/KNOWLEDGE.md` +/// under "v0.5.0 Removal Tracker: apply_fixes" so it survives this function's +/// deletion. A GitHub removal-tracker issue must be opened and cross-linked +/// before the v0.5.0 tag. /// /// # Behavior /// @@ -736,6 +732,7 @@ pub fn apply_plan_unchecked(source: &str, plan: &FixPlan) -> String { /// Returns `FixOutcome::Fixed`, `FixOutcome::Rejected`, or `FixOutcome::NothingToFix`. #[must_use = "a dropped FixOutcome silently discards the fix result"] #[deprecated( + since = "0.4.0", note = "use `apply_fixes_incremental`; not a drop-in swap, the reverify closure must be `Fn`, not `FnOnce`. Removed in v0.5.0; see the item docs." )] pub fn apply_fixes(source: &str, plan: FixPlan, original: &LintResult, reverify: F) -> FixOutcome From 82f46fb8d60417e3164195b92ec4ec1a8e767133 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Mon, 17 Aug 2026 00:25:19 +0300 Subject: [PATCH 05/28] fix(mds-core): fix 3 blocking QA scenarios for PR5 deprecation [#209] S11: fix 20 pre-existing broken intra-doc links in the wave branch so RUSTDOCFLAGS="-D warnings" cargo doc -p mds-core --no-deps exits 0. Changes per file: - fs.rs: MAX_PATH_SEGMENTS is private; remove intra-doc link - lint/config.rs: qualify sanitize_control_chars_wire with crate:: path - lint/diagnostic.rs: fix [.truncated()] / [.standalone()] to use valid rustdoc link syntax ([LintResult::truncated] etc.); qualify to_canonical_json reference - lint/fix.rs: remove link to private dedup_contained_or_identical - resolver.rs: qualify resolve_*_intrinsic links with Self:: so rustdoc resolves them within the impl block - source_path.rs: MapBuilder is pub(crate); remove private intra-doc link - sourcemap.rs: remove links to private MapBuilder, LineTable, encode_mappings, map_source_label items S13: CHANGELOG [Unreleased] Deprecated section now cites live GitHub issue #304 ("v0.5.0 removal tracker: delete apply_fixes") as the removal-tracker issue, satisfying AC-209-13's requirement for a numeric issue reference. S15: AD-209-1 in fix.rs now cross-links #304 (the removal-tracker issue body enumerates the six tests by name, line, and behavior pinned). KNOWLEDGE.md updated to reference #304 as the active tracker. Co-Authored-By: Claude --- .devflow/features/mds-lint/KNOWLEDGE.md | 13 +++++-------- CHANGELOG.md | 8 ++++---- crates/mds-core/src/fs.rs | 2 +- crates/mds-core/src/lint/config.rs | 2 +- crates/mds-core/src/lint/diagnostic.rs | 6 +++--- crates/mds-core/src/lint/fix.rs | 10 ++++------ crates/mds-core/src/resolver.rs | 8 ++++---- crates/mds-core/src/source_path.rs | 2 +- crates/mds-core/src/sourcemap.rs | 12 ++++++------ 9 files changed, 29 insertions(+), 34 deletions(-) diff --git a/.devflow/features/mds-lint/KNOWLEDGE.md b/.devflow/features/mds-lint/KNOWLEDGE.md index 22eaba5..51bbae4 100644 --- a/.devflow/features/mds-lint/KNOWLEDGE.md +++ b/.devflow/features/mds-lint/KNOWLEDGE.md @@ -586,11 +586,9 @@ LintDiagnostic.fix_removals (FixLineSpan) OR .fix_edits (TextEdit) ## v0.5.0 Removal Tracker: apply_fixes `mds::fix::apply_fixes` is deprecated as of v0.4.0 and scheduled for removal at v0.5.0. -Before the v0.5.0 tag, the following six ADR-004 reverify-gate behaviors must either -gain equivalent coverage on the `apply_fixes_incremental` path or be carried in a -GitHub removal-tracker issue (to be opened before the v0.5.0 branch cuts). -This list is kept here — in a tracked, persistent file — so it survives the function's -own deletion. +The removal is tracked in GitHub issue #304. Before the v0.5.0 tag, the following six +ADR-004 reverify-gate behaviors must gain equivalent coverage on the +`apply_fixes_incremental` path or be explicitly retired in #304. | Test name | Line (v0.4.0 HEAD) | Behavior pinned | |---|---|---| @@ -601,6 +599,5 @@ own deletion. | `tier_b_unused_function_standalone_apply_succeeds` | fix.rs:2039 | I-13: Tier B fix applies on standalone file | | `l_fix_rev1_output_delta_causes_rejection` | fix.rs:2110 | L-FIX-REV1: output delta causes rejection | -When a removal-tracker issue is opened, update `crates/mds-core/src/lint/fix.rs` (the -`apply_fixes` doc block) and `CHANGELOG.md` to reference the issue number, then remove -this table once the six behaviors are covered by `apply_fixes_incremental` tests. +Remove this table once all six behaviors are covered by `apply_fixes_incremental` tests +(migration complete) or retired in #304. diff --git a/CHANGELOG.md b/CHANGELOG.md index bcebc3b..52c5fdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1154,10 +1154,10 @@ diagnostic messages must update to check for the `\uXXXX` literal form instead. `#[non_exhaustive]`, existing wildcard arms compile unchanged, but a wildcard that swallows `PartiallyFixed` silently discards partial results. - Scheduled for removal in v0.5.0. The six ADR-004 regression tests that are pinned only - against `apply_fixes` must be ported or tracked in a GitHub removal-tracker issue before - the v0.5.0 tag; the canonical list is in `.devflow/features/mds-lint/KNOWLEDGE.md` - under "v0.5.0 Removal Tracker: apply_fixes". + Scheduled for removal in v0.5.0; tracked in GitHub issue #304. The six ADR-004 + regression tests pinned only against `apply_fixes` must be ported or retired before + the v0.5.0 tag (see #304 for the enumerated list with line numbers and the behavior + each test pins). ### Fixed diff --git a/crates/mds-core/src/fs.rs b/crates/mds-core/src/fs.rs index 6e8d0fa..dc83144 100644 --- a/crates/mds-core/src/fs.rs +++ b/crates/mds-core/src/fs.rs @@ -71,7 +71,7 @@ pub trait FileSystem: Send + Sync { /// - the resolved path escapes the established project root ([`NativeFs`] only) /// /// Returns [`MdsError::ResourceLimit`] when the resolved path exceeds - /// [`MAX_PATH_SEGMENTS`] segments. + /// `MAX_PATH_SEGMENTS` segments. fn normalize_in_dir(&self, dir: &str, relative: &str) -> Result; /// Return the directory portion of a normalized file key. diff --git a/crates/mds-core/src/lint/config.rs b/crates/mds-core/src/lint/config.rs index 6acb6f4..3a42ce5 100644 --- a/crates/mds-core/src/lint/config.rs +++ b/crates/mds-core/src/lint/config.rs @@ -116,7 +116,7 @@ pub fn find_unknown_rule_names(rules: &HashMap) -> Option String { /// published to crates.io. However, deleting it would silently drop coverage /// of six ADR-004 reverify-gate behaviors that are pinned only through this /// function, with no equivalent on the `apply_fixes_incremental` path. These -/// tests must be ported or explicitly tracked before removal at v0.5.0. -/// The canonical list is kept in `.devflow/features/mds-lint/KNOWLEDGE.md` -/// under "v0.5.0 Removal Tracker: apply_fixes" so it survives this function's -/// deletion. A GitHub removal-tracker issue must be opened and cross-linked -/// before the v0.5.0 tag. +/// tests must be ported or retired before removal at v0.5.0. The enumerated +/// list (test names, line numbers, and the behavior each pins) lives in GitHub +/// issue #304 (v0.5.0 removal tracker for `apply_fixes`). /// /// # Behavior /// diff --git a/crates/mds-core/src/resolver.rs b/crates/mds-core/src/resolver.rs index ccae832..4db16ab 100644 --- a/crates/mds-core/src/resolver.rs +++ b/crates/mds-core/src/resolver.rs @@ -360,7 +360,7 @@ impl ModuleCache { self.resolve_intrinsic_by_key(&key, runtime_vars, warnings) } - /// Like [`resolve_path_intrinsic`] but accepts [`crate::CompileOptions`] and + /// Like [`Self::resolve_path_intrinsic`] but accepts [`crate::CompileOptions`] and /// returns `(CompiledOutput, Option)`. pub fn resolve_path_intrinsic_opts( &mut self, @@ -541,7 +541,7 @@ impl ModuleCache { self.resolve_intrinsic_by_key(entry, runtime_vars, warnings) } - /// Like [`resolve_virtual_intrinsic`] but accepts [`crate::CompileOptions`] and + /// Like [`Self::resolve_virtual_intrinsic`] but accepts [`crate::CompileOptions`] and /// returns `(CompiledOutput, Option)`. pub fn resolve_virtual_intrinsic_opts( &mut self, @@ -609,7 +609,7 @@ impl ModuleCache { /// Resolve a module from an in-memory source string, dispatching on output shape. /// - /// Like [`resolve_source`] but dispatches on `has_message_block`, returning a + /// Like [`Self::resolve_source`] but dispatches on `has_message_block`, returning a /// [`crate::CompiledOutput`] (Markdown or Messages) instead of a rendered string. pub fn resolve_source_intrinsic( &mut self, @@ -633,7 +633,7 @@ impl ModuleCache { Self::check_lifo_pop(result, popped, SOURCE_LABEL) } - /// Like [`resolve_source_intrinsic`] but accepts [`crate::CompileOptions`] and + /// Like [`Self::resolve_source_intrinsic`] but accepts [`crate::CompileOptions`] and /// returns `(CompiledOutput, Option)`. pub fn resolve_source_intrinsic_opts( &mut self, diff --git a/crates/mds-core/src/source_path.rs b/crates/mds-core/src/source_path.rs index 18fa3e1..aaa3a7a 100644 --- a/crates/mds-core/src/source_path.rs +++ b/crates/mds-core/src/source_path.rs @@ -27,7 +27,7 @@ use std::path::Path; /// /// # Parameters /// -/// - `source` — raw source key from [`crate::sourcemap::MapBuilder`] +/// - `source` — raw source key from `MapBuilder` /// (`sources[i]`), typically an absolute canonical path on native or a /// virtual key string on WASM. /// - `base` — directory that the source map file will be written to (the diff --git a/crates/mds-core/src/sourcemap.rs b/crates/mds-core/src/sourcemap.rs index ae214f5..959a4f0 100644 --- a/crates/mds-core/src/sourcemap.rs +++ b/crates/mds-core/src/sourcemap.rs @@ -67,9 +67,9 @@ impl std::fmt::Debug for Origin { /// Canonical `sources[]` label for in-memory (string-source) compilations. /// -/// All paths that produce a [`MapBuilder`] for string-source input converge -/// on [`MapBuilder::new`] or [`MapBuilder::source_index`]. Both choke-points -/// apply [`map_source_label`] so the diagnostic sentinel `""` can +/// All paths that produce a `MapBuilder` for string-source input converge +/// on `MapBuilder::new` or `MapBuilder::source_index`. Both choke-points +/// apply `map_source_label` so the diagnostic sentinel `""` can /// never appear in `sources[]`. /// /// All binding surfaces (WASM, napi, Python, CLI) that handle string-source @@ -173,9 +173,9 @@ impl SourceMap { /// Points whose `out_byte_offset` does not land on a UTF-8 char boundary /// are silently dropped (graceful degradation — never panics). /// - /// This function builds a [`LineTable`] over `body`, resolves each + /// This function builds a `LineTable` over `body`, resolves each /// byte offset to a `(line, utf16_col)` pair, then calls - /// [`encode_mappings`] on the resolved points. + /// `encode_mappings` on the resolved points. pub fn from_points( body: &str, sources: Vec, @@ -444,7 +444,7 @@ pub(crate) fn encode_mappings(mut points: Vec<(u32, u32, u32, u32, u32)>) -> Str pub struct CompileOptions { /// Generate a [`SourceMap`] and attach it to [`crate::CompileResult::source_map`]. /// - /// When `false` (the default) no [`MapBuilder`] is allocated — zero overhead + /// When `false` (the default) no `MapBuilder` is allocated — zero overhead /// for callers that do not need mapping data (AC-PERF-01). pub source_map: bool, /// Include source file contents in the `sourcesContent` array. From 6a55d553f24bcb674f9e049f32c9b86e4ac34eef Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Mon, 17 Aug 2026 00:50:22 +0300 Subject: [PATCH 06/28] docs(mds-lint): fix two KNOWLEDGE.md defects flagged by code review KNOWLEDGE.md:577 cited ADR-001 (span-guidance bypass) as the reason apply_plan_unchecked is named that way; the correct citation is ADR-004 (reverify-gate bypass), which is exactly the gate the _unchecked suffix is meant to make visible. KNOWLEDGE.md:593-600 carried a 'Line (v0.4.0 HEAD)' column that was off by +5 at HEAD (written against an intermediate commit), and the same six test locations were already tracked in GitHub issue #304 with a third, different numbering -- three non-agreeing representations of the same work set (PF-009). Fix: drop the line-number column from the table entirely. Test names are stable, greppable identifiers; issue #304 remains the single source of truth for file locations. Co-Authored-By: Claude --- .devflow/features/mds-lint/KNOWLEDGE.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.devflow/features/mds-lint/KNOWLEDGE.md b/.devflow/features/mds-lint/KNOWLEDGE.md index 51bbae4..e5dc0a5 100644 --- a/.devflow/features/mds-lint/KNOWLEDGE.md +++ b/.devflow/features/mds-lint/KNOWLEDGE.md @@ -574,7 +574,7 @@ LintDiagnostic.fix_removals (FixLineSpan) OR .fix_edits (TextEdit) - **PF-007** (cross-surface goldens can't catch divergence): `fix_edits` is emitted unconditionally (null when None) across all surfaces; differential tests cover cross-surface parity. - **PF-013** (vacuous negative security tests): Every ESC-injection test now pairs a NEGATIVE assertion (raw byte absent) with a POSITIVE one (escaped form present) and a non-vacuity guard (diagnostics non-empty, expected rule matched). T-9 was rewritten from a vacuous YAML-rejection vector to a reachable duplicate-import + U+0085 NEL vector. - **PF-014** (sanitize inputs, not rendered artifacts): The `SanitizedReport` design — pre-sanitize message/help/labels before miette renders — is the PF-014-correct boundary. Post-processing the rendered frame corrupts miette's own ANSI SGR codes; CI uses `NO_COLOR=1` and pipes stderr so the failure would stay green. T-ESC-6 pins this on the colour path. -- **ADR-001** (span-guided rewrite + compile-equivalence gate): All `--fix` edits are span-guided byte rewrites. `TextEdit` ranges are validated fail-closed. `apply_plan_unchecked` is explicitly named to make ADR-001 bypass visible. +- **ADR-001** (span-guided rewrite + compile-equivalence gate): All `--fix` edits are span-guided byte rewrites. `TextEdit` ranges are validated fail-closed. `apply_plan_unchecked` is explicitly named to make ADR-004 reverify-gate bypass visible. - **ADR-004** (three-tier --fix safety model, reverify gate): `apply_fixes_incremental`'s batch-first strategy with bounded per-edit fallback is the AC-F-20 implementation. - **ADR-002** (v0.4.0 whitespace contract, interior-verbatim): The `empty-block` rule's "whitespace-only-Text body" definition is directly downstream of this contract. - **ADR-003** (@extends FM emission): The `unused-variable` rule is suppressed on `@extends` children. @@ -590,14 +590,14 @@ The removal is tracked in GitHub issue #304. Before the v0.5.0 tag, the followin ADR-004 reverify-gate behaviors must gain equivalent coverage on the `apply_fixes_incremental` path or be explicitly retired in #304. -| Test name | Line (v0.4.0 HEAD) | Behavior pinned | -|---|---|---| -| `a4_partial_overlap_still_rejected_after_dedup` | fix.rs:1446 | A4: overlapping edits refused after dedup | -| `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` | fix.rs:1743 | A5: rejection message stability | -| `reverify_preexisting_untargeted_survives_and_fix_applies` | fix.rs:1943 | AC-F-23: pre-existing untargeted diagnostic survives | -| `reverify_new_untargeted_diagnostic_is_rejected` | fix.rs:1968 | reverify gate rejects new untargeted diagnostics | -| `tier_b_unused_function_standalone_apply_succeeds` | fix.rs:2039 | I-13: Tier B fix applies on standalone file | -| `l_fix_rev1_output_delta_causes_rejection` | fix.rs:2110 | L-FIX-REV1: output delta causes rejection | +| Test name | Behavior pinned | +|---|---| +| `a4_partial_overlap_still_rejected_after_dedup` | A4: overlapping edits refused after dedup | +| `l_fix_rev1_a5_rejection_message_pins_stable_prefix_and_suffix` | A5: rejection message stability | +| `reverify_preexisting_untargeted_survives_and_fix_applies` | AC-F-23: pre-existing untargeted diagnostic survives | +| `reverify_new_untargeted_diagnostic_is_rejected` | reverify gate rejects new untargeted diagnostics | +| `tier_b_unused_function_standalone_apply_succeeds` | I-13: Tier B fix applies on standalone file | +| `l_fix_rev1_output_delta_causes_rejection` | L-FIX-REV1: output delta causes rejection | Remove this table once all six behaviors are covered by `apply_fixes_incremental` tests (migration complete) or retired in #304. From 821ec23468124ad569e70b6b69ee7182435c8fa4 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Mon, 17 Aug 2026 00:50:41 +0300 Subject: [PATCH 07/28] docs(releasing): add pre-flight grep for deprecated since = attributes [#209] AC-209-16: bump-version.mjs rewrites manifests and CHANGELOG only, never .rs files. Two live since = "0.4.0" sites exist in crates/ (config.rs:296 and fix.rs:733) and must be verified manually before tagging. Add an explicit grep -rn 'since = ' crates/ --include='*.rs' step with a comment naming both sites so a releaser cannot miss the check. Co-Authored-By: Claude --- RELEASING.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASING.md b/RELEASING.md index 32ca105..eaa83b1 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -56,6 +56,11 @@ npm run build -w @mdscript/mds-wasm npm run build --workspaces --if-present npm test --workspaces --if-present node scripts/verify-versions.mjs +# Verify #[deprecated(since = ...)] attributes match the release version. +# bump-version.mjs rewrites manifests and CHANGELOG only -- never .rs files. +# Two live sites: crates/mds-core/src/lint/config.rs and crates/mds-core/src/lint/fix.rs. +# Every hit's quoted version string MUST equal X.Y.Z before you tag. +grep -rn 'since = ' crates/ --include='*.rs' # Source hygiene and pre-merge check gates node scripts/verify-no-control-bytes.mjs From 6255aed460bd7f42d6e8b8ef5c49ee9599ee31ba Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Mon, 17 Aug 2026 00:51:42 +0300 Subject: [PATCH 08/28] ci: add cargo doc gate to prevent intra-doc link rot (OD-209-G) Six files in mds-core (fs.rs, lint/config.rs, lint/diagnostic.rs, resolver.rs, source_path.rs, sourcemap.rs) had private-item intra-doc links downgraded to public-only targets as part of the PR5 deprecation wave. The rewrites are correct and pass locally (exit 0, zero warnings), but nothing in CI prevented the links from silently re-rotting on a future PR. Add a "Rustdoc (warnings are errors)" step to the rust job using RUSTDOCFLAGS="-D warnings" cargo doc -p mds-core --no-deps. The --no-deps flag keeps the check fast and scoped to first-party code. Resolves OD-209-G; satisfies AC-209-11. Verified locally: cargo doc -p mds-core --no-deps exits 0, zero warnings, 0.09s on warm cache. Co-Authored-By: Claude --- .github/workflows/ci.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f06ba8c..bd639a2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,7 @@ env: jobs: rust: - name: Rust — fmt, clippy, test + name: Rust — fmt, clippy, test, doc runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 @@ -38,6 +38,13 @@ jobs: run: cargo clippy --workspace --all-targets -- -D warnings - name: Test run: cargo test --workspace + # Intra-doc link gate (AC-209-11 / OD-209-G): broken intra-doc links in + # mds-core degrade silently to literal text without this check. Six files + # (fs.rs, lint/config.rs, lint/diagnostic.rs, resolver.rs, source_path.rs, + # sourcemap.rs) had private-item links downgraded as part of the PR5 wave; + # --no-deps keeps the check fast and scoped to first-party code. + - name: Rustdoc (warnings are errors) + run: RUSTDOCFLAGS="-D warnings" cargo doc -p mds-core --no-deps msrv: name: MSRV (Rust 1.88) From 49b4b6071463872d6fe5a9fc922173da61cd0582 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Mon, 17 Aug 2026 00:52:05 +0300 Subject: [PATCH 09/28] docs(mds-lint): soften absolute completeness claim in removal tracker (PF-015) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KNOWLEDGE.md:589 read 'the following six ADR-004 reverify-gate behaviors must gain equivalent coverage' -- a closed-set claim that gates a future deletion, with no positive control proving the set is exhaustive (ADR-009). Change 'the following six' → 'at least the following' to remove the PF-015 liability at zero cost. The listed rows remain; the table still serves as the migration checklist. GitHub issue #304 remains the single source of truth for exhaustiveness. Co-Authored-By: Claude --- .devflow/features/mds-lint/KNOWLEDGE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.devflow/features/mds-lint/KNOWLEDGE.md b/.devflow/features/mds-lint/KNOWLEDGE.md index e5dc0a5..a09ffca 100644 --- a/.devflow/features/mds-lint/KNOWLEDGE.md +++ b/.devflow/features/mds-lint/KNOWLEDGE.md @@ -586,7 +586,7 @@ LintDiagnostic.fix_removals (FixLineSpan) OR .fix_edits (TextEdit) ## v0.5.0 Removal Tracker: apply_fixes `mds::fix::apply_fixes` is deprecated as of v0.4.0 and scheduled for removal at v0.5.0. -The removal is tracked in GitHub issue #304. Before the v0.5.0 tag, the following six +The removal is tracked in GitHub issue #304. Before the v0.5.0 tag, at least the following ADR-004 reverify-gate behaviors must gain equivalent coverage on the `apply_fixes_incremental` path or be explicitly retired in #304. From f3518e2574dc0ad363322e61f8109e5f0c4d1228 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Mon, 17 Aug 2026 00:54:06 +0300 Subject: [PATCH 10/28] fix(mds-core): tighten api_surface.rs deprecation pins and mutation-control docs [#209] - Convert #[allow(deprecated)] at lint_types_exist and known_lint_rules_and_unknown_detection to #[expect(deprecated, reason="...")] so both LintConfig::from_rules pins fire unfulfilled_lint_expectations under -D warnings if the deprecation is ever removed (applies same live-assertion mechanism as F-API-3; fixes inconsistency where the file documented #[expect] as correct while still using #[allow] three lines away) - Update F-API-3 / AD-209-2 docstring: - Drop overstatement "behaviorally unchanged"; accurately describe the test as pinning the function signature and the empty-plan early-return path (plan.edits.is_empty(); reverify is never called) - Replace single-command mutation control description with the two-command form (applies ADR-009): cargo clippy --workspace --all-targets aborts after 10 fix.rs expectations before api_surface is compiled; the api_surface pin fires separately under -p mds-core --test api_surface Co-Authored-By: Claude --- crates/mds-core/tests/api_surface.rs | 32 ++++++++++++++++++---------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/crates/mds-core/tests/api_surface.rs b/crates/mds-core/tests/api_surface.rs index 9b7e976..c64ec43 100644 --- a/crates/mds-core/tests/api_surface.rs +++ b/crates/mds-core/tests/api_surface.rs @@ -1086,7 +1086,10 @@ fn lint_api_signatures_exist() { /// L-API-2: pub types LintDiagnostic, Severity, LintConfig, LintResult are accessible /// and have the expected fields/variants. #[test] -#[allow(deprecated)] // exercises LintConfig::from_rules for surface coverage +#[expect( + deprecated, + reason = "L-API-2: pins LintConfig::from_rules deprecated constructor for surface coverage; fires unfulfilled_lint_expectations if the deprecation is removed" +)] fn lint_types_exist() { // Severity has four variants with lowercase serde names. let _off = Severity::Off; @@ -1142,7 +1145,10 @@ fn lint_types_exist() { /// - `UnknownRuleNames` exposes names via accessor, not a public field, and is /// only constructible through the library. #[test] -#[allow(deprecated)] // exercises LintConfig::from_rules for surface coverage +#[expect( + deprecated, + reason = "AC-224-7: pins LintConfig::from_rules deprecated constructor for surface coverage; fires unfulfilled_lint_expectations if the deprecation is removed" +)] fn known_lint_rules_and_unknown_detection() { use mds::{find_unknown_rule_names, KNOWN_LINT_RULES}; @@ -1649,18 +1655,22 @@ fn fix_api_incremental_exists() { ); } -/// F-API-3: `apply_fixes` remains reachable on the public API surface and behaviorally -/// unchanged while deprecated. Remove at v0.5.0 with the function (AD-209-1). +/// F-API-3: `apply_fixes` remains reachable on the public API surface while deprecated. +/// This test pins the function signature and the empty-plan early-return path (the +/// reverify closure is never invoked when `plan.edits.is_empty()`). Remove at v0.5.0 +/// with the function (AD-209-1). /// /// AD-209-2: `#[expect(deprecated)]` was chosen over a `trybuild` compile-fail fixture /// because: (a) trybuild only asserts that the deprecation warning fires; it does not -/// verify the function's signature or return value; (b) this test also asserts the -/// runtime behavior (NothingToFix for an empty plan), giving a stronger pin; and -/// (c) `#[expect(deprecated)]` fires `unfulfilled_lint_expectations` under -/// `cargo clippy --workspace --all-targets -- -D warnings` whenever the -/// `#[deprecated]` attribute is removed from `apply_fixes`, providing the same -/// "must not compile cleanly without the attribute" guarantee as trybuild without -/// a separate test-driver crate. +/// verify the function's signature or return value; (b) this test asserts the runtime +/// behavior (NothingToFix for an empty plan -- the `plan.edits.is_empty()` early-return), +/// giving a stronger pin than a compile-fail fixture alone; and +/// (c) `#[expect(deprecated)]` fires `unfulfilled_lint_expectations` when the +/// `#[deprecated]` attribute is removed from `apply_fixes`. The mutation control +/// requires two commands (applies ADR-009): `cargo clippy --workspace --all-targets +/// -- -D warnings` reports 10 expectations (all in fix.rs) and aborts before the +/// api_surface integration-test target is compiled; `cargo clippy -p mds-core +/// --test api_surface -- -D warnings` reports this pin individually. /// /// All values constructed via named constructors, never struct literals (applies ADR-010). #[expect( From 06e14e800a93263a3f815517d8d27d589b5c5c2d Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Mon, 17 Aug 2026 00:54:33 +0300 Subject: [PATCH 11/28] docs(changelog): fix deprecated section completeness and placement [#209] Three reviewer findings addressed: - [high] Add LintConfig::from_rules to the ### Deprecated section; it carries #[deprecated(since = "0.4.0")] in config.rs but was omitted from the section PR5 created, making the deprecation list incomplete the moment that section heading existed. - [high] Correct the migration instruction at the Nine-public-types BREAKING block: LintConfig::from_rules(rules) -> from_rules_checked, which was still directing users toward the deprecated constructor. - [medium] Move ### Deprecated up to the top-level [Unreleased] run (after ### Added at line 31, before ### **BREAKING**); the previous position ~1130 lines in meant users skimming release notes top-down would hit ### Fixed at line 10 and never reach the only entry that requires a code change. Issue #304 body (PR reference correction: #205 -> #303) is handled in the immediately following commit. Co-Authored-By: Claude --- CHANGELOG.md | 57 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c5fdf..6ad202e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,38 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 only re-exported from `index.ts`, which the package `exports` map does not resolve — so consumers could receive the value but not name its type. Purely additive. +### Deprecated + +- **`mds::fix::apply_fixes` is deprecated in favor of `apply_fixes_incremental` (#209).** + The replacement applies the same ADR-004 three-tier reverify-gate safety contract with a + batch-first attempt plus a bounded per-edit fallback, salvaging the safe subset of fixes + when some edits fail the reverify gate rather than refusing the whole batch. + + Two behavioral differences require manual migration: + + - **Closure bound**: `apply_fixes` takes `F: FnOnce`; `apply_fixes_incremental` requires + `F: Fn` because the reverify closure may be called more than once. A move-once closure + cannot be migrated mechanically. + - **New reachable outcome**: `apply_fixes_incremental` can return + `FixOutcome::PartiallyFixed` when some edits are accepted and some are refused. + `apply_fixes` never returns `PartiallyFixed`. Because `FixOutcome` is + `#[non_exhaustive]`, existing wildcard arms compile unchanged, but a wildcard that + swallows `PartiallyFixed` silently discards partial results. + + Scheduled for removal in v0.5.0; tracked in GitHub issue #304. The six ADR-004 + regression tests pinned only against `apply_fixes` must be ported or retired before + the v0.5.0 tag (see #304 for the enumerated list with line numbers and the behavior + each test pins). + +- **`mds::LintConfig::from_rules` is deprecated in favor of `LintConfig::from_rules_checked` (#224).** + The replacement returns both the config and an unknowns report in a single `#[must_use]` + call, making it structurally impossible to silently skip unknown-rule detection. + `from_rules` still accepts any rule name without error; unknown names have no effect. + + Migration: change `LintConfig::from_rules(map)` to `LintConfig::from_rules_checked(map)` + and handle the `Option` second return value. No removal is scheduled + before a major version bump. + ### **BREAKING** — File-method `basePath` rejection, TypeScript option types, and WASM `basePath` rejection (#180, #213) #### `compileFile` and `checkFile` now reject `basePath` (#180) @@ -194,7 +226,7 @@ via struct literals. Use the named constructor or builder listed for each: - **`RejectedEdit`** — use `RejectedEdit::new(edit, reason)`. - **`FixPlan`** — use `FixPlan::default()` for an empty plan; its fields are `pub`, so they remain directly readable and writable from external crates. -- **`LintConfig`** — use `LintConfig::from_rules(rules)` or `LintConfig::default()` for no overrides. +- **`LintConfig`** — use `LintConfig::from_rules_checked(rules)` or `LintConfig::default()` for no overrides. - **`LintDiagnostic::sanitized_for_render()`** — a new method that returns a sanitized clone suitable for miette render boundaries. `mds-cli`'s diagnostic render path now delegates to this method instead of assembling sanitized copies itself, keeping the escape logic co-located @@ -1136,29 +1168,6 @@ diagnostic messages must update to check for the `\uXXXX` literal form instead. for awareness — it clears as a side effect of applying other fixes (e.g. removing a duplicate import that was also the unused one). -### Deprecated - -- **`mds::fix::apply_fixes` is deprecated in favor of `apply_fixes_incremental` (#209).** - The replacement applies the same ADR-004 three-tier reverify-gate safety contract with a - batch-first attempt plus a bounded per-edit fallback, salvaging the safe subset of fixes - when some edits fail the reverify gate rather than refusing the whole batch. - - Two behavioral differences require manual migration: - - - **Closure bound**: `apply_fixes` takes `F: FnOnce`; `apply_fixes_incremental` requires - `F: Fn` because the reverify closure may be called more than once. A move-once closure - cannot be migrated mechanically. - - **New reachable outcome**: `apply_fixes_incremental` can return - `FixOutcome::PartiallyFixed` when some edits are accepted and some are refused. - `apply_fixes` never returns `PartiallyFixed`. Because `FixOutcome` is - `#[non_exhaustive]`, existing wildcard arms compile unchanged, but a wildcard that - swallows `PartiallyFixed` silently discards partial results. - - Scheduled for removal in v0.5.0; tracked in GitHub issue #304. The six ADR-004 - regression tests pinned only against `apply_fixes` must be ported or retired before - the v0.5.0 tag (see #304 for the enumerated list with line numbers and the behavior - each test pins). - ### Fixed - **`mds build -o build/out.md` with sources in `src/` again emits map-relative From adc3dcdc67ec15174698988a1ddb6463f3c0127d Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Mon, 17 Aug 2026 00:55:17 +0300 Subject: [PATCH 12/28] fix(mds-core): match deprecated attribute order and wrap note in fix.rs Match the ordering convention established by the repo's only other `#[deprecated]` (lint/config.rs:295-300): put `#[deprecated(...)]` before `#[must_use]`, and wrap the note literal at ~80 columns using a Rust `\` line continuation so the source width matches the style of the adjacent site. The rendered string is byte-identical to the original. Also corrects the PR reference in GitHub issue #304 (v0.5.0 removal tracker): the body cited `PR #205` (a live unrelated open issue) as the deprecation PR; the correct PR is #303. Both occurrences in the issue body are now updated via gh issue edit. Findings 1/3/4 from the code-review batch (RELEASING.md missing the `since =` pre-flight grep) were already addressed in a prior commit (lines 59-63 of RELEASING.md cover both live sites with an explicit instruction and the grep command). Co-Authored-By: Claude --- crates/mds-core/src/lint/fix.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/mds-core/src/lint/fix.rs b/crates/mds-core/src/lint/fix.rs index cebbb21..b314c7a 100644 --- a/crates/mds-core/src/lint/fix.rs +++ b/crates/mds-core/src/lint/fix.rs @@ -728,11 +728,12 @@ pub fn apply_plan_unchecked(source: &str, plan: &FixPlan) -> String { /// did (i.e. the edit introduced a new, non-fixed problem). /// /// Returns `FixOutcome::Fixed`, `FixOutcome::Rejected`, or `FixOutcome::NothingToFix`. -#[must_use = "a dropped FixOutcome silently discards the fix result"] #[deprecated( since = "0.4.0", - note = "use `apply_fixes_incremental`; not a drop-in swap, the reverify closure must be `Fn`, not `FnOnce`. Removed in v0.5.0; see the item docs." + note = "use `apply_fixes_incremental`; not a drop-in swap, the reverify closure must \ + be `Fn`, not `FnOnce`. Removed in v0.5.0; see the item docs." )] +#[must_use = "a dropped FixOutcome silently discards the fix result"] pub fn apply_fixes(source: &str, plan: FixPlan, original: &LintResult, reverify: F) -> FixOutcome where F: FnOnce(&str) -> Result, From f127797c4247fa2ff7679bdf5d6c31b5bcb914b3 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Mon, 17 Aug 2026 00:56:55 +0300 Subject: [PATCH 13/28] docs(plan): amend AC-209-04 to whitelist pre-existing config.rs:287,289 [#209] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AC-209-04 contained an absolute prohibition on `allow(deprecated)` in crates/*/src/ that was already violated on the wave base: config.rs lines 287 and 289 carry #[allow(deprecated)] inside the compiled doctest for LintConfig::from_rules (itself deprecated since #302, before this branch). The audit grep (`allow(deprecated)|expect(deprecated)` over crates/*/src/) therefore returns two hits the original criterion did not account for. A reviewer running the audit could not distinguish a genuine suppression leak from a legacy carve-out. Fix (applies PF-015 — absolute completeness claim is a liability): - Restate AC-209-04 with an enumerated whitelist of exactly three permitted locations: fix.rs >1006, crates/mds-core/tests/, and config.rs:287,289. - Add a provenance note: (c) is pre-existing from #302, confirmed via `git show wave/v0.4.0-wave1:crates/mds-core/src/lint/config.rs`. - Update the test plan section 4 expected outcome to match, including the confirmation command reviewers should run to verify the carve-out. No source code changed. Only the plan document is updated. Co-Authored-By: Claude --- .../pr5-deprecate-apply-fixes-plan.md | 424 ++++++++++++++++++ 1 file changed, 424 insertions(+) create mode 100644 .devflow/docs/design/v040-wave1/20260812_0046/pr5-deprecate-apply-fixes-plan.md 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..9b2c98f --- /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 the literal string `allow(deprecated)` must not appear 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. + +--- + +## 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`** before `### Fixed` at 764. 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`, confirm **exactly 11** `unfulfilled_lint_expectations` (10 fix.rs + 1 api_surface.rs), then restore and confirm clean. 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** — plant `// allow(deprecated)` in `crates/mds-core/src/lexer.rs`, confirm `grep -rn 'allow(deprecated)\|expect(deprecated)' crates/ --include='*.rs'` finds it, remove it, re-run. Clean run must hit only fix.rs >1006 and `tests/api_surface.rs`. Read every hit; a bare count is not evidence. (BSD grep has no `-P` — basic alternation only.) +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