Merge pull request peopleworks#4 from peopleworks/fix/dbset-roster-precision

Narrow the DbSet roster to what the application actually registers

Author: peopleworks

CHANGELOG.md

@@ -16,6 +16,32 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   complete. Only classes declared in the analyzed source qualify, so the framework tables a
   DbContext also registers (`ModuleInfo`, `FileData`, `ModelDifference`) stay out. On a
   221-entity application over a legacy LIMS schema this moves extraction from 3 entities to 210.
+  Thanks to [@MBrekhof](https://github.com/MBrekhof).
+
+- **A `partial` class is one entity, not one per file.** Matching by base class could only ever
+  match once, because one part declares the base list; matching by the `DbSet` roster matches on
+  the name, so every part matched — and the scaffolded split that produces two parts is exactly
+  what the roster is for. The class came out twice, each copy holding half its columns: two
+  incomplete truths with nothing to say they were the same class. The parts are now folded into
+  one entity, which also recovers the members XPO extraction had always dropped where a
+  hand-written part carries `: BaseObject` and a generated part carries the mapping.
+
+- **The `DbSet` roster no longer matches on a bare name.** A name is not an identity: an
+  application may keep a `Contracts.Invoice` DTO beside its `BusinessObjects.Invoice` entity, and
+  the roster turned the DTO into a table. Registrations now carry the namespaces they could have
+  been naming — the registering file's usings, its own namespace, and the namespaces enclosing it
+  — which is ordinary C# lookup, the part of it syntax can see.
+
+- **Business object files are read in a fixed order.** The directory hands them over in whatever
+  order the file system keeps them, and that is not the same order on two machines: NTFS compares
+  names without case, ext4 by byte, so `Shipment.Generated.cs` sorts after `Shipment.cs` on one and
+  before it on the other. Extraction is now ordered by path, so a document regenerated on a laptop
+  and in CI can be compared — which is most of what regenerating it is for.
+
+- **Only a `DbContext`'s own properties count as registrations.** `DbSet<T>` written as a local or
+  a parameter is a type name in a method body, not the application declaring a table. Contexts are
+  found through their base chain as well, so an application whose contexts derive from a shared
+  `AuditedDbContext` still registers everything.
 
 ## [0.12.0] — 2026-08-11
 

README.md

@@ -317,7 +317,7 @@ applications. The agent-facing surface is what is landing now, in the open.
 | ✅ | Pluggable publishing targets (`IDocumentationSink`) |
 | ✅ | **MCP server** — 10 tools, live against your source |
 | ✅ | **Installable Claude Code plugin** with skill and MCP server |
-| ✅ | **275 tests** over synthetic XPO and EF Core fixtures — no DevExpress needed |
+| ✅ | **279 tests** over synthetic XPO and EF Core fixtures — no DevExpress needed |
 | ✅ | **DevExpress ground-truth catalog**, generated locally by licensees |
 
 PeopleWorks Copilot, where this tool grew up, is now one sink among several rather than the

src/XafLogicExplainer.Core/Analyzers/EntityAnalyzer.cs

@@ -30,26 +30,37 @@ public List<ExtractedEntity> AnalyzeEntities(string sourceDirectory, ExtractionO
 
         // Parsed once and kept, because the DbSet roster has to be known before the first class is
         // classified and re-parsing every file to build it costs more than holding the trees.
+        //
+        // Ordered, because the directory hands them over in whatever order the file system keeps
+        // them, and that is not the same order on two machines: NTFS compares names without case,
+        // ext4 by byte, so `Shipment.Generated.cs` sorts after `Shipment.cs` on one and before it
+        // on the other. Anything downstream that takes the first of something then answers
+        // differently on a laptop than in CI, in a document whose value is that it can be
+        // regenerated and compared.
         var parsedFiles = csFiles
+            .OrderBy(file => file, StringComparer.Ordinal)
             .Select(file => (File: file, Root: CSharpSyntaxTree.ParseText(File.ReadAllText(file), path: file).GetRoot()))
             .ToList();
 
-        var registeredTypes = CollectDbSetTypeNames(parsedFiles.Select(parsed => parsed.Root));
+        var roster = DbSetRoster.Read(parsedFiles.Select(parsed => parsed.Root));
 
         foreach (var (file, root) in parsedFiles)
         {
             var classDeclarations = root.DescendantNodes().OfType<ClassDeclarationSyntax>();
             foreach (var classDecl in classDeclarations)
             {
                 if (IsXafBusinessObject(classDecl, options.BaseTypeNames)
-                    || registeredTypes.Contains(classDecl.Identifier.Text))
+                    || roster.Registers(GetNamespace(classDecl), classDecl.Identifier.Text))
                 {
                     var entity = ExtractEntity(classDecl, file, options);
                     entities.Add(entity);
                 }
             }
         }
 
+        // One entity per class, not per declaration -- a partial split across files is one thing.
+        entities = MergePartialDeclarations(entities);
+
         // Post-extraction: infer EF Core relationships from navigation properties
         if (ormType == OrmType.EfCore)
             InferEfCoreRelationships(entities);
@@ -476,7 +487,8 @@ private static OrmType DetectOrmType(IEnumerable<string> csFiles)
     }
 
     /// <summary>
-    /// Collects the type names an application registers as <c>DbSet&lt;T&gt;</c>.
+    /// The types an application registers as <c>DbSet&lt;T&gt;</c> on a <c>DbContext</c>, and the
+    /// namespaces each registration could have been naming.
     /// </summary>
     /// <remarks>
     /// Under EF Core this is the application's own statement of what it persists, and it has to be
@@ -489,27 +501,226 @@ private static OrmType DetectOrmType(IEnumerable<string> csFiles)
     /// are declared in DevExpress assemblies and are therefore never seen here, so they drop out
     /// without needing a list of names to exclude.
     /// </para>
+    /// <para>
+    /// The roster carries a namespace scope rather than a bare name because a bare name is not an
+    /// identity: an application is free to have a <c>Contracts.Invoice</c> DTO beside its
+    /// <c>BusinessObjects.Invoice</c> entity, and telling an agent the DTO is persistent is worse
+    /// than the gap this roster exists to close. What is modelled here is ordinary C# lookup, the
+    /// part of it syntax can see: the registering file's usings, its own namespace, and the
+    /// namespaces enclosing it. Aliases, <c>using static</c> and extern aliases are not modelled —
+    /// a registration that needs one of those finds no class and is dropped, which is the safe
+    /// direction.
+    /// </para>
     /// </remarks>
-    private static HashSet<string> CollectDbSetTypeNames(IEnumerable<SyntaxNode> roots)
+    private sealed class DbSetRoster
     {
-        var names = new HashSet<string>(StringComparer.Ordinal);
+        private readonly List<(string Argument, HashSet<string> Scopes)> _registrations = [];
 
-        foreach (var root in roots)
+        /// <summary>
+        /// Reads every <c>DbSet&lt;T&gt;</c> property declared on a context in the parsed source.
+        /// </summary>
+        public static DbSetRoster Read(IEnumerable<SyntaxNode> roots)
         {
-            foreach (var generic in root.DescendantNodes().OfType<GenericNameSyntax>())
+            var roster = new DbSetRoster();
+            var trees = roots.ToList();
+
+            // `global using` reaches every file, so it has to be gathered before any one file is
+            // read -- including from a GlobalUsings.cs that declares nothing else.
+            var globalUsings = trees
+                .SelectMany(root => root.DescendantNodes().OfType<UsingDirectiveSyntax>())
+                .Where(directive => !directive.GlobalKeyword.IsKind(SyntaxKind.None))
+                .Select(directive => directive.Name?.ToString())
+                .Where(name => name is not null)
+                .Select(name => name!)
+                .ToHashSet(StringComparer.Ordinal);
+
+            var contexts = FindContextClasses(trees);
+
+            foreach (var context in contexts)
             {
-                if (generic.Identifier.Text != "DbSet" || generic.TypeArgumentList.Arguments.Count != 1)
+                var root = context.SyntaxTree.GetRoot();
+                var scopes = new HashSet<string>(globalUsings, StringComparer.Ordinal);
+
+                foreach (var directive in root.DescendantNodes().OfType<UsingDirectiveSyntax>())
+                {
+                    if (directive.Alias is null && directive.Name is not null)
+                        scopes.Add(directive.Name.ToString());
+                }
+
+                // The context's own namespace, and every namespace enclosing it: C# resolves an
+                // unqualified name outwards, so an entity one level up needs no using directive.
+                var contextNamespace = GetNamespace(context);
+                for (var scope = contextNamespace; ; )
+                {
+                    scopes.Add(scope);
+                    var dot = scope.LastIndexOf('.');
+                    if (dot < 0) break;
+                    scope = scope[..dot];
+                }
+
+                foreach (var property in context.Members.OfType<PropertyDeclarationSyntax>())
+                {
+                    if (property.Type is not GenericNameSyntax generic) continue;
+                    if (generic.Identifier.Text != "DbSet") continue;
+                    if (generic.TypeArgumentList.Arguments.Count != 1) continue;
+
+                    var argument = generic.TypeArgumentList.Arguments[0].ToString();
+                    if (argument.Length > 0)
+                        roster._registrations.Add((argument, scopes));
+                }
+            }
+
+            return roster;
+        }
+
+        /// <summary>
+        /// Whether the application registers this class -- by name, and from somewhere that could
+        /// actually have been naming this one.
+        /// </summary>
+        public bool Registers(string @namespace, string className)
+        {
+            foreach (var (argument, scopes) in _registrations)
+            {
+                var dot = argument.LastIndexOf('.');
+                var simpleName = dot < 0 ? argument : argument[(dot + 1)..];
+                if (!string.Equals(simpleName, className, StringComparison.Ordinal)) continue;
+
+                if (dot < 0)
+                {
+                    if (scopes.Contains(@namespace)) return true;
                     continue;
+                }
+
+                // A qualified registration -- DbSet<Contracts.Invoice> -- names its own namespace
+                // tail, and that is more specific than anything the usings could tell us.
+                var qualifier = argument[..dot];
+                if (@namespace.Equals(qualifier, StringComparison.Ordinal)
+                    || @namespace.EndsWith("." + qualifier, StringComparison.Ordinal))
+                    return true;
+            }
+
+            return false;
+        }
+
+        /// <summary>
+        /// The classes that are a <c>DbContext</c>, following base classes declared in the source.
+        /// </summary>
+        /// <remarks>
+        /// Following the chain matters because an application that writes its own
+        /// <c>AuditedDbContext : DbContext</c> and derives every real context from it would
+        /// otherwise register nothing -- the same silent-empty failure this roster is here to fix.
+        /// <para>
+        /// The chain is walked by simple name, which cannot distinguish two same-named classes in
+        /// different namespaces. The cost of being wrong is small in this direction: a class only
+        /// contributes to the roster if it also declares <c>DbSet&lt;T&gt;</c> properties, which
+        /// something that is not a context does not do.
+        /// </para>
+        /// </remarks>
+        private static List<ClassDeclarationSyntax> FindContextClasses(List<SyntaxNode> trees)
+        {
+            var declared = trees
+                .SelectMany(root => root.DescendantNodes().OfType<ClassDeclarationSyntax>())
+                .ToList();
+
+            var contextNames = new HashSet<string>(StringComparer.Ordinal) { "DbContext" };
+
+            // A fixed point, because a base class may be read after the class deriving from it.
+            bool grew;
+            do
+            {
+                grew = false;
+                foreach (var candidate in declared)
+                {
+                    if (contextNames.Contains(candidate.Identifier.Text)) continue;
+                    if (!GetBaseTypeNames(candidate).Any(contextNames.Contains)) continue;
+
+                    contextNames.Add(candidate.Identifier.Text);
+                    grew = true;
+                }
+            } while (grew);
+
+            return declared
+                .Where(candidate => GetBaseTypeNames(candidate).Any(contextNames.Contains))
+                .ToList();
+        }
+    }
 
-                var argument = generic.TypeArgumentList.Arguments[0].ToString();
-                var simpleName = argument[(argument.LastIndexOf('.') + 1)..];
+    /// <summary>
+    /// Folds the parts of a <c>partial</c> class into the one entity they describe.
+    /// </summary>
+    /// <remarks>
+    /// A class matched by its base list could only ever match once, because only one part declares
+    /// the base list. A class matched by the DbSet roster matches on its name, so every part
+    /// matches -- and a scaffolded legacy schema, which is exactly what the roster is for, splits
+    /// its classes as a matter of routine. Left alone that reports the entity twice, each copy
+    /// holding half its properties: two incomplete truths, and no way for a reader to tell they
+    /// are the same class.
+    /// <para>
+    /// Merging also recovers the members XPO extraction has always dropped, where a hand-written
+    /// part carries <c>: BaseObject</c> and a generated part carries half the columns.
+    /// </para>
+    /// </remarks>
+    private static List<ExtractedEntity> MergePartialDeclarations(List<ExtractedEntity> entities)
+    {
+        var parts = new Dictionary<(string Namespace, string ClassName), List<ExtractedEntity>>();
+        var order = new List<(string Namespace, string ClassName)>();
 
-                if (simpleName.Length > 0)
-                    names.Add(simpleName);
+        foreach (var entity in entities)
+        {
+            var key = (entity.Namespace, entity.ClassName);
+            if (!parts.TryGetValue(key, out var group))
+            {
+                parts[key] = group = [];
+                order.Add(key);
             }
+            group.Add(entity);
         }
 
-        return names;
+        var merged = new List<ExtractedEntity>();
+
+        foreach (var key in order)
+        {
+            var group = parts[key];
+
+            // The part declaring the base list is the hand-written one, and it is where the class
+            // attributes live. Taking whichever half came first instead would make the reported
+            // file, and the order of the columns, depend on the file system doing the listing.
+            var primary = group.Find(part => part.BaseTypes.Count > 0) ?? group[0];
+            merged.Add(primary);
+
+            foreach (var entity in group)
+            {
+                if (ReferenceEquals(entity, primary)) continue;
+
+                primary.Description ??= entity.Description;
+                primary.NavigationGroup ??= entity.NavigationGroup;
+                primary.DefaultProperty ??= entity.DefaultProperty;
+                primary.ModelCaption ??= entity.ModelCaption;
+                primary.SourceProject ??= entity.SourceProject;
+                primary.IsDefaultClassOptions |= entity.IsDefaultClassOptions;
+                primary.IsCloneable |= entity.IsCloneable;
+
+                // Non-persistent anywhere means non-persistent: one part saying so is the class.
+                primary.IsPersistent &= entity.IsPersistent;
+
+                if (primary.BaseType is "object" or "" && entity.BaseType is not ("object" or ""))
+                    primary.BaseType = entity.BaseType;
+
+                foreach (var baseType in entity.BaseTypes.Where(name => !primary.BaseTypes.Contains(name)))
+                    primary.BaseTypes.Add(baseType);
+
+                var known = primary.Properties.Select(property => property.Name).ToHashSet(StringComparer.Ordinal);
+                primary.Properties.AddRange(entity.Properties.Where(property => known.Add(property.Name)));
+
+                primary.Relationships.AddRange(entity.Relationships);
+                primary.ValidationRules.AddRange(entity.ValidationRules);
+                primary.AppearanceRules.AddRange(entity.AppearanceRules);
+                primary.InferredBusinessRules.AddRange(entity.InferredBusinessRules);
+                primary.SourceComments.AddRange(entity.SourceComments);
+            }
+        }
+
+        return merged;
     }
 
     private static bool IsXafBusinessObject(ClassDeclarationSyntax classDecl, string[] baseTypeNames)

tests/XafLogicExplainer.Tests/Fixtures/LegacyEfSolution/SampleLegacy.Module/BusinessObjects/ArchiveDbContext.cs

@@ -0,0 +1,28 @@
+using Microsoft.EntityFrameworkCore;
+
+namespace SampleLegacy.Module.BusinessObjects;
+
+/// <summary>
+/// The context every other context in this application derives from.
+/// </summary>
+/// <remarks>
+/// A shared base is how an application puts auditing, soft delete or a connection convention in
+/// one place. It means the contexts that actually register entities do not name
+/// <c>DbContext</c> anywhere in their own declaration.
+/// </remarks>
+public abstract class AuditedDbContext : DbContext
+{
+    protected AuditedDbContext(DbContextOptions options) : base(options) { }
+
+    public override int SaveChanges() => base.SaveChanges();
+}
+
+/// <summary>
+/// Closed invoices, moved off the live tables. Registers through the shared base.
+/// </summary>
+public class ArchiveDbContext : AuditedDbContext
+{
+    public ArchiveDbContext(DbContextOptions<ArchiveDbContext> options) : base(options) { }
+
+    public DbSet<ArchivedInvoice> ArchivedInvoices { get; set; }
+}

tests/XafLogicExplainer.Tests/Fixtures/LegacyEfSolution/SampleLegacy.Module/BusinessObjects/ArchivedInvoice.cs

@@ -0,0 +1,21 @@
+using DevExpress.Persistent.Base;
+using System.ComponentModel.DataAnnotations;
+using System.ComponentModel.DataAnnotations.Schema;
+
+namespace SampleLegacy.Module.BusinessObjects;
+
+/// <summary>
+/// An invoice that has been closed out. Reachable only through a context that never names
+/// <c>DbContext</c> itself.
+/// </summary>
+[Table("invoice_archive")]
+[DefaultClassOptions]
+public partial class ArchivedInvoice : BaseEntity
+{
+    [Key]
+    [Column("Id")]
+    public virtual int Id { get; set; }
+
+    [Column("ClosedOn")]
+    public virtual DateTime ClosedOn { get; set; }
+}

tests/XafLogicExplainer.Tests/Fixtures/LegacyEfSolution/SampleLegacy.Module/BusinessObjects/Contracts/Warehouse.cs

@@ -0,0 +1,17 @@
+namespace SampleLegacy.Module.BusinessObjects.Contracts;
+
+/// <summary>
+/// The shape a warehouse takes on the wire. Nothing persists it, and the DbContext -- one
+/// namespace out, importing nothing from here -- could not be naming it: C# resolves an
+/// unqualified name outwards, never down into a child namespace.
+/// </summary>
+/// <remarks>
+/// It exists to share a simple name with a real entity. A roster of bare names cannot tell the two
+/// apart, and would tell an agent this is a table.
+/// </remarks>
+public class Warehouse
+{
+    public string Code { get; set; } = string.Empty;
+
+    public string Payload { get; set; } = string.Empty;
+}