fix: address PR review feedback on class diagram parser

Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>

Author: Copilot

src/DiagramForge/Parsers/Mermaid/MermaidClassDiagramParser.cs

@@ -7,20 +7,22 @@ internal sealed class MermaidClassDiagramParser : IMermaidDiagramParser
 {
     // Relationship operators sorted so that longer tokens are tried first when positions tie.
     // Each entry: (token, relationshipType, isReversed).
-    // "isReversed" is not used for edge direction (we always keep source=left, target=right)
-    // but is stored in metadata for potential renderer use.
+    // isReversed=true means the written textual order is source-on-right / target-on-left,
+    // so SourceId and TargetId must be swapped relative to the left/right parse order.
+    // For example: "Animal <|-- Dog" → left=Animal, right=Dog, but Dog IS the child
+    // (semantic source), so isReversed=true causes Edge(Dog, Animal).
     private static readonly (string Token, string RelType, bool IsReversed)[] RelOperators =
     [
-        ("<|--", "inheritance", false),
-        ("--|>", "inheritance", true),
-        ("<|..", "realization", false),
-        ("..|>", "realization", true),
+        ("<|--", "inheritance", true),
+        ("--|>", "inheritance", false),
+        ("<|..", "realization", true),
+        ("..|>", "realization", false),
         ("*--",  "composition", false),
         ("--*",  "composition", true),
         ("o--",  "aggregation", false),
         ("--o",  "aggregation", true),
-        ("<..",  "dependency",  false),
-        ("..>",  "dependency",  true),
+        ("<..",  "dependency",  true),
+        ("..>",  "dependency",  false),
         ("<--",  "association", true),
         ("-->",  "association", false),
         ("..",   "link",        false),
@@ -83,7 +85,7 @@ Node GetOrCreateNode(string id)
 
             // ── Explicit class declaration: "class ClassName" or
             //    "class ClassName[\"label\"]" or "class ClassName {"  ─────────
-            if (line.StartsWith("class ", StringComparison.Ordinal))
+            if (line.StartsWith("class ", StringComparison.OrdinalIgnoreCase))
             {
                 var rest = line[6..].Trim();
                 bool opensBrace = rest.EndsWith('{');
@@ -117,7 +119,7 @@ Node GetOrCreateNode(string id)
 
             // ── Relationship line (try before colon-member to avoid false ────
             //    positives on "ClassName : member" that looks like a target)   ─
-            if (TryParseRelationship(line, builder, GetOrCreateNode, hints))
+            if (TryParseRelationship(line, builder, GetOrCreateNode))
                 continue;
 
             // ── Colon-member: "ClassName : memberDefinition" ─────────────────
@@ -211,8 +213,7 @@ private static bool TryParseColonMember(string line, Func<string, Node> getOrCre
     private static bool TryParseRelationship(
         string line,
         IDiagramSemanticModelBuilder builder,
-        Func<string, Node> getOrCreate,
-        LayoutHints hints)
+        Func<string, Node> getOrCreate)
     {
         var found = FindRelationshipOp(line);
         if (found is null)
@@ -246,7 +247,13 @@ private static bool TryParseRelationship(
         getOrCreate(leftId);
         getOrCreate(rightId);
 
-        var edge = new Edge(leftId, rightId);
+        // When isReversed=true the semantic "from" end is on the right side of the operator
+        // (e.g. "Animal <|-- Dog": Dog IS the child/source, Animal is the parent/target).
+        // Swap so that SourceId always represents the logical origin of the relationship.
+        string sourceId = isReversed ? rightId : leftId;
+        string targetId = isReversed ? leftId : rightId;
+
+        var edge = new Edge(sourceId, targetId);
         if (label is not null)
             edge.Label = new Label(label);
 
@@ -292,8 +299,10 @@ private static ArrowHeadStyle MapArrowHead(string relType) =>
         relType switch
         {
             "association" or "dependency" => ArrowHeadStyle.Arrow,
-            "composition" or "aggregation" => ArrowHeadStyle.Diamond,
-            _ => ArrowHeadStyle.None, // inheritance, realization, link
+            // Composition and aggregation use a diamond marker shape in UML, but the
+            // current renderer has no distinct diamond marker. Use None here and rely on
+            // class:relationshipType metadata for future renderer support.
+            _ => ArrowHeadStyle.None,
         };
 
     // ── Operator lookup ───────────────────────────────────────────────────────

tests/DiagramForge.Tests/Parsers/Mermaid/MermaidClassDiagramParserTests.cs

@@ -361,11 +361,12 @@ class Animal
     [Fact]
     public void Parse_Inheritance_ForwardOperator_CreatesEdge()
     {
+        // "Animal <|-- Dog": Dog is the child/source, Animal is the parent/target.
         var diagram = _parser.Parse("classDiagram\n    Animal <|-- Dog");
 
         var edge = Assert.Single(diagram.Edges);
-        Assert.Equal("Animal", edge.SourceId);
-        Assert.Equal("Dog", edge.TargetId);
+        Assert.Equal("Dog", edge.SourceId);
+        Assert.Equal("Animal", edge.TargetId);
         Assert.Equal("inheritance", edge.Metadata["class:relationshipType"]);
         Assert.Equal(EdgeLineStyle.Solid, edge.LineStyle);
         Assert.Equal(ArrowHeadStyle.None, edge.ArrowHead);
@@ -374,6 +375,7 @@ public void Parse_Inheritance_ForwardOperator_CreatesEdge()
     [Fact]
     public void Parse_Inheritance_ReverseOperator_CreatesEdge()
     {
+        // "Dog --|> Animal": same relationship as <|--, Dog is still the child/source.
         var diagram = _parser.Parse("classDiagram\n    Dog --|> Animal");
 
         var edge = Assert.Single(diagram.Edges);
@@ -392,15 +394,18 @@ public void Parse_Composition_ForwardOperator_CreatesEdge()
         Assert.Equal("Engine", edge.TargetId);
         Assert.Equal("composition", edge.Metadata["class:relationshipType"]);
         Assert.Equal(EdgeLineStyle.Solid, edge.LineStyle);
-        Assert.Equal(ArrowHeadStyle.Diamond, edge.ArrowHead);
+        Assert.Equal(ArrowHeadStyle.None, edge.ArrowHead);
     }
 
     [Fact]
     public void Parse_Composition_ReverseOperator_CreatesEdge()
     {
+        // "Engine --* Car": Car is still the whole/source, Engine is the part/target.
         var diagram = _parser.Parse("classDiagram\n    Engine --* Car");
 
         var edge = Assert.Single(diagram.Edges);
+        Assert.Equal("Car", edge.SourceId);
+        Assert.Equal("Engine", edge.TargetId);
         Assert.Equal("composition", edge.Metadata["class:relationshipType"]);
     }
 
@@ -413,15 +418,18 @@ public void Parse_Aggregation_ForwardOperator_CreatesEdge()
         Assert.Equal("Zoo", edge.SourceId);
         Assert.Equal("Animal", edge.TargetId);
         Assert.Equal("aggregation", edge.Metadata["class:relationshipType"]);
-        Assert.Equal(ArrowHeadStyle.Diamond, edge.ArrowHead);
+        Assert.Equal(ArrowHeadStyle.None, edge.ArrowHead);
     }
 
     [Fact]
     public void Parse_Aggregation_ReverseOperator_CreatesEdge()
     {
+        // "Animal --o Zoo": Zoo is still the whole/source, Animal is the aggregated/target.
         var diagram = _parser.Parse("classDiagram\n    Animal --o Zoo");
 
         var edge = Assert.Single(diagram.Edges);
+        Assert.Equal("Zoo", edge.SourceId);
+        Assert.Equal("Animal", edge.TargetId);
         Assert.Equal("aggregation", edge.Metadata["class:relationshipType"]);
     }
 
@@ -441,9 +449,12 @@ public void Parse_Association_ForwardOperator_CreatesEdge()
     [Fact]
     public void Parse_Association_ReverseOperator_CreatesEdge()
     {
+        // "Food <-- Animal": same as "Animal --> Food"; Animal is still the source.
         var diagram = _parser.Parse("classDiagram\n    Food <-- Animal");
 
         var edge = Assert.Single(diagram.Edges);
+        Assert.Equal("Animal", edge.SourceId);
+        Assert.Equal("Food", edge.TargetId);
         Assert.Equal("association", edge.Metadata["class:relationshipType"]);
     }
 
@@ -476,21 +487,25 @@ public void Parse_Dependency_ForwardOperator_CreatesEdge()
     [Fact]
     public void Parse_Dependency_ReverseOperator_CreatesEdge()
     {
+        // "Service <.. Client": same as "Client ..> Service"; Client is still the dependent/source.
         var diagram = _parser.Parse("classDiagram\n    Service <.. Client");
 
         var edge = Assert.Single(diagram.Edges);
+        Assert.Equal("Client", edge.SourceId);
+        Assert.Equal("Service", edge.TargetId);
         Assert.Equal("dependency", edge.Metadata["class:relationshipType"]);
         Assert.Equal(EdgeLineStyle.Dashed, edge.LineStyle);
     }
 
     [Fact]
     public void Parse_Realization_ForwardOperator_CreatesEdge()
     {
+        // "IFlyable <|.. Bird": Bird is the implementer/source, IFlyable is the interface/target.
         var diagram = _parser.Parse("classDiagram\n    IFlyable <|.. Bird");
 
         var edge = Assert.Single(diagram.Edges);
-        Assert.Equal("IFlyable", edge.SourceId);
-        Assert.Equal("Bird", edge.TargetId);
+        Assert.Equal("Bird", edge.SourceId);
+        Assert.Equal("IFlyable", edge.TargetId);
         Assert.Equal("realization", edge.Metadata["class:relationshipType"]);
         Assert.Equal(EdgeLineStyle.Dashed, edge.LineStyle);
         Assert.Equal(ArrowHeadStyle.None, edge.ArrowHead);
@@ -499,9 +514,12 @@ public void Parse_Realization_ForwardOperator_CreatesEdge()
     [Fact]
     public void Parse_Realization_ReverseOperator_CreatesEdge()
     {
+        // "Bird ..|> IFlyable": same relationship; Bird is still the implementer/source.
         var diagram = _parser.Parse("classDiagram\n    Bird ..|> IFlyable");
 
         var edge = Assert.Single(diagram.Edges);
+        Assert.Equal("Bird", edge.SourceId);
+        Assert.Equal("IFlyable", edge.TargetId);
         Assert.Equal("realization", edge.Metadata["class:relationshipType"]);
     }
 
@@ -612,20 +630,22 @@ Car o-- Wheel
     // ── Operator reversal metadata ────────────────────────────────────────────
 
     [Fact]
-    public void Parse_ForwardInheritanceOp_IsReversedIsFalse()
+    public void Parse_LeftArrowInheritanceOp_OperatorReversedIsTrue()
     {
+        // <|-- has the arrow on the left; isReversed=true causes SourceId/TargetId swap.
         var diagram = _parser.Parse("classDiagram\n    Animal <|-- Dog");
 
         var edge = Assert.Single(diagram.Edges);
-        Assert.Equal(false, edge.Metadata["class:operatorReversed"]);
+        Assert.Equal(true, edge.Metadata["class:operatorReversed"]);
     }
 
     [Fact]
-    public void Parse_ReverseInheritanceOp_IsReversedIsTrue()
+    public void Parse_RightArrowInheritanceOp_OperatorReversedIsFalse()
     {
+        // --|> has the arrow on the right; isReversed=false, no swap needed.
         var diagram = _parser.Parse("classDiagram\n    Dog --|> Animal");
 
         var edge = Assert.Single(diagram.Edges);
-        Assert.Equal(true, edge.Metadata["class:operatorReversed"]);
+        Assert.Equal(false, edge.Metadata["class:operatorReversed"]);
     }
 }