Add the Buildvana.Core.Versioning library

[rdeago]

Phase 4 of #267. Closes #293.

What's new

• Buildvana.Core.Abstractions gains the versioning definitions (namespace Buildvana.Core.Versioning): the VersionSpec(Major, Minor, Prerelease) record with canonical ToString(), and the VersionSpecChange enum.
• New Buildvana.Core.Versioning library (not packaged standalone; no live code wired to it yet):
  • VersionSpec.Parse/TryParse as C# 14 static extension members implementing the MAJOR.MINOR[-[tag]] grammar (tag text informational; trailing whitespace tolerated).
  • GitHeightCalculator — the single LibGit2Sharp git-height calculator, NBGV-parity rules verified against NBGV's source: a commit belongs to the version line when its committed VERSION parses to the same MAJOR.MINOR; each such commit contributes 1 + max(parent heights), so the bump commit has height 1 and merges take the longest path; prerelease-only changes never reset; 0 is reserved for lines with no committed history (uncommitted VERSION or bump, unborn HEAD). LibGit2Sharp/NuGet.Versioning appear in no public signature.
  • VersioningSettings — resolves release.branches, versioning.prereleaseTag, and versioning.assemblyVersionPrecision (default major); branch matching with anchored, compiled, culture-invariant regexes and a match timeout.
  • VersioningService — reads VERSION (fails loudly when absent or malformed, and when a prerelease line has no configured tag), computes the height, and exposes SemVer (never carries Git metadata), SimpleVersion, AssemblyVersion (precision-masked), InformationalVersion (short commit id appended on non-public builds, mirroring today's NpmPackageVersion behavior), plus IsPublicRelease, IsPrerelease, and CommitId.
• Configuration: prereleaseTag relocated from release to versioning (config is unreleased, so no migration concern); schema regenerated.
• Branch patterns are implicitly anchored: both release.branches matching (VersioningSettings) and release.generateDocsFrom matching (bv's ReleaseSettings) wrap each configured pattern in ^(?:…)$, so a pattern must match the whole short branch name — main no longer matches domain or main2. The generateDocsFrom defaults simplify to main/master, and the config descriptions (and regenerated schema) now document whole-name matching.
• Configuration: AssemblyVersionPrecision.Revision removed.
  The value was a no-op: versions are computed as major.minor.height, so there is no revision component and ComputeAssemblyVersion always emits 0 as the fourth part of the assembly version. Configuring "revision" produced output identical to "build", so Build is now the maximum precision. Removed from the enum, the JSON schema, and the corresponding test case. Breaking for config files that used "revision", which is acceptable on the 2.0 preview line.
• Tests: 72 TUnit cases — parse grammar (valid, malformed, overflow, round-trip), branch matching (including implicit anchoring), assembly-version precision, all height rules on temporary LibGit2Sharp repositories (merge longest-path, no-reset on prerelease flips, height 0 for uncommitted lines, detached/unborn HEAD), and the version string forms. bv's docs-branch matching gains anchoring coverage too.

Unchanged

bv behavior: the tool keeps its own versioning types and NBGV; nothing consumes the new library until Phase 7 (#296). The only bv change is the generateDocsFrom anchoring above. No changelog entry — internal-only, per the issue's acceptance criteria.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds the new Buildvana.Core.Versioning library and associated abstractions/config/schema updates as Phase 4 of native versioning work (introducing VERSION parsing, git-height computation, and a settings/service surface), with a dedicated TUnit test suite.

Changes:

• Introduces Buildvana.Core.Versioning (VERSION parsing via C# 14 extension members, GitHeightCalculator, VersioningSettings, VersioningService).
• Adds versioning primitives to Buildvana.Core.Abstractions (VersionSpec, VersionSpecChange) and relocates prereleaseTag config from release → versioning (including schema regen).
• Adds comprehensive TUnit coverage for parsing, settings matching, version string forms, and git-height rules using temporary LibGit2Sharp repos.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/Buildvana.Core.Versioning.Tests/Buildvana.Core.Versioning.Tests.csproj	Adds new test project for the versioning library.
tests/Buildvana.Core.Versioning.Tests/FixedHomeDirectoryProvider.cs	Test helper to provide a fixed repo root.
tests/Buildvana.Core.Versioning.Tests/GitHeightCalculatorTests.cs	Tests git-height computation semantics (unborn HEAD, merges, resets, etc.).
tests/Buildvana.Core.Versioning.Tests/TempGitRepo.cs	Test helper for creating and manipulating temp LibGit2Sharp repos.
tests/Buildvana.Core.Versioning.Tests/VersioningServiceTests.cs	Tests VersioningService version strings, prerelease handling, and precision.
tests/Buildvana.Core.Versioning.Tests/VersioningSettingsTests.cs	Tests branch regex matching behavior and config defaults/errors.
tests/Buildvana.Core.Versioning.Tests/VersionSpecParseTests.cs	Tests VersionSpec.Parse/TryParse grammar and error cases.
tests/Buildvana.Core.Versioning.Tests/VersionSpecTests.cs	Tests VersionSpec canonical ToString() and equality.
src/Buildvana.Core.Versioning/Buildvana.Core.Versioning.csproj	New library project and dependencies.
src/Buildvana.Core.Versioning/GitHeightCalculator.cs	Implements LibGit2Sharp-based height calculation with NBGV-parity semantics.
src/Buildvana.Core.Versioning/GitHeightResult.cs	Public result record for git-height facts (height/branch/commit).
src/Buildvana.Core.Versioning/VersioningService.cs	Reads VERSION, computes height and exposed version strings.
src/Buildvana.Core.Versioning/VersioningSettings.cs	Resolves release.branches + versioning config, regex matching with timeout.
src/Buildvana.Core.Versioning/VersionSpecExtensions.cs	Implements VersionSpec.Parse/TryParse via C# 14 extension block.
src/Buildvana.Core.Versioning/VersionSpecExtensions-private.cs	Holds generated regex backing for parsing.
src/Buildvana.Core.Configuration/ReleaseConfig.cs	Removes release.prereleaseTag.
src/Buildvana.Core.Configuration/VersioningConfig.cs	Adds versioning.prereleaseTag.
src/Buildvana.Core.Abstractions/Versioning/VersionSpec.cs	Adds shared VersionSpec record with canonical ToString().
src/Buildvana.Core.Abstractions/Versioning/VersionSpecChange.cs	Adds VersionSpecChange enum.
schemas/buildvana.schema.json	Moves prereleaseTag schema from release → versioning.
Buildvana.slnx	Adds the new library and test project to the solution.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

[rdeago]

@claude please review this PR.

[claude]

Claude finished @rdeago's task in 5m 25s —— View job

---

Claude finished reviewing this PR

• Gather context (diff, changed files, prior review comments)
• Read new/changed source files
• Review for correctness, style-guide, and architecture compliance
• Post review feedback

I read the full diff against origin/main, traced the GitHeightCalculator walk against each GitHeightCalculatorTests case, and re-checked the two disputes from the earlier Copilot review rounds. Note: I couldn't run dotnet build/dotnet bv test — those commands require interactive approval that wasn't granted in this session, so the analysis below is static (no compiler/test confirmation). If you'd like a build/test pass, re-run with build permissions allowed.

Re: the disputed Copilot comments

Both of your rebuttals hold up:

• VersionSpecExtensions-private.cs partial modifiers — correct, only one part needs to restate static partial; the others can say just partial, and having a single source of truth for the modifiers is the right call.
• TryParse assigning null to [MaybeNullWhen(false)] out VersionSpec result — correct, this is the standard BCL TryXxx pattern (e.g. Dictionary.TryGetValue); the attribute is exactly what makes this null assignment warning-free under NRT, and it's the idiomatic form (not default!).

Findings

1. AssemblyVersionPrecision.Revision doesn't do what its doc comment says (src/Buildvana.Core.Versioning/VersioningService.cs:154-159, src/Buildvana.Core.Configuration/AssemblyVersionPrecision.cs:20-21)

ComputeAssemblyVersion hardcodes the fourth component to 0 unconditionally:

private static string ComputeAssemblyVersion(VersionSpec spec, int height, AssemblyVersionPrecision precision)
{
    var minor = precision >= AssemblyVersionPrecision.Minor ? spec.Minor : 0;
    var build = precision >= AssemblyVersionPrecision.Build ? height : 0;
    return FormattableString.Invariant($"{spec.Major}.{minor}.{build}.0");
}

There's no branch that changes behavior for precision == Revision — it's indistinguishable from Build. Your own test locks this in:

[Arguments(AssemblyVersionPrecision.Build, "2.3.1.0")]
[Arguments(AssemblyVersionPrecision.Revision, "2.3.1.0")]

But the enum's XML doc says otherwise: /// <summary>All four components are significant (<c>major.minor.build.revision</c>).</summary>. Since this is the first real consumer of AssemblyVersionPrecision (it's pre-existing, unreferenced config until this PR), the mismatch was latent until now. Two ways to resolve it, and I don't have enough context on the original intent to pick one:

• If there's genuinely no fourth data axis to carry (which tracks — deterministic builds are usually a reason to avoid an auto-incrementing revision), update the enum doc to say Revision currently behaves identically to Build and is reserved for future use.
• If a future phase does intend to populate a real revision value, it'd be worth a // TODO or issue reference tying Revision to that future work, so it isn't mistaken for "done."

2. No test exercises the BOM-tolerance path in GitHeightCalculator.IsInLine (src/Buildvana.Core.Versioning/GitHeightCalculator.cs:135-141)

var content = ((Blob)entry.Target).GetContentText();
var hasBom = content.Length > 0 && content[0] == 0xFEFF;
if (hasBom)
{
    content = content[1..];
}

This is hand-rolled (unlike the VersioningService.ReadVersionFile BOM handling, which just relies on File.ReadAllText's well-known BOM-stripping behavior — correctly, per your rebuttal to Copilot on that point). Since this branch depends on what LibGit2Sharp.Blob.GetContentText() actually returns for BOM'd blob content — which I could not verify against LibGit2Sharp's source in this session — a regression here (e.g. if GetContentText() already strips the BOM upstream, making hasBom always false, or if it doesn't and the slice is off) wouldn't be caught by the current 65-case suite. Worth a TempGitRepo case that commits a VERSION file with a leading UTF-8 BOM and asserts the height still increments, to pin down actual behavior either way.

Everything else checks out

• VersionSpec grammar (VersionSpecExtensions-private.cs regex) matches all the parse/reject cases in VersionSpecParseTests, including the overflow, leading-zero, and tag-charset edge cases.
• GitHeightCalculator.ComputeHeight's iterative post-order walk correctly implements "1 + max(parent heights)", verified by hand against Calculate_MergeCommit_UsesLongestPath (longest path = 5) and Calculate_PrereleaseOnlyChanges_DoNotResetHeight.
• VersioningSettings branch-matching, timeout, and error paths are covered and correct.
• Architecture/style: namespace placement (Buildvana.Core.Versioning in both .Abstractions and the new library) matches the .Abstractions discipline rule; no LibGit2Sharp/NuGet.Versioning types leak into public signatures, per the stated design goal; csproj ItemGroup ordering matches sibling projects (e.g. Buildvana.Core.Json.csproj); one-type-per-file and partial-class file-splitting conventions are followed.
• Config/schema relocation (release.prereleaseTag → versioning.prereleaseTag) is consistent across ReleaseConfig.cs, VersioningConfig.cs, and the regenerated buildvana.schema.json.

Nothing here is a blocker — both are worth a look before this lands, but neither is a correctness bug in the sense of "wrong output for documented behavior" except the Revision doc mismatch, which is more a documentation/intent gap than a code defect.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/Buildvana.Core.Versioning/VersioningService.cs:158

• ComputeAssemblyVersion currently ignores AssemblyVersionPrecision.Revision (it always emits ...build.0), which makes the Revision setting ineffective and contradicts the enum's intent (all 4 components significant). Consider encoding the height across build+revision for Revision, and validate component ranges so MSBuild doesn't later fail with an invalid assembly version.

    private static string ComputeAssemblyVersion(VersionSpec spec, int height, AssemblyVersionPrecision precision)
    {
        var minor = precision >= AssemblyVersionPrecision.Minor ? spec.Minor : 0;
        var build = precision >= AssemblyVersionPrecision.Build ? height : 0;
        return FormattableString.Invariant($"{spec.Major}.{minor}.{build}.0");

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

[rdeago]

Both findings addressed:

1. AssemblyVersionPrecision.Revision doc mismatch — resolved by removing the value altogether in bc98ee1 (see the note above): there is no fourth data axis in the major.minor.height scheme, so Revision was indistinguishable from Build. Rather than documenting a no-op, Build is now the maximum precision.
2. BOM-tolerance path untested — a4db098 adds Calculate_CommittedVersionFileWithBom_DoesNotStopWalk, which commits a VERSION file with a leading UTF-8 BOM and asserts the height keeps incrementing, pinning the actual Blob.GetContentText() behavior.

@claude please take another look.

[claude]

Claude finished @rdeago's task in 2m 9s —— View job

---

Claude finished re-reviewing @rdeago's changes

• Gather context (diff since previous review, new commits)
• Verify fix Add initial version of the SDK, adapted from internal Tenacom project. #1: AssemblyVersionPrecision.Revision removal (bc98ee1)
• Verify fix Use the SDK to build itself #2: BOM-tolerance test (a4db098)
• Check remaining unresolved Copilot findings from the last round
• Post final review

Both findings from my previous pass check out.

1. AssemblyVersionPrecision.Revision removal (bc98ee1) — resolved cleanly

src/Buildvana.Core.Configuration/AssemblyVersionPrecision.cs now stops at Build, and its doc comments no longer promise a fourth significant component. ComputeAssemblyVersion (VersioningService.cs:154-159) matches: Build is genuinely the max precision now, so there's no longer a doc/code mismatch. Schema (buildvana.schema.json) and the test case list (VersioningServiceTests.cs:118-120) were updated in the same commit, and a repo-wide grep turns up no leftover Revision references in this area (the one unrelated hit is AssemblyVersionRevision in the NuGet pack targets, a different subsystem).

2. BOM-tolerance test (a4db098) — resolved, pins the intended behavior

Calculate_CommittedVersionFileWithBom_DoesNotStopWalk commits "1.0\n" then "1.0\n" and asserts height 2. I traced it against GitHeightCalculator.IsInLine (GitHeightCalculator.cs:121-148): for the height to come out as 2 rather than 1, the second commit's blob content must still parse as 1.0, which is exactly the behavior the manual hasBom/slice logic is there to guarantee. This is a fair pin of the end-to-end behavior regardless of what LibGit2Sharp.Blob.GetContentText() does internally with the BOM.

One minor observation, not a blocker: whether this test actually exercises the hasBom = true branch specifically (vs. GetContentText() already having stripped the BOM upstream, making the manual slice a no-op) still isn't distinguishable from the assertion alone — both scenarios produce height 2. That's fine as a behavioral regression test; it just means the hand-rolled branch's coverage is still indirect. Not worth blocking on.

Other threads from the last Copilot round

For completeness, since this is a re-review of the whole PR state and not just the two items above:

• The ReadVersionFile leading-whitespace concern was addressed in ca9fd3d, but deliberately in the opposite direction from Copilot's suggestion: rather than rejecting leading whitespace, text = text.Trim(); now tolerates it (and the error message uses the trimmed text, fixing the "misleading error" issue too). That's a legitimate, intentional design choice given the commit message's reasoning — flagging only so it's visible that it was a conscious divergence, not an oversight.
• Two items from that round remain open with no code change: the branch-pattern anchoring suggestion (VersioningSettings.IsMatch, VersioningSettings.cs:78-92, still matches unanchored patterns verbatim) and the Parse XML-doc gap (VersionSpecExtensions.cs:24, still only documents BuildFailedException even though Guard.IsNotNull(str) throws ArgumentNullException for a null input). Neither was part of what this re-review round asked me to check, so I'm not treating them as blockers — just noting they're still there in case they were meant to be picked up too.

Nothing new to flag beyond that. As before, I wasn't able to run dotnet build/dotnet bv test in this session (no build permissions granted), so this remains a static review — the analysis above is based on reading the diffs and tracing the logic by hand, not a compiler/test run.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/Buildvana.Core.Versioning/VersioningService.cs:137

• ReadVersionFile currently uses Trim(), which also removes leading whitespace. That makes the version file accept inputs like " 1.0" even though the parsing grammar (and VersionSpec.TryParse tests) only tolerate trailing whitespace. Using TrimEnd() keeps the intended rule: trailing whitespace/newlines are OK, but leading whitespace still fails loudly.

        text = text.Trim();
        BuildFailedException.ThrowIfNot(
            VersionSpec.TryParse(text, out var spec),
            $"{path} contains an invalid version specification '{text}'.");

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/Buildvana.Core.Versioning/VersionSpecExtensions.cs:36

• VersionSpec.Parse trims the input in the error message (str.Trim()), which can misreport the actual invalid input. For example, an invalid leading-space input like " 1.0" will throw an exception claiming "1.0" is invalid (even though that trimmed value is valid). Consider trimming only trailing whitespace (or not trimming at all) when formatting the message.

            Guard.IsNotNull(str);
            BuildFailedException.ThrowIfNot(
                VersionSpec.TryParse(str, out var result),
                $"Invalid version specification '{str.Trim()}'.");

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/Buildvana.Core.Versioning/VersionSpecExtensions.cs:36

• In VersionSpec.Parse, the exception message trims both leading and trailing whitespace via str.Trim(). This can hide the actual reason parsing failed (e.g., the invalid input " 1.0" would be reported as "1.0", which looks valid and makes debugging harder). Consider trimming only trailing whitespace (newlines) or leaving the input unmodified in the message.

            BuildFailedException.ThrowIfNot(
                VersionSpec.TryParse(str, out var result),
                $"Invalid version specification '{str.Trim()}'.");

[rdeago]

@claude please review this PR again.

[claude]

Claude finished @rdeago's task in 1m 4s —— View job

---

Claude finished re-reviewing this PR

• Gather context (diff since last review, new commits)
• Review new commits since last review pass (0b58c58, 26dd4ca, 5c11263, 874adee)
• Check the two previously-open items are actually resolved (branch anchoring, Parse XML doc)
• Re-check overall PR state for anything missed
• Post final review

Four commits landed since my last pass, and they clean up everything that was still open.

1. Branch-pattern anchoring — resolved (26dd4ca, 5c11263)

VersioningSettings.IsMatch (src/Buildvana.Core.Versioning/VersioningSettings.cs:82-100) and ReleaseSettings.IsMatch (src/Buildvana.Tool/Subcommands/ReleaseSettings.cs:179-193) both now wrap the configured pattern as $"^(?:{pattern})$" before matching, while error messages still quote the pattern as configured (unanchored) — exactly right, since the anchoring is an implementation detail the user shouldn't need to account for when reading their own error. generateDocsFrom's defaults simplified from ["^main$", "^master$"] to ["main", "master"] now that anchoring is implicit, and both <remarks> doc additions plus the regenerated schema descriptions correctly describe the new whole-name-match semantics. New tests (IsPublicReleaseBranch_ImplicitlyAnchorsPatterns, MatchesDocsBranch_ImplicitlyAnchorsPatterns) cover the main vs domain/main2 distinction and an alternation/regex-anchor-free case (release/\d+\.\d+) — good coverage of the actual bug class being fixed.

2. Parse ArgumentNullException doc gap — resolved (874adee)

VersionSpecExtensions.cs:25 now documents <exception cref="ArgumentNullException"> alongside the existing BuildFailedException, matching what Guard.IsNotNull(str) actually throws. Matches the standard pattern used elsewhere in the codebase.

3. BOM test hardening (0b58c58) — a genuine improvement, not just cleanup

This also resolves the one loose end from my previous pass, where I noted the BOM test couldn't distinguish "the manual hasBom/slice branch fired" from "GetContentText() already stripped the BOM upstream, making the slice a no-op." TempGitRepo.WriteFile now takes an Encoding and the test uses new UTF8Encoding(encoderShouldEmitUTF8Identifier: true) instead of embedding a escape in the C# string content. This is a meaningfully different (and stronger) test: `File.WriteAllText(..., encoding)` with that encoding writes the actual 3-byte UTF-8 BOM sequence to disk, so the test now exercises real BOM bytes round-tripping through LibGit2Sharp's blob storage, rather than relying on how `File.WriteAllText` happens to encode a leading char with the default encoding. Good catch turning an incidental follow-up into a real coverage improvement.

Everything else

• GitHeightCalculator.cs itself is unchanged since my last pass — the BOM-tolerance logic at lines 135-141 is exactly as previously reviewed and traced against the test suite.
• CI is green on the current tip (874adee): build/test/pack succeeded, CodeQL clean.
• Diff against origin/main for the schema/config relocation (prereleaseTag → versioning, Revision removal) matches the PR description with no surprises.

No new findings. All previously open items are closed out, and the one addition beyond bugfixes (the BOM test rewrite) is a real improvement rather than scope creep. This looks ready to merge from my side.