docs: add general documentation improvements to README by RJSonnenberg · Pull Request #32 · mta-solutions/fsharp-data-validation

RJSonnenberg · 2026-03-02T18:49:24Z

Overview

This PR adds comprehensive general documentation improvements to the README, including best practices, common patterns, and advanced validation scenarios.

Changes

New Sections Added

Quick Start - A minimal, practical example showing how to validate an email address with key concepts highlighted
- Transformation-oriented validation
- Type safety with private constructors
- Accumulating failures
- refute vs dispute concepts
FAQ / Common Patterns (~600 lines) - Five Q&A sections answering common questions:
- When should I use refute vs dispute?
- How do I compose failures from multiple domains?
- How do I validate nested collections?
- When should I use mapInvalid?
- How do I handle async validation?
Advanced Patterns (~500 lines) - Seven detailed pattern examples:
- Failure type composition across domains
- Conditional field validation
- Validating nested collections with indices
- Using flattenProofs for nested validations
- Cross-field validation
- Smart constructors for validated types
- Error transformation with mapInvalid

Add comprehensive documentation sections: - Installation: How to install the library - Quick Start: Minimal email validation example with key concepts - FAQ / Common Patterns: Guidance on refute vs dispute, failure composition, nested collections, mapInvalid usage, and async validation - Advanced Patterns: Detailed patterns for complex validation scenarios including: * Failure type composition across domains * Conditional field validation * Validating nested collections with indices * Using flattenProofs for nested validations * Cross-field validation * Smart constructors for validated types * Error transformation with mapInvalid These general documentation improvements complement the new features added in PR #29 and help users understand validation patterns and best practices.

Copilot

Pull request overview

This PR adds comprehensive documentation improvements to the README, including a new Installation section, a Quick Start guide, an FAQ / Common Patterns section with five Q&A entries, and an Advanced Patterns section with seven examples. It also makes minor corrections to existing documentation (clarifying a comment on disputeWith and updating the validateEach function signature description).

Changes:

Added Installation and Quick Start sections at the top of the document, with new Table of Contents entries
Added ~600-line FAQ / Common Patterns section covering refute vs dispute, failure composition, nested collections, mapInvalid, and async validation
Added ~500-line Advanced Patterns section with seven detailed examples; made minor accuracy corrections to the disputeWith comment and validateEach description

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

README.md

…rove validation logic

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-03T20:37:32Z

README.md

+            let! _ = validation {
+                withValue (name, username)
+                disputeWithFact NameMatchesUsername (fun (n, u) ->
+                    match n with
+                    | Some nameVal -> Name.unwrap nameVal <> Username.unwrap u
+                    | None -> true
+                )
                qed
            }


The cross-field validation was changed from and! _ = to let! _ =, which diverges from the established pattern used in samples/GettingStarted/Types.fs (line 252) and samples/FSharp.Data.Validation.Samples/ComplexTypes.fs (line 64). The let! form runs the cross-field check sequentially — meaning it is skipped entirely if any preceding field validation was refuted (e.g., if name or username was None). The established and! _ = form using withValue vm runs in parallel with other field validations, ensuring cross-field checks are always evaluated alongside field-level checks. These are semantically different behaviors, and the README example no longer matches the pattern shown in the actual sample code.

The example here is correct. The previous example only compares the values prior to validation. The current method will ensure the username and name do not match only if they are valid. That said, there may be cases where we are fine with performing that simplistic comparison. This particular example is designed to be a simplistic example.

@copilot Please revert this change to the "and!" method and simple, pre-validated name and username value comparison. Add a note to this example that directs them to part of the documentation where global validation is done on previously-validated fields or values.

Copilot · 2026-03-03T20:37:33Z

README.md

+                    // Run async validations asynchronously as possible
+                    let! emailExists = checkEmailExistsAsync email
+                    let! usernameTaken = checkUsernameTakenAsync username
+


The comment "Run async validations asynchronously as possible" is misleading — the code immediately below uses sequential let! emailExists = ... followed by let! usernameTaken = ..., which runs sequentially, not in parallel. The pattern summary at line 1980 also states "Async validations can run in parallel for better performance", further reinforcing the incorrect claim. The comment should either say "sequentially" or the code should use Async.Parallel to demonstrate actual parallel execution.

Suggested change

// Run async validations asynchronously as possible

let! emailExists = checkEmailExistsAsync email

let! usernameTaken = checkUsernameTakenAsync username

// Run async validations in parallel for better performance

let! results =

[ checkEmailExistsAsync email

checkUsernameTakenAsync username ]

|> Async.Parallel

let emailExists = results.[0]

let usernameTaken = results.[1]

Copilot · 2026-03-03T20:37:33Z

README.md

+            and! newPwd =
+                validation {
+                    withField (fun () -> this.NewPassword)
+                    refuteWith (isRequired Required)
+                    refuteWith (fun p ->
+                        if p = oldPwd then Error NewPasswordSameAsOld
+                        else Ok p
+                    )
+                    qed id
+                }
+
+            and! confirmPwd =
+                validation {
+                    withField (fun () -> this.ConfirmNewPassword)
+                    refuteWith (isRequired Required)
+                    qed id
+                }
+
+            and! _ =
+                validation {
+                    // Cross-field validation: compare old and new passwords
+                    withValue (oldPwd, newPwd, confirmPwd)
+                    disputeWithFact NewPasswordSameAsOld (fun (old, new_, _) -> new_ <> old)
+                    disputeWithFact ConfirmationDoesNotMatch (fun (_, new_, confirm) -> new_ = confirm)
+                    qed
+                }


The NewPasswordSameAsOld check is performed twice: once inline inside the and! newPwd block (line 2151-2154, using refuteWith) and once in the and! _ = cross-field block (line 2169, using disputeWithFact). This redundancy is confusing in a documentation example meant to illustrate best practices. The inline check also occurs as a refuteWith which stops validation of newPwd further, while the duplicate check in the cross-field block uses disputeWithFact. Consider removing the inline check and relying solely on the cross-field validation to demonstrate a clean separation of concerns.

Copilot · 2026-03-03T20:37:34Z

README.md

+
+let validateUsername (un: string option) : Proof<UsernameFailures, Username> =
+    validation {
+        withField (fun () -> un)


withField requires a member access expression — such as fun () -> vm.FieldName — to extract the field name via reflection (selector.Body :?> MemberExpression). Using withField (fun () -> un) where un is a function parameter, not a property of a record/object, would throw an InvalidCastException at runtime because the expression body is not a MemberExpression. The correct usage here should be withValue un since un is already the extracted value.

Suggested change

withField (fun () -> un)

withValue un

Copilot · 2026-03-03T20:37:34Z

README.md

+        let! _ = validation {
+            withValue (password, confirmPassword)
+            disputeWithFact PasswordMismatch (fun (p, cp) -> Password.unwrap p = cp)
+            qed
+        }


The cross-field password-match validation uses let! _ = (sequential binding) rather than and! _ = (parallel binding). This means if any of the preceding and! fields were refuted (e.g., missing required password), the cross-field check would be skipped entirely. This diverges from the established pattern in samples/GettingStarted/Types.fs:252 and samples/FSharp.Data.Validation.Samples/ComplexTypes.fs:64 which use and! _ = validation { withValue vm; disputeWithFact ...; qed }. Additionally, the PasswordMismatch failure is also used as the "required" failure for confirmPassword at line 1763 (refuteWith (isRequired PasswordMismatch)), which is semantically confusing — a missing field and a mismatched password produce the same failure value, preventing consumers from distinguishing the two cases.

Copilot · 2026-03-03T20:37:34Z

README.md

+            validation {
+                withValue item
+                refuteWithProof (validateOrderItem >> Proof.mapInvalid ItemError)
+                qed


The validateEach element function incorrectly calls qed at the end. The qed operation strips the ValueCtx wrapper, making the lambda return VCtx<OrderFailure, OrderItem>, but validateEach requires the function to return VCtx<'F, ValueCtx<'B>>. This example would not compile due to a type mismatch. The fix is to remove the qed call — after refuteWithProof, the context is already VCtx<OrderFailure, ValueCtx<OrderItem>> which is the type validateEach expects.

Suggested change

qed

Copilot · 2026-03-03T20:37:34Z

README.md

+This function accepts a function with a signature of `'A -> VCtx<'F, ValueCtx<'B>>` that validates each element.
+The function should be contained in the `validation` computation expression, which returns the appropriate `VCtx` type wrapping a `ValueCtx`.
+The result accumulates all validation failures across elements while preserving valid transformed values.


The description was updated to correctly state that the element function must return VCtx<'F, ValueCtx<'B>>, but the code example below (line 1539) still shows validateEach (fun a -> validation { withValue a; ...; qed; }). Calling qed strips the ValueCtx wrapper, which contradicts the updated description and would produce a type error. The qed call in the existing example should be removed to be consistent with the updated description.

Copilot · 2026-03-03T20:37:34Z

README.md

+            let validateItem idx (item: ItemVM) : VCtx<ItemFailure, ValueCtx<OrderItem>> =
+                validation {
+                    let! name =
+                        validation {
+                            withField (fun () -> item.Name)
+                            refuteWith (isRequired NameRequired)
+                            qed id
+                        }
+                    and! price =
+                        validation {
+                            withField (fun () -> item.Price)
+                            disputeWithFact PriceTooLow (fun p -> p >= 0.01M)
+                            qed id
+                        }
+                    return { Name = name; Price = price }
+                }
+
+            withValue this.Items
+            validateEach validateItem
+            qed id


The validateItem function is annotated as returning VCtx<ItemFailure, ValueCtx<OrderItem>>, but the function body uses let!/and! bindings with qed id and ends with return { Name = name; Price = price }. The return in the CE calls this.Return(a) which produces ValidCtx { Name = name; Price = price }, which is VCtx<ItemFailure, OrderItem> — NOT VCtx<ItemFailure, ValueCtx<OrderItem>>. This is a type annotation mismatch that would cause a compile error. The validateEach operation requires the element function to return VCtx<'F, ValueCtx<'B>>, but a function body using return or qed cannot produce that. This example needs to be reworked to correctly demonstrate validateEach's expected function signature.

Copilot · 2026-03-03T20:37:35Z

README.md

+    validation {
+        withField (fun () -> un)
+        refuteWith (isRequired Empty)
+        refuteWith (fun u -> if String.length u > 50 then Some TooLong else None)


refuteWith expects a function with signature 'A -> Result<'B, 'F>, but the lambda fun u -> if String.length u > 50 then Some TooLong else None returns 'F option (specifically UsernameFailures option). This would be a compile error. The correct usage should return Error TooLong on failure and Ok u on success.

Suggested change

refuteWith (fun u -> if String.length u > 50 then Some TooLong else None)

refuteWith (fun u -> if String.length u > 50 then Error TooLong else Ok u)

RJSonnenberg force-pushed the docs/readme-improvements branch from ddcd6e6 to bb53311 Compare March 2, 2026 19:42

RJSonnenberg requested a review from Copilot March 2, 2026 23:27

RJSonnenberg self-assigned this Mar 2, 2026

RJSonnenberg added the documentation Improvements or additions to documentation label Mar 2, 2026

Copilot started reviewing on behalf of RJSonnenberg March 2, 2026 23:27 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

docs: update README and sample types for F# 8.0 compatibility and imp…

1e39833

…rove validation logic

RJSonnenberg requested a review from Copilot March 3, 2026 20:28

Copilot started reviewing on behalf of RJSonnenberg March 3, 2026 20:28 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

Copilot AI mentioned this pull request Mar 3, 2026

docs: revert cross-field validation to and! with simple comparison; add note for validated-field checks #33

Closed

mta-solutions deleted a comment from Copilot AI Mar 4, 2026

-                    // Run async validations asynchronously as possible
-                    let! emailExists = checkEmailExistsAsync email
-                    let! usernameTaken = checkUsernameTakenAsync username
+                    // Run async validations in parallel for better performance
+                    let! results =
+                        [ checkEmailExistsAsync email
+                          checkUsernameTakenAsync username ]
+                        |> Async.Parallel
+                    let emailExists = results.[0]
+                    let usernameTaken = results.[1]

	refuteWith (fun u -> if String.length u > 50 then Some TooLong else None)
	refuteWith (fun u -> if String.length u > 50 then Error TooLong else Ok u)

Conversation

RJSonnenberg commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Changes

New Sections Added

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

RJSonnenberg Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

RJSonnenberg commented Mar 2, 2026 •

edited

Loading