Skip to content

Include art pack in quick pack-runner defaults#40

Merged
PrzemyslawKlys merged 20 commits intomasterfrom
feature/bench-quick-art-20260127
Jan 27, 2026
Merged

Include art pack in quick pack-runner defaults#40
PrzemyslawKlys merged 20 commits intomasterfrom
feature/bench-quick-art-20260127

Conversation

@PrzemyslawKlys
Copy link
Member

Quick pack-runner runs are already tractable in practice, but the art reliability checks were only included in Full mode by default.

This change adds the art pack to the Quick default set so a normal quick run exercises stylized QR cases out of the box. The hard-art scenarios remain gated to Full mode by the scenario pack definitions.

Verification:

  • dotnet build CodeGlyphX.Benchmarks/CodeGlyphX.Benchmarks.csproj -c Release --framework net8.0
  • dotnet run --project CodeGlyphX.Benchmarks/CodeGlyphX.Benchmarks.csproj -c Release --framework net8.0 -- --pack-runner --mode quick --pack art --iterations 1 --ops-cap 1

Copilot AI review requested due to automatic review settings January 27, 2026 13:01
@claude
Copy link

claude bot commented Jan 27, 2026

Claude finished @PrzemyslawKlys's task —— View job


Pull Request Review ✅

Reviewing PR: Include art pack in quick pack-runner defaults

Review Checklist:

  • Examine core change in QrDecodePackRunner.cs
  • Review new .NET 4.7.2 smoke tests
  • Check benchmark data updates for consistency
  • Assess performance implications of adding art pack to quick mode
  • Review code quality and best practices
  • Check for potential security concerns
  • Evaluate test coverage changes

✅ Code Review Summary

Core Change Assessment

File: /CodeGlyphX.Benchmarks/QrDecodePackRunner.cs:39

  • Change: Added QrDecodeScenarioPacks.Art to QuickDefaultPacks array
  • Impact: Art pack scenarios now run in Quick mode by default
  • Quality: ✅ Clean, minimal change that aligns with stated objective

Performance Implications ⚠️

Found important performance consideration at QrDecodePackRunner.cs:253-255:

if (options.Mode == QrPackMode.Full && string.Equals(scenario.Pack, QrDecodeScenarioPacks.Art, StringComparison.OrdinalIgnoreCase)) {
    targetRuns = Math.Min(targetRuns, 2);
}

Analysis: Art scenarios are deliberately limited to 2 runs in Full mode due to computational expense. Adding to Quick mode may increase benchmark runtime but should remain tractable due to Quick mode's lower iteration defaults (3 iterations vs 5 for Full mode).

Test Coverage ✅

New Files Added:

  1. QrNet472SmokeTests.cs - Excellent comprehensive coverage:

    • ✅ PNG/JPEG decoding roundtrips
    • ✅ Rotation and mirroring scenarios
    • ✅ Noise handling and downscaling
    • ✅ Feature flag validation for .NET 4.7.2
    • ✅ Clean test structure with proper helper methods
  2. QrImageDecoderFallbackTests.cs - Good fallback path coverage:

    • ✅ Tests fallback decoder with large images
    • ✅ Proper test isolation using WithForcedFallback()

Code Quality ✅

  • Consistency: Change follows existing patterns in codebase
  • Readability: Code is clean and well-documented
  • Best Practices: Proper string comparison with StringComparison.OrdinalIgnoreCase
  • Testing: Appropriate test coverage for .NET 4.7.2 scenarios

Security Assessment ✅

  • No security concerns identified
  • Input validation: Existing validation patterns maintained
  • No credential exposure: No secrets or keys present in changes

Benchmark Data Updates ✅

  • Updated benchmark files contain expected performance metrics
  • Data appears consistent with the addition of art pack scenarios
  • Large file size increase (1500+ lines) indicates comprehensive benchmark runs

Recommendation: ✅ APPROVE

This is a well-executed change that achieves its stated goal of including art pack scenarios in Quick mode runs. The performance implications are acceptable given the explicit design for Quick mode to remain tractable. The extensive test coverage additions strengthen the codebase.

Minor suggestion: Consider documenting the performance characteristics of art pack scenarios in code comments or documentation for future maintainers.


Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds the art pack to the Quick mode defaults for the pack-runner benchmark tool, enabling stylized QR code scenarios to be tested in quick runs by default. Previously, the art pack was only available in Full mode.

Changes:

  • Added QrDecodeScenarioPacks.Art to the QuickDefaultPacks array in QrDecodePackRunner.cs

@sonarqubecloud
Copy link

@PrzemyslawKlys PrzemyslawKlys merged commit 3ea9f68 into master Jan 27, 2026
7 checks passed
@PrzemyslawKlys PrzemyslawKlys deleted the feature/bench-quick-art-20260127 branch January 27, 2026 19:24
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.

2 participants