Conversation
Object records carry only a type name, so the checker downstream decides
ownership by matching the namespace prefix against the repository under test.
Namespaces are shared and no rule over the name separates the owners at any
precision. The dataset's `_asm` field records the assembly that declared the
type at capture; this surfaces it so the checker can read it.
The checker sees this TestResult and never the dataset file, so an event on
the failure path is the only channel. The wording is a contract with
CI_Toolkit's VersioningRunner, which parses it in ParseObjectEventAssembly:
Object <FullTypeName> declared in "<TypeName>, <AssemblyName>"
Read from the raw json rather than the deserialised object, because the
serialiser skips every field whose name begins with an underscore and because
this runs where there is often no object left to ask. The reader is a
depth-aware scan rather than a regex, so an identically named field on a
nested fragment cannot be mistaken for the record's own, and rather than a
JSON library, which would mean a new dependency for one field read.
Emitted only on the failure path, only when the field is present, and never
for method records: their declaring assembly already reaches the checker
through the Method event, `_asm` is prohibited on Methods.json, and a method
record's top-level type is System.Reflection.MethodBase, so emitting one would
state something untrue.
DescriptionFromJson is not touched. Its positional splits are unchanged.
|
The
The same failure, with the same message, occurred three times on #348, which was one It does not gate either. The required contexts on Every other check passes, twelve of them, including |
1 participant
NOTE: Depends on
BHoM/CI_Toolkit#22
Merge after that one. It teaches the checker to read the field; this one emits it. Landing this first would emit something nothing reads.
It also depends on the dataset backfill that adds the field to the 9.3 object records, which is prepared and not yet raised. Until both land this emits nothing, because the field is absent from the dataset today.
Issues addressed by this PR
The versioning check has to decide which repository a finding belongs to. An object record carries a type name and nothing else, so ownership is inferred from the namespace, and namespaces are shared across repositories. It therefore reports other repositories' failures against your pull request:
BHoM/BHoM_Adapterrun31844962538reported 1,056 failures, none of them that repository's.BHoM/BHoMrun33849699768reported 145 findings, all attributed by namespace guess and none by evidence.This surfaces the declaring assembly into the test result, which is the only thing the checker reads, so findings that were never this repository's stop being reported against it. Applied to those same 145 findings: 98 are dropped as correctly another repository's, 46 remain unverified for unrelated reasons, and 1 becomes a real failure.
Test files
Run end to end against the real dataset and the real checker, by inducing a failure on three real records:
BH.oM.Structure.Elements.BarStructure_oMBH.Revit.oM.ModelQA…ColumnContinuityConditionRevit_ModelQA_oM_2022BH.oM.Structure.Results.ResultEnvelope\1[[…]]`StructuralEngineering_oMThe third row is the point of the change: its namespace says this repository, its declaring assembly says otherwise, and the declaring assembly is right.
Changelog
Versioning object-record failures now carry the assembly that declares the type, so the check attributes them by evidence instead of by namespace.
Additional comments
BH.oM.Structure.Loads.LoadCombinationbecomes a real failure onBHoM/BHoM, and that is correct. The record is declared byStructure_oM, whichBHoM/BHoMbuilds, so the finding genuinely is that repository's. Until now it was excused because ownership could not be established, not because it was someone else's.ci-versioningis not a required check anywhere, so this is visible rather than blocking.The 98 / 46 / 1 figures are derived rather than observed: real run artefacts replayed through the real checker code, measured twice on different dataset bases for the same answer, runs
34100313934against 9.2 and35986238826against 9.3. No live run of this change exists yet, because that needs all three parts merged. The three-record table above is observed, end to end, on this branch.The checker parses exactly this shape, and it cannot be changed quietly later:
Rewording it does not fail loudly. It silently turns the feature off and returns every finding to the namespace guess. The quoted part holds the assembly alone because closed generic type names contain commas, and a comma-delimited form cannot represent one.
The event is emitted only on the failure path, only when the field is present, and never for method records. It is read from the raw json rather than the deserialised object, because the deserialiser skips fields beginning with an underscore, and read depth-aware so a nested fragment carrying the same name cannot be mistaken for the record's own.
Helpers/DescriptionFromJson.csis untouched. It reads fixed positional indices, and adding a field does shift them on shorter records, but those indices are reached only for method records and every object record returns before them. Measured across all 1,731: 0 descriptions differ.