Skip to content

test(tests): nullable-cleanup Slice C-2 — MmGenerator XDocument + #715 CS8603 (#710, x0ykyn) - #730

Merged
jsboige merged 1 commit into
masterfrom
fix/tests-nullable-slice-c2
Jul 6, 2026
Merged

test(tests): nullable-cleanup Slice C-2 — MmGenerator XDocument + #715 CS8603 (#710, x0ykyn)#730
jsboige merged 1 commit into
masterfrom
fix/tests-nullable-slice-c2

Conversation

@jsboige

@jsboige jsboige commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Slice C-2 of #710 nullable-cleanup — MmGenerator XDocument derefs + the #715 CS8603 (4 warnings)

Second sub-lot of Slice C (CS86xx nullable-flow). Follows Slice A (#727), Slice B (#728), Slice C-1 (#729).

Closes the dispatch's explicit ask: "corrige le CS8603 de #715"

MmGeneratorTests.cs:281 CS8603 is the 96→98 drift documented in the #726 plan note — introduced by #715 (commit 699580ab, the #665 Virtue ar/fa/zh localization fix), which added a FindRepoVirtuesCsv() path-finder helper declared string (non-nullable) with a return null; fallback. This PR fixes all 4 CS86xx in that file, including the #715 one.

Fixes (4 warnings, all low-risk)

Line Code Fix Rationale
L86 CS8602 xmlDoc.Root!.Name.LocalName XDocument.Parse of the test's well-formed .mm yields a non-null Root; ! documents that invariant
L138 CS8602 xmlDoc.Root!.Element("node")!.Elements("node") Both derefs guarded by the two NotBeNull asserts on the preceding lines (L136 Root, L137 node)
L176 CS8602 xmlDoc.Root!.Element("node")!.Elements("node") Same pattern, guarded by L174+L175 asserts
L273/281 CS8603 private static string? FindRepoVirtuesCsv() The #715 helper legitimately returns null when the Virtues CSV isn't found walking up from the bin dir → string? is the honest type. Sole caller (L299) asserts csvPath.Should().NotBeNull(...) on L300; csvPath! at L302 acknowledges that FluentAssertions guard

#715 CS8603 root cause (code=truth)

git blame L274-282 = 699580aba (#715). The helper was added to locate the committed Virtues CSV for the new native-script ar/fa/zh test, declared string (non-null) with a return null; fallback — a classic CS8630-class mismatch the #710 sweep now catches.

DoD (measured on this branch, base master 240511c8)

CsvHelper safety (#710 §3)

NOT IN SCOPE. MmGeneratorTests exercises the mindmap-generation path (.mm XML via XDocument), not the CsvHelper CSV load path. The mapping-risk suites are untouched.

Slice C remaining (~15 distinct CS86xx, subsequent sub-lots)

CsvGridConversion, SvgConversion×2, SvgDisambiguation, SvgPostProcessing, OwlE2E, FallacyLinkFallback×3, LoggerMarkup, UtilityExtensions, PKHierarchical, ImageFileGenerator×2, SimpleImageGenerator.

Non-goals

  • 0 write under Cards/. 0 production / card-rendering code changed. Tests-only.
  • 0 CsvHelper-mapped type changed.

Base 240511c8. Dispatch x0ykyn PRIMARY (Slice C-2). Independent of #727/#728/#729 (different files; this branch off master).

🤖 Generated with Claude Code

Co-authored-by: Claude-Code noreply@anthropic.com

…s + #715 CS8603 (4 warnings)

Second sub-lot of Slice C (CS86xx nullable-flow) of the #710 nullable-cleanup plan
(dispatch x0ykyn PRIMARY; follows Slice A #727, Slice B #728, Slice C-1 #729).

This sub-lot closes the dispatch's explicit ask — "corrige le CS8603 de #715" — by fixing all
4 CS86xx in MmGeneratorTests.cs (the mindmap-generation test fixture), including the CS8603 that
#715 (commit 699580a, #665 Virtue ar/fa/zh loc) introduced at L281 (documented in the #726 drift
note as the 96→98 regression). The file is test-only and exercises the real mindmap-generation
code path (FallacyMindMapDocumentConfig / VirtueMindMapDocumentConfig via FreeMindPath="" headless).

Fixes (4 warnings, all low-risk post-assert suppression or honest nullable return):

- L86  CS8602 `xmlDoc.Root!.Name.LocalName` — XDocument.Parse of the test's well-formed .mm yields
       a non-null Root; the `!` documents that invariant. (No prior Root assert, hence the `!`.)
- L138 CS8602 `xmlDoc.Root!.Element("node")!.Elements("node")` — both derefs are guarded by the
       two FluentAssertions `NotBeNull` on the immediately preceding lines (L136 Root, L137 node).
- L176 CS8602 `xmlDoc.Root!.Element("node")!.Elements("node")` — same pattern, guarded by L174+L175.
- L273/281 CS8603 `private static string? FindRepoVirtuesCsv()` returning `null` — the #715 path-finder
       helper legitimately returns null when the Virtues taxonomy CSV is not found walking up from
       the bin dir. `string?` is the honest return type. The sole caller (L299) asserts
       `csvPath.Should().NotBeNull(...)` on L300 before passing it to LoadVirtuesFromPathAsync on L302;
       `csvPath!` there acknowledges that assertion (the compiler can't track FluentAssertions).

#715 CS8603 root cause (code=truth): git blame L274-282 = 699580a (#715); the helper was added to
locate the committed Virtues CSV for the new native-script ar/fa/zh test, declared `string` (non-null)
with a `return null;` fallback — a classic CS8618/CS8630-class mismatch the #710 sweep now catches.

DoD (measured on this branch, base master 240511c):
- dotnet build Tests.csproj --no-incremental: MmGeneratorTests emits 0 CS86xx (4→0). 0 errors.
- dotnet test --filter MmGeneratorTests: 11/11 pass (incl. the 3 ar/fa/zh InlineData #715 cases).
- dotnet test (full): 587 pass / 1 fail (OWL #133 permanent) / 5 skip (#719). 0 regression.

CsvHelper safety (#710 §3): NOT IN SCOPE — MmGeneratorTests exercises the mindmap-generation path
(.mm XML via XDocument), not the CsvHelper CSV load path. The mapping-risk suites are untouched.

Slice C remaining: 15 distinct CS86xx across ~11 files (CsvGridConversion, SvgConversion×2,
SvgDisambiguation, SvgPostProcessing, OwlE2E, FallacyLinkFallback×3, LoggerMarkup,
UtilityExtensions, PKHierarchical, ImageFileGenerator×2, SimpleImageGenerator). Follow in
subsequent sub-lots.

Base 240511c. 0 Cards/ write, 0 rendering code change.

Dispatch x0ykyn (PRIMARY, Slice C-2 — closes the #715 CS8603 call-out). Base 240511c.

Co-authored-by: Claude-Code <noreply@anthropic.com>

@clusterManager-Myia clusterManager-Myia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[NanoClaw] — LGTM (COMMENT only)

Slice C-2 #710 nullable-cleanup byte-vérifié firsthand au head 41e57f112 (1 fichier, +5/-5, Tests/ only MmGenerator = release freeze unaffected). Second sub-lot de Slice C, suite directe de mon #729 LGTM (C-1). Closes l'ask explicite du dispatch « corrige le CS8603 de #715 ».

5 fixes sound, tous nullable-flow corrects :

  • xmlDoc.Root.Name.LocalNamexmlDoc.Root!.Name.LocalName (L86) : XDocument.Root est XElement? (null si doc vide) — le ! est justifié car le test parse du contenu valide (GenerateMmFile_WithValidCsv) qui a toujours un root.
  • xmlDoc.Root.Element("node").Elements("node")xmlDoc.Root!.Element("node")!.Elements("node") (L138 + L176, ×2) : chaque ! est supporté par une assertion FluentAssertions précédente (xmlDoc.Root.Should().NotBeNull() L136/174 + xmlDoc.Root.Element("node").Should().NotBeNull() L137/175). FluentAssertions n'informe pas le compilateur du null-state, donc le ! acknowlege ce que l'assertion runtime garantit. Pattern standard test-arrange.
  • string FindRepoVirtuesCsv()string? (L273) : annotation honnête — la méthode walk 12 niveaux et retourne null si le CSV Virtues n'est pas trouvé (boucle for ... && dir != null L275). string? reflète la réalité du runtime (pas une suppression aveugle).
  • LoadVirtuesFromPathAsync(csvPath)csvPath! (L302) : caller asserte csvPath.Should().NotBeNull("...must be locatable...") L297 puis déréférence — le ! matche l'assertion précédente.

Soundness : 0 behavior change — ! est compile-time uniquement (suppression warning CS8602/CS8603, pas de runtime check). La string? annotation est une correction de contrat (la méthode peut VRAIMENT retourner null). Commentaires in-code inchangés, scope Tests/ only (0 Cards/ touché, 0 prod entity).

△ Caveat mineur : les xmlDoc.Root! sur L86 (vs L138/L176 qui ont l'assertion .Root.Should().NotBeNull() juste avant) — L86 n'a que xmlDoc.Should().NotBeNull() (pas .Root), donc le ! repose sur la invariant « XDocument.Parse de contenu valide a toujours un Root ». C'est vrai en pratique (le test a généré le mmContent valide juste avant), mais moins défendu que L138/L176. Non-bloquant (l'invariant tient pour ce test). Décision merge = Emerjesse.

@jsboige
jsboige merged commit 822167d into master Jul 6, 2026
3 checks passed
@jsboige
jsboige deleted the fix/tests-nullable-slice-c2 branch July 6, 2026 17:10
jsboige added a commit that referenced this pull request Jul 6, 2026
…er (4 warnings) (#731)

Third sub-lot of Slice C (CS86xx nullable-flow) of the #710 nullable-cleanup plan
(dispatch x0ykyn PRIMARY; follows Slice A #727, B #728, C-1 #729, C-2 #730).

Cohesive sub-lot by area: all 4 remaining CS86xx in the MindmapGeneration SVG cluster
(3 files). This cluster sits in CLAUDE.md "Known Fragile Area #1 — SVG disambiguation" territory,
so each fix is the most conservative honest annotation, not a logic change.

Fixes (4 warnings):

- SvgConversionIntegrationTests.cs L59-60 (CS8602 method-null deref + CS8605 nullable unbox):
  `GetMethod` returns `MethodInfo?` and `Invoke` returns `object?`. The test is `[Fact(Skip=...)]`
  (the #719 Magick.NET skip — root-caused OBSOLETE by #722: the path was replaced by FreeMind GUI
  + Batik). Added `Assert.NotNull(method)` + `method!.Invoke(...)!` so the cast is honest. The
  skipped test never runs; the annotation just satisfies the compiler. NO behavioral change.
- SvgDisambiguationContractTests.cs L37 (CS8625): `G(string text, string id = null)` →
  `G(string text, string? id = null)`. The body already null-checks (`if (id != null)`) before
  `SetAttributeValue`, so `string?` is the honest param type. Caller fixture sites pass null to
  build <g> elements without an id — intentional.
- SvgPostProcessingTests.cs L33 (CS8604): `Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)!`
  — GetDirectoryName returns `string?` (Location can in theory be empty); in a running test the
  assembly location is non-null, so the `!` documents that invariant.

DoD (measured on this branch, base master 240511c):
- dotnet build Tests.csproj --no-incremental: the 3 target files emit 0 CS86xx (4→0). 0 errors.
- dotnet test --filter MindmapGeneration: 85 pass / 1 skip (the Magick test, expected). 0 regression.
- dotnet test (full): 587 pass / 1 fail (OWL #133 permanent) / 5 skip (#719). 0 regression.

CsvHelper safety (#710 §3): NOT IN SCOPE — none of these files is a CsvHelper-mapped entity or
exercises the CSV load path. The mapping-risk suites are untouched.

Slice C remaining: 11 distinct CS86xx across 8 files (CsvGridConversion, ImageFileGenerator×2,
SimpleImageGenerator, OwlE2E, FallacyLinkFallback×3, LoggerMarkup, UtilityExtensions,
PKHierarchical). Follow in subsequent sub-lots.

Base 240511c. 0 Cards/ write, 0 rendering code change.

Dispatch x0ykyn (PRIMARY, Slice C-3). Base 240511c.

Co-authored-by: Your <your.email@example.com>
Co-authored-by: Claude-Code <noreply@anthropic.com>
jsboige added a commit that referenced this pull request Jul 6, 2026
…warnings) (#732)

Fourth sub-lot of Slice C (CS86xx nullable-flow) of the #710 nullable-cleanup plan
(dispatch x0ykyn PRIMARY; follows Slice A #727, B #728, C-1 #729, C-2 #730, C-3 #731).

Cohesive sub-lot by area: all 3 remaining CS86xx in ImageGeneration/ (2 files).
None is a CsvHelper-mapped entity or exercises the CSV load path.

Fixes (3 warnings):

- ImageFileGeneratorTests.cs L65 (CS8604): `Path.GetDirectoryName(fakeImagePath)!` — GetDirectoryName
  returns `string?`; the fake-image path is built by ImageHelper from a known config so the dir part
  is non-null. `!` documents that invariant.
- ImageFileGeneratorTests.cs L166 (CS8600): `result = null!` — the ConcurrentDictionary is the
  classic closure-capture pattern: declared null, assigned inside the `Action act = () => result =
  sut.GenerateDocumentImages(...)`, then asserted `result.Should().NotBeNull()` at L172 before use.
  `null!` is the honest initializer.
- SimpleImageGeneratorTests.cs L40-43 (CS8602): `JsonSerializer.Deserialize` returns
  `AssetConverterConfig?`. Added `config.Should().NotBeNull(...)` (committed test-config.json must
  deserialize) + `config!.BaseTargetDirectoryName`. The FluentAssertions guard + `!` together satisfy
  the flow analysis (0 residual CS8603 at the `return config;`).

DoD (measured on this branch, base master 240511c):
- dotnet build Tests.csproj --no-incremental: the 2 target files emit 0 CS86xx (3→0). 0 errors.
- dotnet test --filter ImageGeneration: 8/8 pass. 0 regression.
- dotnet test (full): 587 pass / 1 fail (OWL #133 permanent) / 5 skip (#719). 0 regression.

CsvHelper safety (#710 §3): NOT IN SCOPE — neither file is a CsvHelper-mapped entity or exercises
the CSV load path.

Slice C remaining: 8 distinct CS86xx across 6 files (CsvGridConversion, OwlE2E,
FallacyLinkFallback×3, LoggerMarkup, UtilityExtensions, PKHierarchical). Follow in subsequent sub-lots.

Base 240511c. 0 Cards/ write, 0 rendering code change.

Dispatch x0ykyn (PRIMARY, Slice C-4). Base 240511c.

Co-authored-by: Your <your.email@example.com>
Co-authored-by: Claude-Code <noreply@anthropic.com>
jsboige added a commit that referenced this pull request Jul 6, 2026
… null (3 warnings) (#733)

Fifth sub-lot of Slice C (CS86xx nullable-flow) of the #710 nullable-cleanup plan
(dispatch x0ykyn PRIMARY; follows Slice A #727, B #728, C-1 #729, C-2 #730, C-3 #731, C-4 #732).

Single-signature fix: 3 CS8625 (L115/135/152) all stem from `SetLink(f, linkProp, null)` where the
helper was declared `SetLink(Fallacy f, string propName, string value)`. The null is INTENTIONAL —
the FallacyLinkFallback test suite deliberately clears a non-source language link to exercise the
mind-map link fallback cascade (non-source null → EN → FR source-floor → null floor). `string? value`
is the honest param type; the body `SetValue(f, value)` accepts object so null is runtime-legal.

This is the fragile-area link-fallback contract pinned by this suite (CLAUDE.md mind-map
localization) — the annotation changes nothing about the cascade behavior, only the declared
parameter type. Direct confirmation: all 29 FallacyLinkFallback tests pass unchanged.

DoD (measured on this branch, base master 240511c):
- dotnet build Tests.csproj --no-incremental: FallacyLinkFallback emits 0 CS86xx (3→0). 0 errors.
- dotnet test --filter FallacyLinkFallback: 29/29 pass.
- dotnet test (full): 587 pass / 1 fail (OWL #133 permanent) / 5 skip (#719). 0 regression.

CsvHelper safety (#710 §3): NOT IN SCOPE — FallacyLinkFallback exercises the link-fallback cascade
via reflection on Fallacy entities, not the CsvHelper CSV load path.

Slice C remaining: 5 distinct CS86xx across 5 files (CsvGridConversion, OwlE2E, LoggerMarkup,
UtilityExtensions, PKHierarchical) — singletons, bundle into a final C-final lot.

Base 240511c. 0 Cards/ write, 0 rendering code change.

Dispatch x0ykyn (PRIMARY, Slice C-5). Base 240511c.

Co-authored-by: Your <your.email@example.com>
Co-authored-by: Claude-Code <noreply@anthropic.com>
jsboige added a commit that referenced this pull request Jul 6, 2026
…omplete (#710) (#734)

Final sub-lot of Slice C (CS86xx nullable-flow) of the #710 nullable-cleanup plan
(dispatch x0ykyn PRIMARY; follows Slice A #727, B #728, C-1 #729, C-2 #730, C-3 #731, C-4 #732,
C-5 #733). This completes Slice C: all 32 distinct CS86xx in Tests/ are addressed across the
7 sub-lots C-1..C-final (merged set), leaving 0 CS86xx once all slice PRs are merged.

The 5 singletons (one warning per file, 5 different areas):

- CsvGridConversionContractTests.cs L176 (CS8625): `new List<object> { "a", null!, "c" }` — the null
  cell is INTENTIONAL (test name "GridToCsv_NullCells_BecomeEmptyStrings", pins the `cell?.ToString()
  ?? ""` guard). `null!` is the honest suppression (runtime-legal null cell, annotation-strict).
- OwlE2EGenerationValidationTests.cs L60 (CS8603): `private static string? ResolveRepoFile(...)` —
  path-finder helper legitimately returns null when the ontology file isn't found walking up from
  the bin dir (same idiom as #715 FindRepoVirtuesCsv). The sole caller (L47) already null-checks and
  throws FileNotFoundException (L48-54); the throw guarantees `path` non-null at L55 FromFile, so no
  caller `!` needed.
- LoggerMarkupSafetyTests.cs L58 (CS8600): `string output = null!;` — closure-capture pattern
  (assigned in the lambda at L59, asserted at L62). `null!` is the honest initializer.
- UtilityExtensionsLayoutTests.cs L142 (CS8600): `((string)null!).PathIsUrl()` — the null is
  INTENTIONAL (test name "PathIsUrl_Returns_False_For_Null", pins the null-guard so a regression
  that dropped it would NRE). The `null!` documents that intent.
- PKHierarchicalGroupingTests.cs L25 (CS8619): `.Select(record => record["path"].ToString()!)` —
  the path key is always set by BuildRecords (L22), so ToString on the boxed string is non-null;
  `!` documents that invariant for the ToHashSet<string>() return type.

DoD (measured on this branch, base master 240511c):
- dotnet build Tests.csproj --no-incremental: the 5 target files emit 0 CS86xx (5→0). 0 errors.
- dotnet test (targeted GSheetSync+Ontology+Utility+PKHierarchical): 52 pass / 1 fail
  (OWL #133 permanent known-fail, pre-existing — not a regression; the 3 OTHER OwlE2E tests pass).
- dotnet test (full): 587 pass / 1 fail (OWL #133) / 5 skip (#719). 0 regression.

CsvHelper safety (#710 §3): NOT IN SCOPE — none of these 5 files is a CsvHelper-mapped entity or
exercises the CSV load path (GSheet grid round-trip, OWL ontology load via OwlAdapter, Logger
markup, path utility, PK grouping reflection).

#710 plan status once all 8 PR (#727-733 + this) merged: Tests/ CS86xx from 98 raw (49 distinct)
to 0. Slice A (6 raw CS0108/CS0219) + Slice B (20 raw CS8618) + Slice C (32 distinct CS86xx) = the
full 98-raw baseline. The #715 drift (CS8603, 96→98) closed at C-2.

Base 240511c. 0 Cards/ write, 0 rendering code change.

Dispatch x0ykyn (PRIMARY, Slice C-final — Slice C complete). Base 240511c.

Co-authored-by: Your <your.email@example.com>
Co-authored-by: Claude-Code <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants