DYN-10109 - add debug mode for node docs#16881
DYN-10109 - add debug mode for node docs#16881johnpierson wants to merge 17 commits intoDynamoDS:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a debug-only menu action to audit node help documentation coverage and export results to a CSV via the Documentation Browser view extension.
Changes:
- Adds a new Debug menu item (“Audit Node Help Docs (CSV)”) wired to a click handler in
DynamoView. - Implements CSV audit generation and a Save File dialog in
DocumentationBrowserViewExtension. - Exposes a couple of previously-private
DocumentationBrowserViewModelhelpers for use by the new audit flow.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs | Adds click handler that locates the Documentation Browser extension and invokes an audit method via reflection. |
| src/DynamoCoreWpf/Views/Core/DynamoView.xaml | Adds Debug menu item to trigger the node help docs audit. |
| src/DynamoCoreWpf/Properties/Resources.resx | Adds localized string for the new Debug menu item header. |
| src/DynamoCoreWpf/Properties/Resources.en-US.resx | Adds en-US localized string for the new Debug menu item header. |
| src/DocumentationBrowserViewExtension/Properties/Resources.resx | Adds strings used by the audit workflow (messages, dialog title/filter, CSV header). |
| src/DocumentationBrowserViewExtension/DocumentationBrowserViewModel.cs | Changes helper methods from private to internal so the extension can reuse path logic. |
| src/DocumentationBrowserViewExtension/DocumentationBrowserViewExtension.cs | Implements the audit workflow: collect search entries, resolve docs paths, and write CSV to disk. |
Files not reviewed (2)
- src/DocumentationBrowserViewExtension/Properties/Resources.Designer.cs: Language not supported
- src/DynamoCoreWpf/Properties/Resources.Designer.cs: Language not supported
johnpierson
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-10109
This reverts commit 0a315ec.
This reverts commit 6dc871a.
This reverts commit a94a1ba.
…/johnpierson/Dynamo into dyn10109_addDebugModeForNodeDocs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.
Files not reviewed (2)
- src/DocumentationBrowserViewExtension/Properties/Resources.Designer.cs: Language not supported
- src/DynamoCoreWpf/Properties/Resources.Designer.cs: Language not supported
| _ = Task.Run(() => | ||
| { | ||
| try | ||
| { | ||
| var csv = new StringBuilder(); | ||
| var csvHeader = Resources.NodeHelpAuditCsvHeader; | ||
| if (string.IsNullOrWhiteSpace(csvHeader)) | ||
| { | ||
| csvHeader = "Library,Category,Name,FullName,MissingMd,MissingDyn,MissingImage,MarkdownPath,SampleGraphPath,ImagePaths"; | ||
| } | ||
| csv.AppendLine(csvHeader); | ||
|
|
||
| foreach (var entry in entries) | ||
| { | ||
| try | ||
| { | ||
| var node = entry.CreateNode(); | ||
| var minimumQualifiedName = DynamoViewModel.GetMinimumQualifiedName(node); | ||
| var packageName = ResolvePackageName(entry, packageRoots, packageAssemblies); |
There was a problem hiding this comment.
Background-thread code is calling entry.CreateNode() and DynamoViewModel.GetMinimumQualifiedName(node), which are very likely not thread-safe (and may require the UI thread / Dynamo model thread). This can lead to sporadic crashes or corrupted state. Consider collecting all required per-entry data on the UI thread first (e.g., via Dispatcher), materializing a simple DTO list (names/paths/flags), and only performing file I/O + string building on the background thread.
| catch (Exception) | ||
| { | ||
| } | ||
| }); | ||
| } | ||
| catch (Exception) | ||
| { |
There was a problem hiding this comment.
There are multiple empty catch (Exception) blocks that fully suppress failures (including per-entry failures and file write errors), which will make this feature very difficult to diagnose and also hides user-facing failures. At minimum, log exceptions via OnMessageLogged(...) (you already added NodeHelpAuditEntryFailed / NodeHelpAuditFailed resources) and include the exception message (and ideally stack trace at a debug log level).
| catch (Exception) | |
| { | |
| } | |
| }); | |
| } | |
| catch (Exception) | |
| { | |
| catch (Exception ex) | |
| { | |
| OnMessageLogged(LogMessage.Warning(Resources.NodeHelpAuditEntryFailed + " " + ex.ToString(), WarningLevel.Mild)); | |
| } | |
| }); | |
| } | |
| catch (Exception ex) | |
| { | |
| OnMessageLogged(LogMessage.Warning(Resources.NodeHelpAuditFailed + " " + ex.ToString(), WarningLevel.Mild)); |
| private void OnNodeHelpAuditRequested() | ||
| { | ||
| RunNodeHelpAudit(); | ||
| } |
There was a problem hiding this comment.
The audit is launched as fire-and-forget (_ = Task.Run(...)) with no completion notification, no UI feedback, and no prevention of repeated invocations (multiple concurrent audits can be started). Consider making RunNodeHelpAudit async, awaiting the background work, and emitting a completion message (you already have NodeHelpAuditCompleted) as well as guarding against reentrancy (e.g., a simple in-flight flag).
There was a problem hiding this comment.
☝️ sounds like a valid argument.
| _ = Task.Run(() => | ||
| { | ||
| try | ||
| { | ||
| var csv = new StringBuilder(); | ||
| var csvHeader = Resources.NodeHelpAuditCsvHeader; | ||
| if (string.IsNullOrWhiteSpace(csvHeader)) | ||
| { | ||
| csvHeader = "Library,Category,Name,FullName,MissingMd,MissingDyn,MissingImage,MarkdownPath,SampleGraphPath,ImagePaths"; | ||
| } | ||
| csv.AppendLine(csvHeader); | ||
|
|
||
| foreach (var entry in entries) | ||
| { | ||
| try | ||
| { | ||
| var node = entry.CreateNode(); | ||
| var minimumQualifiedName = DynamoViewModel.GetMinimumQualifiedName(node); | ||
| var packageName = ResolvePackageName(entry, packageRoots, packageAssemblies); | ||
|
|
||
| var mdPath = docManager.GetAnnotationDoc(minimumQualifiedName, packageName) ?? string.Empty; | ||
| var isBuiltInByPath = !string.IsNullOrEmpty(packageName) && ViewModel.IsBuiltInDocPath(mdPath); | ||
| var isOwnedByPackage = !string.IsNullOrEmpty(packageName) && !isBuiltInByPath; | ||
|
|
||
| var sampleGraphPath = string.IsNullOrWhiteSpace(mdPath) | ||
| ? string.Empty | ||
| : ViewModel.DynamoGraphFromMDFilePath(mdPath, isOwnedByPackage); | ||
|
|
||
| var missingMd = string.IsNullOrWhiteSpace(mdPath) || !File.Exists(mdPath); | ||
| var missingDyn = string.IsNullOrWhiteSpace(sampleGraphPath) || !File.Exists(sampleGraphPath); | ||
|
|
||
| var imagePaths = GetImagePathsFromMarkdownFile(mdPath); | ||
| var missingImage = imagePaths.Count > 0 && imagePaths.Any(path => !File.Exists(path)); | ||
| var imagePathsValue = imagePaths.Count == 0 ? string.Empty : string.Join(";", imagePaths); | ||
|
|
||
| var category = entry.FullCategoryName ?? string.Empty; | ||
| var library = GetLibraryName(category); | ||
|
|
||
| csv.AppendLine(string.Join(",", | ||
| EscapeCsv(library), | ||
| EscapeCsv(category), | ||
| EscapeCsv(entry.Name), | ||
| EscapeCsv(entry.FullName), | ||
| missingMd, | ||
| missingDyn, | ||
| missingImage, | ||
| EscapeCsv(mdPath), | ||
| EscapeCsv(sampleGraphPath), | ||
| EscapeCsv(imagePathsValue))); | ||
| } | ||
| catch (Exception) | ||
| { | ||
| } | ||
| } | ||
|
|
||
| File.WriteAllText(targetPath, csv.ToString(), new UTF8Encoding(false)); | ||
| } | ||
| catch (Exception) | ||
| { | ||
| } | ||
| }); |
There was a problem hiding this comment.
The audit is launched as fire-and-forget (_ = Task.Run(...)) with no completion notification, no UI feedback, and no prevention of repeated invocations (multiple concurrent audits can be started). Consider making RunNodeHelpAudit async, awaiting the background work, and emitting a completion message (you already have NodeHelpAuditCompleted) as well as guarding against reentrancy (e.g., a simple in-flight flag).
| _ = Task.Run(() => | |
| { | |
| try | |
| { | |
| var csv = new StringBuilder(); | |
| var csvHeader = Resources.NodeHelpAuditCsvHeader; | |
| if (string.IsNullOrWhiteSpace(csvHeader)) | |
| { | |
| csvHeader = "Library,Category,Name,FullName,MissingMd,MissingDyn,MissingImage,MarkdownPath,SampleGraphPath,ImagePaths"; | |
| } | |
| csv.AppendLine(csvHeader); | |
| foreach (var entry in entries) | |
| { | |
| try | |
| { | |
| var node = entry.CreateNode(); | |
| var minimumQualifiedName = DynamoViewModel.GetMinimumQualifiedName(node); | |
| var packageName = ResolvePackageName(entry, packageRoots, packageAssemblies); | |
| var mdPath = docManager.GetAnnotationDoc(minimumQualifiedName, packageName) ?? string.Empty; | |
| var isBuiltInByPath = !string.IsNullOrEmpty(packageName) && ViewModel.IsBuiltInDocPath(mdPath); | |
| var isOwnedByPackage = !string.IsNullOrEmpty(packageName) && !isBuiltInByPath; | |
| var sampleGraphPath = string.IsNullOrWhiteSpace(mdPath) | |
| ? string.Empty | |
| : ViewModel.DynamoGraphFromMDFilePath(mdPath, isOwnedByPackage); | |
| var missingMd = string.IsNullOrWhiteSpace(mdPath) || !File.Exists(mdPath); | |
| var missingDyn = string.IsNullOrWhiteSpace(sampleGraphPath) || !File.Exists(sampleGraphPath); | |
| var imagePaths = GetImagePathsFromMarkdownFile(mdPath); | |
| var missingImage = imagePaths.Count > 0 && imagePaths.Any(path => !File.Exists(path)); | |
| var imagePathsValue = imagePaths.Count == 0 ? string.Empty : string.Join(";", imagePaths); | |
| var category = entry.FullCategoryName ?? string.Empty; | |
| var library = GetLibraryName(category); | |
| csv.AppendLine(string.Join(",", | |
| EscapeCsv(library), | |
| EscapeCsv(category), | |
| EscapeCsv(entry.Name), | |
| EscapeCsv(entry.FullName), | |
| missingMd, | |
| missingDyn, | |
| missingImage, | |
| EscapeCsv(mdPath), | |
| EscapeCsv(sampleGraphPath), | |
| EscapeCsv(imagePathsValue))); | |
| } | |
| catch (Exception) | |
| { | |
| } | |
| } | |
| File.WriteAllText(targetPath, csv.ToString(), new UTF8Encoding(false)); | |
| } | |
| catch (Exception) | |
| { | |
| } | |
| }); | |
| try | |
| { | |
| var csv = new StringBuilder(); | |
| var csvHeader = Resources.NodeHelpAuditCsvHeader; | |
| if (string.IsNullOrWhiteSpace(csvHeader)) | |
| { | |
| csvHeader = "Library,Category,Name,FullName,MissingMd,MissingDyn,MissingImage,MarkdownPath,SampleGraphPath,ImagePaths"; | |
| } | |
| csv.AppendLine(csvHeader); | |
| foreach (var entry in entries) | |
| { | |
| try | |
| { | |
| var node = entry.CreateNode(); | |
| var minimumQualifiedName = DynamoViewModel.GetMinimumQualifiedName(node); | |
| var packageName = ResolvePackageName(entry, packageRoots, packageAssemblies); | |
| var mdPath = docManager.GetAnnotationDoc(minimumQualifiedName, packageName) ?? string.Empty; | |
| var isBuiltInByPath = !string.IsNullOrEmpty(packageName) && ViewModel.IsBuiltInDocPath(mdPath); | |
| var isOwnedByPackage = !string.IsNullOrEmpty(packageName) && !isBuiltInByPath; | |
| var sampleGraphPath = string.IsNullOrWhiteSpace(mdPath) | |
| ? string.Empty | |
| : ViewModel.DynamoGraphFromMDFilePath(mdPath, isOwnedByPackage); | |
| var missingMd = string.IsNullOrWhiteSpace(mdPath) || !File.Exists(mdPath); | |
| var missingDyn = string.IsNullOrWhiteSpace(sampleGraphPath) || !File.Exists(sampleGraphPath); | |
| var imagePaths = GetImagePathsFromMarkdownFile(mdPath); | |
| var missingImage = imagePaths.Count > 0 && imagePaths.Any(path => !File.Exists(path)); | |
| var imagePathsValue = imagePaths.Count == 0 ? string.Empty : string.Join(";", imagePaths); | |
| var category = entry.FullCategoryName ?? string.Empty; | |
| var library = GetLibraryName(category); | |
| csv.AppendLine(string.Join(",", | |
| EscapeCsv(library), | |
| EscapeCsv(category), | |
| EscapeCsv(entry.Name), | |
| EscapeCsv(entry.FullName), | |
| missingMd, | |
| missingDyn, | |
| missingImage, | |
| EscapeCsv(mdPath), | |
| EscapeCsv(sampleGraphPath), | |
| EscapeCsv(imagePathsValue))); | |
| } | |
| catch (Exception) | |
| { | |
| } | |
| } | |
| File.WriteAllText(targetPath, csv.ToString(), new UTF8Encoding(false)); | |
| } | |
| catch (Exception) | |
| { | |
| } |
| foreach (Match match in Regex.Matches(markdownContent, @"!\[[^\]]*\]\((?<path>[^)\s]+)[^)]*\)", RegexOptions.IgnoreCase)) | ||
| { | ||
| AddImagePath(match.Groups["path"].Value, baseDirectory, results); | ||
| } | ||
|
|
||
| foreach (Match match in Regex.Matches(markdownContent, "<img[^>]+src=[\"'](?<path>[^\"']+)[\"']", RegexOptions.IgnoreCase)) | ||
| { | ||
| AddImagePath(match.Groups["path"].Value, baseDirectory, results); | ||
| } |
There was a problem hiding this comment.
Regex.Matches is used without a timeout and on arbitrary markdown content (including package-provided docs). This can lead to excessive CPU use or hangs on pathological inputs. Consider using precompiled Regex instances with an explicit timeout (via new Regex(pattern, options, timeout)) and reusing them instead of invoking the static Regex.Matches repeatedly.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| if (string.IsNullOrWhiteSpace(csvHeader)) | ||
| { | ||
| csvHeader = "Library,Category,Name,FullName,MissingMd,MissingDyn,MissingImage,MarkdownPath,SampleGraphPath,ImagePaths"; | ||
| } |
There was a problem hiding this comment.
Do we really need this check? If we know we've added the string resource, we can be quite confident that it will be non-null IMO.
| EscapeCsv(row.SampleGraphPath), | ||
| EscapeCsv(imagePathsValue))); | ||
| } | ||
| catch (Exception) |
There was a problem hiding this comment.
What exceptions do you anticipate can be thrown and caught here?
| var results = new List<string>(); | ||
|
|
||
| foreach (Match match in Regex.Matches(markdownContent, @"!\[[^\]]*\]\((?<path>[^)\s]+)[^)]*\)", RegexOptions.IgnoreCase)) | ||
| { |
There was a problem hiding this comment.
I think it would be nice to assign const variable names to refer to these regex strings, something like:
const string imagePathRegex = "!\[[^\]]*\]\((?<path>[^)\s]+)[^)]*\)";
|
Purpose
Add a debug-only “Audit Node Help Docs (CSV)” command that uses Documentation Browser resolution to enumerate visible nodes and export missing .md/.dyn info for help auditing.
Changes:
Adds a new Debug menu item (“Audit Node Help Docs (CSV)”) wired to a click handler in DynamoView.
Implements CSV audit generation and a Save File dialog in DocumentationBrowserViewExtension.
Exposes a couple of previously-private DocumentationBrowserViewModel helpers for use by the new audit flow.
Declarations
Check these if you believe they are true
Release Notes
Add a debug-only node help audit command that exports a CSV of missing documentation and sample graph artifacts.
Reviewers
@DynamoDS/eidos
FYIs
@QilongTang @achintyabhat @Amoursol