CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 658
DYN-8766 - Add local node help lookup as fallback for DNA #16299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8766
This comment was marked as outdated.
This comment was marked as outdated.
eb9f287
to
21c6822
Compare
src/NodeAutoCompleteViewExtension/Services/LocalAutoCompleteService.cs
Outdated
Show resolved
Hide resolved
For the candidate help file we are looking for |
@johnpierson can you recommend some packages that worth installing while testing this ? This is a very nice enhancement overall! LGTM! |
I would say that the following are good (but not sure which ones actually have
|
I fixed this to search based on category and node name. However, clockwork does not ship |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a fallback mechanism to use local help files when ML-based node autocomplete suggestions fail, and refactors several classes to support this extension.
- Introduce
LocalAutoCompleteService
for querying local package and built-in help files. - Update
NodeAutoCompleteBarViewModel
to invoke the local fallback and expose internals. - Extend
SingleResultItem
with a new constructor for port-based instantiation and adjust assembly accessibility.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/NodeAutoCompleteViewExtension/ViewModels/SingleResultItem.cs | Added constructor to create items from a NodeModel and port |
src/NodeAutoCompleteViewExtension/ViewModels/NodeAutoCompleteBarViewModel.cs | Integrated LocalAutoCompleteService fallback in ML lookup and adjusted AddCluster |
src/NodeAutoCompleteViewExtension/Services/LocalAutoCompleteService.cs | New service with methods for package and built-in help file search |
src/DynamoUtilities/Properties/AssemblyInfo.cs | Granted InternalsVisibleTo("NodeAutoCompleteViewExtension") |
src/DynamoCore/Models/DynamoModel.cs | Changed OpenJsonFile from private to internal |
Comments suppressed due to low confidence (6)
src/NodeAutoCompleteViewExtension/Services/LocalAutoCompleteService.cs:79
- The local variable
daName
is assigned but never used. It can be removed to clean up the methodGetNodeFullName
.
var daName = $"{selectedNode.Category}.{selectedNode.GetOriginalName()}";
src/NodeAutoCompleteViewExtension/Services/LocalAutoCompleteService.cs:46
- Add unit tests for
TryGetLocalAutoCompleteResult
, covering package and built-in fallback paths to ensure this new functionality is validated.
public List<SingleResultItem> TryGetLocalAutoCompleteResult(NodeModel selectedNode, PortModel selectedPortModel)
src/DynamoCore/Models/DynamoModel.cs:2434
- Since
OpenJsonFile
is nowinternal
, consider adding or updating its XML doc comment to explain why external callers need access.
internal bool OpenJsonFile(
src/DynamoUtilities/Properties/AssemblyInfo.cs:34
- [nitpick] Add a brief comment explaining why this internal exposure is needed to help future maintainers understand its purpose.
[assembly: InternalsVisibleTo("NodeAutoCompleteViewExtension")]
src/NodeAutoCompleteViewExtension/ViewModels/SingleResultItem.cs:21
- This new constructor lacks an XML doc comment. Consider adding a
<summary>
that explains its parameters and intended use.
public SingleResultItem(NodeModel nodeModel, int portToConnect, double score = 1.0)
src/NodeAutoCompleteViewExtension/ViewModels/NodeAutoCompleteBarViewModel.cs:860
- [nitpick]
var
is used elsewhere in this file for consistency. Consider reverting tovar newNode = ...
or updating other declarations to explicit types for a uniform style.
NodeModel newNode = dynamoModel.CreateNodeFromNameOrType(Guid.NewGuid(), typeInfo.FullName, true);
src/NodeAutoCompleteViewExtension/ViewModels/NodeAutoCompleteBarViewModel.cs
Show resolved
Hide resolved
…rvice.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…arViewModel.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fix for DYF nodes.
…arViewModel.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
a8d4ae9
to
5284371
Compare
Purpose
This PR adds a fallback to local help files when ML-based node autocomplete suggestions are unavailable and refactors internal access to support the extension.
Declarations
Check these if you believe they are true
*.resx
filesRelease Notes
N/A
Reviewers
@DynamoDS/synapse
FYIs
@DynamoDS/eidos