Conversation
…ecords Object records in the versioning test datasets record what a type is called but not where it is declared, so a consumer reading one has only a type name. Method records do not have this problem: a method's declaring type is serialised assembly-qualified. Adds an `_asm` field naming the assembly the object's type came from. One property read, one field written, on the successful records only. The field is added after the ToJson, FromJson and IsEqual checks, so those run on the record as serialised and the method's verdict is unchanged. The output tuple is unchanged. Method records are skipped. The field is placed immediately before `_bhomVersion` so it sits past the positional indices Helpers.DescriptionFromJson reads. Inert to existing readers: the deserialiser skips fields whose name begins with an underscore.
sakanni
marked this pull request as ready for review
September 14, 2026 14:27
sakanni
requested review from
a team,
adecler and
peterjamesnugent
and removed request for
adecler and
peterjamesnugent
September 14, 2026 14:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues addressed by this PR
Refs BHoM/internal-tickets#35
Two questions for you, and they are the reason this is open now.
Does
#9c_GenerateVersioningDatasets.gh's merge key depend on the record's text? It merges records rather than appending them. If the key comes from the record's text, adding a field could change what merges with what; if it comes from a parsed field such as_t, it cannot. The capture scripts are binary Grasshopper archives and are not reviewable by any means available to us, so we cannot tell which. This is the largest unknown downstream of the change.BHoMBot was cleared by reading its source, not by running it. We traced its versioning path and found nothing this field disturbs: the verdict is status-driven and this field cannot change a record's status, the installer's dataset step reads file names and never contents, and the versioning-error table has no writer. That is a code-review conclusion, not an executed one, and we would rather say so than let it read as tested. Happy to share the full trace.
Timing. This is under a release freeze and is not expected to merge yet. Review is welcome in the meantime; it will sit until the freeze lifts. It is open now rather than later so the two questions above can be answered while there is time.
9.3's dataset will need the declaring assembly backfilled by hand, the way 9.2's was. That is expected and accounted for. What this change does is stop that repeating from 9.4 onward.
What it does. Object records in the versioning datasets record what a type is called, not where it is declared, so anything deciding which repository a record belongs to has to infer it from the namespace, and namespaces are shared between repositories. Method records do not have this problem, because a method's declaring type is serialised assembly-qualified. This adds an
_asmfield naming the declaring assembly. Capture is the only point in the chain where a live object still exists, which is why the change is here. One file, +46/-1.Test files
TryToJsonAndFromJsonAndCheckIfEqualover 400 dummied object types, this branch againstdevelop: tuple width, all eight output counts and their membership identical, and with_asmremoved the 400 records are byte-identical.Helpers.DescriptionFromJsonoutput unchanged on all 400. No existing test is modified. The capture pipeline itself was not run, since it needs Grasshopper and two Rhino versions, so this covers the method and not the pipeline, and says nothing about question 1.Changelog
Captured object records carry
_asm, naming the assembly that declares the object's type.Additional comments
Inert on its own, and inert to existing readers: the deserialiser skips any field whose name begins with an underscore. Nothing reads
_asmuntil BHoM/CI_Toolkit#22 and BHoM/Versioning_Toolkit#352 land.The constraints, which are the part worth scrutinising. The field is added after
ToJson,FromJsonandIsEqualhave run and only to the successful list, so this method's own verdict cannot change. The output tuple is unchanged, content-only inside output 0, because the capture scripts bind to these outputs by position and had to be rebound by hand when it last widened. The field sits immediately before_bhomVersion, whichVersioning_Engineappends last, becauseHelpers.DescriptionFromJsonreads fixed quote-delimited indices and anything inserted earlier would shift every description the versioning test produces. Method records are skipped, their top-level type beingSystem.Reflection.MethodBase; that one rests on the code path rather than the measurement, which contains noMethodBase. A type whose assembly cannot be named is left untouched rather than guessed.