Refacto/improve code quality (ERD) #12

HandyS11 · 2026-02-03T14:40:45Z

No description provided.

…ntainability

…rty handling

…ing logic

Copilot

Pull request overview

This is a comprehensive refactoring PR focused on improving code quality for ERD (Entity Relationship Diagram) functionality in ProjGraph. The changes include extracting magic strings to constants, consolidating regex patterns, removing relationship labels, improving nullable reference type handling, adding value type tracking, and resolving enum/const default values to their underlying values.

Changes:

Extracted all magic strings and regex patterns to centralized constant classes for better maintainability
Removed the Label property from EfRelationship and updated all relationship rendering to use empty labels
Added IsValueType and IsExplicitlyRequired properties to EfProperty for more accurate constraint rendering
Implemented constant value resolution for default values (enums resolve to numeric values, const strings resolve to their values)
Added support for optional one-to-one relationships in Mermaid ERD syntax
Improved performance with early termination checks and optimized file discovery
Added comprehensive tests for enum default values, const string default values, composite keys, and self-referencing entities
Updated sample context with new one-to-one relationships (Author ↔ Profile, Book ↔ BookDetail) and self-referencing (Author.Mentor)

Reviewed changes

Copilot reviewed 27 out of 28 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
EfAnalysisConstants.cs	New centralized constants file consolidating all magic strings and regex patterns
EfAnalysisRegexPatterns.cs	New centralized regex patterns using source-generated regex
FluentApiConfigurationParser.cs	Major refactoring: uses constants, implements constant value resolution, removed label handling
RelationshipAnalyzer.cs	Removed label property, added self-reference detection, uses constants
NavigationPropertyAnalyzer.cs	Fixed inverse navigation detection to exclude self, removed unused FindInverseCollectionName
EntityAnalyzer.cs	Added IsExplicitlyRequired and IsValueType tracking, uses constants
EntityFileDiscovery.cs	Performance optimizations, removed unused methods (ExtractEntityNamespaces, AddSiblingEntityDirectories)
TypeSymbolExtensions.cs	Added IsEfValueType method, improved IsNullable with better NRT handling
MermaidErdRenderer.cs	Fixed indentation, removed label rendering, added optional one-to-one syntax, improved required constraint logic
EfModel.cs	Removed Label from EfRelationship, added IsValueType and IsExplicitlyRequired to EfProperty
EnumDefaultValueTests.cs	New comprehensive tests for enum default value resolution
ConstStringDefaultValueTests.cs	New tests for const string default value resolution
MarkdownErdTests.cs	New integration test validating README examples match actual output
MyDbContext.cs (sample)	Enhanced with one-to-one relationships and self-referencing examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-03T14:49:17Z

src/ProjGraph.Lib/Services/EfAnalysis/EntityFileDiscovery.cs

+    /// This method starts with the context directory and then its parent directory.
+    /// Since the parent directory scan is recursive, it will naturally include the context directory
+    /// and all siblings. The sibling check is kept as a fallback for non-nested structures.
    /// </remarks>


The comment mentions "The sibling check is kept as a fallback for non-nested structures" but the sibling check (AddSiblingEntityDirectories method) has been removed in this refactoring. The comment should be updated to reflect that only the context directory and its parent directory are searched, and the parent directory scan will naturally include siblings through recursive search.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-03T14:49:18Z

src/ProjGraph.Lib/Services/EfAnalysis/EntityFileDiscovery.cs

+            var options = new EnumerationOptions
+            {
+                RecurseSubdirectories = true, IgnoreInaccessible = true, AttributesToSkip = FileAttributes.System
+            };
+
+            foreach (var csFile in Directory.EnumerateFiles(searchDir, EfAnalysisConstants.FilePatterns.CSharpFiles,
+                         options))
            {
-                if (Path.GetFullPath(csFile).Equals(normalizedContextPath, StringComparison.OrdinalIgnoreCase))
+                var fullPath = Path.GetFullPath(csFile);
+                if (fullPath.Equals(normalizedContextPath, StringComparison.OrdinalIgnoreCase))
                {
                    continue;
                }

-                await ProcessSourceFileAsync(csFile, entityTypeNames, entityFiles);
+                // Skip common non-source directories that can be large
+                var pathSegments = fullPath.Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+                if (pathSegments.Any(s => s is "bin" or "obj" or ".git" or "node_modules"))
+                {
+                    continue;
+                }
+
+                await ProcessSourceFileAsync(fullPath, entityTypeNames, entityFiles);
            }


The current approach enumerates all files including those in bin, obj, .git, and node_modules directories, then filters them out one by one. This is inefficient for large projects. Consider using EnumerationOptions.RecurseSubdirectories = false and implementing manual recursion with directory filtering to avoid descending into these directories at all, or use a different enumeration strategy that can exclude directories during traversal.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-03T14:49:18Z

src/ProjGraph.Lib/Services/EfAnalysis/EntityFileDiscovery.cs

+            var options = new EnumerationOptions
            {
-                baseClassFiles[baseClassName] = foundFile;
-            }
-        }
+                RecurseSubdirectories = true, IgnoreInaccessible = true, MaxRecursionDepth = 10
+            };

-        return baseClassFiles;
-    }
+            foreach (var file in Directory.EnumerateFiles(solutionRoot.FullName, "*.cs", options))
+            {
+                var fullPath = Path.GetFullPath(file);
+                // Skip common non-source directories that can be large
+                var pathSegments = fullPath.Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+                if (pathSegments.Any(s => s is "bin" or "obj" or ".git" or "node_modules"))
+                {
+                    continue;
+                }

-    /// <summary>
-    /// Attempts to find the file path of a base class file within the specified solution root directory.
-    /// </summary>
-    /// <param name="baseClassName">The name of the base class to search for.</param>
-    /// <param name="solutionRoot">The root directory of the solution to search within.</param>
-    /// <returns>
-    /// The full file path of the base class file if found; otherwise, <c>null</c>.
-    /// </returns>
-    /// <remarks>
-    /// This method searches for a file matching the base class name with a ".cs" extension
-    /// in the specified solution root directory and its subdirectories, up to a maximum recursion depth of 10.
-    /// If any access errors occur during the directory enumeration, they are ignored.
-    /// </remarks>
-    private static string? TryFindBaseClassFile(string baseClassName, DirectoryInfo solutionRoot)
-    {
-        try
-        {
-            return Directory.EnumerateFiles(
-                    solutionRoot.FullName,
-                    $"{baseClassName}.cs",
-                    new EnumerationOptions
-                    {
-                        IgnoreInaccessible = true, RecurseSubdirectories = true, MaxRecursionDepth = 10
-                    })
-                .FirstOrDefault();
-        }
-        catch
-        {
-            return null;
-        }
-    }
+                var fileName = Path.GetFileNameWithoutExtension(file);
+                if (!baseClassNames.Contains(fileName))
+                {
+                    continue;
+                }

-    /// <summary>
-    /// Adds sibling directories that are likely to contain entity files to the search directories list.
-    /// </summary>
-    /// <param name="parentPath">The path of the parent directory to search for sibling directories.</param>
-    /// <param name="entityNamespaces">A list of entity namespaces to compare against the directory names.</param>
-    /// <param name="searchDirectories">The list of directories to which the sibling directories will be added.</param>
-    /// <remarks>
-    /// This method attempts to find sibling directories in the parent directory and checks if they are likely
-    /// to represent entity directories based on their names or the provided entity namespaces. If access errors
-    /// occur while enumerating directories, they are ignored.
-    /// </remarks>
-    private static void AddSiblingEntityDirectories(
-        string parentPath,
-        List<string> entityNamespaces,
-        List<string> searchDirectories)
-    {
-        try
-        {
-            searchDirectories.AddRange(
-                from siblingDir in Directory.GetDirectories(parentPath, "*", SearchOption.TopDirectoryOnly)
-                let dirName = Path.GetFileName(siblingDir)
-                where IsLikelyEntityDirectory(dirName, entityNamespaces)
-                select siblingDir);
+                baseClassFiles.TryAdd(fileName, fullPath);
+                if (baseClassFiles.Count == baseClassNames.Count)
+                {
+                    break;
+                }
+            }


Similar to the SearchDirectoryForEntitiesAsync method, this enumerates all files including those in bin, obj, .git, and node_modules directories, then filters them out. This is inefficient for large projects. Consider implementing manual recursion with directory filtering to avoid descending into these directories during enumeration.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-03T14:49:18Z

src/ProjGraph.Lib/Services/ClassAnalysis/WorkspaceTypeDiscovery.cs

+        var options = new EnumerationOptions { RecurseSubdirectories = true, IgnoreInaccessible = true };
+
+        foreach (var file in Directory.EnumerateFiles(directory, "*.cs", options))
        {
+            var fullPath = Path.GetFullPath(file);
+            // Skip common non-source directories
+            var pathSegments = fullPath.Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+            if (pathSegments.Any(s => s is "bin" or "obj" or ".git" or "node_modules"))
+            {
+                continue;
+            }


The enumeration will recurse into bin, obj, .git, and node_modules directories then skip their files individually. For better performance, consider implementing manual recursion to avoid descending into these directories during enumeration.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-03T14:49:18Z

src/ProjGraph.Lib/Services/EfAnalysis/Constants/EfAnalysisConstants.cs

+        public static readonly HashSet<string> ValueTypes = new(StringComparer.OrdinalIgnoreCase)
+        {
+            Int,
+            Int32,
+            Int64,
+            Long,
+            Bool,
+            Boolean,
+            Guid,
+            DateTime,
+            DateTimeOffset,
+            TimeSpan,
+            Decimal,
+            Double,
+            Float,
+            Single,
+            Short
+        };


The ValueTypes HashSet is missing some common .NET value types such as byte, sbyte, ushort, uint, ulong, and char. While these types are less commonly used in Entity Framework models, they should be included for completeness to ensure IsValueType detection works correctly for all value types.

@copilot open a new pull request to apply changes based on this feedback

src/ProjGraph.Lib/Services/EfAnalysis/RelationshipAnalyzer.cs

tests/ProjGraph.Tests.Unit/Services/EfAnalysis/EfAnalysisAdvancedTests.cs

Copilot · 2026-02-03T14:49:19Z

src/ProjGraph.Lib/Services/EfAnalysis/Extensions/TypeSymbolExtensions.cs

+            case NullableAnnotation.Annotated:
+                return true;
+            case NullableAnnotation.NotAnnotated:
+                return false;


The switch statement doesn't explicitly handle the NullableAnnotation.None case. While the fall-through behavior may be intentional (treating None as potentially nullable for reference types), it would be clearer and more maintainable to handle all enum values explicitly. Consider adding a case for NullableAnnotation.None or using a default case with a comment explaining the fall-through behavior.

Suggested change

return false;

return false;

case NullableAnnotation.None:

// Fall through to additional analysis below for legacy/non-NRT contexts

break;

Copilot · 2026-02-03T14:49:20Z

src/ProjGraph.Lib/Services/EfAnalysis/EntityFileDiscovery.cs

+            foreach (var csFile in Directory.EnumerateFiles(searchDir, EfAnalysisConstants.FilePatterns.CSharpFiles,
+                         options))
            {
-                if (Path.GetFullPath(csFile).Equals(normalizedContextPath, StringComparison.OrdinalIgnoreCase))
+                var fullPath = Path.GetFullPath(csFile);
+                if (fullPath.Equals(normalizedContextPath, StringComparison.OrdinalIgnoreCase))
                {
                    continue;
                }

-                await ProcessSourceFileAsync(csFile, entityTypeNames, entityFiles);
+                // Skip common non-source directories that can be large
+                var pathSegments = fullPath.Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+                if (pathSegments.Any(s => s is "bin" or "obj" or ".git" or "node_modules"))
+                {
+                    continue;
+                }
+
+                await ProcessSourceFileAsync(fullPath, entityTypeNames, entityFiles);
            }


This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-03T14:50:27Z

@HandyS11 I've opened a new pull request, #13, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-02-03T14:50:56Z

@HandyS11 I've opened a new pull request, #14, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-02-03T14:51:18Z

@HandyS11 I've opened a new pull request, #15, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-02-03T14:51:32Z

@HandyS11 I've opened a new pull request, #16, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-02-03T14:51:52Z

@HandyS11 I've opened a new pull request, #17, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…cedTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2026-02-03T14:53:34Z

@HandyS11 I've opened a new pull request, #18, to work on those changes. Once the pull request is ready, I'll request review from you.

…ling check (#13) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: HandyS11 <62420910+HandyS11@users.noreply.github.com>

…14) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: HandyS11 <62420910+HandyS11@users.noreply.github.com>

…eration (#16) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: HandyS11 <62420910+HandyS11@users.noreply.github.com>

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: HandyS11 <62420910+HandyS11@users.noreply.github.com>

…eration (#15) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: HandyS11 <62420910+HandyS11@users.noreply.github.com>

…upport

HandyS11 added 6 commits February 2, 2026 16:46

feat: introduce EfAnalysisConstants for improved code quality and mai…

fd334ee

…ntainability

refactor: remove relationship labels and improve code clarity

a59fcec

refactor: enhance file search logic and improve code clarity

07ffa2f

refactor: improve code quality and clarity across multiple files

b9842f4

refactor: enhance code quality by adding IsActive and improving prope…

38da66d

…rty handling

refactor: improve code quality by removing unused methods and optimiz…

aaaf35c

…ing logic

Copilot AI review requested due to automatic review settings February 3, 2026 14:40

Copilot started reviewing on behalf of HandyS11 February 3, 2026 14:41 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 3, 2026

Update BuildSearchDirectories documentation to reflect removal of sibling check #13

Merged

This was referenced Feb 3, 2026

Optimize file enumeration to skip build directories during traversal #14

Merged

Optimize directory traversal to skip excluded directories during enumeration #15

Merged

Copilot AI mentioned this pull request Feb 3, 2026

Optimize directory traversal to skip excluded directories during enumeration #16

Merged

Copilot AI mentioned this pull request Feb 3, 2026

Add missing value types to EfAnalysisConstants #17

Merged

HandyS11 and others added 2 commits February 3, 2026 15:52

Update src/ProjGraph.Lib/Services/EfAnalysis/RelationshipAnalyzer.cs

9d7c1e2

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update tests/ProjGraph.Tests.Unit/Services/EfAnalysis/EfAnalysisAdvan…

a74ebce

…cedTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI mentioned this pull request Feb 3, 2026

Refactor foreach loop to use explicit LINQ mapping #18

Closed

5 tasks

Copilot AI and others added 6 commits February 3, 2026 15:58

Update BuildSearchDirectories documentation to reflect removal of sib…

e72aea1

…ling check (#13) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: HandyS11 <62420910+HandyS11@users.noreply.github.com>

Optimize file enumeration to skip build directories during traversal (#…

9cf639d

…14) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: HandyS11 <62420910+HandyS11@users.noreply.github.com>

Optimize directory traversal to skip excluded directories during enum…

4c650ca

…eration (#16) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: HandyS11 <62420910+HandyS11@users.noreply.github.com>

Add missing value types to EfAnalysisConstants (#17)

781e57d

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: HandyS11 <62420910+HandyS11@users.noreply.github.com>

Optimize directory traversal to skip excluded directories during enum…

99ec6c3

…eration (#15) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: HandyS11 <62420910+HandyS11@users.noreply.github.com>

refactor: enhance Mermaid diagram rendering by adding context title s…

6efc84d

…upport

HandyS11 merged commit 308674d into develop Feb 3, 2026
2 checks passed

HandyS11 deleted the refacto/ImproveCodeQuality branch February 3, 2026 16:17

Refacto/improve code quality (ERD) #12

Refacto/improve code quality (ERD) #12

Uh oh!

Conversation

HandyS11 commented Feb 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

HandyS11 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

HandyS11 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

HandyS11 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

HandyS11 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

HandyS11 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

HandyS11 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 3, 2026

Uh oh!

Copilot AI commented Feb 3, 2026

Uh oh!

Copilot AI commented Feb 3, 2026

Uh oh!

Copilot AI commented Feb 3, 2026

Uh oh!

Copilot AI commented Feb 3, 2026

Uh oh!

Copilot AI commented Feb 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants