Skip to content

fix(ci-compliance): exclude Versioning_<version> upgrade maps from dataset pathspec - #21

Open
sakanni wants to merge 1 commit into
developfrom
fix/dataset-pathspec-versioning-upgrade-maps
Open

fix(ci-compliance): exclude Versioning_<version> upgrade maps from dataset pathspec#21
sakanni wants to merge 1 commit into
developfrom
fix/dataset-pathspec-versioning-upgrade-maps

Conversation

@sakanni

@sakanni sakanni commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

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>.json wherever it appears.

What collides

ci-dataset-compliance asserts that every file it selects deserialises into a BH.oM.Data.Library.Dataset. Versioning upgrade maps hold {"Dataset":{"ToNew":..,"ToOld":..}} and are read by BHoM_Engine Versioning_Engine/Query/DatasetToNewPaths.cs from %ProgramData%\BHoM\Upgrades. They carry no _t and can never be Dataset documents.

They are named Versioning_<version-without-the-dot>.json and 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 a datasets substring somewhere in the path. The exception is a repository whose project folder supplies that substring, which is the *_Datasets repositories: BuroHappold_Datasets and BHoM_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_*.json by 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 no datasets in 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

:(icase)*datasets*.json
  :(exclude,icase)*Versioning_Test/Datasets/*
  :(exclude,glob,icase)**/Versioning_[0-9]*.json

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 .json under any directory containing Versioning_, including the Versioning_Test sets and any real dataset under a Versioning_Engine folder. The trailing one drops the entire contents of a directory named Versioning_<digits>.
  • The ** 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.
  • The [0-9] gate. Without it the token drops a genuine dataset named Versioning_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, patterns is 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 are Versioning_33.json and 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.json and Adapters.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 and CFD_Toolkit's 2 are unchanged.

Tests

83 to 92 assertions, all passing.

Two assertions flipped from assert_matches to assert_not_matches, on BuroHappold_Datasets/Versioning_93.json and BHoM_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 a Versioning_Engine project folder (Versioning_Engine is a real project in BHoM_Engine with no dataset folder yet, so that path is one commit away from existing), a genuine dataset named Versioning_Rules.json, and one upgrade map the base selector never reaches.

Control runs, each reverting one element:

variant assertions that fail
token removed (#20 state) the 3 upgrade-map assertions
*Versioning_*.json the 3 must-stay-selected assertions
digit-gated, non-glob the Versioning_93 directory case, and the invariant
glob + digits, single */ the depth-2 assertion
glob + **, no digit gate Datasets/Versioning_Rules.json

Worth 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 without WM_PATHNAME, where ** is not special and **/ degrades to * plus a mandatory /. :(glob) switches WM_PATHNAME on, and there **/ matches zero or more leading components. Measured:

:(glob,icase)**/Versioning_[0-9]*.json   selects root, depth-1 and depth-3
:(icase)**/Versioning_[0-9]*.json        misses the root file

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 *_Datasets repositories' own copies, and having ci-compliance append 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 literal patterns: 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.

…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
sakanni force-pushed the fix/dataset-pathspec-versioning-upgrade-maps branch from d54d61a to 84197a5 Compare September 9, 2026 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant