fix(ci-versioning): do not gate on inferred attribution - #16
Merged
sakanni merged 3 commits intoSep 4, 2026
Conversation
AttributeToSubject falls back to a namespace prefix when the dataset record names no declaring assembly. Findings on that path were counted towards a failing verdict while the run simultaneously warned that they "may not be its own". Recording the uncertainty and then discarding it is the defect. Route fallback-attributed findings to the existing unverified bucket instead. DeriveStatus yields Warning and ExitCodeFor yields 0, so they stay in the log and the artefact and no longer fail the check. Findings carrying a declaring assembly are unaffected and still gate. Precision is not the fix and should not be attempted: BH.oM.Structure.Elements is declared by both BHoM/BHoM and StructuralEngineering_Toolkit, so no string rule can say which of two repos sharing a namespace owns a type. Measured on BHoM/BHoM (run 33849699768, commit d6db0a1): 145 of 145 findings arrived on the fallback path, 0 by declaring assembly, and all 145 proved to be another repository's or an artefact of an incomplete closure. Re-running the same subject against a closure with 186 assemblies loaded instead of 75 cut the count to 98 and left the basis at 100% inferred, so closure quality changes the volume and never the basis. Verified against that closure: Status Warning, Failures 0, exit 0, all 98 diagnostics retained with CountedAsReal false. Also print the attribution basis whenever there are diagnostics rather than only when a count is non-zero. The comment already said a number printed only when non-zero cannot be distinguished from one nobody measured; the guard below it produced exactly that. A sweep of all 62 repos carrying ci-beta.yml could not tell a repo with no fallback attributions from one that had never run. Refs BHoM/internal-tickets#31
…ean one The routing in the previous commit stops inferred attribution failing a build. It also made the job summary read "Versioning passed" over 98 findings that could not be attributed, because $failed derives from exit_code and Warning and Pass are both exit 0 by design. That is the vacuous green this summary exists to prevent, reintroduced one level up. Read the runner's own Status, which the summary already parses out of versioning-result.json, and branch the heading three ways. Warning now reads "Versioning inconclusive: N finding(s) reported, none verified". The findings table's empty branch no longer says only "no regressions", which is true and misleading on its own when nothing was checked that could have produced one. The check conclusion stays success. A GHA job cannot emit neutral, so the summary heading and the coverage table are the only places the difference can show; the two aggregate warning annotations remain the other signal. Verified against the real artefacts: Status Warning with 98 unverified renders the inconclusive heading, Status Error with 145 real renders the failure heading, Status Pass renders the pass heading. No Pester assertions covered the heading, so none needed updating. Refs BHoM/internal-tickets#31
A missing subject assembly list is a precondition failure, not a finding, and the "do not gate on inferred attribution" policy must not reach it. It does not today. ReadSubjectAssemblyList returns null when the list is absent, attribution falls back to the whole loaded closure, and that path records AttributionBasis.NotRecorded while the policy keys on NamespaceFallback, so the over-reporting run still fails. But that safety is incidental rather than designed: both paths match a description against a set of namespaces, so unifying them looks like a harmless refactor and would silently turn a whole-closure red into a green tick. Two cases pin it. Verified by mutation: widening the policy to attributedBy != DeclaringAssembly fails WholeClosureFindingWithNoMethodEvent_StillCountsAsRealAndStillGates, so the test is not vacuous. Prompted by BHoM/Versioning_Toolkit PR #348, run 33619426914, where the action's own guard caught the missing list and exited 1 before the runner started. This is defence in depth behind that guard, not a replacement for it. No production code changed. Refs BHoM/internal-tickets#31
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
1 participant
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.
What
A finding whose ownership was inferred from a namespace prefix no longer fails the check. It is reported as unverified instead: still in the log, still in the artefact,
CountedAsReal: false, exit 0.Findings carrying a declaring assembly are untouched and still gate.
Why
AttributeToSubjectreads the declaring assembly the dataset record names, and falls back to a namespace prefix when there is none. Object records never carry one, becauseFromJsonDatasetsbuilds a failure'sInformationfromCurrentEvents()and only method entries produce aMethod ... from ...event. So for every object finding the basis is a guess.The runner already knew this. It recorded
AttributedBy: NamespaceFallbackper row and warned in plain English that "these specific findings may not be its own", then counted them towards a failing verdict anyway. Recording the uncertainty and discarding it is the defect, not the fallback, which is a reasonable response to having no better evidence.Precision is not the fix and should not be attempted.
BH.oM.Structure.Elementsis declared by bothBHoM/BHoMandBuroHappoldEngineering/StructuralEngineering_Toolkit. No string rule can say which of two repositories sharing a namespace owns a type.Evidence
BHoM/BHoMrun33849699768, commitd6db0a1, a PR adding one file and removing nothing:StructuralEngineering_Toolkit, 47BHoM/BHoM, 9Clustering_Toolkit, 6 the two IES toolkits, 2 unplacedversioningcheck passed the same commit, running the sameVerify.FromJsonDatasetsover the same single dataset version, differing only in thatCloneInstallerbuilds the whole installer setRestricting the 9.2 dataset to the 963 object records the prefix filter would attribute here, and crossing "references a type outside the closure" against "produced a finding":
Zero of the 804 cleanly-resolvable records failed. All 145 findings carry a foreign reference.
Re-running the current runner against a fuller closure, same subject set of 30, same dataset version:
47 cleared, none appeared, and the basis stayed at 100% inferred. Of the 98 survivors, the only 4 owned by
BHoM/BHoMareBH.oM.Data.Requests.{Batch,LogicalAnd,LogicalNot,LogicalOr}Request, and every one embedsBH.oM.Adapters.Sharepoint.Requests.*, absent from that closure too. So closure quality changes the volume and never the basis. Closure work is still worth doing, and is tracked separately; it does not address gating.Verification
Against the 186-assembly closure with this branch built and run:
All 98 diagnostics retained,
CountedAsReal: falseon every one, coverage unchanged. Suite: 123 passed, 0 failed.action.ymlneeds no change: it branches only onexit_code != '0', andWarningwas already exit 0.Second change in here
The attribution basis line is now printed whenever there are diagnostics, not only when a count is non-zero. Its own comment said a number printed only when non-zero cannot be distinguished from one nobody measured, and the guard below it produced exactly that state. Establishing that
BHoM/BHoMis the only affected repository needed a log sweep of all 62 repositories carryingci-beta.yml, where a repository with no fallback attributions was indistinguishable from one that had never run the check. Of those 62, only 11 have ever produced a realci-versioningrun.What this does not do
Coverage on
BHoM/BHoMis effectively zero for object records after this, because 100% of its findings are on the fallback path. That is a truthful zero replacing a false red, but it is still zero, and it must not be the state of things whenversioningbecomes this system's to emit at BHoMBot sunset. Tracked as a sunset blocker.Refs BHoM/internal-tickets#31