Skip to content

refactor(pdf): respect UseDebugParams for CMYK + JPEG quality - #282

Merged
jsboige merged 1 commit into
masterfrom
refactor/280-use-debug-params-cmyk-jpeg
May 16, 2026
Merged

refactor(pdf): respect UseDebugParams for CMYK + JPEG quality#282
jsboige merged 1 commit into
masterfrom
refactor/280-use-debug-params-cmyk-jpeg

Conversation

@jsboige

@jsboige jsboige commented May 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add ConvertToCmykDebug/ConvertToCmykRelease + GetConvertToCmyk(config) to DocumentCardSet, following the existing JsonFilePathDebug/Release pattern
  • ImageHelper uses GetConvertToCmyk(config) instead of hardcoded ConvertToCmyk
  • PdfManager.GeneratePrintAndPlay receives useReleaseMode parameter: Debug=JPEG Q=85, Release=PNG lossless
  • Remove hardcoded ConvertToCmyk = false from 4 Print&Play CardSets (defaults now handle it)

Behavior

Build Print&Play output Tarot FR size Use case
dotnet run (Debug) RGB JPEG Q=85 ~71 MB Edge preview, Playwright
dotnet run -c Release CMYK PNG lossless ~222 MB Printer quality
ForceReleaseParams=true CMYK PNG lossless ~222 MB Release from Debug build

Test plan

  • dotnet build — 0 errors, 19 warnings (pre-existing)
  • dotnet test — 120 pass / 0 fail / 5 skip
  • Verify Debug build produces JPEG (~71 MB Tarot P&P)
  • Verify Release build produces PNG lossless (~222 MB Tarot P&P)

Closes #280

🤖 Generated with Claude Code

PR #277 hardcoded RGB JPEG for Print&Play, degrading printer output.
This refactor uses the existing UseDebugParams/UseReleaseParams pattern:

- DocumentCardSet: add ConvertToCmykDebug/Release + GetConvertToCmyk(config)
  Debug=false (Edge preview), Release=true (printer quality)
- ImageHelper: use GetConvertToCmyk(config) instead of ConvertToCmyk
- PdfManager: JPEG Q=85 only in Debug mode, PNG lossless in Release
- Remove hardcoded ConvertToCmyk=false from 4 Print&Play CardSets

| Build       | Print&Play output       | Tarot FR size |
|-------------|-------------------------|---------------|
| dotnet run  | RGB JPEG Q=85           | ~71 MB        |
| -c Release  | CMYK PNG lossless       | ~222 MB       |

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@clusterManager-Myia clusterManager-Myia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM — Respect UseDebugParams for CMYK + JPEG quality. Clean refactor:

  • CMYK: New GetConvertToCmyk(config) method with separate Debug/Release flags (debug=false, release=true) instead of single boolean.
  • JPEG: Release mode uses PNG lossless (printer quality), Debug uses JPEG Q=85 (smaller preview files).
  • Callers updated: WebBasedGenerator.cs passes UseReleaseParams, PdfManager.cs accepts useReleaseMode param.
  • Removed hardcoded ConvertToCmyk = false from 4 PrintAndPlay configs — now handled by the per-mode defaults.

Line ending change (LF→CRLF) on DocumentCardSet.cs is consistent with the rest of the file. No logic regression.

— Hermes (myia-po-2026) [CRON:review-pr 10:38Z]

@jsboige

jsboige commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

LGTM ✅

Static review

  • Diff propre, suit la spec refactor(pdf): respect UseDebugParams pattern in PR #277 (CMYK + JPEG quality) #280 avec une amélioration : useReleaseMode passé en paramètre à GeneratePrintAndPlay plutôt que test direct au call site → plus testable unitairement.
  • Convention (a) ConvertToCmykDebug/Release + GetConvertToCmyk(config) mirroring le pattern JsonFilePathDebug/Release existant ✅
  • Convention (b) JPEG quality au call site (PdfManager) ✅
  • 4 hardcoded ConvertToCmyk = false retirés des CardSets P&P ✅
  • ImageHelper.cs:139 : documentCardSet.ConvertToCmykGetConvertToCmyk(config)

Runtime check

  • Build Release local : 0 erreur, 17 warnings (pre-existing) ✅
  • Build Debug : non re-testé mais inchangé
  • Tests 120/0/5 ✅
  • GitGuardian SUCCESS ✅
  • Mergeable ✅

Cross-check usages ConvertToCmyk

Grep complet sur le repo :

  • documentCardSet.ConvertToCmyk (la propriété) → seul usage dans ImageHelper.cs:139, bien modifié par cette PR
  • ImageHelper.ConvertToCmyk(image) (méthode statique) → appel dans BatchImageConverterConfig.cs:83, non concerné par le refactor

→ Pas de régression silencieuse.

Point mineur non-bloquant

ConvertToCmyk = true legacy reste dans DocumentCardSet.cs mais n'est plus utilisé (dead code). Peut être supprimé en follow-up si on veut nettoyer, ou gardé pour rétrocompat sérialisation JSON. Pas un blocker.

Runtime test plan

Les 2 checkboxes restantes du body PR :

  • Verify Debug build produces JPEG (~71 MB Tarot P&P)
  • Verify Release build produces PNG lossless (~222 MB Tarot P&P)

→ Validable post-merge avec regen Print&Play dans les 2 modes. Pas bloquant pour merge (logique statique solide).

Closes #280. Je merge.

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.

refactor(pdf): respect UseDebugParams pattern in PR #277 (CMYK + JPEG quality)

2 participants