From 23701954e7efaa974c7761403a115227dffe5b93 Mon Sep 17 00:00:00 2001 From: Copilot Date: Tue, 15 Sep 2026 15:11:49 +0200 Subject: [PATCH 1/6] Plan TDD fix for record field-order diagnostics (#20410) Create one self-contained implementation sprint with RED/GREEN coverage, raw diagnostic assertions, local validation, review, and commit-only delivery. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d19ba29f-8295-468b-af28-e0590fcd57bd --- .tools/ralph/BACKLOG.md | 113 ++++++ .../01_Record_Field_Order_Diagnostics.md | 355 ++++++++++++++++++ 2 files changed, 468 insertions(+) create mode 100644 .tools/ralph/BACKLOG.md create mode 100644 .tools/ralph/sprints/01_Record_Field_Order_Diagnostics.md diff --git a/.tools/ralph/BACKLOG.md b/.tools/ralph/BACKLOG.md new file mode 100644 index 00000000000..e2ffdb2da64 --- /dev/null +++ b/.tools/ralph/BACKLOG.md @@ -0,0 +1,113 @@ +# BACKLOG + +## Original Request + +Process issue https://github.com/dotnet/fsharp/issues/20410 using TDD. + +Use minimal, surgical changes. Validate locally before finishing. Do not push. Only commit. + +### ISSUE REQUEST +The requirements above override conflicting instructions in the issue request. +Fix issue https://github.com/dotnet/fsharp/issues/20410. + +Make the smallest clean, correct, complete fix. Keep it minimal and surgical. Quality matters more than time. Take all the time needed. Smaller is better, but not at the cost of correctness. Preserve progress and evidence across execution windows rather than truncate the work. + +**Verified root cause.** At main `b5c530ed6bc42937de6363e3dcc104ebb833893d`, `checkRecordFields` validates fields by name in both directions, then calls diagnostic-emitting `checkField` again by position. The first reordered pair produces misleading FS0193 before the correct order diagnostic. The actual order diagnostic is **FS0312**, not the issue's FS0313. FS0313 means a required field is missing. Do not change diagnostic numbers. Eighteen current-main matrix compilations give seven expected RED assertions and eleven passing controls. A separate exact original `ResolvedConfig` compilation confirms only FS0193 at the first displaced field and FS0312 on the type. + +**Surgical candidate.** Change only the final record positional predicate in `src/Compiler/Checking/SignatureConformance.fs:640-642`: compare `LogicalName` before calling the existing `checkField`. If names differ, return false without that call. Preserve the existing FS0312 at the implementation type and the false conformance result. Keep the two preceding name-map passes at `631-636` unchanged. + +This guard avoids the misleading diagnostic and the wrong cross-field documentation/range mutation. Keep matching-name calls unchanged. Replacing the whole predicate with pure name equality would also remove existing repeated nullness warnings on correctly ordered records, which is outside this issue. The candidate is not yet applied or proven GREEN. + +Do not sort fields, accept reordered records, suppress shared FS0193, alter diagnostic resources, or broaden the guard to unions, exceptions, or classes. Same-name type, mutability, accessibility, and attribute checks must still run. Constructor parameter order depends on record declaration order. + +**RED-first test plan.** Use `tests/FSharp.Compiler.ComponentTests/Conformance/Signatures/Signatures.fs` and the paired-source compile pipeline. A plain `typecheck` call does not consume `AdditionalSources` at this revision. Use the existing `Fsi` plus implementation-source helper and `compile`, or a demonstrated project-typecheck path. Assert the complete diagnostic list, including code, severity, message, and range. GREEN means correct diagnostics while the invalid permutation still fails compilation. + +| Scenario | Required assertion | +|---|---| +| 1. Exact reported `ResolvedConfig` field permutation, with compact local definitions for its dependent types | Exactly FS0312 on `ResolvedConfig`, no positional FS0193. Also use the reduced two-field swap to isolate the cause. | +| 2. A shared correctly ordered prefix followed by a three-field cycle | Exactly FS0312 on the type, no positional FS0193. | +| 3. Swapped fields with the same `int` type | Exactly FS0312. Equal field types do not make declaration order legal. | +| 4. Generic struct record with `'T` and `'T list` fields swapped | Exactly FS0312. Preserve generic remapping and the representation constraint. | +| 5. Permutation plus conflicting attribute arguments on the same named field | Keep FS1200 from attribute reconciliation and FS0312. Remove only positional FS0193. | +| 6. Identical names, declarations, and order | Successful compilation without diagnostics. | +| 7. Real same-name mismatch | Parameterize changed type, changed mutability, and less-accessible representation. Keep genuine FS0193 identifying the same field, without a spurious order error. | +| 8. Different name sets | Missing, extra, and renamed fields keep FS0313, FS0311, and FS0313 respectively. Equal counts alone do not establish a permutation. | +| 9. Warning-only nullness differences | Keep existing same-name warnings. The observed aligned and prefix-before-permutation cases each have three FS3261 warnings. The prefix case adds FS0312 but loses positional FS0193. Do not make warning cleanup part of this change. | +| 10. Non-record and recovery boundaries | Reuse union, exception, object-field, and malformed-field controls. Union and exception diagnostics stay unchanged. Reordered class fields can still compile. Do not refactor recovered duplicate names or list lengths. | + +Rows 1-5 provide the primary bug and four meaningful RED variants. Current-main reduced forms already fail the intended expectations. Preserve the exact issue case as a compact fixture too. Rows 6-10 are controls, not additional before-fix failure claims. Use one data-driven regression test and separately named controls. Share source-pair construction; do not copy many near-identical files or test bodies. + +First prove RED from the unwanted diagnostic. Then make the local guard and obtain GREEN without weakening tests, changing order-error expectations, suppressing warnings, or refreshing unrelated baselines. Existing field-extended-data, signature-nullness, and attribute-matching sibling selections passed seven rows on the starting main build. Run these and the nearby signature-conformance tests after implementation. + +Format only changed F# files. Invoke `fsharp-diagnostics` after compiler edits and invoke the expert-review skill on the final work. Remove noise and duplicate setup using existing helpers. Leave a clean, compact suite and concise release note. No API change, new diagnostic, extra name set, general suppression flag, or broad conformance refactor is needed. + +Sources: [issue](https://github.com/dotnet/fsharp/issues/20410), [record-specific checks](https://github.com/dotnet/fsharp/blob/b5c530ed6bc42937de6363e3dcc104ebb833893d/src/Compiler/Checking/SignatureConformance.fs#L624-L655), [field checks and side effects](https://github.com/dotnet/fsharp/blob/b5c530ed6bc42937de6363e3dcc104ebb833893d/src/Compiler/Checking/SignatureConformance.fs#L559-L623), [diagnostic identities](https://github.com/dotnet/fsharp/blob/b5c530ed6bc42937de6363e3dcc104ebb833893d/src/Compiler/FSComp.txt#L149-L151). + +## Analysis + +This delivery is architecture only. The implementer receives one sprint with the fix, tests, validation, review, release note, and commit requirements. +Do not implement the compiler change while preparing these files. + +### Verified during planning + +- Repository: `Q:\fsharp-worktrees\issue-878`, branch `fix/issue-20410`, initially clean. +- HEAD: `b5c530ed6bc42937de6363e3dcc104ebb833893d`. The directory name does not identify the target issue. +- Read `Q:\groundhog-while-not-works\templates\SPRINT_TEMPLATE.md` before creating the sprint. +- Retrieved issue #20410 through `gh issue view`. Its FS0313 label is incorrect. +- Read `checkField`, `checkRecordFields`, `checkRecordFieldsForExn`, `checkClassFields`, and `checkAttribs`. +- The two record name-map passes precede the positional call. The positional call can overwrite another field's documentation and range. +- `FSComp.txt` assigns 311 to an extra field, 312 to field order, and 313 to a missing required field. +- `Signatures.fs` already uses `Fsi |> withAdditionalSourceFile (FsSource ...) |> compile`. +- `Compiler.fs` confirms that plain `typecheck` reads only the primary source. `compile` consumes both sources. +- `Compiler.fs` also shows that `withDiagnostics` deduplicates by range and message. It cannot verify repeated warning counts alone. +- Raw `CompilationResult.Output.Diagnostics` retains duplicates. New tests need raw, complete diagnostic assertions, especially for FS3261. +- `checkAttribs` emits FS1200 as a warning unless options promote it. Its fixup replaces implementation attributes with signature attributes. +- The harness treats warnings as failure by default. That wrapper result alone does not prove a warning-only program has a compiler error. +- Found existing `FieldNotContainedDiagnosticExtendedData 01`, `Signature conformance`, `Micro compilation`, and `AttributeMatching01` sibling tests. +- GitHub repository variable `VNEXT` is `11.0.100`. The corresponding compiler-service release-note file exists. +- `dotnet --version` could not resolve the pinned SDK, `11.0.100-rc.1.26420.103`. No compiler build or runtime matrix ran during planning. +- `eng\common\dotnet.ps1` can install and invoke the repository SDK. SDK setup belongs to execution, not this documentation-only delivery. +- `.tools` is ignored. Commit only the two requested plan files with explicit `git add -f` paths. Do not change `.gitignore`. +- Native PowerShell validation passed: one sprint, 19 completion criteria, 17 existing source references, required scenario coverage, and balanced code fences. +- That validation also confirmed ASCII text, no trailing whitespace, and no tracked source or staged changes before staging the planning files. + +The eighteen matrix compilations, seven RED assertions, eleven controls, and separate original fixture are evidence supplied by the request. +No previous-session evidence was found in the bounded history lookup. +Do not present those results as new local executions or infer an exact new test count from them. + +### Main risks + +Pure name equality would remove matching-name checks and repeated nullness warnings. +A shared `checkField` change would affect unions, exceptions, classes, and real field mismatches. +A test using plain `typecheck`, error-code presence, or deduplicated warnings could produce false confidence. +A test run using stale compiler binaries or discovering no selected tests cannot establish RED or GREEN. +Formatting an entire large compiler file can create unrelated changes even when the functional fix is tiny. + +## Approach + +Use one complete vertical sprint. Splitting tests, the guard, and controls would make early sprints intentionally incomplete. +The sprint embeds the original record permutation, minimal fixtures, assertion requirements, source helpers, commands, and restrictions. +Its RED phase captures exact diagnostics before editing production code. +Its GREEN phase changes only the final record predicate and reruns the unchanged tests plus siblings. +The final implementation commit contains the compiler change, compact tests, and one release note. +Keep execution logs under `.tools\ralph\evidence\issue-20410`, outside the implementation commit. +Never push, open a PR, post a review, or fabricate a PR URL. + +### Final verification checklist + +- The sprint starts with two `---` lines and contains all four required headings. +- Every Definition of Done item starts with `- `, without a checkbox. +- Every requested scenario is covered within the same sprint as its implementation. +- The sprint stands alone without this backlog, another sprint, or historical logs. +- The exact request above is preserved, including its distinction between FS0312 and FS0313. +- Local source references, project paths, and release-note path exist. +- The plan explicitly preserves duplicate warnings and complete diagnostic tuples. +- Planning validation checks file structure, paths, coverage, and `git diff --check`. It does not claim compiler GREEN. +- The planning commit contains only `BACKLOG.md` and `01_Record_Field_Order_Diagnostics.md`. +- The implementation verifier later requires actual RED/GREEN logs, passing sibling selections, review resolution, and a local implementation commit. + +## Sprint Overview + +| # | Name | Purpose | +|---|---|---| +| 01 | Record Field Order Diagnostics | Prove RED, guard the record-only positional check, prove GREEN with all controls, review, document, and commit locally. | diff --git a/.tools/ralph/sprints/01_Record_Field_Order_Diagnostics.md b/.tools/ralph/sprints/01_Record_Field_Order_Diagnostics.md new file mode 100644 index 00000000000..5ba37354109 --- /dev/null +++ b/.tools/ralph/sprints/01_Record_Field_Order_Diagnostics.md @@ -0,0 +1,355 @@ +--- +--- +# Sprint: Correct record field-order diagnostics with RED-first coverage + +## Context - WHY this sprint exists + +Fix [dotnet/fsharp issue #20410](https://github.com/dotnet/fsharp/issues/20410) in `Q:\fsharp-worktrees\issue-878`. +This sprint is the complete implementation unit. It has no dependency on another sprint or on `BACKLOG.md`. +Use minimal, surgical changes. Validate locally, then commit. Do not push or publish anything. + +The starting compiler commit is `b5c530ed6bc42937de6363e3dcc104ebb833893d`, on branch `fix/issue-20410`. +Planning documents can be committed above that source revision. Inspect the actual worktree before editing. +Preserve other people's changes and all useful execution evidence. + +`checkRecordFields` in `src\Compiler\Checking\SignatureConformance.fs` first compares record fields by name in both directions. +It then calls the diagnostic-emitting `checkField` positionally through `List.forall2`. +The first displaced field produces misleading FS0193 before the correct FS0312 on the implementation type. +`checkField` also copies XML documentation and updates paired source ranges before comparing names. +The positional call can therefore overwrite the correct same-name association. + +The issue incorrectly calls the order diagnostic FS0313. The existing order diagnostic is **FS0312**. +FS0313 means a required field is missing. FS0311 means an implementation field is absent from the signature. +Do not change these numbers or their messages. +Record constructor parameter order depends on declaration order. A permutation must still fail compilation. + +The request reports eighteen starting-main matrix compilations: seven expected RED assertions and eleven passing controls. +It also reports a separate original `ResolvedConfig` compilation with only positional FS0193 and type-level FS0312. +Those historical logs are not prerequisites. Capture fresh RED and GREEN evidence for the tests in this sprint. +The guard was not applied or proven GREEN during planning. + +## Description - WHAT to implement with DETAILED guidance + +### Files and repository rules + +| Path, relative to the worktree | Purpose | +|---|---| +| `src\Compiler\Checking\SignatureConformance.fs` | Change only the final positional predicate in `checkRecordFields`, initially around lines 640-642. | +| `tests\FSharp.Compiler.ComponentTests\Conformance\Signatures\Signatures.fs` | Add one data-driven regression and separately named controls in `Conformance.Signatures.SignatureConformance`. | +| `docs\release-notes\.FSharp.Compiler.Service\11.0.100.md` | Add one concise `Fixed` entry after successful implementation. `VNEXT` was `11.0.100` during planning. | +| `tests\FSharp.Test.Utilities\Compiler.fs` | Read existing source-pair, compilation, and diagnostic helpers. Do not change this shared harness. | +| `src\Compiler\FSComp.txt` | Read diagnostic identities around lines 149-151. Do not edit diagnostic resources. | +| `.tools\ralph\evidence\issue-20410` | Preserve commands, source revision, fixtures, exit codes, selected test counts, and RED/GREEN logs. Do not commit generated logs. | + +Read these instruction files before editing: + +- `.github\instructions\ExpertReview.instructions.md` +- `.github\instructions\ComponentTests.instructions.md` +- `.github\instructions\NoBloat.instructions.md` + +Read `docs\coding-standards.md` before interpreting compiler abbreviations. +Use existing helpers and F# formatting. Add no API, project, package, diagnostic, feature flag, or new name map. +The existing test file is already included in `tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj`. + +### 1. Establish a usable local baseline + +Run commands from `Q:\fsharp-worktrees\issue-878` in PowerShell. +Set `$env:BUILDING_USING_DOTNET = 'true'` in each fresh command process. +Do not change system-wide environment variables on a shared machine. + +The planning probe `dotnet --version` failed because the pinned SDK was unavailable. +`global.json` requests SDK `11.0.100-rc.1.26420.103` and `Microsoft.Testing.Platform`. +After confirming the missing SDK, use the repository bootstrap: + +```powershell +.\eng\common\dotnet.ps1 --version +``` + +This script installs the repository SDK and invokes it. +Use that SDK for all subsequent commands. If necessary, invoke each command through `.\eng\common\dotnet.ps1`. +Do not change `global.json`, dependency versions, or `eng\common` files to obtain a build. + +Query target frameworks rather than assuming `net472` or another framework: + +```powershell +$env:BUILDING_USING_DOTNET = 'true' +dotnet msbuild tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -getProperty:TargetFrameworks +dotnet msbuild src\Compiler\FSharp.Compiler.Service.fsproj -getProperty:TargetFrameworks +``` + +Confirm the test-runner syntax with the installed SDK's help. +The command patterns below follow the component-test instructions. +If the runner requires a different placement of `--filter-method`, adjust only command syntax. +Confirm nonzero discovery and the intended test names. A zero-test success is not validation. + +### 2. Write complete RED-first paired-source tests + +Follow `Issue 11331 - Public constructor taking internal type should report FS0410 in signature` in `Signatures.fs`. +Share one small source-pair constructor, for example: + +```fsharp +let private recordSignaturePair signature implementation = + Fsi signature + |> withAdditionalSourceFile (FsSource implementation) + |> asLibrary +``` + +Include the same explicit module name in both source strings. +Keep default `.fsi` and `.fs` names paired. Apply options before `compile`. +Use `compile` to consume both sources. +Do not use plain `typecheck`: at this revision, it ignores `AdditionalSources`. +`typecheckProject` handles multiple sources, but changing to that result/assertion model is unnecessary here. + +Use one `[]` with data rows for the six primary cases: exact original, reduced swap, prefix cycle, equal types, generic struct, and attributes. +Use separately named controls for aligned records, real mismatches, different name sets, nullness, and non-record/recovery behavior. +Parameterize related controls rather than copying test bodies. +Include `Issue 20410` in new method names so one filter selects all new coverage. +Use compact inline fixtures and shared declarations. Do not add a separate source file for every row. + +**Assert the entire ordered diagnostic list, including duplicates, severity, number, message, and range.** +`withErrorCode` and substring checks are insufficient. +`withDiagnostics` in `Compiler.fs` calls `assertErrors`, which deduplicates actual diagnostics by range and message. +It also normalizes whitespace. It cannot establish exact duplicate-warning preservation alone. + +Use the existing raw result surface: `result.Output.Diagnostics`. +Project each diagnostic to a stable tuple and compare the complete expected and actual lists. +Include the source basename to distinguish implementation and signature ranges. +For example, project `d.Error`, `Path.GetFileName d.NativeRange.FileName`, `d.Range` coordinates, and `d.Message`. +Raw columns are zero-based. DSL `Col` expectations are one-based. +Choose one convention explicitly and do not mix them. +Normalize CRLF to LF if needed, but do not normalize message wording, filter diagnostics, sort, or deduplicate. +A small local exact-assertion helper is justified. Do not modify the shared assertion framework. +Existing raw-result usage is in `tests\FSharp.Compiler.ComponentTests\Attributes\CompiledNameMultipleValues.fs`. + +Capture current diagnostics and source line layouts before setting control expectations. +Primary regression expectations must exclude only positional FS0193, not genuine diagnostics. +Freeze those expectations before editing production code. +For every invalid permutation, also assert compilation failure and the presence of an actual `Error 312`. + +The harness treats warnings as failure by default, even without compiler `--warnaserror`. +For warning-only controls, assert the complete warning list and absence of errors. +Do not mistake the harness failure wrapper for a language error. +Do not use warning suppression, promotion, or `ignoreWarnings` to simplify the new expectations. + +#### Required fixture and behavior matrix + +| Case | Source guidance | Complete required outcome | +|---|---|---| +| Exact reported record | Preserve the `ResolvedConfig` declarations below. Define all three dependent types locally and identically in both sources. | Exactly one `Error 312` on the implementation `ResolvedConfig` identifier. No positional FS0193. | +| Reduced swap | Signature `type R = { A: int; B: string }`, implementation `type R = { B: string; A: int }`. | Exactly one `Error 312` on implementation `R`. | +| Shared prefix, three-field cycle | Signature `{ Prefix: bool; A: int; B: string; C: decimal }`, implementation `{ Prefix: bool; B: string; C: decimal; A: int }`. | Exactly one `Error 312`, with no error on the displaced field. | +| Equal-type swap | Use `{ A: int; B: int }` and `{ B: int; A: int }`. | Exactly one `Error 312`. Equal types do not make order legal. | +| Generic struct swap | Use `[] type R<'T> = { A: 'T; B: 'T list }` and its reversed implementation. Put `[]` in both sources. | Exactly one `Error 312`. Preserve generic remapping and struct representation checking. | +| Attribute conflict plus swap | Put `[]` on named field `A` in the signature and `"impl"` on `A` in the implementation. Reverse `A` and `B`. | Preserve FS1200 at the attribute and FS0312 at the type. Remove only positional FS0193. Capture actual attribute-target multiplicity. FS1200 is a warning without promotion. | +| Aligned declarations | Compile the same record declaration on both sides. | Successful compilation and an empty raw diagnostic list. | +| Real same-name mismatch | Parameterize `A: int` versus `A: string`, immutable versus mutable `A`, and public versus `internal` record representation. Keep field names and order aligned. | Preserve the genuine field FS0193 and its exact message/range. No spurious FS0312. | +| Different name sets | Signature has `A; B`. For missing, implementation has only `A`. For extra, implementation adds `C`. For renamed, implementation has `A; C`. | Preserve complete lists with FS0313, FS0311, and FS0313 respectively. Do not convert these into order errors. | +| Nullness, aligned and prefix swap | Signature `{ Prefix: string; A: int; B: bool }`. Implementation changes `Prefix` to `string \| null`, then either keeps or swaps `A; B`. | Each variant retains three FS3261 warnings. Aligned has no errors. Prefix-swap adds only FS0312 after the fix. Count raw warnings, including identical duplicates. | +| Union, exception, object fields, recovery | Preserve starting-main controls described below. | Union and exception diagnostics remain unchanged. Reordered class fields can still compile. Malformed fields retain their diagnostics without a compiler crash. | + +For nullness cases, use `withLangVersionPreview`, `withCheckNulls`, and `withWarnOn 3261`. +Do not copy the warning-promotion helper from the nullness test module. +FS1200 is emitted by `checkAttribs`, which also reconciles attributes during name-based checks. +Do not assume repeated attribute checks produce repeated FS1200 warnings. Assert the measured complete list. + +The exact reported field permutation is: + +```fsharp +// Signature declaration +type ResolvedConfig = + { + Config: FormatConfig + Settings: ResolvedSetting list + EditorConfigFiles: string list + Problems: EditorConfigProblem list + } + +// Implementation declaration, in the separate implementation source +type ResolvedConfig = + { + Config: FormatConfig + EditorConfigFiles: string list + Problems: EditorConfigProblem list + Settings: ResolvedSetting list + } +``` + +Add a shared `module M` header and compact dependent definitions to each source. +For example, use distinct single-case unions for `FormatConfig`, `ResolvedSetting`, and `EditorConfigProblem`. +Keep their definitions identical between sources. Preserve all original record field names, types, and order. +Do not introduce Fantomas package dependencies. + +The FS0312 message for `R` is: + +```text +The type definitions for type 'R' in the signature and implementation are not compatible because the order of the fields is different in the signature and implementation +``` + +Use `ResolvedConfig` instead of `R` for that fixture. +Locate the implementation type identifier in each actual source string for its full expected range. +Do not copy ranges from a fixture with different leading lines. + +For boundary coverage, reuse suitable existing controls when available. +Otherwise add compact data rows through the same source-pair helper: + +- Named union fields: `type U = Case of A: int * B: string`, with the two named fields reversed in the implementation. +- Named exception fields: `exception E of A: int * B: string`, with fields reversed in the implementation. +- Explicit class fields: use `val` fields in a class declaration and reverse their order, keeping names, types, and accessibility identical. +- Malformed record fields: duplicate a field name, and retain the missing/extra cases above for unequal-length coverage. + +Establish their exact current diagnostics before applying the guard. +Do not invent diagnostic numbers for these controls or claim that each is a new RED test. +The nullness prefix case intentionally loses only the same unwanted FS0193 as the primary cases. +Do not refactor recovered duplicate names or replace list-length behavior. + +Run and retain the first RED result while `SignatureConformance.fs` still matches the starting source: + +```powershell +$env:BUILDING_USING_DOTNET = 'true' +dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Debug -- --filter-method "*Issue 20410*" +``` + +Verify that failures are unwanted positional FS0193, not syntax errors, missing sources, stale binaries, or test-discovery failures. +Record each control's starting result. Preserve full messages, ranges, severities, and duplicate counts. +Do not assert that a specific total equals the historical eighteen compilations. The exact original fixture adds coverage. + +### 3. Apply only the record positional guard + +In `checkRecordFields`, keep both preceding `NameMap.suball2` expressions unchanged. +Keep the existing constructor-order comment, `List.forall2`, FS0312 error expression, location `m`, and false result. +Replace only the function passed to that final `List.forall2`. +Its body must short-circuit as follows: + +```fsharp +implField.LogicalName = sigField.LogicalName +&& checkField aenv infoReader implTycon sigTycon implField sigField +``` + +Use the existing field type if lambda parameters require annotations. +When names differ, return false without calling `checkField`. +When names match, call the existing `checkField` exactly as before. +Do not replace the predicate with name equality alone. +Do not sort fields, add a set/map, accept reordering, suppress shared FS0193, or alter `checkField`. +Do not change `checkUnionCase`, `checkRecordFieldsForExn`, `checkClassFields`, or diagnostic resources. + +Invoke the `fsharp-diagnostics` skill immediately after the compiler edit. +Run its parse check, then its typecheck for this file: + +```powershell +.\.github\skills\fsharp-diagnostics\scripts\get-fsharp-errors.ps1 -ParseOnly src\Compiler\Checking\SignatureConformance.fs +.\.github\skills\fsharp-diagnostics\scripts\get-fsharp-errors.ps1 src\Compiler\Checking\SignatureConformance.fs +``` + +Fix errors before proceeding. A service diagnostic check does not replace compilation tests. + +### 4. Prove GREEN and preserve siblings + +Rebuild with the edited compiler and rerun the identical `*Issue 20410*` selection. +Do not use `--no-build` for the first run after production edits. +Retain the complete GREEN result beside RED evidence. +The invalid record programs must still fail compilation, but all regression assertions must pass. + +Run nearby signature tests and these existing sibling selections: + +| Existing test location | Selection | +|---|---| +| `Conformance\Signatures\Signatures.fs` | Class `Conformance.Signatures.SignatureConformance`, including `AttributeMatching01 - attribute mismatch between signature and implementation`. | +| `ErrorMessages\ExtendedDiagnosticDataTests.fs` | `FieldNotContainedDiagnosticExtendedData 01`, both `useTransparentCompiler` rows. | +| `Language\Nullness\NullableRegressionTests.fs` | `Signature conformance` and all three `Micro compilation` rows in `Language.NullableRegressions`. | + +All paths in this table are under `tests\FSharp.Compiler.ComponentTests`. +The attribute test, two field-data rows, one signature-nullness row, and three micro rows form the seven reported sibling rows. +Verify their actual discovery locally rather than trusting this count. + +Example commands after a fresh Debug build: + +```powershell +$env:BUILDING_USING_DOTNET = 'true' +dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Debug --no-build -- --filter-class "Conformance.Signatures.*" +dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Debug --no-build -- --filter-method "*FieldNotContainedDiagnosticExtendedData*" +dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Debug --no-build -- --filter-method "*Signature conformance*" --filter-method "*Micro compilation*" +``` + +Confirm that repeated method selectors are a union in the installed runner. +If they are not, run the two nullness selections separately. +Start with these targeted tests. Escalate only if failures require wider coverage. +Use Release configuration if running the full component suite. + +For build failures, invoke `binlog-analysis`, collect a binary log, and fix the cause. +Investigate stale bootstrap outputs before drawing conclusions about the source. +Clean only verified, task-owned build artifacts when needed. Do not remove repository files or another user's work. +A setup or build failure is not RED evidence for this diagnostic bug. + +### 5. Format, review, document, and commit + +Format only changed F# files: + +```powershell +dotnet fantomas src\Compiler\Checking\SignatureConformance.fs tests\FSharp.Compiler.ComponentTests\Conformance\Signatures\Signatures.fs +``` + +Restore the declared Fantomas tool only if the command reports that the tool is missing. +Inspect formatter output. Retain only formatting required for changed code. +If whole-file formatting creates unrelated changes, undo only your formatter's unrelated edits. +Do not format the repository or refresh unrelated baselines. +Rerun the affected diagnostics and tests after final edits. + +Invoke the `reviewing-compiler-prs` skill and the `expert-reviewer` agent on the final local implementation diff. +Keep the review local. Do not post comments, open a PR, or push. +Ask the review to verify record-only scope, same-name calls, source-range side effects, diagnostic multiplicity, and all controls. +Resolve concrete findings and rerun affected tests. +Apply `code-compaction` if the test diff is bloated, duplicated, or reaches its bug-fix size trigger. +Do not split implementation and tests into separate incomplete sprints. + +Invoke `release-notes` after the fix passes. +Confirm `VNEXT` with `gh variable get VNEXT --repo dotnet/fsharp`. +Use the corresponding `.FSharp.Compiler.Service` file and its existing `Fixed` section. +Choose an insertion point with `.github\skills\release-notes\pick-insert-line.fsx`, passing the explicit version file. +Suggested entry: + +```markdown +* Remove misleading FS0193 when record fields differ in order between a signature and its implementation, while retaining FS0312. ([Issue #20410](https://github.com/dotnet/fsharp/issues/20410)) +``` + +There is no PR in this commit-only task. Use the real issue link, not a fabricated PR number. +Leave existing release-note entries unchanged. + +Run `git diff --check` and inspect the complete implementation diff. +Keep the production change limited to the record predicate. +Remove temporary source probes and generated files that you created, but retain useful evidence under the named evidence directory. +Keep planning documents already tracked by the architect. +Stage only the compiler file, test file, and selected release-note file. +Commit the verified implementation with a descriptive message, for example `Fix misleading record field-order diagnostics (#20410)`. +Include the required Copilot trailers using the implementation session's ID. +Do not amend earlier commits or push. + +### Evidence for the independent verifier + +Record source HEAD, compiler-build configuration, exact commands, test discovery counts, exit codes, and log paths. +Keep RED and GREEN output in distinct files under `.tools\ralph\evidence\issue-20410`. +Record which tests reused existing controls and which tests added fixtures. +Preserve evidence when work continues in another execution window. +The verifier must inspect assertions and rerun the tests, not rely only on a prose success statement. +Do not mark this sprint complete if RED, GREEN, review resolution, or the implementation commit is missing. + +## Definition of Done + +- Before the production edit, local regression failures demonstrate unwanted positional FS0193 from compilation of both source files. +- One data-driven regression covers the exact `ResolvedConfig` fixture and all five reduced/variant cases without copied test bodies. +- The complete raw diagnostic assertions include severity, code, message, source identity, range, order, and duplicate count. +- Pure permutations produce exactly FS0312 at the implementation type and still fail compilation. +- Attribute-conflict permutations retain the measured FS1200 diagnostics plus FS0312, without positional FS0193. +- Identical record declarations compile successfully with no diagnostics. +- Same-name type, mutability, and accessibility mismatches retain genuine FS0193 without FS0312. +- Missing, extra, and renamed fields retain their complete FS0313, FS0311, and FS0313 diagnostic lists respectively. +- Both nullness cases retain exactly three raw FS3261 warnings; only the prefix permutation has FS0312. +- Union, exception, class-field, duplicate-name, and unequal-length controls preserve their measured starting behavior without compiler crashes. +- Both record name-map passes and every matching-name positional `checkField` call remain unchanged in behavior. +- Production changes affect only the final `checkRecordFields` predicate, with no shared diagnostic or non-record conformance changes. +- The compiler file passes `fsharp-diagnostics` parse and type checks, and the rebuilt targeted tests pass locally. +- Nearby signature-conformance tests and the named seven sibling rows pass with nonzero discovery. +- Only changed F# files are formatted, and `git diff --check` passes without unrelated formatting or baseline changes. +- The local expert review is complete, concrete findings are resolved, and affected validation is rerun. +- One concise compiler-service release note links issue #20410 without inventing a PR. +- RED/GREEN commands and logs persist under the evidence directory, separate from committed implementation files. +- The compiler fix, tests, and release note are committed locally with required trailers, and no push or publication occurs. From 2bbf4d6d41f8d6216a59598f23c64a65133e7f17 Mon Sep 17 00:00:00 2001 From: Copilot Date: Tue, 15 Sep 2026 15:46:57 +0200 Subject: [PATCH 2/6] Fix misleading record field-order diagnostics (#20410) Guard positional record checks by logical name before calling checkField. Preserve FS0312, matching-name checks, and complete warning multiplicity. Add RED-first paired-source coverage and a compiler-service release note. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 17b0ddb3-0fbb-4b25-b9e8-d3bf1e9d6de9 --- .../.FSharp.Compiler.Service/11.0.100.md | 1 + src/Compiler/Checking/SignatureConformance.fs | 4 +- .../Conformance/Signatures/Signatures.fs | 146 ++++++++++++++++++ 3 files changed, 150 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 4df01e7ceb0..b7c0f2f1f89 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -29,6 +29,7 @@ * Fix `MethodAccessException` under `--realsig+` when a closure (inner `let rec`, `task`/`async` state machine, or quotation splice) inside a member defined in an intrinsic type augmentation (`type C with member ...`) accesses a `private` member of `C`. The synthesized closure is now nested inside the declaring type instead of beside it in the module class. ([Issue #19933](https://github.com/dotnet/fsharp/issues/19933), [PR #19955](https://github.com/dotnet/fsharp/pull/19955)) * Preserve source range for type errors on empty-bodied computation expressions (e.g. `foo {}`) in pipelines, function arguments, and type-annotated contexts, instead of reporting `unknown(1,1)`. ([Issue #19550](https://github.com/dotnet/fsharp/issues/19550), [PR #19849](https://github.com/dotnet/fsharp/pull/19849)) * Fix multiline nested type arguments failing to parse when the closing `>` aligns with the opening type name's column. ([Issue #15171](https://github.com/dotnet/fsharp/issues/15171)) +* Remove misleading FS0193 when record fields differ in order between a signature and its implementation, while retaining FS0312. ([Issue #20410](https://github.com/dotnet/fsharp/issues/20410)) * Tooltip "Full name" now shows demangled companion module names (e.g. `MyType.func` instead of `MyTypeModule.func`). ([Issue #17335](https://github.com/dotnet/fsharp/issues/17335), [PR #19867](https://github.com/dotnet/fsharp/pull/19867)) * Fix spurious FS0410 accessibility error when tuple-deconstructing bindings use private types in the same module scope. ([Issue #4161](https://github.com/dotnet/fsharp/issues/4161), [PR #19947](https://github.com/dotnet/fsharp/pull/19947)) * Fix internal error (FS0193) when calling an indexed property setter with a named argument that matches an indexer parameter. ([Issue #16034](https://github.com/dotnet/fsharp/issues/16034), [PR #19851](https://github.com/dotnet/fsharp/pull/19851)) diff --git a/src/Compiler/Checking/SignatureConformance.fs b/src/Compiler/Checking/SignatureConformance.fs index a56699861ba..7f8896a267d 100644 --- a/src/Compiler/Checking/SignatureConformance.fs +++ b/src/Compiler/Checking/SignatureConformance.fs @@ -637,7 +637,9 @@ type Checker(g, amap, denv, remapInfo: SignatureRepackageInfo, checkingSig) = // This check is required because constructors etc. are externally visible // and thus compiled representations do pick up dependencies on the field order - (if List.forall2 (checkField aenv infoReader implTycon sigTycon) implFields sigFields + (if List.forall2 (fun (implField: RecdField) (sigField: RecdField) -> + implField.LogicalName = sigField.LogicalName && + checkField aenv infoReader implTycon sigTycon implField sigField) implFields sigFields then true else (errorR(Error (FSComp.SR.DefinitionsInSigAndImplNotCompatibleFieldOrderDiffer(kindText, implTyconName), m)); false)) diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Signatures/Signatures.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Signatures/Signatures.fs index 80d3a3d270d..6c694859fe2 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Signatures/Signatures.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Signatures/Signatures.fs @@ -166,3 +166,149 @@ type B(a: A) = class end |> compile |> shouldFail |> withErrorCode 0410 + + let private recordSignaturePair signature implementation = + Fsi ("module M\n" + signature) + |> withAdditionalSourceFile (FsSource ("module M\n" + implementation)) + |> asLibrary + + let private assertRecordDiagnostics expected (result: CompilationResult) = + // Raw diagnostic columns are zero-based; keep repeated warnings and their order. + let actual = + result.Output.Diagnostics + |> List.map (fun d -> + d.Error, System.IO.Path.GetFileName d.NativeRange.FileName, + (d.Range.StartLine, d.Range.StartColumn, d.Range.EndLine, d.Range.EndColumn), + d.Message.Replace("\r\n", "\n")) + Assert.True((expected = actual), sprintf "Expected:\n%A\nActual:\n%A" expected actual) + + let private recordOrderDiagnostic typeName line column = + ErrorType.Error 312, "test.fs", (line, column, line, column + String.length typeName), + $"The type definitions for type '{typeName}' in the signature and implementation are not compatible because the order of the fields is different in the signature and implementation" + + let private fieldMismatch column (implementation: string) (signature: string) (reason: string) = + ErrorType.Error 193, "test.fs", (2, column, 2, column + 1), + $"The module contains the field\n {implementation} \nbut its signature specifies\n {signature} \n{reason}" + + [] + [] + [] + [] + [] + []\ntype R<'T> = { A: 'T; B: 'T list }", "[]\ntype R<'T> = { B: 'T list; A: 'T }", "R", 3, false)>] + [] A: int; B: string }", "type R = { B: string; [] A: int }", "R", 2, true)>] + let ``Issue 20410 - record field permutations retain only genuine diagnostics`` signature implementation typeName line attributeConflict = + let result = recordSignaturePair signature implementation |> compile |> shouldFail + Assert.Contains(result.Output.Diagnostics, fun d -> d.Error = ErrorType.Error 312) + result |> assertRecordDiagnostics [ + if attributeConflict then + Warning 1200, "test.fs", (2, 24, 2, 47), + "The attribute 'ObsoleteAttribute' appears in both the implementation and the signature, but the attribute arguments differ. Only the attribute from the signature will be included in the compiled code." + recordOrderDiagnostic typeName line 5 + ] + + [] + let ``Issue 20410 - aligned record fields compile`` () = + let declaration = "type R = { A: int; B: string }" + recordSignaturePair declaration declaration + |> compile + |> shouldSucceed + |> assertRecordDiagnostics [] + + [] + [] + [] + [] + let ``Issue 20410 - same-name field mismatches are preserved`` implementation field column reason = + recordSignaturePair "type R = { A: int; B: string }" implementation + |> compile + |> shouldFail + |> assertRecordDiagnostics [ + if field = "A: int" then + fieldMismatch 28 "B: string" "B: string" reason + fieldMismatch column field "A: int" reason + ] + + [] + [] + [] + [] + let ``Issue 20410 - different record field name sets are preserved`` implementation extra = + let code, reason = + if extra then 311, "C was present in the implementation but not in the signature" + else 313, "B was required by the signature but was not specified by the implementation" + recordSignaturePair "type R = { A: int; B: string }" implementation + |> compile + |> shouldFail + |> assertRecordDiagnostics [ + ErrorType.Error code, "test.fs", (2, 5, 2, 6), + $"The type definitions for type 'R' in the signature and implementation are not compatible because the field {reason}" + ] + + [] + [] + [] + let ``Issue 20410 - matching prefix retains repeated nullness warnings`` swapped = + let fields = if swapped then "B: bool; A: int" else "A: int; B: bool" + let result = + recordSignaturePair + "type R = { Prefix: string; A: int; B: bool }" + $"type R = {{ Prefix: string | null; {fields} }}" + |> withLangVersionPreview + |> withCheckNulls + |> withWarnOn 3261 + |> compile + if swapped then + result |> shouldFail |> ignore + Assert.Contains(result.Output.Diagnostics, fun d -> d.Error = ErrorType.Error 312) + result |> assertRecordDiagnostics [ + for _ in 1..3 do + Warning 3261, "test.fs", (2, 11, 2, 17), + "Nullness warning: The module contains the field\n Prefix: string | null \nbut its signature specifies\n Prefix: string \nThe types differ in their nullness annotations" + if swapped then + recordOrderDiagnostic "R" 2 5 + ] + + [] + [] + [] + [] + [] + let ``Issue 20410 - non-record and duplicate-field behavior is preserved`` signature implementation code = + let expected = + match code with + | 36 -> [ + fieldMismatch 17 "B: string" "A: int" "The names differ" + ErrorType.Error 36, "test.fs", (2, 9, 2, 13), + "The module contains the constructor\n | Case of B: string * A: int \nbut its signature specifies\n | Case of A: int * B: string \nThe types of the fields differ" + ] + | 63 -> [ + fieldMismatch 15 "B: string" "A: int" "The names differ" + ErrorType.Error 63, "test.fs", (2, 10, 2, 11), + "The exception definitions are not compatible because the order of the fields is different in the signature and implementation. The module contains the exception definition\n exception E of B: string * A: int \nbut its signature specifies\n\texception E of A: int * B: string." + ] + | 37 -> [ErrorType.Error 37, "test.fs", (2, 19, 2, 20), "Duplicate definition of field 'A'"] + | 0 -> [] + | _ -> failwithf "Unexpected control diagnostic: %d" code + let result = recordSignaturePair signature implementation |> compile + result |> (if code = 0 then shouldSucceed else shouldFail) |> assertRecordDiagnostics expected From 5c489dfdb967585f90847a5d6fe7f536d03163cf Mon Sep 17 00:00:00 2001 From: Copilot Date: Tue, 15 Sep 2026 16:56:49 +0200 Subject: [PATCH 3/6] Add release notes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7477690c-d85c-4d8d-87f4-579d2acae391 --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index b7c0f2f1f89..9337553a915 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -29,7 +29,7 @@ * Fix `MethodAccessException` under `--realsig+` when a closure (inner `let rec`, `task`/`async` state machine, or quotation splice) inside a member defined in an intrinsic type augmentation (`type C with member ...`) accesses a `private` member of `C`. The synthesized closure is now nested inside the declaring type instead of beside it in the module class. ([Issue #19933](https://github.com/dotnet/fsharp/issues/19933), [PR #19955](https://github.com/dotnet/fsharp/pull/19955)) * Preserve source range for type errors on empty-bodied computation expressions (e.g. `foo {}`) in pipelines, function arguments, and type-annotated contexts, instead of reporting `unknown(1,1)`. ([Issue #19550](https://github.com/dotnet/fsharp/issues/19550), [PR #19849](https://github.com/dotnet/fsharp/pull/19849)) * Fix multiline nested type arguments failing to parse when the closing `>` aligns with the opening type name's column. ([Issue #15171](https://github.com/dotnet/fsharp/issues/15171)) -* Remove misleading FS0193 when record fields differ in order between a signature and its implementation, while retaining FS0312. ([Issue #20410](https://github.com/dotnet/fsharp/issues/20410)) +* Remove misleading FS0193 when record fields differ in order between a signature and its implementation, while retaining FS0312. ([Issue #20410](https://github.com/dotnet/fsharp/issues/20410), [PR #20559](https://github.com/dotnet/fsharp/pull/20559)) * Tooltip "Full name" now shows demangled companion module names (e.g. `MyType.func` instead of `MyTypeModule.func`). ([Issue #17335](https://github.com/dotnet/fsharp/issues/17335), [PR #19867](https://github.com/dotnet/fsharp/pull/19867)) * Fix spurious FS0410 accessibility error when tuple-deconstructing bindings use private types in the same module scope. ([Issue #4161](https://github.com/dotnet/fsharp/issues/4161), [PR #19947](https://github.com/dotnet/fsharp/pull/19947)) * Fix internal error (FS0193) when calling an indexed property setter with a named argument that matches an indexer parameter. ([Issue #16034](https://github.com/dotnet/fsharp/issues/16034), [PR #19851](https://github.com/dotnet/fsharp/pull/19851)) From 16ce6a6108519401e2474dc0dc6fbba40b7a95a4 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 16 Sep 2026 01:34:11 +0200 Subject: [PATCH 4/6] Plan targeted desktop CI repair for existing PR #20559 Replace the completed compiler implementation sprint with one self-contained CI recovery sprint based on build 1597638. Preserve the existing guard, tests, and release note; require diagnosis, complete Release/net472 batch coverage, local review, and commit-only delivery. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 852fa0ba-f0ae-4405-8299-c6a8b51d1feb --- .tools/ralph/BACKLOG.md | 162 +++++--- .../01_Record_Field_Order_Diagnostics.md | 355 ------------------ .tools/ralph/sprints/01_Repair_Desktop_CI.md | 333 ++++++++++++++++ 3 files changed, 435 insertions(+), 415 deletions(-) delete mode 100644 .tools/ralph/sprints/01_Record_Field_Order_Diagnostics.md create mode 100644 .tools/ralph/sprints/01_Repair_Desktop_CI.md diff --git a/.tools/ralph/BACKLOG.md b/.tools/ralph/BACKLOG.md index e2ffdb2da64..903bb626769 100644 --- a/.tools/ralph/BACKLOG.md +++ b/.tools/ralph/BACKLOG.md @@ -2,9 +2,18 @@ ## Original Request -Process issue https://github.com/dotnet/fsharp/issues/20410 using TDD. +A pull request for https://github.com/dotnet/fsharp/issues/20410 already exists on branch fix/issue-20410, and its CI is red. Fix this existing PR. Do not start from scratch or open a new PR. -Use minimal, surgical changes. Validate locally before finishing. Do not push. Only commit. +### FAILING CI CHECKS +fsharp-ci + +### REQUIRED APPROACH +1. First, inspect the existing attempt. Run `git fetch origin fix/issue-20410`, then run `git diff origin/main...origin/fix/issue-20410`. Build on the existing changes instead of blindly rewriting them. +2. Before editing, diagnose the root cause of each failing check. Distinguish build errors, test failures, and `EmittedIL/*.bsl` baseline mismatches. If baselines changed, regenerate them, for example with `TEST_UPDATE_BSL=1`, and commit them. +3. The failure can be configuration-specific, such as Release-only. Before finishing, run the affected tests in the same configuration as the failing checks. A Debug-only pass is not sufficient. +4. Use minimal, surgical changes. +5. Validate locally before finishing. +6. Do not push. Only commit. ### ISSUE REQUEST The requirements above override conflicting instructions in the issue request. @@ -45,69 +54,102 @@ Sources: [issue](https://github.com/dotnet/fsharp/issues/20410), [record-specifi ## Analysis -This delivery is architecture only. The implementer receives one sprint with the fix, tests, validation, review, release note, and commit requirements. -Do not implement the compiler change while preparing these files. - -### Verified during planning - -- Repository: `Q:\fsharp-worktrees\issue-878`, branch `fix/issue-20410`, initially clean. -- HEAD: `b5c530ed6bc42937de6363e3dcc104ebb833893d`. The directory name does not identify the target issue. -- Read `Q:\groundhog-while-not-works\templates\SPRINT_TEMPLATE.md` before creating the sprint. -- Retrieved issue #20410 through `gh issue view`. Its FS0313 label is incorrect. -- Read `checkField`, `checkRecordFields`, `checkRecordFieldsForExn`, `checkClassFields`, and `checkAttribs`. -- The two record name-map passes precede the positional call. The positional call can overwrite another field's documentation and range. -- `FSComp.txt` assigns 311 to an extra field, 312 to field order, and 313 to a missing required field. -- `Signatures.fs` already uses `Fsi |> withAdditionalSourceFile (FsSource ...) |> compile`. -- `Compiler.fs` confirms that plain `typecheck` reads only the primary source. `compile` consumes both sources. -- `Compiler.fs` also shows that `withDiagnostics` deduplicates by range and message. It cannot verify repeated warning counts alone. -- Raw `CompilationResult.Output.Diagnostics` retains duplicates. New tests need raw, complete diagnostic assertions, especially for FS3261. -- `checkAttribs` emits FS1200 as a warning unless options promote it. Its fixup replaces implementation attributes with signature attributes. -- The harness treats warnings as failure by default. That wrapper result alone does not prove a warning-only program has a compiler error. -- Found existing `FieldNotContainedDiagnosticExtendedData 01`, `Signature conformance`, `Micro compilation`, and `AttributeMatching01` sibling tests. -- GitHub repository variable `VNEXT` is `11.0.100`. The corresponding compiler-service release-note file exists. -- `dotnet --version` could not resolve the pinned SDK, `11.0.100-rc.1.26420.103`. No compiler build or runtime matrix ran during planning. -- `eng\common\dotnet.ps1` can install and invoke the repository SDK. SDK setup belongs to execution, not this documentation-only delivery. -- `.tools` is ignored. Commit only the two requested plan files with explicit `git add -f` paths. Do not change `.gitignore`. -- Native PowerShell validation passed: one sprint, 19 completion criteria, 17 existing source references, required scenario coverage, and balanced code fences. -- That validation also confirmed ASCII text, no trailing whitespace, and no tracked source or staged changes before staging the planning files. - -The eighteen matrix compilations, seven RED assertions, eleven controls, and separate original fixture are evidence supplied by the request. -No previous-session evidence was found in the bounded history lookup. -Do not present those results as new local executions or infer an exact new test count from them. - -### Main risks - -Pure name equality would remove matching-name checks and repeated nullness warnings. -A shared `checkField` change would affect unions, exceptions, classes, and real field mismatches. -A test using plain `typecheck`, error-code presence, or deduplicated warnings could produce false confidence. -A test run using stale compiler binaries or discovering no selected tests cannot establish RED or GREEN. -Formatting an entire large compiler file can create unrelated changes even when the functional fix is tiny. +This delivery replaces an obsolete implementation plan with one self-contained CI repair sprint. +It does not implement the CI repair or claim local compiler/test success. +The requested template was read before creating the replacement sprint. + +### Existing attempt and branch state + +- Ran `git fetch origin fix/issue-20410`, then `git diff origin/main...origin/fix/issue-20410`, before planning changes. +- Existing PR: [#20559](https://github.com/dotnet/fsharp/pull/20559), open, titled "Fix misleading diagnostics for reordered record fields". +- Existing remote head: `5c489dfdb967585f90847a5d6fe7f536d03163cf`. Implementation commit: `2bbf4d6d41f8d6216a59598f23c64a65133e7f17`. +- The local branch initially pointed to `b5c530ed6bc42937de6363e3dcc104ebb833893d`, also the local `origin/main`. +- With a clean tracked worktree, ran `git merge --ff-only origin/fix/issue-20410`. Planning now extends the existing PR history. +- Existing product diff: the record-only logical-name guard, 146 test lines, and one compiler-service release note. +- The source already retains matching-name `checkField` calls, both name-map passes, FS0312, and failed conformance. +- The test suite already has all requested scenarios, shared paired-source construction, and raw exact diagnostic assertions. +- The release note already links both #20410 and #20559. Do not add a duplicate or remove the PR link. +- The old sprint incorrectly says no PR exists and the guard has not been applied. Replace it, rather than leave it executable. + +### Verified CI evidence + +Build [1597638](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1597638), number `20260915.39`, tested merge SHA `5604d594ed08aa786661166a3fffd1811db0e471`. +The current PR head is not that synthetic merge SHA. +The timeline has 48 jobs: 47 succeeded, one canceled. The aggregate `fsharp-ci` check failed because of that cancellation. + +| Surface | Observed result | Classification | +|---|---|---| +| `WindowsNoRealsig_testDesktop`, job `916a2273-64f0-5130-a29e-a4d2f7e48c60` | Agent exceeded the configured 120-minute limit, 15:07:05Z to 17:07:26Z on September 15 | Job timeout, not an observed assertion failure | +| Build task, log 861 | Build summaries show zero errors; solution-wide net472 tests start at 15:27:44Z | No observed compilation failure | +| Component suite in log 861 | 8,284 passed, zero failed, 627 skipped; finished at 16:44:04Z after 76m12s | Successful suite inside canceled job | +| Core and service suites in log 861 | Core: 6,212 passed, 5 skipped; service: 3,487 passed, 306 skipped; both zero failures | Successful suites | +| Legacy `FSharpSuite.Tests` | No completion summary in the canceled job | Remaining workload or runner-lifetime investigation | +| Agent resource warnings | 95.69% memory used at 16:26:44Z and 16:26:49Z | Evidence supporting contention, not proof of a deadlock | +| `WindowsCompressedMetadata_Desktop Batch3`, task log 848 | Isolated legacy suite: 677 total, zero failed; job completed in about 91m18s | Existing isolation pattern succeeds | +| Desktop Batch1 / Batch2 | Jobs completed in about 50m11s / 38m56s | Existing three-batch pattern available | +| `WindowsNoRealsig_testCoreclr`, formatting, ILVerify | All succeeded | No justification to alter these surfaces | +| `EmittedIL` baselines | No observed mismatch in the retrieved failing-task log; component suite passed | Do not regenerate speculatively | +| Test results and binlog publication in canceled job | Those explicit tasks were skipped | Missing artifacts are not proof of passing tests | + +The build-status script reports zero build errors and test failures because it filters `failed` tasks, not this `canceled` task. +The raw timeline and canceled-task log are the decisive evidence. +Do not present the script's empty result as a clean CI run. + +Public evidence endpoints use `https://dev.azure.com/dnceng-public/public/_apis/build/builds/1597638`. +Append `/timeline?api-version=7.1`, `/logs/861?api-version=7.1`, or `/logs/848?api-version=7.1`. +The sprint embeds the essential evidence and does not depend on this backlog or access to another session. + +### Competing hypotheses and prior progress + +| Hypothesis | Evidence and next verification | +|---|---| +| Concurrent desktop suites exceed the job budget under memory pressure | Supported by resource warnings and isolated legacy success. Compare unsplit and isolated local runs with identical binaries and settings. | +| A legacy test or runner teardown hangs | No completion result alone cannot distinguish a hang from slowness. Record progress, exit status, child processes, and a dump if progress stops. | +| Release/compiler regression or IL baseline drift causes the failure | No CI assertion or build error supports this. Run the existing regressions and actual desktop batches before dismissing it. | + +A bounded history lookup found session `83fb7dbc-211a-47df-8f32-557feaf219c2`. +It reports an unsplit local pass in 97m48s and an isolated legacy pass in 69m23s, with 677 passing tests. +It also reports that the VS engine could not load the SDK, while `-msbuildEngine dotnet` built successfully. +Its last available response says proposed Batch1 validation was blocked by a leftover MSBuild assembly lock. +Batch2, Batch3, and dedicated desktop/net11 signature reruns were still pending in that response. +These are historical reports, not fresh verified logs or evidence that the proposed repair is complete. +Recover matching logs if available. Otherwise rerun the missing evidence without resetting completed source work. + +### Repository constraints that matter + +- `azure-pipelines-PR.yml` contains the failing job near line 296 and the working desktop matrix near line 438. +- Reuse `eng\templates\batched-test-steps.yml`, `eng\Build.ps1`'s `-testDesktopBatch`, and `eng\tests\TestSplit.fsx`. +- `TestUsingMSBuild` already supplies net472, xUnit reports, binlogs, and five-minute hang dumps. +- Batch1 has residual component tests plus build tests. Batch2 has the remaining components, core, service, and scripting tests. +- Batch3 isolates `tests\fsharp\FSharpSuite.Tests.fsproj`. Preserve complete, nonoverlapping coverage of all six desktop test projects. +- `BUILDING_USING_DOTNET=true` removes net472 from component-project target frameworks. Override it only in the current process for desktop validation. +- The pinned SDK is `11.0.100-rc.1.26420.103`. The other component target is `net11.0`. +- Use supported SDK/build-engine setup. Do not change SDK versions or shared build scripts to hide a local tooling failure. ## Approach -Use one complete vertical sprint. Splitting tests, the guard, and controls would make early sprints intentionally incomplete. -The sprint embeds the original record permutation, minimal fixtures, assertion requirements, source helpers, commands, and restrictions. -Its RED phase captures exact diagnostics before editing production code. -Its GREEN phase changes only the final record predicate and reruns the unchanged tests plus siblings. -The final implementation commit contains the compiler change, compact tests, and one release note. -Keep execution logs under `.tools\ralph\evidence\issue-20410`, outside the implementation commit. -Never push, open a PR, post a review, or fabricate a PR URL. - -### Final verification checklist - -- The sprint starts with two `---` lines and contains all four required headings. -- Every Definition of Done item starts with `- `, without a checkbox. -- Every requested scenario is covered within the same sprint as its implementation. -- The sprint stands alone without this backlog, another sprint, or historical logs. -- The exact request above is preserved, including its distinction between FS0312 and FS0313. -- Local source references, project paths, and release-note path exist. -- The plan explicitly preserves duplicate warnings and complete diagnostic tuples. -- Planning validation checks file structure, paths, coverage, and `git diff --check`. It does not claim compiler GREEN. -- The planning commit contains only `BACKLOG.md` and `01_Record_Field_Order_Diagnostics.md`. -- The implementation verifier later requires actual RED/GREEN logs, passing sibling selections, review resolution, and a local implementation commit. +Use one complete vertical repair sprint, including diagnosis, the smallest supported repair, its tests, review, and a local commit. +The leading candidate changes only the `WindowsNoRealsig_testDesktop` job to use the existing three-batch mechanism. +Keep the job's flags, environment, pool, and 120-minute per-job limit. Preserve distinct test and artifact names. +Make that change only after diagnosis supports workload isolation. Investigate a reproducible assertion failure or deadlock instead, if found. + +Do not rerun the old implementation sprint or add duplicate regression tests. +Preserve original RED evidence when available. If it is missing, recover it with unchanged regression assertions and an isolated, temporary guard reversal. +Do not leave that reversal in the repair branch or mistake a setup failure for RED. +Require fresh Release/net472 regression and batch execution, followed by targeted Release/net11 checks. +Compare discovered test identities across batches, not counts alone. Discovery is not execution. +Regenerate only proven affected `EmittedIL` baselines and rerun without update mode before staging them. + +Keep logs and resumable state under `.tools\ralph\evidence\issue-20410-ci`, outside implementation commits. +The implementation verifier must inspect exact commands, source SHA, flags, test counts, timings, exit codes, and unresolved limitations. +Local passes cannot make an unpushed GitHub check green. The final report must distinguish local validation from the unchanged remote check. + +Planning verification checks the requested document structure, one active sprint, source paths, preserved request, and whitespace. +The planning commit changes only the backlog and the replacement sprint, retaining the existing compiler/test/release-note commits. +No build or test execution is claimed for this documentation-only planning pass. ## Sprint Overview | # | Name | Purpose | |---|---|---| -| 01 | Record Field Order Diagnostics | Prove RED, guard the record-only positional check, prove GREEN with all controls, review, document, and commit locally. | +| 01 | Repair Desktop CI | Diagnose the existing PR timeout, apply the smallest proven repair, preserve all regression coverage, validate Release desktop batches, review, and commit without pushing. | diff --git a/.tools/ralph/sprints/01_Record_Field_Order_Diagnostics.md b/.tools/ralph/sprints/01_Record_Field_Order_Diagnostics.md deleted file mode 100644 index 5ba37354109..00000000000 --- a/.tools/ralph/sprints/01_Record_Field_Order_Diagnostics.md +++ /dev/null @@ -1,355 +0,0 @@ ---- ---- -# Sprint: Correct record field-order diagnostics with RED-first coverage - -## Context - WHY this sprint exists - -Fix [dotnet/fsharp issue #20410](https://github.com/dotnet/fsharp/issues/20410) in `Q:\fsharp-worktrees\issue-878`. -This sprint is the complete implementation unit. It has no dependency on another sprint or on `BACKLOG.md`. -Use minimal, surgical changes. Validate locally, then commit. Do not push or publish anything. - -The starting compiler commit is `b5c530ed6bc42937de6363e3dcc104ebb833893d`, on branch `fix/issue-20410`. -Planning documents can be committed above that source revision. Inspect the actual worktree before editing. -Preserve other people's changes and all useful execution evidence. - -`checkRecordFields` in `src\Compiler\Checking\SignatureConformance.fs` first compares record fields by name in both directions. -It then calls the diagnostic-emitting `checkField` positionally through `List.forall2`. -The first displaced field produces misleading FS0193 before the correct FS0312 on the implementation type. -`checkField` also copies XML documentation and updates paired source ranges before comparing names. -The positional call can therefore overwrite the correct same-name association. - -The issue incorrectly calls the order diagnostic FS0313. The existing order diagnostic is **FS0312**. -FS0313 means a required field is missing. FS0311 means an implementation field is absent from the signature. -Do not change these numbers or their messages. -Record constructor parameter order depends on declaration order. A permutation must still fail compilation. - -The request reports eighteen starting-main matrix compilations: seven expected RED assertions and eleven passing controls. -It also reports a separate original `ResolvedConfig` compilation with only positional FS0193 and type-level FS0312. -Those historical logs are not prerequisites. Capture fresh RED and GREEN evidence for the tests in this sprint. -The guard was not applied or proven GREEN during planning. - -## Description - WHAT to implement with DETAILED guidance - -### Files and repository rules - -| Path, relative to the worktree | Purpose | -|---|---| -| `src\Compiler\Checking\SignatureConformance.fs` | Change only the final positional predicate in `checkRecordFields`, initially around lines 640-642. | -| `tests\FSharp.Compiler.ComponentTests\Conformance\Signatures\Signatures.fs` | Add one data-driven regression and separately named controls in `Conformance.Signatures.SignatureConformance`. | -| `docs\release-notes\.FSharp.Compiler.Service\11.0.100.md` | Add one concise `Fixed` entry after successful implementation. `VNEXT` was `11.0.100` during planning. | -| `tests\FSharp.Test.Utilities\Compiler.fs` | Read existing source-pair, compilation, and diagnostic helpers. Do not change this shared harness. | -| `src\Compiler\FSComp.txt` | Read diagnostic identities around lines 149-151. Do not edit diagnostic resources. | -| `.tools\ralph\evidence\issue-20410` | Preserve commands, source revision, fixtures, exit codes, selected test counts, and RED/GREEN logs. Do not commit generated logs. | - -Read these instruction files before editing: - -- `.github\instructions\ExpertReview.instructions.md` -- `.github\instructions\ComponentTests.instructions.md` -- `.github\instructions\NoBloat.instructions.md` - -Read `docs\coding-standards.md` before interpreting compiler abbreviations. -Use existing helpers and F# formatting. Add no API, project, package, diagnostic, feature flag, or new name map. -The existing test file is already included in `tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj`. - -### 1. Establish a usable local baseline - -Run commands from `Q:\fsharp-worktrees\issue-878` in PowerShell. -Set `$env:BUILDING_USING_DOTNET = 'true'` in each fresh command process. -Do not change system-wide environment variables on a shared machine. - -The planning probe `dotnet --version` failed because the pinned SDK was unavailable. -`global.json` requests SDK `11.0.100-rc.1.26420.103` and `Microsoft.Testing.Platform`. -After confirming the missing SDK, use the repository bootstrap: - -```powershell -.\eng\common\dotnet.ps1 --version -``` - -This script installs the repository SDK and invokes it. -Use that SDK for all subsequent commands. If necessary, invoke each command through `.\eng\common\dotnet.ps1`. -Do not change `global.json`, dependency versions, or `eng\common` files to obtain a build. - -Query target frameworks rather than assuming `net472` or another framework: - -```powershell -$env:BUILDING_USING_DOTNET = 'true' -dotnet msbuild tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -getProperty:TargetFrameworks -dotnet msbuild src\Compiler\FSharp.Compiler.Service.fsproj -getProperty:TargetFrameworks -``` - -Confirm the test-runner syntax with the installed SDK's help. -The command patterns below follow the component-test instructions. -If the runner requires a different placement of `--filter-method`, adjust only command syntax. -Confirm nonzero discovery and the intended test names. A zero-test success is not validation. - -### 2. Write complete RED-first paired-source tests - -Follow `Issue 11331 - Public constructor taking internal type should report FS0410 in signature` in `Signatures.fs`. -Share one small source-pair constructor, for example: - -```fsharp -let private recordSignaturePair signature implementation = - Fsi signature - |> withAdditionalSourceFile (FsSource implementation) - |> asLibrary -``` - -Include the same explicit module name in both source strings. -Keep default `.fsi` and `.fs` names paired. Apply options before `compile`. -Use `compile` to consume both sources. -Do not use plain `typecheck`: at this revision, it ignores `AdditionalSources`. -`typecheckProject` handles multiple sources, but changing to that result/assertion model is unnecessary here. - -Use one `[]` with data rows for the six primary cases: exact original, reduced swap, prefix cycle, equal types, generic struct, and attributes. -Use separately named controls for aligned records, real mismatches, different name sets, nullness, and non-record/recovery behavior. -Parameterize related controls rather than copying test bodies. -Include `Issue 20410` in new method names so one filter selects all new coverage. -Use compact inline fixtures and shared declarations. Do not add a separate source file for every row. - -**Assert the entire ordered diagnostic list, including duplicates, severity, number, message, and range.** -`withErrorCode` and substring checks are insufficient. -`withDiagnostics` in `Compiler.fs` calls `assertErrors`, which deduplicates actual diagnostics by range and message. -It also normalizes whitespace. It cannot establish exact duplicate-warning preservation alone. - -Use the existing raw result surface: `result.Output.Diagnostics`. -Project each diagnostic to a stable tuple and compare the complete expected and actual lists. -Include the source basename to distinguish implementation and signature ranges. -For example, project `d.Error`, `Path.GetFileName d.NativeRange.FileName`, `d.Range` coordinates, and `d.Message`. -Raw columns are zero-based. DSL `Col` expectations are one-based. -Choose one convention explicitly and do not mix them. -Normalize CRLF to LF if needed, but do not normalize message wording, filter diagnostics, sort, or deduplicate. -A small local exact-assertion helper is justified. Do not modify the shared assertion framework. -Existing raw-result usage is in `tests\FSharp.Compiler.ComponentTests\Attributes\CompiledNameMultipleValues.fs`. - -Capture current diagnostics and source line layouts before setting control expectations. -Primary regression expectations must exclude only positional FS0193, not genuine diagnostics. -Freeze those expectations before editing production code. -For every invalid permutation, also assert compilation failure and the presence of an actual `Error 312`. - -The harness treats warnings as failure by default, even without compiler `--warnaserror`. -For warning-only controls, assert the complete warning list and absence of errors. -Do not mistake the harness failure wrapper for a language error. -Do not use warning suppression, promotion, or `ignoreWarnings` to simplify the new expectations. - -#### Required fixture and behavior matrix - -| Case | Source guidance | Complete required outcome | -|---|---|---| -| Exact reported record | Preserve the `ResolvedConfig` declarations below. Define all three dependent types locally and identically in both sources. | Exactly one `Error 312` on the implementation `ResolvedConfig` identifier. No positional FS0193. | -| Reduced swap | Signature `type R = { A: int; B: string }`, implementation `type R = { B: string; A: int }`. | Exactly one `Error 312` on implementation `R`. | -| Shared prefix, three-field cycle | Signature `{ Prefix: bool; A: int; B: string; C: decimal }`, implementation `{ Prefix: bool; B: string; C: decimal; A: int }`. | Exactly one `Error 312`, with no error on the displaced field. | -| Equal-type swap | Use `{ A: int; B: int }` and `{ B: int; A: int }`. | Exactly one `Error 312`. Equal types do not make order legal. | -| Generic struct swap | Use `[] type R<'T> = { A: 'T; B: 'T list }` and its reversed implementation. Put `[]` in both sources. | Exactly one `Error 312`. Preserve generic remapping and struct representation checking. | -| Attribute conflict plus swap | Put `[]` on named field `A` in the signature and `"impl"` on `A` in the implementation. Reverse `A` and `B`. | Preserve FS1200 at the attribute and FS0312 at the type. Remove only positional FS0193. Capture actual attribute-target multiplicity. FS1200 is a warning without promotion. | -| Aligned declarations | Compile the same record declaration on both sides. | Successful compilation and an empty raw diagnostic list. | -| Real same-name mismatch | Parameterize `A: int` versus `A: string`, immutable versus mutable `A`, and public versus `internal` record representation. Keep field names and order aligned. | Preserve the genuine field FS0193 and its exact message/range. No spurious FS0312. | -| Different name sets | Signature has `A; B`. For missing, implementation has only `A`. For extra, implementation adds `C`. For renamed, implementation has `A; C`. | Preserve complete lists with FS0313, FS0311, and FS0313 respectively. Do not convert these into order errors. | -| Nullness, aligned and prefix swap | Signature `{ Prefix: string; A: int; B: bool }`. Implementation changes `Prefix` to `string \| null`, then either keeps or swaps `A; B`. | Each variant retains three FS3261 warnings. Aligned has no errors. Prefix-swap adds only FS0312 after the fix. Count raw warnings, including identical duplicates. | -| Union, exception, object fields, recovery | Preserve starting-main controls described below. | Union and exception diagnostics remain unchanged. Reordered class fields can still compile. Malformed fields retain their diagnostics without a compiler crash. | - -For nullness cases, use `withLangVersionPreview`, `withCheckNulls`, and `withWarnOn 3261`. -Do not copy the warning-promotion helper from the nullness test module. -FS1200 is emitted by `checkAttribs`, which also reconciles attributes during name-based checks. -Do not assume repeated attribute checks produce repeated FS1200 warnings. Assert the measured complete list. - -The exact reported field permutation is: - -```fsharp -// Signature declaration -type ResolvedConfig = - { - Config: FormatConfig - Settings: ResolvedSetting list - EditorConfigFiles: string list - Problems: EditorConfigProblem list - } - -// Implementation declaration, in the separate implementation source -type ResolvedConfig = - { - Config: FormatConfig - EditorConfigFiles: string list - Problems: EditorConfigProblem list - Settings: ResolvedSetting list - } -``` - -Add a shared `module M` header and compact dependent definitions to each source. -For example, use distinct single-case unions for `FormatConfig`, `ResolvedSetting`, and `EditorConfigProblem`. -Keep their definitions identical between sources. Preserve all original record field names, types, and order. -Do not introduce Fantomas package dependencies. - -The FS0312 message for `R` is: - -```text -The type definitions for type 'R' in the signature and implementation are not compatible because the order of the fields is different in the signature and implementation -``` - -Use `ResolvedConfig` instead of `R` for that fixture. -Locate the implementation type identifier in each actual source string for its full expected range. -Do not copy ranges from a fixture with different leading lines. - -For boundary coverage, reuse suitable existing controls when available. -Otherwise add compact data rows through the same source-pair helper: - -- Named union fields: `type U = Case of A: int * B: string`, with the two named fields reversed in the implementation. -- Named exception fields: `exception E of A: int * B: string`, with fields reversed in the implementation. -- Explicit class fields: use `val` fields in a class declaration and reverse their order, keeping names, types, and accessibility identical. -- Malformed record fields: duplicate a field name, and retain the missing/extra cases above for unequal-length coverage. - -Establish their exact current diagnostics before applying the guard. -Do not invent diagnostic numbers for these controls or claim that each is a new RED test. -The nullness prefix case intentionally loses only the same unwanted FS0193 as the primary cases. -Do not refactor recovered duplicate names or replace list-length behavior. - -Run and retain the first RED result while `SignatureConformance.fs` still matches the starting source: - -```powershell -$env:BUILDING_USING_DOTNET = 'true' -dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Debug -- --filter-method "*Issue 20410*" -``` - -Verify that failures are unwanted positional FS0193, not syntax errors, missing sources, stale binaries, or test-discovery failures. -Record each control's starting result. Preserve full messages, ranges, severities, and duplicate counts. -Do not assert that a specific total equals the historical eighteen compilations. The exact original fixture adds coverage. - -### 3. Apply only the record positional guard - -In `checkRecordFields`, keep both preceding `NameMap.suball2` expressions unchanged. -Keep the existing constructor-order comment, `List.forall2`, FS0312 error expression, location `m`, and false result. -Replace only the function passed to that final `List.forall2`. -Its body must short-circuit as follows: - -```fsharp -implField.LogicalName = sigField.LogicalName -&& checkField aenv infoReader implTycon sigTycon implField sigField -``` - -Use the existing field type if lambda parameters require annotations. -When names differ, return false without calling `checkField`. -When names match, call the existing `checkField` exactly as before. -Do not replace the predicate with name equality alone. -Do not sort fields, add a set/map, accept reordering, suppress shared FS0193, or alter `checkField`. -Do not change `checkUnionCase`, `checkRecordFieldsForExn`, `checkClassFields`, or diagnostic resources. - -Invoke the `fsharp-diagnostics` skill immediately after the compiler edit. -Run its parse check, then its typecheck for this file: - -```powershell -.\.github\skills\fsharp-diagnostics\scripts\get-fsharp-errors.ps1 -ParseOnly src\Compiler\Checking\SignatureConformance.fs -.\.github\skills\fsharp-diagnostics\scripts\get-fsharp-errors.ps1 src\Compiler\Checking\SignatureConformance.fs -``` - -Fix errors before proceeding. A service diagnostic check does not replace compilation tests. - -### 4. Prove GREEN and preserve siblings - -Rebuild with the edited compiler and rerun the identical `*Issue 20410*` selection. -Do not use `--no-build` for the first run after production edits. -Retain the complete GREEN result beside RED evidence. -The invalid record programs must still fail compilation, but all regression assertions must pass. - -Run nearby signature tests and these existing sibling selections: - -| Existing test location | Selection | -|---|---| -| `Conformance\Signatures\Signatures.fs` | Class `Conformance.Signatures.SignatureConformance`, including `AttributeMatching01 - attribute mismatch between signature and implementation`. | -| `ErrorMessages\ExtendedDiagnosticDataTests.fs` | `FieldNotContainedDiagnosticExtendedData 01`, both `useTransparentCompiler` rows. | -| `Language\Nullness\NullableRegressionTests.fs` | `Signature conformance` and all three `Micro compilation` rows in `Language.NullableRegressions`. | - -All paths in this table are under `tests\FSharp.Compiler.ComponentTests`. -The attribute test, two field-data rows, one signature-nullness row, and three micro rows form the seven reported sibling rows. -Verify their actual discovery locally rather than trusting this count. - -Example commands after a fresh Debug build: - -```powershell -$env:BUILDING_USING_DOTNET = 'true' -dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Debug --no-build -- --filter-class "Conformance.Signatures.*" -dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Debug --no-build -- --filter-method "*FieldNotContainedDiagnosticExtendedData*" -dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Debug --no-build -- --filter-method "*Signature conformance*" --filter-method "*Micro compilation*" -``` - -Confirm that repeated method selectors are a union in the installed runner. -If they are not, run the two nullness selections separately. -Start with these targeted tests. Escalate only if failures require wider coverage. -Use Release configuration if running the full component suite. - -For build failures, invoke `binlog-analysis`, collect a binary log, and fix the cause. -Investigate stale bootstrap outputs before drawing conclusions about the source. -Clean only verified, task-owned build artifacts when needed. Do not remove repository files or another user's work. -A setup or build failure is not RED evidence for this diagnostic bug. - -### 5. Format, review, document, and commit - -Format only changed F# files: - -```powershell -dotnet fantomas src\Compiler\Checking\SignatureConformance.fs tests\FSharp.Compiler.ComponentTests\Conformance\Signatures\Signatures.fs -``` - -Restore the declared Fantomas tool only if the command reports that the tool is missing. -Inspect formatter output. Retain only formatting required for changed code. -If whole-file formatting creates unrelated changes, undo only your formatter's unrelated edits. -Do not format the repository or refresh unrelated baselines. -Rerun the affected diagnostics and tests after final edits. - -Invoke the `reviewing-compiler-prs` skill and the `expert-reviewer` agent on the final local implementation diff. -Keep the review local. Do not post comments, open a PR, or push. -Ask the review to verify record-only scope, same-name calls, source-range side effects, diagnostic multiplicity, and all controls. -Resolve concrete findings and rerun affected tests. -Apply `code-compaction` if the test diff is bloated, duplicated, or reaches its bug-fix size trigger. -Do not split implementation and tests into separate incomplete sprints. - -Invoke `release-notes` after the fix passes. -Confirm `VNEXT` with `gh variable get VNEXT --repo dotnet/fsharp`. -Use the corresponding `.FSharp.Compiler.Service` file and its existing `Fixed` section. -Choose an insertion point with `.github\skills\release-notes\pick-insert-line.fsx`, passing the explicit version file. -Suggested entry: - -```markdown -* Remove misleading FS0193 when record fields differ in order between a signature and its implementation, while retaining FS0312. ([Issue #20410](https://github.com/dotnet/fsharp/issues/20410)) -``` - -There is no PR in this commit-only task. Use the real issue link, not a fabricated PR number. -Leave existing release-note entries unchanged. - -Run `git diff --check` and inspect the complete implementation diff. -Keep the production change limited to the record predicate. -Remove temporary source probes and generated files that you created, but retain useful evidence under the named evidence directory. -Keep planning documents already tracked by the architect. -Stage only the compiler file, test file, and selected release-note file. -Commit the verified implementation with a descriptive message, for example `Fix misleading record field-order diagnostics (#20410)`. -Include the required Copilot trailers using the implementation session's ID. -Do not amend earlier commits or push. - -### Evidence for the independent verifier - -Record source HEAD, compiler-build configuration, exact commands, test discovery counts, exit codes, and log paths. -Keep RED and GREEN output in distinct files under `.tools\ralph\evidence\issue-20410`. -Record which tests reused existing controls and which tests added fixtures. -Preserve evidence when work continues in another execution window. -The verifier must inspect assertions and rerun the tests, not rely only on a prose success statement. -Do not mark this sprint complete if RED, GREEN, review resolution, or the implementation commit is missing. - -## Definition of Done - -- Before the production edit, local regression failures demonstrate unwanted positional FS0193 from compilation of both source files. -- One data-driven regression covers the exact `ResolvedConfig` fixture and all five reduced/variant cases without copied test bodies. -- The complete raw diagnostic assertions include severity, code, message, source identity, range, order, and duplicate count. -- Pure permutations produce exactly FS0312 at the implementation type and still fail compilation. -- Attribute-conflict permutations retain the measured FS1200 diagnostics plus FS0312, without positional FS0193. -- Identical record declarations compile successfully with no diagnostics. -- Same-name type, mutability, and accessibility mismatches retain genuine FS0193 without FS0312. -- Missing, extra, and renamed fields retain their complete FS0313, FS0311, and FS0313 diagnostic lists respectively. -- Both nullness cases retain exactly three raw FS3261 warnings; only the prefix permutation has FS0312. -- Union, exception, class-field, duplicate-name, and unequal-length controls preserve their measured starting behavior without compiler crashes. -- Both record name-map passes and every matching-name positional `checkField` call remain unchanged in behavior. -- Production changes affect only the final `checkRecordFields` predicate, with no shared diagnostic or non-record conformance changes. -- The compiler file passes `fsharp-diagnostics` parse and type checks, and the rebuilt targeted tests pass locally. -- Nearby signature-conformance tests and the named seven sibling rows pass with nonzero discovery. -- Only changed F# files are formatted, and `git diff --check` passes without unrelated formatting or baseline changes. -- The local expert review is complete, concrete findings are resolved, and affected validation is rerun. -- One concise compiler-service release note links issue #20410 without inventing a PR. -- RED/GREEN commands and logs persist under the evidence directory, separate from committed implementation files. -- The compiler fix, tests, and release note are committed locally with required trailers, and no push or publication occurs. diff --git a/.tools/ralph/sprints/01_Repair_Desktop_CI.md b/.tools/ralph/sprints/01_Repair_Desktop_CI.md new file mode 100644 index 00000000000..d06266797b6 --- /dev/null +++ b/.tools/ralph/sprints/01_Repair_Desktop_CI.md @@ -0,0 +1,333 @@ +--- +--- +# Sprint: Repair the existing record-field PR's desktop CI + +## Context - WHY this sprint exists + +Repair existing PR [#20559](https://github.com/dotnet/fsharp/pull/20559) for issue [#20410](https://github.com/dotnet/fsharp/issues/20410). +Work in `Q:\fsharp-worktrees\issue-878` on `fix/issue-20410`. +This is one complete implementation unit, including diagnosis, repair, tests, review, and a local commit. +You do not need another sprint, the backlog, or another agent's history. +Do not start over, open another PR, push, post reviews, amend commits, or change remote settings. + +The PR already contains the compiler guard, regression tests, and release note. +Its remote head at planning was `5c489dfdb967585f90847a5d6fe7f536d03163cf`. +The compiler/test implementation is commit `2bbf4d6d41f8d6216a59598f23c64a65133e7f17`. +The architect fast-forwarded this local branch from main `b5c530ed6bc42937de6363e3dcc104ebb833893d` to the existing PR head. +Planning commits can follow that head. Preserve them and any completed repair work. + +### Verified failure, not an assumed diagnostic regression + +CI build [1597638](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1597638) tested synthetic merge `5604d594ed08aa786661166a3fffd1811db0e471`. +Its 48 jobs comprise 47 successes and one cancellation. +The failed aggregate `fsharp-ci` check comes from `WindowsNoRealsig_testDesktop`. +Job ID: `916a2273-64f0-5130-a29e-a4d2f7e48c60`. + +| Evidence | Observed result | +|---|---| +| Timeline | Agent exceeded 120 minutes, September 15, 2026, 15:07:05Z to 17:07:26Z. | +| Canceled Build task, log 861 | Build summaries have zero errors. Solution-wide Release/net472 tests begin at 15:27:44Z. | +| Component suite | Completed at 16:44:04Z: 8,284 passed, zero failed, 627 skipped, duration 76m12s. | +| Core and service suites | Core: 6,212 passed, 5 skipped. Service: 3,487 passed, 306 skipped. Both have zero failures. | +| Legacy `FSharpSuite.Tests` | No completion summary before cancellation. This does not distinguish slow tests from a runner hang. | +| Memory warnings | 95.69% memory used at 16:26:44Z and 16:26:49Z. | +| Existing desktop Batch3, log 848 | Isolated legacy suite completed 677 tests, zero failures. Whole job took about 91m18s. | +| Existing desktop Batch1 / Batch2 | Successful jobs took about 50m11s / 38m56s. | +| Other checks | No-realsig CoreCLR, formatting, and ILVerify succeeded. No failing-task `EmittedIL` baseline mismatch was observed. | + +The build-status script filters failed tasks and can miss canceled tasks. Read the raw canceled-task log as well. +Explicit test-result and binlog publication tasks were skipped after cancellation. Do not interpret missing results as passes. + +A prior execution reported local unsplit success in 97m48s and isolated legacy success in 69m23s. +That execution reported a VS/SDK incompatibility, then successful builds with the supported `-msbuildEngine dotnet` option. +Its final report left proposed Batch1 blocked by an MSBuild assembly lock, with other batch and signature reruns unfinished. +These reports are leads, not accepted validation. Reuse matching raw evidence if available, or reproduce it. +No local test or repair execution occurred during this planning pass. + +## Description - WHAT to implement with DETAILED guidance + +### 1. Inspect and preserve the existing attempt + +Run these commands before editing implementation files: + +```powershell +Set-Location Q:\fsharp-worktrees\issue-878 +git fetch origin fix/issue-20410 +git --no-pager diff origin/main...origin/fix/issue-20410 +git status --short +git --no-pager log -6 --oneline +gh pr view 20559 --repo dotnet/fsharp --json headRefOid,statusCheckRollup +``` + +Confirm the existing PR history is an ancestor of local HEAD. +If behind and clean, fast-forward with `git merge --ff-only origin/fix/issue-20410`. +Never reset local repair commits or overwrite another agent's edits. +Keep evidence under `.tools\ralph\evidence\issue-20410-ci`, outside committed implementation files. +Record source SHA, commands, configuration, environment, exit codes, durations, test counts, and pending work. +Use distinct filenames for unsplit, isolated, Batch1, Batch2, Batch3, and signature runs. + +| File, relative to the worktree | Required use | +|---|---| +| `azure-pipelines-PR.yml` | Primary candidate edit: `WindowsNoRealsig_testDesktop`, initially near line 296. | +| `eng\templates\batched-test-steps.yml` | Reuse the existing template. Read its parameters and publication conditions. | +| `eng\Build.ps1` | Read `TestUsingMSBuild`, `BuildSolution`, and the `testDesktopBatch` branch. Do not add another runner. | +| `eng\tests\TestSplit.fsx` | Reuse the existing three-batch assignments and `--validate`. Do not change the split without evidence. | +| `eng\CIBuildNoPublish.cmd` | Existing CI entry point, including restore, bootstrap, build, pack, sign, and binary logging. | +| `FSharp.slnx` | Source of the unsplit desktop test-project inventory. | +| `src\Compiler\Checking\SignatureConformance.fs` | Preserve the existing record-only `checkRecordFields` guard. | +| `tests\FSharp.Compiler.ComponentTests\Conformance\Signatures\Signatures.fs` | Preserve and run the existing issue tests in `Conformance.Signatures.SignatureConformance`. | +| `tests\FSharp.Test.Utilities\Compiler.fs` | Existing paired-source and raw diagnostic behavior. Do not change the shared harness. | +| `docs\release-notes\.FSharp.Compiler.Service\11.0.100.md` | Preserve the existing concise entry linking #20410 and #20559. | + +Read `.github\instructions\ExpertReview.instructions.md`, `ComponentTests.instructions.md`, and `NoBloat.instructions.md` before any corresponding F# edits. +Those three files are under `.github\instructions`. +Read `eng\common\AGENTS.md` if an edit there becomes necessary. The proposed repair does not edit that directory. + +### 2. Diagnose every failing check before choosing a repair + +Invoke `pr-build-status` and `hypothesis-driven-debugging`. +Refresh all platforms and jobs, not just the first red check. +For the known build, retrieve: + +```powershell +pwsh .github\skills\pr-build-status\scripts\Get-BuildInfo.ps1 -BuildId 1597638 +pwsh .github\skills\pr-build-status\scripts\Get-BuildErrors.ps1 -BuildId 1597638 +$base = 'https://dev.azure.com/dnceng-public/public/_apis/build/builds/1597638' +Invoke-RestMethod "$base/timeline?api-version=7.1" +Invoke-RestMethod "$base/logs/861?api-version=7.1" +Invoke-RestMethod "$base/logs/848?api-version=7.1" +``` + +Save raw output and a `CI_ERRORS.md` under the evidence directory. +Classify each failure as build/restore, test assertion, baseline mismatch, timeout, or cancellation. +If a newer build exists, update the evidence instead of treating these IDs as permanently current. + +Test these competing hypotheses before editing: + +| Hypothesis | Verification | +|---|---| +| Unsplit concurrent desktop suites cause excessive runtime and memory pressure | Compare the unsplit command with isolated legacy execution on identical Release binaries and settings. Record process and memory observations. | +| A legacy case or runner teardown hangs | Check progress, case duration, process exit, and remaining child processes. If progress stops, inspect the existing hang dump or capture the specific owned process. | +| Release conformance or emitted-code behavior regressed | Run existing issue assertions and inspect actual failing test output. Separate expected negative-test diagnostics from runner failures. | + +Invoke `binlog-analysis` for actual build, restore, or WarnAsError errors. +A timeout alone does not justify changing the compiler or diagnostic expectations. +If a test fails, reproduce that exact case in isolation before editing it. +If `EmittedIL\*.bsl` differs, inspect expected versus actual IL and identify the semantic cause. +Regenerate only affected baselines with process-local `TEST_UPDATE_BSL=1`, then remove that variable and rerun. +Commit generated baseline changes only when the semantic change is intended. Do not refresh unrelated baselines. + +### 3. Reproduce with the correct Windows configuration + +Use PowerShell and the repository's pinned SDK `11.0.100-rc.1.26420.103`. +Probe `dotnet --version` first. If missing, use `.\eng\common\dotnet.ps1 --version`. +Use that repository SDK thereafter. Restore tools/packages only after an actual missing-dependency failure. +Do not change `global.json`, package versions, or machine-wide environment variables. + +The failed job uses Release, net472, compressed metadata, no-realsig product binaries, and immediate cache eviction. +For desktop commands, set `BUILDING_USING_DOTNET=false` in the current process. +The value `true` removes net472 from the component project's target frameworks. +Do not use `-noVisualStudio` to silently replace desktop coverage with CoreCLR coverage. + +Reproduce the existing job before editing, or retain verifiable equivalent logs for this exact source and configuration: + +```powershell +$env:BUILDING_USING_DOTNET = 'false' +$env:FSharp_CacheEvictionImmediate = 'true' +$env:NativeToolsOnMachine = 'true' +$env:DOTNET_DbgEnableMiniDump = '1' +$env:DOTNET_DbgMiniDumpType = '2' +$env:DOTNET_DbgMiniDumpName = 'Q:\fsharp-worktrees\issue-878\artifacts\log\Release\issue-20410-%e-%p-%t.dmp' +& .\eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktop -configuration Release +``` + +`eng\Build.ps1` sets `FSHARP_REALSIG=false` for this invocation. +If the installed VS MSBuild cannot load the SDK, preserve that failure and invoke `binlog-analysis`. +Then use the supported `-msbuildEngine dotnet` option with the same command, if it builds all required desktop products. +Record this local engine deviation. Do not change CI's engine or claim an exact CI reproduction for a substituted command. +Do not restore incompatible SDK versions or hide build failures. + +After the successful product build, run `tests\fsharp\FSharpSuite.Tests.fsproj` alone with the repository SDK. +Use `-c Release -f net472 --no-build --no-restore`, `FSHARP_REALSIG=false`, and the same cache-eviction setting. +Retain the runner's reports, actual exit code, and duration. The known inventory is 677 cases. +Do not run competing builds/tests in the same artifacts directory during timing comparisons. +A local unsplit pass does not disprove the CI timeout. Compare contention and duration rather than inventing a deterministic failure. + +Preserve logs outside `artifacts` before cleaning task-owned build outputs. +If an owned process locks an assembly, identify its PID and origin before stopping that PID. +Never kill processes by name or delete another worktree's outputs. +Resume incomplete runs instead of repeating completed work merely because an execution window ended. + +### 4. Apply the smallest evidence-supported repair + +The leading candidate is a job-local three-batch conversion in `azure-pipelines-PR.yml`. +Follow the existing `WindowsCompressedMetadata_Desktop` job near line 438. +Reuse `eng\templates\batched-test-steps.yml` and the existing `-testDesktopBatch` option. +Do not add a new splitting algorithm, project, package, script, or test exclusion. + +Keep the job name `WindowsNoRealsig_testDesktop`, its pool/demand, and `timeoutInMinutes: 120`. +Add the existing matrix pattern: + +```yaml +strategy: + matrix: + Batch1: + batchNumber: 1 + Batch2: + batchNumber: 2 + Batch3: + batchNumber: 3 +``` + +Pass this build command to the shared template: + +```text +eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktopBatch $(batchNumber) -configuration Release +``` + +Preserve `FSharp_CacheEvictionImmediate`, all three `DOTNET_Dbg*` variables, and `NativeToolsOnMachine` through `buildEnv`. +Use `testRunTitlePrefix: 'WindowsNoRealsig_testDesktop'` and a distinct artifact prefix such as `'WindowsNoRealsig testDesktop'`. +Enable `publishBinLog` and `publishDumps`, retaining the Release build-binlog path from the original job. +Let the template append `Batch$(batchNumber)` to report/artifact names. +Retain the template's checked-in Azure include syntax. Do not hand-copy its publication tasks into the job. +Do not change sibling jobs, global parallelism, shared timeouts, or `continueOnError` to conceal failures. + +This candidate is conditional on the diagnosis. +If an isolated legacy test or runner defect reproduces, fix that cause surgically with a regression test in this same sprint. +Do not use batching to hide a reproducible assertion failure, hang, or coverage loss. + +### 5. Verify batch coverage and actual Release execution + +Run the existing split validator and inspect all three generated command sets: + +```powershell +dotnet fsi eng\tests\TestSplit.fsx --validate +dotnet fsi eng\tests\TestSplit.fsx 1 desktop +dotnet fsi eng\tests\TestSplit.fsx 2 desktop +dotnet fsi eng\tests\TestSplit.fsx 3 desktop +``` + +The validator checks project registration, not complete test execution. +Use MTP discovery to compare the full desktop test inventory with the union of batch selections. +Compare test identities, including theory rows, and require disjoint batch membership. +Counts alone are insufficient. Confirm every discovered test is assigned exactly once. +Preserve all six test projects from `FSharp.slnx`: component, build, core, service, private scripting, and legacy tests. +Batch1 is residual components plus build tests. Batch2 has remaining components, core, service, and private scripting tests. +Batch3 contains legacy tests alone. Issue tests belong to Batch1. EmittedIL belongs to Batch2. +Retain existing exclusions and skip reasons. Add none. + +Run each proposed CI batch locally, sequentially, with the environment from section 3: + +```powershell +& .\eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktopBatch 1 -configuration Release +& .\eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktopBatch 2 -configuration Release +& .\eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktopBatch 3 -configuration Release +``` + +Apply the documented supported engine option if necessary. +Stop on any nonzero exit code. Archive each batch's results and binlogs before the next invocation can overwrite them. +Record build and test time separately, plus the whole invocation time. +Each whole invocation must finish below 120 minutes locally, with zero test failures and no missing suite results. +Record available CPU and memory when interpreting timing. Local timing cannot guarantee the unchanged CI host's runtime. +If a batch exceeds the budget, investigate before declaring completion. Do not increase the limit. +A successful build, discovery listing, or partial component pass does not satisfy batch execution. + +### 6. Preserve and rerun the issue contract + +Do not add duplicate tests. Existing `Signatures.fs` contains a six-row permutation theory and separately named controls. +The existing shared helpers are `recordSignaturePair`, `assertRecordDiagnostics`, `recordOrderDiagnostic`, and `fieldMismatch`. +They compile `Fsi` plus `FsSource` using `withAdditionalSourceFile`. +Plain `typecheck` ignores the additional implementation source at this revision. +Raw diagnostics preserve duplicate warnings, zero-based columns, message text, source basename, and order. +Do not replace these assertions with presence checks, sorted lists, or deduplicated diagnostics. + +| Existing scenario | Required result | +|---|---| +| Exact `ResolvedConfig`: signature `Config; Settings; EditorConfigFiles; Problems`, implementation `Config; EditorConfigFiles; Problems; Settings` | Exactly FS0312 on the implementation type, no positional FS0193. Keep compact dependent-type definitions. | +| Reduced `A:int; B:string` swap; matching prefix plus three-field cycle; same-type `int` swap | Exactly FS0312. Every invalid permutation still fails compilation. | +| Generic struct with `'T` and `'T list` reversed | Exactly FS0312, retaining generic remapping and representation constraints. | +| Reordering plus different `Obsolete` arguments on same named field | Existing FS1200 warning and FS0312, without positional FS0193. | +| Identical declarations | Successful compilation, no diagnostics. | +| Same-name type, mutability, and less-accessible representation mismatches | Genuine FS0193 with unchanged field, message, and range. No spurious FS0312. | +| Missing, extra, renamed fields | FS0313, FS0311, FS0313 respectively. | +| Aligned nullness difference and prefix-before-permutation | Exactly three FS3261 warnings each. Only the permutation adds FS0312. | +| Union, exception, class fields, duplicate record name | Existing FS0193/FS0036, FS0193/FS0063, successful class compilation, and FS0037 respectively. | + +Retain both record name-map passes and this final predicate in `checkRecordFields`: + +```fsharp +implField.LogicalName = sigField.LogicalName +&& checkField aenv infoReader implTycon sigTycon implField sigField +``` + +Do not use pure name equality, sort fields, accept reordering, suppress shared FS0193, or change diagnostic numbers. +Do not extend the guard to unions, exceptions, or classes. +Keep matching-name checks and their documentation/range effects. Do not refactor recovery or list lengths. + +Preserve original RED/GREEN evidence when available, including seven expected RED assertions caused by positional FS0193. +If unavailable, reconstruct RED using unchanged current tests and only a temporary guard reversal in an isolated task-owned worktree. +Do not remove the fix from the delivery branch or share build outputs between RED and GREEN. +A build/setup failure is not RED. Return to the guarded compiler and rerun unchanged expectations for GREEN. + +After a verified product build, run the issue tests and nearby signature selection in Release/net472 and Release/net11.0. +Use the SDK's MTP syntax, for example: + +```powershell +$env:BUILDING_USING_DOTNET = 'false' +$env:FSHARP_REALSIG = 'false' +$env:FSharp_CacheEvictionImmediate = 'true' +dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Release -f net472 --no-build -- --filter-method "*Issue 20410*" +dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Release -f net472 --no-build -- --filter-class "Conformance.Signatures.*" +``` + +Repeat for `-f net11.0` only after verifying matching Release binaries exist. +If syntax differs, consult local `--help` and adjust syntax without changing the selection. +Run `FieldNotContainedDiagnosticExtendedData 01` from `ErrorMessages\ExtendedDiagnosticDataTests.fs`. +Run `Signature conformance` and `Micro compilation` from `Language\Nullness\NullableRegressionTests.fs`. +These paths are under `tests\FSharp.Compiler.ComponentTests`. +The nearby signature selection includes `AttributeMatching01 - attribute mismatch between signature and implementation`. +The supplied sibling baseline comprises seven rows. Verify their actual discovery, names, and results. +Require nonzero discovery for every selection. Never use stale binaries after compiler edits. + +### 7. Review and commit locally + +If any compiler `.fs` file changes, immediately invoke `fsharp-diagnostics`, then rebuild and rerun affected tests. +Format only changed F# files with `dotnet fantomas `. +A YAML-only repair needs no F# reformatting. Do not format the whole repository. +Validate YAML structure with available repository tooling and compare flags, environment, and expanded batch coverage against the original job. +Do not introduce a validation dependency solely for this small YAML edit. + +Invoke `reviewing-compiler-prs` and the `expert-reviewer` agent for the final local work. +Request review of the preserved record-only behavior and the CI repair's coverage, configuration, runtime evidence, and artifact names. +Keep all review feedback local. Resolve concrete findings and rerun affected validation. +Apply `code-compaction` if new changes introduce duplicated setup or excessive scope. +Invoke `release-notes` to confirm the existing compiler-service entry remains sufficient. Do not duplicate it for CI-only changes. + +Run `git diff --check` and inspect the repair diff against the existing PR head. +Stage only necessary repair files and any justified, regenerated baselines. +Do not commit raw logs, dumps, generated discovery data, or temporary probes. +Commit on `fix/issue-20410`, for example `Isolate no-realsig desktop CI test batches (#20559)`. +Include `Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>` and `Copilot-Session: ` trailers. +Preserve the existing implementation and planning commits. Do not amend, push, open a PR, post a comment, or request a remote rerun. +Report the local commit and validation accurately. Remote checks remain unchanged until an authorized later push. + +## Definition of Done + +- The existing PR branch and full diff were inspected before editing, and the repair extends its history. +- Every failed or canceled CI job has a recorded classification and raw evidence, including the known desktop timeout. +- Competing contention, runner-hang, and Release-regression hypotheses were tested before selecting the repair. +- Local reproduction uses Release/net472, compressed metadata, no-realsig binaries, and immediate cache eviction, with any engine deviation recorded. +- The repair changes only the proven cause and does not increase timeouts, ignore errors, drop tests, or change unrelated jobs. +- If batching is used, all three generated selections preserve each desktop test identity exactly once across all six projects. +- All three proposed desktop batch commands complete locally below 120 minutes each, with zero failures and complete suite results. +- The existing issue regressions and nearby signature tests pass with nonzero discovery in Release/net472 and Release/net11.0. +- Field-extended-data, signature-nullness, micro-compilation, and attribute-matching siblings pass with recorded selections and counts. +- Exact diagnostic lists, invalid-permutation failure, three repeated nullness warnings, and non-record controls remain unchanged. +- Original diagnostic RED evidence is retained or reconstructed without weakening tests, followed by GREEN on the preserved guard. +- No build error, test assertion, or EmittedIL baseline mismatch remains unresolved; any necessary baseline regeneration is committed and passes without update mode. +- Existing compiler and release-note changes remain intact unless a reproduced failure requires a surgical correction. +- Changed compiler files pass fsharp-diagnostics and rebuilt tests; only changed F# files are formatted, if any. +- YAML structure, preserved environment, distinct report/artifact names, and git diff --check pass. +- Local expert review is complete, concrete findings are resolved, and affected validation is rerun. +- Commands, source revisions, timing, exit codes, test results, and pending-state history persist outside implementation commits. +- The verified repair is committed locally on fix/issue-20410 with required trailers, with no push or other remote mutation. From 77b6dda5de206920566c04d80c9d459b53c79976 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 16 Sep 2026 06:59:16 +0200 Subject: [PATCH 5/6] Isolate no-realsig desktop CI test batches (#20559) Reuse the existing three-batch test template to avoid running the legacy desktop suite alongside the other test projects. Preserve the 120-minute timeout, no-realsig and metadata flags, environment, and compiler regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f0d4ef4c-beb9-46d3-811a-38e35489e05f --- azure-pipelines-PR.yml | 61 +++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 39 deletions(-) diff --git a/azure-pipelines-PR.yml b/azure-pipelines-PR.yml index 9a359b41c23..8de979f4031 100644 --- a/azure-pipelines-PR.yml +++ b/azure-pipelines-PR.yml @@ -298,46 +298,29 @@ stages: name: $(DncEngPublicBuildPool) demands: ImageOverride -equals $(_WindowsMachineQueueName) timeoutInMinutes: 120 + strategy: + matrix: + Batch1: + batchNumber: 1 + Batch2: + batchNumber: 2 + Batch3: + batchNumber: 3 steps: - - checkout: self - clean: true - - - script: eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktop -configuration Release - env: - FSharp_CacheEvictionImmediate: true - DOTNET_DbgEnableMiniDump: 1 - DOTNET_DbgMiniDumpType: 2 # 1=mini, 2=heap, 3=triage, 4=full. Heap dumps include managed object data for debugging. - DOTNET_DbgMiniDumpName: $(Build.SourcesDirectory)\artifacts\log\Release\$(Build.BuildId)-%e-%p-%t.dmp - NativeToolsOnMachine: true - displayName: Build - - - task: PublishTestResults@2 - displayName: Publish Test Results - inputs: - testResultsFormat: 'XUnit' - testRunTitle: WindowsNoRealsig_testDesktop - mergeTestResults: true - testResultsFiles: '*.xml' - searchFolder: '$(Build.SourcesDirectory)/artifacts/TestResults/Release' - condition: succeededOrFailed() - continueOnError: true - - task: PublishBuildArtifacts@1 - displayName: Publish Build BinLog - continueOnError: true - inputs: - PathToPublish: '$(Build.SourcesDirectory)\artifacts\log/Release\Build.VisualFSharp.slnx.binlog' - ArtifactName: 'Windows Release build binlogs' - ArtifactType: Container - parallel: true - - task: PublishBuildArtifacts@1 - displayName: Publish Dumps - condition: failed() - continueOnError: true - inputs: - PathToPublish: '$(Build.SourcesDirectory)\artifacts\log\Release' - ArtifactName: 'Windows Release WindowsNoRealsig_testDesktop process dumps' - ArtifactType: Container - parallel: true + - template: /eng/templates/batched-test-steps.yml + parameters: + buildCommand: eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktopBatch $(batchNumber) -configuration Release + buildEnv: + FSharp_CacheEvictionImmediate: true + DOTNET_DbgEnableMiniDump: 1 + DOTNET_DbgMiniDumpType: 2 + DOTNET_DbgMiniDumpName: $(Build.SourcesDirectory)\artifacts\log\Release\$(Build.BuildId)-%e-%p-%t.dmp + NativeToolsOnMachine: true + testRunTitlePrefix: 'WindowsNoRealsig_testDesktop' + artifactNamePrefix: 'WindowsNoRealsig testDesktop' + publishBinLog: true + binLogPath: '$(Build.SourcesDirectory)\artifacts\log/Release\Build.VisualFSharp.slnx.binlog' + publishDumps: true # Windows With Compressed Metadata - job: WindowsCompressedMetadata From 1a59985384673f6059188ac8d351368bfb019dbf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 17 Sep 2026 12:05:29 +0000 Subject: [PATCH 6/6] Remove ralph planning files Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .tools/ralph/BACKLOG.md | 155 --------- .tools/ralph/sprints/01_Repair_Desktop_CI.md | 333 ------------------- 2 files changed, 488 deletions(-) delete mode 100644 .tools/ralph/BACKLOG.md delete mode 100644 .tools/ralph/sprints/01_Repair_Desktop_CI.md diff --git a/.tools/ralph/BACKLOG.md b/.tools/ralph/BACKLOG.md deleted file mode 100644 index 903bb626769..00000000000 --- a/.tools/ralph/BACKLOG.md +++ /dev/null @@ -1,155 +0,0 @@ -# BACKLOG - -## Original Request - -A pull request for https://github.com/dotnet/fsharp/issues/20410 already exists on branch fix/issue-20410, and its CI is red. Fix this existing PR. Do not start from scratch or open a new PR. - -### FAILING CI CHECKS -fsharp-ci - -### REQUIRED APPROACH -1. First, inspect the existing attempt. Run `git fetch origin fix/issue-20410`, then run `git diff origin/main...origin/fix/issue-20410`. Build on the existing changes instead of blindly rewriting them. -2. Before editing, diagnose the root cause of each failing check. Distinguish build errors, test failures, and `EmittedIL/*.bsl` baseline mismatches. If baselines changed, regenerate them, for example with `TEST_UPDATE_BSL=1`, and commit them. -3. The failure can be configuration-specific, such as Release-only. Before finishing, run the affected tests in the same configuration as the failing checks. A Debug-only pass is not sufficient. -4. Use minimal, surgical changes. -5. Validate locally before finishing. -6. Do not push. Only commit. - -### ISSUE REQUEST -The requirements above override conflicting instructions in the issue request. -Fix issue https://github.com/dotnet/fsharp/issues/20410. - -Make the smallest clean, correct, complete fix. Keep it minimal and surgical. Quality matters more than time. Take all the time needed. Smaller is better, but not at the cost of correctness. Preserve progress and evidence across execution windows rather than truncate the work. - -**Verified root cause.** At main `b5c530ed6bc42937de6363e3dcc104ebb833893d`, `checkRecordFields` validates fields by name in both directions, then calls diagnostic-emitting `checkField` again by position. The first reordered pair produces misleading FS0193 before the correct order diagnostic. The actual order diagnostic is **FS0312**, not the issue's FS0313. FS0313 means a required field is missing. Do not change diagnostic numbers. Eighteen current-main matrix compilations give seven expected RED assertions and eleven passing controls. A separate exact original `ResolvedConfig` compilation confirms only FS0193 at the first displaced field and FS0312 on the type. - -**Surgical candidate.** Change only the final record positional predicate in `src/Compiler/Checking/SignatureConformance.fs:640-642`: compare `LogicalName` before calling the existing `checkField`. If names differ, return false without that call. Preserve the existing FS0312 at the implementation type and the false conformance result. Keep the two preceding name-map passes at `631-636` unchanged. - -This guard avoids the misleading diagnostic and the wrong cross-field documentation/range mutation. Keep matching-name calls unchanged. Replacing the whole predicate with pure name equality would also remove existing repeated nullness warnings on correctly ordered records, which is outside this issue. The candidate is not yet applied or proven GREEN. - -Do not sort fields, accept reordered records, suppress shared FS0193, alter diagnostic resources, or broaden the guard to unions, exceptions, or classes. Same-name type, mutability, accessibility, and attribute checks must still run. Constructor parameter order depends on record declaration order. - -**RED-first test plan.** Use `tests/FSharp.Compiler.ComponentTests/Conformance/Signatures/Signatures.fs` and the paired-source compile pipeline. A plain `typecheck` call does not consume `AdditionalSources` at this revision. Use the existing `Fsi` plus implementation-source helper and `compile`, or a demonstrated project-typecheck path. Assert the complete diagnostic list, including code, severity, message, and range. GREEN means correct diagnostics while the invalid permutation still fails compilation. - -| Scenario | Required assertion | -|---|---| -| 1. Exact reported `ResolvedConfig` field permutation, with compact local definitions for its dependent types | Exactly FS0312 on `ResolvedConfig`, no positional FS0193. Also use the reduced two-field swap to isolate the cause. | -| 2. A shared correctly ordered prefix followed by a three-field cycle | Exactly FS0312 on the type, no positional FS0193. | -| 3. Swapped fields with the same `int` type | Exactly FS0312. Equal field types do not make declaration order legal. | -| 4. Generic struct record with `'T` and `'T list` fields swapped | Exactly FS0312. Preserve generic remapping and the representation constraint. | -| 5. Permutation plus conflicting attribute arguments on the same named field | Keep FS1200 from attribute reconciliation and FS0312. Remove only positional FS0193. | -| 6. Identical names, declarations, and order | Successful compilation without diagnostics. | -| 7. Real same-name mismatch | Parameterize changed type, changed mutability, and less-accessible representation. Keep genuine FS0193 identifying the same field, without a spurious order error. | -| 8. Different name sets | Missing, extra, and renamed fields keep FS0313, FS0311, and FS0313 respectively. Equal counts alone do not establish a permutation. | -| 9. Warning-only nullness differences | Keep existing same-name warnings. The observed aligned and prefix-before-permutation cases each have three FS3261 warnings. The prefix case adds FS0312 but loses positional FS0193. Do not make warning cleanup part of this change. | -| 10. Non-record and recovery boundaries | Reuse union, exception, object-field, and malformed-field controls. Union and exception diagnostics stay unchanged. Reordered class fields can still compile. Do not refactor recovered duplicate names or list lengths. | - -Rows 1-5 provide the primary bug and four meaningful RED variants. Current-main reduced forms already fail the intended expectations. Preserve the exact issue case as a compact fixture too. Rows 6-10 are controls, not additional before-fix failure claims. Use one data-driven regression test and separately named controls. Share source-pair construction; do not copy many near-identical files or test bodies. - -First prove RED from the unwanted diagnostic. Then make the local guard and obtain GREEN without weakening tests, changing order-error expectations, suppressing warnings, or refreshing unrelated baselines. Existing field-extended-data, signature-nullness, and attribute-matching sibling selections passed seven rows on the starting main build. Run these and the nearby signature-conformance tests after implementation. - -Format only changed F# files. Invoke `fsharp-diagnostics` after compiler edits and invoke the expert-review skill on the final work. Remove noise and duplicate setup using existing helpers. Leave a clean, compact suite and concise release note. No API change, new diagnostic, extra name set, general suppression flag, or broad conformance refactor is needed. - -Sources: [issue](https://github.com/dotnet/fsharp/issues/20410), [record-specific checks](https://github.com/dotnet/fsharp/blob/b5c530ed6bc42937de6363e3dcc104ebb833893d/src/Compiler/Checking/SignatureConformance.fs#L624-L655), [field checks and side effects](https://github.com/dotnet/fsharp/blob/b5c530ed6bc42937de6363e3dcc104ebb833893d/src/Compiler/Checking/SignatureConformance.fs#L559-L623), [diagnostic identities](https://github.com/dotnet/fsharp/blob/b5c530ed6bc42937de6363e3dcc104ebb833893d/src/Compiler/FSComp.txt#L149-L151). - -## Analysis - -This delivery replaces an obsolete implementation plan with one self-contained CI repair sprint. -It does not implement the CI repair or claim local compiler/test success. -The requested template was read before creating the replacement sprint. - -### Existing attempt and branch state - -- Ran `git fetch origin fix/issue-20410`, then `git diff origin/main...origin/fix/issue-20410`, before planning changes. -- Existing PR: [#20559](https://github.com/dotnet/fsharp/pull/20559), open, titled "Fix misleading diagnostics for reordered record fields". -- Existing remote head: `5c489dfdb967585f90847a5d6fe7f536d03163cf`. Implementation commit: `2bbf4d6d41f8d6216a59598f23c64a65133e7f17`. -- The local branch initially pointed to `b5c530ed6bc42937de6363e3dcc104ebb833893d`, also the local `origin/main`. -- With a clean tracked worktree, ran `git merge --ff-only origin/fix/issue-20410`. Planning now extends the existing PR history. -- Existing product diff: the record-only logical-name guard, 146 test lines, and one compiler-service release note. -- The source already retains matching-name `checkField` calls, both name-map passes, FS0312, and failed conformance. -- The test suite already has all requested scenarios, shared paired-source construction, and raw exact diagnostic assertions. -- The release note already links both #20410 and #20559. Do not add a duplicate or remove the PR link. -- The old sprint incorrectly says no PR exists and the guard has not been applied. Replace it, rather than leave it executable. - -### Verified CI evidence - -Build [1597638](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1597638), number `20260915.39`, tested merge SHA `5604d594ed08aa786661166a3fffd1811db0e471`. -The current PR head is not that synthetic merge SHA. -The timeline has 48 jobs: 47 succeeded, one canceled. The aggregate `fsharp-ci` check failed because of that cancellation. - -| Surface | Observed result | Classification | -|---|---|---| -| `WindowsNoRealsig_testDesktop`, job `916a2273-64f0-5130-a29e-a4d2f7e48c60` | Agent exceeded the configured 120-minute limit, 15:07:05Z to 17:07:26Z on September 15 | Job timeout, not an observed assertion failure | -| Build task, log 861 | Build summaries show zero errors; solution-wide net472 tests start at 15:27:44Z | No observed compilation failure | -| Component suite in log 861 | 8,284 passed, zero failed, 627 skipped; finished at 16:44:04Z after 76m12s | Successful suite inside canceled job | -| Core and service suites in log 861 | Core: 6,212 passed, 5 skipped; service: 3,487 passed, 306 skipped; both zero failures | Successful suites | -| Legacy `FSharpSuite.Tests` | No completion summary in the canceled job | Remaining workload or runner-lifetime investigation | -| Agent resource warnings | 95.69% memory used at 16:26:44Z and 16:26:49Z | Evidence supporting contention, not proof of a deadlock | -| `WindowsCompressedMetadata_Desktop Batch3`, task log 848 | Isolated legacy suite: 677 total, zero failed; job completed in about 91m18s | Existing isolation pattern succeeds | -| Desktop Batch1 / Batch2 | Jobs completed in about 50m11s / 38m56s | Existing three-batch pattern available | -| `WindowsNoRealsig_testCoreclr`, formatting, ILVerify | All succeeded | No justification to alter these surfaces | -| `EmittedIL` baselines | No observed mismatch in the retrieved failing-task log; component suite passed | Do not regenerate speculatively | -| Test results and binlog publication in canceled job | Those explicit tasks were skipped | Missing artifacts are not proof of passing tests | - -The build-status script reports zero build errors and test failures because it filters `failed` tasks, not this `canceled` task. -The raw timeline and canceled-task log are the decisive evidence. -Do not present the script's empty result as a clean CI run. - -Public evidence endpoints use `https://dev.azure.com/dnceng-public/public/_apis/build/builds/1597638`. -Append `/timeline?api-version=7.1`, `/logs/861?api-version=7.1`, or `/logs/848?api-version=7.1`. -The sprint embeds the essential evidence and does not depend on this backlog or access to another session. - -### Competing hypotheses and prior progress - -| Hypothesis | Evidence and next verification | -|---|---| -| Concurrent desktop suites exceed the job budget under memory pressure | Supported by resource warnings and isolated legacy success. Compare unsplit and isolated local runs with identical binaries and settings. | -| A legacy test or runner teardown hangs | No completion result alone cannot distinguish a hang from slowness. Record progress, exit status, child processes, and a dump if progress stops. | -| Release/compiler regression or IL baseline drift causes the failure | No CI assertion or build error supports this. Run the existing regressions and actual desktop batches before dismissing it. | - -A bounded history lookup found session `83fb7dbc-211a-47df-8f32-557feaf219c2`. -It reports an unsplit local pass in 97m48s and an isolated legacy pass in 69m23s, with 677 passing tests. -It also reports that the VS engine could not load the SDK, while `-msbuildEngine dotnet` built successfully. -Its last available response says proposed Batch1 validation was blocked by a leftover MSBuild assembly lock. -Batch2, Batch3, and dedicated desktop/net11 signature reruns were still pending in that response. -These are historical reports, not fresh verified logs or evidence that the proposed repair is complete. -Recover matching logs if available. Otherwise rerun the missing evidence without resetting completed source work. - -### Repository constraints that matter - -- `azure-pipelines-PR.yml` contains the failing job near line 296 and the working desktop matrix near line 438. -- Reuse `eng\templates\batched-test-steps.yml`, `eng\Build.ps1`'s `-testDesktopBatch`, and `eng\tests\TestSplit.fsx`. -- `TestUsingMSBuild` already supplies net472, xUnit reports, binlogs, and five-minute hang dumps. -- Batch1 has residual component tests plus build tests. Batch2 has the remaining components, core, service, and scripting tests. -- Batch3 isolates `tests\fsharp\FSharpSuite.Tests.fsproj`. Preserve complete, nonoverlapping coverage of all six desktop test projects. -- `BUILDING_USING_DOTNET=true` removes net472 from component-project target frameworks. Override it only in the current process for desktop validation. -- The pinned SDK is `11.0.100-rc.1.26420.103`. The other component target is `net11.0`. -- Use supported SDK/build-engine setup. Do not change SDK versions or shared build scripts to hide a local tooling failure. - -## Approach - -Use one complete vertical repair sprint, including diagnosis, the smallest supported repair, its tests, review, and a local commit. -The leading candidate changes only the `WindowsNoRealsig_testDesktop` job to use the existing three-batch mechanism. -Keep the job's flags, environment, pool, and 120-minute per-job limit. Preserve distinct test and artifact names. -Make that change only after diagnosis supports workload isolation. Investigate a reproducible assertion failure or deadlock instead, if found. - -Do not rerun the old implementation sprint or add duplicate regression tests. -Preserve original RED evidence when available. If it is missing, recover it with unchanged regression assertions and an isolated, temporary guard reversal. -Do not leave that reversal in the repair branch or mistake a setup failure for RED. -Require fresh Release/net472 regression and batch execution, followed by targeted Release/net11 checks. -Compare discovered test identities across batches, not counts alone. Discovery is not execution. -Regenerate only proven affected `EmittedIL` baselines and rerun without update mode before staging them. - -Keep logs and resumable state under `.tools\ralph\evidence\issue-20410-ci`, outside implementation commits. -The implementation verifier must inspect exact commands, source SHA, flags, test counts, timings, exit codes, and unresolved limitations. -Local passes cannot make an unpushed GitHub check green. The final report must distinguish local validation from the unchanged remote check. - -Planning verification checks the requested document structure, one active sprint, source paths, preserved request, and whitespace. -The planning commit changes only the backlog and the replacement sprint, retaining the existing compiler/test/release-note commits. -No build or test execution is claimed for this documentation-only planning pass. - -## Sprint Overview - -| # | Name | Purpose | -|---|---|---| -| 01 | Repair Desktop CI | Diagnose the existing PR timeout, apply the smallest proven repair, preserve all regression coverage, validate Release desktop batches, review, and commit without pushing. | diff --git a/.tools/ralph/sprints/01_Repair_Desktop_CI.md b/.tools/ralph/sprints/01_Repair_Desktop_CI.md deleted file mode 100644 index d06266797b6..00000000000 --- a/.tools/ralph/sprints/01_Repair_Desktop_CI.md +++ /dev/null @@ -1,333 +0,0 @@ ---- ---- -# Sprint: Repair the existing record-field PR's desktop CI - -## Context - WHY this sprint exists - -Repair existing PR [#20559](https://github.com/dotnet/fsharp/pull/20559) for issue [#20410](https://github.com/dotnet/fsharp/issues/20410). -Work in `Q:\fsharp-worktrees\issue-878` on `fix/issue-20410`. -This is one complete implementation unit, including diagnosis, repair, tests, review, and a local commit. -You do not need another sprint, the backlog, or another agent's history. -Do not start over, open another PR, push, post reviews, amend commits, or change remote settings. - -The PR already contains the compiler guard, regression tests, and release note. -Its remote head at planning was `5c489dfdb967585f90847a5d6fe7f536d03163cf`. -The compiler/test implementation is commit `2bbf4d6d41f8d6216a59598f23c64a65133e7f17`. -The architect fast-forwarded this local branch from main `b5c530ed6bc42937de6363e3dcc104ebb833893d` to the existing PR head. -Planning commits can follow that head. Preserve them and any completed repair work. - -### Verified failure, not an assumed diagnostic regression - -CI build [1597638](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1597638) tested synthetic merge `5604d594ed08aa786661166a3fffd1811db0e471`. -Its 48 jobs comprise 47 successes and one cancellation. -The failed aggregate `fsharp-ci` check comes from `WindowsNoRealsig_testDesktop`. -Job ID: `916a2273-64f0-5130-a29e-a4d2f7e48c60`. - -| Evidence | Observed result | -|---|---| -| Timeline | Agent exceeded 120 minutes, September 15, 2026, 15:07:05Z to 17:07:26Z. | -| Canceled Build task, log 861 | Build summaries have zero errors. Solution-wide Release/net472 tests begin at 15:27:44Z. | -| Component suite | Completed at 16:44:04Z: 8,284 passed, zero failed, 627 skipped, duration 76m12s. | -| Core and service suites | Core: 6,212 passed, 5 skipped. Service: 3,487 passed, 306 skipped. Both have zero failures. | -| Legacy `FSharpSuite.Tests` | No completion summary before cancellation. This does not distinguish slow tests from a runner hang. | -| Memory warnings | 95.69% memory used at 16:26:44Z and 16:26:49Z. | -| Existing desktop Batch3, log 848 | Isolated legacy suite completed 677 tests, zero failures. Whole job took about 91m18s. | -| Existing desktop Batch1 / Batch2 | Successful jobs took about 50m11s / 38m56s. | -| Other checks | No-realsig CoreCLR, formatting, and ILVerify succeeded. No failing-task `EmittedIL` baseline mismatch was observed. | - -The build-status script filters failed tasks and can miss canceled tasks. Read the raw canceled-task log as well. -Explicit test-result and binlog publication tasks were skipped after cancellation. Do not interpret missing results as passes. - -A prior execution reported local unsplit success in 97m48s and isolated legacy success in 69m23s. -That execution reported a VS/SDK incompatibility, then successful builds with the supported `-msbuildEngine dotnet` option. -Its final report left proposed Batch1 blocked by an MSBuild assembly lock, with other batch and signature reruns unfinished. -These reports are leads, not accepted validation. Reuse matching raw evidence if available, or reproduce it. -No local test or repair execution occurred during this planning pass. - -## Description - WHAT to implement with DETAILED guidance - -### 1. Inspect and preserve the existing attempt - -Run these commands before editing implementation files: - -```powershell -Set-Location Q:\fsharp-worktrees\issue-878 -git fetch origin fix/issue-20410 -git --no-pager diff origin/main...origin/fix/issue-20410 -git status --short -git --no-pager log -6 --oneline -gh pr view 20559 --repo dotnet/fsharp --json headRefOid,statusCheckRollup -``` - -Confirm the existing PR history is an ancestor of local HEAD. -If behind and clean, fast-forward with `git merge --ff-only origin/fix/issue-20410`. -Never reset local repair commits or overwrite another agent's edits. -Keep evidence under `.tools\ralph\evidence\issue-20410-ci`, outside committed implementation files. -Record source SHA, commands, configuration, environment, exit codes, durations, test counts, and pending work. -Use distinct filenames for unsplit, isolated, Batch1, Batch2, Batch3, and signature runs. - -| File, relative to the worktree | Required use | -|---|---| -| `azure-pipelines-PR.yml` | Primary candidate edit: `WindowsNoRealsig_testDesktop`, initially near line 296. | -| `eng\templates\batched-test-steps.yml` | Reuse the existing template. Read its parameters and publication conditions. | -| `eng\Build.ps1` | Read `TestUsingMSBuild`, `BuildSolution`, and the `testDesktopBatch` branch. Do not add another runner. | -| `eng\tests\TestSplit.fsx` | Reuse the existing three-batch assignments and `--validate`. Do not change the split without evidence. | -| `eng\CIBuildNoPublish.cmd` | Existing CI entry point, including restore, bootstrap, build, pack, sign, and binary logging. | -| `FSharp.slnx` | Source of the unsplit desktop test-project inventory. | -| `src\Compiler\Checking\SignatureConformance.fs` | Preserve the existing record-only `checkRecordFields` guard. | -| `tests\FSharp.Compiler.ComponentTests\Conformance\Signatures\Signatures.fs` | Preserve and run the existing issue tests in `Conformance.Signatures.SignatureConformance`. | -| `tests\FSharp.Test.Utilities\Compiler.fs` | Existing paired-source and raw diagnostic behavior. Do not change the shared harness. | -| `docs\release-notes\.FSharp.Compiler.Service\11.0.100.md` | Preserve the existing concise entry linking #20410 and #20559. | - -Read `.github\instructions\ExpertReview.instructions.md`, `ComponentTests.instructions.md`, and `NoBloat.instructions.md` before any corresponding F# edits. -Those three files are under `.github\instructions`. -Read `eng\common\AGENTS.md` if an edit there becomes necessary. The proposed repair does not edit that directory. - -### 2. Diagnose every failing check before choosing a repair - -Invoke `pr-build-status` and `hypothesis-driven-debugging`. -Refresh all platforms and jobs, not just the first red check. -For the known build, retrieve: - -```powershell -pwsh .github\skills\pr-build-status\scripts\Get-BuildInfo.ps1 -BuildId 1597638 -pwsh .github\skills\pr-build-status\scripts\Get-BuildErrors.ps1 -BuildId 1597638 -$base = 'https://dev.azure.com/dnceng-public/public/_apis/build/builds/1597638' -Invoke-RestMethod "$base/timeline?api-version=7.1" -Invoke-RestMethod "$base/logs/861?api-version=7.1" -Invoke-RestMethod "$base/logs/848?api-version=7.1" -``` - -Save raw output and a `CI_ERRORS.md` under the evidence directory. -Classify each failure as build/restore, test assertion, baseline mismatch, timeout, or cancellation. -If a newer build exists, update the evidence instead of treating these IDs as permanently current. - -Test these competing hypotheses before editing: - -| Hypothesis | Verification | -|---|---| -| Unsplit concurrent desktop suites cause excessive runtime and memory pressure | Compare the unsplit command with isolated legacy execution on identical Release binaries and settings. Record process and memory observations. | -| A legacy case or runner teardown hangs | Check progress, case duration, process exit, and remaining child processes. If progress stops, inspect the existing hang dump or capture the specific owned process. | -| Release conformance or emitted-code behavior regressed | Run existing issue assertions and inspect actual failing test output. Separate expected negative-test diagnostics from runner failures. | - -Invoke `binlog-analysis` for actual build, restore, or WarnAsError errors. -A timeout alone does not justify changing the compiler or diagnostic expectations. -If a test fails, reproduce that exact case in isolation before editing it. -If `EmittedIL\*.bsl` differs, inspect expected versus actual IL and identify the semantic cause. -Regenerate only affected baselines with process-local `TEST_UPDATE_BSL=1`, then remove that variable and rerun. -Commit generated baseline changes only when the semantic change is intended. Do not refresh unrelated baselines. - -### 3. Reproduce with the correct Windows configuration - -Use PowerShell and the repository's pinned SDK `11.0.100-rc.1.26420.103`. -Probe `dotnet --version` first. If missing, use `.\eng\common\dotnet.ps1 --version`. -Use that repository SDK thereafter. Restore tools/packages only after an actual missing-dependency failure. -Do not change `global.json`, package versions, or machine-wide environment variables. - -The failed job uses Release, net472, compressed metadata, no-realsig product binaries, and immediate cache eviction. -For desktop commands, set `BUILDING_USING_DOTNET=false` in the current process. -The value `true` removes net472 from the component project's target frameworks. -Do not use `-noVisualStudio` to silently replace desktop coverage with CoreCLR coverage. - -Reproduce the existing job before editing, or retain verifiable equivalent logs for this exact source and configuration: - -```powershell -$env:BUILDING_USING_DOTNET = 'false' -$env:FSharp_CacheEvictionImmediate = 'true' -$env:NativeToolsOnMachine = 'true' -$env:DOTNET_DbgEnableMiniDump = '1' -$env:DOTNET_DbgMiniDumpType = '2' -$env:DOTNET_DbgMiniDumpName = 'Q:\fsharp-worktrees\issue-878\artifacts\log\Release\issue-20410-%e-%p-%t.dmp' -& .\eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktop -configuration Release -``` - -`eng\Build.ps1` sets `FSHARP_REALSIG=false` for this invocation. -If the installed VS MSBuild cannot load the SDK, preserve that failure and invoke `binlog-analysis`. -Then use the supported `-msbuildEngine dotnet` option with the same command, if it builds all required desktop products. -Record this local engine deviation. Do not change CI's engine or claim an exact CI reproduction for a substituted command. -Do not restore incompatible SDK versions or hide build failures. - -After the successful product build, run `tests\fsharp\FSharpSuite.Tests.fsproj` alone with the repository SDK. -Use `-c Release -f net472 --no-build --no-restore`, `FSHARP_REALSIG=false`, and the same cache-eviction setting. -Retain the runner's reports, actual exit code, and duration. The known inventory is 677 cases. -Do not run competing builds/tests in the same artifacts directory during timing comparisons. -A local unsplit pass does not disprove the CI timeout. Compare contention and duration rather than inventing a deterministic failure. - -Preserve logs outside `artifacts` before cleaning task-owned build outputs. -If an owned process locks an assembly, identify its PID and origin before stopping that PID. -Never kill processes by name or delete another worktree's outputs. -Resume incomplete runs instead of repeating completed work merely because an execution window ended. - -### 4. Apply the smallest evidence-supported repair - -The leading candidate is a job-local three-batch conversion in `azure-pipelines-PR.yml`. -Follow the existing `WindowsCompressedMetadata_Desktop` job near line 438. -Reuse `eng\templates\batched-test-steps.yml` and the existing `-testDesktopBatch` option. -Do not add a new splitting algorithm, project, package, script, or test exclusion. - -Keep the job name `WindowsNoRealsig_testDesktop`, its pool/demand, and `timeoutInMinutes: 120`. -Add the existing matrix pattern: - -```yaml -strategy: - matrix: - Batch1: - batchNumber: 1 - Batch2: - batchNumber: 2 - Batch3: - batchNumber: 3 -``` - -Pass this build command to the shared template: - -```text -eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktopBatch $(batchNumber) -configuration Release -``` - -Preserve `FSharp_CacheEvictionImmediate`, all three `DOTNET_Dbg*` variables, and `NativeToolsOnMachine` through `buildEnv`. -Use `testRunTitlePrefix: 'WindowsNoRealsig_testDesktop'` and a distinct artifact prefix such as `'WindowsNoRealsig testDesktop'`. -Enable `publishBinLog` and `publishDumps`, retaining the Release build-binlog path from the original job. -Let the template append `Batch$(batchNumber)` to report/artifact names. -Retain the template's checked-in Azure include syntax. Do not hand-copy its publication tasks into the job. -Do not change sibling jobs, global parallelism, shared timeouts, or `continueOnError` to conceal failures. - -This candidate is conditional on the diagnosis. -If an isolated legacy test or runner defect reproduces, fix that cause surgically with a regression test in this same sprint. -Do not use batching to hide a reproducible assertion failure, hang, or coverage loss. - -### 5. Verify batch coverage and actual Release execution - -Run the existing split validator and inspect all three generated command sets: - -```powershell -dotnet fsi eng\tests\TestSplit.fsx --validate -dotnet fsi eng\tests\TestSplit.fsx 1 desktop -dotnet fsi eng\tests\TestSplit.fsx 2 desktop -dotnet fsi eng\tests\TestSplit.fsx 3 desktop -``` - -The validator checks project registration, not complete test execution. -Use MTP discovery to compare the full desktop test inventory with the union of batch selections. -Compare test identities, including theory rows, and require disjoint batch membership. -Counts alone are insufficient. Confirm every discovered test is assigned exactly once. -Preserve all six test projects from `FSharp.slnx`: component, build, core, service, private scripting, and legacy tests. -Batch1 is residual components plus build tests. Batch2 has remaining components, core, service, and private scripting tests. -Batch3 contains legacy tests alone. Issue tests belong to Batch1. EmittedIL belongs to Batch2. -Retain existing exclusions and skip reasons. Add none. - -Run each proposed CI batch locally, sequentially, with the environment from section 3: - -```powershell -& .\eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktopBatch 1 -configuration Release -& .\eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktopBatch 2 -configuration Release -& .\eng\CIBuildNoPublish.cmd -compressallmetadata -buildnorealsig -testDesktopBatch 3 -configuration Release -``` - -Apply the documented supported engine option if necessary. -Stop on any nonzero exit code. Archive each batch's results and binlogs before the next invocation can overwrite them. -Record build and test time separately, plus the whole invocation time. -Each whole invocation must finish below 120 minutes locally, with zero test failures and no missing suite results. -Record available CPU and memory when interpreting timing. Local timing cannot guarantee the unchanged CI host's runtime. -If a batch exceeds the budget, investigate before declaring completion. Do not increase the limit. -A successful build, discovery listing, or partial component pass does not satisfy batch execution. - -### 6. Preserve and rerun the issue contract - -Do not add duplicate tests. Existing `Signatures.fs` contains a six-row permutation theory and separately named controls. -The existing shared helpers are `recordSignaturePair`, `assertRecordDiagnostics`, `recordOrderDiagnostic`, and `fieldMismatch`. -They compile `Fsi` plus `FsSource` using `withAdditionalSourceFile`. -Plain `typecheck` ignores the additional implementation source at this revision. -Raw diagnostics preserve duplicate warnings, zero-based columns, message text, source basename, and order. -Do not replace these assertions with presence checks, sorted lists, or deduplicated diagnostics. - -| Existing scenario | Required result | -|---|---| -| Exact `ResolvedConfig`: signature `Config; Settings; EditorConfigFiles; Problems`, implementation `Config; EditorConfigFiles; Problems; Settings` | Exactly FS0312 on the implementation type, no positional FS0193. Keep compact dependent-type definitions. | -| Reduced `A:int; B:string` swap; matching prefix plus three-field cycle; same-type `int` swap | Exactly FS0312. Every invalid permutation still fails compilation. | -| Generic struct with `'T` and `'T list` reversed | Exactly FS0312, retaining generic remapping and representation constraints. | -| Reordering plus different `Obsolete` arguments on same named field | Existing FS1200 warning and FS0312, without positional FS0193. | -| Identical declarations | Successful compilation, no diagnostics. | -| Same-name type, mutability, and less-accessible representation mismatches | Genuine FS0193 with unchanged field, message, and range. No spurious FS0312. | -| Missing, extra, renamed fields | FS0313, FS0311, FS0313 respectively. | -| Aligned nullness difference and prefix-before-permutation | Exactly three FS3261 warnings each. Only the permutation adds FS0312. | -| Union, exception, class fields, duplicate record name | Existing FS0193/FS0036, FS0193/FS0063, successful class compilation, and FS0037 respectively. | - -Retain both record name-map passes and this final predicate in `checkRecordFields`: - -```fsharp -implField.LogicalName = sigField.LogicalName -&& checkField aenv infoReader implTycon sigTycon implField sigField -``` - -Do not use pure name equality, sort fields, accept reordering, suppress shared FS0193, or change diagnostic numbers. -Do not extend the guard to unions, exceptions, or classes. -Keep matching-name checks and their documentation/range effects. Do not refactor recovery or list lengths. - -Preserve original RED/GREEN evidence when available, including seven expected RED assertions caused by positional FS0193. -If unavailable, reconstruct RED using unchanged current tests and only a temporary guard reversal in an isolated task-owned worktree. -Do not remove the fix from the delivery branch or share build outputs between RED and GREEN. -A build/setup failure is not RED. Return to the guarded compiler and rerun unchanged expectations for GREEN. - -After a verified product build, run the issue tests and nearby signature selection in Release/net472 and Release/net11.0. -Use the SDK's MTP syntax, for example: - -```powershell -$env:BUILDING_USING_DOTNET = 'false' -$env:FSHARP_REALSIG = 'false' -$env:FSharp_CacheEvictionImmediate = 'true' -dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Release -f net472 --no-build -- --filter-method "*Issue 20410*" -dotnet test --project tests\FSharp.Compiler.ComponentTests\FSharp.Compiler.ComponentTests.fsproj -c Release -f net472 --no-build -- --filter-class "Conformance.Signatures.*" -``` - -Repeat for `-f net11.0` only after verifying matching Release binaries exist. -If syntax differs, consult local `--help` and adjust syntax without changing the selection. -Run `FieldNotContainedDiagnosticExtendedData 01` from `ErrorMessages\ExtendedDiagnosticDataTests.fs`. -Run `Signature conformance` and `Micro compilation` from `Language\Nullness\NullableRegressionTests.fs`. -These paths are under `tests\FSharp.Compiler.ComponentTests`. -The nearby signature selection includes `AttributeMatching01 - attribute mismatch between signature and implementation`. -The supplied sibling baseline comprises seven rows. Verify their actual discovery, names, and results. -Require nonzero discovery for every selection. Never use stale binaries after compiler edits. - -### 7. Review and commit locally - -If any compiler `.fs` file changes, immediately invoke `fsharp-diagnostics`, then rebuild and rerun affected tests. -Format only changed F# files with `dotnet fantomas `. -A YAML-only repair needs no F# reformatting. Do not format the whole repository. -Validate YAML structure with available repository tooling and compare flags, environment, and expanded batch coverage against the original job. -Do not introduce a validation dependency solely for this small YAML edit. - -Invoke `reviewing-compiler-prs` and the `expert-reviewer` agent for the final local work. -Request review of the preserved record-only behavior and the CI repair's coverage, configuration, runtime evidence, and artifact names. -Keep all review feedback local. Resolve concrete findings and rerun affected validation. -Apply `code-compaction` if new changes introduce duplicated setup or excessive scope. -Invoke `release-notes` to confirm the existing compiler-service entry remains sufficient. Do not duplicate it for CI-only changes. - -Run `git diff --check` and inspect the repair diff against the existing PR head. -Stage only necessary repair files and any justified, regenerated baselines. -Do not commit raw logs, dumps, generated discovery data, or temporary probes. -Commit on `fix/issue-20410`, for example `Isolate no-realsig desktop CI test batches (#20559)`. -Include `Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>` and `Copilot-Session: ` trailers. -Preserve the existing implementation and planning commits. Do not amend, push, open a PR, post a comment, or request a remote rerun. -Report the local commit and validation accurately. Remote checks remain unchanged until an authorized later push. - -## Definition of Done - -- The existing PR branch and full diff were inspected before editing, and the repair extends its history. -- Every failed or canceled CI job has a recorded classification and raw evidence, including the known desktop timeout. -- Competing contention, runner-hang, and Release-regression hypotheses were tested before selecting the repair. -- Local reproduction uses Release/net472, compressed metadata, no-realsig binaries, and immediate cache eviction, with any engine deviation recorded. -- The repair changes only the proven cause and does not increase timeouts, ignore errors, drop tests, or change unrelated jobs. -- If batching is used, all three generated selections preserve each desktop test identity exactly once across all six projects. -- All three proposed desktop batch commands complete locally below 120 minutes each, with zero failures and complete suite results. -- The existing issue regressions and nearby signature tests pass with nonzero discovery in Release/net472 and Release/net11.0. -- Field-extended-data, signature-nullness, micro-compilation, and attribute-matching siblings pass with recorded selections and counts. -- Exact diagnostic lists, invalid-permutation failure, three repeated nullness warnings, and non-record controls remain unchanged. -- Original diagnostic RED evidence is retained or reconstructed without weakening tests, followed by GREEN on the preserved guard. -- No build error, test assertion, or EmittedIL baseline mismatch remains unresolved; any necessary baseline regeneration is committed and passes without update mode. -- Existing compiler and release-note changes remain intact unless a reproduced failure requires a surgical correction. -- Changed compiler files pass fsharp-diagnostics and rebuilt tests; only changed F# files are formatted, if any. -- YAML structure, preserved environment, distinct report/artifact names, and git diff --check pass. -- Local expert review is complete, concrete findings are resolved, and affected validation is rerun. -- Commands, source revisions, timing, exit codes, test results, and pending-state history persist outside implementation commits. -- The verified repair is committed locally on fix/issue-20410 with required trailers, with no push or other remote mutation.