Skip to content

Read the ORM from syntax, and say Unknown when nothing says - #8

Merged
peopleworks merged 1 commit into
peopleworks:mainfrom
MBrekhof:fix/orm-detection-from-syntax
Aug 13, 2026
Merged

Read the ORM from syntax, and say Unknown when nothing says#8
peopleworks merged 1 commit into
peopleworks:mainfrom
MBrekhof:fix/orm-detection-from-syntax

Conversation

@MBrekhof

Copy link
Copy Markdown

What this changes

DetectOrmType reads the ORM from syntax instead of scanning file text for one namespace, ranks the signals it finds, and returns OrmType.Unknown when the source contains no evidence of either — in which case the persistence ground rule is omitted rather than defaulted.

Why

The old detector scanned raw text for DevExpress.Persistent.BaseImpl.EF and otherwise fell through to OrmType.Xpo. An EF Core application whose entities do not use the DevExpress EF base implementation — mapped onto a legacy schema, security tables in another project — came out as XPO.

The reason this is severe rather than untidy is ground rule 1, and it is severe in your own framing: it tells the agent that DbContext, DbSet<T>, OnModelCreating and EF migrations "do not exist in this application and must never be suggested." Every other extraction gap leaves the agent under-informed. This one forbids the correct answer, in the same voice as the things the tool actually read.

There is a second symptom of the same root cause, which I found by tripping over it. Reading text counts a mention as evidence: a comment, a string literal or an #if-disabled block naming the namespace is enough. The fixture I wrote for this fix passed against the old code, because its own doc comment explained which namespace it was avoiding — and named it. I only caught it because the test passed at the RED step, when it had no business passing. It is now in the fixture's remarks as the reason that comment is worded the way it is.

How it decides

Ranked by what it costs to be wrong about each signal:

  1. A DbSet<T> registered on a context → EF Core. The application cannot run unless this is right, so it is the one signal that is never incidental. This reuses DbSetRoster from Narrow the DbSet roster to what the application actually registers #4 rather than re-deriving it — the roster is now read before the ORM is resolved.
  2. using directives and base classesMicrosoft.EntityFrameworkCore, DevExpress.Persistent.BaseImpl.EF, DevExpress.ExpressApp.EFCore, or : DbContext for EF Core; DevExpress.Xpo or XPObject/XPCustomObject/XPLiteObject/XPBaseObject for XPO.
  3. NeitherUnknown.

Namespaces are compared exactly or as a prefix with the dot, never bare StartsWith: DevExpress.Persistent.BaseImpl is XPO and a prefix of the EF Core namespace, so the naive comparison flips an XPO project to EF Core. Aliased usings are skipped, since an alias does not import the namespace under its own name.

What Unknown renders

The ground rule is not written. In its place:

1. The ORM this application uses could not be determined.
No DbSet<T> registration, DbContext, XPO base class or ORM using directive was found in the analyzed source — so neither persistence style is ruled out here. Check what the project actually references before suggesting Session/XPCollection or DbContext/DbSet<T>, and do not infer one from the other files in this document.

This is the part you offered to take instead, so say the word if you would rather own the wording — I have kept it to a statement of what was not found, on the same principle as the screens section reporting why a controller matched.

Verification

before after
PocoEfSolution — EF Core, no DevExpress EF types XPO EF Core
NoOrmSolution — a module that persists nothing XPO Unknown
XpoSolution — existing fixture XPO XPO
EfCoreSolution, LegacyEfSolution — existing EF Core EF Core
WLNCentral — 196 entities, real legacy LIMS app EF Core EF Core, 196
XPO app on a hand-written base (from #6) XPO XPO

282 passed, 0 failed, dotnet build XAFLogicExplainer.slnx clean at 0 warnings. Three new tests; all three fail on main — I checked, after the false pass described above taught me not to assume it.

The two new fixtures are separate solution folders rather than additions to existing ones, since OrmDetectionTests and LegacyEntityDiscoveryTests assert on those fixtures' exact contents.

Closes #2

Checklist

  • dotnet build XAFLogicExplainer.slnx is clean (CI treats warnings as errors)
  • No DevExpress reference was added to XafLogicExplainer.Core
  • Extraction still works on a project that does not compile — both new fixtures are parsed, never compiled
  • An unrecognized variation is skipped, not thrown on — an unknown using is simply not a signal
  • CHANGELOG.md updated under [Unreleased]

Note

OrmType.Unknown is a new member on a public enum. Anything switching on OrmType in Core now has a third case to consider; xaflogic, the MCP server and the generators are updated here, but it is a breaking change for an outside caller doing exhaustive matching, so it may want the same "breaking for anyone calling Core directly" note that IControllerAnalyzer.AnalyzeControllerFile got in 0.12.0.

Detection scanned raw file text for DevExpress.Persistent.BaseImpl.EF and
otherwise fell through to XPO. An EF Core application whose entities do not use
the DevExpress EF base implementation -- mapped onto a legacy schema, security
tables in another project -- was therefore reported as XPO.

That is not an incomplete document. Ground rule 1 goes on to tell the agent
that DbContext, DbSet<T>, OnModelCreating and EF migrations "do not exist in
this application and must never be suggested", so the guess does not leave the
agent uninformed, it forbids the only correct answer.

Signals are now ranked by what it costs to be wrong about each. A DbSet<T>
registered on a context is the application declaring a table and cannot be
incidental; using directives and base classes are weaker but still deliberate.
Namespaces are compared exactly or as a prefix with a dot, because
DevExpress.Persistent.BaseImpl is XPO and a prefix of the EF Core one.

Where neither ORM leaves a trace, the answer is Unknown and the rule is omitted
rather than defaulted, since stating it on a guess is what made this severe.

Reading text rather than syntax also counted a mention as evidence: a comment
naming the namespace was enough. The fixture for this change passed against the
old code until its own doc comment stopped naming what it was there to avoid.

Fixes peopleworks#2

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment on lines +506 to +521
foreach (var directive in root.DescendantNodes().OfType<UsingDirectiveSyntax>())
{
if (directive.Alias is not null || directive.Name is null)
continue;

var name = directive.Name.ToString();

// Exact or namespace-prefixed, never StartsWith on its own:
// `DevExpress.Persistent.BaseImpl` is XPO and a prefix of the EF Core one.
if (IsOrDescends(name, "Microsoft.EntityFrameworkCore")
|| IsOrDescends(name, "DevExpress.Persistent.BaseImpl.EF")
|| IsOrDescends(name, "DevExpress.ExpressApp.EFCore"))
efCore = true;
else if (IsOrDescends(name, "DevExpress.Xpo"))
xpo = true;
}

@peopleworks peopleworks left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best thing in this PR is not the fix. It is this:

The fixture I wrote for this fix passed against the old code, because its own doc comment explained which namespace it was avoiding — and named it. I only caught it because the test passed at the RED step, when it had no business passing.

Most people never run the test before the fix. The ones who do mostly shrug when it passes. You noticed that a green light you had not earned was itself evidence, chased it, and found a second defect in the same root cause — that reading text counts a mention as proof, so a comment, a string or an #if-disabled block is enough. Then you put the reason into the fixture's remarks so the next person cannot delete the comment without understanding why it is worded that way.

That is the practice this project is trying to have. Everything else here is downstream of it.

The design is right too: signals ranked by what it costs to be wrong about each, the roster reused rather than re-derived, and prefixes compared with the dot so DevExpress.Persistent.BaseImpl does not swallow DevExpress.Persistent.BaseImpl.EF. I would have got that one wrong.

Verified

Built and ran the branch in a clean worktree, Windows, .NET 10, Release:

Passed!  Failed: 0, Passed: 282, Skipped: 0     0 warnings, 0 errors

CI green on both runners. The three new tests fail on main.

The wording of the Unknown ground rule is yours and it should stay yours — you offered to hand it over and I am declining. "Neither persistence style is ruled out here" and "do not infer one from the other files in this document" are both doing real work, and the second one closes a hole I would not have thought to close.

One blocking thing: Unknown does not reach the end

AgentContextGenerator handles it properly. The other two renderers still decide in binary, so an Unknown project comes out as XPO in both:

src/XafLogicExplainer.Mcp/Tools/XafDiscoveryTools.cs:40
    var orm = app.OrmType.Contains("EF", StringComparison.OrdinalIgnoreCase)
        ? "Entity Framework Core" : "XPO";

src/XafLogicExplainer.Core/Generators/HtmlExplainerGenerator.cs:94
    var orm = IsEfCore(project.OrmType) ? "Entity Framework Core" : "XPO";

"Unknown" does not contain "EF", so both fall to the else.

The MCP one is the one that matters, because of where it lands. xaf_overview prints Persistence: **XPO**, two lines above These lists are complete, not sampled, from a tool whose own description tells the agent "The lists are exhaustive: if something is not in them, it does not exist in this application." So the MCP server would state the defect this PR exists to remove, in the most authoritative voice the project has.

It is also new rather than pre-existing. Before this change ResolvedOrm was never Unknown, so those two lines were consistent with the core — wrong, but consistent. Now the core knows better and two of the three surfaces throw the knowledge away.

The fix I would take: one function that knows how to name an ORM, used by all three, rather than three places each re-deciding. OrmDisplayName and IsOrmUnknown already exist in AgentContextGenerator — somewhere shared, and the binary branches deleted rather than extended. That is the same argument as #7, which is about a list written in four places: the bug is not the wrong answer, it is that there were three answers to keep in step.

Two small ones

DetectOrmType has two <summary> blocks. The old one survived above the new:

/// Detects ORM mode by scanning file contents for EF-specific namespaces.

That describes the technique this PR removes, and it is first, so it is the one tooling shows. A stale claim about the code, sitting on the change that falsified it.

LogicExtractor's switch ends _ => "XPO". Correct today, since Unknown is explicit above it. But it means the next member added to OrmType silently becomes XPO, which is the shape of the bug being fixed. OrmType.Xpo => "XPO" with the default arm throwing would make the next person's mistake loud.

On the enum being breaking

Yes — treat it exactly like IControllerAnalyzer.AnalyzeControllerFile in 0.12.0: a Changed entry saying it is breaking for anyone consuming XafLogicExplainer.Core directly and matching exhaustively on OrmType, and unaffected for xaflogic and the MCP server. Good instinct to flag it rather than let it ride under Fixed.

On ordering

You were right that #2 outranks #6, and this lands first because of it. Take #6 whenever suits you — there is no queue behind it.

@peopleworks
peopleworks merged commit f5b5066 into peopleworks:main Aug 13, 2026
6 checks passed
peopleworks added a commit to MBrekhof/XAFLogicExplainer that referenced this pull request Aug 13, 2026
peopleworks#8 taught the core to answer Unknown. The agent files listened; the HTML
explainer and the MCP overview kept deciding in a binary that had no third
answer, so a project whose ORM could not be determined came out as XPO.

The MCP one is the worse of the two. xaf_overview prints the ORM two lines
above "These lists are complete, not sampled", from a tool whose description
tells the agent that anything absent does not exist in the application.

Closes peopleworks#11.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@MBrekhof
MBrekhof deleted the fix/orm-detection-from-syntax branch August 18, 2026 17:25
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.

[gap] EF Core project reported as XPO when entities do not use BaseImpl.EF

4 participants