fix(ci-compliance): exclude Versioning_<version> upgrade maps from dataset pathspec - #21
Open
sakanni wants to merge 1 commit into
Open
fix(ci-compliance): exclude Versioning_<version> upgrade maps from dataset pathspec#21sakanni wants to merge 1 commit into
sakanni wants to merge 1 commit into
Conversation
…taset pathspec Follow-up to #20. That exclusion was path-based and covered one directory layout. Versioning_<digits>.json are versioning upgrade maps holding {"Dataset":{"ToNew":..,"ToOld":..}}, read by BHoM_Engine Versioning_Engine/Query/DatasetToNewPaths.cs. They carry no _t, can never be BH.oM.Data.Library.Dataset documents, and sit at a project root in any repo, so a path-scoped rule cannot reach them. Add ':(exclude,glob,icase)**/Versioning_[0-9]*.json' to both tier-bundle templates. Each element is required and is pinned by an assertion: :(glob) keeps the token a filename match rather than a path-substring match, the ** anchor keeps it depth-agnostic, and the [0-9] gate keeps a genuine dataset named Versioning_Rules.json in scope. Flip the two characterisation assertions #20 left as assert_matches, which its own comment recorded as the intended outcome of a widening. Add five fixtures covering the depth-2 case, a real dataset inside a Versioning_<digits> directory, a real dataset under a Versioning_Engine project, and one path that the base selector never reaches. Refine the structural invariant to permit ** only under :(glob). Defect 3 is a property of git's default matcher, where '**' is not special; :(glob) switches on WM_PATHNAME and there '**/' matches zero or more leading components. The invariant previously rejected the only correct way to write a basename match. It still fails ** without :(glob). Suite 83 to 92 assertions. Fleet measurement over 309 repositories on their default branches: no repository loses a file, because no upgrade map currently sits in a selected path on a default branch. Refs BHoM/internal-tickets#36, BHoM/internal-tickets#43.
sakanni
force-pushed
the
fix/dataset-pathspec-versioning-upgrade-maps
branch
from
September 9, 2026 09:41
d54d61a to
84197a5
Compare
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.
Follow-up to #20. That exclusion was path-based and covered one directory layout; this broadens it to a filename-anchored rule covering
Versioning_<digits>.jsonwherever it appears.What collides
ci-dataset-complianceasserts that every file it selects deserialises into aBH.oM.Data.Library.Dataset. Versioning upgrade maps hold{"Dataset":{"ToNew":..,"ToOld":..}}and are read byBHoM_EngineVersioning_Engine/Query/DatasetToNewPaths.csfrom%ProgramData%\BHoM\Upgrades. They carry no_tand can never be Dataset documents.They are named
Versioning_<version-without-the-dot>.jsonand sit at a project root next to the.csproj, so the filename is the only signal and #20's path rule could not reach them. Swept over 309 repositories on their default branches: 15 such files in 11 repositories. None is selected today, because the base selector needs adatasetssubstring somewhere in the path. The exception is a repository whose project folder supplies that substring, which is the*_Datasetsrepositories:BuroHappold_DatasetsandBHoM_Datasets. One of those is failing on an open pull request now, and #20 left that case deliberately uncovered.An earlier review suggested matching
Versioning_*.jsonby name in any repository. That is the right level of generality and this pull request adopts it, with two corrections established by measurement. It also cited an upgrade map in a repository with nodatasetsin the path as evidence the collision was fleet-wide; measured, that file is not selected at all, and it is now a fixture asserting exactly that so the example is recorded as inert rather than refiled later.The change
Three elements, each required, each pinned by an assertion that fails if it is dropped:
:(glob). Without it both wildcards cross/and the token stops being a filename match. The leading one drops every.jsonunder any directory containingVersioning_, including the Versioning_Test sets and any real dataset under aVersioning_Enginefolder. The trailing one drops the entire contents of a directory namedVersioning_<digits>.**anchor. A single*/under:(glob)matches exactly one leading component and passed every other assertion in the suite, so a depth-2 fixture pins it.[0-9]gate. Without it the token drops a genuine dataset namedVersioning_Rules.json. Gating on the current BHoM version instead does not work: nothing in this repository's workflows, actions or scripts exposes a BHoM version,patternsis a static input consumed before any build, and repositories keep the map for whatever version they last needed one at, so six of the 15 areVersioning_33.jsonand would go unmatched.The two exclusions stay separate tokens because they cover different shapes. The Versioning_Test sets are JSON Lines of oM objects whose filenames are
Objects.json,Methods.jsonandAdapters.json; only a path rule reaches those.Loss check
Measured over the same 309 repositories, whole trees on default branches: 1397 files selected before, 1317 after, and the new token removes nothing beyond #20. No upgrade map currently sits in a selected path on any default branch, so the token is preventative plus it fixes the open pull request that motivated it.
BHoM_Engine's 208,Test_Toolkit's 65,StructuralEngineering_Toolkit's 27 andCFD_Toolkit's 2 are unchanged.Tests
83 to 92 assertions, all passing.
Two assertions flipped from
assert_matchestoassert_not_matches, onBuroHappold_Datasets/Versioning_93.jsonandBHoM_Datasets/Versioning_93.json. #20 recorded these as characterisation rather than approval and stated that flipping them is the intended outcome of a widening, not a regression.Five fixtures added: the depth-2 case, a real dataset inside a directory named
Versioning_93, a real dataset under aVersioning_Engineproject folder (Versioning_Engineis a real project inBHoM_Enginewith no dataset folder yet, so that path is one commit away from existing), a genuine dataset namedVersioning_Rules.json, and one upgrade map the base selector never reaches.Control runs, each reverting one element:
*Versioning_*.jsonVersioning_93directory case, and the invariant*/**, no digit gateDatasets/Versioning_Rules.jsonWorth extra review attention: the structural invariant
This loosens an existing guard rail, so it is called out rather than buried.
The invariant rejected any token containing
**. That premise holds only for git's default matcher, wildmatch withoutWM_PATHNAME, where**is not special and**/degrades to*plus a mandatory/.:(glob)switchesWM_PATHNAMEon, and there**/matches zero or more leading components. Measured:So the invariant as written forbade the only correct way to express a basename match in a git pathspec. It now keys off the token's magic:
**passes under:(glob)and still fails without it, which control run 3 above confirms.Scope
Per BHoM/internal-tickets#39 nothing propagates a template change to onboarded repositories, and 59 of 61 currently carry the unfixed pathspec inline. This fixes no repository. It stops the old pathspec reaching future onboardings, and the per-repository edit that takes effect in Versioning_Toolkit is a separate pull request there.
Deliberately not in this pull request: the
*_Datasetsrepositories' own copies, and havingci-complianceappend the dataset exclusions itself so a fix reaches all 61 repositories without a distribution wave. The latter is the only route that clears the live failure without a per-repository edit, and it needs its own review because it makes the effective pathspec differ from the literalpatterns:value in the YAML.The base selector stays as-is; re-anchoring it is BHoM/internal-tickets#43.
Refs BHoM/internal-tickets#36, BHoM/internal-tickets#43.