fix(SVGControl): stop SvgRenderer surfacing parse failures as NullReferenceException - #421
Merged
Merged
Conversation
- Add the active feature folder for the SvgRenderer null-document NRE: issue with AC-1..AC-11, atomic plan v0.3, root-cause research, WinForms designer verification runbook, and Phase 0 baseline evidence - Register SVGControl.Test in TaskMaster.sln with Debug/Release configurations for Any CPU, x64, and x86 - Correct the ExCSS binding redirect in SVGControl.Test/app.config from the non-existent 4.2.4.0 to 4.3.1.0 - Restore the Svg 3.4.7 package pin and add the compile-time Svg reference so SVGControl.Test builds - Record agent-memory entries for legacy csproj compile references, missing VSTO runtime effects on baseline gates, and new solution-member pin divergence - No production C# is modified; SVGControl/SvgRenderer.cs is unchanged and execution is paused at plan task P1-T6 with a SCOPE_EXCEEDED escalation pending a host with the VSTO runtime assemblies Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Record the resume point: execution paused at plan task P1-T6 with SCOPE_EXCEEDED on an MSB3277 System.Runtime.CompilerServices.Unsafe pin divergence in SVGControl.Test - Reproduce the authored-but-unapplied atomic-planner Scope Lock delta verbatim, including the new P1-T6a pin-alignment task and Design Decision 10 - Instruct the receiving orchestrator to re-run Phase 0 baseline capture, since every committed baseline was taken on a host lacking the VSTO runtime assemblies and is invalid elsewhere - Instruct reverting the AC-6 relative-measure amendment and dropping human_interaction requirement H-3, both of which are specific to the originating host - Supply full orchestrator-state reconstruction values, since artifacts/ is gitignored and the checkpoint does not travel with the branch - Carry forward the confirmed root cause, the settled design decisions D1-D3, and the explicit out-of-scope list Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Capture seven Phase 0 baseline evidence artifacts in the 2026-08-04T21-04 series. The committed 14-36 set was invalid twice over: captured on a host lacking the VSTO runtime assemblies, and against a pre-package-update dependency graph. The new baseline records the analyzer and nullable builds each returning EXIT_CODE 0, zero CS0234/MSB3245, MSB3277 count 0, nine test assemblies with 6112/6112 passing, and repo coverage 85.3550% line / 78.5353% branch. The 14-36 artifacts are retained for audit. - Revise the plan in place from v0.3 to v0.6 across three revision passes. Design Decision 10 records that the System.Runtime.CompilerServices.Unsafe pin divergence which halted execution at P1-T6 was resolved upstream by the rebase onto ce0c91e, not by a plan task, so no P1-T6a was added. Design Decision 11 makes nullable annotations mandatory on the new SvgRenderer surface, because the file is #nullable enable and net48 has no NotNullWhen. Split P2-T8's coverage gate into a newly-added >= 90% set and a changed-member no-regression set to match AC-5 rather than exceed it, with a named exception for the CLR-invoked ResolveByNameAndKey. Retarget every Phase 0 baseline reference to the new series and correct stale version literals. - Revert AC-6 to the absolute single-clean-pass form; the relative amendment was specific to the host without the VSTO runtime. Amend AC-9 and AC-10 to record the package versions superseded by PR #419. - Record four agent-memory findings: incremental builds yielding a vacuous nullable baseline, coverage gates on CLR-invoked private members, the nullable context mismatch between production and test projects, and the planner tool surface excluding the MCP validator. No production or test C# is modified; SVGControl/SvgRenderer.cs is untouched. Preflight returns ALL CLEAR against v0.6; execution resumes at P1-T6. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nstructors - GetSvgDocument no longer contains a try/catch. It delegates and keeps its tolerant SvgDocument?-returning contract, so all six existing null-tolerant call sites are unaffected. - Add a fail-fast surface for callers that want one: TryGetSvgDocument (public 3-arg, plus an internal 4-arg seam-bearing overload) and GetSvgDocumentOrThrow, which throws with the original exception as InnerException. - Both byte-array constructors now degrade visibly rather than throwing. They log through the existing log4net logger and through System.Diagnostics.Trace, so the diagnostic is visible in the WinForms designer host where no log4net appender is configured, then leave _doc null without dereferencing it. - ResolveByNameAndKey gains ordered directory probing via Assembly.LoadFrom. The previous fallback returned null because Assembly.Load bound against the host AppDomain's ApplicationBase - the Visual Studio directory - rather than the directory holding SVGControl.dll. The bare catch was also replaced. - Extract the two pure assembly-probe helpers, TryGetDirectoryFromCodeBase and GetProbeDirectories, into SVGControl/SvgAssemblyProbe.cs. SvgRenderer.cs had reached 547 lines against the repository's 500-line limit, and a tightening pass could not close the gap without breaking acceptance clauses that require the log4net and Trace calls to appear literally in the catch block and in both constructors. The cut is correct on cohesion grounds independently of the line count: both helpers are pure path-string functions with no renderer-state dependency. SvgRenderer.cs is now 495 lines and SvgAssemblyProbe.cs is 67. Wired in via an explicit <Compile Include> since SVGControl is legacy non-SDK. - Add 27 tests in SVGControl.Test/SvgRendererParseContractTests.cs. The suite is 6139/6139 passing across nine assemblies, up from a 6112 baseline. Four are the AC-1 regression tests, captured failing before the fix and passing after. Corrects a factual error the tests exposed: the plan and AC-5's note both claimed Array.Empty<byte>() reaches an element-free path where SvgDocument.Open returns null without throwing. It actually raises XmlException "Root element is missing". The affected tests now assert the measured behavior, which is strictly stronger than the original assertion, and the element-free branch is driven through the injected parse delegate instead. AC-5's note still carries the false claim and is corrected before Phase 2 checks AC-5 off. Acceptance criteria checked off: AC-1, AC-2, AC-3, AC-4, AC-7, AC-8, AC-9, AC-10. AC-5 and AC-6 belong to Phase 2. AC-11 needs a human designer-load verification and remains unchecked by design. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Add GetSvgDocumentOrThrow_WithTheBuiltInDefaultImage_ReturnsADocument. The member had two throwing-path tests and no success-path test, so its `return document!;` line was uncovered and it measured ~67% against the 90% new-member gate. It now measures 100%. - Correct two comments that asserted a claim Phase 1 disproved - that element-free input makes SvgDocument.Open return null without throwing. The production comment on OpenFromBytes and the class-level XML doc in the test file now record that the null-returning path is reached through the injected parse delegate and that an empty payload raises XmlException. The replacement used 4 of a 7-line budget, so SvgRenderer.cs went 495 to 497 against the 500-line limit. One consecutive clean toolchain pass, no restart: - csharpier format reformatted zero files; csharpier check exits 0. - Analyzer build: 0 errors, 6 warnings, no new diagnostics vs baseline. - Nullable/TreatWarningsAsErrors: 0 errors, 5 warnings, zero CS86xx. - Tests: 6140/6140 passing across 9 assemblies, up from a 6112 baseline. Coverage: - All seven newly-added members measure 100% line rate against the 90% gate. - Changed members improved: GetSvgDocument 62.5% to 100%, and the SvgRenderer(byte[], Size, AutoSize) constructor 0% to 76.5%. - The SvgRenderer class went 62.559% to 72.109%, +9.55 points and +160 lines. - Repo-wide line 85.3550% to 85.3844%, branch 78.5353% to 78.5521%; both above their floors and both improved, so no denominator-change fallback was needed. - ResolveByNameAndKey's rate fell 72.09% to 68.12% while its covered lines rose 31 to 47. That is a denominator effect from the new strategy-3 assembly-probe wiring, which carries the ratified named coverage exception. AC-5 (coverage on changed code) and AC-6 (toolchain passes in a single clean pass) are checked off, bringing delivered acceptance criteria to ten of eleven. AC-11 requires a human to verify the WinForms designer load against runbooks/verify-winforms-designer-load.runbook.md and remains unchecked by design; the handoff is recorded in evidence/other/. All 46 plan tasks are complete. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lver Remediation cycle 1 for the five non-blocking findings from feature review. All 40 remediation-plan tasks complete. - Extract the AssemblyResolve region - static installer, re-entrancy guard, ResolveByNameAndKey, and the public-key-token comparison - into the new SVGControl/SvgAssemblyResolver.cs (157 lines). SvgRenderer.cs drops 497 to 362, relieving pressure against the 500-line limit. SvgRenderer's static constructor is deliberately RETAINED with its body reduced to SvgAssemblyResolver.Install(); moving it wholesale would have silently disabled the resolver while leaving the build green and the tests passing. Install() measures 100% coverage, so the resolver is proven to still install. - Restore exception containment: a catch (Exception ex) on the outer try/finally, and the invalid-path-character filter now applied to the third probe candidate as well as the first two. Previously Path.Combine, Assembly.Location, and Assembly.CodeBase could throw out of an AssemblyResolve handler, converting a recoverable bind failure into the opaque construction-time failure this issue exists to eliminate - and contradicting SvgAssemblyProbe's own documented "never raises, so it is safe inside an AssemblyResolve handler" contract. - Relocate PublicKeyTokensEqual to SvgAssemblyProbe and cover it: 0/15 to 15/15 line, 18/18 branch. AC-8 requires its behavior be preserved and nothing tested it. Eight cases are required rather than seven: the expression has ten condition outcomes and only (non-empty, null) drives the last two. - Correct two stale comments: the header block that was the sole in-code explanation for the fallback named package versions no longer present in the repository, so a reader could have concluded the fallback was dead code and removed working error handling; and one test comment overstated a claim beyond what was measured. Add <LangVersion>latest</LangVersion> to SVGControl.Test, eliminating CS8630. The measurement contradicted the prediction: Roslyn suppresses nullable diagnostics in generated code, so the *.Designer.cs diagnostics that were expected never appeared and the out-of-scope set measured empty. The 24 in-scope diagnostics were all in the three test files this branch authored, cleared by annotating declarations to match already-declared nullable returns plus #nullable enable on those files - the same convention all three production files use. No NoWarn, no pragma, no severity change, and no assertion or test name altered. Tests: 10 added, suite at 6150/6150 across 9 assemblies. Coverage: repo line 85.3890% to 85.4097% and branch 78.5740% to 78.7220%, both improved. SvgAssemblyProbe reaches 100% line and 100% branch across all three members. The SvgRenderer class rises 72.1088% to 80.1932% with no line losing coverage. Acceptance criteria stay at ten of eleven delivered. This cycle added append-only evidence notes to AC-2, AC-5, and AC-8 and changed no checkbox; git diff shows zero checkbox lines touched. AC-11 still requires the human WinForms designer verification. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Feature-review reaudit at head a62391f, verdict PARTIAL with blocking count changed from 1 to 2: - policy-audit, code-review, feature-audit, and remediation-inputs for the 2026-08-04T22-28 cycle. - remediation-plan.2026-08-05T05-00.md, the cycle-2 plan scoped to the single new blocking finding. - Three feature-review agent-memory entries, including the vstest argument-order transitive-dependency finding. The reaudit verified all of R-2 through R-6 delivered and all seven cycle-1 code-review findings resolved by direct measurement. It then surfaced a new blocking finding that had been present at the previous head and missed in cycle 1: SVGControl.Test fails 6 of 75 tests depending on vstest argument order, because the project references Svg but never ExCSS and legacy packages.config projects do not flow transitive copy-local. Of nine test projects it is the only one whose output lacks ExCSS.dll. That violates UT1 Independence. These artifacts were left uncommitted when the cycle-2 plan was authored, so that plan's tree-state tasks were written against a clean tree that did not exist. Committing them restores the premise rather than encoding an exception for it. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Preflight found the plan was authored against a tree state that did not exist: four reaudit artifacts and three agent-memory files were uncommitted, so the [P0-T5] halt clause would have fired immediately and [P1-T7]'s remedy would have directed the executor to revert another agent's memory writes. Committing the carried-in state as ad60882 restored the premise; this updates the plan's HEAD references to match rather than encoding a permitted-dirt exception. - [P0-T5] and the header move to ad60882 with an empty expected porcelain, and a non-empty result now permits only halt-and-report - never acting on another agent's files. [P1-T7] stays strict; no carried-in set is enumerated. - Decision 5's reuse argument restated: ad60882 adds only markdown and agent memory, no .cs/.csproj/packages.config/app.config, so the 2026-08-05T01-50 series still describes the current source tree exactly. - [P1-T2]'s rationale corrected. packages.config is NOT csharpier-exempt - .csharpierignore covers *.csproj/*.props/*.targets only, and 26 entries in that file are already reflowed. What protects the single-line form is width: entries survive to at least 98 characters and the new one is 62. - [P0-T9] records why UtilitiesSwordfish.Test is excluded from the nine. - [P2-T5] now dispositions diagnostic removals, not just additions: a CoreCompile-gated code that disappears because its project did not recompile is not a regression. - [P1-T4] confirms the HintPath post-build, since Sync-PackageReferences.ps1 rewrites unresolvable HintPaths and a silent rewrite would invalidate [P1-T7]'s added-line count. Counts unchanged at 11/7/12 = 30, no ID moved. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pinning an expected HEAD SHA was the wrong shape: it encodes a fact about the world at authoring time into a document that keeps being edited, so each commit that touched the plan invalidated the plan's own expectation. It rotted once, was fixed by re-pinning, and rotted again one commit later. [P0-T5] now records the HEAD SHA as an observation and gates on three invariants instead: - git status --porcelain is empty; - git diff --stat HEAD is empty for both prior plan files, both read-only; - git diff --name-only a62391f HEAD contains no .cs, .csproj, packages.config, or app.config path. The third is the substantive gate and is strictly stronger than the pin ever was: a SHA match would have told the executor nothing about whether the source tree had diverged. It holds across any number of documentation or agent-memory commits and fails exactly when it should. Decision 5 restated as a condition rather than a SHA, and [P0-T10]'s reuse justification swept for the same rot. The only remaining SHA references are the immutable baseline-capture commit a62391f, used as a diff basis and as the audited-head fact, and one authored-at observation explicitly labelled as such. Counts unchanged at 11/7/12 = 30, no ID moved. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
[P0-T9] required the executor to record that UtilitiesSwordfish.Test's project file is UtilitiesSwordfish.NET.Test.csproj. No such file exists, and no *Swordfish* project file exists anywhere outside packages/ and .claude/. The claim came from reading obj/ cache filenames - the .AssemblyReference.cache entry outlived the tear-down commit - and reporting that inference as a measurement. An executor following the task verbatim would have asserted a nonexistent file as verified fact in an audit artifact. The corrected grounds are strictly stronger: the directory is stale, wholly untracked build output, evidenced by git ls-files returning zero tracked files, no matching project file anywhere in the repository, and bin/Debug holding Swordfish.NET.Test.exe with no *.Test.dll so neither vstest nor the coverage runner can discover it. The tear-down commit is cited by title rather than SHA. Also corrected: three measured figures in [P1-T2]'s rationale (19 not 26 multi-line entries, 97 not 98 characters, 63/65 not 62/64); [P2-T1]'s non-zero branch no longer presupposes a .cs cause, since csharpier also formats packages.config and app.config; and [P1-T7]'s revert remedy now carries the same no-act-on-another-agent's-files prohibition [P0-T5] already had, closing the concurrent-writer route. Counts unchanged at 11/7/12 = 30, no ID moved. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Preflight cleared the cycle-2 plan on pass 3. All three blockers across those passes were defects in the plan's narrative layer, never in its operative layer: commands, edits, insert positions, acceptance thresholds, and halt conditions were correct in every pass, so three passes produced a zero-line change to the intended diff. The structural reason this is not cosmetic: several tasks order an artifact to reproduce their rationale, so a rationale a task requires an artifact to state becomes evidence in the audit trail, and a reaudit that checks it reopens the cycle over prose. The false UtilitiesSwordfish project-file ground was one grep from entering a reference-census artifact as a verified measurement. Recorded rule: rationale clauses an artifact must reproduce are evidence and must be measured before they are written, or stated explicitly as inference. Its specific mechanism gets its own line, because it will recur - obj/*.csproj.AssemblyReference.cache outlives the project it names, so build residue read as proof of existence is a repeatable trap; git ls-files <dir> is the authoritative check. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ependent SVGControl.Test failed 6 of 75 tests depending on vstest argument order. It referenced Svg but never ExCSS, and legacy packages.config projects do not flow transitive copy-local, so of nine test projects it was the only one whose output lacked ExCSS.dll. The test host probes along the first assembly's path, so the suite passed when a sibling ran first and failed when it ran alone or first. That violates UT1 Independence, which requires tests to run in any order. Before, measured on identical binaries verified by SHA-256: - standalone: 75 total, 69 passed, 6 failed, all FileNotFoundException for ExCSS 4.3.2.0 (innermost request 4.2.3.0) - SVGControl.Test first, sibling second: 76 total, 70 passed, 6 failed - sibling first: 76 passed After: - standalone: 75 / 75 / 0 - SVGControl.Test first: 76 / 76 / 0 The fix is one <Reference> and one packages.config line, mirroring what the original plan's P1-T4 already did for Svg for the identical reason. Also adds <Private>True</Private> to the existing Svg reference, which is behavior- preserving since MSBuild already defaults HintPath references to copy-local. Deliberately NOT added: a Fizzler reference, which the cycle inputs directed "for parity with the eight sibling test projects". That justification is false on disk - zero test projects reference Fizzler and none carries Fizzler.dll, so adding it would create divergence rather than parity. Worse, the on-disk identity is 1.3.1.0 while SVGControl.Test/app.config redirects Fizzler to 1.3.0.0, so adding the assembly would activate a stale redirect that is inert today only because no Fizzler.dll is present - the same defect class as #418 itself, already filed separately. Toolchain: analyzer and nullable builds EXIT_CODE 0, zero added diagnostics. One removal, CS2002 in UtilitiesCS.Test, dispositioned non-regressive because it is CoreCompile-gated and that project did not recompile. Suite 6150/6150 across 9 assemblies. Repo line 85.4006% and branch 78.6928%, both above floor. SVGControl package and class coverage figures are byte-identical, because the nine-assembly ordering already supplied ExCSS - the anticipated improvement did not materialize and is recorded as such rather than claimed. AC-11 remains unchecked; it needs the human WinForms designer verification. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Feature-review reaudit at head 69e675d. The ExCSS order-dependence blocker (G-8) is closed, verified by the reviewer running the discriminating command itself rather than reading the evidence: SVGControl.Test standalone returns 75/75/0 against 75/69/6 before the fix, and ExCSS.dll is now present in that project's output. UT1 Independence, UT1 Determinism, UT4, and the C# IDE/CLI parity rule all return to PASS, and AC-10 is upgraded PARTIAL -> PASS. The reviewer confirmed the Fizzler refusal was correct and filed it as a finding against its own cycle-2 remediation inputs rather than against the branch: every clause of the "parity with the eight sibling test projects" justification was false, and complying would have activated a stale binding redirect that is inert only because the file is absent. One blocking item remains, G-2 / AC-11, the human WinForms designer-load runbook. The reviewer verified the human_interaction block directly - H-1 and H-2, both response "exception" with a resolving runbook_path - and deliberately authored no remediation plan, on the grounds that both open items are maintainer decisions with no agent-executable task. Its recommendation is to stop the remediation loop and route to the maintainer; conditional GO, with the code merge-ready once AC-11 is executed or waived. It also corrected artifacts/pr_context.summary.txt again, which reported "Core logic changes: 0 files" against 11 changed C# and build-config files. That misclassification silently disables C# coverage enforcement, because the coverage hook derives its changed-language set from that section. Recurring collector defect, disclosed as G-6. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AC-11 is delivered. The maintainer opened UtilitiesCS/Dialogs/MyBoxViewer.cs in the Visual Studio WinForms designer at head db8b59f: the form renders correctly, there is no NullReferenceException, and the default SVG artwork is visible in the control. All eleven acceptance criteria are now checked. The maintainer reported no detail in the Output window and asked where to find it. That is the correct result rather than a gap: SvgRenderer emits its dual-channel log4net plus Trace diagnostic only on the failure path, and the parse succeeded, so nothing was written. The ambiguity was resolved by asking whether the SVG image itself appeared or the area was blank - a blank area would instead have meant the degrade path fired and a diagnostic should have been present. The image appeared, so the ExCSS bind resolved inside devenv.exe. Three limitations are recorded in the capture rather than glossed: - AC-3's designer-host observability was NOT exercised. Nothing failed, so the diagnostic channel was never driven in devenv.exe. The dual-channel behavior is proven by unit tests and the degrade-without-throwing behavior by the AC-1 regression tests, but "an operator would see it in the VS Output window" remains verified by construction rather than by observation. - Attribution of the successful bind is not established. The binding redirect, the AC-8 directory-probing fallback, and pre-existing shadow-copy presence could each account for it; a pass/fail render cannot separate them. - Open question U-2 stays open. Runbook step 10's ProjectAssemblies observation was not reported. It does not gate AC-11. G-9 is authorized as an exception by the maintainer. The ratified COVERAGE_MEMBER_UNREACHABLE exception for ResolveByNameAndKey is extended to the file-level floor for SvgAssemblyResolver.cs, which that CLR-invoked member now dominates. Scope is that file only; Install() is unaffected at 6/6. The orchestrator disclosed before the decision that the new-file threshold applies only because it sequenced the extraction first to relieve SvgRenderer.cs at 497 of 500 lines, and that the two alternatives - relocating a testable member to lift the ratio, or reverting the extraction - were rejected. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… waiver The confirming feature-review returned PASS with blocking count 0 and found four residual documentation items. Three are fixed here; the fourth needs the maintainer. Retract a false statement in the AC-11 capture. It claimed the dual-channel log4net + Trace behavior is "proven by unit tests in SVGControl.Test". It is not: that project contains zero occurrences of Trace, log4net, Listener, Appender, or DescribeFailure, and no test asserts either channel. The parse-failure tests execute those lines - which is why DescribeFailure measures 100% coverage - but execution is not assertion. The clause was load-bearing, since it was the fallback offered when disclaiming the observability limitation. The corrected basis is static inspection of the implementation, which is what AC-3's operative requirement actually calls for: four paired logger.Error / Trace.TraceError sites with DescribeFailure composing the exception type and message. The degrade-without-throwing behavior IS genuinely proven by the AC-1 regression tests. Correct the U-2 characterization. Recording step 10 as "not reported" read as an operator omission. It was not: the step is explicitly conditional - "Optionally, and only if the designer error page reported a failure to load ExCSS" - and no error page appeared, so the precondition was false and the step was correctly skipped. The runbook was executed in full. Transcribe the G-9 maintainer waiver into issue.md under AC-5. It had been recorded only in artifacts/orchestration/orchestrator-state.json, which is gitignored, so it would have reached neither the pull request nor a fresh clone, and the next coverage audit would have re-derived the finding with no record it was adjudicated. The transcription carries the finding, the authorization, the basis, the scope, the rejected alternatives, and the disclosure that the new-file threshold applies only because the extraction was sequenced first. Verified before writing: no [ExcludeFromCodeCoverage] appears in any changed source file and none was added anywhere in the diff, so this remains a threshold exception rather than a measurement exclusion. Name SVGControl/SvgAssemblyResolver.cs in the coverage-uplift potential entry so the waived file has an owner instead of resurfacing as an unexplained floor failure. No acceptance criterion text or checkbox changed; all eleven remain [x]. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both fields the confirming review flagged as missing are now recorded. Visual Studio Community 2026, product display version 18.8.2, installation version 18.8.12023.21, resolved via vswhere rather than asked for again. Configuration Debug; SVGControl/bin/Debug/SVGControl.dll last built 2026-08-04 23:46:27 UTC, after the final production edit in a62391f. Visual Studio was NOT restarted after the build - the maintainer rebuilt and reopened. Recorded rather than smoothed over, because runbook step 2 exists to guarantee the designer loaded the freshly built assembly instead of one already resident in the devenv.exe AppDomain. The consequence is specific and is now stated: - AC-11 is unaffected. Its criterion is that opening the form loads it without a NullReferenceException. The form was opened and it loaded without one. That is first-hand observation, not inference. - The AC-8 corroboration is weakened from "corroborated" to "corroborated conditionally". Attributing the successful bind to the directory-probing fallback requires that the designer executed the rebuilt assembly. A stale in-process assembly could render identically if the ExCSS bind succeeds on this host for an unrelated reason - for instance ExCSS.dll already resident in the shadow-copy directory, which is exactly what open question U-2 asks and which was not measured. So this capture cannot distinguish "the fix worked" from "the original failure does not reproduce on this host". That second point was already recorded under the attribution limitation; the missing restart is an independent second reason the same attribution cannot be closed, and both are now recorded so they are not mistaken for one. What would close it is a five-minute confirmation - restart VS, reopen the form once, record the result - deliberately not treated as required, because AC-11's stated criterion does not depend on it. Refs: #418 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Suggested title
fix(SVGControl): stop
SvgRenderersurfacing parse failures asNullReferenceExceptionSummary
SvgRenderer.GetSvgDocument(byte[])swallowed every parse exception and returnednull; both byte-array constructors dereferenced that result immediately, so any parse failure surfaced as an opaqueNullReferenceExceptionat control-construction time.log4netandSystem.Diagnostics.Trace, then leaving_docnull without dereferencing it.TryGetSvgDocumentandGetSvgDocumentOrThrow.GetSvgDocumentkeeps its tolerantnull-returning contract, so existing null-tolerant call sites are unchanged.AssemblyResolvefallback gains ordered directory probing, and its exception containment is restored so it cannot throw out of the handler.AssemblyResolveregion and two pure path helpers move toSVGControl/SvgAssemblyResolver.csandSVGControl/SvgAssemblyProbe.cs;SvgRenderer.csdrops from 497 to 362 lines.SVGControl.Testis wired intoTaskMaster.slnfor the first time and gains 27 tests. Suite: 6150/6150 across 9 assemblies.Why
PictureBoxSVGis constructed by designer-generated code in eleven forms, including one that runs inside the Outlook add-in. Opening any of them in the WinForms designer failed to load, and becauseGetSvgDocumentdiscarded the originating exception, the cause was undiagnosable at every call site.The root cause is a binding mismatch: the deployed
Svgassembly carries a reference to anExCSSversion that is not the one deployed, soSvgDocument.OpenthrowsFileNotFoundExceptionin hosts that do not apply the projectapp.configbinding redirects.devenv.exeis such a host. A pre-existingAssemblyResolvefallback was reached but returnednull, becauseAssembly.Loadbinds against the host AppDomain'sApplicationBase— the Visual Studio directory — rather than the directory holdingSVGControl.dll.Two constraints shaped the fix, both recorded in
docs/features/active/2026-08-04-svg-renderer-null-document-nre-418/issue.md:SVGControldeclares alog4netlogger, but there is no evidence an appender is configured insidedevenv.exe, so alog4net-only message could surface nowhere an operator can see it. Hence the dual channel.What Changed
Core fix —
SVGControl/SvgRenderer.csGetSvgDocument(byte[])no longer containstry/catch. It delegates and returnsSvgDocument?, preserving the tolerant contract its existing consumers rely on.TryGetSvgDocument(public, plus an internal seam-bearing overload taking a parse delegate) andGetSvgDocumentOrThrow, which throwsInvalidOperationExceptioncarrying the original exception asInnerException.log4netandTrace, then initialise safely without dereferencing a null document.catch { }was removed.Assembly resolution — new files
SVGControl/SvgAssemblyResolver.csholds the static installer, the re-entrancy guard,ResolveByNameAndKey, and the public-key-token comparison.SvgRenderer's static constructor is retained with its body reduced toSvgAssemblyResolver.Install().SVGControl/SvgAssemblyProbe.csholdsTryGetDirectoryFromCodeBaseandGetProbeDirectories, both pure path-string functions.ResolveByNameAndKeygains a strategy-3Assembly.LoadFromprobe over ordered candidate directories, plus acatchon its outertryso no exception escapes the handler. The invalid-path-character filter applies to all three probe candidates.Tests and build configuration
SVGControl.Testadded toTaskMaster.slnwith all six configuration mappings.SvgRendererParseContractTests.cs,SvgRendererNullToleranceTests.cs,SvgAssemblyProbeDirectoryTests.cs.SvgandExCSS<Reference>items pluspackages.configentries. A non-SDKProjectReferencedoes not flow the referenced project's package assemblies, so both were required explicitly.SVGControl.Test/app.configExCSS binding redirect corrected to the deployed version.<LangVersion>latest</LangVersion>added toSVGControl.Test, eliminating aCS8630that blocked nullable analysis for that project.Architecture / How It Fits Together
PictureBoxSVG→SvgImageSelector→SvgRenderer(byte[], …)is the designer-time construction path. The renderer now has three parse entry points over one shared implementation:GetSvgDocument— tolerant, returnsnull, used by existing consumers.TryGetSvgDocument— reports failure explicitly and surfaces the captured exception. The internal overload accepts aFunc<byte[], SvgDocument?>parse delegate, which is the seam the tests use to drive the null-returning branch deterministically.GetSvgDocumentOrThrow— throws with the original exception attached.SvgAssemblyResolver.Install()is triggered once fromSvgRenderer's static constructor and subscribesResolveByNameAndKeytoAppDomain.CurrentDomain.AssemblyResolve. That handler delegates candidate-directory derivation toSvgAssemblyProbe, which never raises, so the handler cannot fail the bind it is trying to rescue.Verification
Completed
Recorded under
docs/features/active/2026-08-04-svg-renderer-null-document-nre-418/evidence/:csharpier checkexit 0qa-gates/csharpier-check.2026-08-05T05-00.mdqa-gates/analyzer-build.2026-08-05T05-00.mdqa-gates/nullable-build.2026-08-05T05-00.mdqa-gates/test-coverage.2026-08-05T05-00.mdqa-gates/toolchain-clean-pass.2026-08-05T05-00.mdqa-gates/coverage-delta.2026-08-05T05-00.mdregression-testing/ac1-fail-before.2026-08-04T14-36.md) and passing after (regression-testing/ac1-pass-after.2026-08-04T14-36.md).SVGControl.Testpreviously passed or failed depending onvstestargument order, because its output lackedExCSS.dll. Standalone it went from 6 failures to 75/75/0. Seeqa-gates/order-independence.2026-08-05T05-00.md,regression-testing/order-standalone-after.2026-08-05T05-00.md, andregression-testing/order-paired-after.2026-08-05T05-00.md.NullReferenceException. Seeregression-testing/designer-load-2026-08-06T19-47.md.Recommended
Backward Compatibility / Migration Notes
GetSvgDocumentkeeps its signature and its tolerantnull-returning behavior. The new members are additive.NullReferenceExceptionon a parse failure and now degrade to a null document with a logged diagnostic. Callers that relied on the exception — none found in-repo — would need to inspectDocument.SVGControl.Testbecomes a solution member, so it now builds and runs in solution-wide operations for the first time.ResolveByNameAndKeymoved file but remainsprivate staticon a type in the same namespace.Risks and Mitigations
GetSvgDocumentOrThrowavailable where fail-fast is wantedAssemblyResolvehandler could stop installingSvgRenderer's static constructor deliberately retained;Install()measures 6/6 = 100% coverage, so the trigger is exercisedAssemblyResolvehandlertry;SvgAssemblyProbedocumented and tested as never-raising, at 100% line and branchSVGControl.Testentering the solution changes solution-wide build and coverageRollback is a straight revert of the branch; no data migration and no configuration change outside
SVGControl.Test.Review Guide
Suggested order:
SVGControl/SvgRenderer.cs— the substantive change. ReadGetSvgDocument, the twoTryGetSvgDocumentoverloads,GetSvgDocumentOrThrow, and both byte-array constructors.SVGControl/SvgAssemblyResolver.csandSVGControl/SvgAssemblyProbe.cs— largely a move out ofSvgRenderer.cs; the genuinely new logic is strategy-3 probing and the containmentcatch.SVGControl.Test.csproj,packages.config,app.config,TaskMaster.sln,SVGControl.csproj.Noise to expect: the diff is 164 files, but only 11 are code or build configuration (1245 insertions). The remainder is the feature folder — plan, research, audits, and evidence artifacts. The PR-context bundle's "Core logic changes: 0 files" line is a known misclassification by that collector, not a description of this branch.
Follow-ups
Filed rather than folded in, under
docs/features/potential/:2026-08-04-invoke-mstest-scalar-count-strictmode.md—Invoke-MSTest.ps1throws on a single-assembly search root underSet-StrictMode. Kept out because a PowerShell change would pull a second toolchain into a C#-only fix.2026-08-04-stale-fizzler-and-unsafe-binding-redirects.md— twelveFizzlerredirects and oneUnsaferedirect name versions that are not deployed. Latent today; same defect class as this issue.2026-08-05-svgcontrol-coverage-uplift.md— residualSVGControlcoverage, includingSvgAssemblyResolver.cs.Two items carry a maintainer decision, both recorded in
issue.md:SVGControl/SvgAssemblyResolver.cs, whose shortfall is entirely a CLR-invokedAssemblyResolvehandler that cannot be unit-tested without staging an assembly on disk. It is a threshold exception, not a measurement exclusion — no[ExcludeFromCodeCoverage]was added and the lines remain in the repo-wide denominator.GitHub Auto-close