Skip to content

fix(harvest): retry failed sets serially with backoff (#613 Option C) - #676

Merged
jsboige merged 1 commit into
masterfrom
fix/613-retry-serial-harvest
Jul 4, 2026
Merged

fix(harvest): retry failed sets serially with backoff (#613 Option C)#676
jsboige merged 1 commit into
masterfrom
fix/613-retry-serial-harvest

Conversation

@jsboige

@jsboige jsboige commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

What

Closes #613 (Option C — retry serial). After the parallel harvest loop drains (ContinueOnHarvestSetFailure, #614), re-attempt each failed set serially (degree=1) with configurable backoff. A large set that timed out under high parallelism frequently succeeds when it has CardPen/Playwright to itself.

This is the runtime-resilience gap surfaced during the v0.9.0 release regen (cycle where EnableParallelism=true → large sets timed out → Mismatch throw). The first mitigation was #614 (don't abort the whole harvest on one set failure). This PR adds the second mitigation: recover the failed sets automatically before declaring partial failure.

Changes

WebBasedGeneratorConfig.cs (+17) — 2 new config props with XML doc:

  • HarvestSetRetryAttempts (default 1)
  • HarvestSetRetryBackoffSeconds (default 30)

HarvestManager.cs (+102) —

  • Seam (between the Parallel.ForEachAsync drain and the failedSets.IsEmpty check): gated by HarvestSetRetryAttempts > 0, calls RetryFailedHarvestSetsAsync and replaces the bag with the residual.
  • RetryFailedHarvestSetsAsync (private): iterates the failure bag, looks up each CardSetJob by .Name, re-runs ProcessLocalizedHarvest serially per item via the pure RetryAsync helper. Re-renders correctly because a failed set never added its key to the dictionary (ContainsKey guard in ProcessLocalizedHarvest). Returns a residual bag → the existing [HARVEST-PARTIAL] aggregate-error throw then reports only what genuinely could not be harvested.
  • RetryAsync (internal static, pure): runs action × attempts, backoff between, swallows the last exception (logs it) and returns bool. Extracted as a pure helper so the retry/backoff contract is unit-testable without a browser (precedent: ComputeExpectedImageCount).

Design notes

  • Does not touch ParallelismOptimizer — retry is strictly post-loop, orthogonal to parallelism tuning.
  • Reuses the exact same call path (ProcessLocalizedHarvest) — no duplicate harvest logic.
  • Opt-in via HarvestSetRetryAttempts > 0 — defaults are conservative (1 attempt, 30 s backoff); set 0 to disable and recover the current behavior.
  • Serial by construction — no new concurrency concerns (QuestPDF global lock / Playwright page pool unaffected; the parallel loop has already drained).

Verification

  • Build: 0 errors (3 pre-existing warnings SYSLIB0014 + SGEN, unrelated).
  • Tests: 570 pass / 5 skip / 1 known-fail. The single failure is OwlE2EGenerationValidationTests.LoadedOntology_RdfTypeAndInScheme_DroppedByOwl2XmlRoundTrip — pre-existing OWLSharp round-trip bug tracked by feat(ontology): Publish OWL ontology to public endpoint #133, unrelated to harvest. No regression.

Known interaction — #630

Bug #630 (Spectre [HARVEST-FAILURE] fatal) short-circuits #614 on a set-level failure path, which would also bypass this retry. This PR is correct and valuable on its own (it fires whenever #614's ContinueOnHarvestSetFailure path is active — the common case), but a full fix for the fatal-path requires #630 to be resolved separately. Documented here for transparency; not a blocker for this PR.

Garde-fou

Pure pipeline code (.cs), no runtime/2sxc/DNN surface — not subject to the #596 runtime-pending gate. Ready for review.

🤖 Generated with Claude Code

After the parallel harvest loop drains (ContinueOnHarvestSetFailure, #614),
re-attempt each failed set serially (degree=1) with configurable backoff.
A large set that timed out under high parallelism often succeeds when it
has CardPen/Playwright to itself.

- Add HarvestSetRetryAttempts (default 1) + HarvestSetRetryBackoffSeconds
  (default 30) to WebBasedGeneratorConfig.
- Insert RetryFailedHarvestSetsAsync at the seam between the
  Parallel.ForEachAsync drain and the failedSets.IsEmpty check: iterates
  the failure bag, looks up each CardSetJob by name, re-runs
  ProcessLocalizedHarvest serially. Re-renders correctly because a failed
  set never added its key to the dictionary (ContainsKey guard).
- Extract pure RetryAsync helper (action x attempts, backoff between,
  swallows last error + logs) so the retry/backoff contract is unit-
  testable without a browser (precedent: ComputeExpectedImageCount).
- Gated by HarvestSetRetryAttempts > 0; bag is replaced with the residual
  sets that still fail, so the existing aggregate-error path (HARVEST-
  PARTIAL throw) reports only what genuinely could not be harvested.

Build: 0 errors (3 pre-existing warnings SYSLIB0014/SGEN unrelated).
Tests: 570 pass / 5 skip / 1 known-fail (OwlE2E round-trip #133,
pre-existing, unrelated to harvest). No regression.

Refs #613.

Co-Authored-By: Claude-Code <noreply@anthropic.com>
@jsboige
jsboige merged commit d591386 into master Jul 4, 2026
3 checks passed
@jsboige
jsboige deleted the fix/613-retry-serial-harvest branch July 4, 2026 05:09
jsboige added a commit that referenced this pull request Jul 4, 2026
…7) (#678)

The retry-serial fix (#676 / issue #613 Option C) extracted its retry/backoff
contract into the pure, deterministic, internal static HarvestManager.RetryAsync
helper precisely so the contract is unit-testable without a browser (precedent:
ComputeExpectedImageCount). The fix shipped with ZERO unit coverage on that
helper — a gap vs the #613 DoD. This PR closes it.

8 additive cases (5 methods, theories expanded):
- SucceedsOnFirstAttempt → true, invokes once, no delay.
- SucceedsOnNthAttempt (2,3) → true, invokes exactly N times.
- AlwaysFails → false, NEVER throws (swallows last error so the caller's
  residual-bag aggregate-error path can report still-failing sets instead of
  aborting on the first residual failure), invokes `attempts` times.
- BackoffZero → no delay (fast path).
- BackoffPositive → 2 inter-attempt delays applied for 3 attempts.
- AttemptsClampedToAtLeastOne (0,-1) → defensive clamp, invokes exactly once.

Filter run: 8 pass / 0 fail. No warning on the new file (pre-existing
CS8xxx/xUnit1012 warnings in sibling test files are untouched).

Refs #613. Follow-up to #676.

Co-authored-by: Claude-Code <noreply@anthropic.com>
jsboige added a commit that referenced this pull request Jul 4, 2026
…LD rationale (#680)

Dispatch #98vo07 primary (smoke-test retry path #676). Delivers static path
analysis (3 config guards + seam data-flow with exact line refs), the trigger
conditions (including the non-deterministic timeout-under-parallelism), the
rationale for why a runtime smoke-test is HOLD pre-tag (régén HOLD +
non-determinism), the deterministic coverage via RetryAsync unit tests
(companion PR #678), and a post-tag runtime validation plan (capture
[HARVEST-RETRY] logs at next release regen).

Signal, not a verdict — po-2023 reports metrics, ai-01 verdicts runtime.

Refs #613. Companion to #676 (merged) and #678 (RetryAsync unit tests).

Co-authored-by: Claude-Code <noreply@anthropic.com>
jsboige added a commit that referenced this pull request Jul 4, 2026
…tability fixes) (#134) (#689)

The v0.9.0 entry predating bundle v3 missed the print-production and
late-cycle stability work. This refresh brings it in line with master
`21e2c666`, cross-checked against the release-validation dossier v4
(docs/RELEASE-VALIDATION-v0.9.0.md §3.3) and dashboard decisions #26-69.

Added:
- "Print Production (CMYK & Print&Play)" section: Ghostscript post-process
  (#632), `--pdf-cmyk` entry-point (#652), bundle v3 = 80 PDFs DeviceCMYK +
  SWOP OutputIntent (6.18 GB RGB -> 5.30 GB CMYK), P&P Standard/Light
  (#645/#648-650, 64 -> 80 PDFs), GS timeout 180->900s (#670)
- "OWL Ontology (Bilingual EN/FR)" section: Fallacies OWL 5.07 MB (#634
  regen), Virtues OWL (#592/#499 Phase 2), honest scope note (EN+FR only,
  not 8-lang)
- "Fixed - Pipeline Stability (Jun-Jul 2026)" section: harvest deadlock
  #651, serial retry #613/#676, logger Spectre #630/#655, CMYK oxymore,
  Johnny 6.1.3 #653, Rules i18n refonte #640, CSV hygiene #579/#581/#584,
  OWL staleness #634

Corrected (code=truth):
- Magick.NET 14.13.1 -> 14.14.0 (verified in .csproj)
- Test count 548 -> 578 pass / 1 known-fail #133 / 5 skip / 584 total
  (empirical `dotnet test` on Argumentum.AssetConverter.Tests, 2026-07-04,
  .NET 9). Previous counts in docs (548/549/570) were all stale

No existing content regressed; all prior sections preserved.

Co-authored-by: Your <your.email@example.com>
Co-authored-by: Claude-Code <noreply@anthropic.com>
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.

fix(pipeline): ParallelismOptimizer overload aborts release régén on large card sets

1 participant