Skip to content

feat(validation): integrate CSV pipeline and language detection with GameInstallationValidator - #426

Merged
undead2146 merged 7 commits into
developmentfrom
feat/csv-validator-integration
Aug 30, 2026
Merged

feat(validation): integrate CSV pipeline and language detection with GameInstallationValidator#426
undead2146 merged 7 commits into
developmentfrom
feat/csv-validator-integration

Conversation

@undead2146

Copy link
Copy Markdown
Member

Summary

Integrates the multi-language CSV content pipeline (\CsvContentProvider, \CsvDiscoverer, \CsvResolver) and \LanguageDetector\ with \GameInstallationValidator, enabling manifest-driven validation of Command & Conquer: Generals and Zero Hour installations across all 10 supported languages.

Closes #145
Closes #146
Closes #147

Motivation

Vanilla game installations (v1.08 Generals / v1.04 Zero Hour) across various distributors (Steam, EA App, CD/ISO) contain language-specific assets (\Data//\ directories, localized BIG archives such as \German.big\ or \GermanZH.big) as well as shared assets. Previously, \GameInstallationValidator\ relied on static manifest providers and did not support multi-language auto-detection, CSV registry lookup, or detailed granular issue counts.

Changes

  • Core / Models:
    • Enhanced \ValidationResult\ with \TotalFilesValidated\ and computed helper properties (\MissingFilesCount, \CorruptedFilesCount, \ExtraFilesCount).
    • Added \ValidateInstallationAsync\ and language-specific \ValidateAsync\ overloads to \IGameInstallationValidator.
  • Validation:
    • Refactored \GameInstallationValidator\ to inject \ILanguageDetector\ and \CsvContentProvider, orchestrating target discovery, language auto-detection, CSV manifest resolution, and full file integrity verification with fallback to \IManifestProvider.
    • Standardized progress reporting across all validation phases.
  • Language Detection:
    • Updated \LanguageDetector\ with cross-platform path combiners, centralized constants (\LanguageDirectoryNames, \LanguageFilePatterns, \CsvConstants), and uppercase normalization across all 10 languages (\EN, \DE, \FR, \ES, \IT, \KO, \PL, \PT-BR, \ZH-CN, \ZH-TW).
  • Dependency Injection:
    • Registered \ILanguageDetector, LanguageDetector\ in \GameInstallationModule.
    • Registered concrete \CsvContentProvider\ alongside \IContentProvider\ in \ContentPipelineModule.
  • Tests:
    • Added \LanguageDetectorTests.cs\ covering directory patterns, BIG file markers, Zero Hour suffixes, and fallback behavior.
    • Expanded \GameInstallationValidatorTests.cs\ covering CSV provider integration, explicit language overrides, direct path validation, detailed issue counts, provider failure error handling, and cancellation tokens.
  • Documentation:
    • Added \docs/features/content/csv-validation.md\ covering architecture, pipeline flow mermaid diagram, components, metrics, supported language matrix, and DI registrations.

Verification

  • Targeted unit tests executed and passing (\69 / 69\ in \GameInstallationValidatorTests\ & \LanguageDetectorTests)
  • Full core test suite passing (\2,309 / 2,309\ tests in \GenHub.Tests.Core.dll)
  • Solution builds cleanly with 0 errors (\scripts\build-check.ps1)
  • Verified cross-platform compatibility where applicable

Created with Claude 3.7 Sonnet via Antigravity

@qodo-code-review

Copy link
Copy Markdown

ⓘ Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Approval pending

CodeRabbit has no unresolved comments, but it has not reviewed the latest commit.

Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Integrate language-aware CSV installation validation

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Integrates language-aware CSV manifests into installation validation with legacy provider
 fallback.
• Supports direct-path, multi-target validation and detailed file issue metrics.
• Adds DI wiring, comprehensive pipeline tests, and architecture documentation.
Diagram

sequenceDiagram
    actor Install as Game Installation
    participant Validator as Installation Validator
    participant Language as Language Detector
    participant Provider as CSV Provider
    participant Discoverer as CSV Discoverer
    participant Resolver as CSV Resolver
    participant Content as Content Validator
    participant Result as Validation Result
    Install->>Validator: Validate target
    Validator->>Language: Detect language
    Language-->>Validator: Normalized code
    Validator->>Provider: Search manifest
    Provider->>Discoverer: Discover catalog
    Discoverer-->>Provider: Catalog result
    Provider->>Resolver: Resolve CSV
    Resolver-->>Provider: Content manifest
    Provider-->>Validator: Resolved manifest
    Validator->>Content: Validate files
    Content-->>Validator: Validation issues
    Validator->>Result: Aggregate metrics
    Result-->>Install: Return outcome
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Dedicated manifest resolver abstraction
  • ➕ Decouples installation validation from the concrete CSV provider.
  • ➕ Centralizes provider selection and fallback policy.
  • ➕ Simplifies adding future manifest sources.
  • ➖ Introduces another interface and orchestration layer.
  • ➖ Requires broader DI and test refactoring for limited immediate benefit.
2. Parse CSV directly in validator
  • ➕ Reduces the number of runtime collaborators.
  • ➕ Makes the validation path explicit in one component.
  • ➖ Duplicates content-pipeline behavior.
  • ➖ Tightly couples validation to catalog transport and format.
  • ➖ Weakens reuse, security boundaries, and independent testability.

Recommendation: Reusing the existing content pipeline with an IManifestProvider fallback is the best near-term approach because discovery, resolution, and validation remain independently testable. If additional installation-manifest sources are added, introduce a dedicated manifest resolver abstraction so GameInstallationValidator no longer needs concrete CsvContentProvider awareness or source-name selection.

Files changed (15) +2359 / -104

Enhancement (6) +840 / -104
LanguageDetector.csMake language detection normalized and cross-platform +92/-67

Make language detection normalized and cross-platform

• Replaces inline path and language literals with centralized constants, combines relative paths portably, and normalizes all detected language codes. It also honors cancellation, handles invalid paths, and tolerates inaccessible wildcard searches.

GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs

IGameInstallationValidator.csExpose language-aware and direct-path validation APIs +31/-1

Expose language-aware and direct-path validation APIs

• Adds an explicit-language validation overload and a path/game-type entry point while preserving progress and cancellation support.

GenHub/GenHub.Core/Interfaces/Validation/IGameInstallationValidator.cs

ValidationResult.csAdd granular installation validation metrics +13/-1

Add granular installation validation metrics

• Tracks the total manifest files validated and computes missing, corrupted or size-mismatched, and unexpected file counts from validation issues.

GenHub/GenHub.Core/Models/Results/Validation/ValidationResult.cs

CsvContentProvider.csAdd CSV catalog content provider facade +99/-0

Add CSV catalog content provider facade

• Introduces a BaseContentProvider implementation that binds the CSV discoverer and resolver to the HTTP deliverer. It supports exact manifest retrieval and no-op preparation for catalog-backed installation manifests.

GenHub/GenHub/Features/Content/Services/ContentProviders/CsvContentProvider.cs

CsvResolver.csResolve secure language-filtered manifests from CSV catalogs +334/-0

Resolve secure language-filtered manifests from CSV catalogs

• Loads local or HTTP CSV catalogs, filters rows by game and language, rejects unsafe relative paths, and builds typed installation manifests. It classifies local, downloadable, and installation-resident files while propagating cancellation and reporting resolution failures.

GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs

GameInstallationValidator.csOrchestrate multilingual CSV manifest validation +271/-35

Orchestrate multilingual CSV manifest validation

• Refactors installation validation to process Generals and Zero Hour targets, auto-detect or accept language, query the CSV provider, and fall back to IManifestProvider. It adds direct-path validation, consistent progress and cancellation handling, contextual errors, elapsed timing, and aggregated file counts.

GenHub/GenHub/Features/Validation/GameInstallationValidator.cs

Tests (4) +1369 / -0
CsvContentProviderTests.csCover CSV provider orchestration and failure modes +388/-0

Cover CSV provider orchestration and failure modes

• Tests dependency selection, provider metadata, discovery-resolution coordination, exact content ID lookup, invalid requests, and content preparation.

GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Content/CsvContentProviderTests.cs

CsvResolverTests.csCover CSV loading, filtering, and manifest generation +399/-0

Cover CSV loading, filtering, and manifest generation

• Tests local and remote catalogs, game and language filtering, cancellation, network failures, unsafe path rejection, source classification, and resolver overload behavior.

GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Content/CsvResolverTests.cs

LanguageDetectorTests.csCover multilingual installation marker detection +179/-0

Cover multilingual installation marker detection

• Adds cross-platform tests for invalid paths, cancellation, language directories, standard and Zero Hour BIG archives, all supported languages, and English fallback.

GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameInstallations/LanguageDetectorTests.cs

GameInstallationValidatorTests.csCover CSV-backed multilingual installation validation +403/-0

Cover CSV-backed multilingual installation validation

• Expands validator coverage for automatic and explicit languages, direct paths, all supported language codes, detailed issue metrics, provider failures, cancellation-related behavior, and null arguments.

GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Validation/GameInstallationValidatorTests.cs

Documentation (2) +130 / -0
csv-validation.mdDocument the CSV validation pipeline +129/-0

Document the CSV validation pipeline

• Documents architecture, component responsibilities, validation metrics, the ten-language matrix, and required dependency injection registrations for Generals and Zero Hour validation.

docs/features/content/csv-validation.md

index.mdLink CSV validation documentation +1/-0

Link CSV validation documentation

• Adds the CSV validation pipeline guide to the content documentation index.

docs/features/content/index.md

Other (3) +20 / -0
CsvConstants.csAdd CSV provider identity constants +10/-0

Add CSV provider identity constants

• Adds source-name and description constants for the CSV catalog content provider, complementing existing discoverer and resolver identifiers.

GenHub/GenHub.Core/Constants/CsvConstants.cs

ContentPipelineModule.csRegister CSV provider and resolver pipeline services +8/-0

Register CSV provider and resolver pipeline services

• Registers concrete and interface mappings for CsvContentProvider and CsvResolver alongside the existing CSV discoverer.

GenHub/GenHub/Infrastructure/DependencyInjection/ContentPipelineModule.cs

GameInstallationModule.csRegister the language detector service +2/-0

Register the language detector service

• Adds the singleton ILanguageDetector-to-LanguageDetector mapping used by installation validation.

GenHub/GenHub/Infrastructure/DependencyInjection/GameInstallationModule.cs

@deepsource-io

deepsource-io Bot commented Aug 30, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 1f48230...ae90cba on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
C# Aug 30, 2026 2:33p.m. Review ↗
JavaScript Aug 30, 2026 2:33p.m. Review ↗
Shell Aug 30, 2026 2:33p.m. Review ↗
Secrets Aug 30, 2026 2:33p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

Comment thread GenHub/GenHub/Features/Validation/GameInstallationValidator.cs Outdated
…GameInstallationValidator

Integrate CsvContentProvider and LanguageDetector into GameInstallationValidator to enable manifest-driven multi-language game installation validation.

- Enhance ValidationResult with detailed missing, corrupted, and extra file counts
- Add ValidateInstallationAsync and language-aware ValidateAsync overloads to IGameInstallationValidator
- Refactor LanguageDetector to use cross-platform paths and centralized constants
- Register ILanguageDetector in GameInstallationModule and CsvContentProvider in ContentPipelineModule
- Add comprehensive unit tests in LanguageDetectorTests and GameInstallationValidatorTests
- Add CSV validation pipeline architecture documentation

Closes #145, closes #146, closes #147
@undead2146
undead2146 force-pushed the feat/csv-validator-integration branch from ebb6584 to 0bf39ea Compare August 30, 2026 13:04
@qodo-code-review

qodo-code-review Bot commented Aug 30, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (9) 📜 Skill insights (0)

Grey Divider


Action required

1. Validation overloads are ambiguous ✓ Resolved 🐞 Bug ≡ Correctness
Description
The new language overload makes existing calls such as ValidateAsync(installation, null, default)
ambiguous because null can bind to either string? or IProgress<ValidationProgress>?, so the
current test project and existing consumers fail to compile. This is a source-breaking API change
present at seven call sites in GameInstallationValidatorTests.
Code

GenHub/GenHub.Core/Interfaces/Validation/IGameInstallationValidator.cs[41]

+    Task<ValidationResult> ValidateAsync(GameInstallation installation, string? language, IProgress<ValidationProgress>? progress = null, CancellationToken cancellationToken = default);
Relevance

●●● Strong

This is a concrete source-breaking overload-resolution defect affecting existing null call sites.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The interface now exposes unrelated IProgress<ValidationProgress>? and string? second
parameters, while repository tests pass null in that position with a third argument; neither
nullable annotation distinguishes overload resolution.

GenHub/GenHub.Core/Interfaces/Validation/IGameInstallationValidator.cs[31-41]
GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[63-80]
GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Validation/GameInstallationValidatorTests.cs[146-157]
GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Validation/GameInstallationValidatorTests.cs[223-234]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The added language overload conflicts with the existing progress overload whenever callers pass an untyped `null`, causing compile-time ambiguity.

## Issue Context
Preserve source compatibility by giving the language-specific operation a distinct name/signature or otherwise ensuring existing `ValidateAsync(installation, null, token)` calls have only one applicable overload. Update the implementation and tests consistently.

## Fix Focus Areas
- GenHub/GenHub.Core/Interfaces/Validation/IGameInstallationValidator.cs[31-41]
- GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[63-80]
- GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Validation/GameInstallationValidatorTests.cs[157-157]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Fallback preserves CSV failure ✓ Resolved 🐞 Bug ≡ Correctness
Description
ResolveManifestFromCsvProviderAsync adds an error before returning null, and
ValidateInstallationCoreAsync then uses IManifestProvider without removing that provisional
issue. A successful fallback manifest is therefore still reported as an invalid installation solely
because the preferred CSV source was unavailable.
Code

GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[R307-312]

+                issues.Add(new ValidationIssue
+                {
+                    IssueType = ValidationIssueType.MissingFile,
+                    Path = installationPath,
+                    Message = $"No CSV manifest found for {gameType} ({language}): {searchResult.FirstError ?? "No matching catalog entries"}",
+                    Severity = ValidationSeverity.Error,
Relevance

●●● Strong

Recent provider-fallback precedents accept preserving correct failure semantics and avoiding
misleading fallback results.

PR-#213
PR-#425

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
CSV resolution appends an Error-severity issue and returns null; the caller then accepts a fallback
manifest and continues to return the same accumulated issue list, whose Error severity makes
ValidationResult unsuccessful.

GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[203-235]
GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[298-314]
GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[326-352]
GenHub/GenHub.Core/Models/Results/Validation/ValidationResult.cs[42-50]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
CSV lookup failures are immediately appended as validation errors even though a legacy manifest fallback may subsequently succeed.

## Issue Context
Keep CSV diagnostics separate while attempting fallback. Add a final manifest-resolution error only when every source fails; if fallback succeeds, log the CSV failure rather than including it in the returned validation issues.

## Fix Focus Areas
- GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[203-235]
- GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[298-352]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Fallback resolves wrong target ✓ Resolved 🐞 Bug ≡ Correctness
Description
For each per-game target, the fallback passes the original multi-game GameInstallation instead of
a target-specific installation. ManifestProvider chooses Zero Hour whenever HasZeroHour is true,
so the Generals iteration of a combined installation can receive and validate the Zero Hour manifest
against the Generals path.
Code

GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[R216-217]

+            var targetInstall = installation ?? new GameInstallation(installationPath, GameInstallationType.Unknown, null);
+            manifest = await manifestProvider.GetManifestAsync(targetInstall, cancellationToken);
Relevance

●● Moderate

This is a plausible target-selection correctness bug, but no close historical precedent confirms
acceptance.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The validator creates distinct target tuples but forwards the unchanged parent installation to
fallback. ManifestProvider derives its game type exclusively from installation.HasZeroHour, and
later derives the source path from that same choice.

GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[90-122]
GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[213-218]
GenHub/GenHub/Features/Manifest/ManifestProvider.cs[176-189]
GenHub/GenHub/Features/Manifest/ManifestProvider.cs[230-266]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Manifest fallback ignores the current target path and game type when validating a combined Generals/Zero Hour installation.

## Issue Context
Construct or adapt a `GameInstallation` whose path/capability represents only the current `gameType`, or extend the manifest-provider API to accept the explicit target game and path. Ensure Generals and Zero Hour fallback calls resolve independently.

## Fix Focus Areas
- GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[90-122]
- GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[213-218]
- GenHub/GenHub/Features/Manifest/ManifestProvider.cs[176-189]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. ResolveAsync catches generic exceptions 📘 Rule violation ≡ Correctness
Description
ResolveAsync catches Exception in a non-boundary service and converts every unexpected failure
into a normal failure result. This can mask programming defects that should propagate while only
known recoverable exceptions are handled.
Code

GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[R99-102]

+        catch (Exception ex)
+        {
+            logger.LogError(ex, "Failed to resolve CSV catalog manifest from {SourceUrl}", discoveredItem.SourceUrl);
+            return OperationResult<ContentManifest>.CreateFailure($"Resolution failed: {ex.Message}");
Relevance

●●● Strong

Recent review feedback accepts narrowing generic catches and preserving cancellation or unexpected
failures.

PR-#399
PR-#385

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001350 permits generic catches only at explicit process boundaries or when immediately
rethrowing. This resolver catches the base exception, logs it, and returns a failure result instead.

Rule 3001350: Avoid catching generic Exception except at explicit process boundaries
GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[95-103]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ResolveAsync` catches generic `Exception` and suppresses unexpected failures.

## Issue Context
Preserve result-based handling for expected CSV, HTTP, and file failures, but allow programming and invariant failures to propagate. Continue to preserve cooperative cancellation.

## Fix Focus Areas
- GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[95-103]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. sourceType lacks initializer 📘 Rule violation ≡ Correctness
Description
CreateManifestFile declares sourceType without assigning it at declaration. This violates the
required local-variable initialization style even though later branches assign it.
Code

GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[227]

+        ContentSourceType sourceType;
Relevance

●●● Strong

Recent precedents accept declaration-initialization findings, including locals assigned across loops
or branches.

PR-#285
PR-#406

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001343 requires every local variable to be assigned at its declaration. sourceType is
declared first and assigned only in subsequent branches.

Rule 3001343: Initialize and use all local variables; use discards for intentionally unused values
GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[217-239]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The local `sourceType` is declared without an initializer.

## Issue Context
Restructure the branch as a switch/conditional expression or assign a valid initial value when declaring the local.

## Fix Focus Areas
- GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[217-239]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. LanguageDetector static helper misplaced 📘 Rule violation ⚙ Maintainability
Description
The new static CombineRelativePath helper appears after the instance DetectAsync method. Static
methods must precede instance methods under the required member ordering.
Code

GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs[R163-164]

+    private static string CombineRelativePath(string basePath, string relativePath)
+    {
Relevance

●●● Strong

Static-before-instance member ordering has repeatedly been accepted as a StyleCop fix.

PR-#250
PR-#332

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001370 mandates static-before-instance ordering in the methods group. CombineRelativePath is
static but follows the instance DetectAsync implementation.

Rule 3001370: Enforce StyleCop-compliant member ordering in C# classes
GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs[21-167]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The static path helper is declared after an instance method.

## Issue Context
Reorder the methods so static methods precede instance methods as required by the checklist.

## Fix Focus Areas
- GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs[21-167]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (8)
7. detectedLanguage lacks initializer ✓ Resolved 📘 Rule violation ≡ Correctness
Description
ValidateInstallationCoreAsync declares detectedLanguage without an initializer and assigns it
later in conditional branches. The checklist requires initialization at declaration.
Code

GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[181]

+        string detectedLanguage;
Relevance

●●● Strong

Recent precedents accept declaration-initialization findings even when later control flow assigns
the variable.

PR-#285
PR-#406

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001343 explicitly fails locals declared without an initializer. The value is declared on line
181 and assigned only inside the following if/else.

Rule 3001343: Initialize and use all local variables; use discards for intentionally unused values
GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[181-191]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The local `detectedLanguage` is declared without an initializer.

## Issue Context
Use a conditional expression, or otherwise initialize the local on its declaration while retaining asynchronous detection and cancellation propagation.

## Fix Focus Areas
- GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[181-191]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. INIZH.big duplicates existing constant ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The Zero Hour language mapping hardcodes INIZH.big even though
GameClientConstants.ZeroHourIniBig already defines that filename. Keeping duplicate path literals
risks inconsistent updates.
Code

GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs[116]

+            ("INIZH.big", CsvConstants.LanguageEn),
Relevance

●●● Strong

Centralizing duplicated business filenames matches accepted constants-policy feedback in this
repository.

PR-#221
PR-#134

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001297 requires business-meaningful paths and filenames to use centralized constants. The
changed mapping contains INIZH.big, while the repository already exposes the identical value as
GameClientConstants.ZeroHourIniBig.

Rule 3001297: Avoid inline hardcoded paths and use centralized constants
GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs[112-125]
GenHub/GenHub.Core/Constants/GameClientConstants.cs[69-70]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The language detector duplicates the centralized `INIZH.big` filename.

## Issue Context
Use `GameClientConstants.ZeroHourIniBig` so archive names remain centralized.

## Fix Focus Areas
- GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs[116-116]
- GenHub/GenHub.Core/Constants/GameClientConstants.cs[69-70]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. Default version remains hardcoded 📘 Rule violation ⚙ Maintainability
Description
GetVersionString embeds the business default version 1.0 directly in resolver logic. The default
should be centralized to avoid divergence across catalog producers and consumers.
Code

GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[147]

+        return !string.IsNullOrWhiteSpace(item.Version) ? item.Version : "1.0";
Relevance

●●● Strong

Replacing an inline default version with a centralized constant matches accepted repository
precedent.

PR-#134
PR-#424

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001297 requires business-meaningful magic values to be centralized. The resolver uses an
inline version fallback instead of a CsvConstants member.

Rule 3001297: Avoid inline hardcoded paths and use centralized constants
GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[140-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The resolver hardcodes the default catalog version `1.0`.

## Issue Context
Define the default in the appropriate constants class and reference it from resolver logic.

## Fix Focus Areas
- GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[140-147]
- GenHub/GenHub.Core/Constants/CsvConstants.cs[1-80]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


10. Manifest lookup masks all exceptions 📘 Rule violation ≡ Correctness
Description
ResolveManifestFromCsvProviderAsync catches every Exception, appends a validation issue, and
continues normal fallback behavior. Unexpected implementation defects are therefore
indistinguishable from expected provider failures.
Code

GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[R342-343]

+        catch (Exception ex)
+        {
Relevance

●●● Strong

The repository repeatedly accepts findings that narrow broad exception handling or prevent silently
swallowed failures.

PR-#250
PR-#366
PR-#385

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001350 disallows suppressing generic exceptions outside process boundaries. The helper catches
Exception, adds a domain issue, and returns null rather than rethrowing.

Rule 3001350: Avoid catching generic Exception except at explicit process boundaries
GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[338-353]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The CSV manifest lookup catches and suppresses generic `Exception` in service code.

## Issue Context
Translate only known provider, I/O, and parsing failures into validation issues; preserve cancellation and let unexpected defects propagate.

## Fix Focus Areas
- GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[338-353]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


11. CsvResolver static methods misplaced 📘 Rule violation ⚙ Maintainability
Description
Private static helper methods begin after public instance ResolveAsync methods. The required
member order places all static methods before all instance methods.
Code

GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[115]

+    private static string GetGameTypeString(ContentSearchResult item)
Relevance

●●● Strong

Static helper ordering has been explicitly accepted in comparable content-service and test classes.

PR-#250
PR-#332

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001370 requires static methods to precede instance methods. The instance overloads occupy
lines 42-113, followed by the first static helper at line 115.

Rule 3001370: Enforce StyleCop-compliant member ordering in C# classes
GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[42-115]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CsvResolver` declares static helper methods after instance methods.

## Issue Context
Within the methods group, place all static methods before all instance methods while retaining accessibility ordering within each subgroup.

## Fix Focus Areas
- GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[42-334]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


12. Provider usings are unsorted 📘 Rule violation ⚙ Maintainability
Description
The new provider places System.Threading.Tasks before GenHub.Core using directives, which is not
ordinal alphabetical order. All regular using directives must be sorted by their fully qualified
names.
Code

GenHub/GenHub/Features/Content/Services/ContentProviders/CsvContentProvider.cs[R5-7]

+using System.Threading.Tasks;
+using GenHub.Core.Constants;
+using GenHub.Core.Interfaces.Content;
Evidence
Rule 3001335 requires ordinal alphabetical ordering. GenHub.Core.Constants sorts before
System.Threading.Tasks, but appears after it.

Rule 3001335: Place and alphabetize using directives at the top of C# files
GenHub/GenHub/Features/Content/Services/ContentProviders/CsvContentProvider.cs[1-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Regular using directives are not in ordinal alphabetical order.

## Issue Context
Sort the complete regular-using group by fully qualified namespace.

## Fix Focus Areas
- GenHub/GenHub/Features/Content/Services/ContentProviders/CsvContentProvider.cs[1-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


13. Failed files counted validated ✓ Resolved 🐞 Bug ◔ Observability
Description
After ValidateAllAsync throws and is converted into an error result, TotalFilesValidated is
still set to the full manifest file count. This makes the new metric claim that every file was
validated even when validation aborted before completing them.
Code

GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[R277-279]

+        stopwatch.Stop();
+        var totalFiles = manifest.Files?.Count ?? 0;
+        return new ValidationResult(installationPath, issues, stopwatch.Elapsed, totalFiles);
Relevance

●● Moderate

Metric accuracy concerns are plausible, but history lacks a close precedent for partial validation
counts.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The generic exception handler records that content validation failed but execution continues;
immediately afterward the code derives the metric solely from manifest.Files.Count, not from
validation progress or completion.

GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[242-266]
GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[275-279]
GenHub/GenHub.Core/Models/Results/Validation/ValidationResult.cs[20-21]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TotalFilesValidated` is populated from manifest size rather than completed validation work, including after the validation operation throws.

## Issue Context
Use a completed/processed count supplied by the content validator or progress reporting. At minimum, do not report the full manifest count on the exception path.

## Fix Focus Areas
- GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[242-279]
- GenHub/GenHub.Core/Models/Results/Validation/ValidationResult.cs[20-21]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


14. Validator usings are unsorted ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
New GenHub namespaces are inserted after the System namespace group. The checklist requires a
single regular-using group sorted ordinally by fully qualified namespace.
Code

GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[R8-9]

+using GenHub.Core.Constants;
+using GenHub.Core.Features.GameInstallations;
Relevance

●● Moderate

Exact using-order feedback was rejected, but that precedent is old and newer style findings are
often accepted.

PR-#133
PR-#278

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001335 requires ordinal alphabetical ordering. GenHub.Core.Constants sorts before System,
but is added after all System directives.

Rule 3001335: Place and alphabetize using directives at the top of C# files
GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[1-21]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The validator's regular using directives are not ordinally alphabetized.

## Issue Context
Sort all `GenHub`, `Microsoft`, and `System` directives by their complete namespace names.

## Fix Focus Areas
- GenHub/GenHub/Features/Validation/GameInstallationValidator.cs[1-21]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

15. Resolver usings are unsorted 📘 Rule violation ⚙ Maintainability
Description
The new resolver lists System namespaces before CsvHelper and GenHub namespaces. This
contradicts the required ordinal alphabetical ordering for regular using directives.
Code

GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[R8-11]

+using System.Threading.Tasks;
+using CsvHelper;
+using CsvHelper.Configuration;
+using GenHub.Core.Constants;
Relevance

● Weak

Recent precedent rejects changing repository usings to project-before-System ordering; existing
convention is opposite.

PR-#133

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Under ordinal ordering, CsvHelper and GenHub precede System; the new file puts all System
directives first.

Rule 3001335: Place and alphabetize using directives at the top of C# files
GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[1-19]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Regular using directives are not in ordinal alphabetical order.

## Issue Context
Sort `CsvHelper`, `GenHub`, `Microsoft`, and `System` namespaces by fully qualified name.

## Fix Focus Areas
- GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[1-19]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


16. Validator interface usings unsorted 📘 Rule violation ⚙ Maintainability
Description
The interface places added System using directives before GenHub.Core directives. This is the
reverse of the checklist's ordinal alphabetical order.
Code

GenHub/GenHub.Core/Interfaces/Validation/IGameInstallationValidator.cs[R1-4]

+using System;
+using System.Threading;
+using System.Threading.Tasks;
+using GenHub.Core.Models.Enums;
Relevance

● Weak

Repository precedent rejects reversing the established System-before-project using order.

PR-#133

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001335 requires alphabetical ordering by fully qualified target. GenHub.Core sorts before
System, but the new System directives occupy the top of the group.

Rule 3001335: Place and alphabetize using directives at the top of C# files
GenHub/GenHub.Core/Interfaces/Validation/IGameInstallationValidator.cs[1-7]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The interface's regular using directives are not ordinally alphabetized.

## Issue Context
Place `GenHub.Core` namespaces before `System` namespaces under ordinal sorting.

## Fix Focus Areas
- GenHub/GenHub.Core/Interfaces/Validation/IGameInstallationValidator.cs[1-7]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


17. Detector usings are unsorted ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The added GenHub.Core using directives are placed after System directives. Ordinal alphabetical
order requires the GenHub directives first.
Code

GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs[R6-7]

+using GenHub.Core.Constants;
+using GenHub.Core.Models.Content;
Relevance

● Weak

Recent repository precedent rejects moving System usings after project usings; current convention
expects System first.

PR-#133

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001335 requires using directives to be alphabetized by fully qualified namespace. The added
GenHub.Core namespaces sort before, but appear after, System.

Rule 3001335: Place and alphabetize using directives at the top of C# files
GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs[1-7]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The detector's regular using directives are not ordinally alphabetized.

## Issue Context
Move the `GenHub.Core` directives before the `System` directives.

## Fix Focus Areas
- GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs[1-7]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 29 rules
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread GenHub/GenHub/Features/Validation/GameInstallationValidator.cs
Comment thread GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs Outdated
Comment thread GenHub/GenHub/Features/Validation/GameInstallationValidator.cs Outdated
Comment thread GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs
Comment thread GenHub/GenHub/Features/Validation/GameInstallationValidator.cs
Comment thread GenHub/GenHub.Core/Interfaces/Validation/IGameInstallationValidator.cs Outdated
Comment thread GenHub/GenHub/Features/Validation/GameInstallationValidator.cs
Comment thread GenHub/GenHub/Features/Validation/GameInstallationValidator.cs Outdated
Comment thread GenHub/GenHub/Features/Validation/GameInstallationValidator.cs
Comment thread GenHub/GenHub/Features/Validation/GameInstallationValidator.cs Outdated
Comment thread GenHub/GenHub/Features/Validation/GameInstallationValidator.cs Outdated
Comment thread GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs Outdated
Comment thread GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs Outdated
@kilo-code-bot

kilo-code-bot Bot commented Aug 30, 2026

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental review of commit ae90cba: both previously reported findings are resolved — IniZHBig was removed in favor of GameClientConstants.ZeroHourIniBig, and ContentValidator now populates TotalFilesValidated, making the guard in GameInstallationValidator live in production. No remaining references to the removed constant, and no new issues introduced.

Files Reviewed (3 files)
  • GenHub/GenHub.Core/Constants/LanguageFilePatterns.cs
  • GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs
  • GenHub/GenHub/Features/Content/Services/ContentValidator.cs
Previous Review Summaries (2 snapshots, latest commit a444b85)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit a444b85)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
GenHub/GenHub/Features/Validation/GameInstallationValidator.cs 268 The TotalFilesValidated > 0 guard is dead code in production: the registered ContentValidator never populates the metric, so the manifest-count fallback always wins and the fix only affects mocked tests.

SUGGESTION

File Line Issue
GenHub/GenHub.Core/Constants/LanguageFilePatterns.cs 231 New IniZHBig constant duplicates the existing GameClientConstants.ZeroHourIniBig ("INIZH.big"), reintroducing the constant-drift risk.
Files Reviewed (3 files)
  • GenHub/GenHub.Core/Constants/LanguageFilePatterns.cs - 1 issue
  • GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs - 0 issues (previous hardcoded-string findings resolved)
  • GenHub/GenHub/Features/Validation/GameInstallationValidator.cs - 1 issue (previous null-logger finding resolved)

Fix these issues in Kilo Cloud

Previous review (commit 5dbc67a)

Status: 4 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 2
Issue Details (click to expand)

WARNING

File Line Issue
GenHub/GenHub/Features/Validation/GameInstallationValidator.cs 267 TotalFilesValidated set to full manifest.Files?.Count rather than files actually validated, overstating the metric on incomplete validations.
GenHub/GenHub/Features/Validation/GameInstallationValidator.cs 218 new GameInstallation(installationPath, ..., null) passes a null logger, risking NullReferenceException if the type logs internally.

SUGGESTION

File Line Issue
GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs 115 Hardcoded "AudioZH.big" should live in LanguageFilePatterns.
GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs 125 Hardcoded "*ZH.big" wildcard pattern should be centralized in LanguageFilePatterns.
Files Reviewed (10 files)
  • GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs - 2 issues
  • GenHub/GenHub.Core/Interfaces/Validation/IGameInstallationValidator.cs - 0 new issues (overload-ambiguity and binary-breaking concerns already flagged by Qodo/DeepSource)
  • GenHub/GenHub.Core/Models/Results/Validation/ValidationResult.cs - 0 issues
  • GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameInstallations/LanguageDetectorTests.cs - 0 new issues (DeepSource async-suffix findings already addressed in commit 5/5)
  • GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Validation/GameInstallationValidatorTests.cs - 0 new issues
  • GenHub/GenHub/Features/Validation/GameInstallationValidator.cs - 2 issues
  • GenHub/GenHub/Infrastructure/DependencyInjection/ContentPipelineModule.cs - 0 issues
  • GenHub/GenHub/Infrastructure/DependencyInjection/GameInstallationModule.cs - 0 issues
  • docs/features/content/csv-validation.md - documentation, skipped
  • docs/features/content/index.md - documentation, skipped

Fix these issues in Kilo Cloud


Reviewed by glm-5.3 · Input: 45K · Output: 8K · Cached: 422.8K

Comment thread GenHub/GenHub/Features/Validation/GameInstallationValidator.cs
Comment thread GenHub/GenHub.Core/Constants/LanguageFilePatterns.cs Outdated
@sonarqubecloud

Copy link
Copy Markdown

@undead2146
undead2146 merged commit bc929be into development Aug 30, 2026
15 checks passed
@undead2146
undead2146 deleted the feat/csv-validator-integration branch August 30, 2026 15:40
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.

Update documentation for CSV validation feature Add unit tests for CSV pipeline components Integrate with GameInstallationValidator

1 participant