Skip to content

Nlp explanation#171

Merged
kodymoodley merged 22 commits into
mainfrom
nlp-explanation
Sep 16, 2025
Merged

Nlp explanation#171
kodymoodley merged 22 commits into
mainfrom
nlp-explanation

Conversation

@kodymoodley

Copy link
Copy Markdown
Collaborator

This PR adds features to the propose connections page which helps the user validate the proposed connections by the NLP algorithm.

Features

  1. Adds table to the propose connections page with the list of user-specified connections. This creates an integrated view of all connections (proposed and user-specified).
  2. In the proposed connections table, color highlighting is added for tokens that are involved in the semantic text matching process to determine if a connection should exist or not.

@kodymoodley kodymoodley requested a review from sverhoeven July 4, 2025 09:35
@kinsta

kinsta Bot commented Jul 4, 2025

Copy link
Copy Markdown

Preview deployments for INA-tool ⚡️

Status Branch preview Commit preview
✅ Ready Visit preview Visit preview

Commit: 2d23b8b632b6e3533c3e81ab7a7d1e08dd6c5682

Deployment ID: 14ebd5b6-48a4-43ea-842b-9e88e6c472bc

Static site name: ina-tool-5f19y

@sverhoeven

Copy link
Copy Markdown
Collaborator

The loaded example 2 did not give any suggestions. So I changed it to

diff --git a/src/components/LoadExampleButton.tsx b/src/components/LoadExampleButton.tsx
index 48dfd19..fceded6 100644
--- a/src/components/LoadExampleButton.tsx
+++ b/src/components/LoadExampleButton.tsx
@@ -9,7 +9,7 @@ import {
   DropdownMenuItem,
   DropdownMenuTrigger,
 } from "./ui/dropdown-menu";
-import { mockStatements } from "@/nlp/testdata/testData";
+import { statements as statements2 } from "@/nlp/testdata/testData";
 
 const statements: Statement[] = [
   {
@@ -136,7 +136,7 @@ function loadExample() {
 
 function loadExample2() {
   store.getState().setProjectName("Example 2");
-  store.getState().setStatements(mockStatements);
+  store.getState().setStatements(statements2);
   store.getState().setConnections([]);
   store.getState().setConflicts([]);
 }

Now 6 connections are proposed. If correct can you implement this?

@sverhoeven sverhoeven 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.

After correcting load example 2 as described in #171 (comment) I was able to get suggested connections.

The existing connections table renders correctly.

localhost_5173__project=Example+2 (1)
I like how bold and background color can overlap.

This PR could be improved by inline suggestions.

Comment thread src/nlp/testdata/mitigationTestData.ts Outdated
Comment thread src/components/ReadOnlyDrivenConnectionTable.tsx Outdated
Comment thread src/components/ReadOnlyDrivenConnectionTable.tsx Outdated
Comment thread src/components/ReadOnlyDrivenConnectionTable.tsx Outdated
Comment thread src/components/ConflictsTable.tsx
Comment thread src/components/StatementCell.tsx Outdated
Comment thread src/nlp/connectionFinding.ts Outdated
Comment thread src/components/StatementCell.tsx Outdated
Comment thread src/components/ReadOnlyDrivenConnectionTable.tsx Outdated
Comment thread src/components/StatementCell.tsx Outdated
@kodymoodley

Copy link
Copy Markdown
Collaborator Author

Thanks for the thorough list of suggestions @sverhoeven. Will work on them this week.

@kodymoodley

kodymoodley commented Jul 10, 2025

Copy link
Copy Markdown
Collaborator Author

@sverhoeven I have addressed all your comments in the latest commits above. The most interesting was the load example one where I did not even get any proposed connections for Example 1. Not sure if you also experienced this? I therefore had to slightly modify one of the statements for Example 1 to get a proposed connection to appear, and of course any resulting test case results dependent on this dataset.

I also added two additional examples, one for Mitigations network and the other is the Inspections network (both given by Amineh via email). Let me know if this is ready to merge. Edit: actually, I think it's a good idea to ask for Amineh's feedback first before considering merging. I will do that now.

@sverhoeven sverhoeven 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.

Thanks for resolving my requests for change.

For me its good to merge, but let us wait for review by the end-user.

@kodymoodley kodymoodley requested a review from sverhoeven August 26, 2025 08:28

@sverhoeven sverhoeven 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.

Looks good to me.

Only suggestion is to explain the 3 and 4 examples.

Comment thread src/components/LoadExampleButton.tsx
@kodymoodley kodymoodley merged commit 61f60a9 into main Sep 16, 2025
2 checks passed
@kodymoodley kodymoodley deleted the nlp-explanation branch September 16, 2025 11:09
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