From dbdac4315c401d1ca14e23a5d11b9dffa149b9b2 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Sat, 12 Sep 2026 18:33:21 +0100 Subject: [PATCH 1/7] feat(ci-versioning): read the declaring assembly from object records Object records carry a type name and nothing else, so attribution has always had to guess ownership from the namespace prefix. Namespaces are shared, so that guess cannot be made correct at any precision. The dataset's new `_asm` field closes the gap; this is the runner half that consumes it. Three changes: - `ParseObjectEventAssembly` reads the declaring assembly and type from a new event shape, consulted only when the Method event yielded nothing so the method path is untouched. - `ProbeTypeCandidates` collects every loaded assembly declaring a given type, so an object record whose type is declared more than once is counted by the existing ambiguity metric instead of being normalised away silently. - No change to year handling. `StripConfigSuffix` already collapses the Revit `_20NN` suffix and `IsFromSubjectAssembly` already compares on it, so this adds tests rather than code. Inert until Versioning_Toolkit emits the event: the runner's only view of the dataset is the TestResult tree, so with nothing emitting, every path is unchanged. The existing 135 tests pass unmodified. --- tools/VersioningRunner/bhom-subject-list.txt | 0 .../ObjectRecordProvenanceTests.cs | 356 ++++++++++++++++++ .../VersioningRunner/Commands/RunCommand.cs | 115 +++++- 3 files changed, 465 insertions(+), 6 deletions(-) create mode 100644 tools/VersioningRunner/bhom-subject-list.txt create mode 100644 tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs diff --git a/tools/VersioningRunner/bhom-subject-list.txt b/tools/VersioningRunner/bhom-subject-list.txt new file mode 100644 index 0000000..e69de29 diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs new file mode 100644 index 0000000..d963270 --- /dev/null +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs @@ -0,0 +1,356 @@ +using System.Reflection; +using VersioningRunner.Commands; +using VersioningRunner.Models; +using VersioningRunner.Tests.Fixtures; +using Xunit; + +namespace VersioningRunner.Tests +{ + // Object records carry a type name and nothing else, so attribution has always guessed from + // the namespace. The dataset's `_asm` field closes that, and these pin the runner half. + // + // Every test here drives a synthetic TestResult, because the runner has no other view of the + // dataset and Versioning_Toolkit does not emit the event yet. That is the limit of this + // coverage: it proves the runner handles the format it defines, not that anything produces it. + public class ObjectRecordProvenanceTests + { + // The contract Versioning_Toolkit's FromJson.cs is bound to emit. + private static string ObjectEvent(string type, string assembly) => + $"Object {type} declared in \"{assembly}\""; + + // A closed generic. Its name contains commas, which is why the quoted part holds the + // assembly alone: a comma-delimited form silently lost all 23 of these in the 9.2 + // dataset. This one is the case that mattered most, because its declaring assembly is + // another repository's while its namespace is the subject's, so losing the field left + // the original misattribution in place. + private const string ClosedGeneric = + "BH.oM.Structure.Results.ResultEnvelope`1[[BH.oM.Structure.Results.ConnectionForce, StructuralEngineering_oM, Version=9.0.0.0, Culture=neutral, PublicKeyToken=null]]"; + + private const string MethodEvent = + "Method ApplyDuctInsulation from { \"_t\" : \"System.Type\", \"Name\" : \"BH.Revit.Engine.MechanicalPlumbing.Compute, Revit_MechanicalPlumbing_Engine_2022, Version=9.0.0.0, Culture=neutral, PublicKeyToken=null\", \"_bhomVersion\" : \"9.2\" } failed to deserialise."; + + private static FakeTestResult Tree(string description, params string[] events) + { + var leaf = new FakeTestInfo + { + Status = "Error", + Description = description, + Message = "Error: Returned null from json.", + Information = events.Select(m => (object)new FakeEventMessage { Message = m }).ToList() + }; + return new FakeTestResult + { + Status = "Error", + Information = [new FakeTestResult { Status = "Error", Information = [leaf] }] + }; + } + + // Subject names arrive as file names with extension, exactly as ReadSubjectAssemblyListFrom + // produces them, so the extension handling is exercised rather than assumed away. + private static ClosureContext ClosureForSubject(params string[] subjectFileNames) + { + var loaded = new HashSet(StringComparer.Ordinal); + return new ClosureContext( + loaded, + new HashSet(loaded.Select(RunCommand.StripConfigSuffix), StringComparer.Ordinal), + new HashSet( + RunCommand.ReadSubjectAssemblyListFrom(subjectFileNames) + .Select(f => RunCommand.StripConfigSuffix(Path.GetFileNameWithoutExtension(f))), + StringComparer.Ordinal)); + } + + // ------------------------------------------------------------------ + // Reading the field + // ------------------------------------------------------------------ + + [Fact] + public void ObjectEvent_YieldsTheDeclaringAssemblyAndType() + { + var (type, assembly) = RunCommand.ParseObjectEventAssembly( + ObjectEvent("BH.oM.Acoustic.Panel", "Acoustic_oM")); + + Assert.Equal("BH.oM.Acoustic.Panel", type); + Assert.Equal("Acoustic_oM", assembly); + } + + [Fact] + public void DeclaringAssemblyFromAnObjectEvent_ReachesTheDiagnostic() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel", ObjectEvent("BH.oM.Acoustic.Panel", "Acoustic_oM")), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics); + + Assert.Equal("Acoustic_oM", Assert.Single(diagnostics).DeclaringAssembly); + } + + // The whole point of the field: this record used to be attributable only by prefix. + [Fact] + public void ObjectRecordNamingTheSubjectsAssembly_IsAttributedByDeclaringAssembly() + { + var closure = ClosureForSubject("Acoustic_oM.dll"); + + var (attributable, basis) = RunCommand.AttributeToSubject( + "BH.oM.Acoustic.Panel", "Acoustic_oM", new HashSet(StringComparer.Ordinal), closure); + + Assert.True(attributable); + Assert.Equal(AttributionBasis.DeclaringAssembly, basis); + } + + [Fact] + public void ObjectRecordNamingAnotherRepositorysAssembly_IsNotAttributed() + { + var closure = ClosureForSubject("Acoustic_oM.dll"); + + var (attributable, basis) = RunCommand.AttributeToSubject( + "BH.oM.Structure.Elements.Panel", "Structure_oM", new HashSet(StringComparer.Ordinal), closure); + + Assert.False(attributable); + Assert.Equal(AttributionBasis.DeclaringAssembly, basis); + } + + // ------------------------------------------------------------------ + // Inertness. These are the tests that say this PR does nothing on its own. + // ------------------------------------------------------------------ + + [Fact] + public void NoObjectEvent_LeavesTheDeclaringAssemblyNull() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel", "Failed to convert the string into a type: BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics); + + Assert.Null(Assert.Single(diagnostics).DeclaringAssembly); + } + + // A record cannot be both, and the method path must not change. If both events are + // present the Method event still wins, because it is read first and the object read is + // only consulted when it yielded nothing. + [Fact] + public void MethodEventStillWins_WhenBothArePresent() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.Revit.Engine.MechanicalPlumbing.Compute. }", + MethodEvent, + ObjectEvent("BH.oM.Acoustic.Panel", "Acoustic_oM")), + (_, _) => (true, AttributionBasis.NotRecorded), null, + (_, _, _) => (null, ClassificationPath.DeclaringTypeNotLoaded, Array.Empty()), diagnostics); + + Assert.Equal("Revit_MechanicalPlumbing_Engine_2022", Assert.Single(diagnostics).DeclaringAssembly); + } + + // Whole closure discards the declaring assembly for attribution: Execute wires + // `(d, _) =>` and supplies no ClosureContext. So on a run with no subject list this + // change cannot alter which findings are reported, only what the artefact records. + // The backfill's own validation runs were whole-closure, so they could not have + // exercised any of the attribution behaviour above. + [Fact] + public void WholeClosure_IgnoresTheDeclaringAssemblyForAttribution() + { + var nsPrefixes = new HashSet(["BH.oM.Acoustic"], StringComparer.Ordinal); + Func wholeClosure = + (d, _) => (RunCommand.IsFromLoadedNamespace(d, nsPrefixes), AttributionBasis.NotRecorded); + + var withAsm = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel", ObjectEvent("BH.oM.Acoustic.Panel", "Somebody_Elses_oM")), + wholeClosure, null, null, withAsm); + + Assert.Equal(AttributionBasis.NotRecorded, Assert.Single(withAsm).AttributedBy); + Assert.True(withAsm[0].CountedAsReal); + } + + // ------------------------------------------------------------------ + // The Revit year. The backfill wrote an arbitrary lowest year on 158 records, and this + // is what makes that harmless rather than a silent misattribution. + // ------------------------------------------------------------------ + + [Theory] + [InlineData("Revit_ModelQA_oM_2022")] + [InlineData("Revit_ModelQA_oM_2023")] + [InlineData("Revit_ModelQA_oM_2024")] + [InlineData("Revit_ModelQA_oM_2025")] + [InlineData("Revit_ModelQA_oM_2026")] + public void AnyYearAttributesToASubjectBuildingAnyOtherYear(string recordedAssembly) + { + // The subject built Release2024, so infer-verification-config staged exactly one year. + var closure = ClosureForSubject("Revit_ModelQA_oM_2024.dll"); + + Assert.True(RunCommand.IsFromSubjectAssembly(recordedAssembly, closure), + $"{recordedAssembly} must attribute to a Release2024 build: all five year variants are " + + "one repository, so the year carries no information about ownership"); + } + + // "More precise" would be wrong here, and this is the shape of the regression it would + // cause: an exact-year match fails in four configurations out of five and the finding is + // dropped as another repository's. Recorded as a test so nobody reintroduces it. + [Fact] + public void TheYearIsNotComparedExactly() + { + var closure = ClosureForSubject("Revit_ModelQA_oM_2024.dll"); + + Assert.True(RunCommand.IsFromSubjectAssembly("Revit_ModelQA_oM_2022", closure)); + Assert.False(RunCommand.IsFromSubjectAssembly("Revit_Tagging_oM_2022", closure)); + } + + // StripConfigSuffix is anchored `_20\d{2}$`, so an `_asm` carrying a file extension does + // not match the anchor, passes through unchanged, and attributes to nobody. This is why + // the backfill wrote bare simple names, matching how method records already express a + // declaring assembly. Pinned so the dataset convention cannot drift without a red test. + [Fact] + public void AnAssemblyNameCarryingAnExtensionDoesNotAttribute() + { + var closure = ClosureForSubject("Acoustic_oM.dll"); + + Assert.True(RunCommand.IsFromSubjectAssembly("Acoustic_oM", closure)); + Assert.False(RunCommand.IsFromSubjectAssembly("Acoustic_oM.dll", closure)); + } + + // SubjectBaseNames is built with StringComparer.Ordinal while the file-name set upstream + // is OrdinalIgnoreCase, so the base-name comparison is case-sensitive where the name + // comparison is not. Not a defect today, because both sides come from the same build + // output, but it is a real edge and it should fail visibly if it ever starts mattering. + [Fact] + public void BaseNameComparisonIsCaseSensitive() + { + var closure = ClosureForSubject("Acoustic_oM.dll"); + + Assert.True(RunCommand.IsFromSubjectAssembly("Acoustic_oM", closure)); + Assert.False(RunCommand.IsFromSubjectAssembly("acoustic_om", closure)); + } + + // ------------------------------------------------------------------ + // Ambiguity on object records + // ------------------------------------------------------------------ + + [Fact] + public void MoreThanOneAssemblyDeclaringTheType_IsRecordedAsCandidates() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel", ObjectEvent("BH.oM.Acoustic.Panel", "Revit_ModelQA_oM_2022")), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + probeTypeCandidates: _ => ["Revit_ModelQA_oM_2022", "Revit_ModelQA_oM_2023"]); + + var only = Assert.Single(diagnostics); + Assert.NotNull(only.DeclaringTypeCandidates); + Assert.Equal(2, only.DeclaringTypeCandidates!.Count); + } + + // One candidate is the ordinary case; carrying it would add a noise row to every finding. + [Fact] + public void ASingleDeclaringAssembly_LeavesCandidatesNull() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel", ObjectEvent("BH.oM.Acoustic.Panel", "Acoustic_oM")), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + probeTypeCandidates: _ => ["Acoustic_oM"]); + + Assert.Null(Assert.Single(diagnostics).DeclaringTypeCandidates); + } + + // Without the object event there is no type to scan, so the probe is never called and + // the pre-existing behaviour stands. This is the other half of inertness. + [Fact] + public void NoObjectEvent_DoesNotScanForCandidates() + { + bool called = false; + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel", "Failed to convert the string into a type: BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + probeTypeCandidates: _ => { called = true; return []; }); + + Assert.False(called, "a record with no object event has no type to scan and must not be probed"); + Assert.Null(Assert.Single(diagnostics).DeclaringTypeCandidates); + } + + [Fact] + public void ProbeTypeCandidates_ReturnsEveryAssemblyDeclaringTheType() + { + var loaded = new List { typeof(RunCommand).Assembly, typeof(object).Assembly }; + + Assert.Equal(["VersioningRunner"], + RunCommand.ProbeTypeCandidates(loaded, typeof(RunCommand).FullName!)); + Assert.Empty(RunCommand.ProbeTypeCandidates(loaded, "No.Such.Type")); + } + + // ------------------------------------------------------------------ + // Malformed input fails to parse rather than capturing something wrong + // ------------------------------------------------------------------ + + [Theory] + [InlineData("Object BH.oM.Acoustic.Panel declared in \"Acoustic_oM")] + [InlineData("Object declared in \"Acoustic_oM\"")] + [InlineData("BH.oM.Acoustic.Panel declared in \"Acoustic_oM\"")] + [InlineData("Object BH.oM.Acoustic.Panel declared in \"\"")] + [InlineData("")] + public void AMalformedObjectEventYieldsNothing(string message) + { + var (type, assembly) = RunCommand.ParseObjectEventAssembly(message); + + Assert.Null(type); + Assert.Null(assembly); + } + + // ------------------------------------------------------------------ + // Closed generics. The class the first version of this contract silently lost. + // ------------------------------------------------------------------ + + [Fact] + public void AClosedGenericTypeNameParses() + { + var (type, assembly) = RunCommand.ParseObjectEventAssembly( + ObjectEvent(ClosedGeneric, "StructuralEngineering_oM")); + + Assert.Equal(ClosedGeneric, type); + Assert.Equal("StructuralEngineering_oM", assembly); + } + + // The commas inside the type argument list are the whole reason the format changed. + [Fact] + public void TheTypeArgumentListDoesNotTruncateTheAssembly() + { + var (_, assembly) = RunCommand.ParseObjectEventAssembly( + ObjectEvent(ClosedGeneric, "StructuralEngineering_oM")); + + Assert.DoesNotContain(",", assembly); + Assert.DoesNotContain("Version=", assembly!); + } + + // The case that mattered: the type's namespace is the subject's, so the namespace guess + // claims it, while the declaring assembly says it is another repository's. Under the old + // format this record fell back and stayed misattributed. + [Fact] + public void AClosedGenericDeclaredElsewhereIsNotAttributedToTheSubject() + { + var closure = ClosureForSubject("Structure_oM.dll"); + var subjectNs = new HashSet(["BH.oM.Structure.Results"], StringComparer.Ordinal); + + var withField = RunCommand.AttributeToSubject( + ClosedGeneric, "StructuralEngineering_oM", subjectNs, closure); + Assert.False(withField.Attributable); + Assert.Equal(AttributionBasis.DeclaringAssembly, withField.Basis); + + // What the fallback does when the field cannot be read, which is what the earlier + // comma-delimited format produced for this record. + var withoutField = RunCommand.AttributeToSubject(ClosedGeneric, null, subjectNs, closure); + Assert.True(withoutField.Attributable); + Assert.Equal(AttributionBasis.NamespaceFallback, withoutField.Basis); + } + + [Fact] + public void AClosedGenericReachesTheDiagnosticEndToEnd() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree(ClosedGeneric, ObjectEvent(ClosedGeneric, "StructuralEngineering_oM")), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics); + + Assert.Equal("StructuralEngineering_oM", Assert.Single(diagnostics).DeclaringAssembly); + } + } +} diff --git a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs index 0a1a324..8367896 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs @@ -134,6 +134,7 @@ public static int Execute( Func Candidates)> probeSignature = (typeFullName, methodName, declaringAssembly) => ProbeDeclaringType(loaded, typeFullName, methodName, declaringAssembly); + Func> probeTypeCandidates = t => ProbeTypeCandidates(loaded, t); foreach (var method in methods) { object? rawResult; @@ -160,7 +161,7 @@ public static int Execute( return 1; } - var partial = ExtractFilteredResult(rawResult, isAttributable, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex); + var partial = ExtractFilteredResult(rawResult, isAttributable, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates); allFailures.AddRange(partial.Failures); } @@ -490,7 +491,8 @@ public static VersioningResult ExtractFilteredResult(object? rawResult, List (IsFromLoadedNamespace(d, nsPrefixes), AttributionBasis.NotRecorded), - typeIndex: BuildLoadedTypeIndex(loaded)); + typeIndex: BuildLoadedTypeIndex(loaded), + probeTypeCandidates: t => ProbeTypeCandidates(loaded, t)); } public static VersioningResult ExtractFilteredResult( @@ -502,7 +504,10 @@ public static VersioningResult ExtractFilteredResult( ClosureContext? closure = null, // Defaults to Empty so a caller that is not exercising resolution keeps the // pre-existing behaviour rather than having findings reclassified underneath it. - LoadedTypeIndex? typeIndex = null) + LoadedTypeIndex? typeIndex = null, + // Null means object-record ambiguity is not scanned, which is the pre-existing + // behaviour and what every caller that does not supply a closure should get. + Func>? probeTypeCandidates = null) { if (rawResult is null) return new VersioningResult @@ -513,7 +518,7 @@ public static VersioningResult ExtractFilteredResult( }; var failures = new List(); - CollectLeafFailures(rawResult, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex ?? LoadedTypeIndex.Empty, depth: 0); + CollectLeafFailures(rawResult, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex ?? LoadedTypeIndex.Empty, probeTypeCandidates, depth: 0); var status = failures.Count > 0 ? VersioningStatus.Error : VersioningStatus.Pass; return new VersioningResult @@ -531,7 +536,7 @@ private static void CollectLeafFailures( List? unresolvableSkips, Func Candidates)>? probeSignature, List? diagnostics, ClosureContext? closure, - LoadedTypeIndex typeIndex, int depth) + LoadedTypeIndex typeIndex, Func>? probeTypeCandidates, int depth) { // BHoM's TestResult tree has at most 3 levels under the root (outer → per-version // summary → individual type result). Depth 5 gives headroom for unexpected nesting @@ -583,6 +588,21 @@ private static void CollectLeafFailures( .Select(ParseMethodEventAssembly) .FirstOrDefault(a => a is not null); + // Object records carry no Method event, so the assembly comes from the dataset's + // `_asm` field instead, surfaced as its own event. Consulted only when the Method + // event yielded nothing, so the method path is untouched: a record cannot be both. + // Null throughout until Versioning_Toolkit emits the event, which is what makes + // this change inert on its own. + string? objectTypeName = null; + if (declaringAssembly is null) + { + var objectEvent = eventMessages + .Select(ParseObjectEventAssembly) + .FirstOrDefault(p => p.Assembly is not null); + objectTypeName = objectEvent.TypeName; + declaringAssembly = objectEvent.Assembly; + } + var (attributable, attributedBy) = isAttributable(desc, declaringAssembly); if (!attributable) return; @@ -609,7 +629,19 @@ private static void CollectLeafFailures( if (cause is null) { if (eventType is null || eventMethod is null) + { path = ClassificationPath.NoMethodEvent; + + // An object record has no method, so the signature probe cannot run and the + // path stays NoMethodEvent. The ambiguity question is still live and is + // answerable from the type alone: if more than one loaded assembly declares + // it, `_asm` resolved to one of several and the run should say so rather + // than normalise it away silently. Without this the metric below can never + // count an object record, because candidates only ever came from the + // method probe. + if (probeTypeCandidates is not null && objectTypeName is not null) + candidates = probeTypeCandidates(objectTypeName); + } else if (probeSignature is not null) (cause, path, candidates) = probeSignature(eventType, eventMethod, declaringAssembly); else @@ -697,7 +729,7 @@ private static void CollectLeafFailures( { string childStatus = child.GetType().GetProperty("Status")?.GetValue(child)?.ToString() ?? "Pass"; if (childStatus is "Error" or "Warning") - CollectLeafFailures(child, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, depth + 1); + CollectLeafFailures(child, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates, depth + 1); } } } @@ -989,6 +1021,77 @@ public static (string? DeclaringType, string? MethodName) ParseMethodEvent(strin return assembly.Length > 0 ? assembly : null; } + // Object records have no Method event, so until now they had no declaring assembly at all + // and always fell to the namespace guess. The dataset's `_asm` field closes that, and this + // is the wire format it arrives in: + // + // Object declared in "" + // + // THIS IS A CONTRACT. Versioning_Toolkit's FromJson.cs must emit exactly this shape for the + // field to be read; nothing else in the runner can see the dataset. + // + // The quoted part holds the assembly ALONE, and that is a correction rather than a + // preference. This first mirrored the Method event's assembly-qualified "Name", + // `", "`, and that shape cannot represent the data: a closed generic + // type name contains commas, e.g. + // + // BH.oM.Structure.Results.ResultEnvelope`1[[BH.oM.Structure.Results.ConnectionForce, + // StructuralEngineering_oM, Version=...]] + // + // so a comma-delimited group stopped at the first argument and the match failed. Measured + // over the 9.2 dataset, the earlier shape parsed 1,690 of 1,713 records and **silently lost + // 23**, each falling back to the namespace guess with no diagnostic. One of the 23 is + // attributed to the wrong repository by that fallback, which is the exact defect this field + // exists to remove. An assembly simple name can never contain a quote, so the assembly-only + // form is unambiguous and parses 1,713 of 1,713. + // + // The type is still carried, in the leading position, where it needs no delimiter. + private static readonly Regex _objectEventAssemblyPattern = new( + @"^Object\s+(?.+?)\s+declared\s+in\s+""(?[^""]+)""$", + RegexOptions.Compiled); + + // The declaring type is returned alongside the assembly because the ambiguity scan needs a + // type name and the leaf's Description is not reliably one: DescriptionFromJson mangles + // some entries. The event states it directly. + public static (string? TypeName, string? Assembly) ParseObjectEventAssembly(string message) + { + if (string.IsNullOrEmpty(message)) + return (null, null); + + var match = _objectEventAssemblyPattern.Match(message); + if (!match.Success) + return (null, null); + + string type = match.Groups["type"].Value.Trim(); + string assembly = match.Groups["assembly"].Value.Trim(); + return (type.Length > 0 ? type : null, assembly.Length > 0 ? assembly : null); + } + + // Every loaded assembly that yields the named type. The type-only half of + // ProbeDeclaringType's candidate collection, for records that have no method to probe. + // + // Not shared with ProbeDeclaringType, deliberately. That loop interleaves the signature + // probe with the candidate walk and takes its verdict from the first assembly that + // answers; splitting it would give the method path a second pass over the closure and + // change code this PR has no reason to touch. + internal static IReadOnlyList ProbeTypeCandidates(List loaded, string typeFullName) + { + var candidates = new List(); + foreach (var asm in loaded) + { + Type? type; + try { type = asm.GetType(typeFullName, throwOnError: false); } + catch { continue; } + + if (type is null) + continue; + + try { candidates.Add(asm.GetName().Name ?? "(unnamed)"); } + catch { candidates.Add("(unnamed)"); } + } + return candidates; + } + // A failure attributed to the subject that could not actually be verified, and the // non-BHoM type identity that made it unverifiable. public readonly record struct UnverifiedFailure(string Description, string Cause); From 5b674268c7af00cc9e0bab0bd63968e3bbdc8fba Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Tue, 29 Sep 2026 02:49:01 +0100 Subject: [PATCH 2/7] feat(ci-versioning): read the declaring assembly from the staged dataset --- .../DatasetProvenanceTests.cs | 355 ++++++++++++++++++ .../Commands/DatasetProvenance.cs | 179 +++++++++ 2 files changed, 534 insertions(+) create mode 100644 tools/VersioningRunner/src/VersioningRunner.Tests/DatasetProvenanceTests.cs create mode 100644 tools/VersioningRunner/src/VersioningRunner/Commands/DatasetProvenance.cs diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/DatasetProvenanceTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/DatasetProvenanceTests.cs new file mode 100644 index 0000000..df3b76f --- /dev/null +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/DatasetProvenanceTests.cs @@ -0,0 +1,355 @@ +using System.Text.Json; +using VersioningRunner.Commands; +using Xunit; + +namespace VersioningRunner.Tests +{ + // The map from a dataset record's type name to the assembly that declared it. + // + // Every fixture here is synthetic and names only public assemblies. Real 9.3 records name + // the oM assemblies of private Revit tools, and a test fixture is as much of a leak as a + // comment is. + public class DatasetProvenanceTests + { + // A closed generic. Its name carries commas and brackets, and it is the shape that the + // earlier message-based route silently lost 23 of in the 9.2 dataset. + private const string ClosedGeneric = + "BH.oM.Structure.Results.ResultEnvelope`1[[BH.oM.Structure.Results.ConnectionForce, StructuralEngineering_oM, Version=9.0.0.0, Culture=neutral, PublicKeyToken=null]]"; + + // ------------------------------------------------------------------ + // The union across versions + // ------------------------------------------------------------------ + + [Fact] + public void AgreedAcrossVersions_MapsTheType() + { + using var datasets = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")) + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal("Acoustic_oM", map.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + Assert.Empty(map.Disputed); + Assert.Equal(2, map.VersionsRead); + } + + // The reason this reads every version rather than the newest. Two of the 205 + // object-record findings across the 15 saved run artefacts resolve only against 9.2, + // and both are NoMethodEvent findings carrying no declaring assembly, which is exactly + // the population the field exists to serve. A newest-only read loses them and falls + // back to the namespace guess without saying so. + [Fact] + public void PresentInOneVersionOnly_IsStillMapped() + { + using var datasets = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Tagging.Settings.TagSettings", "Tagging_oM")) + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal("Tagging_oM", map.DeclaringAssemblyFor("BH.oM.Tagging.Settings.TagSettings")); + Assert.Equal("Acoustic_oM", map.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + } + + [Fact] + public void DifferentFamiliesAcrossVersions_AreDisputedAndNotMapped() + { + using var datasets = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Structure.Elements.Panel", "Structure_oM")) + .WithObjects("9.3", Record("BH.oM.Structure.Elements.Panel", "StructuralEngineering_oM")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Null(map.DeclaringAssemblyFor("BH.oM.Structure.Elements.Panel")); + Assert.False(map.ByTypeName.ContainsKey("BH.oM.Structure.Elements.Panel")); + Assert.Equal(0, map.TypesMapped); + } + + [Fact] + public void ADisputedTypeCarriesEveryAssemblyThatClaimedIt() + { + using var datasets = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Structure.Elements.Panel", "Structure_oM")) + .WithObjects("9.3", Record("BH.oM.Structure.Elements.Panel", "StructuralEngineering_oM")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal( + ["StructuralEngineering_oM", "Structure_oM"], + map.Disputed["BH.oM.Structure.Elements.Panel"]); + } + + // Ordinal-sorted rather than enumeration-ordered, so the warning text and the artefact + // row do not change between two runs over the same datasets. + [Fact] + public void DisputedCandidatesAreOrderedIndependentlyOfTheVersionTheyCameFrom() + { + using var ab = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Acoustic.Panel", "Alpha_oM")) + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Zulu_oM")); + using var ba = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Acoustic.Panel", "Zulu_oM")) + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Alpha_oM")); + + Assert.Equal( + DatasetProvenance.Build(ab.Root).Disputed["BH.oM.Acoustic.Panel"], + DatasetProvenance.Build(ba.Root).Disputed["BH.oM.Acoustic.Panel"]); + } + + // Two years of one repository are not two answers. Attribution strips the year, so + // calling this a disagreement would drop the finding to the namespace guess for no + // gain, which is the failure "compare year-insensitively" exists to prevent. + [Fact] + public void YearVariantsAcrossVersions_AreNotADisagreement() + { + using var datasets = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Tagging.Settings.TagSettings", "Tagging_oM_2022")) + .WithObjects("9.3", Record("BH.oM.Tagging.Settings.TagSettings", "Tagging_oM_2023")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Empty(map.Disputed); + Assert.Equal("Tagging_oM_2022", map.DeclaringAssemblyFor("BH.oM.Tagging.Settings.TagSettings")); + } + + [Fact] + public void AClosedGenericKeyRoundTripsWhole() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record(ClosedGeneric, "StructuralEngineering_oM")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal("StructuralEngineering_oM", map.DeclaringAssemblyFor(ClosedGeneric)); + } + + // ------------------------------------------------------------------ + // Inertness. The state of the tracked dataset before the backfill lands. + // ------------------------------------------------------------------ + + [Fact] + public void RecordsCarryingNoField_LeaveTheMapEmptyWithoutFailing() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", + Record("BH.oM.Acoustic.Panel", assembly: null), + Record("BH.oM.Structure.Elements.Panel", assembly: null)); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Empty(map.ByTypeName); + Assert.Empty(map.Disputed); + Assert.Null(map.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + } + + // The denominator. Without it an empty map reads the same whether the dataset carries + // no field or the runner read nothing at all, and those need different action. + [Fact] + public void RecordsCarryingNoField_AreStillCountedAsRead() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", + Record("BH.oM.Acoustic.Panel", assembly: null), + Record("BH.oM.Structure.Elements.Panel", assembly: null)); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal(2, map.RecordsRead); + Assert.Equal(0, map.TypesMapped); + } + + [Fact] + public void BlankLinesAreNotRecords() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM"), "", " "); + + Assert.Equal(1, DatasetProvenance.Build(datasets.Root).RecordsRead); + } + + // ------------------------------------------------------------------ + // A field that is present but unusable reads as absent + // ------------------------------------------------------------------ + + [Theory] + [InlineData("{ \"_t\" : \"BH.oM.Acoustic.Panel\", \"_asm\" : 7, \"_bhomVersion\" : \"9.3\" }")] + [InlineData("{ \"_t\" : \"BH.oM.Acoustic.Panel\", \"_asm\" : null, \"_bhomVersion\" : \"9.3\" }")] + [InlineData("{ \"_t\" : \"BH.oM.Acoustic.Panel\", \"_asm\" : \"\", \"_bhomVersion\" : \"9.3\" }")] + [InlineData("{ \"_t\" : \"BH.oM.Acoustic.Panel\", \"_asm\" : \" \", \"_bhomVersion\" : \"9.3\" }")] + [InlineData("{ \"_asm\" : \"Acoustic_oM\", \"_bhomVersion\" : \"9.3\" }")] + [InlineData("{ \"_t\" : \"\", \"_asm\" : \"Acoustic_oM\", \"_bhomVersion\" : \"9.3\" }")] + public void AnUnusableRecordMapsNothingAndIsNotAnError(string line) + { + using var datasets = new TempDatasets().WithObjects("9.3", line); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Empty(map.ByTypeName); + Assert.Equal(1, map.RecordsRead); + Assert.Equal(0, map.LinesUnparseable); + } + + // Counted and reported rather than thrown. Losing the whole map over one bad line + // would convert one dataset defect into a fleet-wide fallback to the namespace guess. + [Theory] + [InlineData("{ \"_t\" : \"BH.oM.Acoustic.Panel\" ")] + [InlineData("not json at all")] + [InlineData("[ \"an array, not a record\" ]")] + public void AMalformedLineIsCountedAndSkipped(string line) + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM"), line); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal(1, map.LinesUnparseable); + Assert.Equal(2, map.RecordsRead); + Assert.Equal("Acoustic_oM", map.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + } + + // ------------------------------------------------------------------ + // What is and is not read + // ------------------------------------------------------------------ + + // Both files carry `_t: System.Reflection.MethodBase` on every record, so the key + // identifies nothing, and their leaves already reach the runner with a declaring + // assembly from the Method event. + [Fact] + public void MethodsAndAdaptersAreNotRead() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")) + .WithFile("9.3", "Methods.json", Record("System.Reflection.MethodBase", "Acoustic_Engine")) + .WithFile("9.3", "Adapters.json", Record("System.Reflection.MethodBase", "Acoustic_Adapter")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Null(map.DeclaringAssemblyFor("System.Reflection.MethodBase")); + Assert.Equal(1, map.RecordsRead); + } + + [Fact] + public void AVersionWithoutAnObjectsJsonIsSkippedRatherThanFatal() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")) + .WithFile("9.2", "Methods.json", Record("System.Reflection.MethodBase", "Acoustic_Engine")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal(1, map.VersionsRead); + Assert.Equal("Acoustic_oM", map.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + } + + // ------------------------------------------------------------------ + // The two states that are a broken precondition rather than an empty result + // ------------------------------------------------------------------ + + [Fact] + public void AnAbsentRootThrows() + { + string missing = Path.Combine(Path.GetTempPath(), "versioning-runner-no-such-datasets-" + Guid.NewGuid()); + + var thrown = Assert.Throws(() => DatasetProvenance.Build(missing)); + Assert.Contains(missing, thrown.Message, StringComparison.Ordinal); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void AnUnsuppliedRootThrows(string? root) + { + Assert.Throws(() => DatasetProvenance.Build(root)); + } + + [Fact] + public void ARootWithNoObjectsJsonAnywhereThrows() + { + using var datasets = new TempDatasets() + .WithFile("9.2", "Methods.json", Record("System.Reflection.MethodBase", "Acoustic_Engine")) + .WithFile("9.3", "Adapters.json", Record("System.Reflection.MethodBase", "Acoustic_Adapter")); + + Assert.Throws(() => DatasetProvenance.Build(datasets.Root)); + } + + [Fact] + public void ARootWithNoVersionDirectoriesThrows() + { + using var datasets = new TempDatasets(); + + Assert.Throws(() => DatasetProvenance.Build(datasets.Root)); + } + + // ------------------------------------------------------------------ + // Lookup + // ------------------------------------------------------------------ + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("No.Such.Type")] + public void LookupOfSomethingNotMappedYieldsNull(string? typeName) + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")); + + Assert.Null(DatasetProvenance.Build(datasets.Root).DeclaringAssemblyFor(typeName)); + } + + [Fact] + public void TheEmptyMapAnswersNullRatherThanThrowing() + { + Assert.Null(DeclaringAssemblyMap.Empty.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + } + + /*************************************/ + /**** Fixtures ****/ + /*************************************/ + + // Field order matches a real record: `_t` first, `_asm` immediately before the last + // `_bhomVersion`, which is where the backfill and the capture both put it. + private static string Record(string type, string? assembly, string version = "9.3") + { + string field = assembly is null + ? string.Empty + : $"\"_asm\" : {JsonSerializer.Serialize(assembly)}, "; + + return $"{{ \"_t\" : {JsonSerializer.Serialize(type)}, \"Name\" : \"fixture\", " + + $"{field}\"_bhomVersion\" : \"{version}\" }}"; + } + + private sealed class TempDatasets : IDisposable + { + public string Root { get; } + + public TempDatasets() + { + Root = Path.Combine(Path.GetTempPath(), "versioning-runner-datasets-" + Guid.NewGuid()); + Directory.CreateDirectory(Root); + } + + public TempDatasets WithObjects(string version, params string[] lines) + => WithFile(version, "Objects.json", lines); + + public TempDatasets WithFile(string version, string fileName, params string[] lines) + { + string directory = Path.Combine(Root, version); + Directory.CreateDirectory(directory); + File.WriteAllLines(Path.Combine(directory, fileName), lines); + return this; + } + + public void Dispose() + { + // A locked or already-removed temp tree must not fail the test that produced it; + // the leftover is a few kilobytes under TEMP. Anything else is a real fault and + // is left to propagate. + try { Directory.Delete(Root, recursive: true); } + catch (IOException) { } + catch (UnauthorizedAccessException) { } + } + } + } +} diff --git a/tools/VersioningRunner/src/VersioningRunner/Commands/DatasetProvenance.cs b/tools/VersioningRunner/src/VersioningRunner/Commands/DatasetProvenance.cs new file mode 100644 index 0000000..3cb831d --- /dev/null +++ b/tools/VersioningRunner/src/VersioningRunner/Commands/DatasetProvenance.cs @@ -0,0 +1,179 @@ +using System.Text.Json; + +namespace VersioningRunner.Commands; + +// The dataset root cannot supply a map at all. +// +// Not thrown for an empty map. A root that exists and whose records carry no `_asm` is the +// state before the backfill lands, and it has to read as inert rather than broken. +public sealed class DatasetProvenanceException : Exception +{ + public DatasetProvenanceException(string message) : base(message) { } +} + +// One object record's declaring assembly, keyed by the type name the runner sees on a leaf. +// +// `TypesMapped` counts agreed entries only. `RecordsRead` counts every non-blank line, +// including those carrying no `_asm`, because it is the denominator that lets a run say +// "0 of 40375 records carry the field" rather than reporting an empty map with no context. +public sealed record DeclaringAssemblyMap( + IReadOnlyDictionary ByTypeName, + IReadOnlyDictionary> Disputed, + int VersionsRead, + int RecordsRead, + int TypesMapped, + int LinesUnparseable) +{ + public static DeclaringAssemblyMap Empty { get; } = new( + new Dictionary(StringComparer.Ordinal), + new Dictionary>(StringComparer.Ordinal), + 0, 0, 0, 0); + + // Null when the dataset names no assembly for the type, and null when versions disagree + // about which. A disagreement is not resolved by picking one: the caller falls back to the + // namespace guess, which is at least recorded as a guess. + public string? DeclaringAssemblyFor(string? typeFullName) + => !string.IsNullOrEmpty(typeFullName) && ByTypeName.TryGetValue(typeFullName, out string? assembly) + ? assembly + : null; +} + +// Where a failing object record's declaring assembly comes from. +// +// The record carries it, in the `_asm` field, and ci-versioning stages the dataset to a known +// path on the same machine in the same job: the action asserts that directory exists and +// enumerates its version folders in a step before the runner runs. So this reads the same file +// the versioning test read, rather than depending on the test to restate the field in a message +// the runner then parses back out. +// +// Only Objects.json is read. Methods.json and Adapters.json carry +// `_t: System.Reflection.MethodBase` on every record, so `_t` identifies nothing there, and +// their leaves already reach the runner with a declaring assembly from the Method event. +public static class DatasetProvenance +{ + /*************************************/ + /**** Public Methods ****/ + /*************************************/ + + // Every version directory present, not the newest. + // + // A pull request reads one version, but which one is a property of Versioning_Toolkit's + // source rather than of anything the runner can see, and deriving it would mean regexing a + // version list out of FromJson.cs. Reading the union costs nothing and is what makes the + // answer independent of that. Measured on the two versions carrying the field: 1711 shared + // type names, 0 disagreements. Measured on the 15 saved run artefacts: 203 of 205 + // object-record findings resolve against 9.3 and the other 2 only against 9.2, so a + // newest-only read would have missed both, and both are NoMethodEvent findings with no + // declaring assembly, which is precisely the population this field exists to serve. + public static DeclaringAssemblyMap Build(string? datasetsRoot) + { + if (string.IsNullOrWhiteSpace(datasetsRoot)) + throw new DatasetProvenanceException("No versioning dataset path was supplied."); + + if (!Directory.Exists(datasetsRoot)) + throw new DatasetProvenanceException( + $"Versioning dataset directory not found at {datasetsRoot}. The build of " + + "Verification.sln stages it through Versioning_Test.csproj's PostBuild step, so its " + + "absence means that build did not run or did not stage."); + + List objectFiles = Directory.EnumerateDirectories(datasetsRoot) + .OrderBy(d => d, StringComparer.Ordinal) + .Select(d => Path.Combine(d, "Objects.json")) + .Where(File.Exists) + .ToList(); + + if (objectFiles.Count == 0) + throw new DatasetProvenanceException( + $"No version directory under {datasetsRoot} contains an Objects.json. The directory " + + "exists but holds no object records, so no finding could be attributed by declaring " + + "assembly and every one would fall back to the namespace guess."); + + // Ordinal-sorted so a disputed entry's candidate list, and the artefact it lands in, do + // not depend on directory enumeration order. + var seen = new Dictionary>(StringComparer.Ordinal); + int records = 0; + int unparseable = 0; + + foreach (string file in objectFiles) + { + foreach (string line in File.ReadLines(file)) + { + if (string.IsNullOrWhiteSpace(line)) + continue; + + records++; + + string? type; + string? assembly; + try + { + using JsonDocument document = JsonDocument.Parse(line); + if (document.RootElement.ValueKind != JsonValueKind.Object) + { + unparseable++; + continue; + } + + type = StringProperty(document.RootElement, "_t"); + assembly = StringProperty(document.RootElement, "_asm"); + } + catch (JsonException) + { + // Counted and reported by the caller rather than thrown. A record the + // dataset cannot express is the dataset's problem, and the versioning test + // reading the same line produces its own finding for it; losing the whole + // map over one line would convert that into a fleet-wide fallback. + unparseable++; + continue; + } + + if (type is null || assembly is null) + continue; + + if (!seen.TryGetValue(type, out SortedSet? assemblies)) + seen[type] = assemblies = new SortedSet(StringComparer.Ordinal); + + assemblies.Add(assembly); + } + } + + var agreed = new Dictionary(StringComparer.Ordinal); + var disputed = new Dictionary>(StringComparer.Ordinal); + + foreach ((string type, SortedSet assemblies) in seen) + { + if (IsOneAssemblyFamily(assemblies)) + agreed[type] = assemblies.First(); + else + disputed[type] = assemblies.ToArray(); + } + + return new DeclaringAssemblyMap(agreed, disputed, objectFiles.Count, records, agreed.Count, unparseable); + } + + /*************************************/ + /**** Private Methods ****/ + /*************************************/ + + // Two versions naming Revit_Tagging_oM_2022 and Revit_Tagging_oM_2023 are not disagreeing. + // They are the same repository under two build configurations, attribution strips the year + // anyway, and treating the pair as disputed would drop the finding to the namespace guess + // for no gain. That is the failure "compare year-insensitively" exists to prevent, applied + // one layer earlier. Different families are a real disagreement and stay disputed. + private static bool IsOneAssemblyFamily(SortedSet assemblies) + => assemblies.Count == 1 + || assemblies.Select(RunCommand.StripConfigSuffix).Distinct(StringComparer.Ordinal).Count() == 1; + + // A field present but not a string, or present and blank, reads the same as absent. Both + // leave the caller on its existing path rather than mapping the type to nothing. + private static string? StringProperty(JsonElement record, string name) + { + if (!record.TryGetProperty(name, out JsonElement value) || value.ValueKind != JsonValueKind.String) + return null; + + string? text = value.GetString(); + return string.IsNullOrWhiteSpace(text) ? null : text; + } + + /*************************************/ +} From 953087a61147536977f779d9cb036df4e3997e79 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Tue, 29 Sep 2026 02:49:02 +0100 Subject: [PATCH 3/7] chore(ci-versioning): drop an empty file committed by accident --- tools/VersioningRunner/bhom-subject-list.txt | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 tools/VersioningRunner/bhom-subject-list.txt diff --git a/tools/VersioningRunner/bhom-subject-list.txt b/tools/VersioningRunner/bhom-subject-list.txt deleted file mode 100644 index e69de29..0000000 From d3b4dfb3a73ee3a200e487c1d390bbce622ccc7b Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Tue, 29 Sep 2026 03:04:26 +0100 Subject: [PATCH 4/7] feat(ci-versioning): resolve object records against the staged dataset --- .github/actions/ci-versioning/action.yml | 15 ++ .../ObjectRecordProvenanceTests.cs | 206 +++++++++++------- .../VersioningRunner/Commands/RunCommand.cs | 181 ++++++++++----- .../Models/VersioningResult.cs | 7 +- .../src/VersioningRunner/Program.cs | 15 +- 5 files changed, 280 insertions(+), 144 deletions(-) diff --git a/.github/actions/ci-versioning/action.yml b/.github/actions/ci-versioning/action.yml index 83ddfab..843a46a 100644 --- a/.github/actions/ci-versioning/action.yml +++ b/.github/actions/ci-versioning/action.yml @@ -594,7 +594,11 @@ runs: dotnet restore "${{ steps.ver_sln.outputs.path }}" dotnet build "${{ steps.ver_sln.outputs.path }}" --no-restore -c ${{ steps.ver_config.outputs.config }} --nologo + # The path is emitted rather than repeated. The runner reads the same tree to resolve an + # object record's declaring assembly, and two literals of one path in one file is how they + # drift apart. - name: Validate versioning datasets + id: datasets if: steps.changed.outputs.count != '0' shell: pwsh run: | @@ -609,6 +613,7 @@ runs: exit 1 } Write-Host "::notice title=Versioning::Found versioning datasets for: $($versions.Name -join ', ')" + "root=$path" | Out-File $env:GITHUB_OUTPUT -Append # Precondition, not a policy choice. Attribution narrows to the namespaces this # repository's own assemblies declare; with no assemblies there is nothing to narrow to, @@ -746,8 +751,14 @@ runs: # line, so no annotation is created. Measured on a sandbox validation run: # Classification reported real/SignatureResolved=2 while zero finding # annotations were produced, against two on a comparable production run. + # --datasets is the tree the step above validated. An object record carries its + # declaring assembly in the record's own `_asm` field, and this is the only place the + # runner can read it: the TestResult tree the test returns does not carry it. + # Attribution falls back to the namespace prefix for any record the dataset does not + # answer for, so this is inert until the backfill lands. & "${{ steps.runner.outputs.runner_exe }}" ` --assemblies 'C:\ProgramData\BHoM\Assemblies' ` + --datasets '${{ steps.datasets.outputs.root }}' ` --subject-assembly-list 'subject-assemblies.txt' ` --configuration 'Release' ` $(if ('${{ steps.vercond.outputs.file }}') { '--version-conditional', '${{ steps.vercond.outputs.file }}' }) ` @@ -883,6 +894,10 @@ runs: $md += "| Dataset versions | $(if ($coverage.DatasetVersions -eq 0) { 'all staged' } else { 'previous only' }) |" $md += "| Assemblies loaded | $($coverage.LoadedAssemblies) |" $md += "| FromJsonDatasets entry points invoked | $($coverage.VerifyEntryPoints) |" + # Zero means every object record was attributed by namespace prefix, which is the + # state until the dataset backfill lands. Printed either way, for the same reason the + # two rows above it are. + $md += "| Dataset types carrying a declaring assembly | $($coverage.TypesWithDeclaringAssembly) |" } $md += "| Build configuration | ``$(if ($configuration) { $configuration } else { 'not recorded' })`` |" $md += "" diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs index d963270..9ea7102 100644 --- a/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs @@ -9,14 +9,28 @@ namespace VersioningRunner.Tests // Object records carry a type name and nothing else, so attribution has always guessed from // the namespace. The dataset's `_asm` field closes that, and these pin the runner half. // - // Every test here drives a synthetic TestResult, because the runner has no other view of the - // dataset and Versioning_Toolkit does not emit the event yet. That is the limit of this - // coverage: it proves the runner handles the format it defines, not that anything produces it. + // Where the map comes from is DatasetProvenanceTests' subject. These drive it in as a value, + // because what they pin is how a leaf consumes it: the join key, the Method event's + // precedence over it, and what a disagreement does. The join key is the leaf description, + // which for an object record is the `_t` value verbatim. + // + // The limit of this coverage: it proves the runner reads a map correctly, not that a live + // FromJsonDatasets tree produces leaves that hit one. Only a real run shows that. public class ObjectRecordProvenanceTests { - // The contract Versioning_Toolkit's FromJson.cs is bound to emit. - private static string ObjectEvent(string type, string assembly) => - $"Object {type} declared in \"{assembly}\""; + // What the staged Objects.json resolved, as the collector receives it. + private static DeclaringAssemblyMap Map(params (string Type, string Assembly)[] entries) + => new( + entries.ToDictionary(e => e.Type, e => e.Assembly, StringComparer.Ordinal), + new Dictionary>(StringComparer.Ordinal), + VersionsRead: 1, RecordsRead: entries.Length, TypesMapped: entries.Length, LinesUnparseable: 0); + + // A type two dataset versions named different assembly families for. + private static DeclaringAssemblyMap Disputed(string type, params string[] assemblies) + => new( + new Dictionary(StringComparer.Ordinal), + new Dictionary>(StringComparer.Ordinal) { [type] = assemblies }, + VersionsRead: 2, RecordsRead: assemblies.Length, TypesMapped: 0, LinesUnparseable: 0); // A closed generic. Its name contains commas, which is why the quoted part holds the // assembly alone: a comma-delimited form silently lost all 23 of these in the 9.2 @@ -63,25 +77,32 @@ private static ClosureContext ClosureForSubject(params string[] subjectFileNames // Reading the field // ------------------------------------------------------------------ + // The join. The leaf's description is the record's `_t` value, so the map is keyed on + // exactly what arrives here. Measured on the 15 saved run artefacts: 205 of 205 + // object-record findings match a dataset record on this key, with no fuzzy matching. [Fact] - public void ObjectEvent_YieldsTheDeclaringAssemblyAndType() + public void TheLeafDescriptionIsTheKeyIntoTheMap() { - var (type, assembly) = RunCommand.ParseObjectEventAssembly( - ObjectEvent("BH.oM.Acoustic.Panel", "Acoustic_oM")); + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Map(("BH.oM.Acoustic.Panel", "Acoustic_oM"))); - Assert.Equal("BH.oM.Acoustic.Panel", type); - Assert.Equal("Acoustic_oM", assembly); + Assert.Equal("Acoustic_oM", Assert.Single(diagnostics).DeclaringAssembly); } + // A map that answers for other types but not this one leaves the leaf where it was. [Fact] - public void DeclaringAssemblyFromAnObjectEvent_ReachesTheDiagnostic() + public void ATypeTheDatasetDoesNotNameIsNotGivenAnAssembly() { var diagnostics = new List(); RunCommand.ExtractFilteredResult( - Tree("BH.oM.Acoustic.Panel", ObjectEvent("BH.oM.Acoustic.Panel", "Acoustic_oM")), - (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics); + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Map(("BH.oM.Structure.Elements.Panel", "Structure_oM"))); - Assert.Equal("Acoustic_oM", Assert.Single(diagnostics).DeclaringAssembly); + Assert.Null(Assert.Single(diagnostics).DeclaringAssembly); } // The whole point of the field: this record used to be attributable only by prefix. @@ -113,8 +134,10 @@ public void ObjectRecordNamingAnotherRepositorysAssembly_IsNotAttributed() // Inertness. These are the tests that say this PR does nothing on its own. // ------------------------------------------------------------------ + // No map supplied at all, which is what every caller that does not pass one gets and + // what the runner does without --datasets. Identical to the behaviour before this change. [Fact] - public void NoObjectEvent_LeavesTheDeclaringAssemblyNull() + public void NoMap_LeavesTheDeclaringAssemblyNull() { var diagnostics = new List(); RunCommand.ExtractFilteredResult( @@ -124,19 +147,31 @@ public void NoObjectEvent_LeavesTheDeclaringAssemblyNull() Assert.Null(Assert.Single(diagnostics).DeclaringAssembly); } - // A record cannot be both, and the method path must not change. If both events are - // present the Method event still wins, because it is read first and the object read is - // only consulted when it yielded nothing. + // The dataset read before the backfill lands: records present, none carrying the field. + [Fact] + public void AnEmptyMap_LeavesTheDeclaringAssemblyNull() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: DeclaringAssemblyMap.Empty); + + Assert.Null(Assert.Single(diagnostics).DeclaringAssembly); + } + + // A record cannot be both, and the method path must not change. If the map happens to + // answer for a method leaf's description, the Method event still wins, because it is + // read first and the dataset is only consulted when it yielded nothing. [Fact] - public void MethodEventStillWins_WhenBothArePresent() + public void MethodEventStillWins_WhenTheMapAlsoAnswers() { var diagnostics = new List(); RunCommand.ExtractFilteredResult( - Tree("BH.Revit.Engine.MechanicalPlumbing.Compute. }", - MethodEvent, - ObjectEvent("BH.oM.Acoustic.Panel", "Acoustic_oM")), + Tree("BH.Revit.Engine.MechanicalPlumbing.Compute. }", MethodEvent), (_, _) => (true, AttributionBasis.NotRecorded), null, - (_, _, _) => (null, ClassificationPath.DeclaringTypeNotLoaded, Array.Empty()), diagnostics); + (_, _, _) => (null, ClassificationPath.DeclaringTypeNotLoaded, Array.Empty()), diagnostics, + provenance: Map(("BH.Revit.Engine.MechanicalPlumbing.Compute. }", "Acoustic_oM"))); Assert.Equal("Revit_MechanicalPlumbing_Engine_2022", Assert.Single(diagnostics).DeclaringAssembly); } @@ -155,8 +190,9 @@ public void WholeClosure_IgnoresTheDeclaringAssemblyForAttribution() var withAsm = new List(); RunCommand.ExtractFilteredResult( - Tree("BH.oM.Acoustic.Panel", ObjectEvent("BH.oM.Acoustic.Panel", "Somebody_Elses_oM")), - wholeClosure, null, null, withAsm); + Tree("BH.oM.Acoustic.Panel"), + wholeClosure, null, null, withAsm, + provenance: Map(("BH.oM.Acoustic.Panel", "Somebody_Elses_oM"))); Assert.Equal(AttributionBasis.NotRecorded, Assert.Single(withAsm).AttributedBy); Assert.True(withAsm[0].CountedAsReal); @@ -230,9 +266,10 @@ public void MoreThanOneAssemblyDeclaringTheType_IsRecordedAsCandidates() { var diagnostics = new List(); RunCommand.ExtractFilteredResult( - Tree("BH.oM.Acoustic.Panel", ObjectEvent("BH.oM.Acoustic.Panel", "Revit_ModelQA_oM_2022")), + Tree("BH.oM.Acoustic.Panel"), (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, - probeTypeCandidates: _ => ["Revit_ModelQA_oM_2022", "Revit_ModelQA_oM_2023"]); + probeTypeCandidates: _ => ["Revit_ModelQA_oM_2022", "Revit_ModelQA_oM_2023"], + provenance: Map(("BH.oM.Acoustic.Panel", "Revit_ModelQA_oM_2022"))); var only = Assert.Single(diagnostics); Assert.NotNull(only.DeclaringTypeCandidates); @@ -245,80 +282,96 @@ public void ASingleDeclaringAssembly_LeavesCandidatesNull() { var diagnostics = new List(); RunCommand.ExtractFilteredResult( - Tree("BH.oM.Acoustic.Panel", ObjectEvent("BH.oM.Acoustic.Panel", "Acoustic_oM")), + Tree("BH.oM.Acoustic.Panel"), (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, - probeTypeCandidates: _ => ["Acoustic_oM"]); + probeTypeCandidates: _ => ["Acoustic_oM"], + provenance: Map(("BH.oM.Acoustic.Panel", "Acoustic_oM"))); Assert.Null(Assert.Single(diagnostics).DeclaringTypeCandidates); } - // Without the object event there is no type to scan, so the probe is never called and - // the pre-existing behaviour stands. This is the other half of inertness. + // Nothing resolved, so there is no type the dataset vouched for and scanning the closure + // would answer a question nobody asked. This is the other half of inertness. [Fact] - public void NoObjectEvent_DoesNotScanForCandidates() + public void NoMapEntry_DoesNotScanForCandidates() { bool called = false; var diagnostics = new List(); RunCommand.ExtractFilteredResult( Tree("BH.oM.Acoustic.Panel", "Failed to convert the string into a type: BH.oM.Acoustic.Panel"), (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, - probeTypeCandidates: _ => { called = true; return []; }); + probeTypeCandidates: _ => { called = true; return []; }, + provenance: DeclaringAssemblyMap.Empty); - Assert.False(called, "a record with no object event has no type to scan and must not be probed"); + Assert.False(called, "a record the dataset does not answer for must not be probed"); Assert.Null(Assert.Single(diagnostics).DeclaringTypeCandidates); } + // Versions disagreeing is not resolved by picking one. The finding stays unattributed + // and carries what the dataset claimed, so the ambiguity is counted rather than + // normalised away. Measured 0 across the 1711 type names the two backfilled versions + // share, so this path is expected to stay empty and is pinned so it stays honest if not. [Fact] - public void ProbeTypeCandidates_ReturnsEveryAssemblyDeclaringTheType() + public void ADisputedTypeIsLeftUnattributedAndCarriesTheClaims() { - var loaded = new List { typeof(RunCommand).Assembly, typeof(object).Assembly }; + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Disputed("BH.oM.Structure.Elements.Panel", "StructuralEngineering_oM", "Structure_oM")); - Assert.Equal(["VersioningRunner"], - RunCommand.ProbeTypeCandidates(loaded, typeof(RunCommand).FullName!)); - Assert.Empty(RunCommand.ProbeTypeCandidates(loaded, "No.Such.Type")); + var only = Assert.Single(diagnostics); + Assert.Null(only.DeclaringAssembly); + Assert.Equal(["StructuralEngineering_oM", "Structure_oM"], only.DeclaringTypeCandidates!); } - // ------------------------------------------------------------------ - // Malformed input fails to parse rather than capturing something wrong - // ------------------------------------------------------------------ - - [Theory] - [InlineData("Object BH.oM.Acoustic.Panel declared in \"Acoustic_oM")] - [InlineData("Object declared in \"Acoustic_oM\"")] - [InlineData("BH.oM.Acoustic.Panel declared in \"Acoustic_oM\"")] - [InlineData("Object BH.oM.Acoustic.Panel declared in \"\"")] - [InlineData("")] - public void AMalformedObjectEventYieldsNothing(string message) + // The dataset's disagreement is the answer, not the closure's. Probing would replace a + // statement about which assembly declared the type with a statement about which + // assemblies could have, and they are different questions. + [Fact] + public void ADisputedTypeDoesNotFallBackToTheClosureScan() { - var (type, assembly) = RunCommand.ParseObjectEventAssembly(message); + bool called = false; + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + probeTypeCandidates: _ => { called = true; return ["Something_Else_oM"]; }, + provenance: Disputed("BH.oM.Structure.Elements.Panel", "StructuralEngineering_oM", "Structure_oM")); - Assert.Null(type); - Assert.Null(assembly); + Assert.False(called); + Assert.Equal(["StructuralEngineering_oM", "Structure_oM"], Assert.Single(diagnostics).DeclaringTypeCandidates!); } - // ------------------------------------------------------------------ - // Closed generics. The class the first version of this contract silently lost. - // ------------------------------------------------------------------ - [Fact] - public void AClosedGenericTypeNameParses() + public void ProbeTypeCandidates_ReturnsEveryAssemblyDeclaringTheType() { - var (type, assembly) = RunCommand.ParseObjectEventAssembly( - ObjectEvent(ClosedGeneric, "StructuralEngineering_oM")); + var loaded = new List { typeof(RunCommand).Assembly, typeof(object).Assembly }; - Assert.Equal(ClosedGeneric, type); - Assert.Equal("StructuralEngineering_oM", assembly); + Assert.Equal(["VersioningRunner"], + RunCommand.ProbeTypeCandidates(loaded, typeof(RunCommand).FullName!)); + Assert.Empty(RunCommand.ProbeTypeCandidates(loaded, "No.Such.Type")); } - // The commas inside the type argument list are the whole reason the format changed. + // ------------------------------------------------------------------ + // Closed generics. The class the earlier message-based route silently lost. + // ------------------------------------------------------------------ + + // 23 of the 9.2 dataset's 1,713 records are closed generics, whose names carry commas + // and brackets. Reaching them through a formatted message lost all 23 to the delimiter, + // with no diagnostic, and one of them was attributed to the wrong repository as a + // result. A dictionary key has no delimiter to lose them to, and the description arrives + // as the record wrote it. DatasetProvenanceTests pins the same property at map level. [Fact] - public void TheTypeArgumentListDoesNotTruncateTheAssembly() + public void AClosedGenericIsAWholeKey() { - var (_, assembly) = RunCommand.ParseObjectEventAssembly( - ObjectEvent(ClosedGeneric, "StructuralEngineering_oM")); + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree(ClosedGeneric), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Map((ClosedGeneric, "StructuralEngineering_oM"))); - Assert.DoesNotContain(",", assembly); - Assert.DoesNotContain("Version=", assembly!); + Assert.Equal("StructuralEngineering_oM", Assert.Single(diagnostics).DeclaringAssembly); } // The case that mattered: the type's namespace is the subject's, so the namespace guess @@ -335,22 +388,11 @@ public void AClosedGenericDeclaredElsewhereIsNotAttributedToTheSubject() Assert.False(withField.Attributable); Assert.Equal(AttributionBasis.DeclaringAssembly, withField.Basis); - // What the fallback does when the field cannot be read, which is what the earlier - // comma-delimited format produced for this record. + // What the fallback does when the dataset does not answer for the record, which is + // every object record until the backfill lands. var withoutField = RunCommand.AttributeToSubject(ClosedGeneric, null, subjectNs, closure); Assert.True(withoutField.Attributable); Assert.Equal(AttributionBasis.NamespaceFallback, withoutField.Basis); } - - [Fact] - public void AClosedGenericReachesTheDiagnosticEndToEnd() - { - var diagnostics = new List(); - RunCommand.ExtractFilteredResult( - Tree(ClosedGeneric, ObjectEvent(ClosedGeneric, "StructuralEngineering_oM")), - (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics); - - Assert.Equal("StructuralEngineering_oM", Assert.Single(diagnostics).DeclaringAssembly); - } } } diff --git a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs index 8367896..95b2517 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs @@ -16,7 +16,8 @@ public static int Execute( bool testAll, string? subjectAssemblyList = null, string? configuration = null, - IReadOnlyCollection? versionConditionalMethods = null) + IReadOnlyCollection? versionConditionalMethods = null, + string? datasetsPath = null) { s_configuration = configuration; s_versionConditional = versionConditionalMethods is null @@ -30,6 +31,26 @@ public static int Execute( else Console.WriteLine($"Version-conditional method list: scan performed, {s_versionConditional.Count} method(s) found."); + // Built before the assemblies load, so a broken precondition costs nothing. The two + // states it throws on are a missing dataset root and a root holding no Objects.json, + // and both mean no object record could be attributed by declaring assembly. Neither is + // reachable through ci-versioning, whose "Validate versioning datasets" step already + // exits 1 on the same conditions before this runs. + DeclaringAssemblyMap provenance; + try + { + provenance = datasetsPath is null + ? DeclaringAssemblyMap.Empty + : DatasetProvenance.Build(datasetsPath); + } + catch (DatasetProvenanceException ex) + { + Console.Error.WriteLine($"::error title=Versioning::{ex.Message}"); + return 1; + } + + ReportProvenance(provenance, datasetsPath); + var loaded = LoadAssemblies(assembliesPath); var methods = FindAllFromJsonDatasetsMethods(loaded); @@ -161,7 +182,7 @@ public static int Execute( return 1; } - var partial = ExtractFilteredResult(rawResult, isAttributable, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates); + var partial = ExtractFilteredResult(rawResult, isAttributable, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates, provenance); allFailures.AddRange(partial.Failures); } @@ -280,7 +301,8 @@ public static int Execute( SubjectAssemblies: s_subjectAssemblyCount, SubjectTypes: s_subjectTypeCount, DatasetVersions: testAll ? 0 : 1, - RecordsUnverified: unresolvableSkips.Count); + RecordsUnverified: unresolvableSkips.Count, + TypesWithDeclaringAssembly: provenance.TypesMapped); var result = new VersioningResult { @@ -507,7 +529,10 @@ public static VersioningResult ExtractFilteredResult( LoadedTypeIndex? typeIndex = null, // Null means object-record ambiguity is not scanned, which is the pre-existing // behaviour and what every caller that does not supply a closure should get. - Func>? probeTypeCandidates = null) + Func>? probeTypeCandidates = null, + // The dataset's answer for an object record's declaring assembly. Empty by default, so + // a caller that supplies nothing keeps the namespace fallback it had before. + DeclaringAssemblyMap? provenance = null) { if (rawResult is null) return new VersioningResult @@ -518,7 +543,7 @@ public static VersioningResult ExtractFilteredResult( }; var failures = new List(); - CollectLeafFailures(rawResult, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex ?? LoadedTypeIndex.Empty, probeTypeCandidates, depth: 0); + CollectLeafFailures(rawResult, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex ?? LoadedTypeIndex.Empty, probeTypeCandidates, provenance ?? DeclaringAssemblyMap.Empty, depth: 0); var status = failures.Count > 0 ? VersioningStatus.Error : VersioningStatus.Pass; return new VersioningResult @@ -536,7 +561,8 @@ private static void CollectLeafFailures( List? unresolvableSkips, Func Candidates)>? probeSignature, List? diagnostics, ClosureContext? closure, - LoadedTypeIndex typeIndex, Func>? probeTypeCandidates, int depth) + LoadedTypeIndex typeIndex, Func>? probeTypeCandidates, + DeclaringAssemblyMap provenance, int depth) { // BHoM's TestResult tree has at most 3 levels under the root (outer → per-version // summary → individual type result). Depth 5 gives headroom for unexpected nesting @@ -589,18 +615,35 @@ private static void CollectLeafFailures( .FirstOrDefault(a => a is not null); // Object records carry no Method event, so the assembly comes from the dataset's - // `_asm` field instead, surfaced as its own event. Consulted only when the Method - // event yielded nothing, so the method path is untouched: a record cannot be both. - // Null throughout until Versioning_Toolkit emits the event, which is what makes - // this change inert on its own. - string? objectTypeName = null; + // `_asm` field instead, read from the staged Objects.json rather than from anything + // in this tree. Consulted only when the Method event yielded nothing, so the method + // path is untouched: a record cannot be both. + // + // The description is the join key because for an object record it is the `_t` value + // verbatim. That is structural rather than lucky: FromJsonItem reaches IToText only + // when the result is non-null and not a CustomObject, and every path with that + // combination returns a PassResult, so an Error leaf always carries + // DescriptionFromJson's output, which is the record's first quoted field. + // + // Null throughout until the dataset carries `_asm`, which is what makes this change + // inert on its own. + string? provenanceType = null; + IReadOnlyList? disputedAssemblies = null; if (declaringAssembly is null) { - var objectEvent = eventMessages - .Select(ParseObjectEventAssembly) - .FirstOrDefault(p => p.Assembly is not null); - objectTypeName = objectEvent.TypeName; - declaringAssembly = objectEvent.Assembly; + string? fromDataset = provenance.DeclaringAssemblyFor(desc); + if (fromDataset is not null) + { + declaringAssembly = fromDataset; + provenanceType = desc; + } + else if (provenance.Disputed.TryGetValue(desc, out IReadOnlyList? claims)) + { + // Versions named different assembly families for this type. Left unattributed + // on purpose: picking one would be a guess dressed as a fact, and the + // namespace fallback at least records itself as a guess. + disputedAssemblies = claims; + } } var (attributable, attributedBy) = isAttributable(desc, declaringAssembly); @@ -639,8 +682,19 @@ private static void CollectLeafFailures( // than normalise it away silently. Without this the metric below can never // count an object record, because candidates only ever came from the // method probe. - if (probeTypeCandidates is not null && objectTypeName is not null) - candidates = probeTypeCandidates(objectTypeName); + // + // The field carries two kinds of ambiguity and they are not the same + // question. Dataset versions disagreeing about which assembly declared the + // type takes precedence, because in that case nothing resolved and scanning + // the closure would answer a question nobody asked. Otherwise it is the + // original one: how many loaded assemblies could have declared it. The + // dataset case is reported separately at run level, so the two remain + // tellable apart; measured 0 across the 1711 type names the two backfilled + // versions share. + if (disputedAssemblies is not null) + candidates = disputedAssemblies; + else if (probeTypeCandidates is not null && provenanceType is not null) + candidates = probeTypeCandidates(provenanceType); } else if (probeSignature is not null) (cause, path, candidates) = probeSignature(eventType, eventMethod, declaringAssembly); @@ -729,7 +783,7 @@ private static void CollectLeafFailures( { string childStatus = child.GetType().GetProperty("Status")?.GetValue(child)?.ToString() ?? "Pass"; if (childStatus is "Error" or "Warning") - CollectLeafFailures(child, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates, depth + 1); + CollectLeafFailures(child, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates, provenance, depth + 1); } } } @@ -1021,50 +1075,59 @@ public static (string? DeclaringType, string? MethodName) ParseMethodEvent(strin return assembly.Length > 0 ? assembly : null; } - // Object records have no Method event, so until now they had no declaring assembly at all - // and always fell to the namespace guess. The dataset's `_asm` field closes that, and this - // is the wire format it arrives in: - // - // Object declared in "" - // - // THIS IS A CONTRACT. Versioning_Toolkit's FromJson.cs must emit exactly this shape for the - // field to be read; nothing else in the runner can see the dataset. - // - // The quoted part holds the assembly ALONE, and that is a correction rather than a - // preference. This first mirrored the Method event's assembly-qualified "Name", - // `", "`, and that shape cannot represent the data: a closed generic - // type name contains commas, e.g. + // What the dataset read produced, on stdout beside Attribution and Classification because + // it is coverage evidence rather than an annotation. // - // BH.oM.Structure.Results.ResultEnvelope`1[[BH.oM.Structure.Results.ConnectionForce, - // StructuralEngineering_oM, Version=...]] - // - // so a comma-delimited group stopped at the first argument and the match failed. Measured - // over the 9.2 dataset, the earlier shape parsed 1,690 of 1,713 records and **silently lost - // 23**, each falling back to the namespace guess with no diagnostic. One of the 23 is - // attributed to the wrong repository by that fallback, which is the exact defect this field - // exists to remove. An assembly simple name can never contain a quote, so the assembly-only - // form is unambiguous and parses 1,713 of 1,713. - // - // The type is still carried, in the leading position, where it needs no delimiter. - private static readonly Regex _objectEventAssemblyPattern = new( - @"^Object\s+(?.+?)\s+declared\s+in\s+""(?[^""]+)""$", - RegexOptions.Compiled); - - // The declaring type is returned alongside the assembly because the ambiguity scan needs a - // type name and the leaf's Description is not reliably one: DescriptionFromJson mangles - // some entries. The event states it directly. - public static (string? TypeName, string? Assembly) ParseObjectEventAssembly(string message) + // Printed in every state, including the two that report nothing. An empty map reads + // identically whether the dataset carries no `_asm` yet, or the path was never supplied, or + // the runner read a tree with nothing in it, and those need different action. A number that + // appears only when it is non-zero cannot be told from one nobody measured. + private static void ReportProvenance(DeclaringAssemblyMap provenance, string? datasetsPath) { - if (string.IsNullOrEmpty(message)) - return (null, null); + if (datasetsPath is null) + { + Console.WriteLine( + "Provenance: no dataset path supplied, so no object record can be attributed by " + + "declaring assembly and every one falls back to the namespace guess."); + return; + } - var match = _objectEventAssemblyPattern.Match(message); - if (!match.Success) - return (null, null); + Console.WriteLine( + $"Provenance: {provenance.VersionsRead} dataset version(s), {provenance.RecordsRead} record(s), " + + $"{provenance.TypesMapped} type(s) mapped to a declaring assembly, " + + $"{provenance.Disputed.Count} disputed."); - string type = match.Groups["type"].Value.Trim(); - string assembly = match.Groups["assembly"].Value.Trim(); - return (type.Length > 0 ? type : null, assembly.Length > 0 ? assembly : null); + if (provenance.RecordsRead > 0 && provenance.TypesMapped == 0 && provenance.Disputed.Count == 0) + { + Console.WriteLine( + $"Provenance: 0 of {provenance.RecordsRead} record(s) carry the declaring-assembly field, " + + "so object records are attributed by namespace as before. Expected until the dataset " + + "backfill lands."); + } + + if (provenance.LinesUnparseable > 0) + { + Console.Error.WriteLine( + $"::warning title=Versioning::{provenance.LinesUnparseable} dataset record(s) could not be " + + "parsed and were skipped, so any finding on one of them falls back to the namespace guess."); + } + + // Measured 0 across the 1711 type names the two backfilled versions share, so this is + // expected to stay silent. It is a warning rather than a counter because the first time + // it is not silent is the first time a type name stops identifying one repository, and + // that is worth someone reading rather than a number in an artefact. + if (provenance.Disputed.Count > 0) + { + var named = provenance.Disputed + .OrderBy(d => d.Key, StringComparer.Ordinal) + .Take(10) + .Select(d => $"{d.Key} ({string.Join(", ", d.Value)})"); + + Console.Error.WriteLine( + $"::warning title=Versioning::{provenance.Disputed.Count} type(s) are named with different " + + "declaring assemblies by different dataset versions, so they are left to the namespace " + + $"guess rather than resolved to one: {string.Join("; ", named)}"); + } } // Every loaded assembly that yields the named type. The type-only half of diff --git a/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs b/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs index 27e1e94..6220b53 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs @@ -165,7 +165,12 @@ public record CoverageCounts( // Present so a green cannot be read as a clean sweep of SubjectTypes: a run reporting // 1191 types checked and 145 records unverified has not examined what the first number // implies. Counts records, not types, because that is the unit the dataset iterates. - int RecordsUnverified = 0); + int RecordsUnverified = 0, + // Type names the staged dataset resolved to a declaring assembly. Zero means every object + // record was attributed by namespace prefix, which is the pre-backfill state and reads + // identically in the artefact to a run that never found the dataset at all. The log says + // which, for 90 days; this is the part that outlives it. + int TypesWithDeclaringAssembly = 0); public class VersioningResult { diff --git a/tools/VersioningRunner/src/VersioningRunner/Program.cs b/tools/VersioningRunner/src/VersioningRunner/Program.cs index 244f28f..7270632 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Program.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Program.cs @@ -29,10 +29,14 @@ if (args.Length == 0 || args.Contains("--help") || args.Contains("-h")) { - Console.WriteLine("Usage: VersioningRunner [--assemblies ] [--output ] [--test-all] [--subject-assembly-list ] [--configuration ] [--version-conditional ]"); + Console.WriteLine("Usage: VersioningRunner [--assemblies ] [--datasets ] [--output ] [--test-all] [--subject-assembly-list ] [--configuration ] [--version-conditional ]"); Console.WriteLine(); Console.WriteLine("Options:"); Console.WriteLine(" --assemblies BHoM assemblies folder (default: C:\\ProgramData\\BHoM\\Assemblies)"); + Console.WriteLine(" --datasets Versioning dataset root, holding one directory per version."); + Console.WriteLine(" Every Objects.json under it is read for the `_asm` field, which"); + Console.WriteLine(" gives an object record its declaring assembly. Omit the flag to"); + Console.WriteLine(" attribute object records by namespace prefix, as before."); Console.WriteLine(" --output Write JSON results to this file"); Console.WriteLine(" --test-all Test all historical dataset versions (default: previous version only)"); Console.WriteLine(" --configuration Build configuration this run compiled, recorded per finding."); @@ -47,6 +51,13 @@ } string assemblies = GetArg(args, "--assemblies") ?? @"C:\ProgramData\BHoM\Assemblies"; + +// No default, unlike --assemblies. A default would make the runner fail hard wherever that +// tree is absent, and versioning-full-history.yml drives this exe without the dataset guard +// ci-versioning runs first. Absent means "nobody supplied a dataset", which is the +// pre-existing behaviour, and the run says so on stdout rather than degrading quietly. +string? datasets = GetArg(args, "--datasets"); + string? output = GetArg(args, "--output"); bool testAll = args.Contains("--test-all"); string? subject = GetArg(args, "--subject-assembly-list"); @@ -70,7 +81,7 @@ .ToArray(); } -return RunCommand.Execute(assemblies, output, testAll, subject, configuration, versionConditional); +return RunCommand.Execute(assemblies, output, testAll, subject, configuration, versionConditional, datasets); static string? GetArg(string[] args, string flag) { From b6ba829c12cced5f82dc0fa48432eff4ee8514d8 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Tue, 29 Sep 2026 09:37:53 +0100 Subject: [PATCH 5/7] fix(ci-versioning): do not reclassify an object record by its recorded Revit year --- .../ObjectRecordProvenanceTests.cs | 93 +++++++++++++++++++ .../VersioningRunner/Commands/RunCommand.cs | 17 ++++ 2 files changed, 110 insertions(+) diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs index 9ea7102..657969c 100644 --- a/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs @@ -353,6 +353,99 @@ public void ProbeTypeCandidates_ReturnsEveryAssemblyDeclaringTheType() Assert.Empty(RunCommand.ProbeTypeCandidates(loaded, "No.Such.Type")); } + // ------------------------------------------------------------------ + // Attribution and reclassification driven together, through the real + // AttributeToSubject rather than a stub. + // + // No test supplied both dataset provenance and a closure until this point, which is + // why the case below went unnoticed: with a stubbed isAttributable the coupling + // between the two is never exercised. + // ------------------------------------------------------------------ + + // A closure as a real run has one: the subject built one Revit year, and that year is + // loaded. ClosureForSubject leaves both loaded sets empty, which cannot reach the + // reclassification block at all. + private static ClosureContext ClosureBuilding(string loadedAssembly) + { + var loaded = new HashSet([loadedAssembly], StringComparer.Ordinal); + var bases = new HashSet(loaded.Select(RunCommand.StripConfigSuffix), StringComparer.Ordinal); + return new ClosureContext(loaded, bases, bases); + } + + private static Func RealAttribution( + ClosureContext closure, params string[] subjectNamespaces) + { + var ns = new HashSet(subjectNamespaces, StringComparer.Ordinal); + return (d, asm) => RunCommand.AttributeToSubject(d, asm, ns, closure); + } + + // The dataset records the lowest Revit year present at capture, which is 2022 on every + // one of the 162 year-suffixed records in 9.3. A repository that has moved on builds a + // later year. That is a difference in how the field was written, not a statement that a + // configuration was skipped, and the finding is real: the record was deserialised + // against the assemblies that are loaded, and it failed against them. There is no + // per-assembly probe here to have been unable to run. + [Fact] + public void AnObjectRecordNamingAnUnbuiltYear_IsStillARealFailure() + { + var closure = ClosureBuilding("Revit_Tagging_oM_2024"); + var diagnostics = new List(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Tagging.Settings.TagSettings"), + RealAttribution(closure, "BH.oM.Tagging.Settings"), null, null, diagnostics, + closure: closure, + probeTypeCandidates: _ => ["Revit_Tagging_oM_2024"], + provenance: Map(("BH.oM.Tagging.Settings.TagSettings", "Revit_Tagging_oM_2022"))); + + var only = Assert.Single(diagnostics); + Assert.Equal("Revit_Tagging_oM_2022", only.DeclaringAssembly); + Assert.Equal(AttributionBasis.DeclaringAssembly, only.AttributedBy); + Assert.Null(only.Cause); + Assert.Equal(ClassificationPath.NoMethodEvent, only.Path); + Assert.True(only.CountedAsReal, + "an object record is deserialised against what is loaded, so a failure is real: " + + "the recorded Revit year is how the field was written, not a configuration that was skipped"); + } + + // The same record where the repository does still build the recorded year. This one + // never reached the reclassification block, because the exact name is loaded, and it is + // here so the fix is not credited with behaviour that already worked. + [Fact] + public void AnObjectRecordNamingABuiltYear_IsAlsoARealFailure() + { + var closure = ClosureBuilding("Revit_Tagging_oM_2022"); + var diagnostics = new List(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Tagging.Settings.TagSettings"), + RealAttribution(closure, "BH.oM.Tagging.Settings"), null, null, diagnostics, + closure: closure, + probeTypeCandidates: _ => ["Revit_Tagging_oM_2022"], + provenance: Map(("BH.oM.Tagging.Settings.TagSettings", "Revit_Tagging_oM_2022"))); + + Assert.True(Assert.Single(diagnostics).CountedAsReal); + } + + // Attribution, not reclassification, is what drops another repository's record, and it + // drops it before the block is reached. Pinned because it is the outcome the block's + // foreign branch looks like it provides and does not. + [Fact] + public void AnObjectRecordDeclaredByAnotherRepository_IsDroppedAtAttribution() + { + var closure = ClosureBuilding("Revit_Tagging_oM_2024"); + var diagnostics = new List(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + RealAttribution(closure, "BH.oM.Structure.Elements"), null, null, diagnostics, + closure: closure, + probeTypeCandidates: _ => ["Structure_oM"], + provenance: Map(("BH.oM.Structure.Elements.Panel", "Structure_oM"))); + + Assert.Empty(diagnostics); + } + // ------------------------------------------------------------------ // Closed generics. The class the earlier message-based route silently lost. // ------------------------------------------------------------------ diff --git a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs index 95b2517..dfefa20 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs @@ -710,7 +710,24 @@ private static void CollectLeafFailures( // describes code we were never asked about. When nothing answered, the path is // DeclaringTypeNotLoaded, which is the signal that the type is genuinely gone; // reclassifying that would convert a real removal into a silent pass. + // Not for object records, and provenanceType is how they are told apart: it is set + // only when the dataset answered. + // + // Both outcomes below need a per-assembly probe to have been prevented from running, + // and an object record has none. It is deserialised against whatever is loaded, so a + // failure is a failure of the code that was built, and "that configuration was not + // compiled" is not an escape available to it. The declaring assembly here is also + // not evidence about which configuration ran: the backfill records the lowest Revit + // year present at capture, 2022 on all 162 year-suffixed records in 9.3, so matching + // it exactly against LoadedNames asks a question about how the field was written. + // That is the exact-year comparison the locked decisions rule out. + // + // Without this, a repository that has moved past 2022 sees a real object-record + // regression demoted to unverified with a cause that reads as a build gap: Error + // becomes Warning and the job exits 0. Nine assembly families are exposed and all + // nine still build 2022 today, so it is latent until one of them drops it. if (cause is null && closure is not null && declaringAssembly is not null + && provenanceType is null && candidates.Count > 0 && !closure.LoadedNames.Contains(declaringAssembly)) { From a8b9afdc3572c7e9ec788eefb3ef521b0023354a Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Tue, 29 Sep 2026 11:20:45 +0100 Subject: [PATCH 6/7] chore(ci-versioning): use fixture names that identify nothing The dataset-provenance tests and one comment in DatasetProvenance.cs named the oM assemblies of three private Revit tools. Each of those names is a literal project directory in a private repository, and a path search on it returns that repository alone, so carrying it in a public repository identifies it. Substituted for names that identify nothing, reusing two schemes already in this repository rather than inventing any: Revit_X_oM_20NN, which SerialiserRunner's HostDependencyTests already uses as the placeholder for a year-suffixed Revit oM assembly, and the public Revit_Core_Engine_2022 method-event fixture from ClosureReclassificationTests. Year suffixes, family distinctness and ordinal ordering are all preserved, so every assertion tests what it tested before. DatasetProvenanceTests' header comment claimed the file named only public assemblies while using one of these names five times. Rewritten to a claim the file meets. 193 tests pass, unchanged from before the substitution. Eleven mutation checks ran, five on the map and six on the collector, and each one failed tests and reverted clean. --- .../DatasetProvenanceTests.cs | 16 +++--- .../ObjectRecordProvenanceTests.cs | 54 +++++++++---------- .../Commands/DatasetProvenance.cs | 2 +- 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/DatasetProvenanceTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/DatasetProvenanceTests.cs index df3b76f..540bbc8 100644 --- a/tools/VersioningRunner/src/VersioningRunner.Tests/DatasetProvenanceTests.cs +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/DatasetProvenanceTests.cs @@ -6,9 +6,9 @@ namespace VersioningRunner.Tests { // The map from a dataset record's type name to the assembly that declared it. // - // Every fixture here is synthetic and names only public assemblies. Real 9.3 records name - // the oM assemblies of private Revit tools, and a test fixture is as much of a leak as a - // comment is. + // Every fixture here names either a public BHoM assembly or a placeholder that resolves + // to nothing. Real records name the oM assemblies of private Revit tools, and a test + // fixture carries such a name into a public repository as readily as a comment does. public class DatasetProvenanceTests { // A closed generic. Its name carries commas and brackets, and it is the shape that the @@ -43,12 +43,12 @@ public void AgreedAcrossVersions_MapsTheType() public void PresentInOneVersionOnly_IsStillMapped() { using var datasets = new TempDatasets() - .WithObjects("9.2", Record("BH.oM.Tagging.Settings.TagSettings", "Tagging_oM")) + .WithObjects("9.2", Record("BH.oM.Environment.Elements.Panel", "Environment_oM")) .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")); var map = DatasetProvenance.Build(datasets.Root); - Assert.Equal("Tagging_oM", map.DeclaringAssemblyFor("BH.oM.Tagging.Settings.TagSettings")); + Assert.Equal("Environment_oM", map.DeclaringAssemblyFor("BH.oM.Environment.Elements.Panel")); Assert.Equal("Acoustic_oM", map.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); } @@ -104,13 +104,13 @@ public void DisputedCandidatesAreOrderedIndependentlyOfTheVersionTheyCameFrom() public void YearVariantsAcrossVersions_AreNotADisagreement() { using var datasets = new TempDatasets() - .WithObjects("9.2", Record("BH.oM.Tagging.Settings.TagSettings", "Tagging_oM_2022")) - .WithObjects("9.3", Record("BH.oM.Tagging.Settings.TagSettings", "Tagging_oM_2023")); + .WithObjects("9.2", Record("BH.oM.Adapters.Revit.Elements.ModelInstance", "Revit_X_oM_2022")) + .WithObjects("9.3", Record("BH.oM.Adapters.Revit.Elements.ModelInstance", "Revit_X_oM_2023")); var map = DatasetProvenance.Build(datasets.Root); Assert.Empty(map.Disputed); - Assert.Equal("Tagging_oM_2022", map.DeclaringAssemblyFor("BH.oM.Tagging.Settings.TagSettings")); + Assert.Equal("Revit_X_oM_2022", map.DeclaringAssemblyFor("BH.oM.Adapters.Revit.Elements.ModelInstance")); } [Fact] diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs index 657969c..36f3a69 100644 --- a/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs @@ -41,7 +41,7 @@ private static DeclaringAssemblyMap Disputed(string type, params string[] assemb "BH.oM.Structure.Results.ResultEnvelope`1[[BH.oM.Structure.Results.ConnectionForce, StructuralEngineering_oM, Version=9.0.0.0, Culture=neutral, PublicKeyToken=null]]"; private const string MethodEvent = - "Method ApplyDuctInsulation from { \"_t\" : \"System.Type\", \"Name\" : \"BH.Revit.Engine.MechanicalPlumbing.Compute, Revit_MechanicalPlumbing_Engine_2022, Version=9.0.0.0, Culture=neutral, PublicKeyToken=null\", \"_bhomVersion\" : \"9.2\" } failed to deserialise."; + "Method TryGetValueFromSource from { \"_t\" : \"System.Type\", \"Name\" : \"BH.Revit.Engine.Core.Compute, Revit_Core_Engine_2022, Version=9.0.0.0, Culture=neutral, PublicKeyToken=null\", \"_bhomVersion\" : \"9.2\" } failed to deserialise."; private static FakeTestResult Tree(string description, params string[] events) { @@ -168,12 +168,12 @@ public void MethodEventStillWins_WhenTheMapAlsoAnswers() { var diagnostics = new List(); RunCommand.ExtractFilteredResult( - Tree("BH.Revit.Engine.MechanicalPlumbing.Compute. }", MethodEvent), + Tree("BH.Revit.Engine.Core.Compute. }", MethodEvent), (_, _) => (true, AttributionBasis.NotRecorded), null, (_, _, _) => (null, ClassificationPath.DeclaringTypeNotLoaded, Array.Empty()), diagnostics, - provenance: Map(("BH.Revit.Engine.MechanicalPlumbing.Compute. }", "Acoustic_oM"))); + provenance: Map(("BH.Revit.Engine.Core.Compute. }", "Acoustic_oM"))); - Assert.Equal("Revit_MechanicalPlumbing_Engine_2022", Assert.Single(diagnostics).DeclaringAssembly); + Assert.Equal("Revit_Core_Engine_2022", Assert.Single(diagnostics).DeclaringAssembly); } // Whole closure discards the declaring assembly for attribution: Execute wires @@ -204,15 +204,15 @@ public void WholeClosure_IgnoresTheDeclaringAssemblyForAttribution() // ------------------------------------------------------------------ [Theory] - [InlineData("Revit_ModelQA_oM_2022")] - [InlineData("Revit_ModelQA_oM_2023")] - [InlineData("Revit_ModelQA_oM_2024")] - [InlineData("Revit_ModelQA_oM_2025")] - [InlineData("Revit_ModelQA_oM_2026")] + [InlineData("Revit_X_oM_2022")] + [InlineData("Revit_X_oM_2023")] + [InlineData("Revit_X_oM_2024")] + [InlineData("Revit_X_oM_2025")] + [InlineData("Revit_X_oM_2026")] public void AnyYearAttributesToASubjectBuildingAnyOtherYear(string recordedAssembly) { // The subject built Release2024, so infer-verification-config staged exactly one year. - var closure = ClosureForSubject("Revit_ModelQA_oM_2024.dll"); + var closure = ClosureForSubject("Revit_X_oM_2024.dll"); Assert.True(RunCommand.IsFromSubjectAssembly(recordedAssembly, closure), $"{recordedAssembly} must attribute to a Release2024 build: all five year variants are " + @@ -225,10 +225,10 @@ public void AnyYearAttributesToASubjectBuildingAnyOtherYear(string recordedAssem [Fact] public void TheYearIsNotComparedExactly() { - var closure = ClosureForSubject("Revit_ModelQA_oM_2024.dll"); + var closure = ClosureForSubject("Revit_X_oM_2024.dll"); - Assert.True(RunCommand.IsFromSubjectAssembly("Revit_ModelQA_oM_2022", closure)); - Assert.False(RunCommand.IsFromSubjectAssembly("Revit_Tagging_oM_2022", closure)); + Assert.True(RunCommand.IsFromSubjectAssembly("Revit_X_oM_2022", closure)); + Assert.False(RunCommand.IsFromSubjectAssembly("Revit_Y_oM_2022", closure)); } // StripConfigSuffix is anchored `_20\d{2}$`, so an `_asm` carrying a file extension does @@ -268,8 +268,8 @@ public void MoreThanOneAssemblyDeclaringTheType_IsRecordedAsCandidates() RunCommand.ExtractFilteredResult( Tree("BH.oM.Acoustic.Panel"), (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, - probeTypeCandidates: _ => ["Revit_ModelQA_oM_2022", "Revit_ModelQA_oM_2023"], - provenance: Map(("BH.oM.Acoustic.Panel", "Revit_ModelQA_oM_2022"))); + probeTypeCandidates: _ => ["Revit_X_oM_2022", "Revit_X_oM_2023"], + provenance: Map(("BH.oM.Acoustic.Panel", "Revit_X_oM_2022"))); var only = Assert.Single(diagnostics); Assert.NotNull(only.DeclaringTypeCandidates); @@ -388,18 +388,18 @@ private static ClosureContext ClosureBuilding(string loadedAssembly) [Fact] public void AnObjectRecordNamingAnUnbuiltYear_IsStillARealFailure() { - var closure = ClosureBuilding("Revit_Tagging_oM_2024"); + var closure = ClosureBuilding("Revit_Y_oM_2024"); var diagnostics = new List(); RunCommand.ExtractFilteredResult( - Tree("BH.oM.Tagging.Settings.TagSettings"), - RealAttribution(closure, "BH.oM.Tagging.Settings"), null, null, diagnostics, + Tree("BH.oM.Adapters.Revit.Elements.ModelInstance"), + RealAttribution(closure, "BH.oM.Adapters.Revit.Elements"), null, null, diagnostics, closure: closure, - probeTypeCandidates: _ => ["Revit_Tagging_oM_2024"], - provenance: Map(("BH.oM.Tagging.Settings.TagSettings", "Revit_Tagging_oM_2022"))); + probeTypeCandidates: _ => ["Revit_Y_oM_2024"], + provenance: Map(("BH.oM.Adapters.Revit.Elements.ModelInstance", "Revit_Y_oM_2022"))); var only = Assert.Single(diagnostics); - Assert.Equal("Revit_Tagging_oM_2022", only.DeclaringAssembly); + Assert.Equal("Revit_Y_oM_2022", only.DeclaringAssembly); Assert.Equal(AttributionBasis.DeclaringAssembly, only.AttributedBy); Assert.Null(only.Cause); Assert.Equal(ClassificationPath.NoMethodEvent, only.Path); @@ -414,15 +414,15 @@ public void AnObjectRecordNamingAnUnbuiltYear_IsStillARealFailure() [Fact] public void AnObjectRecordNamingABuiltYear_IsAlsoARealFailure() { - var closure = ClosureBuilding("Revit_Tagging_oM_2022"); + var closure = ClosureBuilding("Revit_Y_oM_2022"); var diagnostics = new List(); RunCommand.ExtractFilteredResult( - Tree("BH.oM.Tagging.Settings.TagSettings"), - RealAttribution(closure, "BH.oM.Tagging.Settings"), null, null, diagnostics, + Tree("BH.oM.Adapters.Revit.Elements.ModelInstance"), + RealAttribution(closure, "BH.oM.Adapters.Revit.Elements"), null, null, diagnostics, closure: closure, - probeTypeCandidates: _ => ["Revit_Tagging_oM_2022"], - provenance: Map(("BH.oM.Tagging.Settings.TagSettings", "Revit_Tagging_oM_2022"))); + probeTypeCandidates: _ => ["Revit_Y_oM_2022"], + provenance: Map(("BH.oM.Adapters.Revit.Elements.ModelInstance", "Revit_Y_oM_2022"))); Assert.True(Assert.Single(diagnostics).CountedAsReal); } @@ -433,7 +433,7 @@ public void AnObjectRecordNamingABuiltYear_IsAlsoARealFailure() [Fact] public void AnObjectRecordDeclaredByAnotherRepository_IsDroppedAtAttribution() { - var closure = ClosureBuilding("Revit_Tagging_oM_2024"); + var closure = ClosureBuilding("Revit_Y_oM_2024"); var diagnostics = new List(); RunCommand.ExtractFilteredResult( diff --git a/tools/VersioningRunner/src/VersioningRunner/Commands/DatasetProvenance.cs b/tools/VersioningRunner/src/VersioningRunner/Commands/DatasetProvenance.cs index 3cb831d..2522d58 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Commands/DatasetProvenance.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Commands/DatasetProvenance.cs @@ -155,7 +155,7 @@ public static DeclaringAssemblyMap Build(string? datasetsRoot) /**** Private Methods ****/ /*************************************/ - // Two versions naming Revit_Tagging_oM_2022 and Revit_Tagging_oM_2023 are not disagreeing. + // Two versions naming Revit_X_oM_2022 and Revit_X_oM_2023 are not disagreeing. // They are the same repository under two build configurations, attribution strips the year // anyway, and treating the pair as disputed would drop the finding to the namespace guess // for no gain. That is the failure "compare year-insensitively" exists to prevent, applied From ab65aed5a94cef7a91176e662f4c05d8765a8301 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Tue, 29 Sep 2026 16:56:24 +0100 Subject: [PATCH 7/7] fix(ci-versioning): say which ambiguity a finding has, and count what was dropped Two reporting defects, both of which let a run say something that is not true. The ambiguity warning describes one population and now covers three. Candidates used to come only from the method signature probe, where more than one match genuinely means enumeration order decided the answer. Reading an object record's declaring assembly from the dataset added two more sources: a closure scan that reports how many assemblies also declare a type the dataset had already settled, and a dataset dispute where nothing was attributed at all. The single message asserted enumeration order for all three, so it could tell someone load order explained a finding it had no part in. The counter now splits by source and each message says what actually happened. The source is recorded on the diagnostic rather than derived. It is inferable today from Path plus a null check on DeclaringAssembly, which is exactly the coupling worth avoiding: that inference breaks silently the first time either field moves for an unrelated reason. Nothing counted findings dropped at attribution. A dropped leaf returns before it can produce a diagnostic row, so a run reporting zero findings because nothing failed and one reporting zero because attribution discarded everything were the same output in every surface. Both counts are now carried on the coverage record, printed in every state including zero, and shown as a summary row. A run whose entire population was dropped now says so, because that result rests on attribution being right rather than on nothing having failed. The counter counts leaves dropped at attribution. It is not a diff against a previous run and the comment says so in both places, because the two coincide on the validation run and would be easy to conflate. Also removed: six citations of an issue number in a repository whose issues are disabled, so the reference resolved to nothing for any reader. One of the sentences carrying it also named a private repository's assembly, and that went with the rewrite rather than being left in a line being edited anyway. 201 tests pass, 193 of them pre-existing and unmodified. Five mutation checks ran against the new behaviour, covering the drop count, its split by basis, and each of the three candidate sources. Each failed tests and reverted clean. --- .github/actions/ci-versioning/action.yml | 6 + .../AssemblyLoadOrderTests.cs | 2 +- .../ObjectRecordProvenanceTests.cs | 129 ++++++++++++++++++ .../VersioningRunner.Tests/RunCommandTests.cs | 2 +- .../VersioningRunner/Commands/RunCommand.cs | 115 +++++++++++++--- .../Models/VersioningResult.cs | 64 ++++++++- 6 files changed, 296 insertions(+), 22 deletions(-) diff --git a/.github/actions/ci-versioning/action.yml b/.github/actions/ci-versioning/action.yml index 843a46a..f1db736 100644 --- a/.github/actions/ci-versioning/action.yml +++ b/.github/actions/ci-versioning/action.yml @@ -898,6 +898,12 @@ runs: # state until the dataset backfill lands. Printed either way, for the same reason the # two rows above it are. $md += "| Dataset types carrying a declaring assembly | $($coverage.TypesWithDeclaringAssembly) |" + # Findings discarded at attribution as another repository's. Printed either way, + # because without it a run that filtered everything out and a run that found + # nothing wrong produce the same table. It counts leaves dropped at attribution + # and is not a diff against a previous run. + $dropped = $coverage.DroppedByDeclaringAssembly + $coverage.DroppedByNamespaceFallback + $md += "| Dropped as another repository's | $dropped ($($coverage.DroppedByDeclaringAssembly) by declaring assembly, $($coverage.DroppedByNamespaceFallback) by namespace prefix) |" } $md += "| Build configuration | ``$(if ($configuration) { $configuration } else { 'not recorded' })`` |" $md += "" diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs index 30f6d8d..3b647a3 100644 --- a/tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs @@ -6,7 +6,7 @@ namespace VersioningRunner.Tests // Load order is not cosmetic. ProbeDeclaringType takes its verdict from the first // loaded assembly that yields the declaring type, and 42 type names in the fleet are // defined by more than one assembly, so the enumeration order decides the - // classification for those. See CI_Toolkit#161. + // classification for those. public class AssemblyLoadOrderTests { private static string[] Names(IEnumerable paths) diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs index 36f3a69..a3ca163 100644 --- a/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs @@ -446,6 +446,135 @@ public void AnObjectRecordDeclaredByAnotherRepository_IsDroppedAtAttribution() Assert.Empty(diagnostics); } + // ------------------------------------------------------------------ + // Where a candidate list came from. + // + // The three sources mean different things and only one is about enumeration order, + // so the run-level warning splits on this. Recorded rather than derived from Path + // plus a null check, because that derivation breaks silently the first time either + // of those moves for an unrelated reason. + // ------------------------------------------------------------------ + + [Fact] + public void CandidatesFromAClosureScan_AreNotRecordedAsOrderDependent() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + probeTypeCandidates: _ => ["Revit_X_oM_2022", "Revit_X_oM_2023"], + provenance: Map(("BH.oM.Acoustic.Panel", "Revit_X_oM_2022"))); + + Assert.Equal(CandidateSource.ClosureScan, Assert.Single(diagnostics).CandidatesFrom); + } + + [Fact] + public void CandidatesFromADatasetDispute_AreRecordedAsSuch() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Disputed("BH.oM.Structure.Elements.Panel", "StructuralEngineering_oM", "Structure_oM")); + + Assert.Equal(CandidateSource.DatasetDispute, Assert.Single(diagnostics).CandidatesFrom); + } + + // The pre-existing source, and the only one where enumeration order decides anything. + [Fact] + public void CandidatesFromTheSignatureProbe_StayRecordedAsOrderDependent() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.Revit.Engine.Core.Compute. }", MethodEvent), + (_, _) => (true, AttributionBasis.NotRecorded), null, + (_, _, _) => (null, ClassificationPath.SignatureResolved, ["One_Engine", "Two_Engine"]), + diagnostics); + + Assert.Equal(CandidateSource.SignatureProbe, Assert.Single(diagnostics).CandidatesFrom); + } + + [Fact] + public void NoCandidates_LeaveTheSourceUnrecorded() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Map(("BH.oM.Acoustic.Panel", "Acoustic_oM"))); + + Assert.Equal(CandidateSource.NotRecorded, Assert.Single(diagnostics).CandidatesFrom); + } + + // ------------------------------------------------------------------ + // Findings dropped at attribution. + // + // A dropped leaf produces no diagnostic row, so before this it was counted nowhere + // and a run that discarded everything read exactly like a run that found nothing. + // ------------------------------------------------------------------ + + [Fact] + public void ADroppedFinding_IsCountedAgainstTheEvidenceThatDroppedIt() + { + var drops = new AttributionDrops(); + var diagnostics = new List(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (false, AttributionBasis.DeclaringAssembly), null, null, diagnostics, + provenance: Map(("BH.oM.Structure.Elements.Panel", "Structure_oM")), + drops: drops); + + Assert.Empty(diagnostics); + Assert.Equal(1, drops.Total); + Assert.Equal(1, drops.ByDeclaringAssembly); + Assert.Equal(0, drops.ByNamespaceFallback); + } + + [Fact] + public void ADropOnTheNamespaceGuess_IsCountedSeparately() + { + var drops = new AttributionDrops(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (false, AttributionBasis.NamespaceFallback), null, null, null, + drops: drops); + + Assert.Equal(1, drops.ByNamespaceFallback); + Assert.Equal(0, drops.ByDeclaringAssembly); + } + + // The denominator's other half: a kept finding must not be counted as dropped, or the + // two numbers stop summing to the population and neither can be trusted. + [Fact] + public void AKeptFinding_IsNotCountedAsDropped() + { + var drops = new AttributionDrops(); + var diagnostics = new List(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.DeclaringAssembly), null, null, diagnostics, + provenance: Map(("BH.oM.Acoustic.Panel", "Acoustic_oM")), + drops: drops); + + Assert.Single(diagnostics); + Assert.Equal(0, drops.Total); + } + + // Every existing caller passes nothing, and must keep working. + [Fact] + public void NoCounterSupplied_IsNotAnError() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (false, AttributionBasis.DeclaringAssembly), null, null, diagnostics); + + Assert.Empty(diagnostics); + } + // ------------------------------------------------------------------ // Closed generics. The class the earlier message-based route silently lost. // ------------------------------------------------------------------ diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/RunCommandTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/RunCommandTests.cs index 1f38105..5f5de61 100644 --- a/tools/VersioningRunner/src/VersioningRunner.Tests/RunCommandTests.cs +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/RunCommandTests.cs @@ -1020,7 +1020,7 @@ public class ProbeDeclaringTypeTests public void TypeInTwoLoadedAssemblies_RecordsBothCandidates() { // The same assembly listed twice stands in for two repos declaring into one - // namespace, which is the real shape of CI_Toolkit#161. The probe result must be + // namespace, which is the real shape of the collision. The probe result must be // unchanged and the ambiguity must be visible. var twice = new List { typeof(string).Assembly, typeof(string).Assembly }; diff --git a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs index dfefa20..807160d 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs @@ -156,6 +156,7 @@ public static int Execute( (typeFullName, methodName, declaringAssembly) => ProbeDeclaringType(loaded, typeFullName, methodName, declaringAssembly); Func> probeTypeCandidates = t => ProbeTypeCandidates(loaded, t); + var drops = new AttributionDrops(); foreach (var method in methods) { object? rawResult; @@ -182,7 +183,7 @@ public static int Execute( return 1; } - var partial = ExtractFilteredResult(rawResult, isAttributable, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates, provenance); + var partial = ExtractFilteredResult(rawResult, isAttributable, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates, provenance, drops); allFailures.AddRange(partial.Failures); } @@ -302,7 +303,9 @@ public static int Execute( SubjectTypes: s_subjectTypeCount, DatasetVersions: testAll ? 0 : 1, RecordsUnverified: unresolvableSkips.Count, - TypesWithDeclaringAssembly: provenance.TypesMapped); + TypesWithDeclaringAssembly: provenance.TypesMapped, + DroppedByDeclaringAssembly: drops.ByDeclaringAssembly, + DroppedByNamespaceFallback: drops.ByNamespaceFallback); var result = new VersioningResult { @@ -330,6 +333,29 @@ public static int Execute( $"{coverage.LoadedAssemblies} assembl(ies) loaded; " + $"{coverage.VerifyEntryPoints} FromJsonDatasets entry point(s) invoked; " + $"{coverage.RecordsUnverified} record(s) attributed but not verified"); + + // Printed in every state, zero included. This is the only account of findings that + // left the run without producing a row, so a run that filtered everything out and a + // run that found nothing wrong are otherwise the same output. + // + // Counts leaves dropped at attribution. It is NOT a diff against a previous run and + // must not be read as one: it does not say how many findings some earlier + // configuration would have reported, even where the two happen to coincide. + Console.WriteLine( + $"Attribution: {result.FailureCount + unresolvableSkips.Count} finding(s) kept, " + + $"{drops.Total} dropped as another repository's " + + $"({drops.ByDeclaringAssembly} by declaring assembly, " + + $"{drops.ByNamespaceFallback} by namespace prefix)."); + + // A green that rests entirely on having discarded everything is a different statement + // from a green that examined findings and found them sound, and the exit code cannot + // tell them apart. + if (drops.Total > 0 && result.FailureCount == 0 && unresolvableSkips.Count == 0) + Console.Error.WriteLine( + $"::warning title=Versioning::Every one of the {drops.Total} finding(s) in this run was " + + "attributed to another repository, so this result rests on attribution being right rather " + + "than on nothing having failed."); + if (configuration is not null) Console.WriteLine($"Configuration: {configuration}"); @@ -337,11 +363,36 @@ public static int Execute( // result.Failures. Counting every diagnostic made the total disagree with the detail as // soon as a finding could be reclassified to unverified: the warning claimed N ambiguous // findings while fewer than N were listed. - int ambiguous = diagnostics.Count(d => d.CountedAsReal && d.DeclaringTypeCandidates is { Count: > 1 }); - if (ambiguous > 0) + // + // Split by source. A single message over all three was wrong for two of them: only the + // signature probe lets enumeration order decide the answer. The closure scan reports how + // many assemblies also declare a type the dataset had already settled, and a dataset + // dispute means nothing was attributed at all. Telling someone load order explained a + // finding it had no part in sends them to the wrong place. + var ambiguousReal = diagnostics + .Where(d => d.CountedAsReal && d.DeclaringTypeCandidates is { Count: > 1 }) + .ToList(); + + int orderDependent = ambiguousReal.Count(d => d.CandidatesFrom == CandidateSource.SignatureProbe); + if (orderDependent > 0) + Console.Error.WriteLine( + $"::warning title=Versioning::{orderDependent} finding(s) were attributed by probing the loaded " + + "assemblies for the declaring type, and more than one matched, so which assembly was recorded " + + "depended on the order the closure was enumerated in."); + + int alsoDeclared = ambiguousReal.Count(d => d.CandidatesFrom == CandidateSource.ClosureScan); + if (alsoDeclared > 0) Console.Error.WriteLine( - $"::warning title=Versioning::{ambiguous} finding(s) have a declaring type present in more than one loaded assembly, " + - "so their classification depended on assembly enumeration order. See CI_Toolkit#161."); + $"::warning title=Versioning::{alsoDeclared} finding(s) name a type that more than one loaded " + + "assembly declares. The dataset named the declaring assembly and settled each one, so this is " + + "reported for visibility rather than because the answer was in doubt."); + + int disputed = ambiguousReal.Count(d => d.CandidatesFrom == CandidateSource.DatasetDispute); + if (disputed > 0) + Console.Error.WriteLine( + $"::warning title=Versioning::{disputed} finding(s) name a type whose declaring assembly the " + + "dataset versions disagree about, so none was attributed and each fell back to the namespace " + + "prefix."); const int maxLogged = 50; foreach (var failure in result.Failures.Take(maxLogged)) @@ -355,7 +406,19 @@ public static int Execute( if (d?.VersionConditional == VersionConditionalState.Yes) context.Add($"Signature is version-conditional. Built as {d.Configuration ?? "an unrecorded configuration"}."); if (d?.DeclaringTypeCandidates is { Count: > 1 } cands) - context.Add($"Declaring type present in {cands.Count} loaded assemblies ({string.Join(", ", cands.Take(3))}), so classification depended on enumeration order."); + { + string joined = string.Join(", ", cands.Take(3)); + context.Add(d.CandidatesFrom switch + { + CandidateSource.SignatureProbe => + $"Declaring type present in {cands.Count} loaded assemblies ({joined}), so classification depended on enumeration order.", + CandidateSource.ClosureScan => + $"Declaring assembly came from the dataset ({d.DeclaringAssembly}). {cands.Count} loaded assemblies also declare this type ({joined}); the dataset settled it, not enumeration order.", + CandidateSource.DatasetDispute => + $"Dataset versions disagree about the declaring assembly ({joined}), so this finding was not attributed and fell back to the namespace prefix.", + _ => $"Declaring type present in {cands.Count} loaded assemblies ({joined}).", + }); + } string suffix = context.Count > 0 ? " " + string.Join(" ", context) : string.Empty; Console.Error.WriteLine($"::error title=Versioning::{failure.Description}: {failure.Message}{suffix}"); @@ -434,9 +497,9 @@ internal static int ExitCodeFor(VersioningStatus status) // ProbeDeclaringType walks the loaded list and takes its verdict from the FIRST // assembly that yields the declaring type. Where two repos declare the same type, // whichever is enumerated first decides whether the finding reads as a genuine - // regression or as an infrastructure problem. That is CI_Toolkit#161's mechanism: - // BH.Revit.Engine.Core.Compute is defined by both Revit_Core_Engine and - // Revit_ModelQA_Engine, and 42 such type-level collisions exist across the fleet. + // regression or as an infrastructure problem. BH.Revit.Engine.Core.Compute is declared + // by Revit_Core_Engine and by one other assembly, and 42 such type-level collisions + // exist across the fleet. // // Directory.GetFiles documents no ordering. Measured on windows-2025-vs2026 across // four cold-rebuild runs on separate runners, NTFS returned exactly @@ -532,7 +595,10 @@ public static VersioningResult ExtractFilteredResult( Func>? probeTypeCandidates = null, // The dataset's answer for an object record's declaring assembly. Empty by default, so // a caller that supplies nothing keeps the namespace fallback it had before. - DeclaringAssemblyMap? provenance = null) + DeclaringAssemblyMap? provenance = null, + // Counts findings discarded at attribution. Optional so every existing caller is + // unchanged; when absent the drops are simply not counted, as before. + AttributionDrops? drops = null) { if (rawResult is null) return new VersioningResult @@ -543,7 +609,7 @@ public static VersioningResult ExtractFilteredResult( }; var failures = new List(); - CollectLeafFailures(rawResult, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex ?? LoadedTypeIndex.Empty, probeTypeCandidates, provenance ?? DeclaringAssemblyMap.Empty, depth: 0); + CollectLeafFailures(rawResult, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex ?? LoadedTypeIndex.Empty, probeTypeCandidates, provenance ?? DeclaringAssemblyMap.Empty, drops, depth: 0); var status = failures.Count > 0 ? VersioningStatus.Error : VersioningStatus.Pass; return new VersioningResult @@ -562,7 +628,7 @@ private static void CollectLeafFailures( Func Candidates)>? probeSignature, List? diagnostics, ClosureContext? closure, LoadedTypeIndex typeIndex, Func>? probeTypeCandidates, - DeclaringAssemblyMap provenance, int depth) + DeclaringAssemblyMap provenance, AttributionDrops? drops, int depth) { // BHoM's TestResult tree has at most 3 levels under the root (outer → per-version // summary → individual type result). Depth 5 gives headroom for unexpected nesting @@ -648,7 +714,13 @@ private static void CollectLeafFailures( var (attributable, attributedBy) = isAttributable(desc, declaringAssembly); if (!attributable) + { + // Counted before returning. This is the only place a finding leaves the run + // without producing a row, so without the count a filtered-out run and a + // clean run are indistinguishable in every output. + drops?.Count(attributedBy); return; + } // DescriptionFromJson mangles many method entries to ". }", // losing the method name. The Method event still carries both, so prefer it. @@ -666,6 +738,7 @@ private static void CollectLeafFailures( ? ClassificationPath.UnresolvableTypeAbsent : ClassificationPath.UnresolvableFromEvents; IReadOnlyList candidates = Array.Empty(); + var candidatesFrom = CandidateSource.NotRecorded; // No type-level cause recorded means the blocker may be in the signature // rather than the payload, which only reflection over the method can tell. @@ -692,12 +765,21 @@ private static void CollectLeafFailures( // tellable apart; measured 0 across the 1711 type names the two backfilled // versions share. if (disputedAssemblies is not null) + { candidates = disputedAssemblies; + candidatesFrom = CandidateSource.DatasetDispute; + } else if (probeTypeCandidates is not null && provenanceType is not null) + { candidates = probeTypeCandidates(provenanceType); + candidatesFrom = CandidateSource.ClosureScan; + } } else if (probeSignature is not null) + { (cause, path, candidates) = probeSignature(eventType, eventMethod, declaringAssembly); + candidatesFrom = CandidateSource.SignatureProbe; + } else path = ClassificationPath.ProbeNotSupplied; } @@ -792,7 +874,8 @@ private static void CollectLeafFailures( DeclaringTypeCandidates: candidates.Count > 1 ? candidates : null, Configuration: s_configuration, VersionConditional: ClassifyVersionConditional(eventType, eventMethod), - AttributedBy: attributedBy)); + AttributedBy: attributedBy, + CandidatesFrom: candidatesFrom)); } else { @@ -800,7 +883,7 @@ private static void CollectLeafFailures( { string childStatus = child.GetType().GetProperty("Status")?.GetValue(child)?.ToString() ?? "Pass"; if (childStatus is "Error" or "Warning") - CollectLeafFailures(child, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates, provenance, depth + 1); + CollectLeafFailures(child, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates, provenance, drops, depth + 1); } } } @@ -1229,7 +1312,7 @@ internal static VersionConditionalState ClassifyVersionConditional(string? typeF // // The loop used to return on the first match, which made the classification depend on // assembly enumeration order wherever two repos declare into the same namespace - // (CI_Toolkit#161, measured on BH.Revit.Engine.Core). The probe result still comes from + // (measured on BH.Revit.Engine.Core). The probe result still comes from // the first match, so behaviour is unchanged; the candidate list is recorded so a // classification that could have gone either way is visible rather than silent. internal static (string? Cause, ClassificationPath Path, IReadOnlyList Candidates) ProbeDeclaringType( diff --git a/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs b/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs index 6220b53..efa3920 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs @@ -103,6 +103,53 @@ public enum AttributionBasis NamespaceFallback, } +// Where a finding's candidate list came from. Recorded explicitly because the three sources +// mean different things and only one of them is about enumeration order, so a single message +// over all of them is wrong for two. Derivable today from Path plus a null check on +// DeclaringAssembly, which is the coupling this exists to avoid: that derivation breaks +// silently the first time either of those moves for an unrelated reason. +public enum CandidateSource +{ + // No candidate list, or one recorded before this was tracked. + NotRecorded, + // Method record. More than one loaded assembly matched the signature probe, so which one + // was recorded genuinely depended on the order the closure was enumerated in. + SignatureProbe, + // Object record. The dataset named the declaring assembly and settled it; the closure was + // then scanned to report how many other loaded assemblies also declare the type. The scan + // did not decide anything, so enumeration order did not either. + ClosureScan, + // Object record. Dataset versions named different assembly families for the type, so + // nothing was attributed and the finding fell back to the namespace prefix. + DatasetDispute, +} + +// Findings dropped at attribution, which produce no diagnostic row and were previously +// counted nowhere. Without this a run reporting zero findings because nothing failed and one +// reporting zero because attribution discarded everything are the same output. +// +// Counts leaves dropped at attribution. NOT a diff against a previous run: it does not say +// how many findings an earlier configuration would have reported, and must not be read that +// way even when the two coincide. +public sealed class AttributionDrops +{ + public int ByDeclaringAssembly { get; private set; } + public int ByNamespaceFallback { get; private set; } + public int ByUnrecordedBasis { get; private set; } + + public int Total => ByDeclaringAssembly + ByNamespaceFallback + ByUnrecordedBasis; + + public void Count(AttributionBasis basis) + { + switch (basis) + { + case AttributionBasis.DeclaringAssembly: ByDeclaringAssembly++; break; + case AttributionBasis.NamespaceFallback: ByNamespaceFallback++; break; + default: ByUnrecordedBasis++; break; + } + } +} + // What this run actually built, needed to tell "the recorded declaring // assembly is missing because it is someone else's" from "because we did not compile that // configuration" from "because it was genuinely removed". Passed explicitly rather than @@ -138,8 +185,8 @@ public record FailureDiagnostic( string? DeclaringType, string? DeclaringAssembly, int EventCount, - // Every assembly in the loaded set that yielded the declaring type. More than one - // means the classification depended on enumeration order (see CI_Toolkit#161). + // Every assembly that claimed the declaring type. What more than one means depends on + // where the list came from, which is why CandidatesFrom is recorded beside it. IReadOnlyList? DeclaringTypeCandidates = null, // Build configuration this run compiled, carried per row so a finding read on its // own is interpretable. Human-legible; it does not by itself explain a divergence. @@ -147,7 +194,10 @@ public record FailureDiagnostic( VersionConditionalState VersionConditional = VersionConditionalState.Unknown, // Which evidence attributed this failure to the subject. Per row rather than only as a // total, so a reader can tell whether any individual finding rests on the ambiguous path. - AttributionBasis AttributedBy = AttributionBasis.NotRecorded); + AttributionBasis AttributedBy = AttributionBasis.NotRecorded, + // Where DeclaringTypeCandidates came from. Only SignatureProbe means the answer depended + // on enumeration order. + CandidateSource CandidatesFrom = CandidateSource.NotRecorded); // Coverage denominator. A verdict without one cannot be interpreted: a pass over zero // methods reads identically to a pass over seven thousand. BHoMBot reported object @@ -170,7 +220,13 @@ public record CoverageCounts( // record was attributed by namespace prefix, which is the pre-backfill state and reads // identically in the artefact to a run that never found the dataset at all. The log says // which, for 90 days; this is the part that outlives it. - int TypesWithDeclaringAssembly = 0); + int TypesWithDeclaringAssembly = 0, + // Findings discarded at attribution as another repository's, split by the evidence that + // discarded them. Present for the same reason as RecordsUnverified: without it, a run + // that filtered everything out and a run that found nothing wrong are the same output. + // These count leaves dropped at attribution and are not a diff against a previous run. + int DroppedByDeclaringAssembly = 0, + int DroppedByNamespaceFallback = 0); public class VersioningResult {