[Lean Squad] feat(fv): Task 2 + Task 9 — CommandLineOptionsValidator informal spec & CI simplification#8105
Conversation
…fication Task 2: add informal spec for CommandLineOptionsValidator.ValidateOptionsArgumentArity - Detailed preconditions, postconditions, algorithm, invariants - 7 proposed Lean theorems (valid_iff_all_in_range, independent errors, etc.) - 4 open questions including the KeyNotFoundException design issue (OQ-1) - Approximations documented for Lean modelling (MaxInt as ⊤, provider elision) - Advance target #4 to phase 2 in TARGETS.md Task 9: remove Mathlib from lakefile.toml (no .lean files use it yet) - Eliminates heavy network fetch of Mathlib in CI - Remove the lake update step from lean-proofs.yml (no external deps) - Will be re-added when first .lean file imports Mathlib Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR advances the Lean Squad formal-verification effort by adding an informal specification for CommandLineOptionsValidator.ValidateOptionsArgumentArity, updating FV target tracking to reflect Phase 2 progress, and simplifying Lean CI/dependencies by removing the unused Mathlib requirement.
Changes:
- Added a new informal spec markdown document for
ValidateOptionsArgumentArity, including proposed theorems and design questions. - Updated
formal-verification/TARGETS.mdto reflect Phase 2 (“Informal spec extracted”) for the arity validator and parse-result equality targets. - Removed Mathlib from the Lean
lakefile.tomland dropped thelake updateCI step to avoid unnecessary dependency resolution.
Show a summary per file
| File | Description |
|---|---|
| formal-verification/TARGETS.md | Updates FV target phases/status to reflect informal spec extraction progress. |
| formal-verification/specs/commandlineoptionsvalidator_arity_informal.md | Adds the informal spec for ValidateOptionsArgumentArity (pre/postconditions, algorithm, invariants, theorems, open questions). |
| formal-verification/lean/lakefile.toml | Removes unused Mathlib dependency and documents when to re-add it. |
| .github/workflows/lean-proofs.yml | Simplifies Lean CI by removing the dependency-resolution step (lake update). |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 2
Evangelink
left a comment
There was a problem hiding this comment.
Summary
Workflow: Expert Code Reviewer 🧠
Date: 2026-05-11
Repository: microsoft/testfx
Review Scope
4 files changed: .github/workflows/lean-proofs.yml (CI simplification), formal-verification/TARGETS.md (status updates), formal-verification/lean/lakefile.toml (Mathlib removal), formal-verification/specs/commandlineoptionsvalidator_arity_informal.md (new informal spec).
Verification Against Source
I cross-referenced the informal spec against the actual CommandLineOptionsValidator.ValidateOptionsArgumentArity implementation (CommandLineOptionsValidator.cs):
- Algorithm: The spec's pseudocode correctly represents
arity > option.Arity.Max && option.Arity.Max == 0asT > 0 AND Max == 0— logically equivalent ✓ - Argument accumulation: Correctly describes the
GroupBy+sum-over-Arguments.Lengthpattern ✓ - Error branches and message keys: Accurate ✓
- OQ-1 (KeyNotFoundException): The identified risk is real — the direct indexer
providerAndOptionByOptionName[optionName]is used withoutTryGetValue, relying onValidateNoUnknownOptionsrunning first. Correctly flagged as an open question ✓ - OQ-3 (missing required options not flagged here): Accurate —
GroupByonly groups options that appeared; absent options are handled downstream ✓
TARGETS.md consistency
Target #5 (CommandLineParseResult.Equals) advancing to phase 2 is consistent with the existing commandlineparseresult_equals_informal.md file already on main — no inconsistency.
CI change
Removing lake update is safe: (a) with no [[require]] blocks in lakefile.toml, the step would be a no-op, and (b) while there are zero .lean files, lean_count == 0 skips both lake update and lake build anyway.
Key Findings
No correctness, threading, performance, API compatibility, cross-TFM, resource management, security, or defensive coding issues found.
Positive Observations
- The informal spec document is thorough and accurate, covering preconditions, postconditions, invariants, edge cases, approximations for Lean modelling, and proposed theorems
- The CI simplification is well-justified and reversible
- Open questions are honestly documented, including the real
KeyNotFoundExceptiondesign risk in the underlying code
Generated by Expert Code Reviewer
🧠 Reviewed by Expert Code Reviewer 🧠
Evangelink
left a comment
There was a problem hiding this comment.
Summary
Workflow: PR Nitpick Reviewer 🔍
Date: 2026-05-11
Repository: microsoft/testfx
Key Findings
Three minor documentation nitpicks found across the two changed docs:
-
TARGETS.md— inconsistent "Next:" status tag (item 6): Missing "(blocked by Lean toolchain)" qualifier that every other item consistently carries. Either an accidental omission or needs a clarifying note if intentional. -
Spec file — misleading pseudocode notation (
Arguments["--filter A"].Length): The dict-index syntax implies a map keyed by raw command text; the actual model is a list ofCommandLineParseOptionoccurrences, each with an.Argumentsproperty. This matters most for a Lean modeller who needs the correct data-structure shape. -
Spec file — "Not possible" edge-case description: The scenario is possible for users; the correct statement is that the function doesn't reach it due to
GroupBy's contract.
Positive Highlights
- Removing
lake updatefrom CI in tandem with dropping the mathlib dependency is a clean, well-motivated change — the comment inlakefile.tomlexplains the rationale clearly. - The informal spec is comprehensive and well-structured: preconditions, postconditions, algorithm, invariants, edge cases, examples, open questions, and proposed Lean theorems are all present. The "Why Max == 0 is a special case" sub-section is especially useful for a formal verifier.
- The PR title makes the scope clear; the TARGETS.md update correctly advances both items to phase 2.
Recommendations
- Clarify the "Next: Task 3." entry for item 6 in TARGETS.md.
- Replace the
Arguments["..."].Lengthnotation with occurrence-indexed notation. - Replace "Not possible" with "Not reached by this function" in the Edge Cases table.
Generated by PR Nitpick Reviewer
🔍 Meticulously inspected by PR Nitpick Reviewer 🔍
…informal spec & CI simplification (#8105) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Task 2: add informal spec for CommandLineOptionsValidator.ValidateOptionsArgumentArity
Task 9: remove Mathlib from lakefile.toml (no .lean files use it yet)
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com
Fixes #7964