Dynamo Skills Updates

.agents/skills/dynamo-dotnet-expert/SKILL.md

@@ -9,12 +9,13 @@ description: Write and review C#/.NET code in Dynamo following Dynamo coding sta
 
 - Writing or reviewing C#/.NET code in Dynamo.
 - Designing public APIs, choosing types, or making performance decisions.
-- Running or writing NUnit tests.
 - Managing PublicAPI surface files.
 - Refactoring for modern C# patterns.
+- WPF/UI work in `DynamoCoreWpf`.
 
 ## When not to use
 
+- Writing NUnit tests -- use [dynamo-unit-testing](../dynamo-unit-testing/SKILL.md) instead.
 - Architecture / onboarding questions -- use [dynamo-onboarding](../dynamo-onboarding/SKILL.md) instead.
 - PR descriptions -- use [dynamo-pr-description](../dynamo-pr-description/SKILL.md) instead.
 
@@ -28,220 +29,69 @@ Code suggestions with explanations, or a review with specific file/line referenc
 
 ---
 
-## Project Conventions First
+## Workflow
 
-Dynamo follows these standards -- check them before applying generic .NET advice:
+1. **Read before suggesting** — read the relevant file(s) before proposing changes.
+2. **Repo conventions win** — when repo standards conflict with generic .NET advice, follow the repo.
+3. **Check public API impact** — any new public member needs a `PublicAPI.Unshipped.txt` entry.
+4. **Run quality checks** — before finishing, verify against the [quality checklist](./references/quality-checklist.md).
 
-- [Dynamo Coding Standards](https://github.com/DynamoDS/Dynamo/wiki/Coding-Standards)
-- [Dynamo Naming Standards](https://github.com/DynamoDS/Dynamo/wiki/Naming-Standards)
-- `.editorconfig` in the repo (4-space indent, LF line endings, UTF-8)
-- Security analyzers CA2327/CA2329/CA2330/CA2328 are treated as **errors**
+---
 
-When repo conventions conflict with general .NET guidance, repo conventions win.
+## Quick Decision Guide
 
-## Tech Stack
-
-| | Dynamo |
-|---|--------|
-| TFM | net10.0 (full UI: win-x64 only) |
-| Test framework | NUnit |
-| Build | `msbuild src/Dynamo.All.sln` (full) / `dotnet build src/DynamoCore.sln` (core) |
-| IDE | Visual Studio 2022 |
+| Situation | Action |
+|---|---|
+| Adding a public member | Update `PublicAPI.Unshipped.txt` — see [publicapi-guide.md](./assets/publicapi-guide.md) |
+| DTO / immutable data | Use `record` |
+| Small value type (< 16 bytes) | Use `readonly record struct` |
+| Class not designed for inheritance | Mark `sealed` |
+| Async in library code | Add `ConfigureAwait(false)` |
+| User-facing string | Put it in a `.resx` file |
+| New node | Add `.dyn`, `.md`, `.jpg` to `doc/distrib/NodeHelpFiles/` |
+| Breaking public API change | File a GitHub issue first |
 
-## Code Design
+---
 
-### Visibility
+## Project Conventions
 
-Default to minimal exposure: `private` > `internal` > `protected` > `public`. Adding a public member means adding it to `PublicAPI.Unshipped.txt` and committing to backward compatibility.
+- [Dynamo Coding Standards](https://github.com/DynamoDS/Dynamo/wiki/Coding-Standards)
+- [Dynamo Naming Standards](https://github.com/DynamoDS/Dynamo/wiki/Naming-Standards)
+- `.editorconfig` in the repo root defines indent and encoding — check it before editing.
+- Security analyzers CA2327/CA2329/CA2330/CA2328 are treated as **errors**.
 
 ### Naming
 
 - PascalCase for public members, classes, methods.
 - camelCase for private fields and local variables.
-- Prefix private fields with underscore only if the repo already does (Dynamo does not consistently).
-- Pick one style and stay consistent within a file.
+- Prefix private fields with underscore only if the file already does so consistently.
+- XML documentation required on all public methods and properties.
 
 ### Comments
 
-Comments explain *why*, not what. XML documentation is required on all public methods and properties.
-
-### Don't edit auto-generated code
-
-Files matching `/api/*.cs`, `*.g.cs`, or marked `// <auto-generated>` are off-limits.
-
-## Modern C# Patterns
-
-Use modern syntax when the TFM supports it (.NET 10 / C# 14):
-
-- **Records** for DTOs and immutable data: `public record CustomerDto(string Id, string Name);`
-- **Pattern matching** and switch expressions over long if-else chains.
-- **File-scoped namespaces**: `namespace Dynamo.Graph.Workspaces;`
-- **Raw string literals** for multi-line strings.
-- **Collection expressions**: `List<int> items = [1, 2, 3];`
-- **Primary constructors** for simple DI: `public sealed class Service(ILogger<Service> logger)`
-- **Ranges and indices**: `span[^1]`, `array[1..3]`
-
-### Type Design
-
-- **Seal classes** not designed for inheritance -- enables JIT devirtualization.
-- **Use `readonly record struct`** for small value types (avoids defensive copies).
-- **Prefer static pure functions** for logic that doesn't need instance state.
-- **Return immutable collections** from API boundaries: `IReadOnlyList<T>`, `FrozenSet<T>`.
-
-## Error Handling
-
-- Guard early: `ArgumentNullException.ThrowIfNull(x)`, `string.IsNullOrWhiteSpace(x)`.
-- Use precise exception types: `ArgumentException`, `InvalidOperationException` -- never throw base `Exception`.
-- No silent catches: don't swallow errors. Log and rethrow, or let them bubble.
-- No blanket `catch (Exception)` without good reason.
-
-## Async Programming
-
-- All async methods end with `Async`.
-- Always await calls -- no fire-and-forget.
-- Pass `CancellationToken` end-to-end. Call `ThrowIfCancellationRequested()` in loops.
-- Use `ConfigureAwait(false)` in library code; omit in app entry/UI.
-- Prefer `Task` over `ValueTask` unless benchmarks show otherwise.
-- Stream large payloads: `GetAsync(..., ResponseHeadersRead)` then `ReadAsStreamAsync`.
-- **Timeouts**: use a linked `CancellationTokenSource` + `CancelAfter` rather than `Task.WhenAny` — this actually cancels the work instead of abandoning it: `using var linked = CancellationTokenSource.CreateLinkedTokenSource(ct); linked.CancelAfter(TimeSpan.FromSeconds(10));`
-- **Async dispose**: prefer `await using` for streams, HTTP responses, and other async-disposable resources.
-- **Don't wrap needlessly**: if a method only returns another task with no additional logic, return the task directly instead of adding `async`/`await` overhead.
-
-## Immutability
-
-- Prefer records to classes for DTOs.
-- Use `init`-only properties and required members for construction validation.
-- Favor `with` expressions for modifications: `var updated = original with { Name = "new" };`
-
-## Performance
-
-- Simple first; optimize hot paths when measured.
-- Use `Span<T>` / `Memory<T>` / pooling when benchmarks justify it.
-- Avoid LINQ in tight loops -- prefer `foreach` with early exit.
-- Defer enumeration: return `IEnumerable<T>` instead of materializing to `List<T>` when downstream only iterates.
-- Don't allocate in hot paths: prefer `string.Create`, `stackalloc`, or `ArrayPool<T>`.
+Comments explain *why*, not what.
 
-## PublicAPI Management
-
-Dynamo tracks its public API surface using Roslyn analyzers (RS0016, RS0017):
-
-1. New public types, methods, or properties go into `PublicAPI.Unshipped.txt`.
-2. Format: `namespace.ClassName.MemberName -> ReturnType`
-3. On release, entries move to `PublicAPI.Shipped.txt`.
-4. Existing public files: `DynamoCore`, `DynamoUtilities`, `DynamoCoreWpf`, `NodeServices`.
-
-**Breaking changes** (removed/renamed public members, changed signatures) require:
-- Filing a GitHub issue first.
-- Updating [API Changes wiki](https://github.com/DynamoDS/Dynamo/wiki/API-Changes).
-- Following [Semantic Versioning](https://github.com/DynamoDS/Dynamo/wiki/Dynamo-Versions).
-
-### Extend-only design
-
-Once a public API is shipped, treat it as immutable. Add new overloads, new types, opt-in features -- but do not modify or remove existing signatures. Removal only after a deprecation period.
-
-## Testing (NUnit)
-
-Dynamo uses NUnit for all tests. Do not introduce xUnit or MSTest.
-
-### Structure
-
-- Separate test project: `[ProjectName]Tests` (e.g., `DynamoCoreTests`).
-- Mirror source classes: `CatDoor` -> `CatDoorTests`.
-- Name tests by behavior: `WhenCatMeowsThenCatDoorOpens`.
-- One behavior per test. Follow Arrange-Act-Assert.
-
-### Parameterized tests
-
-```csharp
-[TestCase(1, ExpectedResult = 1)]
-[TestCase(5, ExpectedResult = 120)]
-public int Factorial_ReturnsCorrectResult(int n) => MathHelper.Factorial(n);
-```
-
-### Practices
-
-- Tests should run in any order and in parallel.
-- Test through public APIs; don't change visibility for testing.
-- Avoid disk I/O; if needed, randomize paths.
-- New or changed public APIs require tests.
-- Assert specific values, not vague outcomes.
-- Use `Assert.Throws<T>` / `Assert.ThrowsAsync<T>` for exception testing.
-
-### Mocking (Moq)
-
-Dynamo uses Moq for test doubles. Follow these principles:
-
-- **Mock external dependencies only** (services, file system, HTTP) — never mock code whose implementation is part of the solution under test.
-- **Don't change visibility for mocking**: don't add `InternalsVisibleTo` or make methods `virtual` just to enable mocking. Test through public APIs.
-- **Verify mock behavior approximates real behavior**: if a mock returns data, make sure the shape and edge cases match what the real dependency would produce.
-- Keep mock setup minimal. If a test needs elaborate mock configuration, it may be testing too much indirection.
-
-### Running tests
-
-```bash
-# All tests
-dotnet test
-
-# With coverage (requires dotnet-coverage tool)
-dotnet-coverage collect -f cobertura -o coverage.cobertura.xml dotnet test
-```
-
-## Quality Checks (Slopwatch)
+---
 
-After making code changes, verify you haven't introduced shortcuts that mask real problems:
+## Tech Stack
 
-- **Disabled tests**: `[Ignore]`, `[Explicit]` without justification.
-- **Empty catch blocks**: `catch { }` or `catch (Exception) { }` with no logging.
-- **Suppressed warnings**: `#pragma warning disable` without a comment explaining why.
-- **Weakened assertions**: changing `Assert.AreEqual` to `Assert.IsNotNull` or removing assertions.
-- **Magic numbers**: hardcoded values that should be named constants.
+| | Dynamo |
+|---|--------|
+| TFM | net10.0 (full UI: win-x64 only) |
+| Test framework | NUnit |
+| Build (full) | `msbuild src/Dynamo.All.sln` |
+| Build (core) | `dotnet build src/DynamoCore.sln` |
+| IDE | Visual Studio 2022 |
 
-These patterns are signs of reward hacking -- making CI pass without actually fixing the underlying issue.
+---
 
-## Security
+## Assets & References
 
-- Never commit secrets or credentials.
-- No new network connections without documentation and no-network mode testing.
-- No data collection without user consent checks.
-- Security analyzer warnings (CA2327/CA2329/CA2330/CA2328) are treated as errors.
-- User-facing strings must go in `.resx` files for localization.
+- **[csharp-patterns.md](./assets/csharp-patterns.md)** — Modern C#, type design, async, immutability, performance, WPF/UI, examples
+- **[publicapi-guide.md](./assets/publicapi-guide.md)** — PublicAPI.Unshipped.txt workflow, breaking changes, extend-only design
+- **[quality-checklist.md](./references/quality-checklist.md)** — Anti-patterns, security rules, file constraints, PR checklist
 
 ---
 
-**Example 1: Adding a new public method to DynamoCore**
-
-```csharp
-/// <summary>
-/// Returns the workspace name for the given path.
-/// </summary>
-/// <param name="filePath">Absolute path to the .dyn file.</param>
-/// <returns>The workspace name, or null if the file does not exist.</returns>
-public string? GetWorkspaceName(string filePath)
-{
-    ArgumentNullException.ThrowIfNull(filePath);
-    // ...
-}
-```
-
-Then add to `src/DynamoCore/PublicAPI.Unshipped.txt`:
-```
-Dynamo.Core.DynamoModel.GetWorkspaceName(string) -> string?
-```
-
-**Example 2: Reviewing a PR that changes error handling**
-
-Before (bad):
-```csharp
-try { DoWork(); }
-catch { } // swallows all errors
-```
-
-After (good):
-```csharp
-try { DoWork(); }
-catch (InvalidOperationException ex)
-{
-    logger.LogWarning(ex, "DoWork failed for {Input}", input);
-    throw;
-}
-```
+**Related Skills:**
+[dynamo-unit-testing](../dynamo-unit-testing/SKILL.md) • [dynamo-onboarding](../dynamo-onboarding/SKILL.md) • [dynamo-pr-description](../dynamo-pr-description/SKILL.md)

.agents/skills/dynamo-dotnet-expert/assets/csharp-patterns.md

@@ -0,0 +1,115 @@
+# C# Patterns Reference
+
+## Modern C# (use when TFM supports it — .NET 10 / C# 14)
+
+- **Records** for DTOs and immutable data: `public record CustomerDto(string Id, string Name);`
+- **Pattern matching** and switch expressions over long if-else chains.
+- **File-scoped namespaces**: `namespace Dynamo.Graph.Workspaces;`
+- **Raw string literals** for multi-line strings.
+- **Collection expressions**: `List<int> items = [1, 2, 3];`
+- **Primary constructors** for simple DI: `public sealed class Service(ILogger<Service> logger)`
+- **Ranges and indices**: `span[^1]`, `array[1..3]`
+
+---
+
+## Type Design
+
+| Goal | Pattern |
+|---|---|
+| DTO / immutable data | `record` |
+| Small value type (< 16 bytes) | `readonly record struct` |
+| Class not designed for inheritance | `sealed` (enables JIT devirtualization) |
+| API boundary collection | `IReadOnlyList<T>`, `FrozenSet<T>` |
+| Logic with no instance state | `static` pure method |
+
+- Use `init`-only properties and `required` members for construction validation.
+- Favor `with` expressions for modifications: `var updated = original with { Name = "new" };`
+
+---
+
+## Error Handling
+
+- Guard early: `ArgumentNullException.ThrowIfNull(x)`, `string.IsNullOrWhiteSpace(x)`.
+- Use precise exception types: `ArgumentException`, `InvalidOperationException` — never throw base `Exception`.
+- No silent catches: don't swallow errors. Log and rethrow, or let them bubble.
+- No blanket `catch (Exception)` without good reason.
+
+---
+
+## Async Programming
+
+- All async methods end with `Async`.
+- Always await calls — no fire-and-forget.
+- Pass `CancellationToken` end-to-end. Call `ThrowIfCancellationRequested()` in loops.
+- Use `ConfigureAwait(false)` in library code; omit in app entry/UI.
+- Prefer `Task` over `ValueTask` unless benchmarks show otherwise.
+- Stream large payloads: `GetAsync(..., ResponseHeadersRead)` then `ReadAsStreamAsync`.
+- **Timeouts**: use a linked `CancellationTokenSource` + `CancelAfter` rather than `Task.WhenAny` — this actually cancels the work:
+  ```csharp
+  using var linked = CancellationTokenSource.CreateLinkedTokenSource(ct);
+  linked.CancelAfter(TimeSpan.FromSeconds(10));
+  ```
+- **Async dispose**: prefer `await using` for streams, HTTP responses, and other async-disposable resources.
+- **Don't wrap needlessly**: if a method only returns another task with no extra logic, return the task directly — skip the `async`/`await` overhead.
+
+---
+
+## Performance
+
+- Simple first; optimize hot paths only when measured.
+- Use `Span<T>` / `Memory<T>` / pooling when benchmarks justify it.
+- Avoid LINQ in tight loops — prefer `foreach` with early exit.
+- Defer enumeration: return `IEnumerable<T>` instead of materializing to `List<T>` when downstream only iterates.
+- Don't allocate in hot paths: prefer `string.Create`, `stackalloc`, or `ArrayPool<T>`.
+
+---
+
+## WPF / UI (DynamoCoreWpf)
+
+- Follow MVVM: ViewModels hold state and commands; Views bind to them.
+- UI thread access: use `Dispatcher.Invoke` / `DispatcherObject.CheckAccess()` — never touch UI elements from background threads.
+- Implement `INotifyPropertyChanged` via `OnPropertyChanged(nameof(Property))`.
+- Use `RelayCommand` / `DelegateCommand` patterns already present in the codebase — don't introduce new command frameworks.
+- Avoid code-behind logic; keep Views as thin as possible.
+
+---
+
+## Examples
+
+### Adding a new public method to DynamoCore
+
+```csharp
+/// <summary>
+/// Returns the workspace name for the given path.
+/// </summary>
+/// <param name="filePath">Absolute path to the .dyn file.</param>
+/// <returns>The workspace name, or null if the file does not exist.</returns>
+public string? GetWorkspaceName(string filePath)
+{
+    ArgumentNullException.ThrowIfNull(filePath);
+    // ...
+}
+```
+
+Then add to `src/DynamoCore/PublicAPI.Unshipped.txt`:
+```
+Dynamo.Core.DynamoModel.GetWorkspaceName(string) -> string?
+```
+
+### Reviewing error handling in a PR
+
+Before (bad):
+```csharp
+try { DoWork(); }
+catch { } // swallows all errors
+```
+
+After (good):
+```csharp
+try { DoWork(); }
+catch (InvalidOperationException ex)
+{
+    logger.LogWarning(ex, "DoWork failed for {Input}", input);
+    throw;
+}
+```