Version ordering is inconsistent across provider formats and application flows

[bobtista]

Problem

GenHub compares content versions in multiple update, download, profile, and reconciliation paths. These paths can apply different rules to the same provider version.

Providers also use different formats:

• Community Outpost uses ISO dates.
• Generals Online uses MMDDYY_QFE# with optional build tags.
• TheSuperHackers uses numeric, date-stamp, and prefixed versions.

A concrete Generals Online failure occurs across year boundaries:

• Installed: 121525_QFE1 — December 15, 2025
• Available: 060526_QFE1 — June 5, 2026

June 2026 is newer, but integer or raw-string ordering can treat it as older. Build tags can also incorrectly affect release precedence, and separate call sites can disagree about the newest installed version.

Expected behavior

• Each provider declares the version scheme used for ordering.
• Update, download, profile, and reconciliation flows use one shared provider-aware comparison path.
• Generals Online releases order chronologically by year, month, day, then QFE.
• Build tags do not affect release precedence.
• Malformed versions are handled deterministically and culture-invariantly.
• Persistent manifest identity remains separate from release ordering.

Acceptance criteria

• Provider definitions select the applicable numeric, ISO-date, or MMDDYY-QFE scheme.
• Relevant application flows use the shared comparer.
• Cross-year Generals Online versions and QFE revisions order correctly.
• Equivalent releases with different build tags have equal precedence.
• Valid versions sort consistently relative to malformed versions.
• The newest installed version is selected when multiple manifests exist.
• Existing manifest-ID encoding is preserved without migration.
• Regression tests cover supported formats and edge cases.

[coderabbitai]

🔗 Related PRs

#297 - fix: use provider-declared schemes for version ordering [open]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[coderabbitai]

Coding Plan

Summary

• Introduce one static, culture-invariant MMDDYY_QFE#[_TAG] comparer in GenHub.Core/Helpers that orders by year→month→day→QFE, ignores build tags, and sorts malformed versions below valid ones.
• Replace the duplicated/ad-hoc comparison logic in the update service (and newest-installed selection) and align the Downloads UI and any newest-of-many manifest selection to that single comparer.
• Leave the manifest-ID numeric encoding unchanged for existing versions (only stripping build tags so tagged versions still encode), preserving addressability and avoiding migration.
• Add focused regression tests covering cross-year ordering, numeric QFE, tag equivalence, malformed handling, and culture-invariance.

---

File-Level Change Summary & Follow-up Notes

• New: GenHub/GenHub.Core/Helpers/GeneralsOnlineVersionComparer.cs — canonical parser + comparison methods (Phase 1).
• New: GenHub/GenHub.Tests/GenHub.Tests.Core/.../GeneralsOnlineVersionComparerTests.cs — regression tests (Phase 1).
• Modified: GeneralsOnlineUpdateService.cs — remove private ParseVersion/IsNewerVersion, call shared comparer, pick newest installed manifest (Phase 2).
• Modified: DownloadsViewModel.cs / PublisherCardViewModel.cs — select latest GO version/variant via shared comparer (Phase 2).
• Reviewed/Modified as needed: GeneralsOnlineProvider.cs and any newest-of-many GO manifest selection — delegate to shared comparer (Phase 2).
• Modified: GeneralsOnlineManifestFactory.cs (ParseVersionForManifestId) — strip trailing build tags while preserving the existing encoding formula (Phase 3).
• Follow-up prompt for implementing agent: Confirm no other call site orders releases by parsing the manifest-ID numeric segment; if any does, redirect it to the shared comparer. Verify GeneralsOnlineDiscoverer.ParseVersionDate still only populates display metadata and does not need to feed ordering (leave its date extraction intact, but consider reusing the shared parser for consistency if it does not alter behavior).

Design Choices

Design Choice 1: Should the shared version-ordering logic be a static helper or an injectable service?

Options Considered:

1. Static utility class in GenHub.Core/Helpers/ (e.g. GeneralsOnlineVersionComparer), called directly and unit-tested in isolation with no DI/Moq.
2. Injectable interface + implementation registered in ContentPipelineModule, constructor-injected into consumers.

Chosen Option: 1

Rationale: Option 1 matches the established VersionHelper precedent, keeps the parser pure and trivially testable, and requires no DI wiring or interface surface changes. The codebase strongly prefers static parsing/formatting helpers (VersionHelper, ManifestIdGenerator, GitHubInferenceHelper) with no DI, while update services are DI-registered singletons.

Design Choice 2: How should the manifest-ID year-boundary defect be reconciled with the "preserve encoding" requirement?

Options Considered:

1. Leave the manifest-ID numeric encoding formula unchanged for existing (tag-less) versions and never use it for ordering; do all ordering via the new comparer.
2. Change the manifest-ID encoding to be chronologically monotonic (requires manifest migration for existing installs).

Chosen Option: 1

Rationale: Option 1 satisfies "preserve existing manifest-ID encoding / no migration" while fixing ordering correctness in the paths that actually rank releases. The buggy (dateValue*10)+qfeValue encoding lives in the manifest-ID path, but exploration confirms this numeric segment is never used for OrderBy anywhere — ordering happens elsewhere — and the IDs are opaque keys.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Introduce a single, authoritative Generals Online version-ordering utility

This phase creates one canonical parser/comparer that fully models the MMDDYY_QFE#[_TAG] format, orders chronologically (year → month → day → QFE), ignores trailing build tags for precedence, parses culture-invariantly, and defines deterministic behavior for malformed input. It is the shared foundation every other path will consume, and it is validated by dedicated regression tests before any callers are migrated.

Task 1: Create the shared version comparer/parser

Provide the one place that understands and orders Generals Online versions.

• Add a static utility (e.g. GeneralsOnlineVersionComparer) under GenHub/GenHub.Core/Helpers/, following the existing VersionHelper static-helper convention.
• Expose a TryParse that decomposes a version string into a structured value (year, month, day, QFE integer), splitting on the _QFE separator, decoding MMDDYY into explicit month/day/year components (not a raw integer), and stripping/ignoring any trailing _TAG suffix (e.g. _EAC) so tags never affect precedence.
• Expose comparison entry points (e.g. Compare(string a, string b) and a convenience IsNewer(string latest, string? current)) that order by year, then month, then day, then QFE numerically.
• Parse all numeric components using CultureInfo.InvariantCulture/invariant NumberStyles to guarantee culture-invariance.
• Define deterministic malformed-input handling: unparseable versions sort before valid ones (valid versions rank higher), null/empty treated as "no version" (any valid version is newer), and equivalent releases differing only by build tag compare as equal.
• Reference GeneralsOnlineConstants for the _QFE separator; do not introduce new comparison behavior into the manifest-ID path.

Task 2: Add regression tests for the comparer

Lock in the acceptance criteria before wiring consumers.

• Add a test class under GenHub/GenHub.Tests/GenHub.Tests.Core/ (mirroring the source location, following the CommunityOutpost test conventions: [Theory]/[InlineData], Arrange/Act/Assert, XML doc comments, no Moq for a pure static helper).
• Cover: cross-year ordering (121525_QFE1 older than 060526_QFE1), numeric QFE comparison (_QFE2 > _QFE10 handled numerically, not lexically), build-tag equivalence (060526_QFE1 == 060526_QFE1_EAC), malformed-vs-valid ordering, null/empty handling, and culture-invariance (run parsing under a non-invariant culture).

🤖 Prompt for AI agents

Implement a single, authoritative static comparer/parser for Generals Online
version strings and validate it with regression tests before any consumer is
migrated.

- Add a static utility class (e.g. `GeneralsOnlineVersionComparer`) under
`GenHub/GenHub.Core/Helpers/`, following the existing `VersionHelper`
static-helper convention (no DI).
- Implement `TryParse` to decompose a version string of the shape
`MMDDYY_QFE#[_TAG]` into a structured value containing explicit year, month,
day, and QFE integer components: split on the `_QFE` separator (reference
`GeneralsOnlineConstants` for this separator), decode `MMDDYY` into explicit
month/day/year (never as a raw integer), and strip/ignore any trailing `_TAG`
suffix (e.g. `_EAC`) so tags never affect precedence.
- Implement comparison entry points: a `Compare(string a, string b)` and a
convenience `IsNewer(string latest, string? current)`, ordering by year, then
month, then day, then QFE numerically.
- Use `CultureInfo.InvariantCulture` / invariant `NumberStyles` for all numeric
parsing to guarantee culture-invariance.
- Define deterministic malformed-input semantics: unparseable versions sort
before valid ones; null/empty is treated as "no version" (any valid version is
newer than it); releases equal except for build tag compare as equal.
- Do not let this utility introduce any new comparison behavior into the
manifest-ID encoding path — it is purely for ordering.
- Add a test class under `GenHub/GenHub.Tests/GenHub.Tests.Core/` mirroring the
source location, using `[Theory]`/`[InlineData]`, Arrange/Act/Assert structure,
XML doc comments, and no Moq (pure static helper). Cover: cross-year ordering
(e.g. `121525_QFE1` older than `060526_QFE1`), numeric QFE comparison (`_QFE2` >
`_QFE10` handled numerically not lexically), build-tag equivalence
(`060526_QFE1` == `060526_QFE1_EAC`), malformed-vs-valid ordering, null/empty
handling, and culture-invariance (parsing under a non-invariant culture).

Phase 2: Route all ordering paths through the shared comparer

This phase replaces the scattered, inconsistent comparison logic with calls into the Phase 1 utility so update checks, installed-version selection, and the Downloads UI all agree on "which version is newest." It also ensures the correct newest manifest is chosen when multiple installed Generals Online manifests exist.

Task 1: Migrate the update service to the shared comparer and fix newest-installed selection

Make update-availability decisions consistent and correct.

• Update GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineUpdateService.cs to delete the private ParseVersion/IsNewerVersion methods and call the shared comparer instead (feeding results into the unchanged ContentUpdateCheckResult factory methods — no interface or result-shape changes).
• Change GetInstalledVersionAsync from FirstOrDefault to selecting the newest installed Generals Online manifest by ordering the matching manifests with the shared comparer, satisfying "select the newest installed version when multiple manifests exist."

Task 2: Make download version labels/selection provider-aware

Ensure the Downloads UI reflects the truly-latest Generals Online version.

• Update the Generals Online population path in GenHub/GenHub/Features/Downloads/ViewModels/DownloadsViewModel.cs (and any variant-selection in PublisherCardViewModel.cs) so that when multiple releases/variants are present, the "latest" label/selection is chosen via the shared comparer rather than FirstOrDefault or alphabetical/LastUpdated heuristics.
• Keep the change scoped to Generals Online version selection; do not introduce new badge UI beyond aligning existing version-label selection with the shared ordering.

Task 3: Align installed-manifest/reconciliation selection with the shared ordering

Guarantee any "pick the current/newest Generals Online manifest" step uses the same rule.

• Audit Generals Online installed-manifest selection used by reconciliation-adjacent logic (e.g. GeneralsOnlineProvider's already-installed lookup and any place that picks a "current" GO manifest) and ensure that where a newest-of-many decision is made, it delegates to the shared comparer; exact-match lookups by version string remain unchanged.

🤖 Prompt for AI agents

Replace the scattered, inconsistent version-comparison logic across update
checks, installed-version selection, and Downloads UI with calls to the Phase 1
shared comparer so all paths agree on which Generals Online version is newest.

- In
`GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineUpdateService.cs`:
delete the private `ParseVersion` and `IsNewerVersion` methods and call the
shared comparer instead; feed its results into the existing (unchanged)
`ContentUpdateCheckResult` factory methods — do not change the interface or
result shape.
- In the same service, change `GetInstalledVersionAsync` from using
`FirstOrDefault` to selecting the *newest* installed Generals Online manifest by
ordering the matching manifests with the shared comparer, so the correct newest
manifest is chosen when multiple exist.
- In `GenHub/GenHub/Features/Downloads/ViewModels/DownloadsViewModel.cs` (and
any variant-selection logic in `PublisherCardViewModel.cs`), update the Generals
Online population path so that when multiple releases/variants are present, the
"latest" label/selection is chosen via the shared comparer rather than
`FirstOrDefault` or alphabetical/`LastUpdated` heuristics. Keep this change
scoped strictly to Generals Online version selection — do not add new badge UI
beyond aligning the existing version-label selection with the shared ordering.
- Audit Generals Online installed-manifest selection used by
reconciliation-adjacent logic (e.g. `GeneralsOnlineProvider`'s already-installed
lookup and any other place that picks a "current" GO manifest). Where a
newest-of-many decision is made, delegate it to the shared comparer; leave
exact-match lookups by version string unchanged.

Phase 3: Preserve manifest-ID encoding while tolerating build tags

This phase guarantees the manifest-ID numeric encoding stays byte-for-byte identical for existing (tag-less) versions so installed content remains addressable and no migration is required, while making ID generation robust to newly-introduced build tags.

Harden manifest-ID version parsing against build tags without changing existing encoding

Keep IDs stable and addressable.

• Update GeneralsOnlineManifestFactory.ParseVersionForManifestId in GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineManifestFactory.cs so that a trailing _TAG (e.g. _EAC) is stripped before extracting the QFE number, ensuring tagged versions still produce a valid non-negative integer instead of falling back to 0.
• Explicitly preserve the current (dateValue * 10) + qfeValue formula for the existing tag-less MMDDYY_QFE# shape so previously-generated manifest IDs are reproduced identically (no migration).
• Do not reuse this method for ordering decisions; it remains ID-encoding-only.

🤖 Prompt for AI agents

Harden the manifest-ID version parsing so it tolerates build tags while keeping
the numeric encoding byte-for-byte identical for existing versions, preserving
addressability of already-installed content with no migration required.

- In
`GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineManifestFactory.cs`,
update `ParseVersionForManifestId` so a trailing `_TAG` (e.g. `_EAC`) is
stripped before extracting the QFE number, ensuring tagged versions still
produce a valid non-negative integer instead of falling back to `0`.
- Explicitly preserve the current `(dateValue * 10) + qfeValue` formula for the
existing tag-less `MMDDYY_QFE#` shape so previously-generated manifest IDs are
reproduced identically — no migration of existing manifest IDs.
- Do not reuse this method for any ordering decisions; it must remain strictly
ID-encoding-only, separate from the Phase 1 comparer.

Research

Generals Online support lives under GenHub/GenHub/Features/Content/Services/GeneralsOnline/ with models/constants in GenHub/GenHub.Core/Models/GeneralsOnline/ and GenHub/GenHub.Core/Constants/GeneralsOnlineConstants.cs.

Version parsing is currently duplicated in three ad-hoc, incompatible places:

• GeneralsOnlineDiscoverer.ParseVersionDate (correct date, drops QFE/tag)
• GeneralsOnlineManifestFactory.ParseVersionForManifestId (treats the MMDDYY date as a raw integer — the non-monotonic-across-years defect — and encodes it into the manifest ID)
• GeneralsOnlineUpdateService.ParseVersion/IsNewerVersion (structured date+QFE compare, but no tag handling and no culture-invariance)

No shared comparer, IComparable, or reusable ordering utility exists. GenHub.Core/Helpers/VersionHelper.cs is a static digit-extraction helper (currently UI-only) and is the natural precedent for a new static comparer.

Manifest IDs are opaque string keys (schemaVersion.userVersion.publisher.contentType.contentName) matched by exact string equality, so their numeric userVersion segment must remain byte-for-byte stable for already-installed content to remain addressable.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement a single, authoritative static comparer/parser for Generals Online
version strings and validate it with regression tests before any consumer is
migrated.

- Add a static utility class (e.g. `GeneralsOnlineVersionComparer`) under
`GenHub/GenHub.Core/Helpers/`, following the existing `VersionHelper`
static-helper convention (no DI).
- Implement `TryParse` to decompose a version string of the shape
`MMDDYY_QFE#[_TAG]` into a structured value containing explicit year, month,
day, and QFE integer components: split on the `_QFE` separator (reference
`GeneralsOnlineConstants` for this separator), decode `MMDDYY` into explicit
month/day/year (never as a raw integer), and strip/ignore any trailing `_TAG`
suffix (e.g. `_EAC`) so tags never affect precedence.
- Implement comparison entry points: a `Compare(string a, string b)` and a
convenience `IsNewer(string latest, string? current)`, ordering by year, then
month, then day, then QFE numerically.
- Use `CultureInfo.InvariantCulture` / invariant `NumberStyles` for all numeric
parsing to guarantee culture-invariance.
- Define deterministic malformed-input semantics: unparseable versions sort
before valid ones; null/empty is treated as "no version" (any valid version is
newer than it); releases equal except for build tag compare as equal.
- Do not let this utility introduce any new comparison behavior into the
manifest-ID encoding path — it is purely for ordering.
- Add a test class under `GenHub/GenHub.Tests/GenHub.Tests.Core/` mirroring the
source location, using `[Theory]`/`[InlineData]`, Arrange/Act/Assert structure,
XML doc comments, and no Moq (pure static helper). Cover: cross-year ordering
(e.g. `121525_QFE1` older than `060526_QFE1`), numeric QFE comparison (`_QFE2` >
`_QFE10` handled numerically not lexically), build-tag equivalence
(`060526_QFE1` == `060526_QFE1_EAC`), malformed-vs-valid ordering, null/empty
handling, and culture-invariance (parsing under a non-invariant culture).
===============================================================================

Task: 2

Replace the scattered, inconsistent version-comparison logic across update
checks, installed-version selection, and Downloads UI with calls to the Phase 1
shared comparer so all paths agree on which Generals Online version is newest.

- In
`GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineUpdateService.cs`:
delete the private `ParseVersion` and `IsNewerVersion` methods and call the
shared comparer instead; feed its results into the existing (unchanged)
`ContentUpdateCheckResult` factory methods — do not change the interface or
result shape.
- In the same service, change `GetInstalledVersionAsync` from using
`FirstOrDefault` to selecting the *newest* installed Generals Online manifest by
ordering the matching manifests with the shared comparer, so the correct newest
manifest is chosen when multiple exist.
- In `GenHub/GenHub/Features/Downloads/ViewModels/DownloadsViewModel.cs` (and
any variant-selection logic in `PublisherCardViewModel.cs`), update the Generals
Online population path so that when multiple releases/variants are present, the
"latest" label/selection is chosen via the shared comparer rather than
`FirstOrDefault` or alphabetical/`LastUpdated` heuristics. Keep this change
scoped strictly to Generals Online version selection — do not add new badge UI
beyond aligning the existing version-label selection with the shared ordering.
- Audit Generals Online installed-manifest selection used by
reconciliation-adjacent logic (e.g. `GeneralsOnlineProvider`'s already-installed
lookup and any other place that picks a "current" GO manifest). Where a
newest-of-many decision is made, delegate it to the shared comparer; leave
exact-match lookups by version string unchanged.
===============================================================================

Task: 3

Harden the manifest-ID version parsing so it tolerates build tags while keeping
the numeric encoding byte-for-byte identical for existing versions, preserving
addressability of already-installed content with no migration required.

- In
`GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineManifestFactory.cs`,
update `ParseVersionForManifestId` so a trailing `_TAG` (e.g. `_EAC`) is
stripped before extracting the QFE number, ensuring tagged versions still
produce a valid non-negative integer instead of falling back to `0`.
- Explicitly preserve the current `(dateValue * 10) + qfeValue` formula for the
existing tag-less `MMDDYY_QFE#` shape so previously-generated manifest IDs are
reproduced identically — no migration of existing manifest IDs.
- Do not reuse this method for any ordering decisions; it must remain strictly
ID-encoding-only, separate from the Phase 1 comparer.

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!